diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java index 42217f41c437b46f4248bd2f1c91f646b58dafd1..29f20fc6535473e4ebfe03a19f41f6a19fe6f18e 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DefaultStorageProcessor.java @@ -127,6 +127,11 @@ public class DefaultStorageProcessor extends AbstractStorageProcessor @Override protected UnstoreDataAction executeRollback(Throwable ex) { + // This might happen when the transaction is serialized and de-serialized. + if (null == storedDataDirectory) + { + storedDataDirectory = rootDirectory; + } checkParameters(incomingDataSetDirectory, storedDataDirectory); File targetFile = new File(getOriginalDirectory(storedDataDirectory), @@ -138,9 +143,10 @@ public class DefaultStorageProcessor extends AbstractStorageProcessor FileUtils.deleteDirectory(storedDataDirectory); } catch (IOException ex1) { - String message = String.format("Failed to remove stored directory '%s'. " + - "In the future the creation of a data set with the same code will fail. " + - "To fix the problem remove the directory manually."); + String message = + String.format("Failed to remove stored directory '%s'. " + + "In the future the creation of a data set with the same code will fail. " + + "To fix the problem remove the directory manually."); operationLog.warn(message); } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java index f140be84508a429e06777dbb24f301fba6672747..341626369782b6ec24f1c3e8abbb7e760ee20621 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithm.java @@ -161,9 +161,10 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> } /** - * Ask the storage processor to rollback. Used by clients of the algorithm. + * Transition to the rolledback state, but don't actually do anything. The rollback logic will + * be carried out by the rollback stack. */ - public void rollbackStorageProcessor(Throwable throwable) + public void transitionToRolledbackState(Throwable throwable) { // Rollback may be called on in the stored state or in the prepared state. if (state instanceof PreparedState) @@ -182,12 +183,12 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> } StoredState<T> storedState = (StoredState<T>) state; - UnstoreDataAction action = storedState.rollbackStorageProcessor(throwable); + storedState.cleanUp(); - state = new RolledbackState<T>(storedState, action, throwable); + state = new RolledbackState<T>(storedState, UnstoreDataAction.LEAVE_UNTOUCHED, throwable); } - public void executeUndoStoreAction() + public void transitionToUndoneState() { // Rollback may be called on in the stored state or in the prepared state. In the prepared // state, there is nothing to do. @@ -199,10 +200,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> RolledbackState<T> rolledbackState = (RolledbackState<T>) state; - // Do not undo the individual data sets -- the entire transaction needs to be undone as a - // group - // rolledbackState.executeUndoAction(); - state = new UndoneState<T>(rolledbackState); } @@ -445,16 +442,6 @@ public class DataSetStorageAlgorithm<T extends DataSetInformation> this.markerFile = oldState.markerFile; } - /** - * Ask the storage processor to rollback. Used by clients of the algorithm. - */ - public UnstoreDataAction rollbackStorageProcessor(final Throwable throwable) - { - UnstoreDataAction action = transaction.rollback(throwable); - cleanUp(); - return action; - } - /** * Ask the storage processor to commit. Used by clients of the algorithm. */ diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java index aefecd97811393e2b6b583de843533796fa69871..0b874d51e50cb718e365f6e88a399865c697a558 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/DataSetStorageAlgorithmRunner.java @@ -254,7 +254,6 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation> } } - @SuppressWarnings("deprecation") private void rollbackStorageProcessors(Throwable ex) { operationLog.error( @@ -272,10 +271,8 @@ public class DataSetStorageAlgorithmRunner<T extends DataSetInformation> for (int i = dataSetStorageAlgorithms.size() - 1; i >= 0; --i) { DataSetStorageAlgorithm<T> storageAlgorithm = dataSetStorageAlgorithms.get(i); - storageAlgorithm.rollbackStorageProcessor(ex); - storageAlgorithm.executeUndoStoreAction(); - // remove the serialized transaction - rollbackStack.pop(); + storageAlgorithm.transitionToRolledbackState(ex); + storageAlgorithm.transitionToUndoneState(); } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java index 375804015705dccd63883a2b8f363e702862f293..712de5d26ef32eb4253504ec844c19a0c8dd6474 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/IRollbackStack.java @@ -29,13 +29,4 @@ public interface IRollbackStack * Push the command onto the stack and execute it. */ void pushAndExecuteCommand(ITransactionalCommand cmd); - - /** - * Pop the value from the top of the stack *without* rollbacking. - * <p> - * This method should not be called - */ - @Deprecated - ITransactionalCommand pop(); - } \ No newline at end of file diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java index 90ddfa01a4554ce345f982661ededd4699bde364..9fb200a765c4e3e5eb609f9f7c34c670608f5a18 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/registrator/JythonTopLevelDataSetRegistratorRollbackTest.java @@ -20,14 +20,14 @@ import static ch.systemsx.cisd.common.Constants.IS_FINISHED_PREFIX; import java.io.File; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.Properties; import org.apache.commons.io.FileUtils; import org.hamcrest.core.IsAnything; import org.jmock.Expectations; +import org.jmock.api.Invocation; +import org.jmock.lib.action.CustomAction; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -35,6 +35,8 @@ import ch.systemsx.cisd.base.exceptions.IOExceptionUnchecked; import ch.systemsx.cisd.common.filesystem.FileUtilities; import ch.systemsx.cisd.common.mail.IMailClient; import ch.systemsx.cisd.common.utilities.ExtendedProperties; +import ch.systemsx.cisd.etlserver.AbstractStorageProcessorTransaction; +import ch.systemsx.cisd.etlserver.DefaultStorageProcessor; import ch.systemsx.cisd.etlserver.IStorageProcessorTransactional; import ch.systemsx.cisd.etlserver.ITypeExtractor; import ch.systemsx.cisd.etlserver.registrator.api.v1.impl.DataSetRegistrationTransaction; @@ -44,7 +46,6 @@ import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.SearchCriteria.MatchCl import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.builders.ExperimentBuilder; -import ch.systemsx.cisd.openbis.generic.shared.dto.StorageFormat; import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory; /** @@ -78,7 +79,7 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython Properties properties = createThreadPropertiesRelativeToScriptsFolder("simple-transaction.py"); properties.setProperty(IStorageProcessorTransactional.STORAGE_PROCESSOR_KEY, - MockStorageProcessor.class.getName()); + DefaultStorageProcessor.class.getName()); createHandler(properties, false, false); createData(); @@ -99,7 +100,19 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython one(openBisService) .performEntityOperations( with(new IsAnything<ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails>())); - will(throwException(new AssertionError("Fail"))); + + CustomAction makeFileSystemUnavailable = new CustomAction("foo") + { + + @Override + public Object invoke(Invocation invocation) throws Throwable + { + makeFileSystemUnavailable(workingDirectory); + return null; + } + }; + will(doAll(makeFileSystemUnavailable, + throwException(new AssertionError("Fail")))); } }); @@ -113,8 +126,6 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython makeFileSystemAvailable(workingDirectory); DataSetRegistrationTransaction.rollbackDeadTransactions(workingDirectory); } - assertEquals(1, MockStorageProcessor.instance.incomingDirs.size()); - assertEquals(0, MockStorageProcessor.instance.calledCommitCount); assertEquals("[]", Arrays.asList(stagingDirectory.list()).toString()); assertEquals( "hello world1", @@ -125,12 +136,6 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython FileUtilities.loadToString( new File(workingDirectory, "data_set/sub_data_set_2/read2.me")).trim()); - TestingDataSetHandler theHandler = (TestingDataSetHandler) handler; - assertFalse(theHandler.didRollbackDataSetRegistrationFunctionRun); - assertFalse(theHandler.didRollbackServiceFunctionRun); - assertTrue(theHandler.didTransactionRollbackHappen); - assertTrue(theHandler.didRollbackTransactionFunctionRunHappen); - context.assertIsSatisfied(); } @@ -167,104 +172,73 @@ public class JythonTopLevelDataSetRegistratorRollbackTest extends AbstractJython { private static final long serialVersionUID = 1L; - static MockStorageProcessor instance; - - int calledGetStoreRootDirectoryCount = 0; - - int calledCommitCount = 0; - - File storeRootDirectory; - - String dataSetInfoString; - - protected List<File> incomingDirs = new ArrayList<File>(); - - protected List<File> rootDirs = new ArrayList<File>(); - public MockStorageProcessor(ExtendedProperties props) { super(props); instance = this; } - public File getStoreRootDirectory() - { - calledGetStoreRootDirectoryCount++; - return storeRootDirectory; - } - - public void setStoreRootDirectory(File storeRootDirectory) - { - this.storeRootDirectory = storeRootDirectory; - } - - public StorageFormat getStorageFormat() - { - return StorageFormat.PROPRIETARY; - } - - public UnstoreDataAction getDefaultUnstoreDataAction(Throwable exception) - { - return UnstoreDataAction.LEAVE_UNTOUCHED; - } - + @Override public IStorageProcessorTransaction createTransaction( - StorageProcessorTransactionParameters parameters) + final StorageProcessorTransactionParameters parameters) { - final File rootDir = parameters.getRootDir(); - dataSetInfoString = parameters.getDataSetInformation().toString(); - return new IStorageProcessorTransaction() + return new AbstractStorageProcessorTransaction(parameters) { private static final long serialVersionUID = 1L; - private File storedFolder; - - public void storeData(ITypeExtractor typeExtractor, IMailClient mailClient, - File incomingDataSetFile) + @Override + public File tryGetProprietaryData() { + return null; + } - incomingDirs.add(incomingDataSetFile); - rootDirs.add(rootDir); - + @Override + protected File executeStoreData(ITypeExtractor typeExtractor, + IMailClient mailClient) + { try { - if (incomingDataSetFile.isDirectory()) + if (incomingDataSetDirectory.isDirectory()) { - FileUtils.moveDirectoryToDirectory(incomingDataSetFile, new File( - rootDir, "original"), true); + FileUtils.moveDirectoryToDirectory(incomingDataSetDirectory, + new File(rootDirectory, "original"), true); } else { - FileUtils.moveFileToDirectory(incomingDataSetFile, new File( - rootDir, "original"), false); + FileUtils.moveFileToDirectory(incomingDataSetDirectory, new File( + rootDirectory, "original"), false); } makeFileSystemUnavailable(getStoreRootDirectory()); } catch (IOException ex) { throw new IOExceptionUnchecked(ex); } - storedFolder = rootDir; + return rootDirectory; } - public UnstoreDataAction rollback(Throwable exception) + public UnstoreDataAction executeRollback(Throwable exception) { - return null; + rollback(incomingDataSetDirectory, rootDirectory); + return UnstoreDataAction.LEAVE_UNTOUCHED; } - public File getStoredDataDirectory() + private void rollback(File incomingDataSetFile, File aRootDir) { - return storedFolder; + try + { + FileUtils.moveDirectoryToDirectory(new File(aRootDir, "original"), + incomingDataSetFile, true); + } catch (IOException ex) + { + throw new IOExceptionUnchecked(ex); + } } - public void commit() + @Override + protected void executeCommit() { calledCommitCount++; } - - public File tryGetProprietaryData() - { - return null; - } }; } }