#580 Cannot create an asset (#634)

* 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

Author: samjsmith

packages/composer-common/lib/factory.js

@@ -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.
@@ -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({
@@ -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()) {
@@ -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();
 
@@ -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;
     }
@@ -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()) {

packages/composer-common/lib/serializer/instancegenerator.js

@@ -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
@@ -80,6 +82,7 @@ class InstanceGenerator {
      * @private
      */
     visitField(field, parameters) {
+
         if (field.isArray()) {
             let result = [];
             for (let i = 0; i < 3; i++) {
@@ -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();
@@ -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

packages/composer-common/lib/serializer/resourcevalidator.js

@@ -395,8 +395,6 @@ class ResourceValidator {
             if(!message) {
                 message = '';
             }
-
-            console.log('[' + callSite + '] ' + message );
         }
     }
 

packages/composer-common/messages/en.json

@@ -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}",
 

packages/composer-common/test/factory.js

@@ -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', () => {

packages/composer-common/test/models/model.js

@@ -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(

packages/composer-common/test/serializer/instancegenerator.js

@@ -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$/);
+            }
+        });
+
+
+
     });
 
 });

packages/composer-playground/src/app/resource/resource.component.html

@@ -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>
@@ -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>

packages/composer-playground/src/app/resource/resource.component.scss

@@ -31,6 +31,7 @@ resource-modal {
 
     .resource-error-text{
       min-height: 30px;
+      color: $error-colour-1;
       max-width: 600px;
     }
   }

packages/composer-playground/src/app/resource/resource.component.spec.ts

@@ -217,7 +217,7 @@ describe('ResourceComponent', () => {
       };
       component['resourceDeclaration'] = mockClassDeclaration;
       component['generateResource']();
-      should.exist(component['defitionError']);
+      should.exist(component['definitionError']);
     });
   });
 
@@ -288,7 +288,7 @@ describe('ResourceComponent', () => {
 
       component['addOrUpdateResource']();
       tick();
-      should.exist(component['defitionError']);
+      should.exist(component['definitionError']);
       component['actionInProgress'].should.be.false;
     }));
   });
@@ -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']);
     });
   });