From: Robert Varga Date: Mon, 29 Jan 2018 13:22:35 +0000 (+0100) Subject: MDSAL-298: properly handle unkeyed lists X-Git-Tag: release/oxygen~12 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=d6dc62697ecc8fea9098fb2011a666312c1f0ff7 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. It also reuses BindingStructuralType from md-sal, so that we do not have two copies of this critical class lying around requiring maintenance. Change-Id: I30599bc26d3c993c5b6c5e2f9e6f66144a530cb5 Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/sal-binding-broker/pom.xml b/opendaylight/md-sal/sal-binding-broker/pom.xml index 20c56be093..10e6fa6efd 100644 --- a/opendaylight/md-sal/sal-binding-broker/pom.xml +++ b/opendaylight/md-sal/sal-binding-broker/pom.xml @@ -81,6 +81,10 @@ org.opendaylight.mdsal mdsal-binding-dom-codec + + org.opendaylight.mdsal + mdsal-binding-dom-adapter + org.opendaylight.yangtools yang-data-impl diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingStructuralType.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingStructuralType.java deleted file mode 100644 index 64d98ed01a..0000000000 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingStructuralType.java +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Copyright (c) 2015 Cisco Systems, Inc. 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.controller.md.sal.binding.impl; - -import java.util.Optional; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.data.api.schema.AnyXmlNode; -import org.opendaylight.yangtools.yang.data.api.schema.AugmentationNode; -import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; -import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; -import org.opendaylight.yangtools.yang.data.api.schema.LeafNode; -import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; -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.tree.DataTreeCandidateNode; - -/** - * - * 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. - * - * See {@link #NOT_ADDRESSABLE},{@link #INVISIBLE_CONTAINER},{@link #VISIBLE_CONTAINER} - * for more details. - * - * - */ -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. - * - */ - NOT_ADDRESSABLE, - /** - * Data container is addressable in NormalizedNode format, - * but in Binding it is not represented in Instance Identifier. - * - * This are choice / case nodes. - * - * 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. - * - * 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. - * - */ - VISIBLE_CONTAINER, - /** - * Mapping algorithm was unable to detect type or was not updated after introduction - * of new NormalizedNode type. - */ - UNKNOWN; - - static BindingStructuralType from(final DataTreeCandidateNode domChildNode) { - Optional> dataBased = domChildNode.getDataAfter(); - if (!dataBased.isPresent()) { - dataBased = domChildNode.getDataBefore(); - } - if (dataBased.isPresent()) { - return from(dataBased.get()); - } - return from(domChildNode.getIdentifier()); - } - - private static BindingStructuralType from(final PathArgument identifier) { - if(identifier instanceof NodeIdentifierWithPredicates || identifier instanceof AugmentationIdentifier) { - return VISIBLE_CONTAINER; - } - if(identifier instanceof NodeWithValue) { - return NOT_ADDRESSABLE; - } - return UNKNOWN; - } - - static BindingStructuralType from(final NormalizedNode data) { - if(isNotAddressable(data)) { - return NOT_ADDRESSABLE; - } - if(data instanceof MapNode) { - return INVISIBLE_LIST; - } - if(data instanceof ChoiceNode) { - return INVISIBLE_CONTAINER; - } - if(isVisibleContainer(data)) { - return VISIBLE_CONTAINER; - } - return UNKNOWN; - } - - private static boolean isVisibleContainer(final NormalizedNode data) { - return data instanceof MapEntryNode || data instanceof ContainerNode || data instanceof AugmentationNode; - } - - private static boolean isNotAddressable(final NormalizedNode d) { - return d instanceof LeafNode - || d instanceof AnyXmlNode - || d instanceof LeafSetNode - || d instanceof LeafSetEntryNode; - } - -} diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/LazyDataObjectModification.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/LazyDataObjectModification.java index e92467d1d3..525ea4e391 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/LazyDataObjectModification.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/LazyDataObjectModification.java @@ -14,6 +14,7 @@ import java.util.Iterator; import java.util.List; import java.util.Optional; import org.opendaylight.controller.md.sal.binding.api.DataObjectModification; +import org.opendaylight.mdsal.binding.dom.adapter.BindingStructuralType; import org.opendaylight.mdsal.binding.dom.codec.api.BindingCodecTreeNode; import org.opendaylight.yangtools.yang.binding.Augmentation; import org.opendaylight.yangtools.yang.binding.ChildOf; @@ -44,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); @@ -136,28 +139,68 @@ final class LazyDataObjectModification implements DataObje } @Override - public DataObjectModification.ModificationType getModificationType() { - switch(domData.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; } @Override