minor: managed properties batch upload - strip off details from the error message

SVN: 19992

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrors.java

@@ -72,7 +72,7 @@ class PropertiesBatchEvaluationErrors
             ScriptPE script)
     {
         totalFailedRowsNumber++;
-        String errorMessage = evaluationError.getMessage();
+        String errorMessage = getErrorMessage(evaluationError);
         if (shouldSkipDetailsAccumulation(errorMessage))
         {
             return;
@@ -90,6 +90,26 @@ class PropertiesBatchEvaluationErrors
         errorDetails.put(errorMessage, details);
     }
 
+    private String getErrorMessage(EvaluatorException evaluationError)
+    {
+        String result = null;
+        if (evaluationError.getCause() != null)
+        {
+            return evaluationError.getCause().getMessage();
+        } else
+        {
+            result = evaluationError.getMessage();
+        }
+
+        if (result == null)
+        {
+            // do not return null
+            return "";
+        }
+
+        return result;
+    }
+
     /**
      * @return <code>true</code> when the memory is full and no more errors should be
      *         accumulated.
@@ -118,14 +138,29 @@ class PropertiesBatchEvaluationErrors
         message.append(totalRowsNumber);
         message.append(" rows.");
         
-        int numDisplayErrors = Math.min(errorDetails.size(), MAX_ERRORS_IN_USER_MESSAGE);
-        List<ErrorDetail> formatDetails = sortErrorDetailsByRow().subList(0, numDisplayErrors);
-        for (ErrorDetail errDetail : formatDetails)
+        // construct a mapping between message line and failed rows
+        List<String> errorMessageLines = new ArrayList<String>();
+        Map<String, List<Integer>> failedRows = new HashMap<String, List<Integer>>();
+        for (ErrorDetail errDetails : errorDetails.values())
+        {
+            String msgLine = createUserFailureMsgLine(errDetails);
+            if (errorMessageLines.contains(msgLine))
+            {
+                List<Integer> accumulatedRows = failedRows.get(msgLine);
+                accumulatedRows.addAll(errDetails.rows);
+            } else
+            {
+                errorMessageLines.add(msgLine);
+                failedRows.put(msgLine, new ArrayList<Integer>(errDetails.rows));
+            }
+        }
+
+        for (String msgLine : errorMessageLines)
         {
             message.append("\n");
-            appendErrorDetails(message, errDetail, false);
-            message.append(": ");
-            message.append(errDetail.evaluationError.getMessage());
+            List<Integer> rows = failedRows.get(msgLine);
+            message.append(formatRows(rows));
+            message.append(msgLine);
         }
 
         message.append("\n");
@@ -153,6 +188,7 @@ class PropertiesBatchEvaluationErrors
         for (ErrorDetail errDetail : sortErrorDetailsByRow())
         {
             message.append("\n\n");
+            message.append(formatRows(errDetail.rows));
             appendErrorDetails(message, errDetail, true);
             message.append(": ");
             StringWriter sw = new StringWriter();
@@ -176,10 +212,16 @@ class PropertiesBatchEvaluationErrors
         return result;
     }
 
+    private String createUserFailureMsgLine(ErrorDetail details)
+    {
+        StringBuilder builder = new StringBuilder();
+        appendErrorDetails(builder, details, false);
+        return builder.toString();
+    }
+
     private void appendErrorDetails(StringBuilder message, ErrorDetail errDetail,
             boolean includeFullRegistratorDetails)
     {
-        message.append(formatRows(errDetail.rows));
         message.append(" failed due to the property '");
         message.append(errDetail.propertyCode);
         message.append("' causing a malfuction in the script (name = '");

openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrorsTest.java

@@ -16,7 +16,6 @@
 
 package ch.systemsx.cisd.openbis.generic.server.business;
 
-import static ch.systemsx.cisd.openbis.generic.server.business.PropertiesBatchEvaluationErrors.MAX_ERRORS_IN_USER_MESSAGE;
 import static ch.systemsx.cisd.openbis.generic.server.business.PropertiesBatchEvaluationErrors.MAX_ERROR_DETAILS_KEPT;
 
 import org.apache.commons.lang.StringUtils;
@@ -40,6 +39,8 @@ public class PropertiesBatchEvaluationErrorsTest extends AssertJUnit
 
     private final int totalRows = 100;
 
+    private final int totalErrors = totalRows / 2;
+
     private PropertiesBatchEvaluationErrors errors;
 
     private ScriptPE script;
@@ -55,16 +56,16 @@ public class PropertiesBatchEvaluationErrorsTest extends AssertJUnit
     @Test
     public void testAllRowsFailWithTheSameErrorMessage()
     {
-        for (int i = 1; i <= totalRows; i++)
+        for (int i = 1; i <= totalErrors; i++)
         {
             errors.accumulateError(i, new EvaluatorException(ERROR_TEXT), CODE, script);
         }
 
         assertTrue(errors.hasErrors());
         assertEquals(
-                "Script malfunction in 100 out of 100 rows.\n"
-                        + "100 rows including [1, 2, 3] have failed due to the property 'propcode' causing a malfuction in the script "
-                        + "(name = 'script.py', registrator = 'admin@vip.net'): errtext\n"
+                "Script malfunction in 50 out of 100 rows.\n"
+                        + "50 rows including [1, 2, 3] have failed due to the property 'propcode' causing a malfuction in the script "
+                        + "(name = 'script.py', registrator = 'admin@vip.net')\n"
                         + "A detailed error report has been sent to your system administrator.",
                 errors.constructUserFailureMessage());
 
@@ -73,14 +74,15 @@ public class PropertiesBatchEvaluationErrorsTest extends AssertJUnit
         assertEquals(1, StringUtils.countMatches(email, ERROR_TEXT));
 
         String pattern =
-                "100 rows including [1, 2, 3] have failed due to the property 'propcode' causing a malfuction";
+                totalErrors
+                        + " rows including [1, 2, 3] have failed due to the property 'propcode' causing a malfuction";
         assertEquals(1, StringUtils.countMatches(email, pattern));
     }
 
     @Test
     public void testAllRowsFailWithADifferentErrorMessage()
     {
-        for (int i = 1; i <= totalRows; i++)
+        for (int i = 1; i <= totalErrors; i++)
         {
             errors.accumulateError(i, new EvaluatorException(ERROR_TEXT + i), CODE, script);
         }
@@ -88,8 +90,7 @@ public class PropertiesBatchEvaluationErrorsTest extends AssertJUnit
         assertTrue(errors.hasErrors());
 
         String errorMessage = errors.constructUserFailureMessage();
-        // not more than 3 messages shown to the user
-        assertEquals(MAX_ERRORS_IN_USER_MESSAGE, StringUtils.countMatches(errorMessage, ERROR_TEXT));
+        assertEquals(1, StringUtils.countMatches(errorMessage, "script.py"));
 
         String email = errors.constructErrorReportEmail();
         // only the first 10 stack traces are included