diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrors.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrors.java index 5a1c0a10a526a7ce6a8226b820aaf95cc652cd45..d63497e763b7c301c706325a41bd6444074f208e 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrors.java +++ b/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 = '"); diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrorsTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrorsTest.java index 87aa3ec00aa4670f4168854ce0b7e628e6d9552c..8385a9bb70623e32deb9ae921f6da3edf752a5e9 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/PropertiesBatchEvaluationErrorsTest.java +++ b/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