Fixes for issue #949 (#1047)

* Add ClassDeclaration.getSuperTypeDeclaration() function Add additional ClassDeclaration unit test to ensure superclasses are located in both local and remote model files. Refactor (simplify) ModelUtil.isAssignableTo(). * Add ClassDeclaration.getAllSuperTypeDeclarations() Refactor ModelUtil.isAssignableTo() to use this function. Several other places in the code do awkward walking of the superclass hierarchy and these can now be refactored to use this function.

Author: Mark S. Lewis

packages/composer-common/lib/introspect/classdeclaration.js

@@ -356,17 +356,29 @@ class ClassDeclaration {
  * @return {string} the FQN name of the super type or null
  */
  getSuperType() {
- if(this.superType) {
- const type = this.getModelFile().getType(this.superType);
- if(type === null) {
- throw new Error('Could not find super type:' + this.superType );
- }
- else {
- return type.getFullyQualifiedName();
- }
+ const superTypeDeclaration = this.getSuperTypeDeclaration();
+ if (superTypeDeclaration) {
+ return superTypeDeclaration.getFullyQualifiedName();
+ } else {
+ return null;
  }
+ }
 
- return null;
+ /**
+ * Get the super type class declaration for this class.
+ * @return {ClassDeclaration} the super type declaration, or null if there is no super type.
+ */
+ getSuperTypeDeclaration() {
+ if (!this.superType) {
+ return null;
+ }
+
+ const supertypeDeclaration = this.getModelFile().getType(this.superType);
+ if (!supertypeDeclaration) {
+ throw new Error('Could not find super type: ' + this.superType);
+ }
+
+ return supertypeDeclaration;
  }
 
  /**
@@ -407,6 +419,19 @@ class ClassDeclaration {
  return Array.from(results);
  }
 
+ /**
+ * Get all the super-type declarations for this type.
+ * @return {ClassDeclaration[]} super-type declarations.
+ */
+ getAllSuperTypeDeclarations() {
+ const results = [];
+ for (let type = this; (type = type.getSuperTypeDeclaration()); ) {
+ results.push(type);
+ }
+
+ return results;
+ }
+
  /**
  * Returns the property with a given name or null if it does not exist.
  * Fields defined in super-types are also introspected.

packages/composer-common/lib/modelutil.js

@@ -89,64 +89,28 @@ class ModelUtil {
  * Returns true if the type is assignable to the propertyType.
  *
  * @param {ModelFile} modelFile - the ModelFile that owns the Property
- * @param {string} type - the FQN of the type we are trying to assign
+ * @param {string} typeName - the FQN of the type we are trying to assign
  * @param {Property} property - the property that we'd like to store the
  * type in.
  * @return {boolean} - true if the type can be assigned to the property
  * @private
  */
- static isAssignableTo(modelFile, type, property) {
- const propertyType = property.getFullyQualifiedTypeName();
+ static isAssignableTo(modelFile, typeName, property) {
+ const propertyTypeName = property.getFullyQualifiedTypeName();
 
- if (ModelUtil.isPrimitiveType(type) || ModelUtil.isPrimitiveType(propertyType)) {
- return type === propertyType;
+ const isDirectMatch = (typeName === propertyTypeName);
+ if (isDirectMatch || ModelUtil.isPrimitiveType(typeName) || ModelUtil.isPrimitiveType(propertyTypeName)) {
+ return isDirectMatch;
  }
 
- // simple case
- if (type === propertyType) {
- return true;
+ const typeDeclaration = modelFile.getType(typeName);
+ if (!typeDeclaration) {
+ throw new Error('Cannot find type ' + typeName);
  }
 
- // type = SuperType
- // property = BaseType
- // return = true
- const typeDeclaration = modelFile.getType(type);
- if(!typeDeclaration) {
- throw new Error('Cannot find type ' + type );
- }
-
- let superTypeName = typeDeclaration.getSuperType();
- let superType = null;
-
- if(superTypeName) {
- // we cannot assume that the super type is in the same namespace as the derived type!
- superType = modelFile.getModelManager().getType(superTypeName);
-
- if(!superType) {
- throw new Error('Cannot find type ' + superTypeName );
- }
-
- // console.log( 'superTypeName ' + superTypeName );
- // console.log( 'superType ' + superType.getFullyQualifiedName() );
- }
-
- while(superType) {
- if(superType.getFullyQualifiedName() === property.getFullyQualifiedTypeName()) {
- // console.log('Found superType ' + superType.getFullyQualifiedName() );
- return true;
- }
- superTypeName = superType.getSuperType();
-
- if(superTypeName) {
- superType = modelFile.getModelManager().getType(superTypeName);
- // console.log('superType ' + superType.getFullyQualifiedName() );
- }
- else {
- superType = null;
- }
- }
-
- return false;
+ return typeDeclaration.getAllSuperTypeDeclarations().
+ map(type => type.getFullyQualifiedName()).
+ includes(propertyTypeName);
  }
 
  /**
@@ -170,6 +134,7 @@ class ModelUtil {
  const typeDeclaration = modelFile.getType(field.getType());
  return (typeDeclaration !== null && typeDeclaration.isEnum());
  }
+
 }
 
 module.exports = ModelUtil;

packages/composer-common/test/data/parser/classdeclaration.participantwithparents.child.cto

@@ -0,0 +1,7 @@
+namespace com.testing.child
+
+import com.testing.parent.Super
+
+participant Sub extends Super { }
+
+participant Sub2 extends Super { }

packages/composer-common/test/data/parser/classdeclaration.participantwithparents.cto

@@ -0,0 +1,7 @@
+namespace com.testing.parent
+
+abstract participant Base identified by id {
+ o String id
+}
+
+abstract participant Super extends Base { }

packages/composer-common/test/data/parser/classdeclaration.participantwithparents.parent.cto

@@ -35,14 +35,27 @@ describe('ClassDeclaration', () => {
  beforeEach(() => {
  mockModelManager = sinon.createStubInstance(ModelManager);
  mockModelFile = sinon.createStubInstance(ModelFile);
- mockModelFile.getModelManager.returns(mockModelManager);
  });
 
+ /**
+ * Load an arbitrary number of model files.
+ * @param {String[]} modelFileNames array of model file names.
+ * @param {ModelManager} modelManager the model manager to which the created model files will be registered.
+ * @return {ModelFile[]} array of loaded model files, matching the supplied arguments.
+ */
+ const loadModelFiles = (modelFileNames, modelManager) => {
+ const modelFiles = [];
+ for (let modelFileName of modelFileNames) {
+ const modelDefinitions = fs.readFileSync(modelFileName, 'utf8');
+ const modelFile = new ModelFile(modelManager, modelDefinitions);
+ modelFiles.push(modelFile);
+ }
+ modelManager.addModelFiles(modelFiles, modelFileNames);
+ return modelFiles;
+ };
+
  const loadModelFile = (modelFileName) => {
- const modelDefinitions = fs.readFileSync(modelFileName, 'utf8');
- const modelFile = new ModelFile(mockModelManager, modelDefinitions);
- mockModelManager.getModelFiles.returns([modelFile]);
- return modelFile;
+ return loadModelFiles([modelFileName], mockModelManager)[0];
  };
 
  const loadLastDeclaration = (modelFileName, type) => {
@@ -216,35 +229,62 @@ describe('ClassDeclaration', () => {
  });
 
  describe('#getSuperType', function() {
- it('should return superclass when one exists', function() {
- const modelFile = loadModelFile('test/data/parser/classdeclaration.participantwithparents.cto');
- const subclass = modelFile.getLocalType('Sub');
+ const modelFileNames = [
+ 'test/data/parser/classdeclaration.participantwithparents.parent.cto',
+ 'test/data/parser/classdeclaration.participantwithparents.child.cto'
+ ];
+ let modelManager;
+
+ beforeEach(() => {
+ modelManager = new ModelManager();
+ const modelFiles = loadModelFiles(modelFileNames, modelManager);
+ modelManager.addModelFiles(modelFiles);
+ });
+
+ it('should return superclass when one exists in the same model file', function() {
+ const subclass = modelManager.getType('com.testing.parent.Super');
+ should.exist(subclass);
+ const superclassName = subclass.getSuperType();
+ superclassName.should.equal('com.testing.parent.Base');
+ });
+
+ it('should return superclass when one exists in a different model file', function() {
+ const subclass = modelManager.getType('com.testing.child.Sub');
  should.exist(subclass);
  const superclassName = subclass.getSuperType();
- superclassName.should.equal('com.testing.Super');
+ superclassName.should.equal('com.testing.parent.Super');
  });
 
  it('should return null when none exists', function() {
- const modelFile = loadModelFile('test/data/parser/classdeclaration.participantwithparents.cto');
- const baseclass = modelFile.getLocalType('Base');
+ const baseclass = modelManager.getType('com.testing.parent.Base');
  should.exist(baseclass);
  const superclassName = baseclass.getSuperType();
  should.equal(superclassName, null);
  });
  });
 
  describe('#getAssignableClassDeclarations', function() {
+ const modelFileNames = [
+ 'test/data/parser/classdeclaration.participantwithparents.parent.cto',
+ 'test/data/parser/classdeclaration.participantwithparents.child.cto'
+ ];
+ let modelManager;
+
+ beforeEach(() => {
+ modelManager = new ModelManager();
+ const modelFiles = loadModelFiles(modelFileNames, modelManager);
+ modelManager.addModelFiles(modelFiles);
+ });
+
  it('should return itself only if there are no subclasses', function() {
- const modelFile = loadModelFile('test/data/parser/classdeclaration.participantwithparents.cto');
- const baseclass = modelFile.getLocalType('Sub');
+ const baseclass = modelManager.getType('com.testing.child.Sub');
  should.exist(baseclass);
  const subclasses = baseclass.getAssignableClassDeclarations();
  subclasses.should.have.same.members([baseclass]);
  });
 
  it('should return all subclass definitions', function() {
- const modelFile = loadModelFile('test/data/parser/classdeclaration.participantwithparents.cto');
- const baseclass = modelFile.getLocalType('Base');
+ const baseclass = modelManager.getType('com.testing.parent.Base');
  should.exist(baseclass);
  const subclasses = baseclass.getAssignableClassDeclarations();
  const subclassNames = subclasses.map(classDef => classDef.getName());

packages/composer-common/test/introspect/classdeclaration.js

@@ -16,6 +16,7 @@
 
 const ModelFile = require('../lib/introspect/modelfile');
 const Property = require('../lib/introspect/property');
+const ModelManager = require('../lib/modelmanager');
 const ModelUtil = require('../lib/modelutil');
 
 require('chai').should();
@@ -145,7 +146,9 @@ describe('ModelUtil', function () {
  it('throws error when type cannot be found', function() {
  mockProperty.getName.returns('theDoge');
  mockProperty.getFullyQualifiedTypeName.returns('org.doge.BaseDoge');
- mockModelFile.getType.returns(null);
+ const mockModelManager = sinon.createStubInstance(ModelManager);
+ mockModelManager.getType.returns(null);
+ mockModelFile.getModelManager.returns(mockModelManager);
  (() => {
  ModelUtil.isAssignableTo(mockModelFile, 'org.doge.Doge', mockProperty);
  }).should.throw(/Cannot find type/);