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

Relaxation of idShort restrictions (Constraint AASd-002) #295

Closed
sebastiankb opened this issue Oct 25, 2023 · 12 comments
Closed

Relaxation of idShort restrictions (Constraint AASd-002) #295

sebastiankb opened this issue Oct 25, 2023 · 12 comments
Labels
accepted in principle acknowledged in work requires workstream approval strategic decision proposal needs to be prepared in TF spec team specification impact on specification and thus on xml, json etc., label "aas-core" not set additinally
Milestone

Comments

@sebastiankb
Copy link

sebastiankb commented Oct 25, 2023

What

idShort is currently very restricted defined in the specification:

idShort of Referables shall only feature letters, digits,
underscore (""); starting mandatory with a letter, i.e. [a-zA-Z][a-zA-Z0-9]*.

idShorts like "min-temperature-value" or "modbus:function" are not allowed.

idShorts have a variable-like character. However, I cannot follow why only "_" is allowed as a special character. This complicates the reusage of existing names (that are also variable-like characters), e.g., from standards that uses prefixes.

I tried to understand, where the restrictions come from and I received two resealable answers so far.:

  1. "should not cause conflicts with the idShort path approach of the REST interface."
  2. "variables in programming languages also do not support special characters"

Mitigation

Point 1 is not really a justification for this restriction, URL path allows many special characters such as “-”, “$”, “:” etc
Point 2 is more justifiable. However, it is hard to generalize this, since some programming languages allow “$” in variable names. But seriously questions: Why should idShort names identical be reflected as variable names in a programming language? Which tool/lib is doing this? AAS comes with its own serialization approaches. It makes more sense to reflect the serialization model in a programmatically manner (if needed).

Proposal

It makes sense, that idShorts have a variable-like character, however, more flexibility of the idShort values would be desirable, e.g., to adhere existing name conventions that uses also “-” and “:” in the name value.

Proposal 1: Allow more special characters such as “-” and “:”

and/or

Proposal 2: Allow “%” --> allows URL encoding (all special characters can be reflected)

Note: Proposal 1 and 2 are backward compatible!

@BirgitBoss BirgitBoss added the specification impact on specification and thus on xml, json etc., label "aas-core" not set additinally label Nov 8, 2023
@BirgitBoss
Copy link
Collaborator

There is a third reason:

  1. "should not cause conflicts with the idShort path approach of the REST interface."
  2. "variables in programming languages also do not support special characters"
  3. the Value-Only approach of the http/REST API is based on the idShort-Names, so only names valid in JSON should be allowed

We additionally do have a display name (in different languages even) for more elaborate names

@sebastiankb
Copy link
Author

sebastiankb commented Nov 8, 2023

Mitigation to the third reason: JSON key names are very flexible, e.g. spaces are also permitted ":", "-" etc.. Also see rfc8259

We additionally do have a display name (in different languages even) for more elaborate names

I know, but it's not the same, as display names are mainly used for (human-readable) UI purposes. This is more about keeping the naming convention for idShorts, especially for terms/variables that already exist, e.g. from standards. Many RFC and W3C standards use variable names which include "-" and ":" characters. In terms of interoperability and understandability, it would be nice if established names could be used as is.

@BirgitBoss BirgitBoss added the requires workstream approval strategic decision proposal needs to be prepared in TF spec team label Nov 16, 2023
@BirgitBoss
Copy link
Collaborator

Discussion in Workstream AAS on 2023-11-23

  • impact on existing implementation might be huge, because they rely on restrictive definition of idShort
  • especially special characters like % and : migth lead to problems, not all standard proxies might be able to deal with it
  • more difficult to implement because all cases need to be considered

Proposal:

allow "-"

Wish for 4.0:
idShort of Referables shall only feature letters, digits, hyphen ("-") and
underscore ("_"); starting mandatory with a letter and not ending with a hyphen or underscore, i.e. a-zA-Z? .

