From 620149838ab376019c5264ea375bb60a2124b27b Mon Sep 17 00:00:00 2001
From: tpylak <tpylak>
Date: Mon, 5 Sep 2011 11:18:29 +0000
Subject: [PATCH] LMS-2499 fix post-registration task: do not throw exception
 (which blocks registration of all subsequent datasets) if dataset does not
 have some properties used in the template

SVN: 22788
---
 .../postregistration/NotifyingTask.java       | 58 ++++++++++++++++---
 .../NotifyingTaskTest-Example.properties      |  2 +-
 .../postregistration/NotifyingTaskTest.java   | 26 +++++++--
 3 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTask.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTask.java
index 0b5460e455f..04aef94bc23 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTask.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTask.java
@@ -22,6 +22,11 @@ import java.util.Set;
 
 import ch.systemsx.cisd.bds.StringUtils;
 import ch.systemsx.cisd.common.filesystem.FileUtilities;
+import ch.systemsx.cisd.common.logging.ISimpleLogger;
+import ch.systemsx.cisd.common.logging.Log4jSimpleLogger;
+import ch.systemsx.cisd.common.logging.LogCategory;
+import ch.systemsx.cisd.common.logging.LogFactory;
+import ch.systemsx.cisd.common.logging.LogLevel;
 import ch.systemsx.cisd.common.utilities.PropertyUtils;
 import ch.systemsx.cisd.common.utilities.Template;
 import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService;
@@ -53,6 +58,8 @@ public class NotifyingTask extends AbstractPostRegistrationTask
 
     private static final String DATA_SET_CODE_PLACE_HOLDER = "data-set-code";
 
+    private final ISimpleLogger logger;
+
     private final Template messageTemplate;
 
     private final Template destinationPathTemplate;
@@ -60,8 +67,15 @@ public class NotifyingTask extends AbstractPostRegistrationTask
     private final String[] includeDatasetTypePatternsOrNull;
 
     public NotifyingTask(Properties properties, IEncapsulatedOpenBISService service)
+    {
+        this(properties, service, new Log4jSimpleLogger(LogFactory.getLogger(LogCategory.OPERATION,
+                NotifyingTask.class)));
+    }
+
+    NotifyingTask(Properties properties, IEncapsulatedOpenBISService service, ISimpleLogger logger)
     {
         super(properties, service);
+        this.logger = logger;
         messageTemplate =
                 new Template(PropertyUtils.getMandatoryProperty(properties, MESSAGE_TEMPLATE_KEY));
         destinationPathTemplate =
@@ -102,11 +116,13 @@ public class NotifyingTask extends AbstractPostRegistrationTask
             throw new IllegalArgumentException("Unknown data set: " + dataSetCode);
         }
         return new Executor(dataSet, messageTemplate.createFreshCopy(),
-                destinationPathTemplate.createFreshCopy(), includeDatasetTypePatternsOrNull);
+                destinationPathTemplate.createFreshCopy(), includeDatasetTypePatternsOrNull, logger);
     }
 
     private static final class Executor implements IPostRegistrationTaskExecutor
     {
+        private final ISimpleLogger logger;
+
         private final ExternalData dataSet;
 
         private final Template messageTemplate;
@@ -116,12 +132,14 @@ public class NotifyingTask extends AbstractPostRegistrationTask
         private final String[] includeDatasetTypePatternsOrNull;
 
         public Executor(ExternalData dataSet, Template messageTemplate,
-                Template destinationPathTemplate, String[] includeDatasetTypePatternsOrNull)
+                Template destinationPathTemplate, String[] includeDatasetTypePatternsOrNull,
+                ISimpleLogger logger)
         {
             this.dataSet = dataSet;
             this.messageTemplate = messageTemplate;
             this.destinationPathTemplate = destinationPathTemplate;
             this.includeDatasetTypePatternsOrNull = includeDatasetTypePatternsOrNull;
+            this.logger = logger;
         }
 
         public ICleanupTask createCleanupTask()
@@ -133,9 +151,22 @@ public class NotifyingTask extends AbstractPostRegistrationTask
         {
             if (typeMatches())
             {
-                String messageText = fillTemplate(messageTemplate);
-                FileUtilities.writeToFile(new File(fillTemplate(destinationPathTemplate)),
-                        messageText);
+                String messageText;
+                String fileName;
+                try
+                {
+                    messageText = fillTemplate(messageTemplate);
+                    fileName = fillTemplate(destinationPathTemplate);
+                } catch (UnknownPropertyRequested ex)
+                {
+                    logger.log(
+                            LogLevel.WARN,
+                            String.format(
+                                    "Could not produce post registration confirmation file for dataset '%s': %s",
+                                    dataSet.getCode(), ex.getMessage()));
+                    return;
+                }
+                FileUtilities.writeToFile(new File(fileName), messageText);
             }
         }
 
@@ -156,7 +187,7 @@ public class NotifyingTask extends AbstractPostRegistrationTask
             return false;
         }
 
-        private String fillTemplate(Template template)
+        private String fillTemplate(Template template) throws UnknownPropertyRequested
         {
             Set<String> placeholderNames = template.getPlaceholderNames();
             for (String placeholderName : placeholderNames)
@@ -178,7 +209,7 @@ public class NotifyingTask extends AbstractPostRegistrationTask
             return messageText;
         }
 
