TINKERPOP-3147 Prevent aggregate step from having multiple by modulators #3096
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.8-dev #3096 +/- ##
==========================================
Coverage ? 76.33%
Complexity ? 14013
==========================================
Files ? 1105
Lines ? 69743
Branches ? 7514
==========================================
Hits ? 53235
Misses ? 13752
Partials ? 2756 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -69,6 +69,10 @@ public String toString() { | |||
|
|
|||
| @Override | |||
| public void modulateBy(final Traversal.Admin<?, ?> aggregateTraversal) { | |||
| if (this.aggregateTraversal != null) | |||
| { | |||
There was a problem hiding this comment.
Nit: following existing syntax conventions this curly brace should be on the same line as the if statement
| @@ -87,6 +87,10 @@ public List<Traversal.Admin<S, Object>> getLocalChildren() { | |||
|
|
|||
| @Override | |||
| public void modulateBy(final Traversal.Admin<?, ?> storeTraversal) { | |||
| if (this.storeTraversal != null) | |||
| { | |||
There was a problem hiding this comment.
Nit: following existing syntax conventions this curly brace should be on the same line as the if statement
| g.V().aggregate("x").by("name").by("age").cap("x") | ||
| """ | ||
| When iterated to list | ||
| Then the traversal will raise an error |
There was a problem hiding this comment.
Now that #3094 is merged you should be able to check that the error message contains an expected phrase
CHANGELOG.asciidoc
Outdated
| @@ -24,7 +24,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima | |||
| === TinkerPop 3.8.0 (Release Date: NOT OFFICIALLY RELEASED YET) | |||
|
|
|||
| This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>. | |||
|
|
|||
| * Changed aggregate step to not allowing multiple by modulators. | |||
There was a problem hiding this comment.
Nit (grammar): 'not allow'
|
VOTE +1 (pending resolution to @andreachild's above comments) |
|
Note: This change should be reflected user upgrade docs. There are quite a few of these by-modulating changes going into 3.8 right now, perhaps they should all be consolidated into a single upgrade docs entry once all of that work is complete. |
TARGET 3.8-dev
https://issues.apache.org/jira/browse/TINKERPOP-3147
Prevent multiple by operators from being used for aggregate step by throwing IllegalStateException if modulateBy is called more than once.