TINKERPOP-2235 Be smarter about null in SelectOneStep
Can't rely on null to determine if the traverser is "empty" because null could be the value selected from the Map, Path, etc.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java
index fae7431..dee401c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java
@@ -116,6 +116,10 @@
}
public default <S> S getNullableScopeValue(final Pop pop, final String key, final Traverser.Admin<?> traverser) {
+ return getScopeValue(pop, key, traverser, null);
+ }
+
+ public default <S> S getScopeValue(final Pop pop, final String key, final Traverser.Admin<?> traverser, final S valueIfNotFound) {
final Object object = traverser.get();
if (object instanceof Map && ((Map<String, S>) object).containsKey(key))
return ((Map<String, S>) object).get(key);
@@ -127,7 +131,7 @@
if (path.hasLabel(key))
return path.get(pop, key);
///
- return null;
+ return valueIfNotFound;
}
/**
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java
index 98ee83f..a87f9ef 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java
@@ -26,7 +26,6 @@
import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping;
import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
-import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.EmptyTraverser;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
@@ -40,6 +39,7 @@
*/
public final class SelectOneStep<S, E> extends MapStep<S, E> implements TraversalParent, Scoping, PathProcessor, ByModulating {
+ private static final Object NOTHING_SELECTED = new Object();
private final Pop pop;
private final String selectKey;
private Traversal.Admin<S, E> selectTraversal = null;
@@ -53,13 +53,14 @@
@Override
protected E map(final Traverser.Admin<S> traverser) {
- final E end = this.getNullableScopeValue(this.pop, this.selectKey, traverser);
+ final E end = this.getScopeValue(this.pop, this.selectKey, traverser, (E) NOTHING_SELECTED);
+ if (NOTHING_SELECTED == end) return end;
return null != end ? TraversalUtil.applyNullable((S) end, this.selectTraversal) : null;
}
@Override
protected boolean isEmptyTraverser(final E obj) {
- return null == obj;
+ return NOTHING_SELECTED == obj;
}
@Override
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
index fe49e56..b3bfdf5 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
@@ -42,7 +42,7 @@
protected String id = Traverser.Admin.HALT;
protected Traversal.Admin traversal;
protected ExpandableStepIterator<S> starts;
- protected Traverser.Admin<E> nextEnd = null;
+ protected Traverser.Admin<E> nextEnd = EmptyTraverser.instance();
protected boolean traverserStepIdAndLabelsSetByChild = false;
protected Step<?, S> previousStep = EmptyStep.instance();
@@ -82,7 +82,7 @@
@Override
public void reset() {
this.starts.clear();
- this.nextEnd = null;
+ this.nextEnd = EmptyTraverser.instance();
}
@Override
@@ -117,11 +117,11 @@
@Override
public Traverser.Admin<E> next() {
- if (null != this.nextEnd) {
+ if (EmptyTraverser.instance() != this.nextEnd) {
try {
return this.prepareTraversalForNextStep(this.nextEnd);
} finally {
- this.nextEnd = null;
+ this.nextEnd = EmptyTraverser.instance();
}
} else {
while (true) {
@@ -135,7 +135,7 @@
@Override
public boolean hasNext() {
- if (null != this.nextEnd)
+ if (EmptyTraverser.instance() != this.nextEnd)
return true;
else {
try {
@@ -145,7 +145,7 @@
if (this.nextEnd.bulk() > 0)
return true;
else
- this.nextEnd = null;
+ this.nextEnd = EmptyTraverser.instance();
}
} catch (final NoSuchElementException e) {
return false;
@@ -178,7 +178,7 @@
clone.starts = new ExpandableStepIterator<>(clone);
clone.previousStep = EmptyStep.instance();
clone.nextStep = EmptyStep.instance();
- clone.nextEnd = null;
+ clone.nextEnd = EmptyTraverser.instance();
clone.traversal = EmptyTraversal.instance();
clone.labels = new LinkedHashSet<>(this.labels);
clone.reset();
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ImmutablePath.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ImmutablePath.java
index 623b810..0ff981d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ImmutablePath.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ImmutablePath.java
@@ -28,6 +28,7 @@
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
/**
@@ -35,7 +36,8 @@
*/
public class ImmutablePath implements Path, Serializable, Cloneable {
- private static final ImmutablePath TAIL_PATH = new ImmutablePath(null, null, null);
+ private static final Object END = new Object();
+ private static final ImmutablePath TAIL_PATH = new ImmutablePath(null, END, null);
private ImmutablePath previousPath;
private Object currentObject;
@@ -58,7 +60,7 @@
}
private final boolean isTail() {
- return null == this.currentObject;
+ return END == this.currentObject;
}
@Override
@@ -230,7 +232,7 @@
while (true) {
if (currentPath.isTail())
break;
- hashCodes[index] = currentPath.currentObject.hashCode();
+ hashCodes[index] = Objects.hashCode(currentPath.currentObject);
currentPath = currentPath.previousPath;
index--;
}
diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Instruction.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Instruction.cs
index 65847c5..55df44d 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Instruction.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Instruction.cs
@@ -21,6 +21,8 @@
#endregion
+using System;
+
namespace Gremlin.Net.Process.Traversal
{
/// <summary>
@@ -48,5 +50,14 @@
/// Gets the arguments.
/// </summary>
public dynamic[] Arguments { get; }
+
+ /// <summary>
+ /// String representation of the <see cref="Instruction"/>.
+ /// </summary>
+ /// <returns></returns>
+ public override string ToString()
+ {
+ return OperatorName + " [" + String.Join(",", Arguments) + "]";
+ }
}
}
\ No newline at end of file
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs
index 504f7a9..b9135ed 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs
@@ -181,6 +181,7 @@
var rows = table.Rows.ToArray();
Assert.Equal("result", rows[0].Cells.First().Value);
var expected = rows.Skip(1).Select(x => ParseValue(x.Cells.First().Value, _graphName));
+
if (ordered)
{
Assert.Equal(expected, _result);
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs
index fb84fdd..e81e00e 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs
@@ -36,7 +36,9 @@
private static readonly IDictionary<string, Func<GraphTraversalSource, ITraversal>> FixedTranslations =
new Dictionary<string, Func<GraphTraversalSource, ITraversal>>
{
- { "g.V().fold().count(Scope.local)", g => g.V().Fold().Count(Scope.Local)}
+ { "g.V().fold().count(Scope.local)", g => g.V().Fold().Count(Scope.Local)},
+ { "g.inject(10,20,null,20,10,10).groupCount(\"x\").dedup().as(\"y\").project(\"a\",\"b\").by().by(__.select(\"x\").select(__.select(\"y\")))",
+ g => g.Inject<object>(10,20,null,20,10,10).GroupCount("x").Dedup().As("y").Project<object>("a","b").By().By(__.Select<int>("x").Select<object>(__.Select<int>("y")))}
};
private static readonly Regex RegexNumeric =
@@ -169,7 +171,7 @@
{
if (IsNumeric(methodParameter.ParameterType) && IsNumeric(tokenParameterType))
{
- // Acount for implicit conversion of numeric values as an exact match
+ // Account for implicit conversion of numeric values as an exact match
exactMatches++;
}
else if (!methodParameter.ParameterType.GetTypeInfo().IsAssignableFrom(tokenParameterType))
diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs
index 15bd5f9..11b92fe 100644
--- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs
@@ -120,7 +120,7 @@
[Theory, MemberData(nameof(Versions))]
public void ShouldDeserializeDictionary(int version)
{
- var serializedDict = "{\"age\":[{\"@type\":\"g:Int32\",\"@value\":29}],\"name\":[\"marko\"]}";
+ var serializedDict = "{\"age\":[{\"@type\":\"g:Int32\",\"@value\":29}],\"name\":[\"marko\"],\"gender\": null}";
var reader = CreateStandardGraphSONReader(version);
var jObject = JObject.Parse(serializedDict);
@@ -129,7 +129,8 @@
var expectedDict = new Dictionary<string, dynamic>
{
{"age", new List<object> {29}},
- {"name", new List<object> {"marko"}}
+ {"name", new List<object> {"marko"}},
+ {"gender", null}
};
Assert.Equal(expectedDict, deserializedDict);
}
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 d85d678..b536fe6 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
@@ -299,6 +299,9 @@
}
function parseMapValue(value) {
+ if (value === null)
+ return null;
+
if (typeof value === 'string') {
return parseValue.call(this, value);
}
diff --git a/gremlin-test/features/sideEffect/Inject.feature b/gremlin-test/features/sideEffect/Inject.feature
index db5a26c..7e4983d 100644
--- a/gremlin-test/features/sideEffect/Inject.feature
+++ b/gremlin-test/features/sideEffect/Inject.feature
@@ -79,4 +79,17 @@
| null |
| d[1].i |
| d[3].i |
- | null |
\ No newline at end of file
+ | null |
+
+ Scenario: g_injectX10_20_null_20_10_10X_groupCountXxX_dedup_asXyX_projectXa_bX_by_byXselectXxX_selectXselectXyXXX
+ Given the empty graph
+ And the traversal of
+ """
+ g.inject(10,20,null,20,10,10).groupCount("x").dedup().as("y").project("a","b").by().by(__.select("x").select(__.select("y")))
+ """
+ When iterated to list
+ Then the result should be unordered
+ | result |
+ | m[{"a":"d[10].i", "b":"d[3].l"}] |
+ | m[{"a":"d[20].i", "b":"d[2].l"}] |
+ | m[{"a":null, "b":"d[1].l"}] |
\ No newline at end of file
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/InjectTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/InjectTest.java
index 0b3b584..6ce50c9 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/InjectTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/InjectTest.java
@@ -23,6 +23,7 @@
import org.apache.tinkerpop.gremlin.process.GremlinProcessRunner;
import org.apache.tinkerpop.gremlin.process.traversal.Path;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.MapHelper;
import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.junit.Test;
@@ -50,6 +51,8 @@
public abstract Traversal<Integer, Integer> get_g_injectXnull_1_3_nullX();
+ public abstract Traversal<Integer, Map<String, Object>> get_g_injectX10_20_null_20_10_10X_groupCountXxX_dedup_asXyX_projectXa_bX_by_byXselectXxX_selectXselectXyXXX();
+
@Test
@LoadGraphWith(MODERN)
public void g_VX1X_out_injectXv2X_name() {
@@ -94,6 +97,16 @@
}
@Test
+ public void g_injectX10_20_null_20_10_10X_groupCountXxX_dedup_asXyX_projectXa_bX_by_byXselectXxX_selectXselectXyXXX() {
+ final Traversal<Integer, Map<String, Object>> traversal = get_g_injectX10_20_null_20_10_10X_groupCountXxX_dedup_asXyX_projectXa_bX_by_byXselectXxX_selectXselectXyXXX();
+ printTraversalForm(traversal);
+ checkResults(makeMapList(2,
+ "a", 10, "b", 3L,
+ "a", 20, "b", 2L,
+ "a", null, "b", 1L), traversal);
+ }
+
+ @Test
@LoadGraphWith(MODERN)
public void g_VX1X_injectXg_VX4XX_out_name() {
final Traversal<Vertex, String> traversal = get_g_VX1X_injectXg_VX4XX_out_name(convertToVertexId("marko"), convertToVertexId("josh"));
@@ -122,5 +135,14 @@
public Traversal<Integer, Integer> get_g_injectXnull_1_3_nullX() {
return g.inject(null, 1, 3, null);
}
+
+ @Override
+ public Traversal<Integer, Map<String, Object>> get_g_injectX10_20_null_20_10_10X_groupCountXxX_dedup_asXyX_projectXa_bX_by_byXselectXxX_selectXselectXyXXX() {
+ return g.inject(10,20,null,20,10,10).groupCount("x").
+ dedup().as("y").
+ project("a","b").
+ by().
+ by(__.select("x").select(__.select("y")));
+ }
}
}
