You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2018/08/16 15:40:38 UTC
[50/50] tinkerpop git commit: TINKERPOP-1913 Refactored how response
status attributes are set
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/87ede605
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/87ede605
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/87ede605
Branch: refs/heads/TINKERPOP-1913
Commit: 87ede6058dd3bf41acafccf1e2addefb6a005113
Parents: 9c22174
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Aug 16 11:38:09 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Aug 16 11:38:09 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/87ede605/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 dcfc4b9..81e893c 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
@@ -259,18 +259,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/87ede605/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/87ede605/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/87ede605/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/87ede605/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/87ede605/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