Bug 1280: Ensure list parent if written item is list item 19/8619/3
authorTony Tkacik <ttkacik@cisco.com>
Thu, 3 Jul 2014 11:49:38 +0000 (13:49 +0200)
committerTony Tkacik <ttkacik@cisco.com>
Mon, 7 Jul 2014 09:45:54 +0000 (11:45 +0200)
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>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java
opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java [new file with mode: 0644]
opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/sal/binding/test/util/BindingTestContext.java

index 5ce6687..9eceeb1 100644 (file)
@@ -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());
     }
 
index 6b51995..8723fdf 100644 (file)
@@ -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
@@ -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());
+    }
+
+}
index 1ea5638..8ba709a 100644 (file)
@@ -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;
+    }
+
 
 }

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.