From f8758ae4aa8ccde56eb018653c51402cab2a006a Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Tue, 18 Oct 2011 07:01:57 +0000
Subject: [PATCH] bug in ProteinResultDataSetParentLinkingTask fixed.

SVN: 23339
---
 ...DataSetInfoExtractorForProteinResults.java | 62 ++++++++-----
 ...ProteinResultDataSetParentLinkingTask.java | 20 +---
 .../phosphonetx/dto/ParentDataSetCodes.java   | 51 ++++++++++
 ...SetInfoExtractorForProteinResultsTest.java | 92 +++++++++++++++++++
 ...einResultDataSetParentLinkingTaskTest.java | 35 ++++---
 5 files changed, 205 insertions(+), 55 deletions(-)
 create mode 100644 rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/dto/ParentDataSetCodes.java

diff --git a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResults.java b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResults.java
index b68ab4c4b2b..994cb92c143 100644
--- a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResults.java
+++ b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResults.java
@@ -18,7 +18,6 @@ package ch.systemsx.cisd.openbis.etlserver.phosphonetx;
 
 import java.io.File;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.Properties;
 
@@ -31,6 +30,7 @@ import ch.systemsx.cisd.common.utilities.PropertyUtils;
 import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService;
 import ch.systemsx.cisd.openbis.dss.generic.shared.ServiceProvider;
 import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation;
+import ch.systemsx.cisd.openbis.etlserver.phosphonetx.dto.ParentDataSetCodes;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ExperimentType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ExternalData;
@@ -96,14 +96,20 @@ public class DataSetInfoExtractorForProteinResults extends AbstractDataSetInfoEx
         Properties properties =
                 loadSearchProperties(new File(incomingDataSetPath, experimentPropertiesFileName));
         experiment.setProperties(Util.getAndCheckProperties(properties, experimentType));
-        service.registerExperiment(experiment);
         DataSetInformation info = new DataSetInformation();
         info.setExperimentIdentifier(experimentIdentifier);
         String parentDataSetCodesOrNull = getProperty(properties, PARENT_DATA_SET_CODES);
         String baseExperimentIdentifier = getProperty(properties, EXPERIMENT_IDENTIFIER_KEY);
-        List<String> parentDataSetCodes =
+        ParentDataSetCodes parentDataSetCodes =
                 getParentDataSetCodes(parentDataSetCodesOrNull, baseExperimentIdentifier, service);
-        info.setParentDataSetCodes(parentDataSetCodes);
+        if (parentDataSetCodes.getErrorMessage() == null)
+        {
+            info.setParentDataSetCodes(parentDataSetCodes.getDataSetCodes());
+        } else
+        {
+            throw new UserFailureException(parentDataSetCodes.getErrorMessage());
+        }
+        service.registerExperiment(experiment);
         return info;
     }
 
@@ -111,35 +117,45 @@ public class DataSetInfoExtractorForProteinResults extends AbstractDataSetInfoEx
      * Returns data set codes either from the first argument or if <code>null</code> from
      * the data sets of the specified experiment. 
      */
