Skip to content

Commit

Permalink
#580 Cannot create an asset (hyperledger-archives#634)
Browse files Browse the repository at this point in the history
* UI nitpicks 11 and 12

* Order the projects retrieved from GitHub

* Fix unit test

* #417 stop UI from jumping around when updating

* nitpicks 1 and 7 in issue #496

* nitpick 5

* abstract type in model changes
  • Loading branch information
samjsmith authored Apr 6, 2017
1 parent b7a3027 commit edbe5ea
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 40 deletions.
29 changes: 18 additions & 11 deletions packages/composer-common/lib/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,23 @@

const debug = require('debug')('ibm-concerto');
const Globalize = require('./globalize');

const InstanceGenerator = require('./serializer/instancegenerator');
const ValueGeneratorFactory = require('./serializer/valuegenerator');
const Relationship = require('./model/relationship');
const ResourceValidator = require('./serializer/resourcevalidator');
const TypedStack = require('./serializer/typedstack');

const Relationship = require('./model/relationship');
const Resource = require('./model/resource');
const ValidatedResource = require('./model/validatedresource');

const Concept = require('./model/concept');
const ValidatedConcept = require('./model/validatedconcept');

const ResourceValidator = require('./serializer/resourcevalidator');
const TransactionDeclaration = require('./introspect/transactiondeclaration');
const TypedStack = require('./serializer/typedstack');

const uuid = require('uuid');


