GroupStep.GroupBiOperator has serialization issues with Groovy lambdas. This is 
rectified by simply saying -- if the valueTraversal can not be serialized, then 
revert back to 3.2.0 behavior and simply propagate traverser sets instead of 
doing lazy barrier reductions when sets grow. Added a test to 
HadoopGremlinPluginCheck that verifies that both Spark and Giraph are happy. 
Also, updated a HadoopGremlinPluginCheck to ensure that non-sugar remote 
connections don't allow sugar (random side thing I noticed). CTR.


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

Branch: refs/heads/tp31
Commit: d2eb63c4688b2a5c148422b9036419a62f62ea6b
Parents: ff12c59
Author: Marko A. Rodriguez <okramma...@gmail.com>
Authored: Tue May 31 11:59:35 2016 -0600
Committer: Marko A. Rodriguez <okramma...@gmail.com>
Committed: Tue May 31 11:59:35 2016 -0600

----------------------------------------------------------------------
 .../process/traversal/step/map/GroupStep.java   | 15 ++++++-
 .../groovy/plugin/HadoopGremlinPluginCheck.java | 47 +++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/d2eb63c4/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
----------------------------------------------------------------------
diff --git 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
index 77e39bb..dd899de 100644
--- 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
+++ 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GroupStep.java
@@ -30,12 +30,14 @@ import 
org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Barrier;
 import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
 import 
org.apache.tinkerpop.gremlin.process.traversal.step.util.ReducingBarrierStep;
 import 
org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
 import 
org.apache.tinkerpop.gremlin.process.traversal.traverser.util.TraverserSet;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
+import org.apache.tinkerpop.gremlin.util.Serializer;
 import org.apache.tinkerpop.gremlin.util.function.HashMapSupplier;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 import org.javatuples.Pair;
@@ -296,12 +298,21 @@ public final class GroupStep<S, K, V> extends 
ReducingBarrierStep<S, Map<K, V>>
 
         // necessary to control Java Serialization to ensure proper clearing 
of internal traverser data
         private void writeObject(final ObjectOutputStream outputStream) throws 
IOException {
-            outputStream.writeObject(this.valueTraversal.clone());
+            if (null != this.valueTraversal) {
+                try {
+                    // if there is a lambda that can not be serialized, then 
simply use TraverserSets
+                    this.valueTraversal.setParent(EmptyStep.instance());
+                    Serializer.serializeObject(this.valueTraversal);
+                } catch (final IOException e) {
+                    this.valueTraversal = null;
+                }
+            }
+            outputStream.writeObject(null == this.valueTraversal ? null : 
this.valueTraversal.clone());
         }
 
         private void readObject(final ObjectInputStream inputStream) throws 
IOException, ClassNotFoundException {
             this.valueTraversal = (Traversal.Admin<?, V>) 
inputStream.readObject();
-            this.barrierStep = 
TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, 
this.valueTraversal).orElse(null);
+            this.barrierStep = null == this.valueTraversal ? null : 
TraversalHelper.getFirstStepOfAssignableClass(Barrier.class, 
this.valueTraversal).orElse(null);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/d2eb63c4/hadoop-gremlin/src/test/java/org/apache/tinkerpop/gremlin/hadoop/groovy/plugin/HadoopGremlinPluginCheck.java
----------------------------------------------------------------------
diff --git 
a/hadoop-gremlin/src/test/java/org/apache/tinkerpop/gremlin/hadoop/groovy/plugin/HadoopGremlinPluginCheck.java
 
b/hadoop-gremlin/src/test/java/org/apache/tinkerpop/gremlin/hadoop/groovy/plugin/HadoopGremlinPluginCheck.java
index d0a2e61..8e4ff25 100644
--- 
a/hadoop-gremlin/src/test/java/org/apache/tinkerpop/gremlin/hadoop/groovy/plugin/HadoopGremlinPluginCheck.java
+++ 
b/hadoop-gremlin/src/test/java/org/apache/tinkerpop/gremlin/hadoop/groovy/plugin/HadoopGremlinPluginCheck.java
@@ -22,6 +22,7 @@ package org.apache.tinkerpop.gremlin.hadoop.groovy.plugin;
 import org.apache.tinkerpop.gremlin.AbstractGremlinTest;
 import org.apache.tinkerpop.gremlin.LoadGraphWith;
 import org.apache.tinkerpop.gremlin.TestHelper;