-    static List<String> getParentDataSetCodes(String parentDataSetCodesOrNull,
+    static ParentDataSetCodes getParentDataSetCodes(String parentDataSetCodesOrNull,
             String baseExperimentIdentifier, IEncapsulatedOpenBISService service)
     {
-        List<String> parentDataSetCodes = new ArrayList<String>();
+        List<ExternalData> parentDataSets = new ArrayList<ExternalData>();
+        StringBuilder builder = new StringBuilder();
         if (parentDataSetCodesOrNull != null)
         {
-            parentDataSetCodes = Arrays.asList(StringUtils.split(parentDataSetCodesOrNull, ", "));
-        } else
-        {
-            if (baseExperimentIdentifier != null)
+            for (String code : StringUtils.split(parentDataSetCodesOrNull, ", "))
             {
-                ExperimentIdentifier identifier =
-                        new ExperimentIdentifierFactory(baseExperimentIdentifier)
-                                .createIdentifier();
-                Experiment baseExperiment = service.tryToGetExperiment(identifier);
-                if (baseExperiment == null)
+                ExternalData dataSet = service.tryGetDataSet(code);
+                if (dataSet != null)
                 {
-                    throw new UserFailureException("Property " + EXPERIMENT_IDENTIFIER_KEY
-                            + " specifies an unknown experiment: " + baseExperimentIdentifier);
-                }
-                List<ExternalData> dataSets =
-                        service.listDataSetsByExperimentID(baseExperiment.getId());
-                for (ExternalData dataSet : dataSets)
+                    parentDataSets.add(dataSet);
+                } else
                 {
-                    parentDataSetCodes.add(dataSet.getCode());
+                    builder.append(builder.length() == 0 ? "Unknown data sets: " : ", ");
+                    builder.append(code);
                 }
             }
+        } else if (baseExperimentIdentifier != null)
+        {
+            ExperimentIdentifier identifier =
+                    new ExperimentIdentifierFactory(baseExperimentIdentifier).createIdentifier();
+            Experiment baseExperiment = service.tryToGetExperiment(identifier);
+            if (baseExperiment != null)
+            {
+                parentDataSets.addAll(service.listDataSetsByExperimentID(baseExperiment.getId()));
+            } else
+            {
+                builder.append("Unkown experiment ").append(baseExperimentIdentifier);
+            }
+        }
+        List<String> parentDataSetCodes = new ArrayList<String>();
+        for (ExternalData dataSet : parentDataSets)
+        {
+            parentDataSetCodes.add(dataSet.getCode());
         }
-        return parentDataSetCodes;
+        String errorMessage = builder.length() > 0 ? builder.toString() : null;
+        return new ParentDataSetCodes(parentDataSetCodes, errorMessage);
     }
     
     private String getProperty(Properties properties, String key)
diff --git a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTask.java b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTask.java
index 1c33685a076..b5e2450fdd9 100644
--- a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTask.java
+++ b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTask.java
@@ -22,7 +22,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.regex.Pattern;
 
 import org.apache.log4j.Logger;
 
@@ -54,8 +53,6 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier;
  */
 public class ProteinResultDataSetParentLinkingTask implements IMaintenanceTask
 {
-    private static final Pattern DATA_SET_CODE_PATTERN = Pattern.compile("\\d{17}-\\d+");
-
     private static final String PARENT_DATA_SET_CODES_KEY =
             DataSetInfoExtractorForProteinResults.PARENT_DATA_SET_CODES.toUpperCase();
 
@@ -99,8 +96,9 @@ public class ProteinResultDataSetParentLinkingTask implements IMaintenanceTask
                 String parentDataSetCodes =
                         tryGetProperty(propertiesMap, PARENT_DATA_SET_CODES_KEY);
                 List<String> codes =
-                        filter(DataSetInfoExtractorForProteinResults.getParentDataSetCodes(
-                                parentDataSetCodes, baseExperimentIdentifier, service));
+                        DataSetInfoExtractorForProteinResults.getParentDataSetCodes(
+                                parentDataSetCodes, baseExperimentIdentifier, service)
+                                .getDataSetCodes();
                 if (codes.isEmpty())
                 {
                     continue;
@@ -160,16 +158,4 @@ public class ProteinResultDataSetParentLinkingTask implements IMaintenanceTask
         return property == null ? null : property.tryGetAsString();
     }
 
-    private List<String> filter(List<String> codes)
-    {
-        List<String> result = new ArrayList<String>();
-        for (String code : codes)
-        {
-            if (DATA_SET_CODE_PATTERN.matcher(code).matches())
-            {
-                result.add(code);
-            }
-        }
-        return result;
-    }
 }
