From 53a4f9eca6add3edc19c0e0d1301d7f5c3cfaf25 Mon Sep 17 00:00:00 2001 From: Jonas Lagoni Date: Tue, 4 Oct 2022 10:47:53 +0200 Subject: [PATCH] fix: duplicate and self dependencies should not being rendered (#903) --- src/helpers/ConstrainHelpers.ts | 36 ++++++---- src/models/ConstrainedMetaModel.ts | 28 +++----- src/utils/DependencyHelper.ts | 16 +++++ test/helpers/ConstrainHelpers.spec.ts | 3 +- test/models/ConstrainedMetaModel.spec.ts | 85 ++++++++++++++++++++++++ test/utils/DependencyHelper.spec.ts | 26 +++++++- 6 files changed, 160 insertions(+), 34 deletions(-) diff --git a/src/helpers/ConstrainHelpers.ts b/src/helpers/ConstrainHelpers.ts index 29fe0a9a7b..7aa53e49db 100644 --- a/src/helpers/ConstrainHelpers.ts +++ b/src/helpers/ConstrainHelpers.ts @@ -207,26 +207,34 @@ export function constrainMetaModel(typeMapping: TypeMapping, 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 index f4056ebbd5..bef4aa8c81 100644 --- 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 index 7858902a8a..5c8863b111 100644 --- 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 = 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 index f7d348cc16..639cb62583 100644 --- 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 index 60ab2b7d47..9e9a5a65c0 100644 --- 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 index 9f7f1e1319..2efd86dda7 100644 --- 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); + }); + }); });