From 972489522808ad9777d4c190a2a0c3896ed0fca4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 29 Jan 2018 14:18:05 +0100 Subject: [PATCH] MDSAL-298: properly handle unkeyed lists Unkeyed lists are not representable in binding, which means they cannot be reported as modified children, either. This has implications for any data change which contains unrepresentable fields as child modifications. Previously we would report SUBTREE_MODIFIED for the container node, but would fail to report any children, which is obviously wrong, as the user is left guessing as to what exactly happened. This patch modifies LazyDataObjectModification to report a WRITE event if modifications to unrepresentable children are found in SUBTREE_MODIFIED case. Since this is a potentially expensive operation, we cache a child addressability summary in BindingCodecTreeNode, so that we go to this slow path only when needed. We also expose BindingStructuralType enumeration, so controller's sal-binding-broker can reuse the implementation rather than having its own copy. Change-Id: I6642166cd262d0dddb1b2ed6d73a20785d0efff6 Signed-off-by: Robert Varga --- .../dom/adapter/BindingStructuralType.java | 91 +++-- .../adapter/LazyDataObjectModification.java | 61 ++- .../dom/adapter/test/Mdsal298Test.java | 385 ++++++++++++++++++ .../dom/codec/api/BindingCodecTreeNode.java | 29 ++ .../codec/impl/DataContainerCodecContext.java | 5 + .../impl/DataContainerCodecPrototype.java | 89 ++++ .../dom/codec/impl/LeafNodeCodecContext.java | 5 + .../src/main/yang/opendaylight-mdsal298.yang | 59 +++ 8 files changed, 675 insertions(+), 49 deletions(-) create mode 100644 binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal298Test.java create mode 100644 binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal298.yang diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingStructuralType.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingStructuralType.java index 5a7c70e3a5..e78d860e0d 100644 --- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingStructuralType.java +++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingStructuralType.java @@ -7,6 +7,7 @@ */ package org.opendaylight.mdsal.binding.dom.adapter; +import com.google.common.annotations.Beta; import java.util.Optional; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; @@ -22,71 +23,53 @@ import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListEntryNode; +import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; /** - * Defines structural mapping of Normalized Node to Binding data - * addressable by Instance Identifier. + * Defines structural mapping of Normalized Node to Binding data addressable by Instance Identifier. Not all binding + * data are addressable by instance identifier and there are some differences. * *

