Use RestconfException in ExistenceCheck

The only caller is wrapping failures with RestconfDocumentedException,
which means we can unify handling by relocating failure handling to the
original site.

The caller gets the benefit of getOrThrow(), which is completely
appropriate in this execution context.

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

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
index c185d311cb40d1627e68c2ff2a3c8ce76d741704..8055ecd2d190eec65a0aaba2797475c54f415e4e 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
@@ -10,18 +10,18 @@ package org.opendaylight.restconf.nb.rfc8040.rests.transactions;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.util.concurrent.FutureCallback;
-import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
-import com.google.common.util.concurrent.SettableFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
 import org.eclipse.jdt.annotation.NonNull;
-import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.common.api.ReadFailedException;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeReadOperations;
+import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
+import org.opendaylight.restconf.common.errors.RestconfFuture;
+import org.opendaylight.restconf.common.errors.SettableRestconfFuture;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 
@@ -30,7 +30,7 @@ final class ExistenceCheck {
         // Nothing else
     }
 
-    record Conflict(@NonNull YangInstanceIdentifier path, @Nullable ReadFailedException cause) implements Result {
+    record Conflict(@NonNull YangInstanceIdentifier path) implements Result {
         Conflict {
             requireNonNull(path);
         }
@@ -46,7 +46,7 @@ final class ExistenceCheck {
     private static final AtomicIntegerFieldUpdater<ExistenceCheck> UPDATER =
         AtomicIntegerFieldUpdater.newUpdater(ExistenceCheck.class, "outstanding");
 
-    private final SettableFuture<@NonNull Result> future = SettableFuture.create();
+    private final SettableRestconfFuture<Result> future = new SettableRestconfFuture<>();
 
     @SuppressWarnings("unused")
     private volatile int outstanding;
@@ -55,7 +55,7 @@ final class ExistenceCheck {
         outstanding = total;
     }
 
-    static ListenableFuture<@NonNull Result> start(final DOMDataTreeReadOperations tx,
+    static RestconfFuture<Result> start(final DOMDataTreeReadOperations tx,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier parentPath, final boolean expected,
             final Collection<? extends NormalizedNode> children) {
         final var ret = new ExistenceCheck(children.size());
@@ -70,8 +70,8 @@ final class ExistenceCheck {
                 @Override
                 @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE")
                 public void onFailure(final Throwable throwable) {
-                    ret.complete(path, ReadFailedException.MAPPER.apply(
-                        throwable instanceof Exception ex ? ex : new ExecutionException(throwable)));
+                    ret.complete(parentPath, ReadFailedException.MAPPER.apply(
+                        throwable instanceof Exception ex ? ex :  new ExecutionException(throwable)));
                 }
             }, MoreExecutors.directExecutor());
         }
@@ -81,7 +81,7 @@ final class ExistenceCheck {
     private void complete(final YangInstanceIdentifier path, final boolean expected, final boolean present) {
         final int count = UPDATER.decrementAndGet(this);
         if (expected != present) {
-            future.set(new Conflict(path, null));
+            future.set(new Conflict(path));
         } else if (count == 0) {
             future.set(Success.INSTANCE);
         }
@@ -89,6 +89,7 @@ final class ExistenceCheck {
 
     private void complete(final YangInstanceIdentifier path, final ReadFailedException cause) {
         UPDATER.decrementAndGet(this);
-        future.set(new Conflict(path, cause));
+        future.setFailure(new RestconfDocumentedException("Could not determine the existence of path " + path, cause,
+            cause.getErrorList()));
     }
 }

diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfTransaction.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfTransaction.java
index e211da101d86a6b3075e8e6324907999d3613450..f24e3691797194b0e2fa4f6ea404a82d362e5f61 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfTransaction.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfTransaction.java
@@ -12,14 +12,12 @@ import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATI
 
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.Optional;
-import java.util.concurrent.ExecutionException;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.dom.api.DOMDataBroker;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction;
 import org.opendaylight.restconf.common.errors.RestconfDocumentedException;
 import org.opendaylight.restconf.nb.rfc8040.rests.transactions.ExistenceCheck.Conflict;
-import org.opendaylight.restconf.nb.rfc8040.rests.transactions.ExistenceCheck.Result;
 import org.opendaylight.yangtools.yang.common.ErrorTag;
 import org.opendaylight.yangtools.yang.common.ErrorType;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
@@ -80,14 +78,21 @@ final class MdsalRestconfTransaction extends RestconfTransaction {
             ensureParentsByMerge(path);
 
             final var children = ((DistinctNodeContainer<?, ?>) data).body();
+
+            // Fire off an existence check
             final var check = ExistenceCheck.start(verifyNotNull(rwTx), CONFIGURATION, path, false, children);
 
+            // ... and perform any put() operations, which happen-after existence check
             for (var child : children) {
                 final var childPath = path.node(child.name());
                 verifyNotNull(rwTx).put(CONFIGURATION, childPath, child);
             }
+
             // ... finally collect existence checks and abort the transaction if any of them failed.
-            checkExistence(path, check);
+            if (check.getOrThrow() instanceof Conflict) {
+                throw new RestconfDocumentedException("Data already exists", ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS,
+                    path);
+            }
         } else {
             RestconfStrategy.checkItemDoesNotExists(verifyNotNull(rwTx).exists(CONFIGURATION, path), path);
             ensureParentsByMerge(path);
@@ -123,27 +128,4 @@ final class MdsalRestconfTransaction extends RestconfTransaction {
     ListenableFuture<Optional<NormalizedNode>> read(final YangInstanceIdentifier path) {
         return verifyNotNull(rwTx).read(CONFIGURATION, path);
     }
-
-    private static void checkExistence(final YangInstanceIdentifier path,
-            final ListenableFuture<@NonNull Result> future) {
-        final Result result;
-        try {
-            result = future.get();
-        } catch (ExecutionException e) {
-            // This should never happen
-            throw new IllegalStateException(e);
-        } catch (InterruptedException e) {
-            throw new RestconfDocumentedException("Could not determine the existence of path " + path, e);
-        }
-
-        if (result instanceof Conflict conflict) {
-            final var cause = conflict.cause();
-            if (cause == null) {
-                throw new RestconfDocumentedException("Data already exists",
-                    ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, path);
-            }
-            throw new RestconfDocumentedException("Could not determine the existence of path " + conflict.path(), cause,
-                cause.getErrorList());
-        }
-    }
 }