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 db3933f229350c83b90a2c83bb7c35b6be35ca0a Author: stephen <[email protected]> AuthorDate: Tue Nov 26 15:47:56 2019 -0500 TINKERPOP-2235 Fixed up label overrides for property(label,Object) --- CHANGELOG.asciidoc | 1 + .../process/traversal/step/map/AddVertexStartStep.java | 15 ++++++++++++++- .../process/traversal/step/map/AddVertexStep.java | 15 ++++++++++++++- .../process/traversal/step/util/Parameters.java | 11 +++++++++++ .../process/traversal/step/util/ParametersTest.java | 16 ++++++++++++++++ gremlin-test/features/map/AddVertex.feature | 10 ++++++++++ .../process/traversal/step/map/AddVertexTest.java | 18 ++++++++++++++++++ .../gremlin/tinkergraph/structure/TinkerVertex.java | 2 +- 8 files changed, 85 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index c7a2b36..234e1b5 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>. * Allowed the possibility for the propagation of `null` as a `Traverser` in Gremlin. * Ensured better consistency of the use of `null` as arguments to mutation steps. +* Allowed `property(T.label,Object)` to be used if no value was supplied to `addV(String)`. * Added a `Graph.Feature` for `supportsNullPropertyValues`. * Refactored `MapStep` to move its logic to `ScalarMapStep` so that the old behavior could be preserved while allow other implementations to have more flexibility. * Modified TinkerGraph to support `null` property values and can be configured to disable that feature. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java index 6273fa2..eb54e6a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java @@ -78,7 +78,20 @@ public class AddVertexStartStep extends AbstractStep<Vertex, Vertex> @Override public void configure(final Object... keyValues) { - this.parameters.set(this, keyValues); + if (keyValues[0] == T.label && this.parameters.contains(T.label)) { + if (this.parameters.contains(T.label, Vertex.DEFAULT_LABEL)) { + this.parameters.remove(T.label); + this.parameters.set(this, keyValues); + } else { + throw new IllegalArgumentException(String.format("Vertex T.label has already been set to [%s] and cannot be overridden with [%s]", + this.parameters.getRaw().get(T.label).get(0), keyValues[1])); + } + } else if (keyValues[0] == T.id && this.parameters.contains(T.id)) { + throw new IllegalArgumentException(String.format("Vertex T.id has already been set to [%s] and cannot be overridden with [%s]", + this.parameters.getRaw().get(T.id).get(0), keyValues[1])); + } else { + this.parameters.set(this, keyValues); + } } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java index b963db2..5b22d4e 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java @@ -73,7 +73,20 @@ public class AddVertexStep<S> extends ScalarMapStep<S, Vertex> @Override public void configure(final Object... keyValues) { - this.parameters.set(this, keyValues); + if (keyValues[0] == T.label && this.parameters.contains(T.label)) { + if (this.parameters.contains(T.label, Vertex.DEFAULT_LABEL)) { + this.parameters.remove(T.label); + this.parameters.set(this, keyValues); + } else { + throw new IllegalArgumentException(String.format("Vertex T.label has already been set to [%s] and cannot be overridden with [%s]", + this.parameters.getRaw().get(T.label).get(0), keyValues[1])); + } + } else if (keyValues[0] == T.id && this.parameters.contains(T.id)) { + throw new IllegalArgumentException(String.format("Vertex T.id has already been set to [%s] and cannot be overridden with [%s]", + this.parameters.getRaw().get(T.id).get(0), keyValues[1])); + } else { + this.parameters.set(this, keyValues); + } } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java index 22bb54f..f16fd70 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java @@ -74,6 +74,17 @@ public final class Parameters implements Cloneable, Serializable { } /** + * Checks for existence of a key and value in a parameter set. + * + * @param key the key to check + * @param value the value to check + * @return {@code true} if the key and value are present and {@code false} otherwise + */ + public boolean contains(final Object key, final Object value) { + return this.contains(key) && this.parameters.get(key).contains(value); + } + + /** * Renames a key in the parameter set. * * @param oldKey the key to rename diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java index c832cb7..27d8e44 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/ParametersTest.java @@ -221,6 +221,14 @@ public class ParametersTest { } @Test + public void shouldContainKeyValue() { + final Parameters parameters = new Parameters(); + parameters.set(null, "a", "axe", "b", "bat", "c", "cat"); + + assertThat(parameters.contains("b", "bat"), is(true)); + } + + @Test public void shouldNotContainKey() { final Parameters parameters = new Parameters(); parameters.set(null, "a", "axe", "b", "bat", "c", "cat"); @@ -229,6 +237,14 @@ public class ParametersTest { } @Test + public void shouldNotContainKeyAndValue() { + final Parameters parameters = new Parameters(); + parameters.set(null, "a", "axe", "b", "bat", "c", "cat"); + + assertThat(parameters.contains("b", "mat"), is(false)); + } + + @Test public void shouldGetSetMultiple() { final Parameters parameters = new Parameters(); parameters.set(null, "a", "axe", "a", "ant", "b", "bat", "b", "ball", "c", "cat"); diff --git a/gremlin-test/features/map/AddVertex.feature b/gremlin-test/features/map/AddVertex.feature index e8e1248..943b303 100644 --- a/gremlin-test/features/map/AddVertex.feature +++ b/gremlin-test/features/map/AddVertex.feature @@ -421,3 +421,13 @@ Feature: Step - addV() When iterated to list Then the result should have a count of 1 And the graph should return 1 for count of "g.V().hasLabel(\"vertex\")" + + Scenario: g_addV_propertyXlabel_personX + Given the empty graph + And the traversal of + """ + g.addV().property(T.label, "person") + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 1 for count of "g.V().hasLabel(\"person\")" diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java index fec146d..ad0e1dd 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexTest.java @@ -63,6 +63,8 @@ public abstract class AddVertexTest extends AbstractGremlinTest { public abstract Traversal<Vertex, Vertex> get_g_addVXnullX_propertyXid_nullX(); + public abstract Traversal<Vertex, Vertex> get_g_addV_propertyXlabel_personX(); + public abstract Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenmX(); public abstract Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenm_since_2010X(); @@ -132,6 +134,17 @@ public abstract class AddVertexTest extends AbstractGremlinTest { } @Test + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY) + public void g_addV_propertyXlabel_personX() { + final Traversal<Vertex, Vertex> traversal = get_g_addV_propertyXlabel_personX(); + printTraversalForm(traversal); + + final Vertex vertex = traversal.next(); + assertEquals("person", vertex.label()); + } + + @Test @LoadGraphWith(MODERN) @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES) @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_PROPERTY) @@ -403,5 +416,10 @@ public abstract class AddVertexTest extends AbstractGremlinTest { // testing Traversal but should work the same for String return g.addV((Traversal.Admin<?, String>) null).property(T.id, null); } + + @Override + public Traversal<Vertex, Vertex> get_g_addV_propertyXlabel_personX() { + return g.addV().property(T.label, "person"); + } } } \ No newline at end of file diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertex.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertex.java index 1f055fb..1765054 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertex.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertex.java @@ -87,7 +87,6 @@ public final class TinkerVertex extends TinkerElement implements Vertex { if (this.removed) throw elementAlreadyRemoved(Vertex.class, id); ElementHelper.legalPropertyKeyValueArray(keyValues); ElementHelper.validateProperty(key, value); - final Optional<Object> optionalId = ElementHelper.getIdValue(keyValues); // if we don't allow null property values and the value is null then the key can be removed but only if the // cardinality is single. if it is list/set then we can just ignore the null. @@ -98,6 +97,7 @@ public final class TinkerVertex extends TinkerElement implements Vertex { return VertexProperty.empty(); } + final Optional<Object> optionalId = ElementHelper.getIdValue(keyValues); final Optional<VertexProperty<V>> optionalVertexProperty = ElementHelper.stageVertexProperty(this, cardinality, key, value, keyValues); if (optionalVertexProperty.isPresent()) return optionalVertexProperty.get();
