fix: duplicate and self dependencies should not being rendered (#903)

asyncapi · Oct 4, 2022 · 53a4f9e · 53a4f9e
1 parent 7900bf3
commit 53a4f9e
Show file tree

Hide file tree

Showing 6 changed files with 160 additions and 34 deletions.
diff --git a/src/helpers/ConstrainHelpers.ts b/src/helpers/ConstrainHelpers.ts
@@ -207,26 +207,34 @@ export function constrainMetaModel<Options>(typeMapping: TypeMapping<Options>, c
     return constrainObjectModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
   } else if (newContext.metaModel instanceof ReferenceModel) {
     return constrainReferenceModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
-  } else if (newContext.metaModel instanceof AnyModel) {
-    return constrainAnyModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
-  } else if (newContext.metaModel instanceof FloatModel) {
-    return constrainFloatModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
-  } else if (newContext.metaModel instanceof IntegerModel) {
-    return constrainIntegerModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
-  } else if (newContext.metaModel instanceof StringModel) {
-    return constrainStringModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
-  } else if (newContext.metaModel instanceof BooleanModel) {
-    return constrainBooleanModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof DictionaryModel) {
+    return constrainDictionaryModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
   } else if (newContext.metaModel instanceof TupleModel) {
     return constrainTupleModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
   } else if (newContext.metaModel instanceof ArrayModel) {
     return constrainArrayModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
   } else if (newContext.metaModel instanceof UnionModel) {
     return constrainUnionModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
-  } else if (newContext.metaModel instanceof EnumModel) {
-    return ConstrainEnumModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel});
-  } else if (newContext.metaModel instanceof DictionaryModel) {
-    return constrainDictionaryModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel}, alreadySeenModels);
   }
+  // Simple models are those who does not have properties that contain other MetaModels.
+  let simpleModel: ConstrainedMetaModel | undefined;
+  if (newContext.metaModel instanceof EnumModel) {
+    simpleModel = ConstrainEnumModel(typeMapping, constrainRules, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof BooleanModel) {
+    simpleModel = constrainBooleanModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof AnyModel) {
+    simpleModel = constrainAnyModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof FloatModel) {
+    simpleModel = constrainFloatModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof IntegerModel) {
+    simpleModel = constrainIntegerModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  } else if (newContext.metaModel instanceof StringModel) {
+    simpleModel = constrainStringModel(typeMapping, {...newContext, metaModel: newContext.metaModel});
+  }
+  if (simpleModel !== undefined) {
+    alreadySeenModels.set(context.metaModel, simpleModel); 
+    return simpleModel;
+  }
+
   throw new Error('Could not constrain model');
 }
diff --git a/src/models/ConstrainedMetaModel.ts b/src/models/ConstrainedMetaModel.ts
@@ -1,3 +1,4 @@
+import { makeUnique } from '../utils/DependencyHelper';
 import { MetaModel } from './MetaModel';
 
 export abstract class ConstrainedMetaModel extends MetaModel {
@@ -59,7 +60,7 @@ export class ConstrainedTupleModel extends ConstrainedMetaModel {
     }
 
     //Ensure no duplicate references
-    dependencyModels = [...new Set(dependencyModels)];
+    dependencyModels = makeUnique(dependencyModels);
 
     return dependencyModels;
   }
@@ -112,7 +113,7 @@ export class ConstrainedUnionModel extends ConstrainedMetaModel {
     }
 
     //Ensure no duplicate references
-    dependencyModels = [...new Set(dependencyModels)];
+    dependencyModels = makeUnique(dependencyModels);
 
     return dependencyModels;
   }
@@ -131,20 +132,6 @@ export class ConstrainedEnumModel extends ConstrainedMetaModel {
     public values: ConstrainedEnumValueModel[]) {
     super(name, originalInput, type);
   }
-
-  getNearestDependencies(): ConstrainedMetaModel[] {
-    let dependencyModels = Object.values(this.values).filter(
-      (enumModel) => {
-        return enumModel.value instanceof ConstrainedReferenceModel;
-      }
-    ).map((enumModel) => {
-      return enumModel.value as ConstrainedReferenceModel;
-    });
-
-    //Ensure no duplicate references
-    dependencyModels = [...new Set(dependencyModels)];
-    return dependencyModels;
-  }
 }
 export class ConstrainedDictionaryModel extends ConstrainedMetaModel {
   constructor(
@@ -170,7 +157,7 @@ export class ConstrainedDictionaryModel extends ConstrainedMetaModel {
     }
 
     //Ensure no duplicate references
-    dependencyModels = [...new Set(dependencyModels)];
+    dependencyModels = makeUnique(dependencyModels);
 
     return dependencyModels;
   }
@@ -196,8 +183,13 @@ export class ConstrainedObjectModel extends ConstrainedMetaModel {
       }
     }
 
