[MDSAL-440] Reconsider TypeObject equals()/hashCode() Created: 12/Apr/19  Updated: 07/Jan/20  Resolved: 19/Aug/19

Status: Resolved
Project: mdsal
Component/s: Binding codegen
Affects Version/s: None
Fix Version/s: 5.0.0

Type: Task Priority: Medium
Reporter: Robert Varga Assignee: Robert Varga
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to YANGTOOLS-418 Deficiencies in current yangtool gene... Confirmed
relates to MDSAL-90 Simple type derivation relationship l... Confirmed
relates to MDSAL-511 Default to returning Ipv{4,6}AddressN... Resolved

 Description   

TypeObjects are non-final for the purposes of subtyping, and thus have a non-final hashCode()/equals()/toString() methods.

As such they are extensible to potentially include other fields and have different equality contracts – and are using getClass() for their equality contract.

This seems counter-intuitive in their basic use, where they are being derived – derived TypeObjects do not override none of these methods. Furthermore toString() method in the first derived object uses strictly its class for identity.

The combination of these leads to quite surprising behavior:

  • equals() between base type and its derived type does not work, but
  • the base type can freely be substituted by the derived type, and
  • when such substitution happens toString() will not identify the object as being of derived type

From YANG perspective, both are just data, and hence as long as their canonical value matches, they are considered equal – but then YANG does not really consider substitution as each instantiation defines a particular type, which is to say the set constraints that are acceptable and nothing else.

I think the intent here was that the first derived type creates a type domain and binds the equality contract in terms of itself and the value it holds – which would match YANG semantics and reduce surprises – but in that case we should really define equality in terms of instanceof and make hashCode()/equals()/toString() final.

 

An example of a model which is impacted by this change looks roughly like this:

model example {
  typedef base-type {
    type string;
    length 1..5;
  }

  typedef derived-type {
    type base-type;
    length 2..5;
  }
}

Under the change proposed by this issue, the two generated objects will compare as equal if they contain "abc", whereas currently they do not (due to having a different class).

 

 Comments   
Comment by Robert Varga [ 12/Apr/19 ]

YANGTOOLS-418 CanonicalValue works exactly this way: it is a holder of a baseline type, which can be only narrowed-down – there is one Representation class, which defines the representation and further restrictions are captured by Validated Representation class, which must be a subclass of the Representation class.

This also works towards MDSAL-90, where the major problem is that if we bounce a TypeObject through DOM, it will lose its original type capture (which is expected), but the real problem is that it will no longer compare as equal – because it will use a different encapsulation.

To tie this together:

  • YANGTOOLS-418 defines the infrastructure to maintain information about the level of validation while, while forcing coherent equality (through a support class). Assuming proper end-to-end transport and adoption by MD-SAL (so that TypeObjects are CanonicalValues or their holders), this would allow MDSAL-90 be solved, i.e. the value would return back with the same class as it originated with, at least until we hit persistence (which would have to maintain complete type information) or externalization (which is happening in MDSAL-90, it's just that NormalizedNodes are equivalent of persistence without type information).
  • This issue would solve the underlying semantic problem of MDSAL-90 by making original and from-DOM objects compare as equal and indinguishable. Note that the derivation can be restored via instanceof checks and fallback instantiation of the derived type – which is equivalent of CanonicalValueValidator.validateRepresentation() I think. We could perhaps add TypeObject.tryCast() or similar, which would take the top-level derived class, and a Variant describing why the proposed value cannot be validated – but I am not convinced there are too many uses for it.

 

 

Comment by Robert Varga [ 12/Apr/19 ]

Another thing that needs to be evaluated is the impact on unions – I think they are squashing same TypeObjects and getClass() is what makes them work.

Comment by Robert Varga [ 12/Apr/19 ]

Some materials around this:

 

Generated at Wed Feb 07 20:09:49 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.