BUG-4295: fix merge callsite 85/33485/1
authorRobert Varga <rovarga@cisco.com>
Mon, 25 Jan 2016 09:58:55 +0000 (10:58 +0100)
committerTony Tkacik <ttkacik@cisco.com>
Mon, 25 Jan 2016 15:32:27 +0000 (15:32 +0000)
This is a fixup of the previous patch. The problem was expanding a merge
into a new node, which used a direct value propagation with a merge.
That is not correct, as the merge should be pushed via
ModificationApplyOperation, which in case of unkeyed lists will turn it
into a write.

Change-Id: I93c8be80e4467b5d3a1e20f26f5576bd701b1375
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit dfe3c8193b6fd06090de7221e51e5857b75517b6)

yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.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/InMemoryDataTreeModificationCursor.java
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/UnkeyedListModificationStrategy.java

index 831c818647ff333b89366e02032f23126ae179ee..c6190f1cbc311d0c13422ebf859f148ac8fb1208 100644 (file)
@@ -160,7 +160,7 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
         final Collection<NormalizedNode<?, ?>> children = ((NormalizedNodeContainer) value).getValue();
         for (NormalizedNode<?, ?> c : children) {
             final PathArgument id = c.getIdentifier();
-            modification.modifyChild(id, resolveChildOperation(id).getChildPolicy(), version);
+            modification.modifyChild(id, resolveChildOperation(id), version);
         }
         return applyTouch(modification, currentMeta, version);
     }
@@ -169,7 +169,7 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
             final Collection<NormalizedNode<?, ?>> children, final Version version) {
         for (NormalizedNode<?, ?> c : children) {
             final ModificationApplyOperation childOp = resolveChildOperation(c.getIdentifier());
-            final ModifiedNode childNode = modification.modifyChild(c.getIdentifier(), childOp.getChildPolicy(), version);
+            final ModifiedNode childNode = modification.modifyChild(c.getIdentifier(), childOp, version);
             childOp.mergeIntoModifiedNode(childNode, c, version);
         }
     }
@@ -231,16 +231,6 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl
         throw new IllegalArgumentException("Unsupported operation " + modification.getOperation());
     }
 
-    @SuppressWarnings({"rawtypes", "unchecked"})
-    private NormalizedNode<?, ?> createEmptyValue(NormalizedNode<?, ?> value,
-            Collection<NormalizedNode<?, ?>> children) {
-        NormalizedNodeContainerBuilder builder = createBuilder(value);
-        for (NormalizedNode<?, ?> child : children) {
-            builder.removeChild(child.getIdentifier());
-        }
-        return builder.build();
-    }
-
     @Override
     protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, final Version version) {
         /*
index a32e75bbc645cc5ba5c8f19a7b5ef4e9428a3a8c..cf9b2c3377c92f46daf21b8361bccd345e603335 100644 (file)
@@ -159,7 +159,7 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
             operation = potential.get();
             ++i;
 
-            modification = modification.modifyChild(pathArg, operation.getChildPolicy(), version);
+            modification = modification.modifyChild(pathArg, operation, version);
         }
 
         return OperationWithModification.from(operation, modification);
index e13073af02d0a88ca2bf9aaf31ddb882b0cd664f..f837ac8d5148f3dabbbe754222cbe2b545177996 100644 (file)
@@ -34,7 +34,7 @@ final class InMemoryDataTreeModificationCursor extends AbstractCursor<InMemoryDa
         final Optional<ModificationApplyOperation> potential = op.getApplyOperation().getChild(child);
         if (potential.isPresent()) {
             final ModificationApplyOperation operation = potential.get();
-            final ModifiedNode modification = op.getModification().modifyChild(child, operation.getChildPolicy(),
+            final ModifiedNode modification = op.getModification().modifyChild(child, operation,
                 getParent().getVersion());
 
             return OperationWithModification.from(operation, modification);
index 299a52b5770f23439e71c6f6252f7119c7edef28..49a7873ac7b27e3290b20b0bdb82b5abf8b81b65 100644 (file)
@@ -167,12 +167,12 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
      * changes modification type to {@link ModificationType#SUBTREE_MODIFIED}
      *
      * @param child child identifier, may not be null
-     * @param childPolicy child tracking policy for the node we are looking for
+     * @param childOper Child operation
      * @param modVersion Version allocated by the calling {@link InMemoryDataTreeModification}
      * @return {@link ModifiedNode} for specified child, with {@link #getOriginal()}
      *         containing child metadata if child was present in original data.
      */
-    ModifiedNode modifyChild(@Nonnull final PathArgument child, @Nonnull final ChildTrackingPolicy childPolicy,
+    ModifiedNode modifyChild(@Nonnull final PathArgument child, @Nonnull final ModificationApplyOperation childOper,
             @Nonnull final Version modVersion) {
         clearSnapshot();
         if (operation == LogicalOperation.NONE) {
@@ -186,7 +186,7 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
         final Optional<TreeNode> currentMetadata = findOriginalMetadata(child, modVersion);
 
 
-        final ModifiedNode newlyCreated = new ModifiedNode(child, currentMetadata, childPolicy);
+        final ModifiedNode newlyCreated = new ModifiedNode(child, currentMetadata, childOper.getChildPolicy());
         if (operation == LogicalOperation.MERGE && value != null) {
             /*
              * We are attempting to modify a previously-unmodified part of a MERGE node. If the
@@ -195,7 +195,7 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode<Modif
             @SuppressWarnings({ "rawtypes", "unchecked" })
             final Optional<NormalizedNode<?, ?>> childData = ((NormalizedNodeContainer)value).getChild(child);
             if (childData.isPresent()) {
-                newlyCreated.updateValue(LogicalOperation.MERGE, childData.get());
+                childOper.mergeIntoModifiedNode(newlyCreated, childData.get(), modVersion);
             }
         }
 
index 4b569cef0ee9b9b48d0145a23036b16d485a604f..561f5fcc7876ee17cc98138a8cfab40895561c5a 100644 (file)
@@ -38,8 +38,8 @@ final class UnkeyedListModificationStrategy extends SchemaAwareApplyOperation {
 
     @Override
     protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) {
-        // A merge operation is promoted into a write
-        return applyWrite(modification, Optional.of(currentMeta), version);
+        throw new IllegalStateException(String.format("Merge of modification %s on unkeyed list should never be called",
+            modification));
     }
 
     @Override