You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ke...@apache.org on 2025/04/14 18:45:47 UTC
(tinkerpop) branch TINKERPOP-2974 updated (b0e40b30a8 -> 4826299810)
This is an automated email from the ASF dual-hosted git repository.
kenhuuu pushed a change to branch TINKERPOP-2974
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
discard b0e40b30a8 abc
discard 8ac68a0cca update
discard d210608fd7 TINKERPOP-2974 Change PropertyMap to only allow single by() if order not guaranteed
add 619e1f7b95 TINKERPOP-3047 Keywords are allowed as Map keys in grammar.
add a0c5851521 Backport OffsetDateTime changes from 4.0 (#3090)
new c1c9c2362f allow feature test to assert traversal construction exception
new 4826299810 TINKERPOP-2974 Change PropertyMap to only allow single by() if order not guaranteed
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (b0e40b30a8)
\
N -- N -- N refs/heads/TINKERPOP-2974 (4826299810)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGELOG.asciidoc | 4 +
docs/src/reference/the-traversal.asciidoc | 7 +-
docs/src/upgrade/release-3.8.x.asciidoc | 92 +-
.../tinkerpop/gremlin/jsr223/CoreImports.java | 1 +
.../gremlin/language/grammar/ArgumentVisitor.java | 228 +--
.../grammar/DefaultGremlinBaseVisitor.java | 66 +-
.../language/grammar/GenericLiteralVisitor.java | 67 +-
.../language/grammar/GremlinQueryParser.java | 2 -
.../language/grammar/TraversalEnumParser.java | 10 +
.../language/grammar/TraversalMethodVisitor.java | 287 ++--
.../grammar/TraversalPredicateVisitor.java | 16 +-
.../grammar/TraversalSourceSelfMethodVisitor.java | 4 +-
.../grammar/TraversalSourceSpawnMethodVisitor.java | 2 +-
.../translator/AbstractTranslateVisitor.java | 16 +-
.../translator/AnonymizedTranslatorVisitor.java | 4 +-
.../translator/DotNetTranslateVisitor.java | 129 +-
.../language/translator/GoTranslateVisitor.java | 58 +-
.../translator/GroovyTranslateVisitor.java | 8 +-
.../language/translator/JavaTranslateVisitor.java | 35 +-
.../translator/JavascriptTranslateVisitor.java | 53 +-
.../translator/PythonTranslateVisitor.java | 53 +-
.../language/translator/TranslateVisitor.java | 171 +--
.../gremlin/process/traversal/Translator.java | 9 +
.../traversal/dsl/graph/GraphTraversal.java | 28 +-
.../gremlin/process/traversal/dsl/graph/__.java | 18 +-
.../process/traversal/step/map/AsDateStep.java | 29 +-
.../process/traversal/step/map/DateAddStep.java | 60 +-
.../process/traversal/step/map/DateDiffStep.java | 61 +-
.../traversal/step/sideEffect/AddPropertyStep.java | 12 +-
.../translator/AnonymizingTypeTranslator.java | 8 +
.../traversal/translator/DotNetTranslator.java | 6 +
.../traversal/translator/GolangTranslator.java | 14 +
.../traversal/translator/GroovyTranslator.java | 11 +
.../traversal/translator/JavascriptTranslator.java | 6 +
.../traversal/translator/PythonTranslator.java | 8 +
.../tinkerpop/gremlin/util/DatetimeHelper.java | 32 +-
.../language/grammar/ArgumentVisitorTest.java | 103 +-
.../grammar/GeneralLiteralVisitorTest.java | 108 +-
.../language/grammar/TraversalEnumParserTest.java | 4 +-
.../grammar/TraversalPredicateVisitorTest.java | 2 +-
.../language/translator/GremlinTranslatorTest.java | 23 +-
.../process/traversal/step/map/AsDateStepTest.java | 26 +-
.../traversal/step/map/DateAddStepTest.java | 62 +-
.../traversal/step/map/DateDiffStepTest.java | 62 +-
.../translator/AnonymizingTypeTranslatorTest.java | 9 +
.../traversal/translator/DotNetTranslatorTest.java | 9 +-
.../traversal/translator/GolangTranslatorTest.java | 15 +
.../traversal/translator/GroovyTranslatorTest.java | 15 +-
.../translator/JavascriptTranslatorTest.java | 9 +-
.../traversal/translator/PythonTranslatorTest.java | 15 +
.../tinkerpop/gremlin/util/DatetimeHelperTest.java | 54 +-
.../Structure/IO/GraphBinary/DataType.cs | 1 +
.../IO/GraphBinary/TypeSerializerRegistry.cs | 7 +-
...meOffsetSerializer.cs => DateTimeSerializer.cs} | 10 +-
.../GraphBinary/Types/OffsetDateTimeSerializer.cs | 80 +
.../Structure/IO/GraphSON/GraphSONReader.cs | 1 +
.../Structure/IO/GraphSON/GraphSONWriter.cs | 2 +-
...serializer.cs => OffsetDateTimeDeserializer.cs} | 9 +-
...teSerializer.cs => OffsetDateTimeSerializer.cs} | 4 +-
.../Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 31 +-
.../Structure/IO/GraphSON/GraphSONWriterTests.cs | 2 +-
gremlin-go/driver/cucumber/cucumberSteps_test.go | 7 +-
gremlin-go/driver/cucumber/gremlin.go | 23 +-
gremlin-go/driver/graphBinary.go | 73 +-
gremlin-go/driver/graphBinary_test.go | 33 +
gremlin-go/driver/serializer.go | 8 +-
.../lib/structure/io/binary/GraphBinary.js | 1 +
.../structure/io/binary/internals/AnySerializer.js | 1 +
...teSerializer.js => OffsetDateTimeSerializer.js} | 64 +-
.../lib/structure/io/graph-serializer.js | 3 +-
.../lib/structure/io/type-serializers.js | 13 +-
.../gremlin-javascript/test/cucumber/gremlin.js | 23 +-
.../test/unit/graphbinary/AnySerializer-test.js | 16 +-
.../gremlin-javascript/test/unit/graphson-test.js | 4 +-
gremlin-language/src/main/antlr4/Gremlin.g4 | 1594 +++++++++++++-------
.../gremlin/language/corpus/GrammarReader.java | 63 +
.../language/grammar/AbstractGrammarTest.java | 51 +-
.../gremlin/language/grammar/BasicGrammarTest.java | 2 +-
.../language/grammar/NegativeGrammarTest.java | 34 +-
.../language/grammar/ReferenceGrammarTest.java | 89 +-
.../src/test/resources/gremlin-values.txt | 415 +++++
.../test/resources/incorrect-gremlin-values.txt | 19 +
.../gremlin_python/structure/io/graphbinaryV1.py | 40 +-
.../gremlin_python/structure/io/graphsonV2d0.py | 19 +-
.../gremlin_python/structure/io/graphsonV3d0.py | 19 +-
.../src/main/python/radish/feature_steps.py | 2 +-
gremlin-python/src/main/python/radish/gremlin.py | 23 +-
.../main/python/tests/process/test_traversal.py | 3 +-
.../tests/structure/io/test_functionalityio.py | 14 +
.../tests/structure/io/test_graphbinaryV1.py | 32 +-
.../python/tests/structure/io/test_graphsonV2d0.py | 24 +-
.../python/tests/structure/io/test_graphsonV3d0.py | 24 +-
.../server/GremlinServerHttpIntegrateTest.java | 2 +-
.../process/traversal/step/OrderabilityTest.java | 13 +-
.../gremlin/test/features/filter/Dedup.feature | 17 -
.../test/features/integrated/SeedStrategy.feature | 13 -
.../gremlin/test/features/map/AsDate.feature | 12 +
.../gremlin/test/features/map/Order.feature | 36 -
.../util/ser/binary/types/sample/SamplePerson.java | 8 +-
.../types/sample/SamplePersonSerializer.java | 4 +-
.../types/sample/SamplePersonSerializerTest.java | 5 +-
pom.xml | 3 +
.../tinkerpop/gremlin/tinkergraph/TinkerWorld.java | 4 +-
103 files changed, 3291 insertions(+), 1840 deletions(-)
rename gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/{DateTimeOffsetSerializer.cs => DateTimeSerializer.cs} (84%)
create mode 100644 gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/OffsetDateTimeSerializer.cs
copy gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphSON/{MergeDeserializer.cs => OffsetDateTimeDeserializer.cs} (79%)
rename gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphSON/{DateSerializer.cs => OffsetDateTimeSerializer.cs} (87%)
copy gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/{DateSerializer.js => OffsetDateTimeSerializer.js} (55%)
create mode 100644 gremlin-language/src/main/java/org/apache/tinkerpop/gremlin/language/corpus/GrammarReader.java
create mode 100644 gremlin-language/src/test/resources/gremlin-values.txt
create mode 100644 gremlin-language/src/test/resources/incorrect-gremlin-values.txt
(tinkerpop) 01/02: allow feature test to assert traversal construction exception
Posted by ke...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kenhuuu pushed a commit to branch TINKERPOP-2974
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit c1c9c2362fa2b15587d5b8a76a11792b8ebad2c8
Author: Ken Hu <10...@users.noreply.github.com>
AuthorDate: Mon Apr 14 11:41:53 2025 -0700
allow feature test to assert traversal construction exception
---
.../tinkerpop/gremlin/jsr223/JavaTranslator.java | 14 +++++++++++---
gremlin-go/driver/resultSet.go | 12 ++++++++----
gremlin-go/driver/resultSet_test.go | 20 ++++++++++++++++----
gremlin-go/driver/traversal.go | 17 ++++++++++++++---
.../tinkerpop/gremlin/features/StepDefinition.java | 8 ++++++++
5 files changed, 57 insertions(+), 14 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 7689b7d60b..fa56a2cd4f 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
@@ -241,6 +241,7 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
"Could not locate method", delegate, methodName, argumentsCopy));
}
+ boolean found = false;
try {
for (final ReflectedMethod methodx : methodCache.get(methodName)) {
final Method method = methodx.method;
@@ -248,7 +249,7 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
final Parameter[] parameters = methodx.parameters;
if (parameters.length == argumentsCopy.length || methodx.hasVarArgs) {
final Object[] newArguments = new Object[parameters.length];
- boolean found = true;
+ found = true;
for (int i = 0; i < parameters.length; i++) {
if (parameters[i].isVarArgs()) {
final Class<?> parameterClass = parameters[i].getType().getComponentType();
@@ -323,8 +324,15 @@ public final class JavaTranslator<S extends TraversalSource, T extends Traversal
}
}
} catch (final Throwable e) {
- throw new IllegalStateException(generateMethodNotFoundMessage(
- e.getMessage(), null, methodName, argumentsCopy), e);
+ if (found && (e instanceof InvocationTargetException)) {
+ // get the target exception out for a better message, otherwise, it will be null.
+ final InvocationTargetException ite = (InvocationTargetException) e;
+ throw new IllegalStateException(generateMethodNotFoundMessage(
+ ite.getTargetException().getMessage(), null, methodName, argumentsCopy), ite.getTargetException());
+ } else {
+ throw new IllegalStateException(generateMethodNotFoundMessage(
+ e.getMessage(), null, methodName, argumentsCopy), e);
+ }
}
// if it got down here then the method was in the cache but it was never called as it could not be found
diff --git a/gremlin-go/driver/resultSet.go b/gremlin-go/driver/resultSet.go
index 73269deddd..4450cdfb67 100644
--- a/gremlin-go/driver/resultSet.go
+++ b/gremlin-go/driver/resultSet.go
@@ -33,7 +33,7 @@ type ResultSet interface {
setStatusAttributes(statusAttributes map[string]interface{})
GetStatusAttributes() map[string]interface{}
GetRequestID() string
- IsEmpty() bool
+ IsEmpty() (bool, error)
Close()
unlockedClose()
Channel() chan *Result
@@ -78,17 +78,21 @@ func (channelResultSet *channelResultSet) setError(err error) {
}
// IsEmpty returns true when the channelResultSet is empty.
-func (channelResultSet *channelResultSet) IsEmpty() bool {
+func (channelResultSet *channelResultSet) IsEmpty() (bool, error) {
channelResultSet.channelMutex.Lock()
// If our channel is empty and we have no data in it, wait for signal that the state has been updated.
if len(channelResultSet.channel) != 0 {
// Channel is not empty.
channelResultSet.channelMutex.Unlock()
- return false
+ return false, nil
+ } else if channelResultSet.err != nil {
+ // Channel has an error.
+ channelResultSet.channelMutex.Unlock()
+ return true, channelResultSet.err
} else if channelResultSet.closed {
// Channel is empty and closed.
channelResultSet.channelMutex.Unlock()
- return true
+ return true, nil
} else {
// Channel is empty and not closed. Need to wait for signal that state has changed, otherwise
// we do not know if it is empty or not.
diff --git a/gremlin-go/driver/resultSet_test.go b/gremlin-go/driver/resultSet_test.go
index df9752cac8..7e4a81c728 100644
--- a/gremlin-go/driver/resultSet_test.go
+++ b/gremlin-go/driver/resultSet_test.go
@@ -123,28 +123,36 @@ func TestChannelResultSet(t *testing.T) {
t.Run("Test ResultSet IsEmpty before signal.", func(t *testing.T) {
channelResultSet := newChannelResultSet(mockID, getSyncMap())
go closeAfterTime(500, channelResultSet)
- empty := channelResultSet.IsEmpty()
+ empty, _ := channelResultSet.IsEmpty()
assert.True(t, empty)
})
t.Run("Test ResultSet IsEmpty after signal.", func(t *testing.T) {
channelResultSet := newChannelResultSet(mockID, getSyncMap())
channelResultSet.Close()
- empty := channelResultSet.IsEmpty()
+ empty, _ := channelResultSet.IsEmpty()
assert.True(t, empty)
})
t.Run("Test ResultSet IsEmpty after close.", func(t *testing.T) {
channelResultSet := newChannelResultSet(mockID, getSyncMap())
go addAfterTime(500, channelResultSet)
- empty := channelResultSet.IsEmpty()
+ empty, _ := channelResultSet.IsEmpty()
assert.False(t, empty)
channelResultSet.One()
go closeAfterTime(500, channelResultSet)
- empty = channelResultSet.IsEmpty()
+ empty, _ = channelResultSet.IsEmpty()
assert.True(t, empty)
})
+ t.Run("Test ResultSet IsEmpty handles error.", func(t *testing.T) {
+ channelResultSet := newChannelResultSet(mockID, getSyncMap())
+ setError(channelResultSet, fmt.Errorf("injected %s error", "ResultSet"))
+ empty, err := channelResultSet.IsEmpty()
+ assert.True(t, empty)
+ assert.True(t, err != nil)
+ })
+
t.Run("Test ResultSet removes self from container.", func(t *testing.T) {
container := getSyncMap()
assert.Equal(t, 0, container.size())
@@ -172,6 +180,10 @@ func AddResults(resultSet ResultSet, count int) {
}
}
+func setError(resultSet ResultSet, resultError error) {
+ resultSet.setError(resultError)
+}
+
func closeAfterTime(millisecondTicks time.Duration, resultSet ResultSet) {
time.Sleep(millisecondTicks * time.Millisecond)
resultSet.Close()
diff --git a/gremlin-go/driver/traversal.go b/gremlin-go/driver/traversal.go
index a6011eab53..037ea2f269 100644
--- a/gremlin-go/driver/traversal.go
+++ b/gremlin-go/driver/traversal.go
@@ -102,7 +102,13 @@ func (t *Traversal) HasNext() (bool, error) {
if err != nil {
return false, err
}
- return !results.IsEmpty(), nil
+
+ isEmpty, emptyErr := results.IsEmpty()
+ if emptyErr != nil {
+ return false, emptyErr
+ } else {
+ return !isEmpty, nil
+ }
}
// Next returns next result.
@@ -111,9 +117,14 @@ func (t *Traversal) Next() (*Result, error) {
if err != nil {
return nil, err
}
- if results.IsEmpty() {
+
+ isEmpty, emptyErr := results.IsEmpty()
+ if emptyErr != nil {
+ return nil, emptyErr
+ } else if isEmpty {
return nil, newError(err0903NextNoResultsLeftError)
}
+
result, _, err := results.One()
return result, err
}
@@ -718,7 +729,7 @@ func ParseBigDecimal(strValue string) *BigDecimal {
}
// resolve big int
unscaled := new(big.Int)
- unscaled, ok = unscaled.SetString(strings.Replace(bfVal.String(), ".", "",-1), 10)
+ unscaled, ok = unscaled.SetString(strings.Replace(bfVal.String(), ".", "", -1), 10)
if !ok {
return nil
}
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
index 13c6debeac..f14a1a5235 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java
@@ -307,6 +307,10 @@ public final class StepDefinition {
@When("iterated to list")
public void iteratedToList() {
+ if ((null == traversal) && (error != null)) {
+ return; // Error probably occurred during traversal construction. Skip to prevent NPE.
+ }
+
try {
result = traversal.toList();
} catch (Exception ex) {
@@ -316,6 +320,10 @@ public final class StepDefinition {
@When("iterated next")
public void iteratedNext() {
+ if ((null == traversal) && (error != null)) {
+ return; // Error probably occurred during traversal construction. Skip to prevent NPE.
+ }
+
try {
result = traversal.next();
} catch (Exception ex) {
(tinkerpop) 02/02: TINKERPOP-2974 Change PropertyMap to only allow single by() if order not guaranteed
Posted by ke...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kenhuuu pushed a commit to branch TINKERPOP-2974
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 4826299810002fba487a9d05abaf082aec623ddb
Author: Ken Hu <10...@users.noreply.github.com>
AuthorDate: Mon Apr 14 11:43:16 2025 -0700
TINKERPOP-2974 Change PropertyMap to only allow single by() if order not guaranteed
Not all implementations of Gremlin are going to be able to iterate the
map produced by PropertyMap step in order. This means that multiple
by()s can't be applied in order. Therefore, for those implementations,
an exception should be thrown instead.
---
docs/src/dev/provider/gremlin-semantics.asciidoc | 37 +++++++++++++++++++
docs/src/upgrade/release-3.8.x.asciidoc | 15 ++++++++
.../traversal/step/map/PropertyMapStep.java | 41 ++++++++++++----------
.../process/traversal/step/map/ValueMapTest.java | 22 ++++++------
.../gremlin/test/features/map/ValueMap.feature | 21 +++--------
.../structure/TinkerShuffleGraphTest.java | 15 ++++++++
6 files changed, 103 insertions(+), 48 deletions(-)
diff --git a/docs/src/dev/provider/gremlin-semantics.asciidoc b/docs/src/dev/provider/gremlin-semantics.asciidoc
index d16f0a9972..01ff54125c 100644
--- a/docs/src/dev/provider/gremlin-semantics.asciidoc
+++ b/docs/src/dev/provider/gremlin-semantics.asciidoc
@@ -1615,3 +1615,40 @@ Null values from the incoming traverser are not processed and remain as null whe
See: link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TrimGlobalStep.java[source],
link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TrimLocalStep.java[source (local)],
link:https://tinkerpop.apache.org/docs/x.y.z/reference/#trim-step[reference]
+
+[[valueMap-step]]
+=== valueMap()
+
+*Description:* Removes repeatedly seen results from the Traversal Stream.
+
+*Syntax:* `valueMap(String... propertyKeys)` | `valueMap(boolean includeTokens, String... propertyKeys)`
+
+[width="100%",options="header"]
+|=========================================================
+|Start Step |Mid Step |Modulated |Domain |Range
+|N |Y |`by()`/`with()` |`Element` |`Map`
+|=========================================================
+
+*Arguments:*
+
+* `includeTokens` - Determines if the result map should contain entries from `T` (such as `label` and `id`).
+
+* `propertyKeys` - If `valueMap()` is provided a list of propertyKeys, then the result map will only contain entries
+specified by propertyKeys. If the list is empty, then all properties are included.
+
+*Modulation:*
+
+* `by(Traversal)` - Only a single by() modulator is allowed and it applies to all map values. An attempt to add more
+than one by() will result in an Exception that contains the message "valueMap()/propertyMap() step can only have one by
+modulator".
+* `with(key, value)` - Determines which optional entries to add to the map. The key is "tinkerpop.valueMap.tokens" and
+the possible values are:
+** 0 for no tokens
+** 1 for including id (Element)
+** 2 for including label (Vertex/Edge)
+** 4 for including keys (VertexProperty)
+** 8 for including values (VertexProperty)
+** 15 for including all
+
+See: link:https://github.com/apache/tinkerpop/tree/x.y.z/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java[source],
+link:https://tinkerpop.apache.org/docs/x.y.z/reference/#valuemap-step[reference]
diff --git a/docs/src/upgrade/release-3.8.x.asciidoc b/docs/src/upgrade/release-3.8.x.asciidoc
index b241931d94..021d8c1787 100644
--- a/docs/src/upgrade/release-3.8.x.asciidoc
+++ b/docs/src/upgrade/release-3.8.x.asciidoc
@@ -284,6 +284,13 @@ gremlin> g.V().has("person","name",P.within("vadas","peter")).group().by().by(__
See: link:https://issues.apache.org/jira/browse/TINKERPOP-2971[TINKERPOP-2971]
+==== valueMap() and propertyMap() By Modulation Semantics
+
+The `valueMap()` and `propertyMap()`steps have been changed to throw an error if multiple `by()` modulators are applied.
+The previous behavior attempted to round-robin the by() but this wasn't possible for all providers.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2974[TINKERPOP-2974]
+
==== groupCount() By Modulation Semantics
The `groupCount()` step has been changed to throw an error if multiple `by()` modulators are applied. The previous
@@ -302,6 +309,14 @@ The default implementation for date type in Gremlin is now changed from the depr
This may impact providers who use TinkerGraph or whose implementation store dates as `java.util.Date`. While steps will support `Date`, all date manipulations will output `OffsetDateTime`. If a user had persisted `Date` objects in the database, upgrading to 3.8 may lead to the database having both types stored. It is recommended for users to perform transformation of `Date` to `OffsetDateTime` to retain consistency.
+===== Semantics change for PropertyMapStep
+
+The semantics have changed for the handling of by modulators to the `valueMap` and `propertyMap` steps. Only one by
+modulator is required to be accepted and an exception should be thrown when there are more than one by() modulators.
+The exception thrown should contain the following: "valueMap()/propertyMap() step can only have one by modulator".
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2974[TINKERPOP-2974]
+
==== Graph Driver Providers
==== The Switch from Date to OffsetDateTime
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
index 812f13b4d2..3534b7e313 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PropertyMapStep.java
@@ -27,7 +27,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters;
import org.apache.tinkerpop.gremlin.process.traversal.step.util.WithOptions;
import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalProduct;
-import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalRing;
import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.tinkerpop.gremlin.structure.Property;
@@ -61,18 +60,14 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
protected Traversal.Admin<Element, ? extends Property> propertyTraversal;
protected Parameters parameters = new Parameters();
- protected TraversalRing<K, E> traversalRing;
+ protected Traversal.Admin<K, E> valueTraversal;
- public PropertyMapStep(final Traversal.Admin traversal, final PropertyType propertyType, TraversalRing<K, E> traversalRing, final String... propertyKeys) {
+ public PropertyMapStep(final Traversal.Admin traversal, final PropertyType propertyType, final String... propertyKeys) {
super(traversal);
this.propertyKeys = propertyKeys;
this.returnType = propertyType;
this.propertyTraversal = null;
- this.traversalRing = traversalRing;
- }
-
- public PropertyMapStep(final Traversal.Admin traversal, final PropertyType propertyType, final String... propertyKeys) {
- this(traversal, propertyType, new TraversalRing<>(), propertyKeys);
+ this.valueTraversal = null;
}
public PropertyMapStep(final Traversal.Admin traversal, final int options, final PropertyType propertyType, final String... propertyKeys) {
@@ -117,13 +112,18 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
final List<Traversal.Admin<K, E>> result = new ArrayList<>();
if (null != this.propertyTraversal)
result.add((Traversal.Admin) propertyTraversal);
- result.addAll(this.traversalRing.getTraversals());
+ if (null != this.valueTraversal)
+ result.add(this.valueTraversal);
return Collections.unmodifiableList(result);
}
@Override
public void modulateBy(final Traversal.Admin<?, ?> selectTraversal) {
- this.traversalRing.addTraversal(this.integrateChild(selectTraversal));
+ if (null == valueTraversal) {
+ this.valueTraversal = this.integrateChild(selectTraversal);
+ } else {
+ throw new IllegalArgumentException("valueMap()/propertyMap() step can only have one by modulator");
+ }
}
public void setPropertyTraversal(final Traversal.Admin<Element, ? extends Property> propertyTraversal) {
@@ -144,7 +144,7 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
public String toString() {
return StringFactory.stepString(this, Arrays.asList(this.propertyKeys),
- this.traversalRing, this.returnType.name().toLowerCase());
+ this.valueTraversal, this.returnType.name().toLowerCase());
}
@Override
@@ -152,7 +152,8 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
final PropertyMapStep<K,E> clone = (PropertyMapStep<K,E>) super.clone();
if (null != this.propertyTraversal)
clone.propertyTraversal = this.propertyTraversal.clone();
- clone.traversalRing = this.traversalRing.clone();
+ if (null != this.valueTraversal)
+ clone.valueTraversal = this.valueTraversal.clone();
return clone;
}
@@ -161,10 +162,12 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
int result = super.hashCode() ^ this.returnType.hashCode() ^ Integer.hashCode(this.tokens);
if (null != this.propertyTraversal)
result ^= this.propertyTraversal.hashCode();
+ if (null != this.valueTraversal)
+ result ^= this.valueTraversal.hashCode();
for (final String propertyKey : this.propertyKeys) {
result ^= Objects.hashCode(propertyKey);
}
- return result ^ this.traversalRing.hashCode();
+ return result;
}
@Override
@@ -172,7 +175,8 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
super.setTraversal(parentTraversal);
if (null != this.propertyTraversal)
this.integrateChild(this.propertyTraversal);
- this.traversalRing.getTraversals().forEach(this::integrateChild);
+ if (null != this.valueTraversal)
+ integrateChild(this.valueTraversal);
}
@Override
@@ -241,22 +245,21 @@ public class PropertyMapStep<K,E> extends ScalarMapStep<Element, Map<K, E>>
}
protected void applyTraversalRingToMap(Map<Object, Object> map){
- if (!traversalRing.isEmpty()) {
+ if (this.valueTraversal != null) {
// will cop a ConcurrentModification if a key is dropped so need this little copy here
final List<Object> keys = new ArrayList<>(map.keySet());
for (final Object key : keys) {
map.compute(key, (k, v) -> {
- final TraversalProduct product = TraversalUtil.produce(v, (Traversal.Admin) this.traversalRing.next());
+ final TraversalProduct product = TraversalUtil.produce(v, (Traversal.Admin) this.valueTraversal);
// compute() should take the null and remove the key
return product.isProductive() ? product.get() : null;
});
}
- this.traversalRing.reset();
}
}
- public TraversalRing<K, E> getTraversalRing() {
- return traversalRing;
+ public Traversal.Admin<K, E> getValueTraversal() {
+ return this.valueTraversal;
}
}
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ValueMapTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ValueMapTest.java
index dac12d4131..f54df0c5ad 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ValueMapTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/ValueMapTest.java
@@ -35,15 +35,18 @@ import java.util.Map;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.CREW;
import static org.apache.tinkerpop.gremlin.LoadGraphWith.GraphData.MODERN;
+import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.core.Is.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.apache.tinkerpop.gremlin.structure.T.id;
import static org.apache.tinkerpop.gremlin.structure.T.label;
+import static org.hamcrest.core.StringEndsWith.endsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
/**
* @author Marko A. Rodriguez (http://markorodriguez.com)
@@ -345,19 +348,14 @@ public abstract class ValueMapTest extends AbstractGremlinProcessTest {
@Test
@LoadGraphWith(CREW)
public void g_VX1X_valueMapXname_locationX_byXunfoldX_by() {
- final Traversal<Vertex,Map<Object,Object>> traversal = get_g_VX1X_valueMapXname_locationX_byXunfoldX_by(convertToVertexId("marko"));
- printTraversalForm(traversal);
- assertTrue(traversal.hasNext());
- final Map<Object, Object> values = traversal.next();
- assertFalse(traversal.hasNext());
- assertTrue("name value should be a String", values.containsKey("name") && String.class.equals(values.get("name").getClass()));
- assertTrue("location value should be a List", values.containsKey("location") && List.class.isAssignableFrom(values.get("location").getClass()));
- assertEquals("marko", values.get("name"));
- final List locations = (List) values.get("location");
- for (final String location : Arrays.asList("san diego", "santa cruz", "brussels", "santa fe")) {
- assertTrue(locations.contains(location));
+ try {
+ final Traversal<Vertex,Map<Object,Object>> traversal = get_g_VX1X_valueMapXname_locationX_byXunfoldX_by(convertToVertexId("marko"));
+ printTraversalForm(traversal);
+ assertFalse(traversal.hasNext());
+ fail("Should have failed as multiple by() not allowed for valueMap");
+ } catch (Exception ex) {
+ assertThat(ex.getMessage(), containsString("valueMap()/propertyMap() step can only have one by modulator"));
}
- assertEquals(2, values.size());
}
public static class Traversals extends ValueMapTest {
diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/ValueMap.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/ValueMap.feature
index 65e495ffca..6427aaf061 100644
--- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/ValueMap.feature
+++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/ValueMap.feature
@@ -184,9 +184,7 @@ Feature: Step - valueMap()
| m[{"name": ["josh"], "age": [32], "t[label]":"person", "t[id]":"v[josh].id"}] |
| m[{"name": ["peter"], "age": [35], "t[label]":"person", "t[id]":"v[peter].id"}] |
-# NOTE: Insertion order is required for this test due to TINKERPOP-2974.
-# This annotation should be removed once the bug is fixed
- @MultiProperties @MetaProperties @InsertionOrderingRequired
+ @MultiProperties @MetaProperties
Scenario: g_VX1X_valueMapXname_locationX_byXunfoldX_by
Given the crew graph
And using the parameter vid1 defined as "v[marko].id"
@@ -194,10 +192,8 @@ Feature: Step - valueMap()
"""
g.V(vid1).valueMap("name", "location").by(__.unfold()).by()
"""
- When iterated to list
- Then the result should be unordered
- | result |
- | m[{"name": "marko", "location": ["san diego", "santa cruz", "brussels", "santa fe"]}] |
+ When iterated next
+ Then the traversal will raise an error with message containing text of "step can only have one by modulator"
Scenario: g_V_valueMapXname_age_nullX
Given the modern graph
@@ -215,8 +211,6 @@ Feature: Step - valueMap()
| m[{"name": ["lop"]}] |
| m[{"name": ["ripple"]}] |
-# NOTE: Insertion order is required for this test due to TINKERPOP-2974.
-# This annotation should be removed once the bug is fixed
@InsertionOrderingRequired
Scenario: g_V_valueMapXname_ageX_byXisXxXXbyXunfoldX
Given the modern graph
@@ -225,11 +219,4 @@ Feature: Step - valueMap()
g.V().valueMap("name", "age").by(__.is("x")).by(__.unfold())
"""
When iterated to list
- Then the result should be unordered
- | result |
- | m[{"age": 29}] |
- | m[{"age": 32}] |
- | m[{"age": 35}] |
- | m[{"age": 27}] |
- | m[{}] |
- | m[{}] |
\ No newline at end of file
+ Then the traversal will raise an error with message containing text of "step can only have one by modulator"
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerShuffleGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerShuffleGraphTest.java
index 577b824898..dfcf67db4e 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerShuffleGraphTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerShuffleGraphTest.java
@@ -19,6 +19,7 @@
package org.apache.tinkerpop.gremlin.tinkergraph.structure;
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Property;
import org.apache.tinkerpop.gremlin.structure.Vertex;
@@ -37,6 +38,9 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* @author Cole Greer (https://github.com/Cole-Greer)
@@ -180,6 +184,17 @@ public class TinkerShuffleGraphTest {
assertThat(resultRegular, not(contains(resultShuffle)));
}
+ @Test
+ public void shouldThrowForPropertyMapTraversalWithMultipleBy() {
+ final GraphTraversalSource gShuff = createShuffleModern();
+ try {
+ gShuff.V().valueMap("name", "age").by().by(__.unfold());
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().contains(""));
+ }
+ }
+
private GraphTraversalSource createShuffleModern() {
TinkerShuffleGraph graph = TinkerShuffleGraph.open();
TinkerFactory.generateModern(graph);