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.

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

Limit recursion depth for SMEs #333

Open
arnoweiss opened this issue Jan 22, 2024 · 5 comments
Open

Limit recursion depth for SMEs #333

arnoweiss opened this issue Jan 22, 2024 · 5 comments
Labels
accepted accepted in principle to be fixed
Milestone

Comments

@arnoweiss
Copy link

Is your feature request related to a problem? Please describe.
There should be a decision on a maximal recursion depth of SubmodelElements. Some SubmodelElements may contain arbitrary other SubmodelElements - namely:

  • SubmodelElementList
  • SubmodelElementCollection
  • Entity
  • AnnotatedRelationshipElement

This prevents overflow errors and should also serve as a guideline for modeling.

Describe the solution you'd like
Define a maximal recursion depth in the spec. Suggestion: 100.

Describe alternatives you've considered
Leave everything as is. There probably won't be any problems until someone has a really complex scenario where their stack crashes at runtime.

@mhrimaz
Copy link
Contributor

mhrimaz commented Jan 22, 2024

@arnoweiss even the exact 100 as limit could still cause problems. eclipse-basyx/basyx-java-server-sdk#158

@arnoweiss
Copy link
Author

Spoke to some colleagues and they used 32 in similar specifications.

After all, it's up to the workstream AAS if this should be specified at all (I think it's relevant for interop --> not implementation-specific --> relevant for spec) and then decide on a reasonable number.

@BirgitBoss BirgitBoss added the requires workstream approval strategic decision in spec team needed label Jan 26, 2024
@BirgitBoss
Copy link
Collaborator

You are probably right. To restrict recursion would not be backward-compatible but we could give a recommendation. I am happy to learn which depth might be appropriate. We should also have a look at existing Submodel Templates and which depth they used so far.

@BirgitBoss BirgitBoss added this to the V3.1 milestone Jan 26, 2024
@BirgitBoss
Copy link
Collaborator

TF AAS Part 1 2024-06-12 Decision Proposal:
Add the following recommendation:
A minimum recursion depth of 32 for Container-Elements should be supported.
These are the SubmodelElements affected:
SubmodelElementList
SubmodelElementCollection
Entity
AnnotatedRelationshipElement
Operation

For term "Container-Element" see also #420

@BirgitBoss BirgitBoss added ready for approval TF proposes how to resolve the issue. Needs final approval my Workstream and removed requires workstream approval strategic decision in spec team needed labels Jun 12, 2024
@BirgitBoss
Copy link
Collaborator

Workstream AAS Specs 2024-06-13
accepted to add recommendation
A minimum recursion depth of 32 for Container-Elements should be supported.
Additionally add note: This means for certification a maximum of 32 recursion test cases should be tested.

@BirgitBoss BirgitBoss added accepted accepted in principle to be fixed and removed ready for approval TF proposes how to resolve the issue. Needs final approval my Workstream labels Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted accepted in principle to be fixed
Projects
None yet
Development

No branches or pull requests

3 participants