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/05/31 14:48:45 UTC
[tinkerpop] 01/01: TINKERPOP-2099 Consistent behavior of property()
in relation to null values
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2099
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit e9213d83be0c63396cd52f18f24281e2f0bd9e3a
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Fri May 31 10:47:59 2019 -0400
TINKERPOP-2099 Consistent behavior of property() in relation to null values
---
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.5.x.asciidoc | 34 +++++++++++++++++++
.../traversal/step/map/AddVertexStartStep.java | 2 +-
.../process/traversal/step/map/AddVertexStep.java | 2 +-
.../process/traversal/step/util/Parameters.java | 38 ++++++++++------------
.../traversal/step/util/ParametersTest.java | 11 +++++++
6 files changed, 66 insertions(+), 22 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index fa9fdfb..7de3385 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -25,6 +25,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
This release also includes changes from <<release-3-4-3, 3.4.3>>.
+* Modified `null` handling in mutations to be consistent for a new `Vertex` as well as update to an existing one.
* Removed previously deprecated `TraversalSource.withRemote()`.
== TinkerPop 3.4.0 (Avant-Gremlin Construction #3 for Theremin and Flowers)
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index 07a4c07..b1900a3 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -29,6 +29,40 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.5.0/CHANGELOG.asc
=== Upgrading for Users
+==== addV() and null
+
+There was a bit of inconsistency in the handling of `null` in calls to `property()` depending on the type of mutation
+being executed demonstrated as follows in earlier versions:
+
+[source,text]
+----
+gremlin> g.V(1).property("x", 1).property("y", null).property("z", 2)
+The AddPropertyStep does not have a provided value: AddPropertyStep({key=[y]})
+Type ':help' or ':h' for help.
+Display stack trace? [yN]N
+gremlin> g.addV("test").property("x", 1).property("y", null).property("z", 2)
+==>v[13]
+gremlin> g.V(13).properties()
+==>vp[x->1]
+==>vp[z->2]
+----
+
+In this release, this behavior has been altered to become consistent as follows:
+
+[source,text]
+----
+gremlin> g.V(1).property("x", 1).property("y", null).property("z", 2)
+Property value can not be null
+Type ':help' or ':h' for help.
+Display stack trace? [yN]n
+gremlin> g.addV().property("x", 1).property("y", null).property("z", 2)
+Property value can not be null
+Type ':help' or ':h' for help.
+Display stack trace? [yN]
+----
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2099[TINKERPOP-2099]
+
==== Deprecation Removal
The following deprecated classes, methods or fields have been removed in this version:
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 3493ec7..547517c 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
@@ -53,7 +53,7 @@ public class AddVertexStartStep extends AbstractStep<Vertex, Vertex>
public AddVertexStartStep(final Traversal.Admin traversal, final String label) {
super(traversal);
- this.parameters.set(this, T.label, label);
+ this.parameters.set(this, T.label, null == label ? Vertex.DEFAULT_LABEL : label);
}
public AddVertexStartStep(final Traversal.Admin traversal, final Traversal<?, String> vertexLabelTraversal) {
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 167510a..4961025 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
@@ -48,7 +48,7 @@ public class AddVertexStep<S> extends MapStep<S, Vertex>
public AddVertexStep(final Traversal.Admin traversal, final String label) {
super(traversal);
- this.parameters.set(this, T.label, label);
+ this.parameters.set(this, T.label, null == label ? Vertex.DEFAULT_LABEL : label);
}
public AddVertexStep(final Traversal.Admin traversal, final Traversal.Admin<S,String> vertexLabelTraversal) {
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 5c86ad9..22bb54f 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
@@ -37,6 +37,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.function.Supplier;
@@ -180,27 +181,24 @@ public final class Parameters implements Cloneable, Serializable {
if (!(keyValues[ix] instanceof String) && !(keyValues[ix] instanceof T) && !(keyValues[ix] instanceof Traversal))
throw new IllegalArgumentException("The provided key/value array must have a String, T, or Traversal on even array indices");
- if (keyValues[ix + 1] != null) {
-
- // check both key and value for traversal instances. track the list of traversals that are present so
- // that elsewhere in Parameters there is no need to iterate all values to not find any. also grab
- // available labels in traversal values
- for (int iy = 0; iy < 2; iy++) {
- if (keyValues[ix + iy] instanceof Traversal.Admin) {
- final Traversal.Admin t = (Traversal.Admin) keyValues[ix + iy];
- addTraversal(t);
- if (parent != null) parent.integrateChild(t);
- }
+ // check both key and value for traversal instances. track the list of traversals that are present so
+ // that elsewhere in Parameters there is no need to iterate all values to not find any. also grab
+ // available labels in traversal values
+ for (int iy = 0; iy < 2; iy++) {
+ if (keyValues[ix + iy] instanceof Traversal.Admin) {
+ final Traversal.Admin t = (Traversal.Admin) keyValues[ix + iy];
+ addTraversal(t);
+ if (parent != null) parent.integrateChild(t);
}
+ }
- List<Object> values = this.parameters.get(keyValues[ix]);
- if (null == values) {
- values = new ArrayList<>();
- values.add(keyValues[ix + 1]);
- this.parameters.put(keyValues[ix], values);
- } else {
- values.add(keyValues[ix + 1]);
- }
+ List<Object> values = this.parameters.get(keyValues[ix]);
+ if (null == values) {
+ values = new ArrayList<>();
+ values.add(keyValues[ix + 1]);
+ this.parameters.put(keyValues[ix], values);
+ } else {
+ values.add(keyValues[ix + 1]);
}
}
}
@@ -254,7 +252,7 @@ public final class Parameters implements Cloneable, Serializable {
for (final Map.Entry<Object, List<Object>> entry : this.parameters.entrySet()) {
result ^= entry.getKey().hashCode();
for (final Object value : entry.getValue()) {
- result ^= Integer.rotateLeft(value.hashCode(), entry.getKey().hashCode());
+ result ^= Integer.rotateLeft(Objects.hashCode(value), entry.getKey().hashCode());
}
}
return result;
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 ebe40af..c832cb7 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
@@ -43,6 +43,7 @@ import static org.mockito.Mockito.when;
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
public class ParametersTest {
+
@Test
public void shouldGetKeyValuesEmpty() {
final Parameters parameters = new Parameters();
@@ -50,6 +51,16 @@ public class ParametersTest {
}
@Test
+ public void shouldAllowNullValues() {
+ final Parameters parameters = new Parameters();
+ parameters.set(null, "a", null, "b", "bat", "c", "cat");
+
+ final Object[] params = parameters.getKeyValues(mock(Traverser.Admin.class));
+ assertEquals(6, params.length);
+ assertThat(Arrays.equals(new Object[] {"a", null, "b", "bat", "c", "cat"}, params), is(true));
+ }
+
+ @Test
public void shouldGetKeyValues() {
final Parameters parameters = new Parameters();
parameters.set(null, "a", "axe", "b", "bat", "c", "cat");