From 8b40aa7d0b85e10f82a746c0427b0de2bc519bba Mon Sep 17 00:00:00 2001
From: anttil <anttil>
Date: Fri, 8 Nov 2013 15:58:39 +0000
Subject: [PATCH] BIS-575 / SP-995 : Novartis - dynamic properties do not work
anymore in 13.04.5
SVN: 30121
---
.../common/jython/evaluator/Evaluator.java | 5 ++
.../generic/server/JythonEvaluatorPool.java | 75 +++++++++++++------
.../JythonDynamicPropertyCalculator.java | 3 +-
.../server/JythonEvaluatorPoolTest.java | 36 +++++++++
.../recursive_property.py | 5 ++
5 files changed, 100 insertions(+), 24 deletions(-)
create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py
diff --git a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
index 25942e12b44..0fda16012fd 100644
--- a/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
+++ b/common/source/java/ch/systemsx/cisd/common/jython/evaluator/Evaluator.java
@@ -236,6 +236,11 @@ public final class Evaluator
interpreter.set(name, value);
}
+ public Object get(String name)
+ {
+ return interpreter.get(name);
+ }
+
/**
* Deletes the variable <var>name</var> from the evaluator's name space.
*/
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
index cb7a6d00093..b72541f75a3 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPool.java
@@ -16,9 +16,10 @@
package ch.systemsx.cisd.openbis.generic.server;
-import java.util.Collection;
+import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Stack;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -36,9 +37,8 @@ import ch.systemsx.cisd.openbis.generic.shared.managed_property.IEvaluationRunne
import ch.systemsx.cisd.openbis.generic.shared.managed_property.ManagedPropertyFunctions;
/**
- * Pool of Jython Evaluators for managed properties. When the pool is created, it is filled with new
- * Evaluator instances for every managed property script. Enables thread-safe execution of calls to
- * python functions defined in these scripts.
+ * Pool of Jython Evaluators for managed properties. When the pool is created, it is filled with new Evaluator instances for every managed property
+ * script. Enables thread-safe execution of calls to python functions defined in these scripts.
*
* @author anttil
*/
@@ -55,6 +55,8 @@ public class JythonEvaluatorPool
private Lock cacheLock;
+ private static ThreadLocal<Stack<Map<String, Object>>> stack = new ThreadLocal<Stack<Map<String, Object>>>();
+
public JythonEvaluatorPool(IDAOFactory daoFactory, String poolSize)
{
this(daoFactory, createCache(poolSize));
@@ -107,9 +109,8 @@ public class JythonEvaluatorPool
}
/**
- * Evaluate python functions from a script using an Evaluator instance in the pool. If the
- * Evaluator instance does not exist, create it. Give access to the instance for only one thread
- * at a time.
+ * Evaluate python functions from a script using an Evaluator instance in the pool. If the Evaluator instance does not exist, create it. Give
+ * access to the instance for only one thread at a time.
*/
private <T> T evaluate(String expression, Class<?> clazz, String script,
IAtomicEvaluation<T> evaluation)
@@ -134,14 +135,27 @@ public class JythonEvaluatorPool
}
}
- Lock lock = state.getLock();
+ ReentrantLock lock = (ReentrantLock) state.getLock();
+
+ if (false == lock.isHeldByCurrentThread())
+ {
+ stack.set(new Stack<Map<String, Object>>());
+ }
+
lock.lock();
+ state.push();
+
try
{
- return evaluation.evaluate(state.getCleanInstance());
+ return evaluation.evaluate(state.getEvaluator());
} finally
{
+ state.pop();
lock.unlock();
+ if (false == lock.isHeldByCurrentThread())
+ {
+ stack.set(null);
+ }
}
}
@@ -168,37 +182,52 @@ public class JythonEvaluatorPool
}
/**
- * The pooled object. Contains the Evaluator and its initial state to which it is always
- * returned to when a new evaluation is started.
+ * The pooled object. Contains the Evaluator and its initial state to which it is always returned to when a new evaluation is started.
*/
public static class EvaluatorState
{
private final Evaluator evaluator;
- private final Collection<String> globals;
+ private Stack<Map<String, Object>> globalsStack;
private final Lock lock;
public EvaluatorState(Evaluator evaluator)
{
this.evaluator = evaluator;
- this.globals = evaluator.getGlobalVariables();
this.lock = new ReentrantLock();
+ this.globalsStack = new Stack<Map<String, Object>>();
}
- /**
- * Returns an evaluator instance in its initial state.
- */
- public synchronized Evaluator getCleanInstance()
+ public void push()
{
- for (String value : evaluator.getGlobalVariables())
+ Map<String, Object> globalsValues = new HashMap<String, Object>();
+ for (String globalName : evaluator.getGlobalVariables())
{
- if (globals.contains(value) == false)
- {
- evaluator.delete(value);
- }
+ Object globalValue = evaluator.get(globalName);
+ globalsValues.put(globalName, globalValue);
+ }
+ globalsStack.push(globalsValues);
+ }
+
+ public void pop()
+ {
+ Map<String, Object> globalsValues = globalsStack.pop();
+
+ for (String globalName : evaluator.getGlobalVariables())
+ {
+ evaluator.delete(globalName);
}
- return this.evaluator;
+
+ for (String globalName : globalsValues.keySet())
+ {
+ evaluator.set(globalName, globalsValues.get(globalName));
+ }
+ }
+
+ public Evaluator getEvaluator()
+ {
+ return evaluator;
}
public Lock getLock()
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java
index 16f452df86d..2eaf61bd265 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/JythonDynamicPropertyCalculator.java
@@ -107,7 +107,8 @@ public class JythonDynamicPropertyCalculator implements IDynamicPropertyCalculat
try
{
evaluator.set(ENTITY_VARIABLE_NAME, entity);
- return evaluator.evalAsStringLegacy2_2();
+ String x = evaluator.evalAsStringLegacy2_2();
+ return x;
} finally
{
evaluator.releaseResources();
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
index c4fca183e01..0ff99ad9fd6 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTest.java
@@ -48,13 +48,16 @@ import org.jmock.Mockery;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import ch.systemsx.cisd.common.filesystem.FileUtilities;
import ch.systemsx.cisd.common.jython.evaluator.Evaluator;
+import ch.systemsx.cisd.common.utilities.TestResources;
import ch.systemsx.cisd.openbis.generic.server.JythonEvaluatorPool.EvaluatorState;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IScriptDAO;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ScriptType;
import ch.systemsx.cisd.openbis.generic.shared.dto.ScriptPE;
import ch.systemsx.cisd.openbis.generic.shared.managed_property.IAtomicEvaluation;
+import ch.systemsx.cisd.openbis.generic.shared.managed_property.IEvaluationRunner;
/**
* @author anttil
@@ -78,6 +81,39 @@ public class JythonEvaluatorPoolTest
}
}
+ @Test
+ public void globalVariablesAreHandledCorrectlyWhenThereAreNestedCalls() throws Exception
+ {
+ TestResources resources = new TestResources(getClass());
+ String script = FileUtilities.loadToString(resources.getResourceFile("recursive_property.py"));
+ final IEvaluationRunner runner =
+ pool.getRunner("calculate()", Math.class, script);
+
+ final IAtomicEvaluation<String> action = new IAtomicEvaluation<String>()
+ {
+
+ int counter = 5;
+
+ @Override
+ public String evaluate(Evaluator evaluator)
+ {
+ evaluator.set("action", this);
+ evaluator.set("runner", runner);
+ evaluator.set("value", counter);
+ if (counter > 0)
+ {
+ counter--;
+ return evaluator.evalAsStringLegacy2_2();
+ } else
+ {
+ return "";
+ }
+ }
+ };
+ runner.evaluate(action);
+ // Assertion in the python script!
+ }
+
@Test
public void globalVariablesSetInPreviousRunsAreClearedBeforeNewEvaluation() throws Exception
{
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py
new file mode 100644
index 00000000000..255a6cefeed
--- /dev/null
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/JythonEvaluatorPoolTestResources/recursive_property.py
@@ -0,0 +1,5 @@
+def calculate():
+ temp = value
+ runner.evaluate(action);
+ if temp != value:
+ raise Exception(str(temp)+" vs "+str(value))
\ No newline at end of file
--
GitLab