tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject [08/18] tinkerpop git commit: TINKERPOP-1913 Refactored how response status attributes are set
Date Tue, 18 Sep 2018 17:16:45 GMT
TINKERPOP-1913 Refactored how response status attributes are set

Didn't seem necessary to set them directly into a member variable on ResultQueue when they
could be passed more safely by just including them with the completion state update


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/7fca7af7
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/7fca7af7
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/7fca7af7

Branch: refs/heads/TINKERPOP-1913
Commit: 7fca7af74e5bd2695ba6035cf341543f9750fc4d
Parents: 956b177
Author: Stephen Mallette <spmva@genoprime.com>
Authored: Thu Aug 16 11:38:09 2018 -0400
Committer: Stephen Mallette <spmva@genoprime.com>
Committed: Tue Sep 18 12:58:42 2018 -0400

----------------------------------------------------------------------
 .../tinkerpop/gremlin/driver/Handler.java       | 11 +------
 .../tinkerpop/gremlin/driver/ResultQueue.java   | 11 +++++--
 .../tinkerpop/gremlin/driver/ResultSet.java     |  2 +-
 .../gremlin/driver/AbstractResultQueueTest.java |  3 +-
 .../gremlin/driver/ResultQueueTest.java         | 30 +++++++++++++++-----
 .../tinkerpop/gremlin/driver/ResultSetTest.java | 28 +++++++-----------
 6 files changed, 46 insertions(+), 39 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
index d79bed5..a77ec84 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java
@@ -260,18 +260,9 @@ final class Handler {
                     }
                 }
 
