From 0022b7f6be25eb7d3c778b137beb6e8a7d2784ca Mon Sep 17 00:00:00 2001 From: Dan LaRocque Date: Wed, 10 Aug 2016 18:52:13 -0400 Subject: [PATCH 1/2] TINKERPOP-1397 Fix StarGraph.addEdge For self-loops, StarGraph.addEdge used to put a single StarOutEdge into both its inEdges and outEdges maps, potentially causing problems in applyGraphFilter. This change makes StarGraph.addEdge put the appropriate type of edge (Star(Out/In)Edge) in the associated map. The IDs for each edge instance are kept in agreement. This change is @okram's, who suggested it in PR #372. I merely reviewed it and added a couple of comments. --- .../gremlin/structure/util/star/StarGraph.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java index f5166305506..2089c424672 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java @@ -37,6 +37,7 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; @@ -290,14 +291,16 @@ public void dropVertexProperties(final String... propertyKeys) { public Edge addEdge(final String label, final Vertex inVertex, final Object... keyValues) { final Edge edge = this.addOutEdge(label, inVertex, keyValues); if (inVertex.equals(this)) { - if(null == this.inEdges) - this.inEdges = new HashMap<>(); - List inE = this.inEdges.get(label); - if (null == inE) { - inE = new ArrayList<>(); - this.inEdges.put(label, inE); + if (ElementHelper.getIdValue(keyValues).isPresent()) { + // reuse edge ID from method params + this.addInEdge(label, this, keyValues); + } else { + // copy edge ID that we just allocated with addOutEdge + final Object[] keyValuesWithId = Arrays.copyOf(keyValues, keyValues.length + 2); + keyValuesWithId[keyValuesWithId.length - 2] = T.id; + keyValuesWithId[keyValuesWithId.length - 1] = edge.id(); + this.addInEdge(label, this, keyValuesWithId); } - inE.add(edge); } return edge; } From 7842c4e7e2e3c2b33fc1bca7a8a80e165080bc59 Mon Sep 17 00:00:00 2001 From: Dan LaRocque Date: Wed, 10 Aug 2016 18:59:54 -0400 Subject: [PATCH 2/2] TINKERPOP-1397 Add StarGraph bothE filtering test --- .../structure/util/star/StarGraphTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraphTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraphTest.java index 034afad0895..c49dab0bd75 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraphTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraphTest.java @@ -23,6 +23,8 @@ import org.apache.tinkerpop.gremlin.FeatureRequirement; import org.apache.tinkerpop.gremlin.LoadGraphWith; import org.apache.tinkerpop.gremlin.TestHelper; +import org.apache.tinkerpop.gremlin.process.computer.GraphFilter; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.structure.Direction; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Graph; @@ -246,6 +248,38 @@ public void shouldHandleSelfLoops() { TestHelper.validateEdgeEquality(e1, v1.edges(Direction.IN).next()); } + @Test + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_ADD_PROPERTY) + public void shouldHandleBothEdgesGraphFilterOnSelfLoop() { + assertEquals(0l, IteratorUtils.count(graph.vertices())); + assertEquals(0l, IteratorUtils.count(graph.edges())); + + // these vertex label, edge label, and property names/values were copied from existing tests + StarGraph starGraph = StarGraph.open(); + Vertex vertex = starGraph.addVertex(T.label, "person", "name", "furnace"); + Edge edge = vertex.addEdge("self", vertex); + edge.property("acl", "private"); + + // traversing a self-loop should yield the edge once for inE/outE + // and the edge twice for bothE (one edge emitted two times, not two edges) + assertEquals(1L, IteratorUtils.count(starGraph.traversal().V().inE())); + assertEquals(1L, IteratorUtils.count(starGraph.traversal().V().outE())); + assertEquals(2L, IteratorUtils.count(starGraph.traversal().V().bothE())); + + // Try a filter that retains BOTH + GraphFilter graphFilter = new GraphFilter(); + graphFilter.setEdgeFilter(__.bothE("self")); + starGraph = starGraph.applyGraphFilter(graphFilter).get(); + + // Retest traversal counts after filtering + assertEquals(1L, IteratorUtils.count(starGraph.traversal().V().inE())); + assertEquals(1L, IteratorUtils.count(starGraph.traversal().V().outE())); + assertEquals(2L, IteratorUtils.count(starGraph.traversal().V().bothE())); + } + private Pair serializeDeserialize(final StarGraph starGraph) { final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();