You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2020/01/16 16:23:42 UTC
[tinkerpop] branch TINKERPOP-2312 created (now 3586053)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2312
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at 3586053 TINKERPOP-2312 Empty keys to group() should group to null
This branch includes the following new commits:
new 3586053 TINKERPOP-2312 Empty keys to group() should group to null
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2312 Empty keys to group() should
group to null
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2312
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 358605363f68d0d18332973518abf94ae3d0a93c
Author: stephen <sp...@gmail.com>
AuthorDate: Thu Jan 16 11:23:06 2020 -0500
TINKERPOP-2312 Empty keys to group() should group to null
---
docs/src/reference/gremlin-variants.asciidoc | 58 ++++++++++
docs/src/upgrade/release-3.5.x.asciidoc | 21 ++++
...mentValueTraversal.java => ValueTraversal.java} | 26 ++---
.../process/traversal/step/ByModulating.java | 6 +-
.../process/traversal/step/PathProcessor.java | 4 +-
.../process/traversal/step/map/GroupStep.java | 4 +-
.../strategy/decoration/SubgraphStrategy.java | 8 +-
.../process/traversal/util/TraversalHelper.java | 4 +-
.../lambda/ElementValueTraversalTest.java | 78 --------------
.../traversal/lambda/ValueTraversalTest.java | 118 +++++++++++++++++++++
.../optimization/PathProcessorStrategyTest.java | 14 +--
.../Gherkin/GherkinTestRunner.cs | 1 +
.../Gherkin/IgnoreException.cs | 9 +-
.../Gherkin/ScenarioData.cs | 6 +-
.../test/cucumber/feature-steps.js | 2 +
gremlin-test/features/sideEffect/Group.feature | 11 ++
.../traversal/step/sideEffect/GroupTest.java | 27 ++++-
17 files changed, 275 insertions(+), 122 deletions(-)
diff --git a/docs/src/reference/gremlin-variants.asciidoc b/docs/src/reference/gremlin-variants.asciidoc
index 74a58f7..5898050 100644
--- a/docs/src/reference/gremlin-variants.asciidoc
+++ b/docs/src/reference/gremlin-variants.asciidoc
@@ -1135,6 +1135,28 @@ and then it can be called from the application as follows:
include::../../../gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Docs/Reference/GremlinVariantsDslTests.cs[tags=dslExamples]
----
+[[gremlin-dotnet-differences]]
+=== Differences
+
+Gremlin allows for `Map` instances to include `null` keys, but `null` keys in C# `Dictionary` instances are not allowed.
+It is therefore necessary to rewrite a traversal such as:
+
+[source,javascript]
+----
+g.V().groupCount().by('age')
+----
+
+where "age" is not a valid key for all vertices in a way that will remove the need for a `null` to be returned.
+
+[source,javascript]
+----
+g.V().has('age').groupCount().by('age')
+g.V().hasLabel('person').groupCount().by('age')
+----
+
+Either of the above two options accomplishes the desired goal as both prevent `groupCount()` from having to process
+the possibility of `null`.
+
anchor:gremlin-dotnet-template[]
[[dotnet-application-examples]]
=== Application Examples
@@ -1323,3 +1345,39 @@ In situations where Javascript reserved words and global functions overlap with
bits of conflicting Gremlin get an underscore appended as a suffix:
*Steps* - <<from-step,from_()>>, <<in-step,in_()>>, <<with-step,with_()>>
+
+Gremlin allows for `Map` instances to include `null` keys, but `null` keys in Javascript have some interesting behavior
+as in:
+
+[source,text]
+----
+> var a = { null: 'something', 'b': 'else' };
+> JSON.stringify(a)
+'{"null":"something","b":"else"}'
+> JSON.parse(JSON.stringify(a))
+{ null: 'something', b: 'else' }
+> a[null]
+'something'
+> a['null']
+'something'
+----
+
+This behavior needs to be considered when using Gremlin to return such results. A typical situation where this might
+happen is with `group()` or `groupCount()` as in:
+
+[source,javascript]
+----
+g.V().groupCount().by('age')
+----
+
+where "age" is not a valid key for all vertices. In these cases, it will return `null` for that key and group on that.
+It may bet better in Javascript to filter away those vertices to avoid the return of `null` in the returned `Map`:
+
+[source,javascript]
+----
+g.V().has('age').groupCount().by('age')
+g.V().hasLabel('person').groupCount().by('age')
+----
+
+Either of the above two options accomplishes the desired goal as both prevent `groupCount()` from having to process
+the possibility of `null`.
\ No newline at end of file
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index db1426f..08b4ec2 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -164,6 +164,27 @@ gremlin> g.V().has('person','age',null)
==>v[13]
----
+The above described changes also has an effect on steps like `group()` and `groupCount()` which formerly produced
+exceptions when keys could not be found:
+
+[source,text]
+----
+gremlin> g.V().group().by('age')
+The property does not exist as the key has no associated value for the provided element: v[3]:age
+Type ':help' or ':h' for help.
+Display stack trace? [yN]n
+----
+
+The solution was to filter away vertices that did not have the available key so that such steps would work properly
+or to write a more complex `by()` modulator to better handle the possibility of a missing key. With the latest changes
+however none of that is necessary unless desired:
+
+[source,text]
+----
+gremlin> g.V().groupCount().by('age')
+==>[null:2,32:1,35:1,27:1,29:1]
+----
+
In conclusion, this change in greater support of `null` may affect the behavior of existing traversals written in past
versions of TinkerPop as it is no longer possible to rely on `null` to expect a filtering action for traversers.
Please review existing Gremlin carefully to ensure that there are no unintended consequences of this change and that
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversal.java
similarity index 66%
rename from gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java
rename to gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversal.java
index 73f73a6..fa5d90d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversal.java
@@ -21,24 +21,24 @@ package org.apache.tinkerpop.gremlin.process.traversal.lambda;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.Element;
+import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedProperty;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyProperty;
import java.util.Map;
import java.util.Objects;
/**
- * More efficiently extracts a value from an {@link Element} or {@code Map} to avoid strategy application costs. Note
- * that as of 3.4.5 this class is now poorly named as it was originally designed to work with {@link Element} only.
- * In future 3.5.0 this class will likely be renamed but to ensure that we get this revised functionality for
- * {@code Map} without introducing a breaking change this name will remain the same.
+ * More efficiently extracts a value from an {@link Element} or {@code Map} to avoid strategy application costs.
*
* @author Marko A. Rodriguez (http://markorodriguez.com)
+ * @author Stephen Mallette (http://stephen.genoprime.com)
*/
-public final class ElementValueTraversal<V> extends AbstractLambdaTraversal<Element, V> {
+public final class ValueTraversal<T, V> extends AbstractLambdaTraversal<T, V> {
private final String propertyKey;
private V value;
- public ElementValueTraversal(final String propertyKey) {
+ public ValueTraversal(final String propertyKey) {
this.propertyKey = propertyKey;
}
@@ -53,15 +53,11 @@ public final class ElementValueTraversal<V> extends AbstractLambdaTraversal<Elem
}
@Override
- public void addStart(final Traverser.Admin<Element> start) {
+ public void addStart(final Traverser.Admin<T> start) {
if (null == this.bypassTraversal) {
- // playing a game of type erasure here.....technically we can process either Map or Element here in this
- // case after 3.4.5. and obviously users get weird errors along those lines here anyway. will fix up the
- // generics in 3.5.0 when we can take some breaking changes. seemed like this feature would make Gremlin
- // a lot nicer and given the small footprint of the change this seemed like a good hack to take.
- final Object o = start.get();
+ final T o = start.get();
if (o instanceof Element)
- this.value = ((Element) o).value(propertyKey);
+ this.value = (V)((Element) o).property(propertyKey).orElse(null);
else if (o instanceof Map)
this.value = (V) ((Map) o).get(propertyKey);
else
@@ -89,7 +85,7 @@ public final class ElementValueTraversal<V> extends AbstractLambdaTraversal<Elem
@Override
public boolean equals(final Object other) {
- return other instanceof ElementValueTraversal
- && Objects.equals(((ElementValueTraversal) other).propertyKey, this.propertyKey);
+ return other instanceof ValueTraversal
+ && Objects.equals(((ValueTraversal) other).propertyKey, this.propertyKey);
}
}
\ No newline at end of file
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ByModulating.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ByModulating.java
index 01841fa..b808bf6 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ByModulating.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/ByModulating.java
@@ -23,7 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Order;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.ColumnTraversal;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.FunctionTraverser;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
@@ -47,7 +47,7 @@ public interface ByModulating {
}
public default void modulateBy(final String string) throws UnsupportedOperationException {
- this.modulateBy(new ElementValueTraversal(string));
+ this.modulateBy(new ValueTraversal(string));
}
public default void modulateBy(final T token) throws UnsupportedOperationException {
@@ -74,7 +74,7 @@ public interface ByModulating {
}
public default void modulateBy(final String key, final Comparator comparator) {
- this.modulateBy(new ElementValueTraversal<>(key), comparator);
+ this.modulateBy(new ValueTraversal<>(key), comparator);
}
public default void modulateBy(final Comparator comparator) {
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java
index 8a5843a..f0b7e20 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java
@@ -20,7 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
import org.apache.tinkerpop.gremlin.structure.T;
@@ -49,7 +49,7 @@ public interface PathProcessor {
} else if (traversal instanceof TokenTraversal && ((TokenTraversal) traversal).getToken().equals(T.label)) {
if (max.compareTo(ElementRequirement.LABEL) < 0)
max = ElementRequirement.LABEL;
- } else if (traversal instanceof ElementValueTraversal) {
+ } else if (traversal instanceof ValueTraversal) {
if (max.compareTo(ElementRequirement.PROPERTIES) < 0)
max = ElementRequirement.PROPERTIES;
} else {
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 fb0bf3a..4bb06f0 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
@@ -25,7 +25,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.ColumnTraversal;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.FunctionTraverser;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
@@ -223,7 +223,7 @@ public final class GroupStep<S, K, V> extends ReducingBarrierStep<S, Map<K, V>>
///////////////////////
public static <S, E> Traversal.Admin<S, E> convertValueTraversal(final Traversal.Admin<S, E> valueTraversal) {
- if (valueTraversal instanceof ElementValueTraversal ||
+ if (valueTraversal instanceof ValueTraversal ||
valueTraversal instanceof TokenTraversal ||
valueTraversal instanceof IdentityTraversal ||
valueTraversal instanceof ColumnTraversal ||
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
index 60b92a8..5333d2b 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/SubgraphStrategy.java
@@ -24,7 +24,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.ClassFilterStep;
import org.apache.tinkerpop.gremlin.process.traversal.step.filter.FilterStep;
@@ -220,19 +220,19 @@ public final class SubgraphStrategy extends AbstractTraversalStrategy<TraversalS
}
} else {
Stream.concat(((TraversalParent) step).getGlobalChildren().stream(), ((TraversalParent) step).getLocalChildren().stream())
- .filter(t -> t instanceof ElementValueTraversal)
+ .filter(t -> t instanceof ValueTraversal)
.forEach(t -> {
final char propertyType = processesPropertyType(step.getPreviousStep());
if ('p' != propertyType) {
final Traversal.Admin<?, ?> temp = new DefaultTraversal<>();
- temp.addStep(new PropertiesStep<>(temp, PropertyType.PROPERTY, ((ElementValueTraversal) t).getPropertyKey()));
+ temp.addStep(new PropertiesStep<>(temp, PropertyType.PROPERTY, ((ValueTraversal) t).getPropertyKey()));
if ('v' == propertyType)
TraversalHelper.insertTraversal(0, nonCheckPropertyCriterion.clone(), temp);
else
temp.addStep(checkPropertyCriterion.clone());
temp.addStep(new PropertyValueStep<>(temp));
temp.setParent((TraversalParent) step);
- ((ElementValueTraversal) t).setBypassTraversal(temp);
+ ((ValueTraversal) t).setBypassTraversal(temp);
}
});
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
index 120b2ec..4e7eb92 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/TraversalHelper.java
@@ -23,7 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Scope;
import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
import org.apache.tinkerpop.gremlin.process.traversal.step.HasContainerHolder;
@@ -98,7 +98,7 @@ public final class TraversalHelper {
private static char isLocalStarGraph(final Traversal.Admin<?, ?> traversal, char state) {
if (state == 'u' &&
- (traversal instanceof ElementValueTraversal ||
+ (traversal instanceof ValueTraversal ||
(traversal instanceof TokenTraversal && !((TokenTraversal) traversal).getToken().equals(T.id))))
return 'x';
for (final Step step : traversal.getSteps()) {
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java
deleted file mode 100644
index 2752a3d..0000000
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ElementValueTraversalTest.java
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * 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.Vertex;
-import org.apache.tinkerpop.gremlin.structure.VertexProperty;
-import org.junit.Test;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-public class ElementValueTraversalTest {
-
- @Test
- public void shouldWorkOnVertex() {
- final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
- final Vertex v = mock(Vertex.class);
- when(v.value("age")).thenReturn(29);
- t.addStart(new B_O_Traverser(v, 1).asAdmin());
- assertEquals(29, t.next().intValue());
- }
-
- @Test
- public void shouldWorkOnEdge() {
- final ElementValueTraversal<Double> t = new ElementValueTraversal<>("weight");
- final Edge e = mock(Edge.class);
- when(e.value("weight")).thenReturn(1.0d);
- t.addStart(new B_O_Traverser(e, 1).asAdmin());
- assertEquals(1.0d, t.next(), 0.00001d);
- }
-
- @Test
- public void shouldWorkOnVertexProperty() {
- final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
- final VertexProperty vp = mock(VertexProperty.class);
- when(vp.value("age")).thenReturn(29);
- t.addStart(new B_O_Traverser(vp, 1).asAdmin());
- assertEquals(29, t.next().intValue());
- }
-
- @Test
- public void shouldWorkOnMap() {
- final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
- final Map<String,Integer> m = new HashMap<>();
- m.put("age", 29);
- t.addStart(new B_O_Traverser(m, 1).asAdmin());
- assertEquals(29, t.next().intValue());
- }
-
- @Test(expected = IllegalStateException.class)
- public void shouldThrowExceptionWhenTryingUnsupportedType() {
- final ElementValueTraversal<Integer> t = new ElementValueTraversal<>("age");
- t.addStart(new B_O_Traverser(29, 1).asAdmin());
- t.next();
- }
-}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversalTest.java
new file mode 100644
index 0000000..761d0b6
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/ValueTraversalTest.java
@@ -0,0 +1,118 @@
+/*
+ * 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.Vertex;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty;
+import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedProperty;
+import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class ValueTraversalTest {
+
+ @Test
+ public void shouldWorkOnVertex() {
+ final ValueTraversal<Vertex, Integer> t = new ValueTraversal<>("age");
+ final Vertex v = mock(Vertex.class);
+ when(v.property("age")).thenReturn(new DetachedVertexProperty<>(1, "age", 29, null));
+ t.addStart(new B_O_Traverser<>(v, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnVertexWithMissingKey() {
+ final ValueTraversal<Vertex, Integer> t = new ValueTraversal<>("age");
+ final Vertex v = mock(Vertex.class);
+ when(v.property("age")).thenReturn(VertexProperty.empty());
+ t.addStart(new B_O_Traverser<>(v, 1).asAdmin());
+ assertNull(t.next());
+ }
+
+ @Test
+ public void shouldWorkOnEdge() {
+ final ValueTraversal<Edge, Double> t = new ValueTraversal<>("weight");
+ final Edge e = mock(Edge.class);
+ when(e.property("weight")).thenReturn(new DetachedProperty<>("weight", 1.0d));
+ t.addStart(new B_O_Traverser<>(e, 1).asAdmin());
+ assertEquals(1.0d, t.next(), 0.00001d);
+ }
+
+ @Test
+ public void shouldWorkOnEdgeWithMissingKey() {
+ final ValueTraversal<Edge, Double> t = new ValueTraversal<>("weight");
+ final Edge e = mock(Edge.class);
+ when(e.property("weight")).thenReturn(Property.empty());
+ t.addStart(new B_O_Traverser<>(e, 1).asAdmin());
+ assertNull(t.next());
+ }
+
+ @Test
+ public void shouldWorkOnVertexProperty() {
+ final ValueTraversal<VertexProperty, Integer> t = new ValueTraversal<>("age");
+ final VertexProperty vp = mock(VertexProperty.class);
+ when(vp.property("age")).thenReturn(new DetachedProperty<>("age", 29));
+ t.addStart(new B_O_Traverser<>(vp, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnVertexPropertyWithMissingKey() {
+ final ValueTraversal<VertexProperty, Integer> t = new ValueTraversal<>("age");
+ final VertexProperty vp = mock(VertexProperty.class);
+ when(vp.property("age")).thenReturn(Property.empty());
+ t.addStart(new B_O_Traverser<>(vp, 1).asAdmin());
+ assertNull(t.next());
+ }
+
+ @Test
+ public void shouldWorkOnMap() {
+ final ValueTraversal<Map<String,Integer>, Integer> t = new ValueTraversal<>("age");
+ final Map<String,Integer> m = new HashMap<>();
+ m.put("age", 29);
+ t.addStart(new B_O_Traverser<>(m, 1).asAdmin());
+ assertEquals(29, t.next().intValue());
+ }
+
+ @Test
+ public void shouldWorkOnMapWithMissingKey() {
+ final ValueTraversal<Map<String,Integer>, Integer> t = new ValueTraversal<>("not-age");
+ final Map<String,Integer> m = new HashMap<>();
+ m.put("age", 29);
+ t.addStart(new B_O_Traverser<>(m, 1).asAdmin());
+ assertNull(t.next());
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void shouldThrowExceptionWhenTryingUnsupportedType() {
+ final ValueTraversal<Integer, Integer> t = new ValueTraversal<>("age");
+ t.addStart(new B_O_Traverser<>(29, 1).asAdmin());
+ t.next();
+ }
+}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java
index 9616d5a..02635ab 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathProcessorStrategyTest.java
@@ -27,7 +27,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
-import org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
@@ -82,7 +82,7 @@ public class PathProcessorStrategyTest {
{__.select("a"), __.select("a"), Collections.emptyList()},
{__.select("a").by(), __.select("a").by(), Collections.emptyList()},
{__.select("a").by(__.outE().count()), __.select("a").map(__.outE().count()), Collections.emptyList()},
- {__.select("a").by("name"), __.select("a").map(new ElementValueTraversal<>("name")), Collections.emptyList()},
+ {__.select("a").by("name"), __.select("a").map(new ValueTraversal<>("name")), Collections.emptyList()},
{__.select("a").out(), __.select("a").out(), Collections.emptyList()},
{__.select(Pop.all, "a").by(__.values("name")), __.select(Pop.all, "a").by(__.values("name")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
{__.select(Pop.last, "a").by(__.values("name")), __.select(Pop.last, "a").map(__.values("name")), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
@@ -91,11 +91,11 @@ public class PathProcessorStrategyTest {
{__.select("a", "b"), __.select("a", "b"), Collections.emptyList()},
{__.select("a", "b").by(), __.select("a", "b").by(), Collections.emptyList()},
{__.select("a", "b", "c").by(), __.select("a", "b", "c").by(), Collections.emptyList()},
- {__.select("a", "b").by().by("age"), __.select("b").map(new ElementValueTraversal<>("age")).as("b").select("a").map(new IdentityTraversal<>()).as("a").select(Pop.last, "a", "b"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
- {__.select("a", "b").by("name").by("age"), __.select("b").map(new ElementValueTraversal<>("age")).as("b").select("a").map(new ElementValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), Collections.emptyList()},
- {__.select("a", "b", "c").by("name").by(__.outE().count()), __.select("c").map(new ElementValueTraversal<>("name")).as("c").select("b").map(__.outE().count()).as("b").select("a").map(new ElementValueTraversal<>("name")).as("a").select(Pop.last, "a", "b", "c"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
- {__.select(Pop.first, "a", "b").by("name").by("age"), __.select(Pop.first, "b").map(new ElementValueTraversal<>("age")).as("b").select(Pop.first, "a").map(new ElementValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), Collections.emptyList()},
- {__.select(Pop.last, "a", "b").by("name").by("age"), __.select(Pop.last, "b").map(new ElementValueTraversal<>("age")).as("b").select(Pop.last, "a").map(new ElementValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
+ {__.select("a", "b").by().by("age"), __.select("b").map(new ValueTraversal<>("age")).as("b").select("a").map(new IdentityTraversal<>()).as("a").select(Pop.last, "a", "b"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
+ {__.select("a", "b").by("name").by("age"), __.select("b").map(new ValueTraversal<>("age")).as("b").select("a").map(new ValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), Collections.emptyList()},
+ {__.select("a", "b", "c").by("name").by(__.outE().count()), __.select("c").map(new ValueTraversal<>("name")).as("c").select("b").map(__.outE().count()).as("b").select("a").map(new ValueTraversal<>("name")).as("a").select(Pop.last, "a", "b", "c"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
+ {__.select(Pop.first, "a", "b").by("name").by("age"), __.select(Pop.first, "b").map(new ValueTraversal<>("age")).as("b").select(Pop.first, "a").map(new ValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), Collections.emptyList()},
+ {__.select(Pop.last, "a", "b").by("name").by("age"), __.select(Pop.last, "b").map(new ValueTraversal<>("age")).as("b").select(Pop.last, "a").map(new ValueTraversal<>("name")).as("a").select(Pop.last, "a", "b"), TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()},
{__.select(Pop.all, "a", "b").by("name").by("age"), __.select(Pop.all, "a", "b").by("name").by("age"), Collections.emptyList()},
{__.select(Pop.mixed, "a", "b").by("name").by("age"), __.select(Pop.mixed, "a", "b").by("name").by("age"), Collections.emptyList()},
// where(as("a")...)
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs
index 641b9a4..0a3d5ed 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs
@@ -41,6 +41,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
new Dictionary<string, IgnoreReason>
{
// Add here the name of scenarios to ignore and the reason, e.g.:
+ { "g_V_group_byXageX", IgnoreReason.NullKeysInMapNotSupported }
// { "g_V_peerPressure_withXpropertyName_clusterX_withXedges_outEXknowsXX_pageRankX1X_byXrankX_withXedges_outEXknowsX_withXtimes_2X_group_byXclusterX_byXrank_sumX_limitX100X", IgnoreReason.NoReason },
};
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/IgnoreException.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/IgnoreException.cs
index 726e684..0e25440 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/IgnoreException.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/IgnoreException.cs
@@ -50,12 +50,11 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
public enum IgnoreReason
{
+ NoReason,
+
/// <summary>
- /// Deserialization of g:T on GraphSON3 is not supported.
+ /// C# does not allow a `null` value to be used as a key.
/// </summary>
- TraversalTDeserializationNotSupported,
- ReceivedDataDoesntMatchExpected,
- NoReason,
- EmbeddedListAssertion
+ NullKeysInMapNotSupported
}
}
\ No newline at end of file
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
index 8c80982..4a15178 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
@@ -104,14 +104,14 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
private static IDictionary<string, Vertex> GetVertices(GraphTraversalSource g)
{
- try
+ // Property name might not exist and C# doesn't support "null" keys in Dictionary
+ if (g.V().Count().Next() == g.V().Has("name").Count().Next())
{
return g.V().Group<string, object>().By("name").By(__.Tail<Vertex>()).Next()
.ToDictionary(kv => kv.Key, kv => (Vertex) kv.Value);
}
- catch (ResponseException)
+ else
{
- // Property name might not exist
return EmptyVertices;
}
}
diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js
index b536fe6..3b3ddbf 100644
--- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js
+++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js
@@ -58,6 +58,7 @@ const parsers = [
].map(x => [ new RegExp('^' + x[0] + '$'), x[1] ]);
const ignoreReason = {
+ nullKeysInMapNotSupportedWell: "Javascript does not nicely support 'null' as a key in Map instances",
setNotSupported: "There is no Set support in gremlin-javascript",
needsFurtherInvestigation: '',
};
@@ -66,6 +67,7 @@ const ignoredScenarios = {
// An associative array containing the scenario name as key, for example:
'g_withSideEffectXa_setX_V_both_name_storeXaX_capXaX': new IgnoreError(ignoreReason.setNotSupported),
'g_withSideEffectXa_setX_V_both_name_aggregateXlocal_aX_capXaX': new IgnoreError(ignoreReason.setNotSupported),
+ 'g_V_group_byXageX': new IgnoreError(ignoreReason.nullKeysInMapNotSupportedWell),
};
defineSupportCode(function(methods) {
diff --git a/gremlin-test/features/sideEffect/Group.feature b/gremlin-test/features/sideEffect/Group.feature
index e69b447..44705cf 100644
--- a/gremlin-test/features/sideEffect/Group.feature
+++ b/gremlin-test/features/sideEffect/Group.feature
@@ -28,6 +28,17 @@ Feature: Step - group()
| result |
| m[{"ripple":"l[v[ripple]]", "peter":"l[v[peter]]", "vadas":"l[v[vadas]]", "josh": "l[v[josh]]", "lop":"l[v[lop]]", "marko":"l[v[marko]]"}] |
+ Scenario: g_V_group_byXageX
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().group().by("age")
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | m[{"null":"l[v[lop],v[ripple]]", "d[35].i":"l[v[peter]]", "d[27].i":"l[v[vadas]]", "d[32].i": "l[v[josh]]", "d[29].i":"l[v[marko]]"}] |
+
Scenario: g_V_group_byXnameX_by
Given the modern graph
And the traversal of
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
index 5b0cf32..43b694f 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/GroupTest.java
@@ -59,6 +59,8 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
public abstract Traversal<Vertex, Map<String, Collection<Vertex>>> get_g_V_group_byXnameX();
+ public abstract Traversal<Vertex, Map<Integer, Collection<Vertex>>> get_g_V_group_byXageX();
+
public abstract Traversal<Vertex, Map<String, Collection<Vertex>>> get_g_V_group_byXnameX_by();
public abstract Traversal<Vertex, Map<String, Collection<Vertex>>> get_g_V_groupXaX_byXnameX_capXaX();
@@ -110,6 +112,24 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
checkSideEffects(traversal.asAdmin().getSideEffects());
}
+ @Test
+ @LoadGraphWith(MODERN)
+ public void g_V_group_byXageX() {
+ final Traversal<Vertex, Map<Integer, Collection<Vertex>>> traversal = get_g_V_group_byXageX();
+ printTraversalForm(traversal);
+
+ final Map<Integer, Collection<Vertex>> map = traversal.next();
+ assertEquals(5, map.size());
+ map.forEach((key, values) -> {
+ if (null == key)
+ assertEquals(2, values.size());
+ else
+ assertEquals(1, values.size());
+ });
+ assertFalse(traversal.hasNext());
+
+ checkSideEffects(traversal.asAdmin().getSideEffects());
+ }
@Test
@LoadGraphWith(MODERN)
@@ -129,7 +149,7 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
checkSideEffects(traversal.asAdmin().getSideEffects(), "a", HashMap.class);
}
- private void assertCommonA(Traversal<Vertex, Map<String, Collection<Vertex>>> traversal) {
+ private void assertCommonA(final Traversal<Vertex, Map<String, Collection<Vertex>>> traversal) {
final Map<String, Collection<Vertex>> map = traversal.next();
assertEquals(6, map.size());
map.forEach((key, values) -> {
@@ -507,6 +527,11 @@ public abstract class GroupTest extends AbstractGremlinProcessTest {
}
@Override
+ public Traversal<Vertex, Map<Integer, Collection<Vertex>>> get_g_V_group_byXageX() {
+ return g.V().<Integer, Collection<Vertex>>group().by("age");
+ }
+
+ @Override
public Traversal<Vertex, Map<String, Collection<Vertex>>> get_g_V_group_byXnameX_by() {
return g.V().<String, Collection<Vertex>>group().by("name").by();
}