-                // the last message in a OK stream could have meta-data that is useful to
the result. note that error
-                // handling of the status attributes is handled separately above
-                if (statusCode == ResponseStatusCode.SUCCESS || statusCode == ResponseStatusCode.NO_CONTENT)
{
-                    // in 3.4.0 this should get refactored. i think the that the markComplete()
could just take the
-                    // status attributes as its argument - need to investigate that further
-                    queue.statusAttributes = response.getStatus().getAttributes();
-                }
-
                 // as this is a non-PARTIAL_CONTENT code - the stream is done.
                 if (statusCode != ResponseStatusCode.PARTIAL_CONTENT) {
-                    queue.statusAttributes = response.getStatus().getAttributes();
-                    pending.remove(response.getRequestId()).markComplete();
+                    pending.remove(response.getRequestId()).markComplete(response.getStatus().getAttributes());
                 }
             } finally {
                 // in the event of an exception above the exception is tossed and handled
by whatever channelpipeline

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultQueue.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultQueue.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultQueue.java
index 7340763..59b4617 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultQueue.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultQueue.java
@@ -26,6 +26,7 @@ import org.javatuples.Pair;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -56,7 +57,7 @@ final class ResultQueue {
 
     private final Queue<Pair<CompletableFuture<List<Result>>,Integer>>
waiting = new ConcurrentLinkedQueue<>();
 
-    Map<String,Object> statusAttributes = null;
+    private Map<String,Object> statusAttributes = null;
 
     public ResultQueue(final LinkedBlockingQueue<Result> resultLinkedBlockingQueue,
final CompletableFuture<Void> readComplete) {
         this.resultLinkedBlockingQueue = resultLinkedBlockingQueue;
@@ -165,12 +166,14 @@ final class ResultQueue {
         resultLinkedBlockingQueue.drainTo(collection);
     }
 
-    void markComplete() {
+    void markComplete(final Map<String,Object> statusAttributes) {
         // if there was some aggregation performed in the queue then the full object is hanging
out waiting to be
         // added to the ResultSet
         if (aggregatedResult != null)
             add(new Result(aggregatedResult));
 
+        this.statusAttributes = null == statusAttributes ? Collections.emptyMap() : statusAttributes;
+
         this.readComplete.complete(null);
 
         this.drainAllWaiting();
@@ -182,6 +185,10 @@ final class ResultQueue {
         this.drainAllWaiting();
     }
 
+    Map<String,Object> getStatusAttributes() {
+        return statusAttributes;
+    }
+
     /**
      * Completes the next waiting future if there is one.
      */

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultSet.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultSet.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultSet.java
index f608f06..85c74f3 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultSet.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ResultSet.java
@@ -81,7 +81,7 @@ public final class ResultSet implements Iterable<Result> {
      */
     public CompletableFuture<Map<String,Object>> statusAttributes() {
         final CompletableFuture<Map<String,Object>> attrs = new CompletableFuture<>();
-        readCompleted.thenRun(() -> attrs.complete(null == resultQueue.statusAttributes
? Collections.emptyMap() : resultQueue.statusAttributes));
+        readCompleted.thenRun(() -> attrs.complete(null == resultQueue.getStatusAttributes()
? Collections.emptyMap() : resultQueue.getStatusAttributes()));
         return attrs;
     }
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/AbstractResultQueueTest.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/AbstractResultQueueTest.java
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/AbstractResultQueueTest.java
index 3c84ca6..302fda9 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/AbstractResultQueueTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/AbstractResultQueueTest.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.driver;
 
 import org.junit.Before;
 
+import java.util.Collections;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
@@ -84,7 +85,7 @@ public abstract class AbstractResultQueueTest {
                 }
             }
 
-            if (markDone) resultQueue.markComplete();
+            if (markDone) resultQueue.markComplete(Collections.emptyMap());
 
         }, "ResultQueueTest-job-submitter");
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java
index 43442be..436d659 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java
@@ -45,6 +45,10 @@ import static org.junit.Assert.fail;
  */
 public class ResultQueueTest extends AbstractResultQueueTest {
 
+    private static final Map<String,Object> ATTRIBUTES = new HashMap<String,Object>()
{{
+        put("this", "that");
+    }};
+
     @Test
     public void shouldGetSizeUntilError() throws Exception {
         final Thread t = addToQueue(100, 10, true, false, 1);
@@ -135,7 +139,7 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         resultQueue.add(new Result("test3"));
 
         assertThat(future.isDone(), is(false));
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
         assertThat(future.isDone(), is(true));
 
         final List<Result> results = future.get();
@@ -145,6 +149,7 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         assertEquals(3, results.size());
 
         assertThat(resultQueue.isEmpty(), is(true));
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -242,7 +247,7 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         resultQueue.add(new Result("test2"));
         resultQueue.add(new Result("test3"));
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         // you might want 30 but there are only three
         final CompletableFuture<List<Result>> future = resultQueue.await(30);
@@ -255,6 +260,7 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         assertEquals(3, results.size());
 
         assertThat(resultQueue.isEmpty(), is(true));
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -307,12 +313,14 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         resultQueue.addSideEffect(Tokens.VAL_AGGREGATE_TO_BULKSET, new DefaultRemoteTraverser<>("belinda",
6));
         assertThat(o.isDone(), is(false));
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         assertThat(o.isDone(), is(true));
         final BulkSet<String> bulkSet = o.get().get(0).get(BulkSet.class);
         assertEquals(4, bulkSet.get("brian"));
         assertEquals(6, bulkSet.get("belinda"));
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -329,13 +337,15 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         resultQueue.addSideEffect(Tokens.VAL_AGGREGATE_TO_LIST, "dave");
         assertThat(o.isDone(), is(false));
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         assertThat(o.isDone(), is(true));
         final List<String> list = o.get().get(0).get(ArrayList.class);
         assertEquals("stephen", list.get(0));
         assertEquals("daniel", list.get(1));
         assertEquals("dave", list.get(2));
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -352,13 +362,15 @@ public class ResultQueueTest extends AbstractResultQueueTest {
         resultQueue.addSideEffect(Tokens.VAL_AGGREGATE_TO_SET, "dave");
         assertThat(o.isDone(), is(false));
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         assertThat(o.isDone(), is(true));
         final Set<String> set = o.get().get(0).get(HashSet.class);
         assertThat(set.contains("stephen"), is(true));
         assertThat(set.contains("daniel"), is(true));
         assertThat(set.contains("dave"), is(true));
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -376,13 +388,15 @@ public class ResultQueueTest extends AbstractResultQueueTest {
             assertThat(o.isDone(), is(false));
         });
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         assertThat(o.isDone(), is(true));
         final Map<String, String> list = o.get().get(0).get(HashMap.class);
         assertEquals("stephen", list.get("s"));
         assertEquals("daniel", list.get("d"));
         assertEquals("marko", list.get("m"));
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
 
@@ -398,12 +412,14 @@ public class ResultQueueTest extends AbstractResultQueueTest {
 
         resultQueue.addSideEffect(Tokens.VAL_AGGREGATE_TO_NONE, m);
 
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         assertThat(o.isDone(), is(true));
         final Map<String, String> list = o.get().get(0).get(HashMap.class);
         assertEquals("stephen", list.get("s"));
         assertEquals("daniel", list.get("d"));
         assertEquals("marko", list.get("m"));
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/7fca7af7/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultSetTest.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultSetTest.java
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultSetTest.java
index 3163ffe..44f13e4 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultSetTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultSetTest.java
@@ -43,6 +43,10 @@ import static org.junit.Assert.fail;
  */
 public class ResultSetTest extends AbstractResultQueueTest {
 
+    private static final Map<String,Object> ATTRIBUTES = new HashMap<String,Object>()
{{
+        put("this", "that");
+    }};
+
     private ResultSet resultSet;
 
     @Before
@@ -51,22 +55,6 @@ public class ResultSetTest extends AbstractResultQueueTest {
     }
 
     @Test
-    public void shouldReturnResponseAttributes() throws Exception {
-        resultQueue.statusAttributes = new HashMap<String,Object>() {{
-            put("test",123);
-            put("junk","here");
-        }};
-
-        final CompletableFuture<Map<String,Object>> attrs = resultSet.statusAttributes();
-        readCompleted.complete(null);
-
-        final Map<String,Object> m = attrs.get();
-        assertEquals(123, m.get("test"));
-        assertEquals("here", m.get("junk"));
-        assertEquals(2, m.size());
-    }
-
-    @Test
     public void shouldReturnEmptyMapForNoResponseAttributes() throws Exception {
         final CompletableFuture<Map<String,Object>> attrs = resultSet.statusAttributes();
         readCompleted.complete(null);
@@ -132,7 +120,7 @@ public class ResultSetTest extends AbstractResultQueueTest {
         resultQueue.add(new Result("test3"));
 
         assertThat(future.isDone(), is(false));
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
         assertThat(future.isDone(), is(true));
 
         final List<Result> results = future.get();
@@ -143,6 +131,8 @@ public class ResultSetTest extends AbstractResultQueueTest {
 
         assertThat(resultSet.allItemsAvailable(), is(true));
         assertEquals(0, resultSet.getAvailableItemCount());
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test
@@ -153,7 +143,7 @@ public class ResultSetTest extends AbstractResultQueueTest {
         resultQueue.add(new Result("test3"));
 
         assertThat(future.isDone(), is(false));
-        resultQueue.markComplete();
+        resultQueue.markComplete(ATTRIBUTES);
 
         final List<Result> results = future.get();
         assertEquals("test1", results.get(0).getString());
@@ -164,6 +154,8 @@ public class ResultSetTest extends AbstractResultQueueTest {
         assertThat(future.isDone(), is(true));
         assertThat(resultSet.allItemsAvailable(), is(true));
         assertEquals(0, resultSet.getAvailableItemCount());
+
+        assertEquals("that", resultQueue.getStatusAttributes().get("this"));
     }
 
     @Test


Mime
View raw message