You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by jo...@apache.org on 2017/11/22 14:47:11 UTC
[18/50] [abbrv] tinkerpop git commit: fixed a hasId([])
ArrayOutOfBoundsException bug that occurs in the rare situation where a user
provides an empty collection of ids. Test cases developed by @dkuppitz.
fixed a hasId([]) ArrayOutOfBoundsException bug that occurs in the rare situation where a user provides an empty collection of ids. Test cases developed by @dkuppitz.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/74ca03de
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/74ca03de
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/74ca03de
Branch: refs/heads/TINKERPOP-1489
Commit: 74ca03dea1a7db7b2af39f46020cf8a75a2ea5c4
Parents: e20b8ae
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Mon Nov 6 14:36:22 2017 -0700
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Mon Nov 6 14:36:22 2017 -0700
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../process/traversal/step/map/GraphStep.java | 15 +++--
.../traversal/step/util/HasContainer.java | 17 ++++--
.../traversal/step/filter/GroovyHasTest.groovy | 24 +++++++-
.../process/traversal/step/filter/HasTest.java | 63 +++++++++++++++++++-
.../step/sideEffect/Neo4jGraphStep.java | 4 ++
.../step/sideEffect/TinkerGraphStep.java | 8 ++-
7 files changed, 117 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 52ba2a3..128d13d 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-2-7]]
=== TinkerPop 3.2.7 (Release Date: NOT OFFICIALLY RELEASED YET)
+* Fixed an `ArrayOutOfBoundsException` in `hasId()` for the rare situation when the provided collection is empty.
* Bump to Netty 4.0.52
* `TraversalVertexProgram` ``profile()` now accounts for worker iteration in `GraphComputer` OLAP.
* Added a test for self-edges and fixed `Neo4jVertex` to provided repeated self-edges on `BOTH`.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/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..e40271c 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
@@ -36,8 +36,6 @@ import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
import org.apache.tinkerpop.gremlin.util.iterator.EmptyIterator;
-import java.io.Closeable;
-import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -102,10 +100,15 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
}
public void addIds(final Object... newIds) {
- this.ids = ArrayUtils.addAll(this.ids,
- (newIds.length == 1 && newIds[0] instanceof Collection) ?
- ((Collection) newIds[0]).toArray(new Object[((Collection) newIds[0]).size()]) :
- newIds);
+ if (this.ids.length == 0 &&
+ newIds.length == 1 &&
+ newIds[0] instanceof Collection && ((Collection) newIds[0]).isEmpty())
+ this.ids = null;
+ else
+ this.ids = ArrayUtils.addAll(this.ids,
+ (newIds.length == 1 && newIds[0] instanceof Collection) ?
+ ((Collection) newIds[0]).toArray(new Object[((Collection) newIds[0]).size()]) :
+ newIds);
}
public void clearIds() {
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/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..08ab389 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
@@ -59,14 +59,19 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element>
// 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();
+ ((Collection) this.predicate.getValue()).isEmpty() ?
+ new Object() :
+ ((Collection) this.predicate.getValue()).toArray()[0] :
+ 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
// the constructor. the original value in P is maintained separately
this.testingIdString = this.key.equals(T.id.getAccessor()) && valueInstance instanceof String;
if (this.testingIdString)
- this.predicate.setValue(this.predicate.getValue() instanceof Collection ? IteratorUtils.set(IteratorUtils.map(((Collection<Object>) this.predicate.getValue()).iterator(), Object::toString)) : this.predicate.getValue().toString());
+ this.predicate.setValue(this.predicate.getValue() instanceof Collection ?
+ IteratorUtils.set(IteratorUtils.map(((Collection<Object>) this.predicate.getValue()).iterator(), Object::toString)) :
+ this.predicate.getValue().toString());
}
}
@@ -162,9 +167,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()) {
+ 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");
+ }
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/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..c70a50d 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")
+ }
}
-}
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/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..d8457f1 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();
+ }
}
-}
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
----------------------------------------------------------------------
diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
index 7df47d0..f196a8d 100644
--- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
+++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/process/traversal/step/sideEffect/Neo4jGraphStep.java
@@ -52,10 +52,14 @@ public final class Neo4jGraphStep<S, E extends Element> extends GraphStep<S, E>
}
private Iterator<? extends Edge> edges() {
+ if (null == this.ids)
+ return Collections.emptyIterator();
return IteratorUtils.filter(this.getTraversal().getGraph().get().edges(this.ids), edge -> HasContainer.testAll(edge, this.hasContainers));
}
private Iterator<? extends Vertex> vertices() {
+ if (null == this.ids)
+ return Collections.emptyIterator();
final Neo4jGraph graph = (Neo4jGraph) this.getTraversal().getGraph().get();
return graph.getTrait().lookupVertices(graph, this.hasContainers, this.ids);
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/74ca03de/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..5933ebe 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,9 @@ 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 (null == this.ids)
+ return Collections.emptyIterator();
+ else if (this.ids.length > 0)
return this.iteratorList(graph.edges(this.ids));
else
return null == indexedContainer ?
@@ -77,7 +79,9 @@ 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 (null == this.ids)
+ return Collections.emptyIterator();
+ else if (this.ids.length > 0)
return this.iteratorList(graph.vertices(this.ids));
else
return null == indexedContainer ?