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
The following commit(s) were added to refs/heads/TINKERPOP-2235 by this push: new 36888dc TINKERPOP-2235 Added tests and Graph level null support 36888dc is described below commit 36888dc643855602a5417a4587ce75d182caf9cb Author: stephen <spmalle...@gmail.com> AuthorDate: Tue Nov 5 15:26:13 2019 -0500 TINKERPOP-2235 Added tests and Graph level null support The problem that reared its head here is in JavaTranslator (and likely GroovyTranslator) as well where the overloads of Gremlin steps become ambiguous when null is used. Left a big blob comment in JavaTranslator where I came up with a possibly reasonable solution for right now. Added a Graph Feature for null support in property values. Allowed TinkerGraph to support it by default but added a configuration to turn that support off. Neo4j does not support nulls so that feature is disabl [...] --- .../tinkerpop/gremlin/jsr223/JavaTranslator.java | 17 ++++- .../gremlin/process/computer/VertexComputeKey.java | 2 +- .../apache/tinkerpop/gremlin/structure/Graph.java | 18 +++++ .../gremlin/structure/util/ElementHelper.java | 14 ++-- .../gremlin/structure/util/star/StarGraph.java | 16 ++--- .../gremlin/structure/util/ElementHelperTest.java | 23 +++--- gremlin-test/features/map/AddEdge.feature | 28 ++++++++ gremlin-test/features/map/AddVertex.feature | 25 +++++++ gremlin-test/features/map/Constant.feature | 17 ++++- .../process/traversal/step/map/AddEdgeTest.java | 30 ++++++++ .../process/traversal/step/map/AddVertexTest.java | 24 +++++++ .../process/traversal/step/map/ConstantTest.java | 15 +++- .../tinkerpop/gremlin/structure/PropertyTest.java | 84 +++++++++++++++++++--- .../gremlin/neo4j/structure/Neo4jEdge.java | 2 +- .../gremlin/neo4j/structure/Neo4jGraph.java | 12 +++- .../gremlin/neo4j/structure/Neo4jVertex.java | 4 +- .../process/computer/TinkerGraphComputerView.java | 2 +- .../gremlin/tinkergraph/structure/TinkerEdge.java | 4 +- .../gremlin/tinkergraph/structure/TinkerGraph.java | 5 +- .../tinkergraph/structure/TinkerHelper.java | 2 +- .../gremlin/tinkergraph/structure/TinkerIndex.java | 48 +++++++++---- .../tinkergraph/structure/TinkerProperty.java | 5 +- .../tinkergraph/structure/TinkerVertex.java | 6 +- .../structure/TinkerVertexProperty.java | 7 +- 24 files changed, 346 insertions(+), 64 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java index 515bc93..bb9273c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java @@ -226,13 +226,24 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal newArguments[i] = varArgs; break; } else { - if (i < argumentsCopy.length && - (parameters[i].getType().isAssignableFrom(argumentsCopy[i].getClass()) || + // try to detect the right method by comparing the type of the parameter to the type + // of the argument. doesn't always work so well because of null arguments which don't + // bring their type in bytecode and rely on position. this doesn't seem to happen often + // ...luckily...because method signatures tend to be sufficiently unique and do not + // encourage whacky use - like g.V().has(null, null) is clearly invalid so we don't + // even need to try to sort that out. on the other hand g.V().has('name',null) which + // is valid hits like four different possible overloads, but we can rely on the most + // generic one which takes Object as the second parameter. that seems to work in this + // case, but it's a shame this isn't nicer. seems like nicer would mean a heavy + // overhaul to Gremlin or to GLVs/bytecode and/or to serialization mechanisms + if (i < argumentsCopy.length && ((null == argumentsCopy[i] && parameters[i].getType() == Object.class) || + (argumentsCopy[i] != null && ( + parameters[i].getType().isAssignableFrom(argumentsCopy[i].getClass()) || (parameters[i].getType().isPrimitive() && (Number.class.isAssignableFrom(argumentsCopy[i].getClass()) || argumentsCopy[i].getClass().equals(Boolean.class) || argumentsCopy[i].getClass().equals(Byte.class) || - argumentsCopy[i].getClass().equals(Character.class))))) { + argumentsCopy[i].getClass().equals(Character.class))))))) { newArguments[i] = argumentsCopy[i]; } else { found = false; diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexComputeKey.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexComputeKey.java index 284430a..072ad59 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexComputeKey.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexComputeKey.java @@ -37,7 +37,7 @@ public final class VertexComputeKey implements Serializable { private VertexComputeKey(final String key, final boolean isTransient) { this.key = key; this.isTransient = isTransient; - ElementHelper.validateProperty(key, key); + ElementHelper.validateProperty(true, key, key); } public String getKey() { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Graph.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Graph.java index 42fee1f..22ae658 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Graph.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Graph.java @@ -682,6 +682,15 @@ public interface Graph extends AutoCloseable, Host { public static final String FEATURE_ANY_IDS = "AnyIds"; public static final String FEATURE_ADD_PROPERTY = "AddProperty"; public static final String FEATURE_REMOVE_PROPERTY = "RemoveProperty"; + public static final String FEATURE_NULL_PROPERTY_VALUES = "NullPropertyValues"; + + /** + * Determines if an {@link Element} allows properties with {@code null} property values. + */ + @FeatureDescriptor(name = FEATURE_NULL_PROPERTY_VALUES) + public default boolean supportsNullPropertyValues() { + return true; + } /** * Determines if an {@link Element} allows properties to be added. This feature is set independently from @@ -816,6 +825,15 @@ public interface Graph extends AutoCloseable, Host { public static final String FEATURE_UUID_IDS = "UuidIds"; public static final String FEATURE_CUSTOM_IDS = "CustomIds"; public static final String FEATURE_ANY_IDS = "AnyIds"; + public static final String FEATURE_NULL_PROPERTY_VALUES = "NullPropertyValues"; + + /** + * Determines if meta-properties allow for {@code null} property values. + */ + @FeatureDescriptor(name = FEATURE_NULL_PROPERTY_VALUES) + public default boolean supportsNullPropertyValues() { + return true; + } /** * Determines if a {@link VertexProperty} allows properties to be removed. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelper.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelper.java index 829880b..fb18296 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelper.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelper.java @@ -87,13 +87,14 @@ public final class ElementHelper { * Determines whether the property key/value for the specified thing can be legally set. This is typically used as * a pre-condition check prior to setting a property. * + * @param allowNullValue true if {@code null} is allowed as a value to the property * @param key the key of the property - * @param value the value of the property + * @param value the value of the property\ * @throws IllegalArgumentException whether the key/value pair is legal and if not, a clear reason exception * message is provided */ - public static void validateProperty(final String key, final Object value) throws IllegalArgumentException { - if (null == value) + public static void validateProperty(final boolean allowNullValue, final String key, final Object value) throws IllegalArgumentException { + if (!allowNullValue && null == value) throw Property.Exceptions.propertyValueCanNotBeNull(); if (null == key) throw Property.Exceptions.propertyKeyCanNotBeNull(); @@ -105,19 +106,20 @@ public final class ElementHelper { /** * Determines whether a list of key/values are legal, ensuring that there are an even number of values submitted - * and that the key values in the list of arguments are {@link String} or {@link org.apache.tinkerpop.gremlin.structure.Element} objects. + * and that the key values in the list of arguments are {@link String} or {@link Element} objects. * + * @param allowNullValues true if {@code null} is allowed as a value to the property * @param propertyKeyValues a list of key/value pairs * @throws IllegalArgumentException if something in the pairs is illegal */ - public static void legalPropertyKeyValueArray(final Object... propertyKeyValues) throws IllegalArgumentException { + public static void legalPropertyKeyValueArray(final boolean allowNullValues, final Object... propertyKeyValues) throws IllegalArgumentException { if (propertyKeyValues.length % 2 != 0) throw Element.Exceptions.providedKeyValuesMustBeAMultipleOfTwo(); for (int i = 0; i < propertyKeyValues.length; i = i + 2) { if (!(propertyKeyValues[i] instanceof String) && !(propertyKeyValues[i] instanceof T)) throw Element.Exceptions.providedKeyValuesMustHaveALegalKeyOnEvenIndices(); - if (null == propertyKeyValues[i + 1]) { + if (!allowNullValues && null == propertyKeyValues[i + 1]) { throw Property.Exceptions.propertyValueCanNotBeNull(); } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java index 3bcdb7d..935ebef 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/star/StarGraph.java @@ -95,7 +95,7 @@ public final class StarGraph implements Graph, Serializable { @Override public Vertex addVertex(final Object... keyValues) { if (null == this.starVertex) { - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(true, keyValues); this.starVertex = new StarVertex(ElementHelper.getIdValue(keyValues).orElse(this.nextId()), ElementHelper.getLabelValue(keyValues).orElse(Vertex.DEFAULT_LABEL)); ElementHelper.attachProperties(this.starVertex, VertexProperty.Cardinality.list, keyValues); // TODO: is this smart? I say no... cause vertex property ids are not preserved. return this.starVertex; @@ -413,14 +413,14 @@ public final class StarGraph implements Graph, Serializable { @Override public <V> VertexProperty<V> property(final String key, final V value, final Object... keyValues) { - ElementHelper.validateProperty(key, value); - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.validateProperty(true, key, value); + ElementHelper.legalPropertyKeyValueArray(true, keyValues); return this.property(VertexProperty.Cardinality.single, key, value, keyValues); } Edge addOutEdge(final String label, final Vertex inVertex, final Object... keyValues) { ElementHelper.validateLabel(label); - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(true, keyValues); if (null == this.outEdges) this.outEdges = new HashMap<>(); List<Edge> outE = this.outEdges.get(label); @@ -436,7 +436,7 @@ public final class StarGraph implements Graph, Serializable { Edge addInEdge(final String label, final Vertex outVertex, final Object... keyValues) { ElementHelper.validateLabel(label); - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(true, keyValues); if (null == this.inEdges) this.inEdges = new HashMap<>(); List<Edge> inE = this.inEdges.get(label); @@ -452,7 +452,7 @@ public final class StarGraph implements Graph, Serializable { @Override public <V> VertexProperty<V> property(final VertexProperty.Cardinality cardinality, final String key, V value, final Object... keyValues) { - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(true, keyValues); if (null == this.vertexProperties) this.vertexProperties = new HashMap<>(); final List<VertexProperty> list = cardinality.equals(VertexProperty.Cardinality.single) ? new ArrayList<>(1) : this.vertexProperties.getOrDefault(key, new ArrayList<>()); @@ -645,7 +645,7 @@ public final class StarGraph implements Graph, Serializable { @Override public <U> Property<U> property(final String key, final U value) { - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(true, key, value); if (null == metaProperties) metaProperties = new HashMap<>(); Map<String, Object> properties = metaProperties.get(this.id); @@ -760,7 +760,7 @@ public final class StarGraph implements Graph, Serializable { @Override public <V> Property<V> property(final String key, final V value) { - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(true, key, value); if (null == edgeProperties) edgeProperties = new HashMap<>(); Map<String, Object> properties = edgeProperties.get(this.id); diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelperTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelperTest.java index de774da..b466788 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelperTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/ElementHelperTest.java @@ -56,7 +56,7 @@ public class ElementHelperTest { @Test public void shouldValidatePropertyAndNotAllowNullValue() { try { - ElementHelper.validateProperty("test", null); + ElementHelper.validateProperty(false,"test", null); fail("Should fail as property value cannot be null"); } catch (IllegalArgumentException iae) { assertEquals(Property.Exceptions.propertyValueCanNotBeNull().getMessage(), iae.getMessage()); @@ -64,9 +64,14 @@ public class ElementHelperTest { } @Test + public void shouldValidatePropertyAndAllowNullValue() { + ElementHelper.validateProperty(true,"test", null); + } + + @Test public void shouldValidatePropertyAndNotAllowNullKey() { try { - ElementHelper.validateProperty(null, "test"); + ElementHelper.validateProperty(false,null, "test"); fail("Should fail as property key cannot be null"); } catch (IllegalArgumentException iae) { assertEquals(Property.Exceptions.propertyKeyCanNotBeNull().getMessage(), iae.getMessage()); @@ -76,7 +81,7 @@ public class ElementHelperTest { @Test public void shouldValidatePropertyAndNotAllowEmptyKey() { try { - ElementHelper.validateProperty("", "test"); + ElementHelper.validateProperty(false,"", "test"); fail("Should fail as property key cannot be empty"); } catch (IllegalArgumentException iae) { assertEquals(Property.Exceptions.propertyKeyCanNotBeEmpty().getMessage(), iae.getMessage()); @@ -87,7 +92,7 @@ public class ElementHelperTest { public void shouldValidatePropertyAndNotAllowHiddenKey() { final String key = Graph.Hidden.hide("key"); try { - ElementHelper.validateProperty(key, "test"); + ElementHelper.validateProperty(false, key, "test"); fail("Should fail as property key cannot be hidden"); } catch (IllegalArgumentException iae) { assertEquals(Property.Exceptions.propertyKeyCanNotBeAHiddenKey(key).getMessage(), iae.getMessage()); @@ -96,13 +101,13 @@ public class ElementHelperTest { @Test public void shouldHaveValidProperty() { - ElementHelper.validateProperty("aKey", "value"); + ElementHelper.validateProperty(false,"aKey", "value"); } @Test public void shouldAllowEvenNumberOfKeyValues() { try { - ElementHelper.legalPropertyKeyValueArray("aKey", "test", "no-value-for-this-one"); + ElementHelper.legalPropertyKeyValueArray(false,"aKey", "test", "no-value-for-this-one"); fail("Should fail as there is an odd number of key-values"); } catch (IllegalArgumentException iae) { assertEquals(Element.Exceptions.providedKeyValuesMustBeAMultipleOfTwo().getMessage(), iae.getMessage()); @@ -112,7 +117,7 @@ public class ElementHelperTest { @Test public void shouldNotAllowEvenNumberOfKeyValuesAndInvalidKeys() { try { - ElementHelper.legalPropertyKeyValueArray("aKey", "test", "value-for-this-one", 1, 1, "none"); + ElementHelper.legalPropertyKeyValueArray(false,"aKey", "test", "value-for-this-one", 1, 1, "none"); fail("Should fail as there is an even number of key-values, but a bad key"); } catch (IllegalArgumentException iae) { assertEquals(Element.Exceptions.providedKeyValuesMustHaveALegalKeyOnEvenIndices().getMessage(), iae.getMessage()); @@ -121,13 +126,13 @@ public class ElementHelperTest { @Test public void shouldAllowEvenNumberOfKeyValuesAndValidKeys() { - ElementHelper.legalPropertyKeyValueArray("aKey", "test", "value-for-this-one", 1, "1", "none"); + ElementHelper.legalPropertyKeyValueArray(false,"aKey", "test", "value-for-this-one", 1, "1", "none"); } @Test public void shouldNotAllowEvenNumberOfKeyValuesAndInvalidValues() { try { - ElementHelper.legalPropertyKeyValueArray("aKey", "test", "value-for-this-one", 1, "1", null); + ElementHelper.legalPropertyKeyValueArray(false,"aKey", "test", "value-for-this-one", 1, "1", null); } catch (IllegalArgumentException iae) { assertEquals(Property.Exceptions.propertyValueCanNotBeNull().getMessage(), iae.getMessage()); } diff --git a/gremlin-test/features/map/AddEdge.feature b/gremlin-test/features/map/AddEdge.feature index 7f4b0aa..875e474 100644 --- a/gremlin-test/features/map/AddEdge.feature +++ b/gremlin-test/features/map/AddEdge.feature @@ -72,6 +72,34 @@ Feature: Step - addE() And the graph should return 4 for count of "g.V(v1Id).bothE()" And the graph should return 1 for count of "g.V(v1Id).inE().has(\"weight\", 2.0)" + Scenario: g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_nullX + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "marko").property("age", 29).as("marko"). + addV("person").property(T.id, 2).property("name", "vadas").property("age", 27).as("vadas"). + addV("software").property(T.id, 3).property("name", "lop").property("lang", "java").as("lop"). + addV("person").property(T.id, 4).property("name","josh").property("age", 32).as("josh"). + addV("software").property(T.id, 5).property("name", "ripple").property("lang", "java").as("ripple"). + addV("person").property(T.id, 6).property("name", "peter").property("age", 35).as('peter'). + addE("knows").from("marko").to("vadas").property(T.id, 7).property("weight", 0.5). + addE("knows").from("marko").to("josh").property(T.id, 8).property("weight", 1.0). + addE("created").from("marko").to("lop").property(T.id, 9).property("weight", 0.4). + addE("created").from("josh").to("ripple").property(T.id, 10).property("weight", 1.0). + addE("created").from("josh").to("lop").property(T.id, 11).property("weight", 0.4). + addE("created").from("peter").to("lop").property(T.id, 12).property("weight", 0.2) + """ + And using the parameter v1Id defined as "v[marko].id" + And the traversal of + """ + g.V(v1Id).as("a").out("created").addE("createdBy").to("a").property("weight", null) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 7 for count of "g.E()" + And the graph should return 4 for count of "g.V(v1Id).bothE()" + And the graph should return 1 for count of "g.V(v1Id).inE().has(\"weight\", null)" + Scenario: g_V_aggregateXxX_asXaX_selectXxX_unfold_addEXexistsWithX_toXaX_propertyXtime_nowX Given the empty graph And the graph initializer of diff --git a/gremlin-test/features/map/AddVertex.feature b/gremlin-test/features/map/AddVertex.feature index a60fefb..efe3fc0 100644 --- a/gremlin-test/features/map/AddVertex.feature +++ b/gremlin-test/features/map/AddVertex.feature @@ -93,6 +93,31 @@ Feature: Step - addV() Then the result should have a count of 1 And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"stephen\")" + Scenario: g_addVXpersonX_propertyXname_nullX + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "marko").property("age", 29).as("marko"). + addV("person").property(T.id, 2).property("name", "vadas").property("age", 27).as("vadas"). + addV("software").property(T.id, 3).property("name", "lop").property("lang", "java").as("lop"). + addV("person").property(T.id, 4).property("name","josh").property("age", 32).as("josh"). + addV("software").property(T.id, 5).property("name", "ripple").property("lang", "java").as("ripple"). + addV("person").property(T.id, 6).property("name", "peter").property("age", 35).as('peter'). + addE("knows").from("marko").to("vadas").property(T.id, 7).property("weight", 0.5). + addE("knows").from("marko").to("josh").property(T.id, 8).property("weight", 1.0). + addE("created").from("marko").to("lop").property(T.id, 9).property("weight", 0.4). + addE("created").from("josh").to("ripple").property(T.id, 10).property("weight", 1.0). + addE("created").from("josh").to("lop").property(T.id, 11).property("weight", 0.4). + addE("created").from("peter").to("lop").property(T.id, 12).property("weight", 0.2) + """ + And the traversal of + """ + g.addV("person").property("name", null) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 1 for count of "g.V().has(\"person\",\"name\",null)" + Scenario: g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenmX Given the empty graph And the graph initializer of diff --git a/gremlin-test/features/map/Constant.feature b/gremlin-test/features/map/Constant.feature index 9ad8d13..ed25b25 100644 --- a/gremlin-test/features/map/Constant.feature +++ b/gremlin-test/features/map/Constant.feature @@ -19,7 +19,6 @@ Feature: Step - constant() Scenario: g_V_constantX123X Given the modern graph - And using the parameter v1Id defined as "v[marko].id" And the traversal of """ g.V().constant(123) @@ -34,6 +33,22 @@ Feature: Step - constant() | d[123].i | | d[123].i | + Scenario: g_V_constantXnullX + Given the modern graph + And the traversal of + """ + g.V().constant(null) + """ + When iterated to list + Then the result should be unordered + | result | + | null | + | null | + | null | + | null | + | null | + | null | + Scenario: g_V_chooseXhasLabelXpersonX_valuesXnameX_constantXinhumanXX Given the modern graph And the traversal of diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeTest.java index 54c4091..f534a4b 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeTest.java @@ -46,6 +46,7 @@ import static org.apache.tinkerpop.gremlin.structure.Column.keys; import static org.apache.tinkerpop.gremlin.structure.Column.values; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; /** * @author Marko A. Rodriguez (http://markorodriguez.com) @@ -58,6 +59,8 @@ public abstract class AddEdgeTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Edge> get_g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_2X(final Object v1Id); + public abstract Traversal<Vertex, Edge> get_g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_nullX(final Object v1Id); + public abstract Traversal<Vertex, Edge> get_g_V_aggregateXxX_asXaX_selectXxX_unfold_addEXexistsWithX_toXaX_propertyXtime_nowX(); public abstract Traversal<Vertex, Edge> get_g_V_asXaX_outXcreatedX_inXcreatedX_whereXneqXaXX_asXbX_addEXcodeveloperX_fromXaX_toXbX_propertyXyear_2009X(); @@ -121,6 +124,28 @@ public abstract class AddEdgeTest extends AbstractGremlinProcessTest { @Test @LoadGraphWith(MODERN) @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_NULL_PROPERTY_VALUES) + public void g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_nullX() { + final Traversal<Vertex, Edge> traversal = get_g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_nullX(convertToVertexId("marko")); + printTraversalForm(traversal); + int count = 0; + while (traversal.hasNext()) { + final Edge edge = traversal.next(); + assertEquals("createdBy", edge.label()); + assertNull(g.E(edge).<Double>values("weight").next()); + assertEquals(1, g.E(edge).properties().count().next().intValue()); + count++; + + + } + assertEquals(1, count); + assertEquals(7, IteratorUtils.count(g.E())); + assertEquals(6, IteratorUtils.count(g.V())); + } + + @Test + @LoadGraphWith(MODERN) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES) public void g_V_aggregateXxX_asXaX_selectXxX_unfold_addEXexistsWithX_toXaX_propertyXtime_nowX() { final Traversal<Vertex, Edge> traversal = get_g_V_aggregateXxX_asXaX_selectXxX_unfold_addEXexistsWithX_toXaX_propertyXtime_nowX(); printTraversalForm(traversal); @@ -306,6 +331,11 @@ public abstract class AddEdgeTest extends AbstractGremlinProcessTest { } @Override + public Traversal<Vertex, Edge> get_g_VX1X_asXaX_outXcreatedX_addEXcreatedByX_toXaX_propertyXweight_nullX(final Object v1Id) { + return g.V(v1Id).as("a").out("created").addE("createdBy").to("a").property("weight", null); + } + + @Override public Traversal<Vertex, Edge> get_g_V_aggregateXxX_asXaX_selectXxX_unfold_addEXexistsWithX_toXaX_propertyXtime_nowX() { return g.V().aggregate("x").as("a").select("x").unfold().addE("existsWith").to("a").property("time", "now"); } 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 21fbb8c..ebad007 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 @@ -42,6 +42,7 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.V; import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.select; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** @@ -56,6 +57,8 @@ public abstract class AddVertexTest extends AbstractGremlinTest { public abstract Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXname_stephenX(); + public abstract Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXname_nullX(); + 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(); @@ -130,6 +133,22 @@ public abstract class AddVertexTest extends AbstractGremlinTest { @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) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_NULL_PROPERTY_VALUES) + public void g_addVXpersonX_propertyXname_nullX() { + final Traversal<Vertex, Vertex> traversal = get_g_addVXpersonX_propertyXname_nullX(); + printTraversalForm(traversal); + final Vertex nulled = traversal.next(); + assertFalse(traversal.hasNext()); + assertEquals("person", nulled.label()); + assertNull(nulled.value("name")); + assertEquals(1, IteratorUtils.count(nulled.properties())); + assertEquals(7, IteratorUtils.count(g.V())); + } + + @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) public void g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenmX() { final Traversal<Vertex, Vertex> traversal = get_g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenmX(); printTraversalForm(traversal); @@ -315,6 +334,11 @@ public abstract class AddVertexTest extends AbstractGremlinTest { } @Override + public Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXname_nullX() { + return g.addV("person").property("name", null); + } + + @Override public Traversal<Vertex, Vertex> get_g_addVXpersonX_propertyXsingle_name_stephenX_propertyXsingle_name_stephenmX() { return g.addV("person").property(VertexProperty.Cardinality.single, "name", "stephen").property(VertexProperty.Cardinality.single, "name", "stephenm"); } diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConstantTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConstantTest.java index e35db43..07a7b55 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConstantTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ConstantTest.java @@ -46,9 +46,10 @@ public abstract class ConstantTest extends AbstractGremlinProcessTest { public abstract Traversal<Vertex, Integer> get_g_V_constantX123X(); + public abstract Traversal<Vertex, Void> get_g_V_constantXnullX(); + public abstract Traversal<Vertex, String> get_g_V_chooseXhasLabelXpersonX_valuesXnameX_constantXinhumanXX(); - /** Scenario: Trivial use case from GraphTraversal */ @Test @LoadGraphWith(MODERN) public void g_V_constantX123X() { @@ -56,6 +57,13 @@ public abstract class ConstantTest extends AbstractGremlinProcessTest { printTraversalForm(traversal); assertEquals(Arrays.asList(123, 123, 123, 123, 123, 123), traversal.toList()); } + @Test + @LoadGraphWith(MODERN) + public void g_V_constantXnullX() { + final Traversal<Vertex, Void> traversal = get_g_V_constantXnullX(); + printTraversalForm(traversal); + assertEquals(Arrays.asList(null, null, null, null, null, null), traversal.toList()); + } /** Scenario: Anonymous traversal within choose */ @Test @@ -74,6 +82,11 @@ public abstract class ConstantTest extends AbstractGremlinProcessTest { } @Override + public Traversal<Vertex, Void> get_g_V_constantXnullX() { + return g.V().constant(null); + } + + @Override public Traversal<Vertex, String> get_g_V_chooseXhasLabelXpersonX_valuesXnameX_constantXinhumanXX() { return g.V().choose(hasLabel("person"), values("name"), constant("inhuman")); } diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/PropertyTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/PropertyTest.java index fb50841..68c9245 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/PropertyTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/PropertyTest.java @@ -41,6 +41,7 @@ import java.util.Map; import static org.apache.tinkerpop.gremlin.structure.Graph.Features.PropertyFeatures.FEATURE_PROPERTIES; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; @@ -56,6 +57,9 @@ public class PropertyTest { /** * Basic tests for the {@link org.apache.tinkerpop.gremlin.structure.Property} class. */ + @ExceptionCoverage(exceptionClass = Property.Exceptions.class, methods = { + "propertyValueCanNotBeNull" + }) public static class BasicPropertyTest extends AbstractGremlinTest { @Test @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) @@ -141,11 +145,78 @@ public class PropertyTest { fail("Removing an edge property that was already removed should not throw an exception"); } } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_NULL_PROPERTY_VALUES, supported = false) + public void shouldNotAllowNullAddVertex() throws Exception { + try { + this.graph.addVertex("name", null); + fail("Call to addVertex() should have thrown an exception as null is not a supported property value"); + } catch (Exception ex) { + validateException(Property.Exceptions.propertyValueCanNotBeNull(), ex); + } + } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.SIMPLE) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_NULL_PROPERTY_VALUES, supported = false) + public void shouldNotAllowNullAddEdge() throws Exception { + try { + final Vertex v = this.graph.addVertex(); + v.addEdge("self", v, "name", null); + fail("Call to addEdge() should have thrown an exception as null is not a supported property value"); + } catch (Exception ex) { + validateException(Property.Exceptions.propertyValueCanNotBeNull(), ex); + } + } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_NULL_PROPERTY_VALUES) + public void shouldAllowNullAddVertex() throws Exception { + final Vertex v = this.graph.addVertex("name", null); + assertNull(v.value("name")); + } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.SIMPLE) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_NULL_PROPERTY_VALUES) + public void shouldAllowNullAddEdge() throws Exception { + final Vertex v = this.graph.addVertex(); + final Edge e = v.addEdge("self", v, "name", null); + assertNull(e.value("name")); + } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) + @FeatureRequirement(featureClass = Graph.Features.VertexPropertyFeatures.class, feature = Graph.Features.VertexPropertyFeatures.FEATURE_NULL_PROPERTY_VALUES) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_META_PROPERTIES) + public void shouldAllowNullAddVertexProperty() throws Exception { + final Vertex v = this.graph.addVertex("person"); + final VertexProperty vp = v.property("location", "santa fe", "startTime", 1995, "endTime", null); + assertEquals(1995, (int) vp.value("startTime")); + assertNull(vp.value("endTime")); + } + + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) + @FeatureRequirement(featureClass = Graph.Features.VertexPropertyFeatures.class, feature = Graph.Features.VertexPropertyFeatures.FEATURE_NULL_PROPERTY_VALUES, supported = false) + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_META_PROPERTIES) + public void shouldNotAllowNullAddVertexProperty() throws Exception { + try { + final Vertex v = this.graph.addVertex("person"); + final VertexProperty vp = v.property("location", "santa fe", "startTime", 1995, "endTime", null); + fail("Call to property() should have thrown an exception as null is not a supported property value"); + } catch (Exception ex) { + validateException(Property.Exceptions.propertyValueCanNotBeNull(), ex); + } + } } /** - * Checks that properties added to an {@link org.apache.tinkerpop.gremlin.structure.Element} are validated in a consistent way when they are added at - * {@link org.apache.tinkerpop.gremlin.structure.Vertex} or {@link org.apache.tinkerpop.gremlin.structure.Edge} construction by throwing an appropriate exception. + * Checks that properties added to an {@link Element} are validated in a consistent way when they are added at + * {@link Vertex} or {@link Edge} construction by throwing an appropriate exception. */ @RunWith(Parameterized.class) @ExceptionCoverage(exceptionClass = Element.Exceptions.class, methods = { @@ -153,7 +224,6 @@ public class PropertyTest { "providedKeyValuesMustHaveALegalKeyOnEvenIndices" }) @ExceptionCoverage(exceptionClass = Property.Exceptions.class, methods = { - "propertyValueCanNotBeNull", "propertyKeyCanNotBeEmpty" }) public static class PropertyValidationOnAddExceptionConsistencyTest extends AbstractGremlinTest { @@ -164,7 +234,6 @@ public class PropertyTest { {"providedKeyValuesMustBeAMultipleOfTwo", new Object[]{"odd", "number", "arguments"}, Element.Exceptions.providedKeyValuesMustBeAMultipleOfTwo()}, {"providedKeyValuesMustBeAMultipleOfTwo", new Object[]{"odd"}, Element.Exceptions.providedKeyValuesMustBeAMultipleOfTwo()}, {"providedKeyValuesMustHaveALegalKeyOnEvenIndices", new Object[]{"odd", "number", 123, "test"}, Element.Exceptions.providedKeyValuesMustHaveALegalKeyOnEvenIndices()}, - {"propertyValueCanNotBeNull", new Object[]{"odd", null}, Property.Exceptions.propertyValueCanNotBeNull()}, {"providedKeyValuesMustHaveALegalKeyOnEvenIndices", new Object[]{null, "val"}, Element.Exceptions.providedKeyValuesMustHaveALegalKeyOnEvenIndices()}, {"propertyKeyCanNotBeEmpty", new Object[]{"", "val"}, Property.Exceptions.propertyKeyCanNotBeEmpty()}}); } @@ -245,13 +314,11 @@ public class PropertyTest { /** - * Checks that properties added to an {@link org.apache.tinkerpop.gremlin.structure.Element} are validated in a - * consistent way when they are set after {@link Vertex} or {@link Edge} construction by throwing an - * appropriate exception. + * Checks that properties added to an {@link Element} are validated in a consistent way when they are set after + * {@link Vertex} or {@link Edge} construction by throwing an appropriate exception. */ @RunWith(Parameterized.class) @ExceptionCoverage(exceptionClass = Property.Exceptions.class, methods = { - "propertyValueCanNotBeNull", "propertyKeyCanNotBeNull", "propertyKeyCanNotBeEmpty", "propertyKeyCanNotBeAHiddenKey" @@ -261,7 +328,6 @@ public class PropertyTest { @Parameterized.Parameters(name = "expect({0})") public static Iterable<Object[]> data() { return Arrays.asList(new Object[][]{ - {"propertyValueCanNotBeNull", "k", null, Property.Exceptions.propertyValueCanNotBeNull()}, {"propertyKeyCanNotBeNull", null, "v", Property.Exceptions.propertyKeyCanNotBeNull()}, {"propertyKeyCanNotBeEmpty", "", "v", Property.Exceptions.propertyKeyCanNotBeEmpty()}, {"propertyKeyCanNotBeAHiddenKey", Graph.Hidden.hide("systemKey"), "value", Property.Exceptions.propertyKeyCanNotBeAHiddenKey(Graph.Hidden.hide("systemKey"))}}); diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jEdge.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jEdge.java index 29ccef1..2729f9f 100644 --- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jEdge.java +++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jEdge.java @@ -114,7 +114,7 @@ public final class Neo4jEdge extends Neo4jElement implements Edge, WrappedEdge<N @Override public <V> Property<V> property(final String key, final V value) { - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(false, key, value); this.graph.tx().readWrite(); try { this.baseElement.setProperty(key, value); diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jGraph.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jGraph.java index aa62ce6..2a18d47 100644 --- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jGraph.java +++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jGraph.java @@ -127,7 +127,7 @@ public final class Neo4jGraph implements Graph, WrappedGraph<Neo4jGraphAPI> { @Override public Vertex addVertex(final Object... keyValues) { - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(false, keyValues); if (ElementHelper.getIdValue(keyValues).isPresent()) throw Vertex.Exceptions.userSuppliedIdsNotSupported(); this.tx().readWrite(); @@ -417,6 +417,11 @@ public final class Neo4jGraph implements Graph, WrappedGraph<Neo4jGraphAPI> { } @Override + public boolean supportsNullPropertyValues() { + return false; + } + + @Override public boolean supportsUserSuppliedIds() { return false; } @@ -448,6 +453,11 @@ public final class Neo4jGraph implements Graph, WrappedGraph<Neo4jGraphAPI> { } @Override + public boolean supportsNullPropertyValues() { + return false; + } + + @Override public boolean supportsMapValues() { return false; } diff --git a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jVertex.java b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jVertex.java index c5cce7d..2bbca2f 100644 --- a/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jVertex.java +++ b/neo4j-gremlin/src/main/java/org/apache/tinkerpop/gremlin/neo4j/structure/Neo4jVertex.java @@ -56,7 +56,7 @@ public final class Neo4jVertex extends Neo4jElement implements Vertex, WrappedVe public Edge addEdge(final String label, final Vertex inVertex, final Object... keyValues) { if (null == inVertex) throw Graph.Exceptions.argumentCanNotBeNull("inVertex"); ElementHelper.validateLabel(label); - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(false, keyValues); if (ElementHelper.getIdValue(keyValues).isPresent()) throw Edge.Exceptions.userSuppliedIdsNotSupported(); @@ -91,7 +91,7 @@ public final class Neo4jVertex extends Neo4jElement implements Vertex, WrappedVe @Override public <V> VertexProperty<V> property(final VertexProperty.Cardinality cardinality, final String key, final V value, final Object... keyValues) { - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(false, key, value); if (ElementHelper.getIdValue(keyValues).isPresent()) throw Vertex.Exceptions.userSuppliedIdsNotSupported(); this.graph.tx().readWrite(); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/computer/TinkerGraphComputerView.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/computer/TinkerGraphComputerView.java index 43998fb..b61fc2a 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/computer/TinkerGraphComputerView.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/computer/TinkerGraphComputerView.java @@ -79,7 +79,7 @@ public final class TinkerGraphComputerView { } public <V> Property<V> addProperty(final TinkerVertex vertex, final String key, final V value) { - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(true, key, value); if (isComputeKey(key)) { final TinkerVertexProperty<V> property = new TinkerVertexProperty<V>((TinkerVertex) vertex, key, value) { @Override diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerEdge.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerEdge.java index bcfe485..d594fea 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerEdge.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerEdge.java @@ -43,18 +43,20 @@ public final class TinkerEdge extends TinkerElement implements Edge { protected Map<String, Property> properties; protected final Vertex inVertex; protected final Vertex outVertex; + private final boolean allowNullPropertyValues; protected TinkerEdge(final Object id, final Vertex outVertex, final String label, final Vertex inVertex) { super(id, label); this.outVertex = outVertex; this.inVertex = inVertex; + this.allowNullPropertyValues = outVertex.graph().features().edge().supportsNullPropertyValues(); TinkerHelper.autoUpdateIndex(this, T.label.getAccessor(), this.label, null); } @Override public <V> Property<V> property(final String key, final V value) { if (this.removed) throw elementAlreadyRemoved(Edge.class, id); - ElementHelper.validateProperty(key, value); + ElementHelper.validateProperty(allowNullPropertyValues, key, value); final Property oldProperty = super.property(key); final Property<V> newProperty = new TinkerProperty<>(this, key, value); if (null == this.properties) this.properties = new HashMap<>(); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java index 67df21a..7e2cf46 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java @@ -83,6 +83,7 @@ public final class TinkerGraph implements Graph { public static final String GREMLIN_TINKERGRAPH_DEFAULT_VERTEX_PROPERTY_CARDINALITY = "gremlin.tinkergraph.defaultVertexPropertyCardinality"; public static final String GREMLIN_TINKERGRAPH_GRAPH_LOCATION = "gremlin.tinkergraph.graphLocation"; public static final String GREMLIN_TINKERGRAPH_GRAPH_FORMAT = "gremlin.tinkergraph.graphFormat"; + public static final String GREMLIN_TINKERGRAPH_ALLOW_NULL_PROPERTY_VALUES = "gremlin.tinkergraph.allowNullPropertyValues"; private final TinkerGraphFeatures features = new TinkerGraphFeatures(); @@ -99,6 +100,7 @@ public final class TinkerGraph implements Graph { protected final IdManager<?> edgeIdManager; protected final IdManager<?> vertexPropertyIdManager; protected final VertexProperty.Cardinality defaultVertexPropertyCardinality; + protected final boolean allowNullPropertyValues; private final Configuration configuration; private final String graphLocation; @@ -114,6 +116,7 @@ public final class TinkerGraph implements Graph { vertexPropertyIdManager = selectIdManager(configuration, GREMLIN_TINKERGRAPH_VERTEX_PROPERTY_ID_MANAGER, VertexProperty.class); defaultVertexPropertyCardinality = VertexProperty.Cardinality.valueOf( configuration.getString(GREMLIN_TINKERGRAPH_DEFAULT_VERTEX_PROPERTY_CARDINALITY, VertexProperty.Cardinality.single.name())); + allowNullPropertyValues = configuration.getBoolean(GREMLIN_TINKERGRAPH_ALLOW_NULL_PROPERTY_VALUES, true); graphLocation = configuration.getString(GREMLIN_TINKERGRAPH_GRAPH_LOCATION, null); graphFormat = configuration.getString(GREMLIN_TINKERGRAPH_GRAPH_FORMAT, null); @@ -158,7 +161,7 @@ public final class TinkerGraph implements Graph { @Override public Vertex addVertex(final Object... keyValues) { - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(allowNullPropertyValues, keyValues); Object idValue = vertexIdManager.convert(ElementHelper.getIdValue(keyValues).orElse(null)); final String label = ElementHelper.getLabelValue(keyValues).orElse(Vertex.DEFAULT_LABEL); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerHelper.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerHelper.java index 4ed3fdd..2b1a928 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerHelper.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerHelper.java @@ -49,7 +49,7 @@ public final class TinkerHelper { protected static Edge addEdge(final TinkerGraph graph, final TinkerVertex outVertex, final TinkerVertex inVertex, final String label, final Object... keyValues) { ElementHelper.validateLabel(label); - ElementHelper.legalPropertyKeyValueArray(keyValues); + ElementHelper.legalPropertyKeyValueArray(graph.features().edge().supportsNullPropertyValues(), keyValues); Object idValue = graph.edgeIdManager.convert(ElementHelper.getIdValue(keyValues).orElse(null)); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java index f5872cf..75924da 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerIndex.java @@ -49,7 +49,7 @@ final class TinkerIndex<T extends Element> { protected void put(final String key, final Object value, final T element) { Map<Object, Set<T>> keyMap = this.index.get(key); if (null == keyMap) { - this.index.putIfAbsent(key, new ConcurrentHashMap<Object, Set<T>>()); + this.index.putIfAbsent(key, new ConcurrentHashMap<>()); keyMap = this.index.get(key); } Set<T> objects = keyMap.get(value); @@ -65,7 +65,7 @@ final class TinkerIndex<T extends Element> { if (null == keyMap) { return Collections.emptyList(); } else { - Set<T> set = keyMap.get(value); + Set<T> set = keyMap.get(indexable(value)); if (null == set) return Collections.emptyList(); else @@ -78,7 +78,7 @@ final class TinkerIndex<T extends Element> { if (null == keyMap) { return 0; } else { - Set<T> set = keyMap.get(value); + final Set<T> set = keyMap.get(indexable(value)); if (null == set) return 0; else @@ -89,7 +89,7 @@ final class TinkerIndex<T extends Element> { public void remove(final String key, final Object value, final T element) { final Map<Object, Set<T>> keyMap = this.index.get(key); if (null != keyMap) { - Set<T> objects = keyMap.get(value); + final Set<T> objects = keyMap.get(indexable(value)); if (null != objects) { objects.remove(element); if (objects.size() == 0) { @@ -111,17 +111,11 @@ final class TinkerIndex<T extends Element> { public void autoUpdate(final String key, final Object newValue, final Object oldValue, final T element) { if (this.indexedKeys.contains(key)) { - if (oldValue != null) - this.remove(key, oldValue, element); + this.remove(key, oldValue, element); this.put(key, newValue, element); } } - public void autoRemove(final String key, final Object oldValue, final T element) { - if (this.indexedKeys.contains(key)) - this.remove(key, oldValue, element); - } - public void createKeyIndex(final String key) { if (null == key) throw Graph.Exceptions.argumentCanNotBeNull("key"); @@ -133,8 +127,8 @@ final class TinkerIndex<T extends Element> { this.indexedKeys.add(key); (Vertex.class.isAssignableFrom(this.indexClass) ? - this.graph.vertices.values().<T>parallelStream() : - this.graph.edges.values().<T>parallelStream()) + this.graph.vertices.values().parallelStream() : + this.graph.edges.values().parallelStream()) .map(e -> new Object[]{((T) e).property(key), e}) .filter(a -> ((Property) a[0]).isPresent()) .forEach(a -> this.put(key, ((Property) a[0]).value(), (T) a[1])); @@ -147,7 +141,35 @@ final class TinkerIndex<T extends Element> { this.indexedKeys.remove(key); } + /** + * Provides a way for an index to have a {@code null} value as {@code ConcurrentHashMap} will not allow a + * {@code null} key. + */ + public static Object indexable(final Object obj) { + return null == obj ? IndexedNull.instance() : obj; + } + public Set<String> getIndexedKeys() { return this.indexedKeys; } + + public static final class IndexedNull { + private static final IndexedNull inst = new IndexedNull(); + + private IndexedNull() {} + + static IndexedNull instance() { + return inst; + } + + @Override + public int hashCode() { + return 751912123; + } + + @Override + public boolean equals(final Object o) { + return o instanceof IndexedNull; + } + } } diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerProperty.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerProperty.java index 0f7077b..db40346 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerProperty.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerProperty.java @@ -54,9 +54,12 @@ public final class TinkerProperty<V> implements Property<V> { return this.value; } + /** + * The existence of this object implies the property is present, thus even a {@code null} value means "present". + */ @Override public boolean isPresent() { - return null != this.value; + return true; } @Override 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 be08137..7fe7ce0 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 @@ -46,10 +46,12 @@ public final class TinkerVertex extends TinkerElement implements Vertex { protected Map<String, Set<Edge>> outEdges; protected Map<String, Set<Edge>> inEdges; private final TinkerGraph graph; + private boolean allowNullPropertyValues; protected TinkerVertex(final Object id, final String label, final TinkerGraph graph) { super(id, label); this.graph = graph; + this.allowNullPropertyValues = graph.features().vertex().supportsNullPropertyValues(); } @Override @@ -83,8 +85,8 @@ public final class TinkerVertex extends TinkerElement implements Vertex { @Override public <V> VertexProperty<V> property(final VertexProperty.Cardinality cardinality, final String key, final V value, final Object... keyValues) { if (this.removed) throw elementAlreadyRemoved(Vertex.class, id); - ElementHelper.legalPropertyKeyValueArray(keyValues); - ElementHelper.validateProperty(key, value); + ElementHelper.legalPropertyKeyValueArray(allowNullPropertyValues, keyValues); + ElementHelper.validateProperty(allowNullPropertyValues, key, value); 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(); diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertexProperty.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertexProperty.java index 3ca871a..f01a3f2 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertexProperty.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerVertexProperty.java @@ -45,6 +45,7 @@ public class TinkerVertexProperty<V> extends TinkerElement implements VertexProp private final TinkerVertex vertex; private final String key; private final V value; + private final boolean allowNullPropertyValues; /** * This constructor will not validate the ID type against the {@link Graph}. It will always just use a @@ -53,10 +54,11 @@ public class TinkerVertexProperty<V> extends TinkerElement implements VertexProp */ public TinkerVertexProperty(final TinkerVertex vertex, final String key, final V value, final Object... propertyKeyValues) { super(((TinkerGraph) vertex.graph()).vertexPropertyIdManager.getNextId((TinkerGraph) vertex.graph()), key); + this.allowNullPropertyValues = vertex.graph().features().vertex().properties().supportsNullPropertyValues(); this.vertex = vertex; this.key = key; this.value = value; - ElementHelper.legalPropertyKeyValueArray(propertyKeyValues); + ElementHelper.legalPropertyKeyValueArray(allowNullPropertyValues, propertyKeyValues); ElementHelper.attachProperties(this, propertyKeyValues); } @@ -66,10 +68,11 @@ public class TinkerVertexProperty<V> extends TinkerElement implements VertexProp */ public TinkerVertexProperty(final Object id, final TinkerVertex vertex, final String key, final V value, final Object... propertyKeyValues) { super(id, key); + this.allowNullPropertyValues = vertex.graph().features().vertex().properties().supportsNullPropertyValues(); this.vertex = vertex; this.key = key; this.value = value; - ElementHelper.legalPropertyKeyValueArray(propertyKeyValues); + ElementHelper.legalPropertyKeyValueArray(allowNullPropertyValues, propertyKeyValues); ElementHelper.attachProperties(this, propertyKeyValues); }