From 4b187c6d39e0f84bf6985b85e36cf3a102207260 Mon Sep 17 00:00:00 2001
From: buczekp <buczekp>
Date: Tue, 2 Feb 2010 09:44:40 +0000
Subject: [PATCH] [LMS-1352] fixed and extended some tests

SVN: 14600
---
 .../plugins/standard/DataSetCopier.java       |  45 ++++--
 .../plugins/standard/DataSetCopierTest.java   | 143 ++++++++++++------
 2 files changed, 129 insertions(+), 59 deletions(-)

diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopier.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopier.java
index cbb446bc04a..b8fbb1774d2 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopier.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/DataSetCopier.java
@@ -28,6 +28,7 @@ import ch.systemsx.cisd.common.filesystem.BooleanStatus;
 import ch.systemsx.cisd.common.filesystem.FileUtilities;
 import ch.systemsx.cisd.common.filesystem.IPathCopier;
 import ch.systemsx.cisd.common.filesystem.rsync.RsyncCopier;
+import ch.systemsx.cisd.common.filesystem.ssh.ISshCommandExecutor;
 import ch.systemsx.cisd.common.filesystem.ssh.SshCommandExecutor;
 import ch.systemsx.cisd.common.highwatermark.HostAwareFile;
 import ch.systemsx.cisd.common.highwatermark.HostAwareFileWithHighwaterMark;
@@ -58,13 +59,15 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
 
     private static final String ALREADY_EXIST_MSG = "already exist";
 
-    private static final String COPYING_FAILED_MSG = "copying failed";
+    @Private
+    static final String COPYING_FAILED_MSG = "copying failed";
 
     private static final String RSYNC_EXEC = "rsync";
 
     private static final String SSH_EXEC = "ssh";
 
-    private static final long SSH_TIMEOUT_MILLIS = 15 * 1000; // 15s
+    @Private
+    static final long SSH_TIMEOUT_MILLIS = 15 * 1000; // 15s
 
     @Private
     static interface IPathCopierFactory
@@ -82,6 +85,23 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
         }
     }
 
