BUG-2383: deprecate ModificationType.MERGE 21/16821/3
authorRobert Varga <rovarga@cisco.com>
Thu, 19 Mar 2015 11:23:57 +0000 (12:23 +0100)
committerRobert Varga <nite@hq.sk>
Thu, 19 Mar 2015 14:43:44 +0000 (14:43 +0000)
The entire ModificationType enum is an accidental leak from the
implementation. The most pressing deficiency is that we do not want to
leak the merge operation, but report it either as a subtree modification
or a write.

Introduce a new type, LogicalOperation, which contains all the current
and use it internally. The new name also implies slight changes in how
the states are called:

ModificationType.UNMODIFIED maps to LogicalOperation.NONE
ModificationType.SUBTREE_MODIFIED maps to LogicalOperation.TOUCH

Change-Id: I446e4391583de060b2ab722deb5c004f60c8e6e7
Signed-off-by: Robert Varga <rovarga@cisco.com>
yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/ModificationType.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTree.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeCandidate.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/InMemoryDataTreeModification.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/LogicalOperation.java [new file with mode: 0644]
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModifiedNode.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/NodeModification.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/NoopDataTreeCandidate.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java

index 047d71e481bccae3df5bffd2014f6c9b12e94f80..3c2c299fff8735852f988dabb8773843bfb75e8d 100644 (file)
@@ -40,6 +40,11 @@ public enum ModificationType {
      * contents, it has been merged. This means that any incoming nodes which
      * were present in the tree have been replaced, but their child nodes have
      * been retained.
+     *
+     * @deprecated This value is never reported. Whenever it would be reported, based
+     *             on the actual effects, either {@link #WRITE} or {@link #SUBTREE_MODIFIED}
+     *             is reported.
      */
+    @Deprecated
     MERGE,
 }
index 215de62490fc031a20348509a5be8c259687c1f5..0f680fddadf30a6cf11410bf7ef2cb31e069c14a 100644 (file)
@@ -19,7 +19,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTree;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification;
 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.spi.TreeNode;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.slf4j.Logger;
@@ -85,7 +84,7 @@ final class InMemoryDataTree implements DataTree {
         final InMemoryDataTreeModification m = (InMemoryDataTreeModification)modification;
         final ModifiedNode root = m.getRootModification();
 
-        if (root.getType() == ModificationType.UNMODIFIED) {
+        if (root.getOperation() == LogicalOperation.NONE) {
             return new NoopDataTreeCandidate(PUBLIC_ROOT_PATH, root);
         }
 
index f6c5081bb67e2a0f0044122dfd665a8b6989e5f6..f6bb6ba2aac4e8da8c0ba922bd09d0f5621b43cb 100644 (file)
@@ -15,6 +15,10 @@ import java.util.Collection;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 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.OrderedLeafSetNode;
+import org.opendaylight.yangtools.yang.data.api.schema.OrderedMapNode;
+import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode;
@@ -69,7 +73,39 @@ final class InMemoryDataTreeCandidate extends AbstractDataTreeCandidate {
 
         @Override
         public ModificationType getModificationType() {
-            return mod.getType();
+            switch (mod.getOperation()) {
+            case DELETE:
+                return ModificationType.DELETE;
+            case MERGE:
+                // Merge into non-existing data is a write
+                if (oldMeta == null) {
+                    return ModificationType.WRITE;
+                }
+
+                // Data-based checks to narrow down types
+                final NormalizedNode<?, ?> data = newMeta.getData();
+
+                // leaf or anyxml are always written
+                if (!(data instanceof NormalizedNodeContainer)) {
+                    return ModificationType.WRITE;
+                }
+
+                // Unkeyed collections are always written
+                if (data instanceof UnkeyedListNode || data instanceof OrderedMapNode || data instanceof OrderedLeafSetNode) {
+                    return ModificationType.WRITE;
+                }
+
+                // Everything else is subtree modified
+                return ModificationType.SUBTREE_MODIFIED;
+            case TOUCH:
+                return ModificationType.SUBTREE_MODIFIED;
+            case NONE:
+                return ModificationType.UNMODIFIED;
+            case WRITE:
+                return ModificationType.WRITE;
+            }
+
+            throw new IllegalStateException("Unhandled internal operation " + mod.getOperation());
         }
 
         private Optional<NormalizedNode<?, ?>> optionalData(final TreeNode meta) {
index 0b0c8516d84969e262b2b9878574b14bbc073620..eb6b9726bece91e6d9afa97af1d3037b21187117 100644 (file)
@@ -16,7 +16,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgum
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.StoreTreeNodes;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version;
@@ -118,7 +117,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
 
     private ModificationApplyOperation resolveModificationStrategy(final YangInstanceIdentifier path) {
         LOG.trace("Resolving modification apply strategy for {}", path);
-        if (rootNode.getType() == ModificationType.UNMODIFIED) {
+        if (rootNode.getOperation() == LogicalOperation.NONE) {
             strategyTree.upgradeIfPossible();
         }
 
@@ -164,7 +163,7 @@ final class InMemoryDataTreeModification implements DataTreeModification {
     public DataTreeModification newModification() {
         Preconditions.checkState(sealed == 1, "Attempted to chain on an unsealed modification");
 
-        if (rootNode.getType() == ModificationType.UNMODIFIED) {
+        if (rootNode.getOperation() == LogicalOperation.NONE) {
             // Simple fast case: just use the underlying modification
             return snapshot.newModification();
         }
diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/LogicalOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/LogicalOperation.java
new file mode 100644 (file)
index 0000000..bdaa954
--- /dev/null
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2014 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.yangtools.yang.data.impl.schema.tree;
+
+import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
+
+/**
+ * Enumeration of all possible node modification states. These are used in
+ * data tree modification context to quickly assess what sort of modification
+ * the node is undergoing. This is a superset of {@link ModificationType}:
+ * where this type represents a logical operation, {@link ModificationType}
+ * represents a physical operation.
+ */
+enum LogicalOperation {
+    /**
+     * Node is currently unmodified.
+     */
+    NONE,
+
+    /**
+     * A child node, either direct or indirect, has been modified. This means
+     * that the data representation of this node has potentially changed.
+     */
+    TOUCH,
+
+    /**
+     * This node has been placed into the tree, potentially completely replacing
+     * pre-existing contents.
+     */
+    WRITE,
+
+    /**
+     * This node has been deleted along with any of its child nodes.
+     */
+    DELETE,
+
+    /**
+     * Node has been written into the tree, but instead of replacing pre-existing
+     * contents, it has been merged. This means that any incoming nodes which
+     * were present in the tree have been replaced, but their child nodes have
+     * been retained.
+     */
+    MERGE,
+}
index 448828bd9a0544b7f4e9d152d1f22c6e32aa5087..053e21ff2642b5fc5dfeca79fbbfaf514e8a4d44 100644 (file)
@@ -38,24 +38,24 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
         @Override
         public boolean apply(final @Nonnull ModifiedNode input) {
             Preconditions.checkNotNull(input);
-            switch (input.getType()) {
+            switch (input.getOperation()) {
             case DELETE:
             case MERGE:
             case WRITE:
                 return true;
-            case SUBTREE_MODIFIED:
-            case UNMODIFIED:
+            case TOUCH:
+            case NONE:
                 return false;
             }
 
-            throw new IllegalArgumentException(String.format("Unhandled modification type %s", input.getType()));
+            throw new IllegalArgumentException(String.format("Unhandled modification type %s", input.getOperation()));
         }
     };
 
     private final Map<PathArgument, ModifiedNode> children;
     private final Optional<TreeNode> original;
     private final PathArgument identifier;
-    private ModificationType modificationType = ModificationType.UNMODIFIED;
+    private LogicalOperation operation = LogicalOperation.NONE;
     private Optional<TreeNode> snapshotCache;
     private NormalizedNode<?, ?> value;
 
@@ -84,24 +84,15 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
         return identifier;
     }
 
-    /**
-     *
-     * Returns original store metadata
-     * @return original store metadata
-     */
     @Override
     Optional<TreeNode> getOriginal() {
         return original;
     }
 
-    /**
-     * Returns modification type
-     *
-     * @return modification type
-     */
+
     @Override
-    ModificationType getType() {
-        return modificationType;
+    LogicalOperation getOperation() {
+        return operation;
     }
 
     /**
@@ -131,8 +122,8 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
      */
     ModifiedNode modifyChild(final PathArgument child, final boolean isOrdered) {
         clearSnapshot();
-        if (modificationType == ModificationType.UNMODIFIED) {
-            updateModificationType(ModificationType.SUBTREE_MODIFIED);
+        if (operation == LogicalOperation.NONE) {
+            updateModificationType(LogicalOperation.TOUCH);
         }
         final ModifiedNode potential = children.get(child);
         if (potential != null) {
@@ -166,16 +157,16 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
      * Records a delete for associated node.
      */
     void delete() {
-        final ModificationType newType;
+        final LogicalOperation newType;
 
-        switch (modificationType) {
+        switch (operation) {
         case DELETE:
-        case UNMODIFIED:
+        case NONE:
             // We need to record this delete.
-            newType = ModificationType.DELETE;
+            newType = LogicalOperation.DELETE;
             break;
         case MERGE:
-        case SUBTREE_MODIFIED:
+        case TOUCH:
         case WRITE:
             /*
              * We are canceling a previous modification. This is a bit tricky,
@@ -185,10 +176,10 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
              * As documented in BUG-2470, a delete of data introduced in this
              * transaction needs to be turned into a no-op.
              */
-            newType = original.isPresent() ? ModificationType.DELETE : ModificationType.UNMODIFIED;
+            newType = original.isPresent() ? LogicalOperation.DELETE : LogicalOperation.NONE;
             break;
         default:
-            throw new IllegalStateException("Unhandled deletion of node with " + modificationType);
+            throw new IllegalStateException("Unhandled deletion of node with " + operation);
         }
 
         clearSnapshot();
@@ -204,14 +195,14 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
      */
     void write(final NormalizedNode<?, ?> value) {
         clearSnapshot();
-        updateModificationType(ModificationType.WRITE);
+        updateModificationType(LogicalOperation.WRITE);
         children.clear();
         this.value = value;
     }
 
     void merge(final NormalizedNode<?, ?> value) {
         clearSnapshot();
-        updateModificationType(ModificationType.MERGE);
+        updateModificationType(LogicalOperation.MERGE);
 
         /*
          * Blind overwrite of any previous data is okay, no matter whether the node
@@ -245,14 +236,14 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
             final ModifiedNode child = it.next();
             child.seal();
 
-            if (child.modificationType == ModificationType.UNMODIFIED) {
+            if (child.operation == LogicalOperation.NONE) {
                 it.remove();
             }
         }
 
-        // A SUBTREE_MODIFIED node without any children is a no-op
-        if (modificationType == ModificationType.SUBTREE_MODIFIED && children.isEmpty()) {
-            updateModificationType(ModificationType.UNMODIFIED);
+        // A TOUCH node without any children is a no-op
+        if (operation == LogicalOperation.TOUCH && children.isEmpty()) {
+            updateModificationType(LogicalOperation.NONE);
         }
     }
 
@@ -269,15 +260,15 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
         return snapshot;
     }
 
-    private void updateModificationType(final ModificationType type) {
-        modificationType = type;
+    private void updateModificationType(final LogicalOperation type) {
+        operation = type;
         clearSnapshot();
     }
 
     @Override
     public String toString() {
         return "NodeModification [identifier=" + identifier + ", modificationType="
-                + modificationType + ", childModification=" + children + "]";
+                + operation + ", childModification=" + children + "]";
     }
 
     /**
index f3282faa7bd7e20985a0c711e69be709e5dee6f2..225a931ec0a5eb65286cfe288936696bb28c27d1 100644 (file)
@@ -7,12 +7,10 @@
  */
 package org.opendaylight.yangtools.yang.data.impl.schema.tree;
 
-
 import com.google.common.base.Optional;
 import java.util.Collection;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode;
 
 /**
@@ -24,9 +22,9 @@ abstract class NodeModification implements Identifiable<PathArgument> {
     /**
      * Get the type of modification.
      *
-     * @return Modification type.
+     * @return Operation type.
      */
-    abstract ModificationType getType();
+    abstract LogicalOperation getOperation();
 
     /**
      * Get the original tree node to which the modification is to be applied.
index 7a91e232d85dce3f3286bb6e4b1c4f9826895495..979e79c1c71382ef74ba7cb1f4a375e072c76cc8 100644 (file)
@@ -57,7 +57,7 @@ final class NoopDataTreeCandidate extends AbstractDataTreeCandidate {
 
     protected NoopDataTreeCandidate(final YangInstanceIdentifier rootPath, final ModifiedNode modificationRoot) {
         super(rootPath);
-        Preconditions.checkArgument(modificationRoot.getType() == ModificationType.UNMODIFIED);
+        Preconditions.checkArgument(modificationRoot.getOperation() == LogicalOperation.NONE);
     }
 
     @Override
index 39c164faf1ecc5d3fb6945e42c8f22155c4d8aab..aaee9c30d8fb96c27c9ca216d87a09da2861fe16 100644 (file)
@@ -18,7 +18,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 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.IncorrectDataStructureException;
-import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version;
 import org.opendaylight.yangtools.yang.model.api.AugmentationSchema;
@@ -113,17 +112,17 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation {
 
     @Override
     public void verifyStructure(final ModifiedNode modification) throws IllegalArgumentException {
-        if (modification.getType() == ModificationType.WRITE) {
+        if (modification.getOperation() == LogicalOperation.WRITE) {
             verifyWrittenStructure(modification.getWrittenValue());
         }
     }
 
     @Override
     public final void checkApplicable(final YangInstanceIdentifier path,final NodeModification modification, final Optional<TreeNode> current) throws DataValidationFailedException {
-        switch (modification.getType()) {
+        switch (modification.getOperation()) {
         case DELETE:
             checkDeleteApplicable(modification, current);
-        case SUBTREE_MODIFIED:
+        case TOUCH:
             checkSubtreeModificationApplicable(path, modification, current);
             return;
         case WRITE:
@@ -132,10 +131,10 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation {
         case MERGE:
             checkMergeApplicable(path, modification, current);
             return;
-        case UNMODIFIED:
+        case NONE:
             return;
         default:
-            throw new UnsupportedOperationException("Suplied modification type "+ modification.getType()+ "is not supported.");
+            throw new UnsupportedOperationException("Suplied modification type "+ modification.getOperation()+ "is not supported.");
         }
 
     }
@@ -192,10 +191,10 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation {
     public final Optional<TreeNode> apply(final ModifiedNode modification,
             final Optional<TreeNode> currentMeta, final Version version) {
 
-        switch (modification.getType()) {
+        switch (modification.getOperation()) {
         case DELETE:
             return modification.setSnapshot(Optional.<TreeNode> absent());
-        case SUBTREE_MODIFIED:
+        case TOUCH:
             Preconditions.checkArgument(currentMeta.isPresent(), "Metadata not available for modification",
                     modification);
             return modification.setSnapshot(Optional.of(applySubtreeChange(modification, currentMeta.get(),
@@ -213,7 +212,7 @@ abstract class SchemaAwareApplyOperation implements ModificationApplyOperation {
             return modification.setSnapshot(Optional.of(result));
         case WRITE:
             return modification.setSnapshot(Optional.of(applyWrite(modification, currentMeta, version)));
-        case UNMODIFIED:
+        case NONE:
             return currentMeta;
         default:
             throw new IllegalArgumentException("Provided modification type is not supported.");