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 <ttkacik@cisco.com>

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 5ce66874b129ce3684cca1638a4308646165d058..9eceeb1e436b96fd1e063e8b79e3a96bcaf54c94 100644 (file)
--- 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<T extends DOMDataWriteTransaction> extends
 
     protected final void doPut(final LogicalDatastoreType store,
             final InstanceIdentifier<?> path, final DataObject data) {
-        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec()
+       final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> 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.
+     *
+     * <p>
+     * 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.
+     *
+     * <p>
+     * This actually makes writes such as
+     * <pre>
+     * put("Nodes", new NodesBuilder().build());
+     * put("Nodes/Node[key]", new NodeBuilder().setKey("key").build());
+     * </pre>
+     * To result in three DOM operations:
+     * <pre>
+     * put("/nodes",domNodes);
+     * merge("/nodes/node",domNodeList);
+     * put("/nodes/node/node[key]",domNode);
+     * </pre>
+     *
+     *
+     * 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<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> 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<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier> getParent(
+            final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier child) {
+
+        Iterable<PathArgument> 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.<PathArgument>emptyList()));
+        } else {
+            return Optional.absent();
+        }
+    }
+
     protected final void doMerge(final LogicalDatastoreType store,
             final InstanceIdentifier<?> path, final DataObject data) {
+
         final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> 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 6b519955accd308109657dcb44823e4bca3d595d..8723fdf82a931b846482914fd5e85e69e8c10cf1 100644 (file)
--- 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<PathArgument> 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 (file)
index 0000000..ad6e1a7
--- /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> NODES_PATH = InstanceIdentifier.create(Nodes.class);
+
+    private static final NodeKey NODE_KEY = new NodeKey(new NodeId("foo"));
+
+    private static final InstanceIdentifier<Node> 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 1ea56381ef1cf9a4b248d2be2795d6c290f81c0d..8ba709ad30c461e5b69c50b0d34c9b2d78579fbb 100644 (file)
--- 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;
+    }
+
 
 }