BUG-3674: delete of non-existing data is a no-op

[yangtools.git] / yang / yang-data-impl / src / main / java / org / opendaylight / yangtools / yang / data / impl / schema / tree / AbstractNodeContainerModificationStrategy.java

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java
index c0ddf6c4aa5f332d4c6c4cff3be79d2c97b34324..8a8afb046a859c2787280efdcc9a86b738dab662 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java
@@ -14,6 +14,7 @@ import java.util.Collection;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModifiedNodeDoesNotExistException;
@@ -140,7 +141,8 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
 
         /*
          * The user has issued an empty merge operation. In this case we do not perform
-         * a data tree mutation, do not pass GO, and do not collect useless garbage.
+         * a data tree mutation, do not pass GO, and do not collect useless garbage. It
+         * also means the ModificationType is UNMODIFIED.
          */
         final Collection<ModifiedNode> children = modification.getChildren();
         if (children.isEmpty()) {
@@ -150,21 +152,27 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
         }
 
         @SuppressWarnings("rawtypes")
-        NormalizedNodeContainerBuilder dataBuilder = createBuilder(currentMeta.getData());
+        final NormalizedNodeContainerBuilder dataBuilder = createBuilder(currentMeta.getData());
+        final TreeNode ret = mutateChildren(newMeta, dataBuilder, version, children);
 
         /*
-         * TODO: this is not entirely accurate. If there is only an empty merge operation
-         *       among the children, its effect is ModificationType.UNMODIFIED. That would
-         *       mean this operation can be turned into UNMODIFIED, cascading that further
-         *       up the root -- potentially turning the entire transaction into a no-op
-         *       from the perspective of physical replication.
+         * It is possible that the only modifications under this node were empty merges,
+         * which were turned into UNMODIFIED. If that is the case, we can turn this operation
+         * into UNMODIFIED, too, potentially cascading it up to root. This has the benefit
+         * of speeding up any users, who can skip processing child nodes.
          *
-         *       In order to do that, though, we either have to walk the children ourselves
-         *       (looking for a non-UNMODIFIED child), or have mutateChildren() pass that
-         *       information back to us.
+         * In order to do that, though, we have to check all child operations are UNMODIFIED.
+         * Let's do precisely that, stopping as soon we find a different result.
          */
-        modification.resolveModificationType(ModificationType.SUBTREE_MODIFIED);
-        return mutateChildren(newMeta, dataBuilder, version, children);
+        for (ModifiedNode child : children) {
+            if (child.getModificationType() != ModificationType.UNMODIFIED) {
+                modification.resolveModificationType(ModificationType.SUBTREE_MODIFIED);
+                return ret;
+            }
+        }
+
+        modification.resolveModificationType(ModificationType.UNMODIFIED);
+        return ret;
     }
 
     @Override
@@ -174,15 +182,24 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
             throw new ModifiedNodeDoesNotExistException(path, String.format("Node %s does not exist. Cannot apply modification to its children.", path));
         }
 
-        SchemaAwareApplyOperation.checkConflicting(path, current.isPresent(), "Node was deleted by other transaction.");
-        checkChildPreconditions(path, modification, current);
+        if (!current.isPresent()) {
+            throw new ConflictingModificationAppliedException(path, "Node was deleted by other transaction.");
+        }
+
+        checkChildPreconditions(path, modification, current.get());
     }
 
-    private void checkChildPreconditions(final YangInstanceIdentifier path, final NodeModification modification, final Optional<TreeNode> current) throws DataValidationFailedException {
-        final TreeNode currentMeta = current.get();
+    /**
+     * Recursively check child preconditions.
+     *
+     * @param path current node path
+     * @param modification current modification
+     * @param current Current data tree node.
+     */
+    private void checkChildPreconditions(final YangInstanceIdentifier path, final NodeModification modification, final TreeNode current) throws DataValidationFailedException {
         for (NodeModification childMod : modification.getChildren()) {
             final YangInstanceIdentifier.PathArgument childId = childMod.getIdentifier();
-            final Optional<TreeNode> childMeta = currentMeta.getChild(childId);
+            final Optional<TreeNode> childMeta = current.getChild(childId);
 
             YangInstanceIdentifier childPath = path.node(childId);
             resolveChildOperation(childId).checkApplicable(childPath, childMod, childMeta);
@@ -192,8 +209,8 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
     @Override
     protected void checkMergeApplicable(final YangInstanceIdentifier path, final NodeModification modification,
             final Optional<TreeNode> current) throws DataValidationFailedException {
-        if(current.isPresent()) {
-            checkChildPreconditions(path, modification,current);
+        if (current.isPresent()) {
+            checkChildPreconditions(path, modification, current.get());
         }
     }