Decision:
Due to backward compatiblity we need to allow underscore at the end of the idShort:

idShort of Referables shall only feature letters, digits, hyphen ("-") and
underscore ("_"); starting mandatory with a letter and not ending with a hyphen, i.e. a-zA-Z? .

To be checked: regular expression with "-": correct like this?

@BirgitBoss BirgitBoss added this to the V3.1 milestone Nov 23, 2023
@BirgitBoss BirgitBoss added the accepted accepted in principle to be fixed label Nov 23, 2023
@sebastiankb
Copy link
Author

this should be the right regular expression:

^[a-zA-Z][a-zA-Z0-9_-]*[a-zA-Z0-9_]+$

@BirgitBoss
Copy link
Collaborator

this should be the right regular expression:

^[a-zA-Z][a-zA-Z0-9_-]*[a-zA-Z0-9_]+$

I think the regular expression is not backward compatible because it requests at least two characters (we relaxed this constraint with V3.0RC02)
This should be correct:

^[a-zA-Z] ([a-zA-Z0-9_-][a-zA-Z0-9_]+ | [a-zA-Z0-9_] ) $

@BirgitBoss
Copy link
Collaborator

Another issue: in Annex C Backus-Naur-Form we do not explain the characters ^ and $. What do they mean?
^ means beginning of line
$ means "end of line"

@BirgitBoss BirgitBoss changed the title Relaxation of idShort restrictions Relaxation of idShort restrictions AASD-aasd002 Jan 26, 2024
@BirgitBoss BirgitBoss changed the title Relaxation of idShort restrictions AASD-aasd002 Relaxation of idShort restrictions (Constraint AASd-002) Jan 26, 2024
BirgitBoss added a commit that referenced this issue Jan 26, 2024
see #295 
for backward compatibility reasons idShort can end with an underscore. Additionally also 1 char idShorts are allowed.
@BirgitBoss
Copy link
Collaborator

BirgitBoss commented Jan 27, 2024

see decision #295 (comment)

#Constraint AASd-002:# _idShort_ of __Referable__s shall only feature letters, digits, hyphen ("-") and underscore ("_"); starting mandatory with a letter, and not ending with a hyphen, i.e. ^[a-zA-Z] ([a-zA-Z0-9_-][a-zA-Z0-9_]+ | [a-zA-Z0-9_] ) $.]

For SMT submodel elements (see https://industrialdigitaltwin.org/en/content-hub/create-a-submodel) also other special characters like "{000}" are used.

Alternatives:
a) make three constraints, one for submodel elements with a Submodel instance and one for submodel elements within a Submodel template and one for elements not being a submodel element but referable
b) only one constraint for submodel instances (the existing AASd-002). This means everything allowed in SMT
c) extend existing constraint AASd-002

For a) and c) the question is how strict to make it: just
#Constraint AASd-00x:# _idShort_ of __SubmodelElement__s within a Submodel template (Submodel/kind = Template) shall only feature letters, digits, hyphen ("-") and underscore ("_"); starting mandatory with a letter, and not ending with a hyphen. Additionally for wildcards also {00} or {000} is allowed to be used. i.e. ^[a-zA-Z] ([a-zA-Z0-9_-][a-zA-Z0-9_]+ | [a-zA-Z0-9_] ) < { 0[0]+ }$.

