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 2018/10/19 18:59:10 UTC
[tinkerpop] 02/02: TINKERPOP-2072 Refactored TypeTranslator for
direction extension
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2072
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit b1998da6e23271b404051cb334106c96e55f5f34
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Fri Oct 19 14:50:46 2018 -0400
TINKERPOP-2072 Refactored TypeTranslator for direction extension
This approach makes it much easier to extend TypeTranslators as the core functionality of the GroovyTranslator is itself a TypeTranslator. Simply extend that and go. Probably should have been this way from the beginning.
---
CHANGELOG.asciidoc | 1 +
docs/src/upgrade/release-3.4.x.asciidoc | 16 ++++++++
.../gremlin/process/traversal/Translator.java | 29 ++------------
.../groovy/jsr223/GremlinGroovyScriptEngine.java | 2 +-
.../gremlin/groovy/jsr223/GroovyTranslator.java | 45 ++++++----------------
.../groovy/jsr223/GroovyTranslatorTest.java | 33 +++++++---------
6 files changed, 47 insertions(+), 79 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ec6cae7..d6a8dfb 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -40,6 +40,7 @@ This release also includes changes from <<release-3-3-3, 3.3.3>>.
* Deprecated `Graph.io()` and related infrastructure.
* `GraphMLReader` better handles edge and vertex properties with the same name.
* Maintained order of annotations in metrics returned from `profile()`-step.
+* Refactored `TypeTranslator` to be directly extensible for `ScriptTranslator` functions.
* Bumped to Netty 4.1.25.
* Bumped to Spark 2.3.1.
* Bumped to Groovy 2.5.2.
diff --git a/docs/src/upgrade/release-3.4.x.asciidoc b/docs/src/upgrade/release-3.4.x.asciidoc
index cafcc46..2023bfd 100644
--- a/docs/src/upgrade/release-3.4.x.asciidoc
+++ b/docs/src/upgrade/release-3.4.x.asciidoc
@@ -600,3 +600,19 @@ The extent to which TinkerPop tests "upsert" is fairly narrow. Graph providers t
should consider their own test suites carefully to ensure appropriate coverage.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-1685[TINKERPOP-1685]
+
+==== TypeTranslator Changes
+
+The `TypeTranslator` experienced a change in its API and `GroovyTranslator` a change in expectations.
+
+`TypeTranslator` now implements `BiFunction` and takes the graph traversal source name as an argument along with the
+object to translate. This interface is implemented by default for Groovy with `GroovyTranslator.DefaultTypeTranslator`
+which encapsulates all the functionality of what `GroovyTranslator` formerly did by default. To provide customize
+translation, simply extend the `DefaultTypeTranslator` and override the methods.
+
+`GroovyTranslator` now expects that the `TypeTranslator` provider to it as part of its `of()` static method overload
+is "complete" - i.e. that it provides all the functionality to translate the types passed to it. Thus, a "complete"
+`TypeTranslator` is one that does everything that `DefaultTypeTranslator` does as a minimum requirement. Therefore,
+the extension model described above is the easiest way to get going with a custom `TypeTranslator` approach.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2072[TINKERPOP-2072]
\ No newline at end of file
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Translator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Translator.java
index 0346092..8cdd84d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Translator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Translator.java
@@ -19,7 +19,7 @@
package org.apache.tinkerpop.gremlin.process.traversal;
-import java.util.function.UnaryOperator;
+import java.util.function.BiFunction;
/**
* A Translator will translate {@link Bytecode} into another representation. That representation may be a
@@ -63,31 +63,10 @@ public interface Translator<S, T> {
public interface ScriptTranslator extends Translator<String, String> {
/**
- * Provides a way to customize and override the standard translation process. A {@link ScriptTranslator}
- * implementation can choose to expose a way to accept a {@code TypeTranslator} which will convert an incoming
- * object to a different form which will then be normally processed or can return a {@link Handled} object
- * with the already translated script.
+ * Provides a way for the {@link ScriptTranslator} to convert various data types to their string
+ * representations in their target language.
*/
- public interface TypeTranslator extends UnaryOperator<Object> {
- public static TypeTranslator identity() {
- return t -> t;
- }
- }
-
- /**
- * Contains a completed type translation from the {@link TypeTranslator}.
- */
- public class Handled {
- private final String translation;
-
- public Handled(final String translation) {
- this.translation = translation;
- }
-
- public String getTranslation() {
- return translation;
- }
- }
+ public interface TypeTranslator extends BiFunction<String, Object, Object> { }
}
/**
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
index dae427c..0cd55be 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GremlinGroovyScriptEngine.java
@@ -278,7 +278,7 @@ public class GremlinGroovyScriptEngine extends GroovyScriptEngineImpl implements
filter(p -> p instanceof TranslatorCustomizer).
map(p -> (TranslatorCustomizer) p).findFirst();
typeTranslator = translatorCustomizer.isPresent() ? translatorCustomizer.get().createTypeTranslator() :
- Translator.ScriptTranslator.TypeTranslator.identity();
+ new GroovyTranslator.DefaultTypeTranslator();
createClassLoader();
}
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslator.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslator.java
index 64dac56..b961821 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslator.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslator.java
@@ -62,22 +62,23 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
private GroovyTranslator(final String traversalSource, final TypeTranslator typeTranslator) {
this.traversalSource = traversalSource;
- this.typeTranslator = new DefaultTypeTranslator(traversalSource, typeTranslator);
+ this.typeTranslator = typeTranslator;
}
public static final GroovyTranslator of(final String traversalSource) {
- return of(traversalSource, TypeTranslator.identity());
+ return of(traversalSource, null);
}
public static final GroovyTranslator of(final String traversalSource, final TypeTranslator typeTranslator) {
- return new GroovyTranslator(traversalSource, Optional.ofNullable(typeTranslator).orElse(TypeTranslator.identity()));
+ return new GroovyTranslator(traversalSource,
+ Optional.ofNullable(typeTranslator).orElseGet(DefaultTypeTranslator::new));
}
///////
@Override
public String translate(final Bytecode bytecode) {
- return typeTranslator.apply(bytecode).toString();
+ return typeTranslator.apply(traversalSource, bytecode).toString();
}
@Override
@@ -100,40 +101,16 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
*/
public static class DefaultTypeTranslator implements TypeTranslator {
- private final String traversalSource;
- private TypeTranslator customTypeTranslator;
-
- public DefaultTypeTranslator() {
- this("__", TypeTranslator.identity());
- }
-
- public DefaultTypeTranslator(final String traversalSource) {
- this(traversalSource, TypeTranslator.identity());
- }
-
- public DefaultTypeTranslator(final String traversalSource, final TypeTranslator customTypeTranslator) {
- this.traversalSource = traversalSource;
- this.customTypeTranslator = customTypeTranslator;
- }
-
@Override
- public Object apply(final Object o) {
+ public Object apply(final String traversalSource, final Object o) {
if (o instanceof Bytecode)
return internalTranslate(traversalSource, (Bytecode) o);
else
return convertToString(o);
}
- protected String convertToString(final Object o) {
-
- final Object object = customTypeTranslator.apply(o);
-
- // an object that is Handled means that the "custom" TypeTranslator figured out how to convert the
- // object to a string and it should be used as-is, otherwise it gets passed down the line through the normal
- // process
- if (object instanceof Handled)
- return ((Handled) object).getTranslation();
- else if (object instanceof Bytecode.Binding)
+ protected String convertToString(final Object object) {
+ if (object instanceof Bytecode.Binding)
return ((Bytecode.Binding) object).variable();
else if (object instanceof Bytecode)
return internalTranslate("__", (Bytecode) object);
@@ -235,7 +212,7 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
return null == object ? "null" : object.toString();
}
- private String internalTranslate(final String start, final Bytecode bytecode) {
+ protected String internalTranslate(final String start, final Bytecode bytecode) {
final StringBuilder traversalScript = new StringBuilder(start);
for (final Bytecode.Instruction instruction : bytecode.getInstructions()) {
final String methodName = instruction.getOperator();
@@ -253,7 +230,7 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
return traversalScript.toString();
}
- private StringBuilder convertPToString(final P p, final StringBuilder current) {
+ protected StringBuilder convertPToString(final P p, final StringBuilder current) {
if (p instanceof TextP) return convertTextPToString((TextP) p, current);
if (p instanceof ConnectiveP) {
final List<P<?>> list = ((ConnectiveP) p).getPredicates();
@@ -268,7 +245,7 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
return current;
}
- private StringBuilder convertTextPToString(final TextP p, final StringBuilder current) {
+ protected StringBuilder convertTextPToString(final TextP p, final StringBuilder current) {
current.append("TextP.").append(p.getBiPredicate().toString()).append("(").append(convertToString(p.getValue())).append(")");
return current;
}
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
index 2cbc962..e56b105 100644
--- a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorTest.java
@@ -20,7 +20,6 @@
package org.apache.tinkerpop.gremlin.groovy.jsr223;
import org.apache.commons.configuration.MapConfiguration;
-import org.apache.tinkerpop.gremlin.AbstractGremlinTest;
import org.apache.tinkerpop.gremlin.jsr223.TranslatorCustomizer;
import org.apache.tinkerpop.gremlin.process.traversal.Order;
import org.apache.tinkerpop.gremlin.process.traversal.Pop;
@@ -225,17 +224,6 @@ public class GroovyTranslatorTest {
}
@Test
- public void shouldOverrideDefaultTypeTranslationWithSomethingBonkers() {
- final TinkerGraph graph = TinkerGraph.open();
- final GraphTraversalSource g = graph.traversal();
- final String thingToSuffixAllStringsWith = "-why-would-anyone-do-this";
- final String script = GroovyTranslator.of("g", x -> x instanceof String ? x + thingToSuffixAllStringsWith : x).
- translate(g.inject("yyy", "xxx").asAdmin().getBytecode());
- assertEquals(String.format("g.inject(\"yyy%s\",\"xxx%s\")", thingToSuffixAllStringsWith, thingToSuffixAllStringsWith), script);
- assertThatScriptOk(script, "g", g);
- }
-
- @Test
public void shouldIncludeCustomTypeTranslationForSomethingSilly() throws Exception {
final TinkerGraph graph = TinkerGraph.open();
final SillyClass notSillyEnough = SillyClass.from("not silly enough", 100);
@@ -247,10 +235,7 @@ public class GroovyTranslatorTest {
assertEquals(String.format("g.inject(%s)", "not silly enough:100"), scriptBad);
// with type translation we get valid gremlin
- final String scriptGood = GroovyTranslator.of("g",
- x -> x instanceof SillyClass ?
- new Translator.ScriptTranslator.Handled(String.format("org.apache.tinkerpop.gremlin.groovy.jsr223.GroovyTranslatorTest.SillyClass.from('%s', (int) %s)",
- ((SillyClass) x).getX(), ((SillyClass) x).getY())) : x).
+ final String scriptGood = GroovyTranslator.of("g", new SillyClassTranslator()).
translate(g.inject(notSillyEnough).asAdmin().getBytecode());
assertEquals(String.format("g.inject(org.apache.tinkerpop.gremlin.groovy.jsr223.GroovyTranslatorTest.SillyClass.from('%s', (int) %s))", notSillyEnough.getX(), notSillyEnough.getY()), scriptGood);
assertThatScriptOk(scriptGood, "g", g);
@@ -389,13 +374,23 @@ public class GroovyTranslatorTest {
}
}
+ public static class SillyClassTranslator extends GroovyTranslator.DefaultTypeTranslator {
+
+ @Override
+ protected String convertToString(final Object object) {
+ if (object instanceof SillyClass)
+ return String.format("org.apache.tinkerpop.gremlin.groovy.jsr223.GroovyTranslatorTest.SillyClass.from('%s', (int) %s)",
+ ((SillyClass) object).getX(), ((SillyClass) object).getY());
+ else
+ return super.convertToString(object);
+ }
+ }
+
public static class SillyClassTranslatorCustomizer implements TranslatorCustomizer {
@Override
public Translator.ScriptTranslator.TypeTranslator createTypeTranslator() {
- return x -> x instanceof SillyClass ?
- new Translator.ScriptTranslator.Handled(String.format("org.apache.tinkerpop.gremlin.groovy.jsr223.GroovyTranslatorTest.SillyClass.from('%s', (int) %s)",
- ((SillyClass) x).getX(), ((SillyClass) x).getY())) : x;
+ return new SillyClassTranslator();
}
}
}