+import org.apache.tinkerpop.gremlin.groovy.loaders.GremlinLoader;
 import org.apache.tinkerpop.gremlin.groovy.plugin.RemoteAcceptor;
 import org.apache.tinkerpop.gremlin.groovy.util.SugarTestHelper;
 import org.apache.tinkerpop.gremlin.groovy.util.TestableConsolePluginAcceptor;
@@ -34,11 +35,13 @@ import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * This is an test that is mean to be used in the context of the {@link 
HadoopGremlinSuite} and shouldn't be
@@ -84,7 +87,14 @@ public class HadoopGremlinPluginCheck extends 
AbstractGremlinTest {
         SugarTestHelper.clearRegistry(this.graphProvider);
         this.console.addBinding("graph", this.graph);
         this.console.addBinding("g", this.g);
-        this.remote.connect(Arrays.asList("graph"));
+        //
+        this.remote.connect(Arrays.asList("graph", "g"));
+        try {
+            this.remote.submit(Arrays.asList("g.V.name.map{it.length()}.sum"));
+            fail("Should not allow sugar usage");
+        } catch (final Exception e) {
+            // this is good
+        }
         //
         this.remote.configure(Arrays.asList("useSugar", "true"));
         this.remote.connect(Arrays.asList("graph", "g"));
@@ -96,6 +106,41 @@ public class HadoopGremlinPluginCheck extends 
AbstractGremlinTest {
 
     @Test
     @LoadGraphWith(LoadGraphWith.GraphData.MODERN)
+    public void shouldSupportRemoteGroupTraversal() throws Exception {
+        SugarTestHelper.clearRegistry(this.graphProvider);
+        GremlinLoader.load();
+        this.console.addBinding("graph", this.graph);
+        this.console.addBinding("g", this.g);
+        this.remote.connect(Arrays.asList("graph"));
+        //
+        this.remote.connect(Arrays.asList("graph", "g"));
+        Traversal<?, Map<String, List<String>>> traversal = (Traversal<?, 
Map<String, List<String>>>) 
this.remote.submit(Arrays.asList("g.V().out().group().by{it.value('name')[1]}.by('name')"));
+        Map<String, List<String>> map = traversal.next();
+        assertEquals(3, map.size());
+        assertEquals(1, map.get("a").size());
+        assertEquals("vadas", map.get("a").get(0));
+        assertEquals(1, map.get("i").size());
+        assertEquals("ripple", map.get("i").get(0));
+        assertEquals(4, map.get("o").size());
+        assertTrue(map.get("o").contains("josh"));
+        assertTrue(map.get("o").contains("lop"));
+        assertNotNull(this.console.getBindings().get(RemoteAcceptor.RESULT));
+        //
+        traversal = (Traversal<?, Map<String, List<String>>>) 
this.remote.submit(Arrays.asList("g.V().out().group().by(label).by{it.value('name')[1]}"));
+        map = traversal.next();
+        assertEquals(2, map.size());
+        assertEquals(4, map.get("software").size());
+        assertTrue(map.get("software").contains("o"));
+        assertTrue(map.get("software").contains("i"));
+        assertEquals(2, map.get("person").size());
+        assertTrue(map.get("person").contains("o"));
+        assertTrue(map.get("person").contains("a"));
+        assertNotNull(this.console.getBindings().get(RemoteAcceptor.RESULT));
+    }
+
+
+    @Test
+    @LoadGraphWith(LoadGraphWith.GraphData.MODERN)
     public void shouldSupportHDFSMethods() throws Exception {
         List<String> ls = (List<String>) this.console.eval("hdfs.ls()");
         for (final String line : ls) {

Reply via email to