aas-core-works / aas-core-meta

Provide formalized meta-models for Asset Administration Shell (AAS).
Other
9 stars 3 forks source link

Put 0 as explicit lower bound in regex patterns #342

Open mhrimaz opened 3 weeks ago

mhrimaz commented 3 weeks ago

@s-heppner mentioned in https://github.com/admin-shell-io/aas-specs/pull/426#issuecomment-2175441483 that zero is optional in regex quantifiers. I don't think that is true, at least in Java this doesn't work.

image

This occurs in language tags and probably other places: https://github.com/aas-core-works/aas-core-meta/blob/31d6afd348a86f43cc658cc5bffab49f1e47bd24/aas_core_meta/v3_1.py#L286C6-L286C46

mristin commented 3 weeks ago

@mhrimaz There is no single regex language, unfortunately. We correctly transpile the patterns in Java: https://github.com/aas-core-works/aas-core3.0-java/blob/4aa354e023b8f003fff4706c4bf519e22fb09d38/src/main/java/aas_core/aas3_0/verification/Verification.java#L211

In most regex pattern languages (or better said, dialects?), zero is indeed optional. It all depends on which regex language and which regex engine you use.

Can you please elaborate a bit more on the issue?

mhrimaz commented 3 weeks ago

Well, if you want to validate the shacl shape(https://github.com/admin-shell-io/aas-specs/blob/9a118059bea5e9cf12405db7c871b5d5869acaa4/schemas/rdf/shacl-schema.ttl#L39), using a java shacl validator like Apache Jena, the issue appears again.

So I think having 0 is indeed better—more explicit and probably less troublesome. Does adding zero cause any issues with other engines?

mristin commented 3 weeks ago

@mhrimaz:

So I think having 0 is indeed better—more explicit and probably less troublesome. Does adding zero cause any issues with other engines?

Not as far as I know. @sebbader-sap @sebbader @s-heppner can you please double-check and let me know how to change the schemas?

XSD and JSON Schema probably suffer from this issue in Java as well?

@mhrimaz please also note that the Java standard regex library runs in exponential time. For some patterns, you have to use another library. This is fixed in the aas-core-java (and aas-core-cpp).

s-heppner commented 3 weeks ago

I'd be very careful with any Regex changes, they seem to often make the problem worse than better. However, I will bring this up in the respective IDTA working groups and will get back to you with an answer.

mhrimaz commented 3 weeks ago

XSD and JSON Schema probably suffer from this issue in Java as well?

In official repo (https://github.com/admin-shell-io/aas-specs/blob/9a118059bea5e9cf12405db7c871b5d5869acaa4/schemas/json/aas.json#L39) and in code-gen repo (https://github.com/aas-core-works/aas-core-codegen/blob/e1d8c19a08afaa18485c78fe1ff90934daa5decd/test_data/jsonschema/test_main/aas_core_meta.v3/expected_output/schema.json#L39C66-L40C11) language regex have zero in regex qunatifier ({0,2})

Same in XSD. Apparently, the regexes are not the same in SHACL.

mristin commented 3 weeks ago

@mhrimaz do I understand correctly: this issue affects only SHACL?

mhrimaz commented 3 weeks ago

@mristin it seems so. SHACL is using different regex patterns, including the utf related patterns (https://github.com/aas-core-works/aas-core-codegen/blob/e1d8c19a08afaa18485c78fe1ff90934daa5decd/test_data/rdf_shacl/test_main/expected/aas_core_meta.v3/expected_output/shacl-schema.ttl#L48).

But also in meta (https://github.com/aas-core-works/aas-core-meta/blob/31d6afd348a86f43cc658cc5bffab49f1e47bd24/aas_core_meta/v3_1.py#L286C6-L286C46), 0 is not included, so I don't know if a change is required there or not.

mristin commented 3 weeks ago

@mhrimaz thanks for fdigging into this! Aas-core-meta is fine as it is parsed, and Python requires no zeros.

I'll fix SHACL then soon, and @s-heppner can put it then on aas-specs repo.

mristin commented 3 weeks ago

@mhrimaz I made the fix in aas-core-codegen so that we use the canonical regex renderer. Can you please test on your side that the canonical patterns work as expected? Here's the new SHACL: https://github.com/aas-core-works/aas-core-codegen/pull/517/files#diff-123eac332ab2df0595966e11a9b259234031ebeae72724978db116956ac5f7e0

Once you give me a go, I'll merge the pull request in aas-core-codegen, and then we can update aas-specs repository.