idempotent close cursor (DE-727, #528)

Author: rashtao

Diff for: core/src/main/java/com/arangodb/internal/ArangoDatabaseImpl.java

@@ -213,7 +213,14 @@ public CursorEntity<T> next(final String id, final String nextBatchId) {
 
             @Override
             public void close(final String id) {
-                executorSync().execute(queryCloseRequest(id, options), Void.class, hostHandle);
+                try {
+                    executorSync().execute(queryCloseRequest(id, options), Void.class, hostHandle);
+                } catch (final ArangoDBException e) {
+                    // ignore errors Response: 404, Error: 1600 - cursor not found
+                    if (!matches(e, 404, 1600)) {
+                        throw e;
+                    }
+                }
             }
         };
 

Diff for: core/src/main/java/com/arangodb/internal/cursor/ArangoCursorAsyncImpl.java

@@ -1,13 +1,17 @@
 package com.arangodb.internal.cursor;
 
 import com.arangodb.ArangoCursorAsync;
+import com.arangodb.ArangoDBException;
 import com.arangodb.entity.CursorEntity;
 import com.arangodb.internal.ArangoDatabaseAsyncImpl;
 import com.arangodb.internal.InternalArangoCursor;
 import com.arangodb.internal.net.HostHandle;
 
 import java.util.NoSuchElementException;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+
+import static com.arangodb.internal.ArangoErrors.matches;
 
 public class ArangoCursorAsyncImpl<T> extends InternalArangoCursor<T> implements ArangoCursorAsync<T> {
 
@@ -41,7 +45,19 @@ public CompletableFuture<ArangoCursorAsync<T>> nextBatch() {
     @Override
     public CompletableFuture<Void> close() {
         if (getId() != null && (allowRetry() || Boolean.TRUE.equals(hasMore()))) {
-            return executorAsync().execute(this::queryCloseRequest, Void.class, hostHandle);
+            return executorAsync().execute(this::queryCloseRequest, Void.class, hostHandle)
+                    .exceptionally(err -> {
+                        Throwable e = err instanceof CompletionException ? err.getCause() : err;
+                        if (e instanceof ArangoDBException) {
+                            ArangoDBException aEx = (ArangoDBException) e;
+                            // ignore errors Response: 404, Error: 1600 - cursor not found
+                            if (matches(aEx, 404, 1600)) {
+                                return null;
+                            }
+                        }
+                        throw ArangoDBException.of(e);
+                    })
+                    .thenApply(__ -> null);
         } else {
             return CompletableFuture.completedFuture(null);
         }

Diff for: driver/src/test/java/com/arangodb/ArangoDatabaseAsyncTest.java

@@ -911,6 +911,23 @@ void queryClose(ArangoDBAsync arangoDB) throws ExecutionException, InterruptedEx
         assertThat(ex.getMessage()).contains("cursor not found");
     }
 
+    @ParameterizedTest(name = "{index}")
+    @MethodSource("asyncArangos")
+    void queryCloseShouldBeIdempotent(ArangoDBAsync arangoDB) throws ExecutionException, InterruptedException {
+        ArangoCursorAsync<Integer> cursor = arangoDB.db().query("for i in 1..2 return i", Integer.class,
+                new AqlQueryOptions().batchSize(1)).get();
+        cursor.close().get();
+        cursor.close().get();
+    }
+
+    @ParameterizedTest(name = "{index}")
+    @MethodSource("asyncArangos")
+    void queryCloseOnCursorWithoutId(ArangoDBAsync arangoDB) throws ExecutionException, InterruptedException {
+        ArangoCursorAsync<Integer> cursor = arangoDB.db().query("return 1", Integer.class).get();
+        cursor.close().get();
+        cursor.close().get();
+    }
+
     @ParameterizedTest(name = "{index}")
     @MethodSource("asyncDbs")
     void queryNoResults(ArangoDatabaseAsync db) throws ExecutionException, InterruptedException {

Diff for: driver/src/test/java/com/arangodb/ArangoDatabaseTest.java

@@ -986,6 +986,23 @@ void queryClose(ArangoDB arangoDB) throws IOException {
         assertThat(count).hasValue(1);
     }
 
+    @ParameterizedTest(name = "{index}")
+    @MethodSource("arangos")
+    void queryCloseShouldBeIdempotent(ArangoDB arangoDB) throws IOException {
+        ArangoCursor<Integer> cursor = arangoDB.db().query("for i in 1..2 return i", Integer.class,
+                new AqlQueryOptions().batchSize(1));
+        cursor.close();
+        cursor.close();
+    }
+
+    @ParameterizedTest(name = "{index}")
+    @MethodSource("arangos")
+    void queryCloseOnCursorWithoutId(ArangoDB arangoDB) throws IOException {
+        ArangoCursor<Integer> cursor = arangoDB.db().query("return 1", Integer.class);
+        cursor.close();
+        cursor.close();
+    }
+
     @ParameterizedTest(name = "{index}")
     @MethodSource("dbs")
     void queryNoResults(ArangoDatabase db) throws IOException {
@@ -1153,7 +1170,7 @@ void getCurrentlyRunningQueries(ArangoDatabase db) throws InterruptedException {
         assertThat(queryEntity.getBindVars()).isEmpty();
         assertThat(queryEntity.getStarted()).isInThePast();
         assertThat(queryEntity.getRunTime()).isPositive();
-        if(isAtLeastVersion(3,11)){
+        if (isAtLeastVersion(3, 11)) {
             assertThat(queryEntity.getPeakMemoryUsage()).isNotNull();
         }
         assertThat(queryEntity.getState()).isEqualTo(QueryExecutionState.EXECUTING);
@@ -1213,7 +1230,7 @@ void getAndClearSlowQueries(ArangoDatabase db) {
         assertThat(queryEntity.getBindVars()).isEmpty();
         assertThat(queryEntity.getStarted()).isInThePast();
         assertThat(queryEntity.getRunTime()).isPositive();
-        if(isAtLeastVersion(3,11)){
+        if (isAtLeastVersion(3, 11)) {
             assertThat(queryEntity.getPeakMemoryUsage()).isNotNull();
         }
         assertThat(queryEntity.getState()).isEqualTo(QueryExecutionState.FINISHED);