From d53001e2cf8f763becdc60b3c512748ee2fa130c Mon Sep 17 00:00:00 2001 From: brinn <brinn> Date: Mon, 15 Nov 2010 22:43:17 +0000 Subject: [PATCH] fix: add timeout for remote connection attempts SVN: 18719 --- .../server/plugins/standard/Copier.java | 3 +- .../plugins/standard/DataSetCopierTest.java | 6 +- .../server/plugins/MsInjectionCopier.java | 63 ++++++++------- .../server/plugins/MsInjectionCopierTest.java | 79 +++++++++++-------- 4 files changed, 87 insertions(+), 64 deletions(-) diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/Copier.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/Copier.java index 78bd535cbea..b10ed5322f1 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/Copier.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/Copier.java @@ -88,7 +88,8 @@ public class Copier implements Serializable, IPostRegistrationDatasetHandler copier.check(); if (host != null) { - FileUtilities.checkPathCopier(copier, host, null, rsyncModule, rsyncPasswordFile); + FileUtilities.checkPathCopier(copier, host, null, rsyncModule, rsyncPasswordFile, + DataSetCopier.SSH_TIMEOUT_MILLIS); } File destination = hostAwareFile.getFile(); File destinationFile = new File(destination, originalData.getName()); diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopierTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopierTest.java index e29d35ede47..c743832f62d 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopierTest.java +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopierTest.java @@ -467,11 +467,13 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase { exactly(numberOfExpectedCreations).of(copier) .checkRsyncConnectionViaRsyncServer(hostOrNull, - rsyncModuleOrNull, rsyncModuleOrNull + "-password"); + rsyncModuleOrNull, rsyncModuleOrNull + "-password", + DataSetCopier.SSH_TIMEOUT_MILLIS); } else { exactly(numberOfExpectedCreations).of(copier) - .checkRsyncConnectionViaSsh(hostOrNull, null); + .checkRsyncConnectionViaSsh(hostOrNull, null, + DataSetCopier.SSH_TIMEOUT_MILLIS); } will(returnValue(checkingResult)); } diff --git a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopier.java b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopier.java index 991a096f5f9..14e0627f6bb 100644 --- a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopier.java +++ b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopier.java @@ -47,18 +47,18 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.IPostRegistrationDatasetHandl import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; /** - * - * * @author Franz-Josef Elmer */ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler { - - @Private static final String SAMPLE_UNKNOWN = "sample-unknown"; + + @Private + static final String SAMPLE_UNKNOWN = "sample-unknown"; private static final class ExceptionWithStatus extends RuntimeException { private static final long serialVersionUID = 1L; + private final Status status; ExceptionWithStatus(Status status) @@ -77,15 +77,18 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler return status; } } - + private static interface IExecutor { BooleanStatus exists(File file); + void deleteFolder(File folder); + void copyDataSet(File dataSet, File destination); + void renameTo(File newFile, File oldFile); } - + private static final class LocalExcecutor implements IExecutor { private final IFileOperations fileOperations; @@ -94,7 +97,7 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler { this.fileOperations = fileOperations; } - + public BooleanStatus exists(File file) { return BooleanStatus.createFromBoolean(fileOperations.exists(file)); @@ -140,15 +143,19 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler throw new ExceptionWithStatus(Status.createError("rename failed")); } } - + } - + private static final class RemoteExecutor implements IExecutor { private final ISshCommandExecutor executor; + private final IPathCopier copier; + private final String host; + private final String rsyncModuleNameOrNull; + private final String rsyncPasswordFileOrNull; public RemoteExecutor(ISshCommandExecutor executor, IPathCopier copier, String host, @@ -160,7 +167,7 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler this.rsyncModuleNameOrNull = rsyncModuleNameOrNull; this.rsyncPasswordFileOrNull = rsyncPasswordFileOrNull; } - + public BooleanStatus exists(File file) { return executor.exists(file.getPath(), DataSetCopier.SSH_TIMEOUT_MILLIS); @@ -169,12 +176,12 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler public void deleteFolder(File folder) { ProcessResult result = - executor.executeCommandRemotely("rm -rf " + folder.getPath(), - DataSetCopier.SSH_TIMEOUT_MILLIS); + executor.executeCommandRemotely("rm -rf " + folder.getPath(), + DataSetCopier.SSH_TIMEOUT_MILLIS); if (result.isOK() == false) { - operationLog.error("Remote deletion of '" + folder - + "' failed with exit value: " + result.getExitValue()); + operationLog.error("Remote deletion of '" + folder + "' failed with exit value: " + + result.getExitValue()); throw new ExceptionWithStatus(Status.createError("couldn't delete")); } List<String> output = result.getOutput(); @@ -186,7 +193,7 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler throw new ExceptionWithStatus(Status.createError("deletion leads to a problem")); } } - + public void copyDataSet(File dataSet, File destination) { Status result = @@ -201,19 +208,19 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler public void renameTo(File newFile, File oldFile) { ProcessResult result = - executor.executeCommandRemotely("mv " + oldFile.getPath() + " " - + newFile.getPath(), DataSetCopier.SSH_TIMEOUT_MILLIS); + executor.executeCommandRemotely( + "mv " + oldFile.getPath() + " " + newFile.getPath(), + DataSetCopier.SSH_TIMEOUT_MILLIS); if (result.isOK() == false) { - operationLog.error("Remote move of '" + oldFile - + "' to '" + newFile + "' failed with exit value: " + result.getExitValue()); + operationLog.error("Remote move of '" + oldFile + "' to '" + newFile + + "' failed with exit value: " + result.getExitValue()); throw new ExceptionWithStatus(Status.createError("couldn't move")); } List<String> output = result.getOutput(); if (output.isEmpty() == false) { - operationLog.error("Remote move of '" + oldFile - + "' to '" + newFile + operationLog.error("Remote move of '" + oldFile + "' to '" + newFile + "' seemed to be successful but produced following output:\n" + StringUtilities.concatenateWithNewLine(output)); throw new ExceptionWithStatus(Status.createError("moving leads to a problem")); @@ -223,8 +230,8 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler private static final long serialVersionUID = 1L; - private final static Logger operationLog = - LogFactory.getLogger(LogCategory.OPERATION, MsInjectionCopier.class); + private final static Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, + MsInjectionCopier.class); private final Properties properties; @@ -233,9 +240,9 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler private final ISshCommandExecutorFactory sshCommandExecutorFactory; private transient IExecutor executor; - + private transient File destination; - + MsInjectionCopier(Properties properties, IPathCopierFactory pathCopierFactory, ISshCommandExecutorFactory sshCommandExecutorFactory) { @@ -264,7 +271,8 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler String rsyncModule = hostAwareFile.tryGetRsyncModule(); String rsyncPasswordFile = properties.getProperty(DataSetCopier.RSYNC_PASSWORD_FILE_KEY); - FileUtilities.checkPathCopier(copier, hostOrNull, null, rsyncModule, rsyncPasswordFile); + FileUtilities.checkPathCopier(copier, hostOrNull, null, rsyncModule, rsyncPasswordFile, + DataSetCopier.SSH_TIMEOUT_MILLIS); ISshCommandExecutor sshCommandExecutor = sshCommandExecutorFactory.create(sshExecutable, hostOrNull); executor = @@ -296,7 +304,7 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler executor.copyDataSet(originalData, destination); executor.renameTo(targetFolder, new File(destination, originalData.getName())); return Status.OK; - + } catch (ExceptionWithStatus ex) { return ex.getStatus(); @@ -319,4 +327,3 @@ class MsInjectionCopier implements Serializable, IPostRegistrationDatasetHandler } } - diff --git a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopierTest.java b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopierTest.java index e1dc456e57f..02e084c501c 100644 --- a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopierTest.java +++ b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/dss/phosphonetx/server/plugins/MsInjectionCopierTest.java @@ -44,32 +44,44 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType; /** - * - * * @author Franz-Josef Elmer */ -@Friend(toClasses=MsInjectionCopier.class) +@Friend(toClasses = MsInjectionCopier.class) public class MsInjectionCopierTest extends AbstractFileSystemTestCase { - private static final ProcessResult OK_RESULT = - new ProcessResult(Arrays.asList(""), 0, null, null, 0, null, null, null); + private static final ProcessResult OK_RESULT = new ProcessResult(Arrays.asList(""), 0, null, + null, 0, null, null, null); + private static final String SAMPLE_CODE = "my-sample"; + private static final DataSetType DATA_SET_TYPE = new DataSetType("MY"); + private static final String DATA_SET_CODE = "my-dataset-123"; + private static final String FOLDER_NAME = DATA_SET_CODE; + private static final String DATA = "hello test"; - + private Mockery context; + private File dataSet; + private IPathCopierFactory copierFactory; + private IPathCopier copier; + private ISshCommandExecutorFactory sshExecutorFactory; + private ISshCommandExecutor sshExecutor; + private File destination; + private File dataFile; + private File rsyncExec; + private File sshExec; - + @BeforeMethod public void beforeMethod() throws Exception { @@ -89,7 +101,7 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase sshExec = new File(workingDirectory, "my-rssh"); sshExec.createNewFile(); } - + @AfterMethod public void afterMethod() { @@ -97,7 +109,7 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase // Otherwise one do not known which test failed. context.assertIsSatisfied(); } - + @Test public void testLocalWithKnownSample() { @@ -106,45 +118,45 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase MsInjectionCopier msInjectionCopier = new MsInjectionCopier(properties, copierFactory, sshExecutorFactory); prepareForCheckingLastModifiedDate(); - + DataSetInformation dataSetInformation = new DataSetInformation(); dataSetInformation.setDataSetCode(DATA_SET_CODE); dataSetInformation.setDataSetType(DATA_SET_TYPE); HashMap<String, String> parameterBindings = new HashMap<String, String>(); parameterBindings.put(DATA_SET_CODE, SAMPLE_CODE); Status status = msInjectionCopier.handle(dataSet, dataSetInformation, parameterBindings); - + assertEquals(Status.OK, status); File copiedDataSet = new File(destination, FOLDER_NAME); assertEquals(true, copiedDataSet.isDirectory()); assertEquals(dataSet.lastModified(), copiedDataSet.lastModified()); - assertEquals(DATA, FileUtilities.loadToString(new File(copiedDataSet, dataFile.getName())).trim()); - + assertEquals(DATA, FileUtilities.loadToString(new File(copiedDataSet, dataFile.getName())) + .trim()); + context.assertIsSatisfied(); } - + @Test public void testLocalWithUnknownSample() { Properties properties = new Properties(); properties.setProperty(DataSetCopier.DESTINATION_KEY, destination.getPath()); MsInjectionCopier msInjectionCopier = - new MsInjectionCopier(properties, copierFactory, sshExecutorFactory); - + new MsInjectionCopier(properties, copierFactory, sshExecutorFactory); + DataSetInformation dataSetInformation = new DataSetInformation(); dataSetInformation.setDataSetCode(DATA_SET_CODE); dataSetInformation.setDataSetType(DATA_SET_TYPE); HashMap<String, String> parameterBindings = new HashMap<String, String>(); Status status = msInjectionCopier.handle(dataSet, dataSetInformation, parameterBindings); - + assertEquals(Status.OK, status); - File copiedDataSet = - new File(destination, DATA_SET_CODE); + File copiedDataSet = new File(destination, DATA_SET_CODE); assertEquals(true, copiedDataSet.isDirectory()); - + context.assertIsSatisfied(); } - + @Test public void testLocalWithAlreadyExistingDestination() { @@ -156,22 +168,23 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase copiedDataSet.mkdirs(); File dummy = new File(copiedDataSet, "dummy"); FileUtilities.writeToFile(dummy, "hello"); - + DataSetInformation dataSetInformation = new DataSetInformation(); dataSetInformation.setDataSetCode(DATA_SET_CODE); dataSetInformation.setDataSetType(DATA_SET_TYPE); HashMap<String, String> parameterBindings = new HashMap<String, String>(); parameterBindings.put(DATA_SET_CODE, SAMPLE_CODE); Status status = msInjectionCopier.handle(dataSet, dataSetInformation, parameterBindings); - + assertEquals(Status.OK, status); assertEquals(true, copiedDataSet.isDirectory()); - assertEquals(DATA, FileUtilities.loadToString(new File(copiedDataSet, dataFile.getName())).trim()); + assertEquals(DATA, FileUtilities.loadToString(new File(copiedDataSet, dataFile.getName())) + .trim()); assertEquals(false, dummy.exists()); - + context.assertIsSatisfied(); } - + @Test public void testRemote() { @@ -184,14 +197,15 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase { one(copierFactory).create(rsyncExec, sshExec); will(returnValue(copier)); - + one(copier).check(); - one(copier).checkRsyncConnectionViaSsh("localhost", null); + one(copier).checkRsyncConnectionViaSsh("localhost", null, + DataSetCopier.SSH_TIMEOUT_MILLIS); will(returnValue(true)); - + one(sshExecutorFactory).create(sshExec, "localhost"); will(returnValue(sshExecutor)); - + final String copiedDataSet = new File(destination, FOLDER_NAME).getPath(); one(sshExecutor).exists(copiedDataSet, SSH_TIMEOUT_MILLIS); will(returnValue(BooleanStatus.createTrue())); @@ -218,11 +232,11 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase HashMap<String, String> parameterBindings = new HashMap<String, String>(); parameterBindings.put(DATA_SET_CODE, SAMPLE_CODE); Status status = msInjectionCopier.handle(dataSet, dataSetInformation, parameterBindings); - + assertEquals(Status.OK, status); context.assertIsSatisfied(); } - + private void prepareForCheckingLastModifiedDate() { // Sleep long enough to test last modified date of target will be same as of source. @@ -235,4 +249,3 @@ public class MsInjectionCopierTest extends AbstractFileSystemTestCase } } } - -- GitLab