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 2016/01/22 00:16:16 UTC
incubator-tinkerpop git commit: TINKERPOP-860 Patched up a bug
introduced by this ticket.
Repository: incubator-tinkerpop
Updated Branches:
refs/heads/master 484746b96 -> 5e6d66828
TINKERPOP-860 Patched up a bug introduced by this ticket.
ENGINE_SCOPE bindings were being re-applied between requests. Moved the plugin bindings to the GLOBAL_SCOPE to avoid polluting the ENGINE_SCOPE space and the duplication.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/5e6d6682
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/5e6d6682
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/5e6d6682
Branch: refs/heads/master
Commit: 5e6d66828feb36cca5584b7835eda0753e24cd2d
Parents: 484746b
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu Jan 21 18:14:11 2016 -0500
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu Jan 21 18:14:11 2016 -0500
----------------------------------------------------------------------
.../gremlin/groovy/engine/ScriptEngines.java | 5 ++-
.../jsr223/ScriptEnginePluginAcceptor.java | 10 +++--
.../groovy/engine/ScriptEnginesTest.java | 25 +++++++++++
.../server/GremlinDriverIntegrateTest.java | 45 ++++++++++++++++++++
4 files changed, 81 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/5e6d6682/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
index a03acee..84b0e9b 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEngines.java
@@ -35,6 +35,7 @@ import javax.script.ScriptContext;
import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import java.io.Reader;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
@@ -403,9 +404,11 @@ public class ScriptEngines implements AutoCloseable {
*/
private static Bindings mergeBindings(final Bindings bindings, final ScriptEngine engine) {
// plugins place "globals" here - see ScriptEnginePluginAcceptor
- final Bindings all = engine.getBindings(ScriptContext.ENGINE_SCOPE);
+ final Bindings global = engine.getBindings(ScriptContext.GLOBAL_SCOPE);
+ if (null == global) return bindings;
// merge the globals with the incoming bindings where local bindings "win"
+ final Bindings all = new SimpleBindings(global);
all.putAll(bindings);
return all;
}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/5e6d6682/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptEnginePluginAcceptor.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptEnginePluginAcceptor.java b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptEnginePluginAcceptor.java
index 1a72864..0bd51c2 100644
--- a/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptEnginePluginAcceptor.java
+++ b/gremlin-groovy/src/main/java/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptEnginePluginAcceptor.java
@@ -24,6 +24,7 @@ import org.apache.tinkerpop.gremlin.groovy.plugin.PluginAcceptor;
import javax.script.ScriptContext;
import javax.script.ScriptEngine;
import javax.script.ScriptException;
+import javax.script.SimpleBindings;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
@@ -46,8 +47,11 @@ public class ScriptEnginePluginAcceptor implements PluginAcceptor {
*/
@Override
public void addBinding(final String key, final Object val) {
- // added to the engine scope because the plugin will be applied to each scriptengine independently anyway
- scriptEngine.getContext().setAttribute(key, val, ScriptContext.ENGINE_SCOPE);
+ // The binding was originally added to the engine scope but that proved to be "bad" as it mixed with other
+ // bindings in that space.
+ if (null == scriptEngine.getContext().getBindings(ScriptContext.GLOBAL_SCOPE))
+ scriptEngine.getContext().setBindings(new SimpleBindings(), ScriptContext.GLOBAL_SCOPE);
+ scriptEngine.getContext().setAttribute(key, val, ScriptContext.GLOBAL_SCOPE);
}
/**
@@ -56,7 +60,7 @@ public class ScriptEnginePluginAcceptor implements PluginAcceptor {
@Override
public Map<String, Object> getBindings() {
// as these "global" bindings were added to engine scope they should be pulled from the same place
- return scriptEngine.getBindings(ScriptContext.ENGINE_SCOPE);
+ return scriptEngine.getBindings(ScriptContext.GLOBAL_SCOPE);
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/5e6d6682/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEnginesTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEnginesTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEnginesTest.java
index 940a530..eb45c0b 100644
--- a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEnginesTest.java
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/engine/ScriptEnginesTest.java
@@ -18,6 +18,8 @@
*/
package org.apache.tinkerpop.gremlin.groovy.engine;
+import groovy.lang.MissingPropertyException;
+import org.apache.commons.lang.exception.ExceptionUtils;
import org.apache.tinkerpop.gremlin.groovy.plugin.GremlinPlugin;
import org.apache.tinkerpop.gremlin.groovy.plugin.IllegalEnvironmentException;
import org.apache.tinkerpop.gremlin.groovy.plugin.PluginAcceptor;
@@ -34,14 +36,37 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.IntStream;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
public class ScriptEnginesTest {
@Test
+ public void shouldNotPreserveInstantiatedVariablesBetweenEvals() throws Exception {
+ final ScriptEngines engines = new ScriptEngines(se -> {});
+ engines.reload("gremlin-groovy", Collections.<String>emptySet(),
+ Collections.<String>emptySet(), Collections.emptyMap());
+
+ final Bindings localBindingsFirstRequest = new SimpleBindings();
+ localBindingsFirstRequest.put("x", "there");
+ assertEquals("herethere", engines.eval("z = 'here' + x", localBindingsFirstRequest, "gremlin-groovy"));
+
+ try {
+ final Bindings localBindingsSecondRequest = new SimpleBindings();
+ engines.eval("z", localBindingsSecondRequest, "gremlin-groovy");
+ fail("Should not have knowledge of z");
+ } catch (Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root, instanceOf(MissingPropertyException.class));
+ }
+ }
+
+ @Test
public void shouldMergeBindingsFromLocalAndGlobal() throws Exception {
final ScriptEngines engines = new ScriptEngines(se -> {});
engines.reload("gremlin-groovy", Collections.<String>emptySet(),
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/5e6d6682/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
index 7f53c3f..a0d06f1 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
@@ -37,6 +37,7 @@ import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
import org.apache.tinkerpop.gremlin.util.TimeUtil;
import groovy.json.JsonBuilder;
import org.apache.tinkerpop.gremlin.util.function.FunctionUtils;
+import org.hamcrest.core.IsInstanceOf;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -834,6 +835,50 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
}
@Test
+ public void shouldNotHaveKnowledgeOfBindingsBetweenRequestsWhenSessionless() throws Exception {
+ final Cluster cluster = Cluster.build().create();
+ final Client client1 = cluster.connect();
+ final Client client2 = cluster.connect();
+ final Client client3 = cluster.connect();
+
+ final ResultSet results11 = client1.submit("x = 1");
+ final ResultSet results21 = client2.submit("x = 2");
+ final ResultSet results31 = client3.submit("x = 3");
+ assertEquals(1, results11.all().get().get(0).getInt());
+ assertEquals(2, results21.all().get().get(0).getInt());
+ assertEquals(3, results31.all().get().get(0).getInt());
+
+ try {
+ client1.submit("x").all().get();
+ fail("The variable 'x' should not be present on the new request.");
+ } catch (Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root, IsInstanceOf.instanceOf(ResponseException.class));
+ assertThat(root.getMessage(), containsString("No such property: x for class"));
+ }
+
+ try {
+ client2.submit("x").all().get();
+ fail("The variable 'x' should not be present on the new request.");
+ } catch (Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root, IsInstanceOf.instanceOf(ResponseException.class));
+ assertThat(root.getMessage(), containsString("No such property: x for class"));
+ }
+
+ try {
+ client3.submit("x").all().get();
+ fail("The variable 'x' should not be present on the new request.");
+ } catch (Exception ex) {
+ final Throwable root = ExceptionUtils.getRootCause(ex);
+ assertThat(root, IsInstanceOf.instanceOf(ResponseException.class));
+ assertThat(root.getMessage(), containsString("No such property: x for class"));
+ }
+
+ cluster.close();
+ }
+
+ @Test
public void shouldBeThreadSafeToUseOneClient() throws Exception {
final Cluster cluster = Cluster.build().create();
final Client client = cluster.connect();