diff --git a/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/dto/ParentDataSetCodes.java b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/dto/ParentDataSetCodes.java
new file mode 100644
index 00000000000..3305a4f05a9
--- /dev/null
+++ b/rtd_phosphonetx/source/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/dto/ParentDataSetCodes.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2011 ETH Zuerich, CISD
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ch.systemsx.cisd.openbis.etlserver.phosphonetx.dto;
+
+import java.util.List;
+
+/**
+ * Data transfer object which contains a list of data set codes and an optional
+ * error message.
+ *
+ * @author Franz-Josef Elmer
+ */
+public class ParentDataSetCodes
+{
+    private final List<String> dataSetCodes;
+    
+    private final String errorMessage;
+
+    public ParentDataSetCodes(List<String> dataSetCodes, String errorMessage)
+    {
+        super();
+        this.dataSetCodes = dataSetCodes;
+        this.errorMessage = errorMessage;
+    }
+
+    public List<String> getDataSetCodes()
+    {
+        return dataSetCodes;
+    }
+
+    public String getErrorMessage()
+    {
+        return errorMessage;
+    }
+    
+    
+}
diff --git a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResultsTest.java b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResultsTest.java
index 0ffa9aae8dc..e9f10fe211f 100644
--- a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResultsTest.java
+++ b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/DataSetInfoExtractorForProteinResultsTest.java
@@ -96,6 +96,10 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
         String experimentType = "MY_EXPERIMENT";
         properties.setProperty(EXPERIMENT_TYPE_CODE_KEY, experimentType);
         properties.setProperty(EXPERIMENT_PROPERTIES_FILE_NAME_KEY, propertiesFile);
+        prepareGetDataSet("1");
+        prepareGetDataSet("2");
+        prepareGetDataSet("3");
+        prepareGetDataSet("4");
         prepare(experimentType);
         context.checking(new Expectations()
             {
@@ -120,6 +124,10 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
                 DataSetInfoExtractorForProteinResults.DEFAULT_EXPERIMENT_PROPERTIES_FILE_NAME),
                 "answer=42\nblabla=blub\n" + PARENT_DATA_SET_CODES + "=1 2  3   4\n");
         prepare(DEFAULT_EXPERIMENT_TYPE_CODE);
