RestconfStrategy.patchData() should result in RestconfFuture

This method performs an intrinsically asynchronous operation, clarify
that in its contract.

JIRA: NETCONF-718
Change-Id: I1c4584c27ad78107ed5bf8299ab60287b8942a2b
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
index 651726b019bcfa7cd6fae16ab93704258aa5027c..996962c8c9351fdeaa048a9f1a2eada1fe03de0b 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java
@@ -701,7 +701,7 @@ public final class RestconfDataServiceImpl {
      *
      * @param identifier path to target
      * @param body YANG Patch body
-     * @return {@link PatchStatusContext}
+     * @param ar {@link AsyncResponse} which needs to be completed with a {@link PatchStatusContext}
      */
     @PATCH
     @Path("/data/{identifier:.+}")
@@ -710,10 +710,10 @@ public final class RestconfDataServiceImpl {
         MediaTypes.APPLICATION_YANG_DATA_JSON,
         MediaTypes.APPLICATION_YANG_DATA_XML
     })
-    public PatchStatusContext yangPatchDataXML(@Encoded @PathParam("identifier") final String identifier,
-            final InputStream body) {
+    public void yangPatchDataXML(@Encoded @PathParam("identifier") final String identifier, final InputStream body,
+            @Suspended final AsyncResponse ar) {
         try (var xmlBody = new XmlPatchBody(body)) {
-            return yangPatchData(identifier, xmlBody);
+            yangPatchData(identifier, xmlBody, ar);
         }
     }
 
@@ -722,7 +722,7 @@ public final class RestconfDataServiceImpl {
      * <a href="https://www.rfc-editor.org/rfc/rfc8072#section-2">RFC8072, section 2</a>.
      *
      * @param body YANG Patch body
-     * @return {@link PatchStatusContext}
+     * @param ar {@link AsyncResponse} which needs to be completed with a {@link PatchStatusContext}
      */
     @PATCH
     @Path("/data")
@@ -731,9 +731,9 @@ public final class RestconfDataServiceImpl {
         MediaTypes.APPLICATION_YANG_DATA_JSON,
         MediaTypes.APPLICATION_YANG_DATA_XML
     })
-    public PatchStatusContext yangPatchDataXML(final InputStream body) {
+    public void yangPatchDataXML(final InputStream body, @Suspended final AsyncResponse ar) {
         try (var xmlBody = new XmlPatchBody(body)) {
-            return yangPatchData(xmlBody);
+            yangPatchData(xmlBody, ar);
         }
     }
 
@@ -743,7 +743,7 @@ public final class RestconfDataServiceImpl {
      *
      * @param identifier path to target
      * @param body YANG Patch body
-     * @return {@link PatchStatusContext}
+     * @param ar {@link AsyncResponse} which needs to be completed with a {@link PatchStatusContext}
      */
     @PATCH
     @Path("/data/{identifier:.+}")
@@ -752,10 +752,10 @@ public final class RestconfDataServiceImpl {
         MediaTypes.APPLICATION_YANG_DATA_JSON,
         MediaTypes.APPLICATION_YANG_DATA_XML
     })
-    public PatchStatusContext yangPatchDataJSON(@Encoded @PathParam("identifier") final String identifier,
-            final InputStream body) {
+    public void yangPatchDataJSON(@Encoded @PathParam("identifier") final String identifier,
+            final InputStream body, @Suspended final AsyncResponse ar) {
         try (var jsonBody = new JsonPatchBody(body)) {
-            return yangPatchData(identifier, jsonBody);
+            yangPatchData(identifier, jsonBody, ar);
         }
     }
 
@@ -764,7 +764,7 @@ public final class RestconfDataServiceImpl {
      * <a href="https://www.rfc-editor.org/rfc/rfc8072#section-2">RFC8072, section 2</a>.
      *
      * @param body YANG Patch body
-     * @return {@link PatchStatusContext}
+     * @param ar {@link AsyncResponse} which needs to be completed with a {@link PatchStatusContext}
      */
     @PATCH
     @Path("/data")
@@ -773,29 +773,41 @@ public final class RestconfDataServiceImpl {
         MediaTypes.APPLICATION_YANG_DATA_JSON,
         MediaTypes.APPLICATION_YANG_DATA_XML
     })
-    public PatchStatusContext yangPatchDataJSON(final InputStream body) {
+    public void yangPatchDataJSON(final InputStream body, @Suspended final AsyncResponse ar) {
         try (var jsonBody = new JsonPatchBody(body)) {
-            return yangPatchData(jsonBody);
+            yangPatchData(jsonBody, ar);
         }
     }
 
-    private PatchStatusContext yangPatchData(final @NonNull PatchBody body) {
+    private void yangPatchData(final @NonNull PatchBody body, final AsyncResponse ar) {
         final var context = databindProvider.currentContext().modelContext();
-        return yangPatchData(context, parsePatchBody(context, YangInstanceIdentifier.of(), body), null);
+        yangPatchData(context, parsePatchBody(context, YangInstanceIdentifier.of(), body), null, ar);
     }
 
-    private PatchStatusContext yangPatchData(final String identifier, final @NonNull PatchBody body) {
+    private void yangPatchData(final String identifier, final @NonNull PatchBody body,
+            final AsyncResponse ar) {
         final var reqPath = server.bindRequestPath(databindProvider.currentContext(), identifier);
         final var modelContext = reqPath.getSchemaContext();
-        return yangPatchData(modelContext, parsePatchBody(modelContext, reqPath.getInstanceIdentifier(), body),
-            reqPath.getMountPoint());
+        yangPatchData(modelContext, parsePatchBody(modelContext, reqPath.getInstanceIdentifier(), body),
+            reqPath.getMountPoint(), ar);
     }
 
 
     @VisibleForTesting
-    @NonNull PatchStatusContext yangPatchData(final @NonNull EffectiveModelContext modelContext,
-            final @NonNull PatchContext patch, final @Nullable DOMMountPoint mountPoint) {
-        return server.getRestconfStrategy(modelContext, mountPoint).patchData(patch);
+    void yangPatchData(final @NonNull EffectiveModelContext modelContext,
+            final @NonNull PatchContext patch, final @Nullable DOMMountPoint mountPoint, final AsyncResponse ar) {
+        Futures.addCallback(server.getRestconfStrategy(modelContext, mountPoint).patchData(patch),
+            new FutureCallback<>() {
+                @Override
+                public void onSuccess(final PatchStatusContext result) {
+                    ar.resume(result);
+                }
+
+                @Override
+                public void onFailure(final Throwable cause) {
+                    ar.resume(cause);
+                }
+            }, MoreExecutors.directExecutor());
     }
 
     private static @NonNull PatchContext parsePatchBody(final @NonNull EffectiveModelContext context,

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
index 90d0d3729092255c2a59e35b58ccea6caf7ef96e..e2efbd6a6ef4563ba3b5e1dc3789fc135aa2fdf6 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java
@@ -418,7 +418,7 @@ public abstract class RestconfStrategy {
      * @param patch Patch context to be processed
      * @return {@link PatchStatusContext}
      */
-    public final @NonNull PatchStatusContext patchData(final PatchContext patch) {
+    public final @NonNull RestconfFuture<PatchStatusContext> patchData(final PatchContext patch) {
         final var editCollection = new ArrayList<PatchStatusEntity>();
         final var tx = prepareWriteExecution();
 
@@ -487,21 +487,29 @@ public abstract class RestconfStrategy {
             }
         }
 
-        // if no errors then submit transaction, otherwise cancel
-        final var patchId = patch.patchId();
-        if (noError) {
-            try {
-                TransactionUtil.syncCommit(tx.commit(), "PATCH", null);
-            } catch (RestconfDocumentedException e) {
+        final var ret = new SettableRestconfFuture<PatchStatusContext>();
+        // We have errors
+        if (!noError) {
+            tx.cancel();
+            ret.set(new PatchStatusContext(modelContext, patch.patchId(), List.copyOf(editCollection), false, null));
+            return ret;
+        }
+
+        Futures.addCallback(tx.commit(), new FutureCallback<CommitInfo>() {
+            @Override
+            public void onSuccess(final CommitInfo result) {
+                ret.set(new PatchStatusContext(modelContext, patch.patchId(), List.copyOf(editCollection), true, null));
+            }
+
+            @Override
+            public void onFailure(final Throwable cause) {
                 // if errors occurred during transaction commit then patch failed and global errors are reported
-                return new PatchStatusContext(modelContext, patchId, List.copyOf(editCollection), false, e.getErrors());
+                ret.set(new PatchStatusContext(modelContext, patch.patchId(), List.copyOf(editCollection), false,
+                    TransactionUtil.decodeException(cause, "PATCH", null).getErrors()));
             }
+        }, MoreExecutors.directExecutor());
 
-            return new PatchStatusContext(modelContext, patchId, List.copyOf(editCollection), true, null);
-        } else {
-            tx.cancel();
-            return new PatchStatusContext(modelContext, patchId, List.copyOf(editCollection), false, null);
-        }
+        return ret;
     }
 
     private void insertWithPointPost(final RestconfTransaction tx, final YangInstanceIdentifier path,

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImplTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImplTest.java
index d2204b465c14b489835c7c9cd6336f28576737e8..0e8ad60ae3568a51ac47b57a54638d3c0f400752 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImplTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImplTest.java
@@ -39,6 +39,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.mdsal.common.api.CommitInfo;
@@ -116,6 +117,8 @@ public class RestconfDataServiceImplTest extends AbstractJukeboxTest {
     private MultivaluedMap<String, String> queryParamenters;
     @Mock
     private AsyncResponse asyncResponse;
+    @Captor
+    private ArgumentCaptor<PatchStatusContext> patchStatus;
 
     private RestconfDataServiceImpl dataService;
 
@@ -415,7 +418,9 @@ public class RestconfDataServiceImplTest extends AbstractJukeboxTest {
                 .when(readWrite).exists(LogicalDatastoreType.CONFIGURATION, JUKEBOX_IID);
         doReturn(immediateTrueFluentFuture())
                 .when(readWrite).exists(LogicalDatastoreType.CONFIGURATION, GAP_IID);
-        final var status = dataService.yangPatchData(JUKEBOX_SCHEMA, patch, null);
+        doReturn(true).when(asyncResponse).resume(patchStatus.capture());
+        dataService.yangPatchData(JUKEBOX_SCHEMA, patch, null, asyncResponse);
+        final var status = patchStatus.getValue();
         assertTrue(status.ok());
         assertEquals(3, status.editCollection().size());
         assertEquals("replace data", status.editCollection().get(1).getEditId());
@@ -433,7 +438,9 @@ public class RestconfDataServiceImplTest extends AbstractJukeboxTest {
                 .when(readWrite).exists(LogicalDatastoreType.CONFIGURATION, JUKEBOX_IID);
         doReturn(immediateTrueFluentFuture()).when(readWrite).exists(LogicalDatastoreType.CONFIGURATION, GAP_IID);
 
-        final var status = dataService.yangPatchData(JUKEBOX_SCHEMA, patch, mountPoint);
+        doReturn(true).when(asyncResponse).resume(patchStatus.capture());
+        dataService.yangPatchData(JUKEBOX_SCHEMA, patch, mountPoint, asyncResponse);
+        final var status = patchStatus.getValue();
         assertTrue(status.ok());
         assertEquals(3, status.editCollection().size());
         assertNull(status.globalErrors());
@@ -441,7 +448,7 @@ public class RestconfDataServiceImplTest extends AbstractJukeboxTest {
 
     @Test
     public void testPatchDataDeleteNotExist() {
-        final PatchContext patch = new PatchContext("test patch id", List.of(
+        final var patch = new PatchContext("test patch id", List.of(
             new PatchEntity("create data", Operation.Create, JUKEBOX_IID, EMPTY_JUKEBOX),
             new PatchEntity("remove data", Operation.Remove, GAP_IID),
             new PatchEntity("delete data", Operation.Delete, GAP_IID)));
@@ -453,7 +460,9 @@ public class RestconfDataServiceImplTest extends AbstractJukeboxTest {
                 .when(readWrite).exists(LogicalDatastoreType.CONFIGURATION, GAP_IID);
         doReturn(true).when(readWrite).cancel();
 
-        final PatchStatusContext status = dataService.yangPatchData(JUKEBOX_SCHEMA, patch, null);
+        doReturn(true).when(asyncResponse).resume(patchStatus.capture());
+        dataService.yangPatchData(JUKEBOX_SCHEMA, patch, null, asyncResponse);
+        final var status = patchStatus.getValue();
 
         assertFalse(status.ok());
         assertEquals(3, status.editCollection().size());

diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/AbstractRestconfStrategyTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/AbstractRestconfStrategyTest.java
index 4826c7eed85ba80975011726505df1c1f0cadb5a..50878d6efef3bfb06832a0605bb5b18ffaa74b13 100644 (file)
--- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/AbstractRestconfStrategyTest.java
+++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/AbstractRestconfStrategyTest.java
@@ -330,7 +330,7 @@ abstract class AbstractRestconfStrategyTest extends AbstractJukeboxTest {
     public final void testDeleteNonexistentData() {
         final var patchStatusContext = deleteNonexistentDataTestStrategy().patchData(new PatchContext("patchD",
             List.of(new PatchEntity("edit", Operation.Delete, CREATE_AND_DELETE_TARGET))));
-        assertFalse(patchStatusContext.ok());
+        assertFalse(patchStatusContext.getOrThrow().ok());
     }
 
     abstract @NonNull RestconfStrategy deleteNonexistentDataTestStrategy();
@@ -467,7 +467,7 @@ abstract class AbstractRestconfStrategyTest extends AbstractJukeboxTest {
     }
 
     private static void patch(final PatchContext patchContext, final RestconfStrategy strategy, final boolean failed) {
-        final var patchStatusContext = strategy.patchData(patchContext);
+        final var patchStatusContext = strategy.patchData(patchContext).getOrThrow();
         for (var entity : patchStatusContext.editCollection()) {
             if (failed) {
                 assertTrue("Edit " + entity.getEditId() + " failed", entity.isOk());