From 4a7a8bf08678dd89495547c504cbb4016a0228d3 Mon Sep 17 00:00:00 2001
From: kaloyane <kaloyane>
Date: Mon, 3 Oct 2011 11:55:59 +0000
Subject: [PATCH] [LMS-2568] do not persist a record in the core_plugins able
 if the master data registration scripts fails

SVN: 23167
---
 .../impl/MasterDataRegistrationException.java | 17 ++++++++++
 .../MasterDataRegistrationScriptRunner.java   | 34 ++-----------------
 ...ataRegistrationScriptRunnerStandalone.java | 17 ++++++++--
 .../MasterDataRegistrationTransaction.java    | 18 ++++++++++
 .../v1/impl/MasterDataTransactionErrors.java  |  8 ++---
 ...asterDataRegistrationScriptRunnerTest.java | 33 ++++++++++--------
 6 files changed, 75 insertions(+), 52 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationException.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationException.java
index 72a856b68b6..9d5501c7b5d 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationException.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationException.java
@@ -19,6 +19,9 @@ package ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl;
 import java.util.List;
 
 import ch.systemsx.cisd.common.exceptions.HighLevelException;
+import ch.systemsx.cisd.common.logging.ISimpleLogger;
+import ch.systemsx.cisd.common.logging.LogLevel;
+import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataTransactionErrors.TransactionError;
 
 /**
  * @author Kaloyan Enimanev
@@ -42,4 +45,18 @@ public class MasterDataRegistrationException extends HighLevelException
         return transactionErrors;
     }
 
+    /**
+     * Logs the accumulated errors.
+     */
+    public void logErrors(ISimpleLogger errorLogger)
+    {
+        for (MasterDataTransactionErrors errors : getTransactionErrors())
+        {
+            for (TransactionError error : errors.getErrors())
+            {
+                errorLogger.log(LogLevel.ERROR, error.getDescription());
+            }
+        }
+    }
+
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunner.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunner.java
index 1fa72547d2c..4ca7dd5e753 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunner.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunner.java
@@ -21,9 +21,6 @@ import java.io.File;
 import org.python.util.PythonInterpreter;
 
 import ch.systemsx.cisd.common.filesystem.FileUtilities;
