You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by dk...@apache.org on 2017/11/01 14:43:30 UTC

tinkerpop git commit: Handle empty collections in hasId()

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1802 [created] 31e9bddc7


Handle empty collections in hasId()


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

Branch: refs/heads/TINKERPOP-1802
Commit: 31e9bddc7681dece0c68aa2a004727e8efa33485
Parents: bef43d6
Author: Daniel Kuppitz <da...@hotmail.com>
Authored: Wed Oct 18 19:54:19 2017 -0700
Committer: Daniel Kuppitz <da...@hotmail.com>
Committed: Wed Nov 1 07:42:59 2017 -0700

----------------------------------------------------------------------
 .../process/traversal/step/map/GraphStep.java   |  4 ++
 .../traversal/step/util/HasContainer.java       | 17 ++++--
 .../traversal/step/filter/GroovyHasTest.groovy  | 22 +++++++
 .../process/traversal/step/filter/HasTest.java  | 61 ++++++++++++++++++++
 .../step/sideEffect/TinkerGraphStep.java        |  8 +--
 .../structure/TinkerGraphPlayTest.java          | 25 +++-----
 6 files changed, 111 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
index 7ab7d13..3566279 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
@@ -51,6 +51,7 @@ import java.util.function.Supplier;
 public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implements GraphComputing, AutoCloseable {
 
     protected final Class<E> returnClass;
+    protected boolean filterByIds;
     protected Object[] ids;
     protected transient Supplier<Iterator<E>> iteratorSupplier;
     protected boolean isStart;
@@ -62,6 +63,7 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
     public GraphStep(final Traversal.Admin traversal, final Class<E> returnClass, final boolean isStart, final Object... ids) {
         super(traversal);
         this.returnClass = returnClass;
+        this.filterByIds = ids.length > 0;
         this.ids = (ids.length == 1 && ids[0] instanceof Collection) ? ((Collection) ids[0]).toArray(new Object[((Collection) ids[0]).size()]) : ids;
         this.isStart = isStart;
         this.iteratorSupplier = () -> (Iterator<E>) (Vertex.class.isAssignableFrom(this.returnClass) ?
@@ -102,6 +104,7 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
     }
 
     public void addIds(final Object... newIds) {
+        this.filterByIds = true;
         this.ids = ArrayUtils.addAll(this.ids,
                 (newIds.length == 1 && newIds[0] instanceof Collection) ?
                         ((Collection) newIds[0]).toArray(new Object[((Collection) newIds[0]).size()]) :
@@ -109,6 +112,7 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
     }
 
     public void clearIds() {
+        this.filterByIds = false;
         this.ids = new Object[0];
     }
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
index 3a3d9cc..b367c45 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java
@@ -58,8 +58,11 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element>
             enforceHomogenousCollectionIfPresent(predicateValue);
 
             // grab an instance of a value which is either the first item in a homogeneous collection or the value itself
-            final Object valueInstance = this.predicate.getValue() instanceof Collection ?
-                    ((Collection) this.predicate.getValue()).toArray()[0] : this.predicate.getValue();
+            final Object valueInstance;
+            if (this.predicate.getValue() instanceof Collection) {
+                final Collection collection = (Collection) this.predicate.getValue();
+                valueInstance = collection.isEmpty() ? null : collection.iterator().next();
+            } else valueInstance = this.predicate.getValue();
 
             // if the key being evaluated is id then the has() test can evaluate as a toString() representation of the
             // identifier.  this could be done in the test() method but it seems cheaper to do the conversion once in
@@ -162,9 +165,11 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element>
     private void enforceHomogenousCollectionIfPresent(final Object predicateValue) {
         if (predicateValue instanceof Collection) {
             final Collection collection = (Collection) predicateValue;
-            Class<?> first = collection.toArray()[0].getClass();
-            if (!((Collection) predicateValue).stream().map(Object::getClass).allMatch(c -> first.equals(c)))
-                throw new IllegalArgumentException("Has comparisons on a collection of ids require ids to all be of the same type");
+            if (!collection.isEmpty()) {
+                final Class<?> first = collection.iterator().next().getClass();
+                if (!((Collection) predicateValue).stream().map(Object::getClass).allMatch(first::equals))
+                    throw new IllegalArgumentException("Has comparisons on a collection of ids require ids to all be of the same type");
+            }
         }
     }
 
@@ -173,7 +178,7 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element>
             if (!hasContainer.test(element))
                 return false;
         }
-        return true;
+        return !hasContainers.isEmpty();
     }
 
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyHasTest.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyHasTest.groovy b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyHasTest.groovy
index 76e79a4..1dec2f4 100644
--- a/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyHasTest.groovy
+++ b/gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/filter/GroovyHasTest.groovy
@@ -18,7 +18,9 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal.step.filter
 
+import org.apache.tinkerpop.gremlin.process.traversal.P
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__
 import org.apache.tinkerpop.gremlin.process.traversal.util.ScriptTraversal
 import org.apache.tinkerpop.gremlin.structure.Edge
 import org.apache.tinkerpop.gremlin.structure.Vertex
@@ -169,5 +171,25 @@ public abstract class GroovyHasTest {
         public Traversal<Vertex, Vertex> get_g_V_hasLabelXpersonX_hasLabelXsoftwareX() {
             new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasLabel('person').hasLabel('software')")
         }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXemptyX_count() {
+            new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasId([]).count")
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXwithinXemptyXX_count() {
+            new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasId(within([])).count")
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXwithoutXemptyXX_count() {
+            new ScriptTraversal<>(g, "gremlin-groovy", "g.V.hasId(without([])).count")
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_notXhasIdXwithinXemptyXXX_count() {
+            new ScriptTraversal<>(g, "gremlin-groovy", "g.V.not(hasId(within([]))).count")
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
index 9bd3e23..29904b5 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
@@ -35,6 +35,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.CREW;
@@ -108,6 +109,14 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
 
     public abstract Traversal<Vertex, Vertex> get_g_V_hasLabelXpersonX_hasLabelXsoftwareX();
 
+    public abstract Traversal<Vertex, Long> get_g_V_hasIdXemptyX_count();
+
+    public abstract Traversal<Vertex, Long> get_g_V_hasIdXwithinXemptyXX_count();
+
+    public abstract Traversal<Vertex, Long> get_g_V_hasIdXwithoutXemptyXX_count();
+
+    public abstract Traversal<Vertex, Long> get_g_V_notXhasIdXwithinXemptyXXX_count();
+
     @Test
     @LoadGraphWith(MODERN)
     public void g_V_outXcreatedX_hasXname__mapXlengthX_isXgtX3XXX_name() {
@@ -449,6 +458,38 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
         assertFalse(traversal.hasNext());
     }
 
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_hasIdXemptyX_count() {
+        final Traversal<Vertex, Long> traversal = get_g_V_hasIdXemptyX_count();
+        printTraversalForm(traversal);
+        assertEquals(0L, traversal.next().longValue());
+    }
+
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_hasIdXwithinXemptyXX_count() {
+        final Traversal<Vertex, Long> traversal = get_g_V_hasIdXwithinXemptyXX_count();
+        printTraversalForm(traversal);
+        assertEquals(0L, traversal.next().longValue());
+    }
+
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_hasIdXwithoutXemptyXX_count() {
+        final Traversal<Vertex, Long> traversal = get_g_V_hasIdXwithoutXemptyXX_count();
+        printTraversalForm(traversal);
+        assertEquals(6L, traversal.next().longValue());
+    }
+
+    @Test
+    @LoadGraphWith(MODERN)
+    public void g_V_notXhasIdXwithinXemptyXXX_count() {
+        final Traversal<Vertex, Long> traversal = get_g_V_notXhasIdXwithinXemptyXXX_count();
+        printTraversalForm(traversal);
+        assertEquals(6L, traversal.next().longValue());
+    }
+
     public static class Traversals extends HasTest {
         @Override
         public Traversal<Edge, Edge> get_g_EX11X_outV_outE_hasXid_10X(final Object e11Id, final Object e8Id) {
@@ -589,5 +630,25 @@ public abstract class HasTest extends AbstractGremlinProcessTest {
         public Traversal<Vertex, Vertex> get_g_V_hasLabelXpersonX_hasLabelXsoftwareX() {
             return g.V().hasLabel("person").hasLabel("software");
         }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXemptyX_count() {
+            return g.V().hasId(Collections.emptyList()).count();
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXwithinXemptyXX_count() {
+            return g.V().hasId(P.within(Collections.emptyList())).count();
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_hasIdXwithoutXemptyXX_count() {
+            return g.V().hasId(P.without(Collections.emptyList())).count();
+        }
+
+        @Override
+        public Traversal<Vertex, Long> get_g_V_notXhasIdXwithinXemptyXXX_count() {
+            return g.V().not(__.hasId(P.within(Collections.emptyList()))).count();
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java
index 36c67ac..860ca04 100644
--- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java
+++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/sideEffect/TinkerGraphStep.java
@@ -63,7 +63,7 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E>
         final TinkerGraph graph = (TinkerGraph) this.getTraversal().getGraph().get();
         final HasContainer indexedContainer = getIndexKey(Edge.class);
         // ids are present, filter on them first
-        if (this.ids != null && this.ids.length > 0)
+        if (this.ids != null && this.filterByIds)
             return this.iteratorList(graph.edges(this.ids));
         else
             return null == indexedContainer ?
@@ -77,7 +77,7 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E>
         final TinkerGraph graph = (TinkerGraph) this.getTraversal().getGraph().get();
         final HasContainer indexedContainer = getIndexKey(Vertex.class);
         // ids are present, filter on them first
-        if (this.ids != null && this.ids.length > 0)
+        if (this.ids != null && this.filterByIds)
             return this.iteratorList(graph.vertices(this.ids));
         else
             return null == indexedContainer ?
@@ -109,8 +109,8 @@ public final class TinkerGraphStep<S, E extends Element> extends GraphStep<S, E>
         final List<E> list = new ArrayList<>();
         while (iterator.hasNext()) {
             final E e = iterator.next();
-            if (HasContainer.testAll(e, this.hasContainers))
-                list.add(e);
+            if (this.hasContainers.isEmpty() || HasContainer.testAll(e, this.hasContainers))
+                if (!(this.filterByIds && this.ids.length == 0)) list.add(e);
         }
         return list.iterator();
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/31e9bddc/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
----------------------------------------------------------------------
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
index 37187fe..2f7f51a 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphPlayTest.java
@@ -20,11 +20,7 @@ package org.apache.tinkerpop.gremlin.tinkergraph.structure;
 
 import org.apache.tinkerpop.gremlin.jsr223.JavaTranslator;
 import org.apache.tinkerpop.gremlin.process.computer.Computer;
-import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
-import org.apache.tinkerpop.gremlin.process.traversal.Operator;
-import org.apache.tinkerpop.gremlin.process.traversal.Order;
-import org.apache.tinkerpop.gremlin.process.traversal.Scope;
-import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.*;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
@@ -40,12 +36,7 @@ import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.function.Supplier;
 
 import static org.apache.tinkerpop.gremlin.process.traversal.P.lt;
@@ -244,12 +235,14 @@ public class TinkerGraphPlayTest {
     @Ignore
     public void testPlayDK() throws Exception {
 
-        Graph graph = TinkerGraph.open();
+        Graph graph = TinkerFactory.createModern();
         GraphTraversalSource g = graph.traversal();
-        graph.io(GraphMLIo.build()).readGraph("/projects/apache/tinkerpop/data/grateful-dead.xml");
-        System.out.println(g.V().filter(outE("sungBy").count().is(0)).explain());
-        System.out.println(g.V().filter(outE("sungBy").count().is(lt(1))).explain());
-        System.out.println(g.V().filter(outE("sungBy").count().is(1)).explain());
+        System.out.println("-- 1 --");
+        g.V().hasId(Collections.emptyList()).forEachRemaining(System.out::println);
+        System.out.println("-- 2 --");
+        g.V().hasId(P.within(Collections.emptyList())).forEachRemaining(System.out::println);
+        System.out.println("-- 3 --");
+        g.V().hasId(P.without(Collections.emptyList())).forEachRemaining(System.out::println);
     }
 
     @Test