Bug 2690 - Yang-Data-Impl: write and then merge on same list in

modification produces incorrect result

* added check for previous WRITE operation in recursiveMerge().
If there is some WRITE operation before MERGE we create new WRITE
operations for children => our data created by WRITE won't be
overwritten by merge on container/list.
* These children WRITE transactions will be recursively pushed down in the tree
while there are merge operations on them

Change-Id: Ie346c61c52cb000ee950d137f39d42e1ead3e1ff
Signed-off-by: Jan Hajnar <jhajnar@cisco.com>
(cherry picked from commit 2d7548bb1ebb925625358da86fba0831cd9b73ad)

diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java
index 6d20d385ee194b908f6aa015679324fb5cbd3e39..08fbcaa9a6f764ae7f46ce6e0f5c1211ce7ccbd0 100644 (file)
--- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java
+++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java
@@ -35,6 +35,22 @@ final class OperationWithModification {
         if (data instanceof NormalizedNodeContainer<?,?,?>) {
             @SuppressWarnings({ "rawtypes", "unchecked" })
             NormalizedNodeContainer<?,?,NormalizedNode<PathArgument, ?>> dataContainer = (NormalizedNodeContainer) data;
+
+            /*
+             * if there was write before on this node and it is of NormalizedNodeContainer type
+             * merge would overwrite our changes. So we create write modifications from data children to
+             * retain children created by past write operation.
+             * These writes will then be pushed down in the tree while there are merge modifications on these children
+             */
+            if (modification.getOperation().equals(LogicalOperation.WRITE)) {
+                @SuppressWarnings({ "rawtypes", "unchecked" })
+                NormalizedNodeContainer<?,?,NormalizedNode<PathArgument, ?>> odlDataContainer =
+                        (NormalizedNodeContainer) modification.getWrittenValue();
+                for (NormalizedNode<PathArgument, ?> child : odlDataContainer.getValue()) {
+                    PathArgument childId = child.getIdentifier();
+                    forChild(childId).write(child);
+                }
+            }
             for (NormalizedNode<PathArgument, ?> child : dataContainer.getValue()) {
                 PathArgument childId = child.getIdentifier();
                 forChild(childId).recursiveMerge(child);

diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/leafref/context/test/DataTreeCandidateValidatorTest3.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/leafref/context/test/DataTreeCandidateValidatorTest3.java
index e345354e335a99085714b51daa9c49ad57d2f3d1..1d9ad774f58adc1e81dbbaa729bf396b7c4d63ad 100644 (file)
--- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/leafref/context/test/DataTreeCandidateValidatorTest3.java
+++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/leafref/context/test/DataTreeCandidateValidatorTest3.java
@@ -150,7 +150,7 @@ public class DataTreeCandidateValidatorTest3 {
         final YangInstanceIdentifier devicesPath = YangInstanceIdentifier.of(devices);
         final DataTreeModification mergeModification = inMemoryDataTree
                 .takeSnapshot().newModification();
-        // mergeModification.write(devicesPath, devicesContainer);
+        mergeModification.write(devicesPath, devicesContainer);
         mergeModification.merge(devicesPath, devicesContainer);
 
         final DataTreeCandidate mergeDevicesCandidate = inMemoryDataTree

diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug2690FixTest.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug2690FixTest.java
new file mode 100644 (file)
index 0000000..50a17c4
--- /dev/null
+++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug2690FixTest.java
@@ -0,0 +1,99 @@
+/*
+ * 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.yangtools.yang.data.impl.schema.tree;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import com.google.common.base.Optional;
+import java.io.InputStream;
+import java.util.Collections;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+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.NormalizedNodeContainer;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException;
+import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
+import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
+import org.opendaylight.yangtools.yang.model.api.Module;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.parser.impl.YangParserImpl;
+
+public class Bug2690FixTest {
+    private static final String ODL_DATASTORE_TEST_YANG = "/odl-datastore-test.yang";
+    private SchemaContext schemaContext;
+    private RootModificationApplyOperation rootOper;
+
+    private InMemoryDataTree inMemoryDataTree;
+
+    @Before
+    public void prepare() {
+        schemaContext = createTestContext();
+        assertNotNull("Schema context must not be null.", schemaContext);
+        rootOper = RootModificationApplyOperation.from(SchemaAwareApplyOperation.from(schemaContext));
+        inMemoryDataTree =  (InMemoryDataTree) InMemoryDataTreeFactory.getInstance().create();
+        inMemoryDataTree.setSchemaContext(schemaContext);
+    }
+
+    public static final InputStream getDatastoreTestInputStream() {
+        return getInputStream(ODL_DATASTORE_TEST_YANG);
+    }
+
+    private static InputStream getInputStream(final String resourceName) {
+        return TestModel.class.getResourceAsStream(ODL_DATASTORE_TEST_YANG);
+    }
+
+    public static SchemaContext createTestContext() {
+        final YangParserImpl parser = new YangParserImpl();
+        final Set<Module> modules = parser.parseYangModelsFromStreams(Collections.singletonList(getDatastoreTestInputStream()));
+        return parser.resolveSchemaContext(modules);
+    }
+
+    @Test
+    public void testWriteMerge1() throws DataValidationFailedException {
+        final MapEntryNode fooEntryNode = ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, 1);
+        final MapEntryNode barEntryNode = ImmutableNodes.mapEntry(TestModel.OUTER_LIST_QNAME, TestModel.ID_QNAME, 2);
+        final MapNode mapNode1 = ImmutableNodes.mapNodeBuilder()
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TestModel.OUTER_LIST_QNAME))
+                .withChild(fooEntryNode).build();
+        final MapNode mapNode2 = ImmutableNodes.mapNodeBuilder()
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TestModel.OUTER_LIST_QNAME))
+                .withChild(barEntryNode).build();
+
+        final ContainerNode cont1 = Builders.containerBuilder()
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TestModel.TEST_QNAME))
+                .withChild(mapNode1).build();
+
+        final ContainerNode cont2 = Builders.containerBuilder()
+                .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TestModel.TEST_QNAME))
+                .withChild(mapNode2).build();
+
+        final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification();
+        modificationTree.write(TestModel.TEST_PATH, cont1);
+        modificationTree.merge(TestModel.TEST_PATH, cont2);
+
+        inMemoryDataTree.validate(modificationTree);
+        final DataTreeCandidate prepare = inMemoryDataTree.prepare(modificationTree);
+        inMemoryDataTree.commit(prepare);
+
+        final InMemoryDataTreeSnapshot snapshotAfterTx = inMemoryDataTree.takeSnapshot();
+        final InMemoryDataTreeModification modificationAfterTx = snapshotAfterTx.newModification();
+        final Optional<NormalizedNode<?, ?>> readNode = modificationAfterTx.readNode(TestModel.OUTER_LIST_PATH);
+        assertTrue(readNode.isPresent());
+        assertEquals(2, ((NormalizedNodeContainer<?,?,?>)readNode.get()).getValue().size());
+
+    }
+
+
+}

diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
index 7f4e20f8a6fb626eed56a9fd271d6813f1dee4a5..e866abb0d89d09102c3fe0e0ac20175b25ba638a 100644 (file)
--- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
+++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListConstraintsValidationTest.java
@@ -117,7 +117,7 @@ public class ListConstraintsValidationTest {
                 .withChild(barEntryNode).build();
 
         final InMemoryDataTreeModification modificationTree = inMemoryDataTree.takeSnapshot().newModification();
-        modificationTree.merge(MIN_MAX_LIST_PATH, mapNode1);
+        modificationTree.write(MIN_MAX_LIST_PATH, mapNode1);
         modificationTree.merge(MIN_MAX_LIST_PATH, mapNode2);
         // TODO: check why write and then merge on list commits only "bar" child