-import ch.systemsx.cisd.common.logging.ISimpleLogger;
-import ch.systemsx.cisd.common.logging.LogLevel;
-import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataTransactionErrors.TransactionError;
 
 /**
  * A class for running python scripts that register master data.
@@ -36,27 +33,16 @@ public class MasterDataRegistrationScriptRunner
 
     private final EncapsulatedCommonServer commonServer;
 
-    private final ISimpleLogger errorLogger;
-
-    public MasterDataRegistrationScriptRunner(EncapsulatedCommonServer commonServer,
-            ISimpleLogger errorLogger)
+    public MasterDataRegistrationScriptRunner(EncapsulatedCommonServer commonServer)
     {
         this.commonServer = commonServer;
-        this.errorLogger = errorLogger;
     }
 
-    public void executeScript(File jythonScript)
+    public void executeScript(File jythonScript) throws MasterDataRegistrationException
     {
         MasterDataRegistrationService service = new MasterDataRegistrationService(commonServer);
         runScript(service, jythonScript);
-
-        try
-        {
-            service.commit();
-        } catch (MasterDataRegistrationException mdre)
-        {
-            handleRegistrationException(jythonScript, mdre);
-        }
+        service.commit();
     }
 
     private void runScript(MasterDataRegistrationService service, File jythonScript)
@@ -93,18 +79,4 @@ public class MasterDataRegistrationScriptRunner
                     + jythonScript.getAbsolutePath());
         }
     }
-
-    private void handleRegistrationException(File jythonScript, MasterDataRegistrationException mdre)
-    {
-        errorLogger.log(LogLevel.ERROR, "Failed to commit all transactions for script "
-                + jythonScript.getAbsolutePath());
-        for (MasterDataTransactionErrors errors : mdre.getTransactionErrors())
-        {
-            for (TransactionError error : errors.getErrors())
-            {
-                errorLogger.log(LogLevel.ERROR, error.getDescription());
-            }
-        }
-    }
-
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunnerStandalone.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunnerStandalone.java
index 8fbe426f492..7455f691d36 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunnerStandalone.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationScriptRunnerStandalone.java
@@ -24,6 +24,8 @@ import ch.systemsx.cisd.args4j.ExampleMode;
 import ch.systemsx.cisd.args4j.Option;
 import ch.systemsx.cisd.common.cli.ConsoleClientArguments;
 import ch.systemsx.cisd.common.logging.ConsoleLogger;
+import ch.systemsx.cisd.common.logging.ISimpleLogger;
+import ch.systemsx.cisd.common.logging.LogLevel;
 
 /**
  * A standalone command line tool allowing the execution of Jython scripts to initialize the master
@@ -82,8 +84,19 @@ public class MasterDataRegistrationScriptRunnerStandalone
                         arguments.getUsername(), arguments.getPassword());
 
         MasterDataRegistrationScriptRunner scriptRunner =
-                new MasterDataRegistrationScriptRunner(commonServer, new ConsoleLogger());
-        scriptRunner.executeScript(new File(arguments.scriptFileName));
+                new MasterDataRegistrationScriptRunner(commonServer);
+
+        final File jythonScript = new File(arguments.scriptFileName);
+        try
+        {
+            scriptRunner.executeScript(jythonScript);
+        } catch (MasterDataRegistrationException mdre)
+        {
+            ISimpleLogger errorLogger = new ConsoleLogger();
+            errorLogger.log(LogLevel.ERROR, "Failed to commit all transactions for script "
+                    + jythonScript.getAbsolutePath());
+            mdre.logErrors(errorLogger);
+        }
     }
 
     public static void main(String[] args)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationTransaction.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationTransaction.java
index f68d872a475..d3a84af84f5 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationTransaction.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataRegistrationTransaction.java
@@ -204,6 +204,19 @@ public class MasterDataRegistrationTransaction implements IMasterDataRegistratio
         return null;
     }
 
+    private IVocabularyImmutable findVocabularyForCode(List<IVocabularyImmutable> vocabularies,
+            String code)
+    {
+        for (IVocabularyImmutable vocabulary : vocabularies)
+        {
+            if (vocabulary.getCode().equalsIgnoreCase(code))
+            {
+                return vocabulary;
+            }
+        }
+        return null;
+    }
+
     private PropertyAssignment createAssignment(EntityKind entityKind, IEntityType type,
             IPropertyTypeImmutable propertyType)
     {
@@ -237,6 +250,11 @@ public class MasterDataRegistrationTransaction implements IMasterDataRegistratio
         return vocabulary;
     }
 
+    public IVocabularyImmutable getVocabulary(String code)
+    {
+        return findVocabularyForCode(commonServer.listVocabularies(), code);
+    }
+
     public List<IVocabularyImmutable> listVocabularies()
     {
         return commonServer.listVocabularies();
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataTransactionErrors.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataTransactionErrors.java
index 12e473d1139..5adca4dc143 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataTransactionErrors.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/jython/api/v1/impl/MasterDataTransactionErrors.java
@@ -43,7 +43,7 @@ public class MasterDataTransactionErrors
             return exception;
         }
 
-        abstract String getDescription();
+        public abstract String getDescription();
     }
 
     private static class TypeRegistrationError extends TransactionError
@@ -57,7 +57,7 @@ public class MasterDataTransactionErrors
         }
 
         @Override
-        String getDescription()
+        public String getDescription()
         {
             return String.format("Failed to register type '%s': %s", type.getCode(), getException()
                     .getMessage());
@@ -75,7 +75,7 @@ public class MasterDataTransactionErrors
         }
 
         @Override
-        String getDescription()
+        public String getDescription()
         {
             return String.format("Failed to assign property '%s' <-> '%s': %s",
                     propertyAssignment.getEntityTypeCode(),
@@ -94,7 +94,7 @@ public class MasterDataTransactionErrors
         }
 
         @Override
-        String getDescription()
+        public String getDescription()
         {
             return String.format("Failed to register vocabulary '%s': %s", vocabulary.getCode(),
                     getException().getMessage());
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/jython/api/v1/impl/MasterDataRegistrationScriptRunnerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/jython/api/v1/impl/MasterDataRegistrationScriptRunnerTest.java
index b557203e63f..086b803fa9a 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/jython/api/v1/impl/MasterDataRegistrationScriptRunnerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/client/jython/api/v1/impl/MasterDataRegistrationScriptRunnerTest.java
@@ -26,11 +26,12 @@ import org.testng.AssertJUnit;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
-import ch.systemsx.cisd.common.logging.AssertingLogger;
-import ch.systemsx.cisd.common.logging.LogLevel;
 import ch.systemsx.cisd.common.test.RecordingMatcher;
 import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.EncapsulatedCommonServer;
+import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataRegistrationException;
 import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataRegistrationScriptRunner;
+import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataTransactionErrors;
+import ch.systemsx.cisd.openbis.generic.server.jython.api.v1.impl.MasterDataTransactionErrors.TransactionError;
 import ch.systemsx.cisd.openbis.generic.shared.ICommonServer;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityKind;
@@ -59,8 +60,6 @@ public class MasterDataRegistrationScriptRunnerTest extends AssertJUnit
 
     private MasterDataRegistrationScriptRunner pluginScriptRunner;
 
-    private AssertingLogger errorLogger;
-
     @BeforeMethod
     public void beforeMethod()
     {
@@ -68,10 +67,7 @@ public class MasterDataRegistrationScriptRunnerTest extends AssertJUnit
         commonServer = context.mock(ICommonServer.class);
         EncapsulatedCommonServer encapsulatedServer =
                 EncapsulatedCommonServer.create(commonServer, SESSION_TOKEN);
-        errorLogger = new AssertingLogger();
-        pluginScriptRunner =
-                new MasterDataRegistrationScriptRunner(encapsulatedServer, errorLogger);
-
+        pluginScriptRunner = new MasterDataRegistrationScriptRunner(encapsulatedServer);
     }
 
     @Test
@@ -116,7 +112,6 @@ public class MasterDataRegistrationScriptRunnerTest extends AssertJUnit
 
         File scriptFile = getScriptFile("simple-transaction.py");
         pluginScriptRunner.executeScript(scriptFile);
-        errorLogger.assertNumberOfMessage(0);
 
         assertEquals(1, fileFormatMatcher.getRecordedObjects().size());
         FileFormatType fileFormatType = fileFormatMatcher.recordedObject();
@@ -222,11 +217,9 @@ public class MasterDataRegistrationScriptRunnerTest extends AssertJUnit
             });
 
         File scriptFile = getScriptFile("simple-transaction.py");
-        pluginScriptRunner.executeScript(scriptFile);
 
         List<String> errorLines =
                 Arrays.asList(
-                        "Failed to commit all transactions for script .*",
                         "Failed to register type 'FILE-FORMAT-TYPE': FAILED FILE FORMAT",
                         "Failed to register vocabulary 'ANIMALS': FAILED VOCABULARY",
                         "Failed to register type 'EXPERIMENT-TYPE': FAILED EXPERIMENT TYPE",
@@ -238,11 +231,21 @@ public class MasterDataRegistrationScriptRunnerTest extends AssertJUnit
                         "Failed to assign property 'SAMPLE-TYPE' <-> 'MATERIAL-PROPERTY-TYPE': FAILED ASSIGNMENT",
                         "Failed to assign property 'EXPERIMENT-TYPE' <-> 'VARCHAR-PROPERTY-TYPE': FAILED ASSIGNMENT");
 
-        errorLogger.assertNumberOfMessage(errorLines.size());
-        errorLogger.assertMatches(0, LogLevel.ERROR, errorLines.get(0));
-        for (int i = 1; i < errorLines.size(); i++)
+        try
+        {
+            pluginScriptRunner.executeScript(scriptFile);
+            fail("MasterDataRegistrationException expected.");
+        } catch (MasterDataRegistrationException mdre)
         {
-            errorLogger.assertEq(i, LogLevel.ERROR, errorLines.get(i));
+            int pos = 0;
+            for (MasterDataTransactionErrors mderr : mdre.getTransactionErrors())
+            {
+                for (TransactionError err : mderr.getErrors())
+                {
+                    assertEquals(errorLines.get(pos), err.getDescription());
+                    pos++;
+                }
+            }
         }
     }
 
-- 
GitLab