/**
* Use the Factory to create instances of Resource: transactions, participants
* and assets.
Expand Down Expand Up @@ -84,7 +86,6 @@ class Factory {
* @throws {ModelException} if the type is not registered with the ModelManager
*/
newResource(ns, type, id, options) {

if(!id || typeof(id) !== 'string') {
let formatter = Globalize.messageFormatter('factory-newinstance-invalididentifier');
throw new Error(formatter({
Expand Down Expand Up @@ -119,9 +120,12 @@ class Factory {
}

let classDecl = modelFile.getType(type);

if(classDecl.isAbstract()) {
throw new Error('Cannot create abstract type ' + classDecl.getFullyQualifiedName());
let formatter = Globalize.messageFormatter('factory-newinstance-abstracttype');
throw new Error(formatter({
namespace: ns,
type: type
}));
}

if(classDecl.isConcept()) {
Expand All @@ -131,10 +135,10 @@ class Factory {
let newObj = null;
options = options || {};
if(options.disableValidation) {
newObj = new Resource(this.modelManager,ns,type,id);
newObj = new Resource(this.modelManager, ns, type, id);
}
else {
newObj = new ValidatedResource(this.modelManager,ns,type,id, new ResourceValidator());
newObj = new ValidatedResource(this.modelManager, ns, type, id, new ResourceValidator());
}
newObj.assignFieldDefaults();

Expand All @@ -153,7 +157,6 @@ class Factory {
// if we have an identifier, we set it now
let idField = classDecl.getIdentifierFieldName();
newObj[idField] = id;

debug('Factory.newResource created %s', id );
return newObj;
}
Expand Down Expand Up @@ -191,7 +194,11 @@ class Factory {
let classDecl = modelFile.getType(type);

if(classDecl.isAbstract()) {
throw new Error('Cannot create abstract type ' + classDecl.getFullyQualifiedName());
let formatter = Globalize.messageFormatter('factory-newinstance-abstracttype');
throw new Error(formatter({
namespace: ns,
type: type
}));
}

if(!classDecl.isConcept()) {
Expand Down
58 changes: 55 additions & 3 deletions packages/composer-common/lib/serializer/instancegenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

const ClassDeclaration = require('../introspect/classdeclaration');
const EnumDeclaration = require('../introspect/enumdeclaration');
const Introspector = require('../introspect/introspector');
const Field = require('../introspect/field');
const leftPad = require('left-pad');
const ModelUtil = require('../modelutil');
const RelationshipDeclaration = require('../introspect/relationshipdeclaration');
const Util = require('../util');
const ValueGeneratorFactory = require('./valuegenerator');
const Globalize = require('../globalize');

/**
* Generate sample instance data for the specified class declaration
Expand Down Expand Up @@ -80,6 +82,7 @@ class InstanceGenerator {
* @private
*/
visitField(field, parameters) {

if (field.isArray()) {
let result = [];
for (let i = 0; i < 3; i++) {
Expand Down Expand Up @@ -121,9 +124,17 @@ class InstanceGenerator {
let enumValues = classDeclaration.getOwnProperties();
return enumValues[Math.floor(Math.random() * enumValues.length)].getName();
}
else if (classDeclaration.isConcept()) {
let resource = parameters.factory.newConcept(classDeclaration.getModelFile().getNamespace(), classDeclaration.getName());
parameters.stack.push(resource);
else if (classDeclaration.isAbstract) {
let newClassDecl = this.findExtendingLeafType(classDeclaration);
if(newClassDecl !== null) {
classDeclaration = newClassDecl;
type = newClassDecl.getName();
}
}

if (classDeclaration.isConcept()) {
let concept = parameters.factory.newConcept(classDeclaration.getModelFile().getNamespace(), classDeclaration.getName());
parameters.stack.push(concept);
return classDeclaration.accept(this, parameters);
} else {
let identifierFieldName = classDeclaration.getIdentifierFieldName();
Expand All @@ -136,6 +147,47 @@ class InstanceGenerator {
}
}

/**
* Find a type that extends the provided abstract type and return it.
* TODO: work out whether this has to be a leaf node or whether the closest type can be used
* It depends really since the closest type will satisfy the model but whether it satisfies
* any transaction code which attempts to use the generated resource is another matter.
* @param {any} inputType the class declaration.
* @return {any} the closest extending concrete class definition - null if none are found.
*/
findExtendingLeafType(inputType) {
let modelManager = inputType.getModelFile().getModelManager();
let returnType = null;
if(inputType.isAbstract()) {
let introspector = new Introspector(modelManager);
let allClassDeclarations = introspector.getClassDeclarations();
let contenders = [];
allClassDeclarations.forEach((classDecl) => {
let superType = classDecl.getSuperType();
if(!classDecl.isAbstract() && (superType !== null)) {
if(superType === inputType.getFullyQualifiedName()) {
contenders.push(classDecl);
}
}
});

if(contenders.length > 0) {
returnType = contenders[0];
} else {
let formatter = Globalize.messageFormatter('instancegenerator-newinstance-noconcreteclass');
throw new Error(formatter({
type: inputType.getFullyQualifiedName()
}));
}
} else {
// we haven't been given an abstract type so just return what we were given.
returnType = inputType;
}
return returnType;
}



/**
* Visitor design pattern
* @param {RelationshipDeclaration} relationshipDeclaration - the object being visited
Expand Down
2 changes: 0 additions & 2 deletions packages/composer-common/lib/serializer/resourcevalidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,6 @@ class ResourceValidator {
if(!message) {
message = '';
}

console.log('[' + callSite + '] ' + message );
}
}

Expand Down
3 changes: 3 additions & 0 deletions packages/composer-common/messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@
"factory-newinstance-typenotdeclaredinns": "Type {type} is not declared in namespace {namespace}",
"factory-newinstance-missingidentifier": "Missing identifier for Type {type} in namespace {namespace}",
"factory-newinstance-invalididentifier": "Invalid or missing identifier for Type {type} in namespace {namespace}",
"factory-newinstance-abstracttype": "Cannot instantiate Abstract Type {type} in namespace {namespace}",

"factory-newrelationship-notregisteredwithmm": "ModelFile for namespace {namespace} has not been registered with the ModelManager",
"factory-newrelationship-typenotdeclaredinns": "Type {type} is not declared in namespace {namespace}",

"instancegenerator-newinstance-noconcreteclass": "No concrete extending type for {type}",

"modelmanager-resolvetype-nonsfortype": "No registered namespace for type {type} in {context}",
"modelmanager-resolvetype-notypeinnsforcontext": "No type {type} in namespace {namespace} for {context}",

Expand Down
2 changes: 1 addition & 1 deletion packages/composer-common/test/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ describe('Factory', () => {
it('should throw if concept is abstract', () => {
(() => {
factory.newConcept('org.acme.test', 'AbstractConcept');
}).should.throw(/Cannot create abstract type org.acme.test.AbstractConcept/);
}).should.throw(/Cannot instantiate Abstract Type AbstractConcept in namespace org.acme.test/);
});

it('should create a new concept', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/composer-common/test/models/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('Model Tests', function(){
let factory = new Factory(modelManager);

// attempt to create an abstract asset
assert.throws( function() {factory.newResource('org.acme.base', 'AbstractAsset', '123' );}, /.+Cannot create abstract type org.acme.base.AbstractAsset/, 'did not throw with expected message');
assert.throws( function() {factory.newResource('org.acme.base', 'AbstractAsset', '123' );}, /.+Cannot instantiate Abstract Type AbstractAsset in namespace org.acme.base/, 'did not throw with expected message');

// create a new instance
let resource = factory.newResource(
Expand Down
32 changes: 32 additions & 0 deletions packages/composer-common/test/serializer/instancegenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,38 @@ describe('InstanceGenerator', () => {
resource.inheritedValue.should.be.a('string');
});

it('should generate a concrete class for an abstract type if one is available', () => {
let resource = test(`namespace org.acme.test
abstract concept BaseConcept {
o String inheritedValue
}
concept MyConcept extends BaseConcept {
o String concreteConceptValue
}
asset MyAsset identified by id {
o String id
o BaseConcept aConcept
}`);
resource.aConcept.$type.should.match(/^MyConcept$/);
});

it('should throw an error when trying to generate a resource from a model that uses an Abstract type with no concrete Implementing type', () => {
try {
test(`namespace org.acme.test
abstract concept BaseConcept {
o String inheritedValue
}
asset MyAsset identified by id {
o String id
o BaseConcept aConcept
}`);
} catch (error) {
error.should.match(/^Error: No concrete extending type for org.acme.test.BaseConcept$/);
}
});



});

});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ <h2>In registry: <b>{{registryID}}</b></h2>
<div class="resource-bound">
<codemirror [(ngModel)]="resourceDefinition" [config]="codeConfig" (ngModelChange)="onDefinitionChanged()" width="100%" height="100%">
</codemirror>
<div class="resource-error-text" ng-if="defitionError!=null">
<p>{{defitionError}}</p>
<div class="resource-error-text" ng-if="definitionError!=null">
<p>{{definitionError}}</p>
</div>
</div>
</section>
Expand All @@ -29,7 +29,7 @@ <h2>In registry: <b>{{registryID}}</b></h2>
<button type="button" class="secondary" (click)="activeModal.close();">
<span>Cancel</span>
</button>
<button type="button" class="primary" (click)="addOrUpdateResource()" [disabled]="defitionError!=null || actionInProgress ">
<button type="button" class="primary" (click)="addOrUpdateResource()" [disabled]="definitionError!=null || actionInProgress ">
<div *ngIf="!actionInProgress">
<span>{{resourceAction}}</span>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ resource-modal {

.resource-error-text{
min-height: 30px;
color: $error-colour-1;
max-width: 600px;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('ResourceComponent', () => {
};
component['resourceDeclaration'] = mockClassDeclaration;
component['generateResource']();
should.exist(component['defitionError']);
should.exist(component['definitionError']);
});
});

Expand Down Expand Up @@ -288,7 +288,7 @@ describe('ResourceComponent', () => {

component['addOrUpdateResource']();
tick();
should.exist(component['defitionError']);
should.exist(component['definitionError']);
component['actionInProgress'].should.be.false;
}));
});
Expand All @@ -304,13 +304,13 @@ describe('ResourceComponent', () => {
mockSerializer.fromJSON.should.be.called;
mockSerializer.fromJSON.should.be.calledWith({'$class': 'org.acme'});
mockResource.validate.should.be.called;
should.not.exist(component['defitionError']);
should.not.exist(component['definitionError']);
});

it('should set definitionError', () => {
component['resourceDefinition'] = 'will error';
component['onDefinitionChanged']();
should.exist(component['defitionError']);
should.exist(component['definitionError']);
});
});

Expand Down
31 changes: 16 additions & 15 deletions packages/composer-playground/src/app/resource/resource.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class ResourceComponent implements OnInit {
private resourceDefinition: string = null;
private resourceDeclaration: ClassDeclaration = null;
private actionInProgress: boolean = false;
private defitionError: string = null;
private definitionError: string = null;

private codeConfig = {
lineNumbers: true,
Expand Down Expand Up @@ -112,19 +112,20 @@ export class ResourceComponent implements OnInit {
let idx = Math.round(Math.random() * 9999).toString();
idx = leftPad(idx, 4, '0');
let id = `${this.resourceDeclaration.getIdentifierFieldName()}:${idx}`;
let resource = factory.newResource(
this.resourceDeclaration.getModelFile().getNamespace(),
this.resourceDeclaration.getName(),
id,
{ generate: true, 'withSampleData': withSampleData });
let serializer = this.clientService.getBusinessNetwork().getSerializer();
try {
let json = serializer.toJSON(resource);
this.resourceDefinition = JSON.stringify(json, null, 2);
let resource = factory.newResource(
this.resourceDeclaration.getModelFile().getNamespace(),
this.resourceDeclaration.getName(),
id,
{ generate: true, 'withSampleData': withSampleData }
);
let serializer = this.clientService.getBusinessNetwork().getSerializer();
let json = serializer.toJSON(resource);
this.resourceDefinition = JSON.stringify(json, null, 2);
} catch (error) {
// We can't generate a sample instance for some reason.
this.defitionError = error.toString();
this.resourceDefinition = '';
// We can't generate a sample instance for some reason.
this.definitionError = error.toString();
this.resourceDefinition = '';
}
}

Expand Down Expand Up @@ -155,7 +156,7 @@ export class ResourceComponent implements OnInit {
this.activeModal.close();
})
.catch((error) => {
this.defitionError = error.toString();
this.definitionError = error.toString();
this.actionInProgress = false;
});
}
Expand All @@ -170,9 +171,9 @@ export class ResourceComponent implements OnInit {
let serializer = this.clientService.getBusinessNetwork().getSerializer();
let resource = serializer.fromJSON(json);
resource.validate();
this.defitionError = null;
this.definitionError = null;
} catch (e) {
this.defitionError = e.toString();
this.definitionError = e.toString();
}
}

Expand Down

0 comments on commit edbe5ea

Please sign in to comment.