From 156015d027bd716b16b37ec90e129a603d59c477 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Thu, 3 Jul 2014 13:49:38 +0200 Subject: [PATCH] Bug 1280: Ensure list parent if written item is list item One of properties of Java Binding is that it is imposible to represent list as a whole and thus it was possible to create put operation, which was correct by Binding Spec but failed. This fixes behaviour, if user is writing list item (MapEntryNode), and whole list (MapNode) does not exists, list is automatically added. Change-Id: I41e0e30d349720f50697a6f555bab5850ec7d30c Signed-off-by: Tony Tkacik --- .../impl/AbstractWriteTransaction.java | 68 ++++++++++++++++++- .../impl/BindingToNormalizedNodeCodec.java | 21 ++++++ .../impl/test/WriteTransactionTest.java | 54 +++++++++++++++ .../binding/test/util/BindingTestContext.java | 17 ++++- 4 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java index 5ce66874b1..9eceeb1e43 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java @@ -7,18 +7,23 @@ */ package org.opendaylight.controller.md.sal.binding.impl; +import java.util.Collections; import java.util.Map.Entry; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction; import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.Identifiable; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.RpcResult; +import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Optional; +import com.google.common.collect.Iterables; import com.google.common.util.concurrent.ListenableFuture; /** @@ -38,15 +43,76 @@ public class AbstractWriteTransaction extends protected final void doPut(final LogicalDatastoreType store, final InstanceIdentifier path, final DataObject data) { - final Entry> normalized = getCodec() + final Entry> normalized = getCodec() .toNormalizedNode(path, data); + ensureListParentIfNeeded(store,path,normalized); getDelegate().put(store, normalized.getKey(), normalized.getValue()); } + + /** + * + * Ensures list parent if item is list, otherwise noop. + * + *

+ * One of properties of binding specification is that it is imposible + * to represent list as a whole and thus it is impossible to write + * empty variation of MapNode without creating parent node, with + * empty list. + * + *

+ * This actually makes writes such as + * 

+     * put("Nodes", new NodesBuilder().build());
+     * put("Nodes/Node[key]", new NodeBuilder().setKey("key").build());
+     *
+ * To result in three DOM operations: + * 
+     * put("/nodes",domNodes);
+     * merge("/nodes/node",domNodeList);
+     * put("/nodes/node/node[key]",domNode);
+     *
+ * + * + * In order to allow that to be inserted if necessary, if we know + * item is list item, we will try to merge empty MapNode or OrderedNodeMap + * to ensure list exists. + * + * @param store Data Store type + * @param path Path to data (Binding Aware) + * @param normalized Normalized version of data to be written + */ + private void ensureListParentIfNeeded(final LogicalDatastoreType store, final InstanceIdentifier path, + final Entry> normalized) { + if(Identifiable.class.isAssignableFrom(path.getTargetType())) { + org.opendaylight.yangtools.yang.data.api.InstanceIdentifier parentMapPath = getParent(normalized.getKey()).get(); + NormalizedNode emptyParent = getCodec().getDefaultNodeFor(parentMapPath); + getDelegate().merge(store, parentMapPath, emptyParent); + } + + } + + // FIXME (should be probaly part of InstanceIdentifier) + protected static Optional getParent( + final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier child) { + + Iterable mapEntryItemPath = child.getPathArguments(); + int parentPathSize = Iterables.size(mapEntryItemPath) - 1; + if(parentPathSize > 1) { + return Optional.of(org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.create(Iterables.limit(mapEntryItemPath, parentPathSize))); + } else if(parentPathSize == 0) { + return Optional.of(org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.create(Collections.emptyList())); + } else { + return Optional.absent(); + } + } + protected final void doMerge(final LogicalDatastoreType store, final InstanceIdentifier path, final DataObject data) { + final Entry> normalized = getCodec() .toNormalizedNode(path, data); + ensureListParentIfNeeded(store,path,normalized); getDelegate().merge(store, normalized.getKey(), normalized.getValue()); } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java index 6b519955ac..8723fdf82a 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.md.sal.binding.impl; import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.AbstractMap.SimpleEntry; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map.Entry; @@ -470,4 +471,24 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return Optional.absent(); } } + + /** + * Returns an default object according to YANG schema for supplied path. + * + * @param path DOM Path + * @return Node with defaults set on. + */ + public NormalizedNode getDefaultNodeFor(final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier path) { + Iterator iterator = path.getPathArguments().iterator(); + DataNormalizationOperation currentOp = legacyToNormalized.getRootOperation(); + while (iterator.hasNext()) { + PathArgument currentArg = iterator.next(); + try { + currentOp = currentOp.getChild(currentArg); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Invalid child encountered in path %s", path), e); + } + } + return currentOp.createDefault(path.getLastPathArgument()); + } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java new file mode 100644 index 0000000000..ad6e1a77ce --- /dev/null +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java @@ -0,0 +1,54 @@ +/* + * 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.controller.md.sal.binding.impl.test; + +import static org.junit.Assert.assertEquals; + +import java.util.concurrent.ExecutionException; + +import org.junit.Test; +import org.opendaylight.controller.md.sal.binding.api.DataBroker; +import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; +import org.opendaylight.controller.md.sal.common.api.TransactionStatus; +import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.controller.sal.binding.test.AbstractDataServiceTest; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodesBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + + +public class WriteTransactionTest extends AbstractDataServiceTest { + + private DataBroker dataBroker; + + private static final InstanceIdentifier NODES_PATH = InstanceIdentifier.create(Nodes.class); + + private static final NodeKey NODE_KEY = new NodeKey(new NodeId("foo")); + + private static final InstanceIdentifier NODE_PATH = NODES_PATH.child(Node.class, NODE_KEY); + + @Override + public void setUp() { + super.setUp(); + + dataBroker = testContext.getDataBroker(); + } + + @Test + public void test() throws InterruptedException, ExecutionException { + WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction(); + writeTx.put(LogicalDatastoreType.OPERATIONAL, NODES_PATH, new NodesBuilder().build()); + writeTx.put(LogicalDatastoreType.OPERATIONAL, NODE_PATH, new NodeBuilder().setKey(NODE_KEY).build()); + assertEquals(TransactionStatus.COMMITED, writeTx.commit().get().getResult()); + } + +} diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/sal/binding/test/util/BindingTestContext.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/sal/binding/test/util/BindingTestContext.java index 1ea56381ef..8ba709ad30 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/sal/binding/test/util/BindingTestContext.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/sal/binding/test/util/BindingTestContext.java @@ -17,7 +17,9 @@ import java.util.concurrent.Future; import javassist.ClassPool; +import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.impl.ForwardedBackwardsCompatibleDataBroker; +import org.opendaylight.controller.md.sal.binding.impl.ForwardedBindingDataBroker; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker; import org.opendaylight.controller.md.sal.dom.broker.impl.DOMDataBrokerImpl; @@ -112,6 +114,8 @@ public class BindingTestContext implements AutoCloseable { private final MockSchemaService mockSchemaService = new MockSchemaService(); + private DataBroker dataBroker; + public DOMDataBroker getDomAsyncDataBroker() { @@ -150,6 +154,12 @@ public class BindingTestContext implements AutoCloseable { biDataLegacyBroker = biDataImpl; } + public void startNewDataBroker() { + checkState(executor != null, "Executor needs to be set"); + checkState(newDOMDataBroker != null, "DOM Data Broker must be set"); + dataBroker = new ForwardedBindingDataBroker(newDOMDataBroker, mappingServiceImpl, mockSchemaService); + } + public void startNewDomDataBroker() { checkState(executor != null, "Executor needs to be set"); InMemoryDOMDataStore operStore = new InMemoryDOMDataStore("OPER", executor); @@ -312,11 +322,12 @@ public class BindingTestContext implements AutoCloseable { public void start() { startNewDomDataBroker(); + startDomBroker(); startDomMountPoint(); startBindingToDomMappingService(); + startNewDataBroker(); startNewBindingDataBroker(); - startBindingNotificationBroker(); startBindingBroker(); @@ -418,5 +429,9 @@ public class BindingTestContext implements AutoCloseable { return biMountImpl; } + public DataBroker getDataBroker() { + return dataBroker; + } + } -- 2.36.6