- * Not all binding data are addressable by instance identifier - * and there are some differences. + * See {@link #NOT_ADDRESSABLE},{@link #INVISIBLE_CONTAINER},{@link #VISIBLE_CONTAINER} for more details. * *

- * See {@link #NOT_ADDRESSABLE},{@link #INVISIBLE_CONTAINER},{@link #VISIBLE_CONTAINER} - * for more details. + * NOTE: this class is exposed for migration purposes only, no new users outside of its package should be introduced. */ -enum BindingStructuralType { - +@Beta +public enum BindingStructuralType { /** - * DOM Item is not addressable in Binding Instance Identifier, - * data is not lost, but are available only via parent object. - * - *

- * Such types of data are leaf-lists, leafs, list without keys - * or anyxml. - * + * DOM Item is not addressable in Binding InstanceIdentifier, data is not lost, but are available only via parent + * object. Such types of data are leaf-lists, leafs, list without keys or anyxml. */ NOT_ADDRESSABLE, /** - * Data container is addressable in NormalizedNode format, - * but in Binding it is not represented in Instance Identifier. + * Data container is addressable in NormalizedNode format, but in Binding it is not represented in + * InstanceIdentifier. These are choice / case nodes. * *

- * This are choice / case nodes. - * - *

- * This data is still accessible using parent object and their - * children are addressable. - * + * This data is still accessible using parent object and their children are addressable. */ INVISIBLE_CONTAINER, /** - * Data container is addressable in NormalizedNode format, - * but in Binding it is not represented in Instance Identifier. - * - *

- * This are list nodes. + * Data container is addressable in NormalizedNode format, but in Binding it is not represented in + * InstanceIdentifier. These are list nodes. * *

- * This data is still accessible using parent object and their - * children are addressable. - * + * This data is still accessible using parent object and their children are addressable. */ INVISIBLE_LIST, /** - * Data container is addressable in Binding Instance Identifier format - * and also YangInstanceIdentifier format. - * + * Data container is addressable in Binding InstanceIdentifier format and also YangInstanceIdentifier format. */ VISIBLE_CONTAINER, /** - * Mapping algorithm was unable to detect type or was not updated after introduction - * of new NormalizedNode type. + * Mapping algorithm was unable to detect type or was not updated after introduction of new NormalizedNode type. */ UNKNOWN; - static BindingStructuralType from(final DataTreeCandidateNode domChildNode) { + public static BindingStructuralType from(final DataTreeCandidateNode domChildNode) { Optional> dataBased = domChildNode.getDataAfter(); if (!dataBased.isPresent()) { dataBased = domChildNode.getDataBefore(); @@ -123,6 +106,35 @@ enum BindingStructuralType { return UNKNOWN; } + public static BindingStructuralType recursiveFrom(final DataTreeCandidateNode node) { + final BindingStructuralType type = BindingStructuralType.from(node); + switch (type) { + case INVISIBLE_CONTAINER: + case INVISIBLE_LIST: + // This node is invisible, try to resolve using a child node + for (final DataTreeCandidateNode child : node.getChildNodes()) { + final BindingStructuralType childType = recursiveFrom(child); + switch (childType) { + case INVISIBLE_CONTAINER: + case INVISIBLE_LIST: + // Invisible nodes are not addressable + return BindingStructuralType.NOT_ADDRESSABLE; + case NOT_ADDRESSABLE: + case UNKNOWN: + case VISIBLE_CONTAINER: + return childType; + default: + throw new IllegalStateException("Unhandled child type " + childType + " for child " + + child); + } + } + + return BindingStructuralType.NOT_ADDRESSABLE; + default: + return type; + } + } + private static boolean isVisibleContainer(final NormalizedNode data) { return data instanceof MapEntryNode || data instanceof ContainerNode || data instanceof AugmentationNode; } @@ -131,7 +143,8 @@ enum BindingStructuralType { return normalizedNode instanceof LeafNode || normalizedNode instanceof AnyXmlNode || normalizedNode instanceof LeafSetNode - || normalizedNode instanceof LeafSetEntryNode; + || normalizedNode instanceof LeafSetEntryNode + || normalizedNode instanceof UnkeyedListNode + || normalizedNode instanceof UnkeyedListEntryNode; } - } diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataObjectModification.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataObjectModification.java index 991d7a1115..bde86c9352 100644 --- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataObjectModification.java +++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/LazyDataObjectModification.java @@ -45,7 +45,9 @@ final class LazyDataObjectModification implements DataObje private final BindingCodecTreeNode codec; private final DataTreeCandidateNode domData; private final PathArgument identifier; - private Collection> childNodesCache; + + private volatile Collection> childNodesCache; + private volatile ModificationType modificationType; private LazyDataObjectModification(final BindingCodecTreeNode codec, final DataTreeCandidateNode domData) { this.codec = Preconditions.checkNotNull(codec); @@ -137,29 +139,68 @@ final class LazyDataObjectModification implements DataObje } @Override - public DataObjectModification.ModificationType getModificationType() { + public ModificationType getModificationType() { + ModificationType localType = modificationType; + if (localType != null) { + return localType; + } + switch (domData.getModificationType()) { case APPEARED: case WRITE: - return DataObjectModification.ModificationType.WRITE; - case SUBTREE_MODIFIED: - return DataObjectModification.ModificationType.SUBTREE_MODIFIED; + localType = ModificationType.WRITE; + break; case DISAPPEARED: case DELETE: - return DataObjectModification.ModificationType.DELETE; - + localType = ModificationType.DELETE; + break; + case SUBTREE_MODIFIED: + localType = resolveSubtreeModificationType(); + break; default: // TODO: Should we lie about modification type instead of exception? throw new IllegalStateException("Unsupported DOM Modification type " + domData.getModificationType()); } + + modificationType = localType; + return localType; + } + + private ModificationType resolveSubtreeModificationType() { + switch (codec.getChildAddressabilitySummary()) { + case ADDRESSABLE: + // All children are addressable, it is safe to report SUBTREE_MODIFIED + return ModificationType.SUBTREE_MODIFIED; + case UNADDRESSABLE: + // All children are non-addressable, report WRITE + return ModificationType.WRITE; + case MIXED: + // This case is not completely trivial, as we may have NOT_ADDRESSABLE nodes underneath us. If that + // is the case, we need to turn this modification into a WRITE operation, so that the user is able + // to observe those nodes being introduced. This is not efficient, but unfortunately unavoidable, + // as we cannot accurately represent such changes. + for (DataTreeCandidateNode child : domData.getChildNodes()) { + if (BindingStructuralType.recursiveFrom(child) == BindingStructuralType.NOT_ADDRESSABLE) { + // We have a non-addressable child, turn this modification into a write + return ModificationType.WRITE; + } + } + + // No unaddressable children found, proceed in addressed mode + return ModificationType.SUBTREE_MODIFIED; + default: + throw new IllegalStateException("Unsupported child addressability summary " + + codec.getChildAddressabilitySummary()); + } } @Override public Collection> getModifiedChildren() { - if (childNodesCache == null) { - childNodesCache = from(codec, domData.getChildNodes()); + Collection> local = childNodesCache; + if (local == null) { + childNodesCache = local = from(codec, domData.getChildNodes()); } - return childNodesCache; + return local; } @SuppressWarnings("unchecked") diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal298Test.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal298Test.java new file mode 100644 index 0000000000..7da7f65699 --- /dev/null +++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/test/Mdsal298Test.java @@ -0,0 +1,385 @@ +/* + * Copyright (c) 2018 Pantheon Technologies, s.ro.. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.mdsal.binding.dom.adapter.test; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; +import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATION; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.concurrent.ExecutionException; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.opendaylight.mdsal.binding.api.DataObjectModification; +import org.opendaylight.mdsal.binding.api.DataObjectModification.ModificationType; +import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; +import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; +import org.opendaylight.mdsal.binding.api.DataTreeModification; +import org.opendaylight.mdsal.binding.api.WriteTransaction; +import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteTransaction; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.AddressableCont; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.AddressableContBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.Container; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.ContainerBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.UnaddressableCont; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.UnaddressableContBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.WithChoice; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.WithChoiceBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.addressable.cont.AddressableChild; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.addressable.cont.AddressableChildBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.container.Keyed; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.container.KeyedBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.container.KeyedKey; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.container.Unkeyed; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.container.UnkeyedBuilder; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.Foo; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.foo.addressable._case.Addressable; +import org.opendaylight.yang.gen.v1.urn.test.opendaylight.mdsal298.rev180129.with.choice.foo.addressable._case.AddressableBuilder; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.Item; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; +import org.opendaylight.yangtools.yang.data.impl.schema.Builders; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; + +public class Mdsal298Test extends AbstractDataBrokerTest { + private static final InstanceIdentifier CONTAINER = InstanceIdentifier.create(Container.class); + private static final DataTreeIdentifier CONTAINER_TID = DataTreeIdentifier.create(CONFIGURATION, + CONTAINER); + private static final NodeIdentifier CONTAINER_NID = new NodeIdentifier(Container.QNAME); + private static final QName FOO_QNAME = QName.create(Container.QNAME, "foo"); + private static final QName BAZ_QNAME = QName.create(UnaddressableCont.QNAME, "baz"); + + private static final InstanceIdentifier CHOICE_CONTAINER = InstanceIdentifier.create(WithChoice.class); + private static final DataTreeIdentifier CHOICE_CONTAINER_TID = DataTreeIdentifier.create(CONFIGURATION, + CHOICE_CONTAINER); + private static final NodeIdentifier CHOICE_CONTAINER_NID = new NodeIdentifier(WithChoice.QNAME); + private static final NodeIdentifier CHOICE_NID = new NodeIdentifier(Foo.QNAME); + private static final InstanceIdentifier ADDRESSABLE_CASE = CHOICE_CONTAINER + .child((Class)Addressable.class); + + private static final InstanceIdentifier ADDRESSABLE_CONTAINER = + InstanceIdentifier.create(AddressableCont.class); + private static final DataTreeIdentifier ADDRESSABLE_CONTAINER_TID = DataTreeIdentifier.create( + CONFIGURATION, ADDRESSABLE_CONTAINER); + private static final NodeIdentifier ADDRESSABLE_CONTAINER_NID = new NodeIdentifier(AddressableCont.QNAME); + + private static final InstanceIdentifier UNADDRESSABLE_CONTAINER = + InstanceIdentifier.create(UnaddressableCont.class); + private static final DataTreeIdentifier UNADDRESSABLE_CONTAINER_TID = DataTreeIdentifier.create( + CONFIGURATION, UNADDRESSABLE_CONTAINER); + private static final NodeIdentifier UNADDRESSABLE_CONTAINER_NID = new NodeIdentifier(UnaddressableCont.QNAME); + + @Test + public void testKeyedDataTreeModification() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenContainer(Container.QNAME, Container.class, + new ContainerBuilder().build()); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(CONTAINER_NID).node(Keyed.QNAME), + Builders.orderedMapBuilder() + .withNodeIdentifier(new NodeIdentifier(Keyed.QNAME)) + .addChild(Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(Keyed.QNAME, ImmutableMap.of(FOO_QNAME, "foo"))) + .addChild(ImmutableNodes.leafNode(FOO_QNAME, "foo")) + .build()) + .addChild(Builders.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(Keyed.QNAME, ImmutableMap.of(FOO_QNAME, "bar"))) + .addChild(ImmutableNodes.leafNode(FOO_QNAME, "bar")) + .build()) + .build()); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification change = capture.iterator().next(); + assertEquals(CONTAINER_TID, change.getRootPath()); + final DataObjectModification changedContainer = change.getRootNode(); + assertEquals(new Item<>(Container.class), changedContainer.getIdentifier()); + assertEquals(ModificationType.SUBTREE_MODIFIED, changedContainer.getModificationType()); + + final Container containerAfter = changedContainer.getDataAfter(); + assertEquals(new ContainerBuilder() + .setKeyed(ImmutableList.of( + new KeyedBuilder().setFoo("foo").setKey(new KeyedKey("foo")).build(), + new KeyedBuilder().setFoo("bar").setKey(new KeyedKey("bar")).build())) + .build(), containerAfter); + + final Collection> changedChildren = changedContainer.getModifiedChildren(); + assertEquals(2, changedChildren.size()); + + final Iterator> it = changedChildren.iterator(); + final DataObjectModification changedChild1 = it.next(); + assertEquals(ModificationType.WRITE, changedChild1.getModificationType()); + assertEquals(Collections.emptyList(), changedChild1.getModifiedChildren()); + final Keyed child1After = (Keyed) changedChild1.getDataAfter(); + assertEquals("foo", child1After.getFoo()); + + final DataObjectModification changedChild2 = it.next(); + assertEquals(ModificationType.WRITE, changedChild2.getModificationType()); + assertEquals(Collections.emptyList(), changedChild2.getModifiedChildren()); + final Keyed child2After = (Keyed) changedChild2.getDataAfter(); + assertEquals("bar", child2After.getFoo()); + } + + @Test + public void testUnkeyedDataTreeModification() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenContainer(Container.QNAME, Container.class, + new ContainerBuilder().build()); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(CONTAINER_NID).node(Unkeyed.QNAME), + Builders.unkeyedListBuilder() + .withNodeIdentifier(new NodeIdentifier(Unkeyed.QNAME)) + .withChild(Builders.unkeyedListEntryBuilder() + .withNodeIdentifier(new NodeIdentifier(Unkeyed.QNAME)) + .addChild(ImmutableNodes.leafNode(FOO_QNAME, "foo")) + .build()) + .withChild(Builders.unkeyedListEntryBuilder() + .withNodeIdentifier(new NodeIdentifier(Unkeyed.QNAME)) + .addChild(ImmutableNodes.leafNode(FOO_QNAME, "bar")) + .build()) + .build()); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification change = capture.iterator().next(); + assertEquals(CONTAINER_TID, change.getRootPath()); + final DataObjectModification changedContainer = change.getRootNode(); + assertEquals(new Item<>(Container.class), changedContainer.getIdentifier()); + assertEquals(ModificationType.WRITE, changedContainer.getModificationType()); + + final Container containerAfter = changedContainer.getDataAfter(); + assertEquals(new ContainerBuilder() + .setUnkeyed(ImmutableList.of( + new UnkeyedBuilder().setFoo("foo").build(), + new UnkeyedBuilder().setFoo("bar").build())) + .build(), containerAfter); + + final Collection> changedChildren = changedContainer.getModifiedChildren(); + assertEquals(0, changedChildren.size()); + } + + @Test + public void testChoiceDataTreeModificationAddressable() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenWithChoice(); + + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + final WriteTransaction writeTx = getDataBroker().newWriteOnlyTransaction(); + writeTx.put(CONFIGURATION, ADDRESSABLE_CASE, new AddressableBuilder().build()); + writeTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification choiceChange = capture.iterator().next(); + assertEquals(CHOICE_CONTAINER_TID, choiceChange.getRootPath()); + final DataObjectModification changedContainer = choiceChange.getRootNode(); + assertEquals(ModificationType.SUBTREE_MODIFIED, changedContainer.getModificationType()); + assertEquals(new Item<>(WithChoice.class), changedContainer.getIdentifier()); + + final Collection> choiceChildren = changedContainer.getModifiedChildren(); + assertEquals(1, choiceChildren.size()); + + final DataObjectModification changedCase = (DataObjectModification) choiceChildren + .iterator().next(); + assertEquals(ModificationType.WRITE, changedCase.getModificationType()); + assertEquals(new Item<>(Addressable.class), changedCase.getIdentifier()); + assertEquals(new AddressableBuilder().build(), changedCase.getDataAfter()); + } + + @Test + public void testDataTreeModificationAddressable() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenContainer(AddressableCont.QNAME, + AddressableCont.class, new AddressableContBuilder().build()); + + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + final WriteTransaction writeTx = getDataBroker().newWriteOnlyTransaction(); + writeTx.put(CONFIGURATION, ADDRESSABLE_CONTAINER.child(AddressableChild.class), + new AddressableChildBuilder().build()); + writeTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification contChange = capture.iterator().next(); + assertEquals(ADDRESSABLE_CONTAINER_TID, contChange.getRootPath()); + final DataObjectModification changedContainer = contChange.getRootNode(); + assertEquals(ModificationType.SUBTREE_MODIFIED, changedContainer.getModificationType()); + assertEquals(new Item<>(AddressableCont.class), changedContainer.getIdentifier()); + + final Collection> contChildren = changedContainer.getModifiedChildren(); + assertEquals(1, contChildren.size()); + + final DataObjectModification changedChild = (DataObjectModification) contChildren + .iterator().next(); + assertEquals(ModificationType.WRITE, changedChild.getModificationType()); + assertEquals(new Item<>(AddressableChild.class), changedChild.getIdentifier()); + assertEquals(new AddressableChildBuilder().build(), changedChild.getDataAfter()); + } + + @Test + public void testDataTreeModificationUnaddressable() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenContainer(UnaddressableCont.QNAME, + UnaddressableCont.class, new UnaddressableContBuilder().build()); + + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(UNADDRESSABLE_CONTAINER_NID) + .node(QName.create(UnaddressableCont.QNAME, "baz")), + ImmutableNodes.leafNode(BAZ_QNAME, "baz")); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification contChange = capture.iterator().next(); + assertEquals(UNADDRESSABLE_CONTAINER_TID, contChange.getRootPath()); + final DataObjectModification changedContainer = contChange.getRootNode(); + assertEquals(ModificationType.WRITE, changedContainer.getModificationType()); + assertEquals(new Item<>(UnaddressableCont.class), changedContainer.getIdentifier()); + + final Collection> contChildren = changedContainer.getModifiedChildren(); + assertEquals(0, contChildren.size()); + } + + @Test + public void testChoiceDataTreeModificationUnaddressable() throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = assertWrittenWithChoice(); + + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(CHOICE_CONTAINER_NID).node(Foo.QNAME), + Builders.choiceBuilder() + .withNodeIdentifier(new NodeIdentifier(Foo.QNAME)) + .withChild(Builders.leafSetBuilder() + .withNodeIdentifier(new NodeIdentifier(QName.create(Foo.QNAME, "unaddressable"))) + .withChildValue("foo") + .build()) + .build()); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification choiceChange = capture.iterator().next(); + assertEquals(CHOICE_CONTAINER_TID, choiceChange.getRootPath()); + final DataObjectModification changedContainer = choiceChange.getRootNode(); + + // Should be write + assertEquals(ModificationType.WRITE, changedContainer.getModificationType()); + assertEquals(new Item<>(WithChoice.class), changedContainer.getIdentifier()); + + final Collection> choiceChildren = changedContainer.getModifiedChildren(); + assertEquals(0, choiceChildren.size()); + } + + private DataTreeChangeListener assertWrittenContainer(final QName qname, + final Class bindingClass, final T expected) + throws InterruptedException, ExecutionException { + final DataTreeChangeListener listener = mock(DataTreeChangeListener.class); + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + final DataTreeIdentifier dti = DataTreeIdentifier.create(CONFIGURATION, + InstanceIdentifier.create(bindingClass)); + getDataBroker().registerDataTreeChangeListener(dti, listener); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(new NodeIdentifier(qname)), + ImmutableNodes.containerNode(qname)); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification change = capture.iterator().next(); + assertEquals(dti, change.getRootPath()); + final DataObjectModification changedContainer = change.getRootNode(); + assertEquals(ModificationType.WRITE, changedContainer.getModificationType()); + assertEquals(new Item<>(bindingClass), changedContainer.getIdentifier()); + + final T containerAfter = changedContainer.getDataAfter(); + assertEquals(expected, containerAfter); + + // No further modifications should occur + assertEquals(Collections.emptyList(), changedContainer.getModifiedChildren()); + + reset(listener); + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + return listener; + } + + private DataTreeChangeListener assertWrittenWithChoice() throws InterruptedException, + ExecutionException { + final DataTreeChangeListener listener = mock(DataTreeChangeListener.class); + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + getDataBroker().registerDataTreeChangeListener(CHOICE_CONTAINER_TID, listener); + + final DOMDataTreeWriteTransaction domTx = getDomBroker().newWriteOnlyTransaction(); + domTx.put(CONFIGURATION, YangInstanceIdentifier.create(CHOICE_CONTAINER_NID), + Builders.containerBuilder() + .withNodeIdentifier(CHOICE_CONTAINER_NID) + .withChild(Builders.choiceBuilder().withNodeIdentifier(CHOICE_NID).build()) + .build()); + domTx.submit().get(); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(Collection.class); + verify(listener).onDataTreeChanged(captor.capture()); + Collection> capture = captor.getValue(); + assertEquals(1, capture.size()); + + final DataTreeModification change = capture.iterator().next(); + assertEquals(CHOICE_CONTAINER_TID, change.getRootPath()); + final DataObjectModification changedContainer = change.getRootNode(); + assertEquals(ModificationType.WRITE, changedContainer.getModificationType()); + assertEquals(new Item<>(WithChoice.class), changedContainer.getIdentifier()); + + final WithChoice containerAfter = changedContainer.getDataAfter(); + assertEquals(new WithChoiceBuilder().build(), containerAfter); + + // No further modifications should occur + assertEquals(Collections.emptyList(), changedContainer.getModifiedChildren()); + + reset(listener); + doNothing().when(listener).onDataTreeChanged(any(Collection.class)); + + return listener; + } +} diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/api/BindingCodecTreeNode.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/api/BindingCodecTreeNode.java index 1230a9f0a9..d6108b6966 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/api/BindingCodecTreeNode.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/api/BindingCodecTreeNode.java @@ -140,4 +140,33 @@ public interface BindingCodecTreeNode extends BindingNorma * @return A schema node. */ @Nonnull WithStatus getSchema(); + + /** + * Return a summary of addressability of potential children. Binding specification does not allow all DOM tree + * elements to be directly addressed, which means some recursive tree operations, like data tree changes do not + * have a one-to-one mapping from DOM to binding in all cases. This method provides an optimization hint to guide + * translation of data structures, allowing for fast paths when all children are known to either be addressable + * or non-addressable. + * + * @return Summary children addressability. + */ + @Nonnull ChildAddressabilitySummary getChildAddressabilitySummary(); + + /** + * Enumeration of possible addressability attribute of all children. + */ + enum ChildAddressabilitySummary { + /** + * All children are addressable. + */ + ADDRESSABLE, + /** + * All children are non-addressable, including the case when this node does not have any children. + */ + UNADDRESSABLE, + /** + * Mixed children, some are addressable and some are not. + */ + MIXED + } } diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataContainerCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataContainerCodecContext.java index 3d23477e1f..5d202d6c4d 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataContainerCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataContainerCodecContext.java @@ -42,6 +42,11 @@ abstract class DataContainerCodecContext implements NodeContextSupplier { + private static final Logger LOG = LoggerFactory.getLogger(DataContainerCodecPrototype.class); private final T schema; private final QNameModule namespace; @@ -36,6 +44,8 @@ final class DataContainerCodecPrototype implements NodeCon private final Class bindingClass; private final InstanceIdentifier.Item bindingArg; private final YangInstanceIdentifier.PathArgument yangArg; + private final ChildAddressabilitySummary childAddressabilitySummary; + private volatile DataContainerCodecContext instance = null; @SuppressWarnings({"rawtypes", "unchecked"}) @@ -53,6 +63,81 @@ final class DataContainerCodecPrototype implements NodeCon } else { this.namespace = arg.getNodeType().getModule(); } + + this.childAddressabilitySummary = computeChildAddressabilitySummary(nodeSchema); + } + + private static ChildAddressabilitySummary computeChildAddressabilitySummary(final WithStatus nodeSchema) { + if (nodeSchema instanceof DataNodeContainer) { + boolean haveAddressable = false; + boolean haveUnaddressable = false; + for (DataSchemaNode child : ((DataNodeContainer) nodeSchema).getChildNodes()) { + if (child instanceof ContainerSchemaNode || child instanceof AugmentationSchemaNode) { + haveAddressable = true; + } else if (child instanceof ListSchemaNode) { + if (((ListSchemaNode) child).getKeyDefinition().isEmpty()) { + haveUnaddressable = true; + } else { + haveAddressable = true; + } + } else if (child instanceof AnyDataSchemaNode || child instanceof AnyXmlSchemaNode + || child instanceof TypedDataSchemaNode) { + haveUnaddressable = true; + } else if (child instanceof ChoiceSchemaNode) { + switch (computeChildAddressabilitySummary(child)) { + case ADDRESSABLE: + haveAddressable = true; + break; + case MIXED: + haveAddressable = true; + haveUnaddressable = true; + break; + case UNADDRESSABLE: + haveUnaddressable = true; + break; + default: + throw new IllegalStateException("Unhandled accessibility summary for " + child); + } + } else { + LOG.warn("Unhandled child node {}", child); + } + } + + if (!haveAddressable) { + // Empty or all are unaddressable + return ChildAddressabilitySummary.UNADDRESSABLE; + } + + return haveUnaddressable ? ChildAddressabilitySummary.MIXED : ChildAddressabilitySummary.ADDRESSABLE; + } else if (nodeSchema instanceof ChoiceSchemaNode) { + boolean haveAddressable = false; + boolean haveUnaddressable = false; + for (CaseSchemaNode child : ((ChoiceSchemaNode) nodeSchema).getCases().values()) { + switch (computeChildAddressabilitySummary(child)) { + case ADDRESSABLE: + haveAddressable = true; + break; + case UNADDRESSABLE: + haveUnaddressable = true; + break; + case MIXED: + // A child is mixed, which means we are mixed, too + return ChildAddressabilitySummary.MIXED; + default: + throw new IllegalStateException("Unhandled accessibility summary for " + child); + } + } + + if (!haveAddressable) { + // Empty or all are unaddressable + return ChildAddressabilitySummary.UNADDRESSABLE; + } + + return haveUnaddressable ? ChildAddressabilitySummary.MIXED : ChildAddressabilitySummary.ADDRESSABLE; + } + + // No child nodes possible: return unaddressable + return ChildAddressabilitySummary.UNADDRESSABLE; } static DataContainerCodecPrototype rootPrototype(final CodecContextFactory factory) { @@ -83,6 +168,10 @@ final class DataContainerCodecPrototype implements NodeCon return schema; } + ChildAddressabilitySummary getChildAddressabilitySummary() { + return childAddressabilitySummary; + } + protected QNameModule getNamespace() { return namespace; } diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LeafNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LeafNodeCodecContext.java index 8e1fff3904..3df83dd09e 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LeafNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LeafNodeCodecContext.java @@ -223,6 +223,11 @@ final class LeafNodeCodecContext extends NodeCodecContext< return schema; } + @Override + public ChildAddressabilitySummary getChildAddressabilitySummary() { + return ChildAddressabilitySummary.UNADDRESSABLE; + } + /** * Return the default value object. * diff --git a/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal298.yang b/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal298.yang new file mode 100644 index 0000000000..cbfdd40c39 --- /dev/null +++ b/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal298.yang @@ -0,0 +1,59 @@ +module opendaylight-mdsal298 { + namespace "urn:test:opendaylight-mdsal298"; + prefix mdsal298; + + revision 2018-01-29; + + container container { + presence "for persistence"; + + list unkeyed { + ordered-by user; + leaf foo { + type string; + } + } + + list keyed { + ordered-by user; + key foo; + leaf foo { + type string; + } + } + } + + container with-choice { + presence "for persistence"; + + choice foo { + case addressable-case { + container addressable { + presence "for persistence"; + } + } + case unaddressable-case { + leaf-list unaddressable { + type string; + } + } + } + } + + container addressable-cont { + presence "for persistence"; + + container addressable-child { + presence "for persistence"; + } + } + + container unaddressable-cont { + presence "for persistence"; + + leaf baz { + type string; + } + } +} + -- 2.36.6