This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-1682
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 12a6ef0b9b071518fa4419cbbdbd7a34849fd8de
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Thu May 7 08:53:23 2020 -0400
TINKERPOP-1682 Support by(T) on Property
---
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.5.x.asciidoc | 29 ++++++++
.../process/traversal/lambda/TokenTraversal.java | 19 +++++-
.../process/traversal/step/sideEffect/IoStep.java | 2 +-
.../ByModulatorOptimizationStrategy.java | 8 +--
.../structure/io/graphml/GraphMLWriter.java | 1 -
.../traversal/lambda/TokenTraversalTest.java | 78 ++++++++++++++++++++++
.../ByModulatorOptimizationStrategyTest.java | 12 ++--
gremlin-test/features/map/Select.feature | 24 +++++++
.../process/traversal/step/map/SelectTest.java | 32 ++++++++-
10 files changed, 191 insertions(+), 15 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index a80aac5..9f4bf28 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -30,6 +30,7 @@ This release also includes changes from <<release-3-4-3,
3.4.3>>.
* Ensured better consistency of the use of `null` as arguments to mutation
steps.
* Allowed `property(T.label,Object)` to be used if no value was supplied to
`addV(String)`.
* Added a `Graph.Feature` for `supportsNullPropertyValues`.
+* Modified `TokenTraversal` to support `Property` thus `by(key)` and
`by(value)` can now apply to `Edge` and meta-properties.
* Added `Grouping` step interface.
* Added `TraversalParent.replaceTraversal()` which can replace a direct child
traversal.
* Added `ByModulatorOptimizationStrategy` which replaces certain standard
traversals w/ optimized traversals (e.g. `TokenTraversal`).
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc
b/docs/src/upgrade/release-3.5.x.asciidoc
index 50180dd..f1b2ee3 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -209,6 +209,35 @@ be replaced by `by(id)`, thus replacing a step-based
traversal with a token-base
See: link:https://issues.apache.org/jira/browse/TINKERPOP-1682[TINKERPOP-1682]
+==== by(T) for Property
+
+The `Property` interface is not included in the hierarchy of `Element`. This
means that an edge property or a
+meta-property are not considered elements the way that a `VertexProperty` is.
As a result, some usages of `T` in
+relation to properties do not work consistently. One such example is `by(T)`,
a token-based traversal, where the
+following works for a `VertexProperty` but will not for edge properties or
meta-properties:
+
+[source,text]
+----
+gremlin> g.V(1).properties().as('a').select('a').by(key)
+==>name
+==>age
+----
+
+For a `Property` you would need to use `key()`-step:
+
+[source,text]
+----
+gremlin> g.E(11).properties().as('a').select(last,'a').by(key())
+==>weight
+----
+
+Aside from the inconsistency, this issue also presents a situation where
performance is impacted as token-based
+traversals are inherently faster than step-based ones. In 3.5.0, this issue
has been resolved in conjunction with the
+introduction of `ByModulatorOptimizationStrategy` which will optimize
`by(key())` and `by(value())` to their
+appropriate token versions automatically.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1682[TINKERPOP-1682]
+
==== Python 2.x Support
The gremlinpython module no longer supports Python 2.x. Users must use Python
3 going forward. For the most part, from
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversal.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversal.java
index d41c402..e272ccf 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversal.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversal.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.lambda;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
import org.apache.tinkerpop.gremlin.structure.Element;
+import org.apache.tinkerpop.gremlin.structure.Property;
import org.apache.tinkerpop.gremlin.structure.T;
import java.util.Objects;
@@ -27,7 +28,7 @@ import java.util.Objects;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
-public final class TokenTraversal<S extends Element, E> extends
AbstractLambdaTraversal<S, E> {
+public final class TokenTraversal<S, E> extends AbstractLambdaTraversal<S, E> {
private E e;
private final T t;
@@ -43,7 +44,21 @@ public final class TokenTraversal<S extends Element, E>
extends AbstractLambdaTr
@Override
public void addStart(final Traverser.Admin<S> start) {
- this.e = (E) this.t.apply(start.get());
+ final S s = start.get();
+ if (s instanceof Element)
+ this.e = (E) this.t.apply((Element) start.get());
+ else if (s instanceof Property) {
+ // T.apply() doesn't work on Property because the inheritance
hierarchy doesn't make it an Element. have
+ // to special case it here. only T.key/value make any sense for it.
+ if (t == T.key)
+ this.e = (E) ((Property) s).key();
+ else if (t == T.value)
+ this.e = ((Property<E>) s).value();
+ else
+ throw new IllegalStateException(String.format("TokenTraversal
support of Property does not allow selection by %s", t));
+ } else
+ throw new IllegalStateException(String.format("TokenTraversal
support of %s does not allow selection by %s", s.getClass().getName(), t));
+
}
@Override
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
index 861b318..0ca02ee 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/IoStep.java
@@ -188,7 +188,7 @@ public class IoStep<S> extends AbstractStep<S,S> implements
ReadWriting {
final GryoMapper.Builder builder = GryoMapper.build();
detectRegistries().forEach(builder::addRegistry);
return GryoWriter.build().mapper(builder.create()).create();
- }else if (objectOrClass.equals(IO.graphml))
+ } else if (objectOrClass.equals(IO.graphml))
return GraphMLWriter.build().create();
else {
try {
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java
index 579dcf4..0faaef9 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java
@@ -32,6 +32,8 @@ import
org.apache.tinkerpop.gremlin.process.traversal.step.map.GroupStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.IdStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.LabelStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertiesStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyKeyStep;
+import
org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyValueStep;
import
org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.GroupSideEffectStep;
import
org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.IdentityStep;
import
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
@@ -95,12 +97,10 @@ public final class ByModulatorOptimizationStrategy extends
AbstractTraversalStra
step.replaceLocalChild(traversal, new TokenTraversal<>(T.id));
} else if (singleStep instanceof LabelStep) {
step.replaceLocalChild(traversal, new TokenTraversal<>(T.label));
-/* todo: this fails for `Property`s (e.g.
outE().property().as("a").select("a").by(key/value))
} else if (singleStep instanceof PropertyKeyStep) {
- step.setModulateByTraversal(n, new TokenTraversal<>(T.key));
+ step.replaceLocalChild(traversal, new TokenTraversal<>(T.key));
} else if (singleStep instanceof PropertyValueStep) {
- step.setModulateByTraversal(n, new TokenTraversal<>(T.value));
-*/
+ step.replaceLocalChild(traversal, new TokenTraversal<>(T.value));
} else if (singleStep instanceof IdentityStep) {
step.replaceLocalChild(traversal, new IdentityTraversal<>());
}
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
index 3991e4e..5c70e99 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
@@ -24,7 +24,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversalTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversalTest.java
new file mode 100644
index 0000000..7d1ad54
--- /dev/null
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/TokenTraversalTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.process.traversal.lambda;
+
+import org.apache.tinkerpop.gremlin.process.traversal.traverser.B_O_Traverser;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Property;
+import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class TokenTraversalTest {
+ @Test
+ public void shouldWorkOnVertex() {
+ final TokenTraversal<Vertex, Integer> t = new TokenTraversal<>(T.id);
+ final Vertex v = mock(Vertex.class);
+ when(v.id()).thenReturn(100);
+ t.addStart(new B_O_Traverser<>(v, 1).asAdmin());
+ assertEquals(100, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnVertexProperty() {
+ final TokenTraversal<VertexProperty, Integer> t = new
TokenTraversal<>(T.id);
+ final VertexProperty vo = mock(VertexProperty.class);
+ when(vo.id()).thenReturn(100);
+ t.addStart(new B_O_Traverser<>(vo, 1).asAdmin());
+ assertEquals(100, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnEdge() {
+ final TokenTraversal<Edge, Integer> t = new TokenTraversal<>(T.id);
+ final Edge e = mock(Edge.class);
+ when(e.id()).thenReturn(100);
+ t.addStart(new B_O_Traverser<>(e, 1).asAdmin());
+ assertEquals(100, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnPropertyKey() {
+ final TokenTraversal<Property<String>, String> t = new
TokenTraversal<>(T.key);
+ final Property<String> p = mock(Property.class);
+ when(p.key()).thenReturn("name");
+ t.addStart(new B_O_Traverser<>(p, 1).asAdmin());
+ assertEquals("name", t.next());
+ }
+
+ @Test
+ public void shouldWorkOnPropertyValue() {
+ final TokenTraversal<Property<String>, String> t = new
TokenTraversal<>(T.value);
+ final Property<String> p = mock(Property.class);
+ when(p.value()).thenReturn("marko");
+ t.addStart(new B_O_Traverser<>(p, 1).asAdmin());
+ assertEquals("marko", t.next());
+ }
+}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategyTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategyTest.java
index 23c4576..a3a0044 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategyTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategyTest.java
@@ -90,14 +90,14 @@ public class ByModulatorOptimizationStrategyTest {
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.label()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.label),
});
- /*result.add(new Traversal[]{
+ originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.key()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.key),
});
- result.add(new Traversal[]{
+ originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.value()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.value),
- });*/
+ });
originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.identity()),
((GraphTraversal) traversal.asAdmin().clone()).by(),
@@ -137,14 +137,14 @@ public class ByModulatorOptimizationStrategyTest {
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.label().fold()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.label),
});
- /*result.add(new Traversal[]{
+ originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.key().fold()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.key),
});
- result.add(new Traversal[]{
+ originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.value().fold()),
((GraphTraversal) traversal.asAdmin().clone()).by(T.value),
- });*/
+ });
originalAndOptimized.add(new Traversal[]{
((GraphTraversal<?, ?>)
traversal.asAdmin().clone()).by(__.identity()),
((GraphTraversal) traversal.asAdmin().clone()).by(),
diff --git a/gremlin-test/features/map/Select.feature
b/gremlin-test/features/map/Select.feature
index 0d4913a..8c759d6 100644
--- a/gremlin-test/features/map/Select.feature
+++ b/gremlin-test/features/map/Select.feature
@@ -744,3 +744,27 @@ Feature: Step - select()
| s[a,b] |
| s[c] |
And the graph should return 6 for count of "g.V().as(\"a\",
\"b\").out().as(\"c\").path().select(Column.keys)"
+
+ Scenario: g_EX11X_propertiesXweightX_asXaX_selectXaX_byXkeyX
+ Given the modern graph
+ And using the parameter e11Id defined as "e[josh-created->lop].id"
+ And the traversal of
+ """
+ g.E(e11Id).properties("weight").as("a").select("a").by(T.key)
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | weight |
+
+ Scenario: g_EX11X_propertiesXweightX_asXaX_selectXaX_byXvalueX
+ Given the modern graph
+ And using the parameter e11Id defined as "e[josh-created->lop].id"
+ And the traversal of
+ """
+ g.E(e11Id).properties("weight").as("a").select("a").by(T.value)
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | d[0.4].d |
\ No newline at end of file
diff --git
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java
index 0f5afbd..4e5d4a0 100644
---
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java
+++
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectTest.java
@@ -26,6 +26,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Pop;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -38,7 +39,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.stream.Stream;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.CREW;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
@@ -155,6 +155,10 @@ public abstract class SelectTest extends
AbstractGremlinProcessTest {
public abstract Traversal<Vertex, Vertex>
get_g_VX1X_asXaX_repeatXout_asXaXX_timesX2X_selectXlast_aX(final Object v1Id);
+ public abstract Traversal<Edge, Object>
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXkeyX(final Object e11Id);
+
+ public abstract Traversal<Edge, Object>
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXvalueX(final Object e11Id);
+
// when labels don't exist
public abstract Traversal<Vertex, String>
get_g_V_untilXout_outX_repeatXin_asXaXX_selectXaX_byXtailXlocalX_nameX();
@@ -703,6 +707,22 @@ public abstract class SelectTest extends
AbstractGremlinProcessTest {
assertEquals(Collections.emptyList(), traversal.toList());
}
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_EX11X_propertiesXweightX_asXaX_selectXaX_byXkeyX() {
+ final Traversal<Edge, Object> traversal =
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXkeyX(convertToEdgeId("josh",
"created", "lop"));
+ printTraversalForm(traversal);
+ assertEquals("weight", traversal.next());
+ }
+
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_EX11X_propertiesXweightX_asXaX_selectXaX_byXvalueX() {
+ final Traversal<Edge, Object> traversal =
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXvalueX(convertToEdgeId("josh",
"created", "lop"));
+ printTraversalForm(traversal);
+ assertEquals(0.4d, (double) traversal.next(), 0.0001d);
+ }
+
// when labels don't exist
@Test
@@ -1074,6 +1094,16 @@ public abstract class SelectTest extends
AbstractGremlinProcessTest {
return g.V().as("a").where(__.out("knows")).<Vertex>select("a");
}
+ @Override
+ public Traversal<Edge, Object>
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXkeyX(final Object e11Id) {
+ return
g.E(e11Id).properties("weight").as("a").select("a").by(T.key);
+ }
+
+ @Override
+ public Traversal<Edge, Object>
get_g_EX11X_propertiesXweightX_asXaX_selectXaX_byXvalueX(final Object e11Id) {
+ return
g.E(e11Id).properties("weight").as("a").select("a").by(T.value);
+ }
+
// select columns test
@Override
