Make APIs type safe according to AAS meta model spec about aas4j

[sirea-07]

The problem with java.util.Optional is that it's not intended as a "maybe type", which obviously could deliberately be ignored, but more importantly it doesn't support null as a value. If there is a difference between null as a value and the property not being present, Optional cannot be used. Now I don't know if this is relevant for any property in AAS, but as an example due to this issue, OpenAPITools has added the type JsonNullable.

[atextor]

Yes, I'm aware of how Optional was originally intended. From the post you linked, citing Brian Goetz:

Our intention was to provide a limited mechanism for library method return types where there needed to be a clear way to represent "no result", and using null for such was overwhelmingly likely to cause errors.

This is exactly what we have here, and what I'm requesting.

I do know that in some cases distinguishing between present/not present/null is necessary (in particular for PUT/PATCH requests in REST APIs) and I also know JsonNullable. However, I don't think this is relevant here. The AAS meta model clearly states that some fields are optional, a value of null to indicate anything specifically, however, is not mentioned anywhere. Therefore Optional both can be used, and is used in the way it was intended.

[sirea-07]

Although I have a different interpretation of that quote, it doesn't really matter. As long as there is no need to differentiate between null and a value not being present, Optional is fine to use imo as well and I support your suggested change.

[mjacoby]

What exactly is the benefit of having optional properties return Optional<>? A getter that returns an Optional<> can still be null (if not explicitly enforced, e.g. in every setter method), i.e. you still have to check if the value is null before accessing it which means at the end of the day you need to check if the return value is null and if not again check Optional.isPresent().
In this case, what would be the semantics of each of those cases and wouldn't they in the end not have the same meaning?

It maybe is helpful that the AAS metamodel classes in AAS4J are designed in such a way that it is possible to create instances that are not valid, e.g. by not setting the idShort where it is actually required. We made the design decision that we think it is better to allow the creation of invalid instances and than add a validator component that will check if a given instance if compliant to the standard. Unfortunately, this validator is currently not available as we did not yet finish updating it to the metamodel v3.0.

Generally speaking, there are multiple things the AAS metamodel classes can be used for, e.g. as result format from loading AAS model files, to create AAS models from code or as payload for API operations. The only scenario I really think using Optional<> could be beneficial is when using the classes as payload for PATCH commands because in this case there is a strong semantic difference between a property being not present (meaning it should not be changed) or it being null (meaning it should be set to null, i.e. deleted/removed).
If you use AAS4J in this case to parse the payload, this information is lost, i.e. the metamodel classes are not helpful in this case.

My approach upon implementing PATCH using AAS4J was therefore not to parse the payload to the AAS4J classes but to treat it as a generic JSON Merge request and use libraries that can execute any JSON Merge request on any object instance.

Coming back to the potential implications of using Optional<> and assuming we would enforce that properties of this type could never actually be null but only Optional.empty, how would you suggest to handle collections that can be empty, e.g. Submodel.submodelElement or SubmodlElementCollection.value and so on? As they have a cardinality of 0..* I would consider them optional and therefore would expect the API to return something like Optional<Collection<>> which seems cumbersome.

For clarification, I do not categorically oppose the use of Optional<> but to make an informed decision we need a detailed list of pros and cons. If you think we should use Optional<> please provide your arguments in a comprehensible way, e.g. why it is not suitable to check if AdministrativeInformation#getVersion() returns null instead of Optional.empty.

[atextor]

Hi @mjacoby, thanks for the comprehensive answer.

What exactly is the benefit of having optional properties return Optional<>? A getter that returns an Optional<> can still be null (if not explicitly enforced, e.g. in every setter method), i.e. you still have to check if the value is null before accessing it which means at the end of the day you need to check if the return value is null and if not again check Optional.isPresent().

The point is to have caller code not ever receive null, so by providing Optional<>, the APIs agree to this contract and never return null intentionally. As a caller using a method that returns Optional<>, I can rely on never receiving null (if I do, then it's a bug). For the cases where getters return fields with arbitary values, you'd return an Optional.ofNullable() instead of the plain value. The benefit is that the API clearly communicates the optionality of the return value, in a way the compiler and an IDE can make use of it. When I call the method, autocompletion will directly tell me - at build time/compile time - that I have to deal with it, for example by calculating a fallback value via .orElseGet(). The alternatives are NullPointerExceptions that only appear at runtime, learning the behavior of every API method by heart, constantly switching between code and API documentation (which currently does not convey this information, but could be improved) to check for every method if it can return null, or defensively wrapping every API method call in a null-check. None of these options are very appealing.

The only scenario I really think using Optional<> could be beneficial is when using the classes as payload for PATCH commands

This is interesting, although not my current use case.

how would you suggest to handle collections that can be empty

Best practice here is to always return a (possibly empty) collection, never null nor Optional. This is exactly what 0..* means. For specific corner cases, e.g., a method List<T> getLastTwoElements(List<T> input) called with an empty or one-element input list, a corresponding documented exception should be thrown.

If you think we should use Optional<> please provide your arguments in a comprehensible way

Using Optional<> as return type in methods:

• clearly communicates the intent "the value could be intentionally missing" using the method signature
• allows autocomplete-based handling of cases at development time and saves time browsing API docs
• can reduce code size when processing the results of the method, because .map(), .flatMap(), .orElseGet() etc. can be used instead of if (... == null) checks.
• explicitly distinguishes between purposeful and accidental absence of a value, i.e., if null appears as a value, then it's clearly a bug
• has no real technical downsides apart from not being serializable - I assume have classes be serializable is irrelevant here, but I could be wrong.