+    @Private
+    static interface ISshCommandExecutorFactory
+    {
+        ISshCommandExecutor create(File sshExecutableOrNull, String host);
+    }
+
+    private static final class SshCommandExecutorFactory implements Serializable,
+            ISshCommandExecutorFactory
+    {
+        private static final long serialVersionUID = 1L;
+
+        public ISshCommandExecutor create(File sshExecutableOrNull, String host)
+        {
+            return new SshCommandExecutor(sshExecutableOrNull, host);
+        }
+    }
+
     private static final class Copier implements Serializable, IPostRegistrationDatasetHandler
     {
 
@@ -93,7 +113,7 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
 
         private final File sshExecutable;
 
-        private final SshCommandExecutor sshCommandExecutor;
+        private final ISshCommandExecutor sshCommandExecutor;
 
         private final Properties properties;
 
@@ -105,19 +125,20 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
 
         private final String rsyncPasswordFile;
 
-        private final IPathCopierFactory factory;
+        private final IPathCopierFactory pathCopierFactory;
 
-        public Copier(Properties properties, IPathCopierFactory factory)
+        public Copier(Properties properties, IPathCopierFactory pathCopierFactory,
+                ISshCommandExecutorFactory sshCommandExecutorFactory)
         {
             this.properties = properties;
-            this.factory = factory;
+            this.pathCopierFactory = pathCopierFactory;
             rsyncPasswordFile = properties.getProperty(RSYNC_PASSWORD_FILE_KEY);
             rsyncExecutable = getExecutable(RSYNC_EXEC);
             sshExecutable = getExecutable(SSH_EXEC);
             HostAwareFile hostAwareFile =
                     HostAwareFileWithHighwaterMark.fromProperties(properties, DESTINATION_KEY);
             host = hostAwareFile.tryGetHost();
-            sshCommandExecutor = new SshCommandExecutor(sshExecutable, host);
+            sshCommandExecutor = sshCommandExecutorFactory.create(sshExecutable, host);
             rsyncModule = hostAwareFile.tryGetRsyncModule();
             destination = hostAwareFile.getFile();
             getCopier();
@@ -127,7 +148,7 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
         {
             if (copier == null)
             {
-                copier = factory.create(rsyncExecutable, sshExecutable);
+                copier = pathCopierFactory.create(rsyncExecutable, sshExecutable);
                 copier.check();
                 if (host != null)
                 {
@@ -214,13 +235,15 @@ public class DataSetCopier extends AbstractDropboxProcessingPlugin
 
     public DataSetCopier(Properties properties, File storeRoot)
     {
-        this(properties, storeRoot, new RsyncCopierFactory());
+        this(properties, storeRoot, new RsyncCopierFactory(), new SshCommandExecutorFactory());
     }
 
     @Private
-    DataSetCopier(Properties properties, File storeRoot, IPathCopierFactory factory)
+    DataSetCopier(Properties properties, File storeRoot, IPathCopierFactory pathCopierFactory,
+            ISshCommandExecutorFactory sshCommandExecutorFactory)
     {
-        super(properties, storeRoot, new Copier(properties, factory));
+        super(properties, storeRoot, new Copier(properties, pathCopierFactory,
+                sshCommandExecutorFactory));
     }
 
 }
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 f0060bd6105..0955afaca64 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
@@ -33,10 +33,13 @@ import org.testng.annotations.Test;
 import ch.rinn.restrictions.Friend;
 import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
 import ch.systemsx.cisd.common.exceptions.ConfigurationFailureException;
-import ch.systemsx.cisd.common.exceptions.EnvironmentFailureException;
 import ch.systemsx.cisd.common.exceptions.Status;
+import ch.systemsx.cisd.common.filesystem.BooleanStatus;
 import ch.systemsx.cisd.common.filesystem.IPathCopier;
+import ch.systemsx.cisd.common.filesystem.ssh.ISshCommandExecutor;
 import ch.systemsx.cisd.openbis.dss.generic.server.plugins.standard.DataSetCopier.IPathCopierFactory;
+import ch.systemsx.cisd.openbis.dss.generic.server.plugins.standard.DataSetCopier.ISshCommandExecutorFactory;
+import ch.systemsx.cisd.openbis.dss.generic.server.plugins.tasks.ProcessingStatus;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatasetDescription;
 
 /**
@@ -51,10 +54,14 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
 
     private Mockery context;
 
-    private IPathCopierFactory factory;
+    private IPathCopierFactory pathFactory;
+
+    private ISshCommandExecutorFactory sshFactory;
 
     private IPathCopier copier;
 
+    private ISshCommandExecutor sshCommandExecutor;
+
     private File storeRoot;
 
     private File sshExecutableDummy;
@@ -75,8 +82,10 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
     public void beforeMethod() throws IOException
     {
         context = new Mockery();
-        factory = context.mock(DataSetCopier.IPathCopierFactory.class);
+        pathFactory = context.mock(DataSetCopier.IPathCopierFactory.class);
+        sshFactory = context.mock(DataSetCopier.ISshCommandExecutorFactory.class);
         copier = context.mock(IPathCopier.class);
+        sshCommandExecutor = context.mock(ISshCommandExecutor.class);
         storeRoot = new File(workingDirectory, "store");
         storeRoot.mkdirs();
         sshExecutableDummy = new File(workingDirectory, "my-ssh");
@@ -111,7 +120,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
     {
         try
         {
-            new DataSetCopier(new Properties(), storeRoot, factory);
+            new DataSetCopier(new Properties(), storeRoot, pathFactory, sshFactory);
             fail("ConfigurationFailureException expected");
         } catch (ConfigurationFailureException ex)
         {
@@ -128,7 +137,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         rsyncExecutableDummy.delete();
         try
         {
-            new DataSetCopier(properties, storeRoot, factory);
+            new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
             fail("ConfigurationFailureException expected");
         } catch (ConfigurationFailureException ex)
         {
@@ -145,7 +154,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         sshExecutableDummy.delete();
         try
         {
-            new DataSetCopier(properties, storeRoot, factory);
+            new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
             fail("ConfigurationFailureException expected");
         } catch (ConfigurationFailureException ex)
         {
@@ -163,7 +172,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         prepareCreateAndCheckCopier("host", null, false);
         try
         {
-            new DataSetCopier(properties, storeRoot, factory);
+            new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
             fail("ConfigurationFailureException expected");
         } catch (ConfigurationFailureException ex)
         {
@@ -181,7 +190,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         prepareCreateAndCheckCopier("host", "abc", false);
         try
         {
-            new DataSetCopier(properties, storeRoot, factory);
+            new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
             fail("ConfigurationFailureException expected");
         } catch (ConfigurationFailureException ex)
         {
@@ -205,7 +214,8 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
                     will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
         dataSetCopier.process(Arrays.asList(ds1, ds2));
 
@@ -222,37 +232,45 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
                 {
                     one(copier).copyToRemote(ds1Data, new File("tmp/test"), null, null, null);
                     will(returnValue(Status.createError("error message")));
+
+                    one(copier).copyToRemote(ds2Data, new File("tmp/test"), null, null, null);
+                    will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
-        try
-        {
-            dataSetCopier.process(Arrays.asList(ds1, ds2));
-            fail("EnvironmentFailureException expected");
-        } catch (EnvironmentFailureException ex)
-        {
-            assertEquals(
-                    "Could not copy data set ds1 to destination folder 'tmp/test': error message",
-                    ex.getMessage());
-        }
+        ProcessingStatus processingStatus = dataSetCopier.process(Arrays.asList(ds1, ds2));
+
+        // processing first data set fails but second one is processed successfully
+        Status errorStatus = Status.createError(DataSetCopier.COPYING_FAILED_MSG);
+        assertEquals(1, processingStatus.getErrorStatuses().size());
+        assertEquals(errorStatus, processingStatus.getErrorStatuses().get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(errorStatus).size());
+        assertEquals(ds1, processingStatus.getDatasetsByStatus(errorStatus).get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(Status.OK).size());
+        assertEquals(ds2, processingStatus.getDatasetsByStatus(Status.OK).get(0));
 
         context.assertIsSatisfied();
     }
 
     @Test
-    public void testCopyRemotlyViaSSH()
+    public void testCopyRemotelyViaSsh()
     {
         properties.setProperty(DESTINATION_KEY, "host:tmp/test");
         prepareCreateAndCheckCopier("host", null, true);
         context.checking(new Expectations()
             {
                 {
+                    one(sshCommandExecutor).exists(new File("tmp/test/data.txt"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
                     one(copier).copyToRemote(ds1Data, new File("tmp/test"), "host", null, null);
                     will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
         dataSetCopier.process(Arrays.asList(ds1));
 
@@ -260,35 +278,45 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
     }
 
     @Test
-    public void testCopyRemotelyViaSSH()
+    public void testCopyRemotelyViaSshWithErrors()
     {
         properties.setProperty(DESTINATION_KEY, "host:tmp/test");
         prepareCreateAndCheckCopier("host", null, true);
         context.checking(new Expectations()
             {
                 {
+                    one(sshCommandExecutor).exists(new File("tmp/test/data.txt"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
                     one(copier).copyToRemote(ds1Data, new File("tmp/test"), "host", null, null);
                     will(returnValue(Status.createError("error message")));
+
+                    one(sshCommandExecutor).exists(new File("tmp/test/images"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
+                    one(copier).copyToRemote(ds2Data, new File("tmp/test"), "host", null, null);
+                    will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
-        try
-        {
-            dataSetCopier.process(Arrays.asList(ds1, ds2));
-            fail("EnvironmentFailureException expected");
-        } catch (EnvironmentFailureException ex)
-        {
-            assertEquals(
-                    "Could not copy data set ds1 to destination folder 'tmp/test' on host 'host': "
-                            + "error message", ex.getMessage());
-        }
+        ProcessingStatus processingStatus = dataSetCopier.process(Arrays.asList(ds1, ds2));
+
+        // processing first data set fails but second one is processed successfully
+        Status errorStatus = Status.createError(DataSetCopier.COPYING_FAILED_MSG);
+        assertEquals(1, processingStatus.getErrorStatuses().size());
+        assertEquals(errorStatus, processingStatus.getErrorStatuses().get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(errorStatus).size());
+        assertEquals(ds1, processingStatus.getDatasetsByStatus(errorStatus).get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(Status.OK).size());
+        assertEquals(ds2, processingStatus.getDatasetsByStatus(Status.OK).get(0));
 
         context.assertIsSatisfied();
     }
 
     @Test
-    public void testCopyRemotlyViaRsyncServer()
+    public void testCopyRemotelyViaRsyncServer()
     {
         properties.setProperty(DESTINATION_KEY, "host:abc:tmp/test");
         properties.setProperty(RSYNC_PASSWORD_FILE_KEY, "abc-password");
@@ -296,12 +324,16 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         context.checking(new Expectations()
             {
                 {
+                    one(sshCommandExecutor).exists(new File("tmp/test/data.txt"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
                     one(copier).copyToRemote(ds1Data, new File("tmp/test"), "host", "abc",
                             "abc-password");
                     will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
         dataSetCopier.process(Arrays.asList(ds1));
 
@@ -309,7 +341,7 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
     }
 
     @Test
-    public void testCopyRemotelyViaRsyncServer()
+    public void testCopyRemotelyViaRsyncServerWithErrors()
     {
         properties.setProperty(DESTINATION_KEY, "host:abc:tmp/test");
         properties.setProperty(RSYNC_PASSWORD_FILE_KEY, "abc-password");
@@ -317,22 +349,34 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         context.checking(new Expectations()
             {
                 {
+                    one(sshCommandExecutor).exists(new File("tmp/test/data.txt"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
                     one(copier).copyToRemote(ds1Data, new File("tmp/test"), "host", "abc",
                             "abc-password");
                     will(returnValue(Status.createError("error message")));
+
+                    one(sshCommandExecutor).exists(new File("tmp/test/images"),
+                            DataSetCopier.SSH_TIMEOUT_MILLIS);
+                    will(returnValue(BooleanStatus.createFalse()));
+                    one(copier).copyToRemote(ds2Data, new File("tmp/test"), "host", "abc",
+                            "abc-password");
+                    will(returnValue(Status.OK));
                 }
             });
-        DataSetCopier dataSetCopier = new DataSetCopier(properties, storeRoot, factory);
+        DataSetCopier dataSetCopier =
+                new DataSetCopier(properties, storeRoot, pathFactory, sshFactory);
 
-        try
-        {
-            dataSetCopier.process(Arrays.asList(ds1, ds2));
-            fail("EnvironmentFailureException expected");
-        } catch (EnvironmentFailureException ex)
-        {
-            assertEquals("Could not copy data set ds1 to destination folder 'tmp/test' "
-                    + "on host 'host' for rsync module 'abc': error message", ex.getMessage());
-        }
+        ProcessingStatus processingStatus = dataSetCopier.process(Arrays.asList(ds1, ds2));
+
+        // processing first data set fails but second one is processed successfully
+        Status errorStatus = Status.createError(DataSetCopier.COPYING_FAILED_MSG);
+        assertEquals(1, processingStatus.getErrorStatuses().size());
+        assertEquals(errorStatus, processingStatus.getErrorStatuses().get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(errorStatus).size());
+        assertEquals(ds1, processingStatus.getDatasetsByStatus(errorStatus).get(0));
+        assertEquals(1, processingStatus.getDatasetsByStatus(Status.OK).size());
+        assertEquals(ds2, processingStatus.getDatasetsByStatus(Status.OK).get(0));
 
         context.assertIsSatisfied();
     }
@@ -343,9 +387,12 @@ public class DataSetCopierTest extends AbstractFileSystemTestCase
         context.checking(new Expectations()
             {
                 {
-                    one(factory).create(rsyncExecutableDummy, sshExecutableDummy);
+                    one(pathFactory).create(rsyncExecutableDummy, sshExecutableDummy);
                     will(returnValue(copier));
 
+                    one(sshFactory).create(sshExecutableDummy, hostOrNull);
+                    will(returnValue(sshCommandExecutor));
+
                     one(copier).check();
                     if (hostOrNull != null)
                     {
-- 
GitLab