This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch TINKERPOP-2235 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 4e688ca834e676819a6c685d9a5a510ff6ea2059 Author: stephen <[email protected]> AuthorDate: Mon Nov 18 13:13:26 2019 -0500 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. --- .../gremlin/process/traversal/step/Scoping.java | 6 +++++- .../process/traversal/step/map/SelectOneStep.java | 7 ++++--- .../process/traversal/step/util/AbstractStep.java | 14 +++++++------- .../process/traversal/step/util/ImmutablePath.java | 8 +++++--- .../Gremlin.Net/Process/Traversal/Instruction.cs | 11 +++++++++++ .../Gherkin/CommonSteps.cs | 1 + .../Gherkin/TraversalEvaluation/TraversalParser.cs | 6 ++++-- .../Structure/IO/GraphSON/GraphSONReaderTests.cs | 5 +++-- .../test/cucumber/feature-steps.js | 3 +++ gremlin-test/features/sideEffect/Inject.feature | 15 ++++++++++++++- .../traversal/step/sideEffect/InjectTest.java | 22 ++++++++++++++++++++++ 11 files changed, 79 insertions(+), 19 deletions(-) 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 interface Scoping { } 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 @@ public interface Scoping { 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.PathProcessor; 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 @@ import java.util.Set; */ 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 @@ public final class SelectOneStep<S, E> extends MapStep<S, E> implements Traversa @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 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { 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 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { @Override public void reset() { this.starts.clear(); - this.nextEnd = null; + this.nextEnd = EmptyTraverser.instance(); } @Override @@ -117,11 +117,11 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { @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 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { @Override public boolean hasNext() { - if (null != this.nextEnd) + if (EmptyTraverser.instance() != this.nextEnd) return true; else { try { @@ -145,7 +145,7 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { 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 @@ public abstract class AbstractStep<S, E> implements Step<S, E> { 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.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Objects; import java.util.Set; /** @@ -35,7 +36,8 @@ import java.util.Set; */ 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 @@ public class ImmutablePath implements Path, Serializable, Cloneable { } private final boolean isTail() { - return null == this.currentObject; + return END == this.currentObject; } @Override @@ -230,7 +232,7 @@ public class ImmutablePath implements Path, Serializable, Cloneable { 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 @@ namespace Gremlin.Net.Process.Traversal /// 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 @@ namespace Gremlin.Net.IntegrationTest.Gherkin 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 @@ namespace Gremlin.Net.IntegrationTest.Gherkin.TraversalEvaluation 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 @@ namespace Gremlin.Net.IntegrationTest.Gherkin.TraversalEvaluation { 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 @@ namespace Gremlin.Net.UnitTest.Structure.IO.GraphSON [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 @@ namespace Gremlin.Net.UnitTest.Structure.IO.GraphSON 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 toMap(stringMap) { } 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 @@ Feature: Step - inject() | 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.AbstractGremlinProcessTest; 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 class InjectTest extends AbstractGremlinProcessTest { 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 @@ public abstract class InjectTest extends AbstractGremlinProcessTest { } @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 abstract class InjectTest extends AbstractGremlinProcessTest { 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"))); + } } }
