From e1527cf150ac5b4d5a3cf53f751d7ab7d57ff55f Mon Sep 17 00:00:00 2001 From: Ertugrul Karademir Date: Tue, 6 Feb 2024 14:55:21 +0000 Subject: [PATCH] perf(core): remove usage of arrays while forming duplicate item errors (#804) * refactor: don't use arrays to check uniqueness Signed-off-by: Ertugrul Karademir * refactor: also refactor unique property name check Signed-off-by: Ertugrul Karademir * refactor: remove array for decorator uniqueness check Signed-off-by: Ertugrul Karademir * refactor: remove uniqueness check from scalar declarations as well Signed-off-by: Ertugrul Karademir --------- Signed-off-by: Ertugrul Karademir --- .../lib/introspect/classdeclaration.js | 31 +++++++++---------- .../concerto-core/lib/introspect/decorated.js | 29 ++++++++--------- .../lib/introspect/scalardeclaration.js | 16 ---------- .../test/introspect/modelfile.js | 7 +++++ .../test/introspect/scalardeclaration.js | 9 ------ 5 files changed, 34 insertions(+), 58 deletions(-) diff --git a/packages/concerto-core/lib/introspect/classdeclaration.js b/packages/concerto-core/lib/introspect/classdeclaration.js index 3eda772f2f..453e3ceb9a 100644 --- a/packages/concerto-core/lib/introspect/classdeclaration.js +++ b/packages/concerto-core/lib/introspect/classdeclaration.js @@ -255,24 +255,21 @@ class ClassDeclaration extends Declaration { } // we also have to check fields defined in super classes const properties = this.getProperties(); - const propertyFieldNames = properties.map( - d => d.getName() - ); - const uniquePropertyFieldNames = new Set(propertyFieldNames); - - if (uniquePropertyFieldNames.size !== properties.length) { - const duplicateElements = propertyFieldNames - .filter( - (item, index) => propertyFieldNames.indexOf(item) !== index + const uniquePropertyNames = new Set(); + properties.forEach(p => { + const propertyName = p.getName(); + if (!uniquePropertyNames.has(propertyName)) { + uniquePropertyNames.add(propertyName); + } else { + const formatter = Globalize('en').messageFormatter( + 'classdeclaration-validate-duplicatefieldname' ); - const formatter = Globalize('en').messageFormatter( - 'classdeclaration-validate-duplicatefieldname' - ); - throw new IllegalModelException(formatter({ - 'class': this.name, - 'fieldName': duplicateElements[0] - }), this.modelFile, this.ast.location); - } + throw new IllegalModelException(formatter({ + 'class': this.name, + 'fieldName': propertyName + }), this.modelFile, this.ast.location); + } + }); for (let n = 0; n < properties.length; n++) { let field = properties[n]; diff --git a/packages/concerto-core/lib/introspect/decorated.js b/packages/concerto-core/lib/introspect/decorated.js index a91c3fd9c0..bcfd155d3b 100644 --- a/packages/concerto-core/lib/introspect/decorated.js +++ b/packages/concerto-core/lib/introspect/decorated.js @@ -116,23 +116,20 @@ class Decorated { } // check we don't have this decorator twice - const decoratorNames = this.decorators.map( - d => d.getName() - ); - const uniqueDecoratorNames = new Set(decoratorNames); - - if (uniqueDecoratorNames.size !== this.decorators.length) { - const duplicateElements = decoratorNames - .filter( - (item, index) => decoratorNames.indexOf(item) !== index + const uniqueDecoratorNames = new Set(); + this.decorators.forEach(d => { + const decoratorName = d.getName(); + if(!uniqueDecoratorNames.has(decoratorName)) { + uniqueDecoratorNames.add(decoratorName); + } else { + const modelFile = this.getModelFile(); + throw new IllegalModelException( + `Duplicate decorator ${decoratorName}`, + modelFile, + this.ast.location, ); - const modelFile = this.getModelFile(); - throw new IllegalModelException( - `Duplicate decorator ${duplicateElements[0]}`, - modelFile, - this.ast.location, - ); - } + } + }); } } diff --git a/packages/concerto-core/lib/introspect/scalardeclaration.js b/packages/concerto-core/lib/introspect/scalardeclaration.js index 3fab478621..cb3bcebfa5 100644 --- a/packages/concerto-core/lib/introspect/scalardeclaration.js +++ b/packages/concerto-core/lib/introspect/scalardeclaration.js @@ -17,7 +17,6 @@ const { MetaModelNamespace } = require('@accordproject/concerto-metamodel'); const Declaration = require('./declaration'); -const IllegalModelException = require('./illegalmodelexception'); const NumberValidator = require('./numbervalidator'); const StringValidator = require('./stringvalidator'); @@ -106,21 +105,6 @@ class ScalarDeclaration extends Declaration { */ validate() { super.validate(); - - const declarations = this.getModelFile().getAllDeclarations(); - const declarationNames = declarations.map( - d => d.getFullyQualifiedName() - ); - const uniqueNames = new Set(declarationNames); - - if (uniqueNames.size !== declarations.length) { - const duplicateElements = declarationNames.filter( - (item, index) => declarationNames.indexOf(item) !== index - ); - throw new IllegalModelException( - `Duplicate class name ${duplicateElements[0]}` - ); - } } /** diff --git a/packages/concerto-core/test/introspect/modelfile.js b/packages/concerto-core/test/introspect/modelfile.js index 7836d21b23..16718d3d74 100644 --- a/packages/concerto-core/test/introspect/modelfile.js +++ b/packages/concerto-core/test/introspect/modelfile.js @@ -206,6 +206,13 @@ describe('ModelFile', () => { describe('#validate', () => { + it('should throw when scalar name is duplicted in a modelfile', () => { + let asset = introspectUtils.loadModelFile('test/data/parser/scalardeclaration.dupeboolean.cto'); + (() => { + asset.validate(); + }).should.throw(/Duplicate class/); + }); + it('should throw when asset name is duplicted in a modelfile', () => { let asset = introspectUtils.loadModelFile('test/data/parser/classdeclaration.dupeassetname.cto'); (() => { diff --git a/packages/concerto-core/test/introspect/scalardeclaration.js b/packages/concerto-core/test/introspect/scalardeclaration.js index 2e05ff0ede..f48bf827b8 100644 --- a/packages/concerto-core/test/introspect/scalardeclaration.js +++ b/packages/concerto-core/test/introspect/scalardeclaration.js @@ -37,15 +37,6 @@ describe('ScalarDeclaration', () => { modelFile = ParserUtil.newModelFile(modelManager, 'namespace com.hyperledger.testing', 'org.acme.cto'); }); - describe('#validate', () => { - it('should throw when scalar name is duplicted in a modelfile', () => { - let asset = introspectUtils.loadLastDeclaration('test/data/parser/scalardeclaration.dupeboolean.cto', ScalarDeclaration); - (() => { - asset.validate(); - }).should.throw(/Duplicate class/); - }); - }); - describe('#accept', () => { it('should call the visitor', () => { let clz = new ScalarDeclaration(modelFile, {