Bug 2983: Added support for creation of multiple list entries

JSON payload for POST allows user to specify multiple list
entries to be created. This resulted in confusing behaviour
where only one entry was allowed.

Updated RESTCONF to allow multiple entry creation via POST
for lists only.

Change-Id: I7dc11098676d4e761e1b31a0504f3e5af75157f3
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>

diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonNormalizedNodeBodyReader.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonNormalizedNodeBodyReader.java
index 10399ffeffa85a20b7bb0c3c8de77aff069a86ac..4d8463adae66cb1e53ba06b2014b0bc8702fc4b4 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonNormalizedNodeBodyReader.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonNormalizedNodeBodyReader.java
@@ -90,7 +90,7 @@ public class JsonNormalizedNodeBodyReader extends AbstractIdentifierAwareJaxRsPr
             NormalizedNode<?, ?> partialResult = resultHolder.getResult();
             final NormalizedNode<?, ?> result;
 
-            // unwrap result from augmentation and choice nodes on PUT
+            // FIXME: Also II should be updated unwrap result from augmentation and choice nodes on PUT
             if (!isPost()) {
                 while (partialResult instanceof AugmentationNode || partialResult instanceof ChoiceNode) {
                     final Object childNode = ((DataContainerNode) partialResult).getValue().iterator().next();
@@ -98,7 +98,7 @@ public class JsonNormalizedNodeBodyReader extends AbstractIdentifierAwareJaxRsPr
                 }
             }
 
-            if (partialResult instanceof MapNode) {
+            if (partialResult instanceof MapNode && !isPost()) {
                 result = Iterables.getOnlyElement(((MapNode) partialResult).getValue());
             } else {
                 result = partialResult;

diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.java
index 0378ae40ee39aa09a2c2a40199b4a5161964bd50..6a3adcccd6576b8f7ac766eb521264b05a04f6b8 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/BrokerFacade.java
@@ -39,6 +39,7 @@ import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 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.impl.schema.ImmutableNodes;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
@@ -204,19 +205,36 @@ public class BrokerFacade {
             final YangInstanceIdentifier parentPath, final NormalizedNode<?, ?> payload, final SchemaContext schemaContext) {
         // FIXME: This is doing correct post for container and list children
         //        not sure if this will work for choice case
-        final YangInstanceIdentifier path;
-        if(payload instanceof MapEntryNode) {
-            path = parentPath.node(payload.getNodeType()).node(payload.getIdentifier());
+        if(payload instanceof MapNode) {
+            final YangInstanceIdentifier mapPath = parentPath.node(payload.getIdentifier());
+            final NormalizedNode<?, ?> emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, mapPath);
+            rWTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
+            ensureParentsByMerge(datastore, mapPath, rWTransaction, schemaContext);
+            for(final MapEntryNode child : ((MapNode) payload).getValue()) {
+                final YangInstanceIdentifier childPath = mapPath.node(child.getIdentifier());
+                checkItemDoesNotExists(rWTransaction, datastore, childPath);
+                rWTransaction.put(datastore, childPath, child);
+            }
         } else {
-            path = parentPath.node(payload.getIdentifier());
+            final YangInstanceIdentifier path;
+            if(payload instanceof MapEntryNode) {
+                path = parentPath.node(payload.getNodeType()).node(payload.getIdentifier());
+            } else {
+                path = parentPath.node(payload.getIdentifier());
+            }
+            checkItemDoesNotExists(rWTransaction,datastore, path);
+            ensureParentsByMerge(datastore, path, rWTransaction, schemaContext);
+            rWTransaction.put(datastore, path, payload);
         }
+        return rWTransaction.submit();
+    }
 
-        final ListenableFuture<Optional<NormalizedNode<?, ?>>> futureDatastoreData = rWTransaction.read(datastore, path);
+    private void checkItemDoesNotExists(final DOMDataReadWriteTransaction rWTransaction,final LogicalDatastoreType store, final YangInstanceIdentifier path) {
+        final ListenableFuture<Boolean> futureDatastoreData = rWTransaction.exists(store, path);
         try {
-            final Optional<NormalizedNode<?, ?>> optionalDatastoreData = futureDatastoreData.get();
-            if (optionalDatastoreData.isPresent() && payload.equals(optionalDatastoreData.get())) {
+            if (futureDatastoreData.get()) {
                 final String errMsg = "Post Configuration via Restconf was not executed because data already exists";
-                LOG.trace(errMsg + ":{}", path);
+                LOG.debug(errMsg + ":{}", path);
                 rWTransaction.cancel();
                 throw new RestconfDocumentedException("Data already exists for path: " + path, ErrorType.PROTOCOL,
                         ErrorTag.DATA_EXISTS);
@@ -225,10 +243,6 @@ public class BrokerFacade {
             LOG.trace("It wasn't possible to get data loaded from datastore at path " + path);
         }
 
-        ensureParentsByMerge(datastore, path, rWTransaction, schemaContext);
-        rWTransaction.merge(datastore, path, payload);
-        LOG.trace("Post " + datastore.name() + " via Restconf: {}", path);
-        return rWTransaction.submit();
     }
 
     private CheckedFuture<Void, TransactionCommitFailedException> putDataViaTransaction(

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
index 6542396612f6a2c4d57bc0226567e4ac507681c3..9c5de08c5ce1d3346c713986d3d1ce0d31ee84b0 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
@@ -197,13 +197,8 @@ public class BrokerFacadeTest {
         @SuppressWarnings("unchecked")
         final CheckedFuture<Void, TransactionCommitFailedException> expFuture = mock(CheckedFuture.class);
 
-        final NormalizedNode<?, ?> dummyNode2 = createDummyNode("dummy:namespace2", "2014-07-01", "dummy local name2");
-
-        when(rwTransaction.read(eq(LogicalDatastoreType.CONFIGURATION), any(YangInstanceIdentifier.class))).thenReturn(
-                wrapDummyNode(dummyNode2));
-
         when(rwTransaction.exists(eq(LogicalDatastoreType.CONFIGURATION), any(YangInstanceIdentifier.class))).thenReturn(
-            wrapExistence(true));
+            wrapExistence(false));
 
 
         when(rwTransaction.submit()).thenReturn(expFuture);
@@ -215,14 +210,16 @@ public class BrokerFacadeTest {
 
         final InOrder inOrder = inOrder(domDataBroker, rwTransaction);
         inOrder.verify(domDataBroker).newReadWriteTransaction();
-        inOrder.verify(rwTransaction).merge(LogicalDatastoreType.CONFIGURATION, instanceID, dummyNode);
+        inOrder.verify(rwTransaction).exists(LogicalDatastoreType.CONFIGURATION, instanceID);
+        inOrder.verify(rwTransaction).put(LogicalDatastoreType.CONFIGURATION, instanceID, dummyNode);
         inOrder.verify(rwTransaction).submit();
     }
 
     @Test(expected = RestconfDocumentedException.class)
     public void testCommitConfigurationDataPostAlreadyExists() {
-        when(rwTransaction.read(eq(LogicalDatastoreType.CONFIGURATION), any(YangInstanceIdentifier.class))).thenReturn(
-                dummyNodeInFuture);
+        final CheckedFuture<Boolean, ReadFailedException> successFuture = Futures.immediateCheckedFuture(Boolean.TRUE);
+        when(rwTransaction.exists(eq(LogicalDatastoreType.CONFIGURATION), any(YangInstanceIdentifier.class))).thenReturn(
+                successFuture);
         try {
             // Schema context is only necessary for ensuring parent structure
             brokerFacade.commitConfigurationDataPost((SchemaContext)null, instanceID, dummyNode);