From f4ddaed8236a05f191d031985a46d5b3b61f07bd Mon Sep 17 00:00:00 2001 From: Vaclav Demcak Date: Wed, 14 May 2014 14:39:10 +0200 Subject: [PATCH] Bug 1002 - REST POST transaction is not commited fix REST POST create workflow (create data only), fix according to specification http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#section-2.4 I'm missing implementation for ietf-restconf:errors, so I can't do more for now http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#page-48 * fix a check for an existing node in DataStore * add warn log and a correct ResponseException * fix tests for BrokerFacade Change-Id: I68c95e831f42cae4df0608a40c585251a631693f Signed-off-by: Vaclav Demcak --- .../sal/restconf/impl/BrokerFacade.java | 52 ++++++++---------- .../sal/restconf/impl/RestconfImpl.java | 29 ++++------ .../restconf/impl/test/BrokerFacadeTest.java | 54 +++++++++++-------- 3 files changed, 65 insertions(+), 70 deletions(-) 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 68c9340bb1..1cc1f783d6 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 @@ -7,9 +7,6 @@ */ package org.opendaylight.controller.sal.restconf.impl; -import com.google.common.base.Objects; - -import java.util.Map; import java.util.concurrent.Future; import javax.ws.rs.core.Response.Status; @@ -22,7 +19,6 @@ import org.opendaylight.controller.sal.core.api.data.DataChangeListener; import org.opendaylight.controller.sal.core.api.data.DataModificationTransaction; import org.opendaylight.controller.sal.core.api.mount.MountInstance; import org.opendaylight.controller.sal.rest.impl.RestconfProvider; -import org.opendaylight.controller.sal.restconf.impl.ResponseException; import org.opendaylight.controller.sal.streams.listeners.ListenerAdapter; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.common.QName; @@ -133,23 +129,22 @@ public class BrokerFacade implements DataReader> commitConfigurationDataPost( final InstanceIdentifier path, - final CompositeNode payload ) { + final CompositeNode payload) { this.checkPreconditions(); final DataModificationTransaction transaction = dataService.beginTransaction(); - transaction.putConfigurationData( path, payload ); - Map createdConfigurationData = - transaction.getCreatedConfigurationData(); - CompositeNode createdNode = createdConfigurationData.get( path ); - - if( Objects.equal( payload, createdNode ) ) { - LOG.trace( "Post Configuration via Restconf: {}", path ); - return transaction.commit(); + /* check for available Node in Configuration DataStore by path */ + CompositeNode availableNode = transaction.readConfigurationData( path ); + if (availableNode != null) { + String errMsg = "Post Configuration via Restconf was not executed because data already exists"; + BrokerFacade.LOG.warn((new StringBuilder(errMsg)).append(" : ").append(path).toString()); + // FIXME: return correct ietf-restconf:errors -> follow specification + // (http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#page-48) + throw new ResponseException(Status.CONFLICT, errMsg); } - - LOG.trace( "Post Configuration via Restconf was not executed because data already exists: {}", - path ); - return null; + BrokerFacade.LOG.trace( "Post Configuration via Restconf: {}", path ); + transaction.putConfigurationData( path, payload ); + return transaction.commit(); } public Future> commitConfigurationDataPostBehindMountPoint( @@ -157,19 +152,18 @@ public class BrokerFacade implements DataReader createdConfigurationData = - transaction.getCreatedConfigurationData(); - CompositeNode createdNode = createdConfigurationData.get( path ); - - if( Objects.equal( payload, createdNode ) ) { - LOG.trace( "Post Configuration via Restconf: {}", path ); - return transaction.commit(); + /* check for available Node in Configuration DataStore by path */ + CompositeNode availableNode = transaction.readConfigurationData( path ); + if (availableNode != null) { + String errMsg = "Post Configuration via Restconf was not executed because data already exists"; + BrokerFacade.LOG.warn((new StringBuilder(errMsg)).append(" : ").append(path).toString()); + // FIXME: return correct ietf-restconf:errors -> follow specification + // (http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#page-48) + throw new ResponseException(Status.CONFLICT, errMsg); } - - LOG.trace( "Post Configuration via Restconf was not executed because data already exists: {}", - path ); - return null; + BrokerFacade.LOG.trace( "Post Configuration via Restconf: {}", path ); + transaction.putConfigurationData( path, payload ); + return transaction.commit(); } public Future> commitConfigurationDataDelete( final InstanceIdentifier path ) { diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java index 2e198ec053..0b7b693b0c 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java @@ -7,14 +7,6 @@ */ package org.opendaylight.controller.sal.restconf.impl; -import com.google.common.base.Objects; -import com.google.common.base.Preconditions; -import com.google.common.base.Predicate; -import com.google.common.base.Splitter; -import com.google.common.base.Strings; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; - import java.net.URI; import java.text.ParseException; import java.text.SimpleDateFormat; @@ -34,17 +26,6 @@ import javax.ws.rs.core.UriInfo; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.sal.core.api.mount.MountInstance; import org.opendaylight.controller.sal.rest.api.RestconfService; -import org.opendaylight.controller.sal.restconf.impl.BrokerFacade; -import org.opendaylight.controller.sal.restconf.impl.CompositeNodeWrapper; -import org.opendaylight.controller.sal.restconf.impl.ControllerContext; -import org.opendaylight.controller.sal.restconf.impl.EmptyNodeWrapper; -import org.opendaylight.controller.sal.restconf.impl.IdentityValuesDTO; -import org.opendaylight.controller.sal.restconf.impl.InstanceIdWithSchemaNode; -import org.opendaylight.controller.sal.restconf.impl.NodeWrapper; -import org.opendaylight.controller.sal.restconf.impl.ResponseException; -import org.opendaylight.controller.sal.restconf.impl.RestCodec; -import org.opendaylight.controller.sal.restconf.impl.SimpleNodeWrapper; -import org.opendaylight.controller.sal.restconf.impl.StructuredData; import org.opendaylight.controller.sal.streams.listeners.ListenerAdapter; import org.opendaylight.controller.sal.streams.listeners.Notificator; import org.opendaylight.controller.sal.streams.websockets.WebSocketServer; @@ -76,6 +57,14 @@ import org.opendaylight.yangtools.yang.model.util.EmptyType; import org.opendaylight.yangtools.yang.parser.builder.impl.ContainerSchemaNodeBuilder; import org.opendaylight.yangtools.yang.parser.builder.impl.LeafSchemaNodeBuilder; +import com.google.common.base.Objects; +import com.google.common.base.Preconditions; +import com.google.common.base.Predicate; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; + @SuppressWarnings("all") public class RestconfImpl implements RestconfService { private final static RestconfImpl INSTANCE = new RestconfImpl(); @@ -681,6 +670,7 @@ public class RestconfImpl implements RestconfService { status = future == null ? null : future.get(); } } + catch( ResponseException e) { throw e; } catch( Exception e ) { throw new ResponseException( e, "Error creating data" ); } @@ -730,6 +720,7 @@ public class RestconfImpl implements RestconfService { status = future == null ? null : future.get(); } } + catch( ResponseException e) { throw e; } catch( Exception e ) { throw new ResponseException( e, "Error creating data" ); } 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 987beb072b..18199de8c6 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 @@ -8,13 +8,19 @@ package org.opendaylight.controller.sal.restconf.impl.test; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; -import java.util.Collections; import java.util.Map; import java.util.concurrent.Future; +import javax.ws.rs.core.Response.Status; + import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; @@ -212,18 +218,20 @@ public class BrokerFacadeTest { inOrder.verify( mockTransaction ).commit(); } - @Test + @Test(expected=ResponseException.class) public void testCommitConfigurationDataPostAlreadyExists() { when( dataBroker.beginTransaction() ).thenReturn( mockTransaction ); mockTransaction.putConfigurationData( instanceID, dataNode ); - when( mockTransaction.getCreatedConfigurationData() ) - .thenReturn( Collections.emptyMap() ); - - Future> actualFuture = - brokerFacade.commitConfigurationDataPost( instanceID, dataNode ); - - assertNull( "Retruned non-null Future", actualFuture ); - verify( mockTransaction, never() ).commit(); + when ( mockTransaction.readConfigurationData( instanceID ) ) + .thenReturn( dataNode ); + try { + brokerFacade.commitConfigurationDataPost( instanceID, dataNode ); + } catch (ResponseException e) { + assertEquals("Unexpect Exception Status -> " + + "http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#page-48", + (e.getResponse().getStatus()), Status.CONFLICT.getStatusCode()); + throw e; + } } @Test @@ -251,20 +259,22 @@ public class BrokerFacadeTest { inOrder.verify( mockTransaction ).commit(); } - @Test + @Test(expected=ResponseException.class) public void testCommitConfigurationDataPostBehindMountPointAlreadyExists() { when( mockMountInstance.beginTransaction() ).thenReturn( mockTransaction ); mockTransaction.putConfigurationData( instanceID, dataNode ); - when( mockTransaction.getCreatedConfigurationData() ) - .thenReturn( Collections.emptyMap() ); - - Future> actualFuture = - brokerFacade.commitConfigurationDataPostBehindMountPoint( mockMountInstance, - instanceID, dataNode ); - - assertNull( "Retruned non-null Future", actualFuture ); - verify( mockTransaction, never() ).commit(); + when ( mockTransaction.readConfigurationData( instanceID ) ) + .thenReturn( dataNode ); + try { + brokerFacade.commitConfigurationDataPostBehindMountPoint( mockMountInstance, + instanceID, dataNode ); + } catch (ResponseException e) { + assertEquals("Unexpect Exception Status -> " + + "http://tools.ietf.org/html/draft-bierman-netconf-restconf-03#page-48", + e.getResponse().getStatus(), Status.CONFLICT.getStatusCode()); + throw e; + } } @Test -- 2.36.6