You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2020/12/31 14:09:47 UTC
[tinkerpop] 01/01: TINKERPOP-2473 Prevented assignment of more than
one strategy of the same type.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2473
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 01691de876eda78fc9d15fe6929b12987f0f3c00
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Dec 31 09:06:57 2020 -0500
TINKERPOP-2473 Prevented assignment of more than one strategy of the same type.
Don't think that this is a major change in the sense that it's unlikely that anyone was relying on the old behavior. The code now just better enforces what was expected to happen.
---
CHANGELOG.asciidoc | 1 +
.../gremlin/process/traversal/TraversalSource.java | 2 +-
.../process/traversal/TraversalStrategies.java | 9 +++----
.../process/traversal/dsl/GremlinDslProcessor.java | 6 +++--
.../traversal/dsl/graph/GraphTraversalSource.java | 10 ++++----
.../traversal/util/DefaultTraversalStrategies.java | 3 ++-
.../util/DefaultTraversalStrategiesTest.java | 30 +++++++++++++++-------
7 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 191355f..ce89d50 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -28,6 +28,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
* Allowed the possibility for the propagation of `null` as a `Traverser` in Gremlin.
* Fixed a bug where spark-gremlin was not re-attaching properties when using `dedup()`.
* Ensured better consistency of the use of `null` as arguments to mutation steps.
+* Prevented `TraversalStrategy` instances from being added more than once.
* Allowed `property(T.label,Object)` to be used if no value was supplied to `addV(String)`.
* Allowed additional arguments to `Client.submit()` in Javascript driver to enable setting of parameters like `scriptEvaluationTimeout`.
* Gremlin.Net driver no longer supports skipping deserialization by default. Users can however create their own `IMessageSerializer` if they need this functionality.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
index fafef6c..31f0701 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalSource.java
@@ -367,7 +367,7 @@ public interface TraversalSource extends Cloneable, AutoCloseable {
return clone;
}
- public default Optional<Class> getAnonymousTraversalClass() {
+ public default Optional<Class<?>> getAnonymousTraversalClass() {
return Optional.empty();
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
index 63842b0..d2d04dc 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
@@ -97,8 +97,9 @@ public interface TraversalStrategies extends Serializable, Cloneable, Iterable<T
}
/**
- * Add all the provided {@link TraversalStrategy} instances to the current collection.
- * When all the provided strategies have been added, the collection is resorted.
+ * Add all the provided {@link TraversalStrategy} instances to the current collection. When all the provided
+ * strategies have been added, the collection is resorted. If a strategy class is found to already be defined, it
+ * is removed and replaced by the newly added one.
*
* @param strategies the traversal strategies to add
* @return the newly updated/sorted traversal strategies collection
@@ -136,7 +137,6 @@ public interface TraversalStrategies extends Serializable, Cloneable, Iterable<T
MultiMap.put(strategiesByCategory, s.getTraversalCategory(), s.getClass());
});
-
//Initialize all the dependencies
strategies.forEach(strategy -> {
strategy.applyPrior().forEach(s -> {
@@ -191,7 +191,6 @@ public interface TraversalStrategies extends Serializable, Cloneable, Iterable<T
+ seenStrategyClases + ']');
}
-
if (unprocessedStrategyClasses.contains(strategyClass)) {
seenStrategyClases.add(strategyClass);
for (Class<? extends TraversalStrategy> dependency : MultiMap.get(dependencyMap, strategyClass)) {
@@ -209,7 +208,7 @@ public interface TraversalStrategies extends Serializable, Cloneable, Iterable<T
* Keeps track of {@link GraphComputer} and/or {@link Graph} classes that have been initialized to the
* classloader so that they do not have to be reflected again.
*/
- private static Set<Class> LOADED = ConcurrentHashMap.newKeySet();
+ private static final Set<Class<?>> LOADED = ConcurrentHashMap.newKeySet();
private static final Map<Class<? extends Graph>, TraversalStrategies> GRAPH_CACHE = new HashMap<>();
private static final Map<Class<? extends GraphComputer>, TraversalStrategies> GRAPH_COMPUTER_CACHE = new HashMap<>();
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/GremlinDslProcessor.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/GremlinDslProcessor.java
index f40d870..295efcd 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/GremlinDslProcessor.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/GremlinDslProcessor.java
@@ -27,6 +27,7 @@ import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeVariableName;
+import com.squareup.javapoet.WildcardTypeName;
import org.apache.tinkerpop.gremlin.process.remote.RemoteConnection;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
@@ -57,10 +58,10 @@ import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.DeclaredType;
-import javax.lang.model.type.NoType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVariable;
+import javax.lang.model.type.WildcardType;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
import javax.tools.Diagnostic;
@@ -355,7 +356,8 @@ public class GremlinDslProcessor extends AbstractProcessor {
.addModifiers(Modifier.PUBLIC)
.addAnnotation(Override.class)
.addStatement("return Optional.of(__.class)")
- .returns(ParameterizedTypeName.get(Optional.class, Class.class))
+ .returns(ParameterizedTypeName.get(ClassName.get(Optional.class),
+ ParameterizedTypeName.get(ClassName.get(Class.class), WildcardTypeName.subtypeOf(Object.class))))
.build());
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
index 011bae7..b08ec4f 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java
@@ -75,11 +75,6 @@ public class GraphTraversalSource implements TraversalSource {
////////////////
- @Override
- public Optional<Class> getAnonymousTraversalClass() {
- return Optional.of(__.class);
- }
-
public GraphTraversalSource(final Graph graph, final TraversalStrategies traversalStrategies) {
this.graph = graph;
this.strategies = traversalStrategies;
@@ -96,6 +91,11 @@ public class GraphTraversalSource implements TraversalSource {
}
@Override
+ public Optional<Class<?>> getAnonymousTraversalClass() {
+ return Optional.of(__.class);
+ }
+
+ @Override
public TraversalStrategies getStrategies() {
return this.strategies;
}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategies.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategies.java
index 3e39d1b..b246f81 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategies.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategies.java
@@ -41,7 +41,8 @@ public class DefaultTraversalStrategies implements TraversalStrategies {
@SuppressWarnings({"unchecked", "varargs"})
public TraversalStrategies addStrategies(final TraversalStrategy<?>... strategies) {
for (final TraversalStrategy<?> addStrategy : strategies) {
- this.traversalStrategies.remove(addStrategy);
+ // search by class to prevent strategies from being added more than once
+ getStrategy(addStrategy.getClass()).ifPresent(s -> this.traversalStrategies.remove(s));
}
Collections.addAll(this.traversalStrategies, strategies);
this.traversalStrategies = TraversalStrategies.sortStrategies(this.traversalStrategies);
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategiesTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategiesTest.java
index c63ed93..cf1c694 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategiesTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalStrategiesTest.java
@@ -26,7 +26,8 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SideEf
import org.junit.Test;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -48,9 +49,20 @@ public class DefaultTraversalStrategiesTest {
o = new TraversalStrategiesTest.StrategyO();
@Test
+ public void testNoRepeatStrategies() {
+ final TraversalStrategies s = new DefaultTraversalStrategies();
+ s.addStrategies(b, a, d, b, a, a, d, d);
+ s.addStrategies(new TraversalStrategiesTest.StrategyD(), new TraversalStrategiesTest.StrategyB());
+ assertEquals(3, s.toList().size());
+ assertEquals(a, s.toList().get(0));
+ assertEquals(b, s.toList().get(1));
+ assertEquals(d, s.toList().get(2));
+ }
+
+ @Test
public void testWellDefinedDependency() {
//Dependency well defined
- TraversalStrategies s = new DefaultTraversalStrategies();
+ final TraversalStrategies s = new DefaultTraversalStrategies();
s.addStrategies(b, a);
assertEquals(2, s.toList().size());
assertEquals(a, s.toList().get(0));
@@ -60,7 +72,7 @@ public class DefaultTraversalStrategiesTest {
@Test
public void testNoDependency() {
//No dependency
- TraversalStrategies s = new DefaultTraversalStrategies();
+ final TraversalStrategies s = new DefaultTraversalStrategies();
s.addStrategies(c, a);
assertEquals(2, s.toList().size());
}
@@ -84,7 +96,7 @@ public class DefaultTraversalStrategiesTest {
@Test
public void testCircularDependency() {
//Circular dependency => throws exception
- TraversalStrategies s = new DefaultTraversalStrategies();
+ final TraversalStrategies s = new DefaultTraversalStrategies();
try {
s.addStrategies(c, k, a, b);
fail();
@@ -116,7 +128,7 @@ public class DefaultTraversalStrategiesTest {
@Test
public void testCircularDependency2() {
//Circular dependency => throws exception
- TraversalStrategies s = new DefaultTraversalStrategies();
+ final TraversalStrategies s = new DefaultTraversalStrategies();
try {
s.addStrategies(d, c, k, a, e, b);
fail();
@@ -199,10 +211,10 @@ public class DefaultTraversalStrategiesTest {
assertEquals(1, fifthTraversal.getSideEffects().keys().size());
assertEquals(2, fifthTraversal.getSideEffects().<Integer>get("a").intValue());
- assertTrue(firstTraversal.getStrategies() == secondTraversal.getStrategies());
- assertTrue(firstTraversal.getStrategies() != thirdTraversal.getStrategies());
- assertTrue(forthTraversal.getStrategies() == thirdTraversal.getStrategies());
- assertTrue(fifthTraversal.getStrategies() == firstTraversal.getStrategies());
+ assertSame(firstTraversal.getStrategies(), secondTraversal.getStrategies());
+ assertNotSame(firstTraversal.getStrategies(), thirdTraversal.getStrategies());
+ assertSame(forthTraversal.getStrategies(), thirdTraversal.getStrategies());
+ assertSame(fifthTraversal.getStrategies(), firstTraversal.getStrategies());
}
}