fix(graph): add edge to map relationship between declarations and decorators on fields

[muhabdulkadir]

Closes #

Changes

In creating a connected graph, we observe that the relationship between declarations and fields are not reflected as expected. Specifically, when a field has a decorator, the associated decorator is not included in the filtered model manager, leading to an error due to failed decorator validation.

This change add the edge to reflect the relationship between the parent declaration and it's fields.

Author Checklist

• Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
• Vital features and changes captured in unit and/or integration tests
• Commits messages follow AP format
• Extend the documentation, if necessary
• Merging to main from fork:branchname

[coveralls]

coverage: 98.771% (-0.02%) from 98.794%
when pulling 2c40067 on muhabdulkadir:main
into 02644a1 on accordproject:main.

[dselman]

I think we can divide this into a couple of sub-tasks:

1. Function that gets the set of types for a decorator (decorator declaration, if present, as well as all types references from decorator arguments that exist)
2. Unit tests for this function, checking its behaviour when decorator declaration is present/absent and when decorator argument types are present/absent.

Create the edges from the elements that can have decorators applied:

• Namespace
• Declarations (Enum, Concept, Scalar, Map)
• Enum Values
• Concept Properties
• Map Keys and Values

We don't currently represent properties as nodes in the graph, so I think that decorators applied on Enum Values, Concept Properties, Map Keys and Values will need to be "hoisted" onto their parent declaration (taking care to remove duplicates!). This can be a second function, to get the node name for a model element (which could be based on the FQN of the element, or the FQN of the parent of the element).

I.e. if I have something like:

concept pii extends Decorator {}

concept Person {
   @pii
   o String name
}

We will create a graph with 2 nodes, one for the Person concept and one for the declaration of the pii decorator, with a single edge from Person to pii.

If we have this, however:

concept Category {}

concept Person {
   @pii(Category)
   o String name
}

I would expect a graph with 2 nodes, one for Person and one for Category, with an edge from Person to Category. The pii decorator does not appear in the graph because it was not declared.

With this:

concept pii extends Decorator {
   o Category category
}

concept Category {}

concept Person {
   @pii(Category)
   o String name
}

I would expect a graph with 3 nodes, one for Person, one for pii and one for Category, with an edge from Person to pii and an edge from pii to Category, and an edge from Person to Category.

I would start by creating some test model files for these scenarios and making sure that the tests fail given the current code. I'd then start to refine the code until the tests pass.

We will want to create test files for all the model elements that can have decorators applied.

[dselman]

This is looking very nice. One comment on the snap...

More generally.

I don't understand how the functionality is split between ConcertoGraphVisitor and its base class DiagramVisitor. If I've understood this correctly the DiagramVisitor is writing PlantUML output to the FileWriter and then we specialise that to build a graph.

Those two operations don't appear to be related, or at least graph building is not a specialisation of PlantUML output. Why don't we just have two separate visitors, one for building a graph, one for PlantUML output?

I'm also not convinced that we need parameters.stack in ConcertoGraphVisitor - where we read the stack it seems to be to determine the parent ClassDeclaration of a field, relationship etc. We can get that by calling getParent on the field, relationship.

[mttrbrts]

I don't understand how the functionality is split between ConcertoGraphVisitor and its base class DiagramVisitor. If I've understood this correctly the DiagramVisitor is writing PlantUML output to the FileWriter and then we specialise that to build a graph.

I take responsibility for this one. ...

DiagramVisitor was originally less diagram specific, and was a helpful default visitor implementation so that the specialized visitors could avoid duplicating the tree navigation logic. I agree that it would make sense to further hoist the navigation logic, and specialise the Diagram visitor from it.

[dselman]

Looking great. Just doc updates...

[dselman]

Awesome!