[MDSAL-434] Fix unordered keyed list mapping Created: 09/Apr/19 Updated: 23/Apr/20 Resolved: 23/Apr/20 |
|
| Status: | Resolved |
| Project: | mdsal |
| Component/s: | Binding codegen, Binding runtime |
| Affects Version/s: | None |
| Fix Version/s: | 6.0.0 |
| Type: | Improvement | Priority: | High |
| 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: |
|
||||||||
| Description |
|
Due to legacy reasons, the following snippet:
container foo {
list bar {
key baz;
leaf baz {
type string;
}
}
}
ends up being mapped as this:
interface Foo {
List<Bar> getBar();
}
which ends up being nasty for multiple reasons:
The mapping for keyed lists with "order-by system", the mapping should result in the following:
interface Foo {
Map<BarKey, Bar> getBar();
}
which properly captures the contract here, binding use to Map.equals(). Note that unkeyed lists must not be affected, nor should the "order-by user" lists be changed. For dealing with user-ordered lists we will need a custom interface, which is an extension of List and provides a secondary map-like lookups, still deriving equality from List.equals().
|
| Comments |
| Comment by Robert Varga [ 09/Apr/19 ] |
|
While the end result is obvious, the path towards it is not, as this change will affect a lot of users in less then subtle ways. We need to think about four primary aspects here:
For the setter part we can easily overload the current setter, as we can concurrently have
FooBuilder setBar(List<Bar> bar); FooBuilder setBar(Map<BarKey, Bar> bar);
and can simulate perform translation to either form – either through Maps.uniqueIndex() in the List->Map case, or through yangtools.util.CollectionWrappers.wrapAsList(Map.values()) in the Map -> List case. For the getter we have two options, depending on where we want to land, but we are still restricted by simple prefix:
Since we are past the point of making incompatible changes in MD-SAL 4.0.0, the field (and thus equality) needs to be pinned to List. In MD-SAL 5, the internal field should become a Map, thus fixing the equality problem. As for nonnull-getter interaction, there is no neat answer here, as it really needs to follow the primary getter – unless we allocate something ugly like 'nonnulllist' or 'nonnullmap' to bind the secondary getter. From runtime perspective, binding-dom-codec would be most happy if it could use maps directly today.
|
| Comment by Robert Varga [ 23/Apr/20 ] |
|
We need a transitional setter in the builder, otherwise the migration is just too much pain for downstreams. |