@BirgitBoss BirgitBoss added the requires workstream approval strategic decision proposal needs to be prepared in TF spec team label Jan 27, 2024
BirgitBoss added a commit that referenced this issue Jan 29, 2024
* * (Editorial) MultiLanguageNameType inconsistent description, maxLength=128 (schemata were correct) ([#313](#313))
* (Editorial) HasKind/kind explanataroy text: use template, not type ([#313](#313))
* (Editorial) DataTypeDefXsd links now correctly to xmlschema11-2, not to xsd V1.1 ([#312](#312))
+ minor editorial changes

* New value "Role" in Enumeration AssetKind (#294)

* add new value "AssetKind" enumeration "Role"  #294
Change PathType to allow not only file:  #299

* Identifier: change length from 2000 to 2024 characters #299

* allow hyphens in idShort of Referables #295

* make Entity/entityType optional #287

* editorial change, add missing link to issue #295

* Update Constraint AASd-014 (part 2 of #287)

* EmbeddedDataSpecification/dataSpecification now mandatory #296

* Update Change Appendix w.r.t. EmbeddedDataSpecifcation/dataSpecification (now mandatory) #296

* #296 update Changes Section

* Editorial changes
#313

* semanticId of specificAssetId with name "globalAssetId"
#298

* Correct AASd-002

see #295 
for backward compatibility reasons idShort can end with an underscore. Additionally also 1 char idShorts are allowed.

* path .Specification.pdf

remove \

* fix typo
@sebastiankb
Copy link
Author

this should be the right regular expression:
^[a-zA-Z][a-zA-Z0-9_-]*[a-zA-Z0-9_]+$

I think the regular expression is not backward compatible because it requests at least two characters (we relaxed this constraint with V3.0RC02) This should be correct:

^[a-zA-Z] ([a-zA-Z0-9_-][a-zA-Z0-9]+ | [a-zA-Z0-9_]_ ) $

The pattern I have proposed takes this restriction into account with at least two characters. You can test this with this tool: https://regex101.com/

One character --> no match

image

Two character --> match

image

@sebastiankb
Copy link
Author

For SMT submodel elements (see https://industrialdigitaltwin.org/en/content-hub/create-a-submodel) also other special characters like "{000}" are used.

Alternatives: a) make three constraints, one for submodel elements with a Submodel instance and one for submodel elements within a Submodel template and one for elements not being a submodel element but referable b) only one constraint for submodel instances (the existing AASd-002). This means everything allowed in SMT c) extend existing constraint AASd-002

A good catch. I would prefer to make an exception for templates. On the other hand, it becomes inconsistent if we have different variants of constraints depending on the AAS form (template/instance/type). We should discuss this in the next meeting.

@BirgitBoss
Copy link
Collaborator

BirgitBoss commented Jan 31, 2024

into account with at least two characters

This is exactly NOT valid, 1-letter idShorts are valid as well

@sebastiankb
Copy link
Author

sebastiankb commented Jan 31, 2024

Sorry, I misunderstood. However, your proposed reg expression above do not allow 1-letter idShort either.

This version should work:

^[a-zA-Z]([a-zA-Z0-9_-]*[a-zA-Z0-9_]+)?$

s-heppner added a commit to aas-core-works/aas-core-meta that referenced this issue Feb 15, 2024
Previously, the `matches_id_short` regular expression only allowed
letters, digits and underscores.

With the decision of
[aas-specs#295](admin-shell-io/aas-specs#295),
this condition was relaxed to allow also hyphens.

Fixes #304
s-heppner added a commit to aas-core-works/aas-core-meta that referenced this issue Feb 19, 2024
Previously, the `matches_id_short` regular expression only allowed
letters, digits and underscores.

With the decision of
[aas-specs#295](admin-shell-io/aas-specs#295),
this condition was relaxed to allow also hyphens.

Fixes #304
s-heppner added a commit to aas-core-works/aas-core-meta that referenced this issue Feb 19, 2024
Previously, the `matches_id_short` regular expression only allowed
letters, digits and underscores.

With the decision of
[aas-specs#295](admin-shell-io/aas-specs#295),
this condition was relaxed to allow also hyphens.

Fixes #304
@BirgitBoss
Copy link
Collaborator

the regex is: ^[a-zA-Z][a-zA-Z0-9_-]*[a-zA-Z0-9_]+$

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted in principle acknowledged in work requires workstream approval strategic decision proposal needs to be prepared in TF spec team specification impact on specification and thus on xml, json etc., label "aas-core" not set additinally
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants