archimatetool / archi

Archi: ArchiMate Modelling Tool
https://www.archimatetool.com
MIT License
944 stars 269 forks source link

Bug with model validator #747

Closed jbsarrodie closed 3 years ago

jbsarrodie commented 3 years ago

See the forum post for more information

The validator raises a warning for the attached model while it shouldn't. If we remove the (not used) flow, then the model validates fine.

The model: test.zip

jbsarrodie commented 3 years ago

I'll look at it...

Phillipus commented 3 years ago

Might be because of this?

    private boolean isNestedTypeRelationship(IArchimateRelationship r) {
        return r instanceof ICompositionRelationship
                || r instanceof ISpecializationRelationship
                || r instanceof IAggregationRelationship
                || r instanceof IAssignmentRelationship
                || r instanceof IRealizationRelationship
                || r instanceof IAccessRelationship;
    }
jbsarrodie commented 3 years ago

No, this looks like if at some point all relations that exist between both elements (without even being used in the view) were taken into consideration and in this case the flow is returned and the warning happens.

I have to check the whole logic.

Maybe also time to take ARM settings into account (there's an old issue of mine on this topic).

Phillipus commented 3 years ago

No, this looks like if at some point all relations that exist between both elements (without even being used in the view) were taken into consideration and in this case the flow is returned and the warning happens.

Indeed. In the example case, it's reporting that there is a non-nesting relationship...because there is (in the model).

Depends whether this is the desired behaviour, or whether to only check only for diagram instances or to use ARM settings.

jbsarrodie commented 3 years ago

it's reporting that there is a non-nesting relationship...because there is (in the model).

Well, that's not the intended behavior, even with the current version (ie. not taking ARM into account).

This should go as follows for this rule: For each nesting:

  1. Get the list of hidden relationships used in the view, between outside and inside elements (as this relies on ARM, we can simplify a bit by getting the list of all relationships between the two elements and used in the view)
  2. For each of these relationships, check if they are valid for the nesting involved.
  3. If they are all valid, then stop here and don't warn (the user explicitely used valid relationships for this nesting)
  4. If some of them are invalid, then stop here and warn (the user explicitely used an invalid relationship for this nesting)
  5. If there were no relationship used in the view, then get the list of relationships involving the two elements, defined in the model but not used in the view.
  6. For each of these relationships, check if they are valid for the nesting involved.
  7. If at least one is valid, then stop here and don't warn (user was lazy and didn't add a relationship to the view, but the model confirms his modelling choice)
  8. If none is valid, then warn (user was lazy and didn't add a relationship, and nothing in the model supports his modelling choice)
Phillipus commented 3 years ago

Commit added to nested-model-checker branch that should fix the original problem and uses the rules above.

Phillipus commented 3 years ago

Committed to master. Will be in 4.9.