Skip to content

Conversation

e-filchenko-bosh
Copy link
Contributor

Description

Bug Description:
This bug reproduces inconsistently.

Reason:
When we upload AspectModels with all dependencies, we also save fileLocation as part of the context. Currently, these locations are saved without ordering. As a result, during the next step of resolving files, we extract the name from the list of Aspect names that we saved based on the ordering in fileLocations.

In this scenario, the name .../target/Aspect.ttl might be lower in the order than .../second/Aspect.ttl. Consequently, we end up using the semantic ID from the wrong URN.

This problem also affects the logic in bug #798.

Fixes #794

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • [-] I have commented my code, particularly in hard-to-understand areas
  • [-] I have made corresponding changes to the documentation
  • [-] My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Copy link
Contributor

@atextor atextor left a comment

Choose a reason for hiding this comment

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

While all changes and checks in the PR are valid and usful, I think it does not fix the reported bug. At least one problem remains samm-cli: In samm-cli's AbstractInputHandler.java:154, the list of loaded Aspects is filtered for the "designated name" (i.e., the name of the originally loaded input file, minus ".ttl") and the first Aspect with that name is used. But this is non-deterministic if there happen to be multiple Aspects with that name in the resolved model. In order to fix this, another test is needed in SammCliTest (using @RepeatedTest is a good idea). A possible fix would be to have AbstractInputHandler#expectedAspectName() not return the local name of the Aspect as a String, but the full AspectModelUrn.

samm:events ( ) .

:test a samm:Property ;
samm:name "test property" ;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line, "samm:name" was removed in SAMM 2.0.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, my initial thought when I started investigating this issue was to fix this part of the code: AbstractInputHandler#expectedAspectName(). However, I encountered a problem. When we talk about expectedAspectName as a short name, it’s easy to compare it with the aspect name because our implementation has enough information for this type of name:

AspectModelUrnInputHandler → urn.getName()
GitHubUrlInputHandler → Extract the name from the URL
FileInputHandler → Extract the name from the file name
The problem arises when we try to compare it with the URN:

AspectModelUrnInputHandler → We already have the URN
GitHubUrlInputHandler → We can create the URN from the URL
FileInputHandler → In this case, we don’t have enough information about the URN, and we need to extract it from inside the file, if I’m not mistaken
That’s why I tried to decrease the chances of reproducing this issue by adding an order to the file upload process.

I will now try to enrich the context for FileInputHandler with the URN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] SAMM2AASX Generator: wrong generated semanticId for Submodel
2 participants