This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit e7f0669f4acc2285659b569df2775511ff1838fd Author: Stephen Mallette <stepm...@amazon.com> AuthorDate: Wed Feb 17 13:49:15 2021 -0500 Improved error message Can't mutate graph elements T values CTR --- CHANGELOG.asciidoc | 1 + .../traversal/step/sideEffect/AddPropertyStep.java | 8 ++++- .../tinkergraph/structure/TinkerGraphTest.java | 35 +++++++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 61f293b..06777b3 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima [[release-3-4-11]] === TinkerPop 3.4.11 (Release Date: NOT OFFICIALLY RELEASED YET) +* Improved error message for `property(T,Object)` when mutating graph elements. * Added method caching for GraphSON 3.0 deserialization of `P` and `TextP` instances. * Fixed bug with Javascript Groovy `Translator` when generating Gremlin with multiple embedded traversals. * Modified Gremlin Server `Settings` to be more extensible allowing for custom options with the YAML parser. diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java index 0056507..cee4346 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java @@ -82,9 +82,15 @@ public class AddPropertyStep<S extends Element> extends SideEffectStep<S> @Override protected void sideEffect(final Traverser.Admin<S> traverser) { - final String key = (String) this.parameters.get(traverser, T.key, () -> { + final Object k = this.parameters.get(traverser, T.key, () -> { throw new IllegalStateException("The AddPropertyStep does not have a provided key: " + this); }).get(0); + + // T identifies immutable components of elements. only property keys can be modified. + if (k instanceof T) + throw new IllegalStateException(String.format("T.%s is immutable on existing elements", ((T) k).name())); + + final String key = (String) k; final Object value = this.parameters.get(traverser, T.value, () -> { throw new IllegalStateException("The AddPropertyStep does not have a provided value: " + this); }).get(0); diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java index a2208f5..329a648 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java @@ -27,7 +27,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.VerificationException; import org.apache.tinkerpop.gremlin.process.traversal.util.Metrics; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics; import org.apache.tinkerpop.gremlin.structure.Edge; @@ -713,6 +712,40 @@ public class TinkerGraphTest { } } + @Test + public void shouldProvideClearErrorWhenTryingToMutateT() { + final GraphTraversalSource g = TinkerGraph.open().traversal(); + g.addV("person").property(T.id, 100).iterate(); + + try { + g.V(100).property(T.label, "software").iterate(); + fail("Should have thrown an error"); + } catch (IllegalStateException ise) { + assertEquals("T.label is immutable on existing elements", ise.getMessage()); + } + + try { + g.V(100).property(T.id, 101).iterate(); + fail("Should have thrown an error"); + } catch (IllegalStateException ise) { + assertEquals("T.id is immutable on existing elements", ise.getMessage()); + } + + try { + g.V(100).property("name", "marko").property(T.label, "software").iterate(); + fail("Should have thrown an error"); + } catch (IllegalStateException ise) { + assertEquals("T.label is immutable on existing elements", ise.getMessage()); + } + + try { + g.V(100).property(T.id, 101).property("name", "marko").iterate(); + fail("Should have thrown an error"); + } catch (IllegalStateException ise) { + assertEquals("T.id is immutable on existing elements", ise.getMessage()); + } + } + /** * Coerces a {@code Color} to a {@link TinkerGraph} during serialization. Demonstrates how custom serializers * can be developed that can coerce one value to another during serialization.