Merge "Bug 1002 - REST POST transaction is not commited"

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 68c9340bb1672551c1479a8a267474bd7302cfd3..1cc1f783d676ed69f68c959d1024a58349dc51d9 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
@@ -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<InstanceIdentifier, CompositeNod
     }
 
     public Future<RpcResult<TransactionStatus>> commitConfigurationDataPost( final InstanceIdentifier path,
-                                                                             final CompositeNode payload ) {
+                                                                             final CompositeNode payload) {
         this.checkPreconditions();
 
         final DataModificationTransaction transaction = dataService.beginTransaction();
-        transaction.putConfigurationData( path, payload );
-        Map<InstanceIdentifier, CompositeNode> 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<RpcResult<TransactionStatus>> commitConfigurationDataPostBehindMountPoint(
@@ -157,19 +152,18 @@ public class BrokerFacade implements DataReader<InstanceIdentifier, CompositeNod
         this.checkPreconditions();
 
         final DataModificationTransaction transaction = mountPoint.beginTransaction();
-        transaction.putConfigurationData( path, payload );
-        Map<InstanceIdentifier, CompositeNode> 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<RpcResult<TransactionStatus>> 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 2e198ec053d64e018c62224fea94f46167e2b300..0b7b693b0c485b5686492ead95a3f3aa75201732 100644 (file)
--- 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 987beb072b4e3cb03516665b5cba10fc9b740c6d..18199de8c63ca056ed8aacbf947fe0d36f2afa0d 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
@@ -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.<InstanceIdentifier,CompositeNode>emptyMap() );
-
-        Future<RpcResult<TransactionStatus>> 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.<InstanceIdentifier,CompositeNode>emptyMap() );
-
-        Future<RpcResult<TransactionStatus>> 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