From e89320aae6e40db4600eec423f2e02160fadda80 Mon Sep 17 00:00:00 2001 From: Jan Hajnar Date: Mon, 27 Apr 2015 14:29:09 +0200 Subject: [PATCH] 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 (cherry picked from commit 2d7548bb1ebb925625358da86fba0831cd9b73ad) --- .../tree/OperationWithModification.java | 16 +++ .../test/DataTreeCandidateValidatorTest3.java | 2 +- .../data/impl/schema/tree/Bug2690FixTest.java | 99 +++++++++++++++++++ .../tree/ListConstraintsValidationTest.java | 2 +- 4 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug2690FixTest.java 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 6d20d385ee..08fbcaa9a6 100644 --- 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> 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> odlDataContainer = + (NormalizedNodeContainer) modification.getWrittenValue(); + for (NormalizedNode child : odlDataContainer.getValue()) { + PathArgument childId = child.getIdentifier(); + forChild(childId).write(child); + } + } for (NormalizedNode 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 e345354e33..1d9ad774f5 100644 --- 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 index 0000000000..50a17c46d2 --- /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 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> 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 7f4e20f8a6..e866abb0d8 100644 --- 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 -- 2.36.6