From a2704d269a40eb966c5d308173e647d3dfa75003 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Fri, 24 Apr 2015 14:59:07 +0200 Subject: [PATCH] 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 (cherry picked from commit 12df4c8c678ce58aa0f82c34d0ce8e6cccc15dca) --- .../impl/JsonNormalizedNodeBodyReader.java | 4 +- .../sal/restconf/impl/BrokerFacade.java | 38 +++++++++++++------ .../restconf/impl/test/BrokerFacadeTest.java | 15 +++----- 3 files changed, 34 insertions(+), 23 deletions(-) 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 10399ffeff..4d8463adae 100644 --- 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 0378ae40ee..6a3adcccd6 100644 --- 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>> futureDatastoreData = rWTransaction.read(datastore, path); + private void checkItemDoesNotExists(final DOMDataReadWriteTransaction rWTransaction,final LogicalDatastoreType store, final YangInstanceIdentifier path) { + final ListenableFuture futureDatastoreData = rWTransaction.exists(store, path); try { - final Optional> 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 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 6542396612..9c5de08c5c 100644 --- 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 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 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); -- 2.36.6