TINKERPOP-2312 Empty keys to group() should group to null
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 @@
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 @@
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 @@
==>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 @@
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 @@
}
@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 @@
@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.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 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 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 @@
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 @@
} 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.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 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.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 @@
}
} 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.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 @@
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.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 @@
{__.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 @@
{__.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 @@
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 @@
public enum IgnoreReason
{
- /// <summary>
- /// Deserialization of g:T on GraphSON3 is not supported.
- /// </summary>
- TraversalTDeserializationNotSupported,
- ReceivedDataDoesntMatchExpected,
NoReason,
- EmbeddedListAssertion
+
+ /// <summary>
+ /// C# does not allow a `null` value to be used as a key.
+ /// </summary>
+ 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 @@
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 @@
].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 @@
// 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 @@
| 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 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 @@
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 @@
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 @@
}
@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();
}
