[MDSAL-730] Clean up InstanceIdentifier semantics Created: 10/Mar/22 Updated: 12/Mar/22 Resolved: 12/Mar/22 |
|
| Status: | Resolved |
| Project: | mdsal |
| Component/s: | Binding runtime |
| Affects Version/s: | None |
| Fix Version/s: | 9.0.0 |
| Type: | Improvement | Priority: | Highest |
| Reporter: | Robert Varga | Assignee: | Robert Varga |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
With org.opendaylight.mdsal.binding.dom.codec.api.IncorrectNestingException: interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev200120.Update is not a valid data tree child of SchemaRootCodecContext [interface org.opendaylight.yangtools.yang.binding.DataRoot] at org.opendaylight.mdsal.binding.dom.codec.api.IncorrectNestingException.create(IncorrectNestingException.java:26) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext.createDataTreeChildContext(SchemaRootCodecContext.java:280) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext$1.load(SchemaRootCodecContext.java:70) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext$1.load(SchemaRootCodecContext.java:67) at com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache.java:3533) at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2282) at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2159) at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2049) at com.google.common.cache.LocalCache.get(LocalCache.java:3966) at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3989) at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4950) at com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(LocalCache.java:4956) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext.getOrRethrow(SchemaRootCodecContext.java:394) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext.streamChild(SchemaRootCodecContext.java:231) at org.opendaylight.mdsal.binding.dom.codec.impl.DataContainerCodecContext.bindingPathArgumentChild(DataContainerCodecContext.java:113) at org.opendaylight.mdsal.binding.dom.codec.impl.SchemaRootCodecContext.bindingPathArgumentChild(SchemaRootCodecContext.java:376) at org.opendaylight.mdsal.binding.dom.codec.impl.BindingCodecContext.getCodecContextNode(BindingCodecContext.java:241) at org.opendaylight.mdsal.binding.dom.codec.impl.BindingCodecContext.newWriterAndIdentifier(BindingCodecContext.java:203) at org.opendaylight.mdsal.binding.dom.codec.impl.BindingCodecContext.toNormalizedNode(BindingCodecContext.java:522) at org.opendaylight.mdsal.binding.dom.codec.spi.ForwardingBindingDOMCodecServices.toNormalizedNode(ForwardingBindingDOMCodecServices.java:66) at org.opendaylight.protocol.bgp.rib.spi.AbstractRIBSupportTest.createNlriWithDrawnRoute(AbstractRIBSupportTest.java:125) at org.opendaylight.protocol.bgp.inet.IPv4RIBSupportTest.testDeleteRoutes(IPv4RIBSupportTest.java:85) As the error indicates, for some reason we are attempting to access a Notification as a data context – which is definitely not right. We are most likely missing a recognition that we need to access a class through Notification code path. The core problem is that InstanceIdentifier harks back to initial design where a DataObject was enough to identify an item. This capability was then relaxed to allow reuse of InstanceIdentifier to identify even RpcInput/RpcOutput/Notification and Groupings in the context of mdsal-binding-dom-codec. This is obviously wrong, as noted in Tie up the loose ends in InstanceIdentifier definition to require properly-rooted hierarchy, thus rejecting attempts to address invalid items. There are two different solutions. For simple cases, like
module foo {
namespace foo;
prefix foo;
container foo;
}
the following continues to work InstanceIdentifier<Foo> id = InstanceIdentifier.create(Foo.class); For more complex cases involving grouping and choices, such as: module foo {
namespace foo;
prefix foo;
choice foo {
container bar;
}
grouping grp {
container baz;
}
uses grp;
}
The solution is to go through a specifically-crafted InstanceIdentifierBuilder. The choice/case interface is already covered with pre-existing methods, but for the case of used groupings, the root needs to be specified like this: InstanceIdentifier<Bar> id = InstanceIdentifier.builderOfInherited(FooData.class, Bar.class).build(); Here FooData identifies the module in which the grouping is instantiated and Bar identifies the container as it would normally do. |
| Comments |
| Comment by Robert Varga [ 10/Mar/22 ] |
|
The problem boils down to a Notification data being identified as an InstanceIdentifier, beginning here:
public static <T extends DataObject> @NonNull InstanceIdentifier<T> create(final Class<@NonNull T> type) {
return (InstanceIdentifier<T>) create(ImmutableList.of(Item.of(type)));
}
This is quite wrong as Update is not a valid InstanceIdentifier (in the data tree sense, as YangInstanceIdentifier) and we using that to address nodes with: InstanceIdentifier{targetType=interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.attributes.reach.MpReachNlri, path=[org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev200120.Update, org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev200120.path.attributes.Attributes, org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.AttributesReach, org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.attributes.reach.MpReachNlri]}
This should not be allowed and we need a separate construct to address Notifications and other constructs, which are addressable through DataObjects, but are not data tree items. Otherwise this identifier could very well be used to query the datastore – and obviously would fail miserably. We need to introduce a more general concept and specialize it for data that is identified at a particular DataObject, be it a subclass of Notification, RpcInput or RpcOutput, with explicit handling for it.
|