+        prepareGetDataSet("1");
+        prepareGetDataSet("2");
+        prepareGetDataSet("3");
+        prepareGetDataSet("4");
 
         context.checking(new Expectations()
             {
@@ -214,6 +222,70 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
         context.assertIsSatisfied();
     }
     
+    @Test
+    public void testWithUnkownBaseExperiment()
+    {
+        String propertiesFile = "my.properties";
+        FileUtilities.writeToFile(new File(dataSet, propertiesFile), "answer=42\nblabla=blub\n"
+                + EXPERIMENT_IDENTIFIER_KEY + "= /TEST/PROJECT/EXP1\n");
+        Properties properties = new Properties();
+        String experimentType = "MY_EXPERIMENT";
+        properties.setProperty(EXPERIMENT_TYPE_CODE_KEY, experimentType);
+        properties.setProperty(EXPERIMENT_PROPERTIES_FILE_NAME_KEY, propertiesFile);
+        prepare(experimentType);
+        context.checking(new Expectations()
+            {
+                {
+                    one(service).tryToGetExperiment(
+                            new ExperimentIdentifier(new ProjectIdentifier("TEST", "PROJECT"),
+                                    "EXP1"));
+                    will(returnValue(null));
+
+                }
+            });
+        
+        IDataSetInfoExtractor extractor = createExtractor(properties);
+        
+        try
+        {
+            extractor.getDataSetInformation(dataSet, service);
+            fail("UserFailureException expected.");
+        } catch (UserFailureException ex)
+        {
+            assertEquals("Unkown experiment /TEST/PROJECT/EXP1", ex.getMessage());
+        }
+        context.assertIsSatisfied();
+    }
+    
+    @Test
+    public void testWithUnkownParentDataSets()
+    {
+        String propertiesFile = "my.properties";
+        FileUtilities.writeToFile(new File(dataSet, propertiesFile), "answer=42\nblabla=blub\n"
+                + EXPERIMENT_IDENTIFIER_KEY + "= /TEST/PROJECT/EXP1\n" + PARENT_DATA_SET_CODES_KEY
+                + " = ds1 ds2, ds3");
+        Properties properties = new Properties();
+        String experimentType = "MY_EXPERIMENT";
+        properties.setProperty(EXPERIMENT_TYPE_CODE_KEY, experimentType);
+        properties.setProperty(EXPERIMENT_PROPERTIES_FILE_NAME_KEY, propertiesFile);
+        prepare(experimentType);
+        prepareGetDataSet("ds1");
+        prepareGetDataSet("ds2", null);
+        prepareGetDataSet("ds3", null);
+
+        IDataSetInfoExtractor extractor = createExtractor(properties);
+
+        try
+        {
+            extractor.getDataSetInformation(dataSet, service);
+            fail("UserFailureException expected.");
+        } catch (UserFailureException ex)
+        {
+            assertEquals("Unknown data sets: ds2, ds3", ex.getMessage());
+        }
+        context.assertIsSatisfied();
+    }
+    
     @Test
     public void testWithParentDataSetsSeparatedBySpaces()
     {
@@ -226,6 +298,8 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
         properties.setProperty(EXPERIMENT_TYPE_CODE_KEY, experimentType);
         properties.setProperty(EXPERIMENT_PROPERTIES_FILE_NAME_KEY, propertiesFile);
         prepare(experimentType);
+        prepareGetDataSet("ds1");
+        prepareGetDataSet("ds2");
         context.checking(new Expectations()
             {
                 {
@@ -253,6 +327,8 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
         properties.setProperty(EXPERIMENT_TYPE_CODE_KEY, experimentType);
         properties.setProperty(EXPERIMENT_PROPERTIES_FILE_NAME_KEY, propertiesFile);
         prepare(experimentType);
+        prepareGetDataSet("ds1");
+        prepareGetDataSet("ds2");
         context.checking(new Expectations()
             {
                 {
@@ -290,6 +366,22 @@ public class DataSetInfoExtractorForProteinResultsTest extends AbstractFileSyste
             });
     }
     
+    private void prepareGetDataSet(final String dataSetCode)
+    {
+        prepareGetDataSet(dataSetCode, new DataSetBuilder().code(dataSetCode).getDataSet());
+    }
+    
+    private void prepareGetDataSet(final String dataSetCode, final ExternalData data)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    one(service).tryGetDataSet(dataSetCode);
+                    will(returnValue(data));
+                }
+            });
+    }
+    
     private IDataSetInfoExtractor createExtractor(Properties properties)
     {
         return new DataSetInfoExtractorForProteinResults(properties, service);
diff --git a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTaskTest.java b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTaskTest.java
index 53aff1de1eb..616c2309f51 100644
--- a/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTaskTest.java
+++ b/rtd_phosphonetx/sourceTest/java/ch/systemsx/cisd/openbis/etlserver/phosphonetx/ProteinResultDataSetParentLinkingTaskTest.java
@@ -85,25 +85,21 @@ public class ProteinResultDataSetParentLinkingTaskTest extends AssertJUnit
                         .getExperiment();
         final Experiment e2 =
                 new ExperimentBuilder().id(2).identifier("/A/P2/E2")
-                        .property(PARENT_DATA_SET_CODES_KEY, "non-sense").getExperiment();
+                        .property(PARENT_DATA_SET_CODES_KEY, "non-sense2").getExperiment();
         final Experiment e3 =
-                new ExperimentBuilder()
-                        .id(3)
-                        .identifier("/A/P2/E3")
-                        .property(PARENT_DATA_SET_CODES_KEY,
-                                "20100930111833087-297733, 20100511163311581-25265")
-                        .getExperiment();
+                new ExperimentBuilder().id(3).identifier("/A/P2/E3")
+                        .property(PARENT_DATA_SET_CODES_KEY, "ds1, ds3").getExperiment();
         final Experiment e4 =
                 new ExperimentBuilder().id(4).identifier("/S/P1/E4")
                         .property(BASE_EXPERIMENT_KEY, "/S/P1/E1").getExperiment();
         final DataSet ds1 =
-                new DataSetBuilder(1).code("20100930111811581-25265").fileFormat("A")
+                new DataSetBuilder(1).code("ds1").fileFormat("A")
                         .experiment(e1).modificationDate(new Date(11)).getDataSet();
         final DataSet ds2 =
-                new DataSetBuilder(2).code("20100930111811087-29765").fileFormat("B")
+                new DataSetBuilder(2).code("ds2").fileFormat("B")
                         .experiment(e4).modificationDate(new Date(22)).getDataSet();
         final DataSet ds3 =
-                new DataSetBuilder(3).code("20100530111833087-297733").fileFormat("C")
+                new DataSetBuilder(3).code("ds3").fileFormat("C")
                         .experiment(e3).modificationDate(new Date(33)).property("ALPHA", "3.1")
                         .getDataSet();
         final RecordingMatcher<AtomicEntityOperationDetails> operationRecorder =
@@ -122,7 +118,16 @@ public class ProteinResultDataSetParentLinkingTaskTest extends AssertJUnit
 
                     one(service).tryToGetExperiment(ExperimentIdentifierFactory.parse("/S/P1/E1"));
                     will(returnValue(e1));
-
+                    
+                    one(service).tryGetDataSet("non-sense2");
+                    will(returnValue(null));
+                    
+                    one(service).tryGetDataSet("ds1");
+                    will(returnValue(ds1));
+                    
+                    one(service).tryGetDataSet("ds3");
+                    will(returnValue(ds3));
+                    
                     one(service).listDataSetsByExperimentID(e1.getId());
                     will(returnValue(Arrays.asList(ds1)));
 
@@ -139,10 +144,10 @@ public class ProteinResultDataSetParentLinkingTaskTest extends AssertJUnit
         task.execute();
 
         assertEquals("INFO  OPERATION.ProteinResultDataSetParentLinkingTask - "
-                + "Parent data set links of data set 20100930111811087-29765 "
+                + "Parent data set links of data set ds2 "
                 + "from experiment /S/P1/E4 will be updated.\n"
                 + "INFO  OPERATION.ProteinResultDataSetParentLinkingTask - "
-                + "Parent data set links of data set 20100530111833087-297733 "
+                + "Parent data set links of data set ds3 "
                 + "from experiment /A/P2/E3 will be updated.\n"
                 + "INFO  OPERATION.ProteinResultDataSetParentLinkingTask - "
                 + "Parent data set links for 2 data sets have been updated.",
@@ -155,7 +160,7 @@ public class ProteinResultDataSetParentLinkingTaskTest extends AssertJUnit
         assertEquals("[]", dataSetUpdates.get(0).getProperties().toString());
         assertEquals(e4.getIdentifier(), dataSetUpdates.get(0).getExperimentIdentifierOrNull()
                 .toString());
-        assertEquals("[20100930111811581-25265]",
+        assertEquals("[ds1]",
                 Arrays.asList(dataSetUpdates.get(0).getModifiedParentDatasetCodesOrNull())
                         .toString());
         assertEquals(3L, dataSetUpdates.get(1).getDatasetId().getId().longValue());
@@ -164,7 +169,7 @@ public class ProteinResultDataSetParentLinkingTaskTest extends AssertJUnit
         assertEquals("[ALPHA: 3.1]", dataSetUpdates.get(1).getProperties().toString());
         assertEquals(e3.getIdentifier(), dataSetUpdates.get(1).getExperimentIdentifierOrNull()
                 .toString());
-        assertEquals("[20100930111833087-297733, 20100511163311581-25265]",
+        assertEquals("[ds1, ds3]",
                 Arrays.asList(dataSetUpdates.get(1).getModifiedParentDatasetCodesOrNull())
                         .toString());
         assertEquals(2, dataSetUpdates.size());
-- 
GitLab