Refactor BatchedExistenceCheck

Users are only interested in a ListenableFuture, which now results in a
'Conflict' record, which captures the semantics of the Entry we use to
return.

The 'Batched' part is not entirely useful, as we can handle single
requests just as well (if the need arises) -- just rename the class to
ExistenceCheck.

As a final twist, pass the presence expectation from the caller, so this
turns a more powerful building block -- which we may need later.

JIRA: NETCONF-718
Change-Id: I71f43e02ff91d8e4fd796dee151c59919663cdc6
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/BatchedExistenceCheck.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
similarity index 50%
rename from restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java
rename to restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
index 25c339cc7757f9f0340e16d49c08f3a2cc1dac0f..c185d311cb40d1627e68c2ff2a3c8ce76d741704 100644 (file)
--- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java
+++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/ExistenceCheck.java
@@ -7,82 +7,88 @@
  */
 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.AbstractMap.SimpleImmutableEntry;
 import java.util.Collection;
-import java.util.Map.Entry;
 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.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 
-final class BatchedExistenceCheck {
-    private static final AtomicIntegerFieldUpdater<BatchedExistenceCheck> UPDATER =
-        AtomicIntegerFieldUpdater.newUpdater(BatchedExistenceCheck.class, "outstanding");
+final class ExistenceCheck {
+    sealed interface Result {
+        // Nothing else
+    }
+
+    record Conflict(@NonNull YangInstanceIdentifier path, @Nullable ReadFailedException cause) implements Result {
+        Conflict {
+            requireNonNull(path);
+        }
+    }
+
+    // Hidden on purpose:
+    // - users care only about conflicts
+    // - we could use null, but that is ugly and conflicts with RestconfException, which we want to use
+    private static final class Success implements Result {
+        static final @NonNull Success INSTANCE = new Success();
+    }
+
+    private static final AtomicIntegerFieldUpdater<ExistenceCheck> UPDATER =
+        AtomicIntegerFieldUpdater.newUpdater(ExistenceCheck.class, "outstanding");
 
-    private final SettableFuture<Entry<YangInstanceIdentifier, ReadFailedException>> future = SettableFuture.create();
+    private final SettableFuture<@NonNull Result> future = SettableFuture.create();
 
     @SuppressWarnings("unused")
     private volatile int outstanding;
 
-    private BatchedExistenceCheck(final int total) {
+    private ExistenceCheck(final int total) {
         outstanding = total;
     }
 
-    static BatchedExistenceCheck start(final DOMDataTreeReadOperations tx, final LogicalDatastoreType datastore,
-            final YangInstanceIdentifier parentPath, final Collection<? extends NormalizedNode> children) {
-        final var ret = new BatchedExistenceCheck(children.size());
+    static ListenableFuture<@NonNull 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());
         for (var child : children) {
             final var path = parentPath.node(child.name());
             tx.exists(datastore, path).addCallback(new FutureCallback<Boolean>() {
                 @Override
                 public void onSuccess(final Boolean result) {
-                    ret.complete(path, result);
+                    ret.complete(path, expected, result);
                 }
 
                 @Override
                 @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE")
                 public void onFailure(final Throwable throwable) {
-                    final Exception e;
-                    if (throwable instanceof Exception) {
-                        e = (Exception) throwable;
-                    } else {
-                        e = new ExecutionException(throwable);
-                    }
-
-                    ret.complete(path, ReadFailedException.MAPPER.apply(e));
+                    ret.complete(path, ReadFailedException.MAPPER.apply(
+                        throwable instanceof Exception ex ? ex : new ExecutionException(throwable)));
                 }
             }, MoreExecutors.directExecutor());
         }
-        return ret;
-    }
-
-    Entry<YangInstanceIdentifier, ReadFailedException> getFailure() throws InterruptedException {
-        try {
-            return future.get();
-        } catch (ExecutionException e) {
-            // This should never happen
-            throw new IllegalStateException(e);
-        }
+        return ret.future;
     }
 
-    private void complete(final YangInstanceIdentifier childPath, final boolean present) {
+    private void complete(final YangInstanceIdentifier path, final boolean expected, final boolean present) {
         final int count = UPDATER.decrementAndGet(this);
-        if (present) {
-            future.set(new SimpleImmutableEntry<>(childPath, null));
+        if (expected != present) {
+            future.set(new Conflict(path, null));
         } else if (count == 0) {
-            future.set(null);
+            future.set(Success.INSTANCE);
         }
     }
 
-    private void complete(final YangInstanceIdentifier childPath, final ReadFailedException cause) {
+    private void complete(final YangInstanceIdentifier path, final ReadFailedException cause) {
         UPDATER.decrementAndGet(this);
-        future.set(new SimpleImmutableEntry<>(childPath, cause));
+        future.set(new Conflict(path, cause));
     }
 }

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 66dfd8dc0c5dc3b9ca32de86ec8894e415e6e355..a5b89bd124ec6da6a132395b49c87db0e6659178 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
@@ -11,14 +11,15 @@ import static com.google.common.base.Verify.verifyNotNull;
 import static org.opendaylight.mdsal.common.api.LogicalDatastoreType.CONFIGURATION;
 
 import com.google.common.util.concurrent.ListenableFuture;
-import java.util.Map;
 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.common.api.ReadFailedException;
 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;
@@ -79,7 +80,7 @@ final class MdsalRestconfTransaction extends RestconfTransaction {
             ensureParentsByMerge(path);
 
             final var children = ((DistinctNodeContainer<?, ?>) data).body();
-            final var check = BatchedExistenceCheck.start(verifyNotNull(rwTx), CONFIGURATION, path, children);
+            final var check = ExistenceCheck.start(verifyNotNull(rwTx), CONFIGURATION, path, false, children);
 
             for (var child : children) {
                 final var childPath = path.node(child.name());
@@ -123,23 +124,26 @@ final class MdsalRestconfTransaction extends RestconfTransaction {
         return verifyNotNull(rwTx).read(CONFIGURATION, path);
     }
 
-    private static void checkExistence(final YangInstanceIdentifier path, final BatchedExistenceCheck check) {
-        final Map.Entry<YangInstanceIdentifier, ReadFailedException> failure;
+    private static void checkExistence(final YangInstanceIdentifier path,
+            final ListenableFuture<@NonNull Result> future) {
+        final Result result;
         try {
-            failure = check.getFailure();
+            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 (failure != null) {
-            final ReadFailedException e = failure.getValue();
-            if (e == null) {
+        if (result instanceof Conflict conflict) {
+            final var cause = conflict.cause();
+            if (cause == null) {
                 throw new RestconfDocumentedException("Data already exists",
-                    ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, failure.getKey());
+                    ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, conflict.path());
             }
-
-            throw new RestconfDocumentedException(
-                "Could not determine the existence of path " + failure.getKey(), e, e.getErrorList());
+            throw new RestconfDocumentedException("Could not determine the existence of path " + conflict.path(), cause,
+                cause.getErrorList());
         }
     }
 }