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 2019/12/03 17:59:15 UTC

[tinkerpop] 23/23: TINKERPOP-2235 Fixed up label overrides for property(label, Object)

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 <sp...@gmail.com>
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();