MDSAL-298: properly handle unkeyed lists

[controller.git] / opendaylight / md-sal / sal-binding-broker / src / main / java / org / opendaylight / controller / md / sal / binding / impl / LazyDataObjectModification.java

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 e92467d1d3ed66770e1da990d55336352d141db0..525ea4e39140e646187e4c9428b19d10ef5e64b7 100644 (file)
--- 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<T extends DataObject> implements DataObje
     private final BindingCodecTreeNode<T> codec;
     private final DataTreeCandidateNode domData;
     private final PathArgument identifier;
-    private Collection<DataObjectModification<? extends DataObject>> childNodesCache;
+
+    private volatile Collection<DataObjectModification<? extends DataObject>> childNodesCache;
+    private volatile ModificationType modificationType;
 
     private LazyDataObjectModification(final BindingCodecTreeNode<T> codec, final DataTreeCandidateNode domData) {
         this.codec = Preconditions.checkNotNull(codec);
@@ -136,28 +139,68 @@ final class LazyDataObjectModification<T extends DataObject> 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<DataObjectModification<? extends DataObject>> getModifiedChildren() {
-        if (childNodesCache == null) {
-            childNodesCache = from(codec, domData.getChildNodes());
+        Collection<DataObjectModification<? extends DataObject>> local = childNodesCache;
+        if (local == null) {
+            childNodesCache = local = from(codec, domData.getChildNodes());
         }
-        return childNodesCache;
+        return local;
     }
 
     @Override