-        private String getProperty(String propertyName)
+        private String getProperty(String propertyName) throws UnknownPropertyRequested
         {
             for (IEntityProperty property : dataSet.getProperties())
             {
@@ -187,9 +218,20 @@ public class NotifyingTask extends AbstractPostRegistrationTask
                     return property.tryGetAsString();
                 }
             }
-            throw new IllegalArgumentException("Unknown property: " + propertyName);
+            throw new UnknownPropertyRequested(String.format("Property '%s' is not set.",
+                    propertyName));
         }
 
     }
 
+    private final static class UnknownPropertyRequested extends Exception
+    {
+        private static final long serialVersionUID = 1L;
+
+        public UnknownPropertyRequested(String message)
+        {
+            super(message);
+        }
+    }
+
 }
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest-Example.properties b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest-Example.properties
index c2d735efe92..737cb0a75e1 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest-Example.properties
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest-Example.properties
@@ -2,4 +2,4 @@ message-template = storage_provider.storage.status = STORAGE_SUCCESSFUL\n\
                    storage_provider.dataset.id = ${data-set-code}\n\
                    ibrain2.dataset.id = ${property:ibrain-data-set-id}\n
 destination-path-template = targets/ibrain-${property:ibrain-data-set-id}.txt
-include-dataset-type-patterns = .*XYZ.*, ABC
+include-dataset-type-patterns = .*XYZ.*, accepted-type
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest.java
index a4427d4bd78..65ada26a002 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/etlserver/postregistration/NotifyingTaskTest.java
@@ -30,6 +30,8 @@ import org.testng.annotations.Test;
 
 import ch.systemsx.cisd.base.tests.AbstractFileSystemTestCase;
 import ch.systemsx.cisd.common.filesystem.FileUtilities;
+import ch.systemsx.cisd.common.logging.AssertingLogger;
+import ch.systemsx.cisd.common.logging.LogLevel;
 import ch.systemsx.cisd.openbis.dss.generic.shared.IEncapsulatedOpenBISService;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.builders.DataSetBuilder;
 
@@ -44,6 +46,8 @@ public class NotifyingTaskTest extends AbstractFileSystemTestCase
 
     private static final String DATA_SET_CODE_3 = "ds-3";
 
+    private static final String DATA_SET_CODE_4 = "ds-4";
+
     private Mockery context;
 
     private IEncapsulatedOpenBISService service;
@@ -78,8 +82,11 @@ public class NotifyingTaskTest extends AbstractFileSystemTestCase
             IOUtils.closeQuietly(inStream);
         }
         final DataSetBuilder dataSet1 = createDataset(DATA_SET_CODE_1, "ooo_XYZ_ooo", "ibrain-2");
-        final DataSetBuilder dataSet2 = createDataset(DATA_SET_CODE_2, "ABC", "ibrain-3");
-        final DataSetBuilder filteredDataSet = createDataset(DATA_SET_CODE_3, "ibrain-4", null);
+        final DataSetBuilder dataSet2 = createDataset(DATA_SET_CODE_2, "accepted-type", "ibrain-3");
+        final DataSetBuilder filteredDataSet =
+                createDataset(DATA_SET_CODE_3, "filtered-type", null);
+        final DataSetBuilder noPropertiesDataSet =
+                createDataset(DATA_SET_CODE_4, "accepted-type", null);
 
         context.checking(new Expectations()
             {
@@ -92,9 +99,13 @@ public class NotifyingTaskTest extends AbstractFileSystemTestCase
 
                     one(service).tryGetDataSet(DATA_SET_CODE_3);
                     will(returnValue(filteredDataSet.getDataSet()));
+
+                    one(service).tryGetDataSet(DATA_SET_CODE_4);
+                    will(returnValue(noPropertiesDataSet.getDataSet()));
                 }
             });
-        NotifyingTask notifyingTask = new NotifyingTask(properties, service);
+        AssertingLogger logger = new AssertingLogger();
+        NotifyingTask notifyingTask = new NotifyingTask(properties, service, logger);
 
         IPostRegistrationTaskExecutor executor = execute(notifyingTask, DATA_SET_CODE_1);
         ICleanupTask cleanupTask = executor.createCleanupTask();
@@ -109,9 +120,16 @@ public class NotifyingTaskTest extends AbstractFileSystemTestCase
                 "targets/ibrain-ibrain-3.txt").isFile());
 
         execute(notifyingTask, DATA_SET_CODE_3);
-        assertFalse("confirnation file for " + DATA_SET_CODE_3 + " should not be created!",
+        assertFalse("confirmation file for " + DATA_SET_CODE_3 + " should not be created!",
                 new File("targets/ibrain-ibrain-4.txt").exists());
 
+        execute(notifyingTask, DATA_SET_CODE_4);
+        logger.assertNumberOfMessage(1);
+        logger.assertEq(
+                0,
+                LogLevel.WARN,
+                "Could not produce post registration confirmation file for dataset 'ds-4': Property 'ibrain-data-set-id' is not set.");
+
         context.assertIsSatisfied();
     }
 
-- 
GitLab