Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(common): check dependecy in decorators #126

Merged
merged 4 commits into from
Sep 20, 2024
Merged

feat(common): check dependecy in decorators #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
62f59ed
feat(common): check dependecy in decorators
Sep 18, 2024
79364a2
Merge branch 'accordproject:main' into main
muhabdulkadir Sep 18, 2024
198f02b
fix(graph): use existing visitor pattern
Sep 19, 2024
acf9357
refactor(visitor): follow visitor pattern
Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions lib/common/diagramvisitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const ModelUtil = require('@accordproject/concerto-core').ModelUtil;
/* eslint-disable no-unused-vars */
/* istanbul ignore next */
if (global === undefined) {
const { ModelManager, ModelFile, ClassDeclaration, ScalarDeclaration, Field, EnumValueDeclaration, RelationshipDeclaration} = require('@accordproject/concerto-core');
const { ModelManager, ModelFile, ClassDeclaration, ScalarDeclaration, Field, EnumValueDeclaration, RelationshipDeclaration, Decorator} = require('@accordproject/concerto-core');
}
/* eslint-enable no-unused-vars */

Expand Down Expand Up @@ -70,6 +70,8 @@ class DiagramVisitor {
return this.visitEnumValueDeclaration(thing, parameters);
} else if (thing.isScalarDeclaration?.()) {
return this.visitScalarDeclaration(thing, parameters);
} else if (thing.isDecorator?.()) {
return this.visitDecorator(thing, parameters);
} else {
throw new Error('Unrecognised ' + JSON.stringify(thing) );
}
Expand Down Expand Up @@ -97,6 +99,7 @@ class DiagramVisitor {
modelFile.getAllDeclarations().forEach((decl) => {
decl.accept(this, parameters);
});
modelFile.getDecorators().forEach(decorator => decorator.accept(this, parameters));
}

/**
Expand Down Expand Up @@ -161,6 +164,7 @@ class DiagramVisitor {
classDeclaration.getOwnProperties().forEach((property) => {
property.accept(this, parameters);
});
classDeclaration.getDecorators().forEach(decorator => decorator.accept(this, parameters));
}

/**
Expand All @@ -170,6 +174,7 @@ class DiagramVisitor {
* @protected
*/
visitScalarDeclaration(scalarDeclaration, parameters) {
scalarDeclaration.getDecorators().forEach(decorator => decorator.accept(this, parameters));
return;
}

