ModelGenerator: Put default values into the Class Constructor

[sebbader-sap]

The attributes SubmodelElementList.orderRelevant and HasKind.kind have default values (true and Instance) which have not been regarded by the model. Currently, if not set explicitly in the Builders, orderRelevant is initialised with false.

Proposal: Add a clause to the ontology or shapes, describing the default value, and add a function to the ModelGenerator to set these default values in the class constructors. For instance for Submodel:

public DefaultSubmodel() {
    this.kind = ModelKind.Instance ;
}

[sebbader-sap]

@mjacoby can you please review the constructors of https://github.com/eclipse-digitaltwin/aas4j/tree/update-to-rc02/model/src/main/java/org/eclipse/aas4j/v3/model, e.g., the DefaultSubmodel?

[mjacoby]

Most default values seem to be properly intialized. However, I found some more mentions of default values in the specification (v3.0RC02):

1. DataElement.category = "VARIABLE"
   Section 5.7.7.5, p.70, DataElement class definition, Constraint AASd-090:

   For data elements category (inherited by Referable) shall be one of the following values: CONSTANT, PARAMETER or VARIABLE. Default: VARIABLE

2. ConceptDescription.category = "PROPERTY"
   Section 5.7.8, p.81, ConceptDescriptionclass definition, Constraint AASd-051:

   A ConceptDescription shall have one of the following categories: VALUE, PROPERTY, REFERENCE, DOCUMENT, CAPABILITY, RELATIONSHIP, COLLECTION, FUNCTION, EVENT, ENTITY, APPLICATION_CLASS, QUALIFIER, VIEW. Default: PROPERTY.

3. AccessControl.selectable*
   Section 7.4.4, p.134, AccessControl class definition

   • selectableSubjectAttributes

     "Default: reference to the submodel referenced via defaultSubjectAttributes."

   • selectablePermissions

     "Default: reference to the submodel referenced via defaultPermissions."

   • selectableEnvironmentAttributes

     "Default: reference to the submodel referenced via defaultEnvironmentAttributes."

Conclusion

I'm not 100% sure how to handle these because these default values are defined differently from the other default values.

1 & 2 are defined as constraints, so they theoretically should not be relevant for designing the model classes. However, they define default values and the constructors seem to be the right place to handling default values. My personal opinion would be to handle default values in 1 & 2 like regular default values and propose to change the specification to move the default value definition from constraint to the class definition.

3 is part of the security part of the meta-model that we currently do not implement and which is currently not normative AFAIK. Therefore we can probably ignore it for now. However, we should also think how to address these default values once this part becomes normative as it cannot be solved with the generated code pattern we are using now.

@sebbader-sap What is your take on this?

[sebbader-sap]

The 'easy' topic first:

3 is part of the security part of the meta-model that we currently do not implement and which is currently not normative AFAIK.

True, therefore I also vote for leaving it out for now. There shall be a rework of the security model, and I want to wait until then.

[sebbader-sap]

1 & 2 are defined as constraints, so they theoretically should not be relevant for designing the model classes. However, they define default values and the constructors seem to be the right place to handling default values. My personal opinion would be to handle default values in 1 & 2 like regular default values and propose to change the specification to move the default value definition from constraint to the class definition.

I have a conceptual problem with these constraints in the generator, which currently prohibit me to find a proper pattern to generate the constructor...

[sebbader-sap]

I agree with you however that this should be the way to go.

[mjacoby]

What do you propose that we do now? From my perspective we are currently on hold until this issue is resolved in one way or another.

[sebbader-sap]

First, less meetings and more time to get things done :-)

But seriously, I can certainly come up with something in the generator today or tomorrow. If that's the only thing left which blocks us here, that'll be possible.

[mjacoby]

Ok, so we are going to set the default values for those two constraints in the constructors. That is something I can live and work with.
Once this is done we can continue there eclipse-aas4j/aas4j#22

[sebbader]

I need to further dig into this on Monday, no chance to get it fixed this week.

[sebbader-sap]

I just found a solution and pushed the newly generated classes: eclipse-aas4j/aas4j@b83940f @mjacoby please check them.

[mjacoby]

Looks good - all default values seem to be properly set.

However, I noticed two other issues

1. What is the expected value of properties that have a default value when they are explicitely set to null? Is it null or the default value? Kind of depends on the interpretation of default value...
   I guess for now the current implementation is ok, but we should think about this case.
2. The wildcard imports are back - probably because they are produced by the generator.
   This is nothing major and can for now stay as it is but we should put it in the backlog for the generator to create proper imports

[sebbader-sap]

The wildcard imports are back

I have noticed this, too. It's a bit of an arbitrary behaviour of the spotless-maven-plugin which I haven't completely understood yet.

[mjacoby]

Interesting, as FA³ST is using spotless and I never ever encountered that it introduces wildcard imports. So I would assume it's probably just an configuration issue. There seems to also be a feature in spotless to replace wildcard imports with regular imports diffplug/spotless#649 (comment)