From 727318362bce764fcb3b6fd9b79b68cb4c8fca3e Mon Sep 17 00:00:00 2001
From: jakubs <jakubs>
Date: Thu, 23 Feb 2012 16:30:05 +0000
Subject: [PATCH] LMS-2794: Check hook functions in a dropbox script for wrong
signatures.
SVN: 24558
---
.../JythonTopLevelDataSetHandler.java | 136 +++++++++++-------
.../AbstractJythonDataSetHandlerTest.java | 17 ---
.../JythonTopLevelDataSetRegistratorTest.java | 52 +++++--
...e-postregistration-hook-wrong-signature.py | 8 ++
4 files changed, 131 insertions(+), 82 deletions(-)
create mode 100644 datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/testcase-postregistration-hook-wrong-signature.py
diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java
index 382d0be6da3..76d219f561a 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetHandler.java
@@ -17,9 +17,11 @@
package ch.systemsx.cisd.etlserver.registrator;
import java.io.File;
+import java.util.LinkedList;
import java.util.List;
import org.python.core.Py;
+import org.python.core.PyBaseCode;
import org.python.core.PyException;
import org.python.core.PyFunction;
import org.python.util.PythonInterpreter;
@@ -43,43 +45,58 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation;
public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
AbstractOmniscientTopLevelDataSetRegistrator<T>
{
- /**
- * The name of the function to define to hook into the service rollback mechanism.
- */
- private static final String ROLLBACK_SERVICE_FUNCTION_NAME = "rollback_service";
+ private enum JythonHookFunction
+ {
+ /**
+ * The name of the function to define to hook into the service rollback mechanism.
+ */
+ ROLLBACK_SERVICE_FUNCTION_NAME("rollback_service", 2),
- /**
- * The name of the function to define to hook into the transaction rollback mechanism.
- */
- private static final String ROLLBACK_TRANSACTION_FUNCTION_NAME = "rollback_transaction";
+ /**
+ * The name of the function to define to hook into the transaction rollback mechanism.
+ */
+ ROLLBACK_TRANSACTION_FUNCTION_NAME("rollback_transaction", 4),
- /**
- * The name of the function called after successful transaction commit.
- */
- private static final String COMMIT_TRANSACTION_FUNCTION_NAME = "commit_transaction";
+ /**
+ * The name of the function called after successful transaction commit.
+ */
+ COMMIT_TRANSACTION_FUNCTION_NAME("commit_transaction", 2),
- /**
- * The name of the function called after successful transaction commit.
- */
- private static final String POST_STORAGE_FUNCTION_NAME = "post_storage";
+ /**
+ * The name of the function called after successful transaction commit.
+ */
+ POST_STORAGE_FUNCTION_NAME("post_storage", 2),
- /**
- * The name of the function called just before registration of datasets in application server.
- */
- private static final String PRE_REGISTRATION_FUNCTION_NAME = "pre_metadata_registration";
+ /**
+ * The name of the function called just before registration of datasets in application
+ * server.
+ */
+ PRE_REGISTRATION_FUNCTION_NAME("pre_metadata_registration", 2),
- /**
- * The name of the function called just after successful registration of datasets in application
- * server.
- */
- private static final String POST_REGISTRATION_FUNCTION_NAME = "post_metadata_registration";
+ /**
+ * The name of the function called just after successful registration of datasets in
+ * application server.
+ */
+ POST_REGISTRATION_FUNCTION_NAME("post_metadata_registration", 2),
- /**
- * The name of the function called when secondary transactions, DynamicTransactionQuery objects,
- * fail.
- */
- private static final String DID_ENCOUNTER_SECONDARY_TRANSACTION_ERRORS_FUNCTION_NAME =
- "did_encounter_secondary_transaction_errors";
+ /**
+ * The name of the function called when secondary transactions, DynamicTransactionQuery
+ * objects, fail.
+ */
+ DID_ENCOUNTER_SECONDARY_TRANSACTION_ERRORS_FUNCTION_NAME(
+ "did_encounter_secondary_transaction_errors", 3);
+
+ String name;
+
+ int argCount;
+
+ private JythonHookFunction(String name, int argCount)
+ {
+ this.name = name;
+ this.argCount = argCount;
+
+ }
+ }
private static final String FACTORY_VARIABLE_NAME = "factory";
@@ -144,6 +161,30 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
// Invoke the evaluator
interpreter.exec(scriptString);
+
+ for (JythonHookFunction function : JythonHookFunction.values())
+ {
+ PyFunction py = tryJythonFunction(interpreter, function);
+ if (py != null)
+ {
+ if (py.func_code instanceof PyBaseCode)
+ {
+ int co_argcount = ((PyBaseCode) py.func_code).co_argcount;
+ if (co_argcount != function.argCount)
+ {
+ throw new IllegalArgumentException(
+ String.format(
+ "The function %s in %s has wrong number of arguments(%s instead of %s).",
+ function.name, scriptFile.getName(), co_argcount,
+ function.argCount));
+ }
+ } else
+ {
+ System.err
+ .println("Possibly incorrect python code. Can't verify script correctness.");
+ }
+ }
+ }
}
/**
@@ -182,7 +223,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
protected void rollback(DataSetRegistrationService<T> service, Throwable throwable)
{
PythonInterpreter interpreter = getInterpreterFromService(service);
- PyFunction function = tryJythonFunction(interpreter, ROLLBACK_SERVICE_FUNCTION_NAME);
+ PyFunction function = tryJythonFunction(interpreter, JythonHookFunction.ROLLBACK_SERVICE_FUNCTION_NAME);
if (null != function)
{
invokeRollbackServiceFunction(function, service, throwable);
@@ -239,7 +280,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
DataSetStorageAlgorithmRunner<T> algorithmRunner, Throwable ex)
{
PythonInterpreter interpreter = getInterpreterFromService(service);
- PyFunction function = tryJythonFunction(interpreter, ROLLBACK_TRANSACTION_FUNCTION_NAME);
+ PyFunction function = tryJythonFunction(interpreter, JythonHookFunction.ROLLBACK_TRANSACTION_FUNCTION_NAME);
if (null != function)
{
invokeRollbackTransactionFunction(function, service, transaction, algorithmRunner, ex);
@@ -247,7 +288,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
{
// No Rollback transaction function was called, see if the rollback service function was
// defined, and call it.
- function = tryJythonFunction(interpreter, ROLLBACK_SERVICE_FUNCTION_NAME);
+ function = tryJythonFunction(interpreter, JythonHookFunction.ROLLBACK_SERVICE_FUNCTION_NAME);
if (null != function)
{
invokeRollbackServiceFunction(function, service, ex);
@@ -260,13 +301,13 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
{
PythonInterpreter interpreter = getInterpreterFromService(service);
- PyFunction function = tryJythonFunction(interpreter, POST_STORAGE_FUNCTION_NAME);
+ PyFunction function = tryJythonFunction(interpreter, JythonHookFunction.POST_STORAGE_FUNCTION_NAME);
if (null != function)
{
invokeTransactionFunctionWithContext(function, service, transaction);
} else
{
- function = tryJythonFunction(interpreter, COMMIT_TRANSACTION_FUNCTION_NAME);
+ function = tryJythonFunction(interpreter, JythonHookFunction.COMMIT_TRANSACTION_FUNCTION_NAME);
if (null != function)
{
invokeServiceTransactionFunction(function, service, transaction);
@@ -278,7 +319,8 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
DataSetRegistrationTransaction<T> transaction)
{
PythonInterpreter interpreter = getInterpreterFromService(service);
- PyFunction function = tryJythonFunction(interpreter, PRE_REGISTRATION_FUNCTION_NAME);
+ PyFunction function = tryJythonFunction(interpreter, JythonHookFunction.PRE_REGISTRATION_FUNCTION_NAME);
+
if (null != function)
{
invokeTransactionFunctionWithContext(function, service, transaction);
@@ -289,7 +331,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
DataSetRegistrationTransaction<T> transaction)
{
PythonInterpreter interpreter = getInterpreterFromService(service);
- PyFunction function = tryJythonFunction(interpreter, POST_REGISTRATION_FUNCTION_NAME);
+ PyFunction function = tryJythonFunction(interpreter, JythonHookFunction.POST_REGISTRATION_FUNCTION_NAME);
if (null != function)
{
invokeTransactionFunctionWithContext(function, service, transaction);
@@ -303,7 +345,7 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
PythonInterpreter interpreter = getInterpreterFromService(service);
PyFunction function =
tryJythonFunction(interpreter,
- DID_ENCOUNTER_SECONDARY_TRANSACTION_ERRORS_FUNCTION_NAME);
+ JythonHookFunction.DID_ENCOUNTER_SECONDARY_TRANSACTION_ERRORS_FUNCTION_NAME);
if (null != function)
{
invokeDidEncounterSecondaryTransactionErrorsFunction(function, service, transaction,
@@ -311,11 +353,12 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
}
}
- private PyFunction tryJythonFunction(PythonInterpreter interpreter, String functionName)
+ private PyFunction tryJythonFunction(PythonInterpreter interpreter,
+ JythonHookFunction functionDefinition)
{
try
{
- PyFunction function = interpreter.get(functionName, PyFunction.class);
+ PyFunction function = interpreter.get(functionDefinition.name, PyFunction.class);
return function;
} catch (Exception e)
{
@@ -344,17 +387,6 @@ public class JythonTopLevelDataSetHandler<T extends DataSetInformation> extends
Py.java2py(algorithmRunner), Py.java2py(throwable));
}
- /**
- * Pulled out as a separate method so tests can hook in.
- */
- protected void invokeRollbackDataSetRegistrationFunction(PyFunction function,
- DataSetRegistrationService<T> service,
- DataSetRegistrationAlgorithm registrationAlgorithm, Throwable throwable)
- {
- function.__call__(Py.java2py(service), Py.java2py(registrationAlgorithm),
- Py.java2py(throwable));
- }
-
/**
* Pulled out as a separate method so tests can hook in.
*/
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/AbstractJythonDataSetHandlerTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/AbstractJythonDataSetHandlerTest.java
index 27359ccae6f..0599ab14453 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/AbstractJythonDataSetHandlerTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/AbstractJythonDataSetHandlerTest.java
@@ -386,8 +386,6 @@ public abstract class AbstractJythonDataSetHandlerTest extends AbstractFileSyste
protected boolean didRollbackServiceFunctionRun = false;
- protected boolean didRollbackDataSetRegistrationFunctionRun = false;
-
protected boolean didTransactionRollbackHappen = false;
protected boolean didRollbackTransactionFunctionRunHappen = false;
@@ -470,21 +468,6 @@ public abstract class AbstractJythonDataSetHandlerTest extends AbstractFileSyste
interpreter.get("didRollbackServiceFunctionRun", Boolean.class);
}
- @Override
- protected void invokeRollbackDataSetRegistrationFunction(PyFunction function,
- DataSetRegistrationService<DataSetInformation> service,
- DataSetRegistrationAlgorithm registrationAlgorithm, Throwable throwable)
- {
- super.invokeRollbackDataSetRegistrationFunction(function, service,
- registrationAlgorithm, throwable);
-
- PythonInterpreter interpreter =
- ((JythonDataSetRegistrationService<DataSetInformation>) service)
- .getInterpreter();
- didRollbackDataSetRegistrationFunctionRun =
- interpreter.get("didRollbackServiceFunctionRun", Boolean.class);
- }
-
@Override
protected void invokeRollbackTransactionFunction(PyFunction function,
DataSetRegistrationService<DataSetInformation> service,
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java
index fc425f841f6..176233aacca 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorTest.java
@@ -244,11 +244,21 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
"Postregistration hook error should not prevent succesfull registration.");
testCase.dropboxScriptPath = "testcase-postregistration-hook-failed.py";
testCases.add(testCase);
-
- // TODO: Add more scenarios:
- // - Test move to error
- // - Test moving of the original file in case of validation error
- // - Test other error scenarios
+
+ testCase = new TestCaseParameters("Postregistration hook has wrong signature.");
+ testCase.dropboxScriptPath = "testcase-postregistration-hook-wrong-signature.py";
+ testCase.shouldThrowExceptionDuringRegistration = true;
+ testCase.exceptionAcceptor = new IPredicate<Exception>()
+ {
+ public boolean execute(Exception arg)
+ {
+ System.out.println(arg);
+ System.out.println(arg.getMessage());
+ return arg.getMessage().contains("wrong number of arguments");
+ }
+ };
+ testCase.failurePoint = TestCaseParameters.FailurePoint.AFTER_GET_EXPERIMENT;
+ testCases.add(testCase);
// here is crappy code for
// return parameters.map( (x) => new Object[]{x} )
@@ -306,6 +316,9 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
*/
protected boolean shouldValidationFail = false;
+ /**
+ * Specifies how far we expect the registration process to go.
+ */
protected FailurePoint failurePoint = null;
/**
@@ -335,10 +348,10 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
return title;
}
- //add more when necessary
+ // add more when necessary
public enum FailurePoint
{
- AFTER_CREATE_DATA_SET_CODE, BEFORE_OPENBIS_REGISTRATION
+ AT_THE_BEGINNING, AFTER_CREATE_DATA_SET_CODE, BEFORE_OPENBIS_REGISTRATION, AFTER_GET_EXPERIMENT
}
}
@@ -358,6 +371,7 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
{
createHandler(properties, false, true);
}
+
if (testCase.createDataSetDelegate != null)
{
testCase.createDataSetDelegate.execute();
@@ -376,8 +390,19 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
{
boolean broken = false;
- one(openBisService).createDataSetCode();
- will(returnValue(DATA_SET_CODE));
+ // this is to initialize openBis
+ //allowing(openBisService).getClass();
+
+ if (testCase.failurePoint == TestCaseParameters.FailurePoint.AT_THE_BEGINNING)
+ {
+ broken = true;
+ }
+
+ if (false == broken)
+ {
+ one(openBisService).createDataSetCode();
+ will(returnValue(DATA_SET_CODE));
+ }
if (testCase.failurePoint == TestCaseParameters.FailurePoint.AFTER_CREATE_DATA_SET_CODE)
{
@@ -392,6 +417,11 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
will(returnValue(experiment));
}
+ if (testCase.failurePoint == TestCaseParameters.FailurePoint.AFTER_GET_EXPERIMENT)
+ {
+ broken = true;
+ }
+
if (false == broken)
{
one(dataSetValidator).assertValidDataSet(
@@ -524,7 +554,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
{
} else if (testCase.shouldRegistrationFail)
{
- assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun);
assertFalse(theHandler.didRollbackServiceFunctionRun);
assertTrue(theHandler.didTransactionRollbackHappen);
assertTrue(theHandler.didRollbackTransactionFunctionRunHappen);
@@ -596,7 +625,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
new File(workingDirectory, "data_set/sub_data_set_2/read2.me")).trim());
TestingDataSetHandler theHandler = (TestingDataSetHandler) handler;
- assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun);
assertFalse(theHandler.didRollbackServiceFunctionRun);
// These do not get called when the caller herself invokes a rollback
@@ -1047,7 +1075,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
assertTrue(logAppender.getLogContent(), logAppender.getLogContent().length() > 0);
TestingDataSetHandler theHandler = (TestingDataSetHandler) handler;
- assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun);
assertTrue(theHandler.didRollbackServiceFunctionRun);
context.assertIsSatisfied();
}
@@ -1112,7 +1139,6 @@ public class JythonTopLevelDataSetRegistratorTest extends AbstractJythonDataSetH
TestingDataSetHandler theHandler = (TestingDataSetHandler) handler;
assertFalse(didServiceRollbackHappen);
assertFalse(theHandler.didTransactionRollbackHappen);
- assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun);
assertFalse(theHandler.didRollbackServiceFunctionRun);
context.assertIsSatisfied();
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/testcase-postregistration-hook-wrong-signature.py b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/testcase-postregistration-hook-wrong-signature.py
new file mode 100644
index 00000000000..4669c577aaa
--- /dev/null
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/testcase-postregistration-hook-wrong-signature.py
@@ -0,0 +1,8 @@
+execfile("sourceTest/java/ch/systemsx/cisd/etlserver/registrator/all-hooks.py")
+
+def post_metadata_registration(transaction, context, unnecessary_argument):
+ global didPostRegistrationFunctionRunHappen
+ didPostRegistrationFunctionRunHappen = True
+
+execfile("sourceTest/java/ch/systemsx/cisd/etlserver/registrator/simple-transaction.py")
+
--
GitLab