Expand All @@ -180,6 +185,7 @@ class DiagramVisitor {
* @protected
*/
visitMapDeclaration(mapDeclaration, parameters) {
mapDeclaration.getDecorators().forEach(decorator => decorator.accept(this, parameters));
return;
}

Expand All @@ -191,6 +197,7 @@ class DiagramVisitor {
*/
visitScalarField(field, parameters) {
this.visitField(field.getScalarField(), parameters);
field.getDecorators().forEach(decorator => decorator.accept(this, parameters));
}

/**
Expand All @@ -215,7 +222,7 @@ class DiagramVisitor {
if(field.isArray()) {
array = '[]';
}

field.getDecorators().forEach(decorator => decorator.accept(this, parameters));
parameters.fileWriter.writeLine(1, '+ ' + field.getType() + array + ' ' + field.getName());
}

Expand All @@ -226,9 +233,20 @@ class DiagramVisitor {
* @protected
*/
visitEnumValueDeclaration(enumValueDeclaration, parameters) {
enumValueDeclaration.getDecorators().forEach(decorator => decorator.accept(this, parameters));
parameters.fileWriter.writeLine(1, '+ ' + enumValueDeclaration.getName());
}

/**
* Visitor design pattern
* @param {Decorator} decorator - the object being visited
* @param {Object} parameters - the parameter
* @protected
*/
visitDecorator(decorator, parameters) {
return;
}

/**
* Visitor design pattern
* @param {ClassDeclaration} classDeclaration - the object being visited
Expand Down
27 changes: 25 additions & 2 deletions lib/common/graph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const DiagramVisitor = require('./diagramvisitor');
/* eslint-disable no-unused-vars */
/* istanbul ignore next */
if (global === undefined) {
const { ClassDeclaration, MapDeclaration, ScalarDeclaration, Field, EnumValueDeclaration, RelationshipDeclaration} = require('@accordproject/concerto-core');
const { ClassDeclaration, MapDeclaration, ScalarDeclaration, Field, Decorator, EnumValueDeclaration, RelationshipDeclaration} = require('@accordproject/concerto-core');
const { Writer } = require('@accordproject/concerto-util');
}
/* eslint-enable no-unused-vars */
Expand Down Expand Up @@ -201,9 +201,28 @@ class ConcertoGraphVisitor extends DiagramVisitor {
* @protected
*/
visitScalarDeclaration(scalarDeclaration, parameters) {
super.visitScalarDeclaration(scalarDeclaration, parameters);
parameters.graph.addVertex(scalarDeclaration.getFullyQualifiedName());
}

/**
* Visitor design pattern
* @param {Decorator} decorator - the object being visited
* @param {Object} parameters - the parameter
* @protected
*/
visitDecorator(decorator, parameters) {
const parent = decorator.getParent();
const modelFile = parent.getModelFile();
decorator.getArguments()?.forEach(argument => {
const fqn = modelFile.getFullyQualifiedTypeName(argument.name);
muhabdulkadir marked this conversation as resolved.
Show resolved Hide resolved
if (fqn) {
parameters.graph.addVertex(fqn);
(parent === modelFile) ? parameters.graph.addEdge(parent.getNamespace(), fqn) : parameters.graph.addEdge(parent.getFullyQualifiedName(), fqn);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(parent === modelFile) ? parameters.graph.addEdge(parent.getNamespace(), fqn)

This allows for a model decorated with a type reference to act as a vertex in a connected graph, causing it to be included when users search for subgraphs.

This behavior arises because we currently allow models to be decorated with type references, which implicitly suggests a composition. However, since we treat models as packages, we should avoid compositions on the packages themselves.

}
});
}

/**
* Visitor design pattern
* @param {MapDeclaration} mapDeclaration - the object being visited
Expand All @@ -228,7 +247,7 @@ class ConcertoGraphVisitor extends DiagramVisitor {
valueEdge = modelFile.getType(mapDeclaration.getValue().getType()).getFullyQualifiedName();
}
}

super.visitMapDeclaration(mapDeclaration, parameters);
parameters.graph.addVertex(fqn);
parameters.graph.addEdge(fqn, keyEdge);
parameters.graph.addEdge(fqn, valueEdge);
Expand All @@ -241,6 +260,7 @@ class ConcertoGraphVisitor extends DiagramVisitor {
* @protected
*/
visitScalarField(scalar, parameters) {
super.visitScalarField(scalar, parameters);
parameters.graph.addEdge(parameters.stack.slice(-1), scalar.getFullyQualifiedTypeName());
}

Expand All @@ -251,6 +271,7 @@ class ConcertoGraphVisitor extends DiagramVisitor {
* @protected
*/
visitField(field, parameters) {
super.visitField(field, parameters);
if (!ModelUtil.isPrimitiveType(field.getFullyQualifiedTypeName())) {
parameters.graph.addEdge(parameters.stack.slice(-1), field.getFullyQualifiedTypeName());
}
Expand All @@ -263,6 +284,7 @@ class ConcertoGraphVisitor extends DiagramVisitor {
* @protected
*/
visitRelationship(relationship, parameters) {
super.visitRelationship(relationship, parameters);
parameters.graph.addEdge(parameters.stack.slice(-1), relationship.getFullyQualifiedTypeName());
}

Expand All @@ -273,6 +295,7 @@ class ConcertoGraphVisitor extends DiagramVisitor {
* @protected
*/
visitEnumValueDeclaration(enumValueDeclaration, parameters) {
super.visitEnumValueDeclaration(enumValueDeclaration, parameters);
return;
}
}
Expand Down
Loading
Loading