-
Notifications
You must be signed in to change notification settings - Fork 35
Fix P0 AAS XML/AASX compliance and spec alignment #458
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stiebitzhofer ,
Many thanks for the PR!
I have some remarks.
General
During the upgradation of aas4j to 2.0 we have ignored some XML validation tests due to issues #389 and #390. However, when I unignored these tests, all unignored tests are failing. So, I think this PR needs to be adjusted in order to determine the root cause and fix it. Could you please check this as well?
...rmat-aasx/src/main/java/org/eclipse/digitaltwin/aas4j/v3/dataformat/aasx/AASXSerializer.java
Outdated
Show resolved
Hide resolved
| public static final Environment ENVIRONMENT = createEnvironment(); | ||
| public static final String AAS_3_1_DATA_SPECIFICATION_IEC_61360 = | ||
| "https://admin-shell.io/aas/3/1/DataSpecificationIec61360"; | ||
| "https://admin-shell.io/DataSpecificationTemplates/DataSpecificationIec61360/3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the latest documentation of IDTA-01003-a - Data Specification IEC61360, the schema for IEC61360 is included as embedded data specification in the release of IDTA-01001-3-1 or its bugfix releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to look up the link again (I should add that to the commit): https://industrialdigitaltwin.io/aas-specifications/IDTA-01003-a/v3.1.1/specification.html Your thoughts?
| private static final String SUBMODEL_OPERATIONAL_DATA_PROPERTY_VALUETYPE = "integer"; | ||
| public static final String AAS_3_1_DATA_SPECIFICATION_IEC_61360 = | ||
| "https://admin-shell.io/aas/3/1/DataSpecificationIec61360"; | ||
| "https://admin-shell.io/DataSpecificationTemplates/DataSpecificationIec61360/3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
| .type(KeyTypes.GLOBAL_REFERENCE) | ||
| .value( | ||
| "https://admin-shell.io/aas/3/1/DataSpecificationIec61360") | ||
| "https://admin-shell.io/DataSpecificationTemplates/DataSpecificationIec61360/3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
| "keys" : [ { | ||
| "type" : "GlobalReference", | ||
| "value" : "https://admin-shell.io/aas/3/1/DataSpecificationIec61360" | ||
| "value" : "https://admin-shell.io/DataSpecificationTemplates/DataSpecificationIec61360/3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above regarding 'DataSpecificationIec61360'
...aas4j/v3/dataformat/xml/internal/deserialization/EmbeddedDataSpecificationsDeserializer.java
Outdated
Show resolved
Hide resolved
| <aas:key> | ||
| <aas:type>GlobalReference</aas:type> | ||
| <aas:value>https://admin-shell.io/aas/3/1/DataSpecificationIec61360</aas:value> | ||
| <aas:value>https://admin-shell.io/DataSpecificationTemplates/DataSpecificationIec61360/3</aas:value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above regarding 'DataSpecificationIec61360'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above regarding 'DataSpecificationIec61360'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above regarding 'DataSpecificationIec61360'
1ca3c93 to
33f8c7f
Compare
…clipse-aas4j#456, eclipse-aas4j#390) - Fix SubmodelElementMixin property order to match XSD requirements - Change "hasExtensions" to "extensions" and "qualifier" to "qualifiers" - Remove duplicate properties from SubmodelElement ordering - Add OperationVariable empty list handling to prevent invalid XML - Remove test skip logic as all 2,593 XML validation tests now pass Signed-off-by: Hannes Stiebitzhofer <hannes@stiebitzhofer.com>
33f8c7f to
c2c61f3
Compare
Add SLF4J debug logging to help troubleshoot deserialization issues when attempting to deserialize DataSpecificationContent subtypes. The logging only activates when debug level is enabled and provides visibility into which classes were attempted and why they failed. Include unit tests that exercise the deserialization code paths with both valid IEC61360 data specifications and empty content. Signed-off-by: Hannes Stiebitzhofer <hannes@stiebitzhofer.com>
Remove automatic inclusion of AAS.xsd in AASX packages as this is not part of the official AASX specification (IDTA-01005-3-0). While including the schema enables offline validation, it is not explicitly required or recommended by the specification. Changes: - Remove addXmlSchemaPart() method from AASXSerializer - Remove XML_SCHEMA_PATH and XML_SCHEMA_RESOURCE constants - Remove XSD inclusion call in XML serialization path - Update AASXSerializerTest to remove XSD assertion Addresses review feedback from mdanish98 on PR eclipse-aas4j#458. Signed-off-by: Hannes Stiebitzhofer <hannes@stiebitzhofer.com>
Summary
AAS.xsdin AASX packagesTesting
mvn -Djacoco.skip=true -DargLine="-javaagent:/Users/hannes/.m2/repository/org/mockito/mockito-core/5.20.0/mockito-core-5.20.0.jar" testCommits
764c693Enforce AASd-108 SubmodelElementList typing (Enforce AASd-108 #230)650e494Update IEC61360 data specification template IDs (Consider Bugfix release IDTA-01003-a: Data Spec IDs of IEC61360 template changed #305)f6ae093Fix AAS XML order and include AAS.xsd in AASX (Non-compliant XML element order with AAS.xsd in AssetAdministrationShell serialization #456)843397fFix XML Operation serialization/deserialization wrappers ([Bug] Errors in XML serialization of operation properties #448)2403df3Handle embeddedDataSpecifications in XML submodel elements (EmbeddedDataSpecifications inside XML SubmodelElements are not deserialised #208)ecd3800Fix nested embeddedDataSpecifications XML deserialization (Unable to deserialize the nested embeddedDataSpecifications #389)