+    //Ensure no self references
+    dependencyModels = dependencyModels.filter((referenceModel) => {
+      return referenceModel.name !== this.name;
+    });
+
     //Ensure no duplicate references
-    dependencyModels = [...new Set(dependencyModels)];
+    dependencyModels = makeUnique(dependencyModels);
 
     return dependencyModels;
   }

diff --git a/src/utils/DependencyHelper.ts b/src/utils/DependencyHelper.ts
@@ -1,3 +1,4 @@
+import { ConstrainedMetaModel, ConstrainedReferenceModel } from '../models';
 
 /**
  * Function to make it easier to render JS/TS dependencies based on module system
@@ -11,3 +12,18 @@ export function renderJavaScriptDependency(toImport: string, fromModule: string,
     ? `const ${toImport} = require('${fromModule}');`
     : `import ${toImport} from '${fromModule}';`;
 }
+
+/**
+ * Function to make an array of ConstrainedMetaModels only contain unique values (ignores different in memory instances)
+ * 
+ * @param array to make unique
+ */
+export function makeUnique(array: ConstrainedMetaModel[]): ConstrainedMetaModel[] {
+  const seen: Map<ConstrainedMetaModel, boolean> = new Map();
+  return array.filter((item: ConstrainedMetaModel) => {
+    if (item instanceof ConstrainedReferenceModel) {
+      return seen.has(item.ref) ? false : seen.set(item.ref, true);
+    }
+    return seen.has(item) ? false : seen.set(item, true);
+  });
+}
diff --git a/test/helpers/ConstrainHelpers.spec.ts b/test/helpers/ConstrainHelpers.spec.ts
@@ -229,7 +229,8 @@ describe('ConstrainHelpers', () => {
   describe('constrain DictionaryModel', () => {
     test('should constrain correctly', () => {
       const stringModel = new StringModel('', undefined);
-      const metaModel = new DictionaryModel('test', undefined, stringModel, stringModel);
+      const stringModel2 = new StringModel('', undefined);
+      const metaModel = new DictionaryModel('test', undefined, stringModel, stringModel2);
       const constrainedModel = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
         metaModel,
         options: {},

diff --git a/test/models/ConstrainedMetaModel.spec.ts b/test/models/ConstrainedMetaModel.spec.ts
@@ -131,6 +131,23 @@ describe('ConstrainedMetaModel', () => {
       const dependencies = model.getNearestDependencies();
       expect(dependencies).toHaveLength(1);
     });
+
+    test('should not return duplicate dependencies when different reference instances', () => {
+      const stringModel = new StringModel('', undefined);
+      const referenceModel = new ReferenceModel('', undefined, stringModel);
+      const referenceModel2 = new ReferenceModel('', undefined, stringModel);
+      const referenceTupleModel = new TupleValueModel(0, referenceModel);
+      const reference2TupleModel = new TupleValueModel(1, referenceModel2);
+      const rawModel = new TupleModel('test', undefined, [referenceTupleModel, reference2TupleModel]);
+
+      const model = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
+        metaModel: rawModel,
+        constrainedName: '',
+        options: undefined
+      }) as ConstrainedTupleModel;
+      const dependencies = model.getNearestDependencies();
+      expect(dependencies).toHaveLength(1);
+    });
   });
   describe('ObjectModel', () => {
     test('should return inner reference dependencies', () => {
@@ -171,6 +188,21 @@ describe('ConstrainedMetaModel', () => {
       expect(dependencies[0]).toEqual(model.properties['reference'].property);
     });
 
+    test('should not return self reference', () => {
+      const rawModel = new ObjectModel('ObjectTest', undefined, {});
+      const referenceModel = new ReferenceModel(rawModel.name, undefined, rawModel);
+      const referenceObjectPropertyModel = new ObjectPropertyModel('self', false, referenceModel);
+      rawModel.properties['self'] = referenceObjectPropertyModel;
+
+      const model = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
+        metaModel: rawModel,
+        constrainedName: '',
+        options: undefined
+      }) as ConstrainedObjectModel;
+      const dependencies = model.getNearestDependencies();
+      expect(dependencies).toHaveLength(0);
+    });
+
     test('should not return duplicate dependencies', () => {
       const stringModel = new StringModel('string', undefined);
       const referenceModel = new ReferenceModel('reference', undefined, stringModel);
@@ -191,6 +223,27 @@ describe('ConstrainedMetaModel', () => {
       expect(dependencies[0]).toEqual(model.properties['reference'].property);
     });
 
+    test('should not return duplicate dependencies when different reference instances', () => {
+      const stringModel = new StringModel('string', undefined);
+      const referenceModel = new ReferenceModel('reference', undefined, stringModel);
+      const referenceModel2 = new ReferenceModel('reference', undefined, stringModel);
+      const referenceObjectPropertyModel = new ObjectPropertyModel('reference', false, referenceModel);
+      const reference2ObjectPropertyModel = new ObjectPropertyModel('reference2', false, referenceModel2);
+      const rawModel = new ObjectModel('test', undefined, {
+        reference: referenceObjectPropertyModel,
+        reference2: reference2ObjectPropertyModel
+      });
+
+      const model = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
+        metaModel: rawModel,
+        constrainedName: '',
+        options: undefined
+      }) as ConstrainedObjectModel;
+      const dependencies = model.getNearestDependencies();
+      expect(dependencies).toHaveLength(1);
+      expect(dependencies[0]).toEqual(model.properties['reference'].property);
+    });
+
     describe('containsPropertyType', () => {
       test('should find present property type and those who are not', () => {
         const stringModel = new ConstrainedStringModel('', undefined, '');
@@ -264,6 +317,22 @@ describe('ConstrainedMetaModel', () => {
       expect(dependencies).toHaveLength(1);
       expect(dependencies[0]).toEqual(model.key);
     });
+
+    test('should not return duplicate dependencies when different reference instances', () => {
+      const stringModel = new StringModel('', undefined);
+      const referenceModel = new ReferenceModel('', undefined, stringModel);
+      const referenceModel2 = new ReferenceModel('', undefined, stringModel);
+      const rawModel = new DictionaryModel('test', undefined, referenceModel, referenceModel2);
+
+      const model = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
+        metaModel: rawModel,
+        constrainedName: '',
+        options: undefined
+      }) as ConstrainedDictionaryModel;
+      const dependencies = model.getNearestDependencies();
+      expect(dependencies).toHaveLength(1);
+      expect(dependencies[0]).toEqual(model.key);
+    });
   });
   describe('ArrayModel', () => {
     test('should return all reference dependencies', () => {
@@ -339,5 +408,21 @@ describe('ConstrainedMetaModel', () => {
       expect(dependencies).toHaveLength(1);
       expect(dependencies[0]).toEqual(model.union[0]);
     });
+
+    test('should not return duplicate dependencies when different reference instances', () => {
+      const stringModel = new StringModel('', undefined);
+      const referenceModel = new ReferenceModel('', undefined, stringModel);
+      const referenceModel2 = new ReferenceModel('', undefined, stringModel);
+      const rawModel = new UnionModel('test', undefined, [referenceModel, referenceModel2]);
+
+      const model = constrainMetaModel(mockedTypeMapping, mockedConstraints, {
+        metaModel: rawModel,
+        constrainedName: '',
+        options: undefined
+      }) as ConstrainedUnionModel;
+      const dependencies = model.getNearestDependencies();
+      expect(dependencies).toHaveLength(1);
+      expect(dependencies[0]).toEqual(model.union[0]);
+    });
   });
 });
diff --git a/test/utils/DependencyHelper.spec.ts b/test/utils/DependencyHelper.spec.ts
@@ -1,4 +1,5 @@
-import {renderJavaScriptDependency} from '../../src/utils'; 
+import { ConstrainedReferenceModel, ConstrainedStringModel } from '../../src';
+import {renderJavaScriptDependency, makeUnique} from '../../src/utils'; 
 
 describe('DependencyHelper', () => {
   describe('renderJavaScriptDependency', () => {
@@ -11,4 +12,27 @@ describe('DependencyHelper', () => {
       expect(renderedDependency).toEqual('import test from \'test2\';');
     });
   });
+  describe('makeUnique', () => {
+    test('should remove duplicate regular instances', () => {
+      const stringModel = new ConstrainedStringModel('', undefined, '');
+      const nonUniqueArray = [stringModel, stringModel];
+      const uniqueArray = makeUnique(nonUniqueArray);
+      expect(uniqueArray).toHaveLength(1);
+    });
+    test('should remove duplicate reference instances', () => {
+      const stringModel = new ConstrainedStringModel('', undefined, '');
+      const ref1 = new ConstrainedReferenceModel('', undefined, '', stringModel);
+      const nonUniqueArray = [ref1, ref1];
+      const uniqueArray = makeUnique(nonUniqueArray);
+      expect(uniqueArray).toHaveLength(1);
+    });
+    test('should remove duplicate reference value instances', () => {
+      const stringModel = new ConstrainedStringModel('', undefined, '');
+      const ref1 = new ConstrainedReferenceModel('', undefined, '', stringModel);
+      const ref2 = new ConstrainedReferenceModel('', undefined, '', stringModel);
+      const nonUniqueArray = [ref1, ref2];
+      const uniqueArray = makeUnique(nonUniqueArray);
+      expect(uniqueArray).toHaveLength(1);
+    });
+  });
 });