From 025659357ad88dfc44947e4e7deaa5dbd46e756f Mon Sep 17 00:00:00 2001 From: Vivek Singh Date: Wed, 28 Aug 2024 14:47:25 +0530 Subject: [PATCH] avniproject/avni-webapp#1306 - Location create csv check for no location provided. UserCatchment CSV - minor changes to error message. Fixed check for missing headers. Only add one error for a field. check wrong bool values in track location and beneficiary mode. Check for empty string instead of null value. Do not send regex in error message instead send in valid chars. General/All - ignore additional cells in data row. more test scenarios. --- .../org/avni/server/domain/JsonObject.java | 4 +- .../java/org/avni/server/domain/User.java | 4 +- .../batch/csv/writer/BulkLocationCreator.java | 28 ++-- .../csv/writer/UserAndCatchmentWriter.java | 67 ++++---- .../avni/server/importer/batch/model/Row.java | 14 +- .../org/avni/server/util/ValidationUtil.java | 4 +- .../java/org/avni/server/domain/UserTest.java | 1 - .../batch/csv/writer/BaseCSVImportTest.java | 7 +- .../BulkLocationCreatorIntegrationTest.java | 25 +++ .../BulkLocationEditorIntegrationTest.java | 85 +++++++--- ...UserAndCatchmentWriterIntegrationTest.java | 154 +++++++++++++++++- .../server/importer/batch/model/RowTest.java | 8 +- .../avni/server/util/ValidationUtilTest.java | 13 ++ 13 files changed, 329 insertions(+), 85 deletions(-) create mode 100644 avni-server-api/src/test/java/org/avni/server/util/ValidationUtilTest.java diff --git a/avni-server-api/src/main/java/org/avni/server/domain/JsonObject.java b/avni-server-api/src/main/java/org/avni/server/domain/JsonObject.java index 08df8bdf1..0f8365601 100644 --- a/avni-server-api/src/main/java/org/avni/server/domain/JsonObject.java +++ b/avni-server-api/src/main/java/org/avni/server/domain/JsonObject.java @@ -21,8 +21,8 @@ public JsonObject with(String key, Object value) { return this; } - public JsonObject withEmptyCheckAndTrim(String key, String value){ - if(!S.isEmpty(value)){ + public JsonObject withEmptyCheckAndTrim(String key, String value) { + if (!S.isEmpty(value)) { super.put(key, value.trim()); } return this; diff --git a/avni-server-api/src/main/java/org/avni/server/domain/User.java b/avni-server-api/src/main/java/org/avni/server/domain/User.java index 1ab1d93b9..65363baa7 100644 --- a/avni-server-api/src/main/java/org/avni/server/domain/User.java +++ b/avni-server-api/src/main/java/org/avni/server/domain/User.java @@ -394,7 +394,7 @@ public static void validateUsername(String username, String userSuffix) { throw new ValidationException(String.format("Invalid username '%s'. Include correct userSuffix %s at the end", username, userSuffix)); } if (ValidationUtil.checkNullOrEmptyOrContainsDisallowedCharacters(username.trim(), ValidationUtil.COMMON_INVALID_CHARS_PATTERN)) { - throw new ValidationException(String.format("Invalid username '%s', contains atleast one disallowed character %s", username, ValidationUtil.COMMON_INVALID_CHARS_PATTERN)); + throw new ValidationException(String.format("Invalid username '%s', contains at least one disallowed character %s", username, ValidationUtil.COMMON_INVALID_CHARS)); } } @@ -403,7 +403,7 @@ public static void validateUsername(String username, String userSuffix) { */ public static void validateName(String name) { if (ValidationUtil.checkNullOrEmptyOrContainsDisallowedCharacters(name, ValidationUtil.NAME_INVALID_CHARS_PATTERN)) { - throw new ValidationException(String.format("Invalid name '%s', contains atleast one disallowed character %s", name, ValidationUtil.NAME_INVALID_CHARS_PATTERN)); + throw new ValidationException(String.format("Invalid name '%s', contains at least one disallowed character %s", name, ValidationUtil.NAME_INVALID_CHARS)); } } } diff --git a/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreator.java b/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreator.java index 3c3b75277..e505e241f 100644 --- a/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreator.java +++ b/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreator.java @@ -1,5 +1,6 @@ package org.avni.server.importer.batch.csv.writer; +import com.google.common.collect.Sets; import org.avni.server.application.FormElement; import org.avni.server.builder.BuilderException; import org.avni.server.dao.AddressLevelTypeRepository; @@ -20,10 +21,7 @@ import org.springframework.util.StringUtils; import javax.transaction.Transactional; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Comparator; -import java.util.List; +import java.util.*; import java.util.stream.Collectors; // This class is need so that the logic can be instantiated in integration tests. Spring batch configuration is not working in integration tests. @@ -68,23 +66,29 @@ public void createLocation(Row row, List allErrorMsgs, List loca } } - private List validateCreateModeHeaders(String[] headers, List allErrorMsgs, String locationHierarchy) { + private List validateHeaders(String[] headers, List allErrorMsgs, String locationHierarchy) { List headerList = Arrays.asList(headers); - List locationTypeHeaders = checkIfHeaderHasLocationTypesInOrderForHierarchy(locationHierarchy, headerList, allErrorMsgs); - List additionalHeaders = new ArrayList<>(headerList.subList(locationTypeHeaders.size(), headerList.size())); + List locationTypeHeaders = checkIfHeaderHasLocationTypesAndInOrderForHierarchy(locationHierarchy, headerList, allErrorMsgs); + List additionalHeaders = headerList.size() > locationTypeHeaders.size() ? new ArrayList<>(headerList.subList(locationTypeHeaders.size(), headerList.size())) : new ArrayList<>(); checkIfHeaderRowHasUnknownHeaders(additionalHeaders, allErrorMsgs); return locationTypeHeaders; } - private List checkIfHeaderHasLocationTypesInOrderForHierarchy(String locationHierarchy, List headerList, List allErrorMsgs) { - List locationTypeNamesForHierachy = importService.getAddressLevelTypesForCreateModeSingleHierarchy(locationHierarchy) + private List checkIfHeaderHasLocationTypesAndInOrderForHierarchy(String locationHierarchy, List headerList, List allErrorMsgs) { + List locationTypeNamesForHierarchy = importService.getAddressLevelTypesForCreateModeSingleHierarchy(locationHierarchy) .stream().map(AddressLevelType::getName).collect(Collectors.toList()); - if (headerList.size() >= locationTypeNamesForHierachy.size() && !headerList.subList(0, locationTypeNamesForHierachy.size()).equals(locationTypeNamesForHierachy)) { + HashSet expectedHeaders = new HashSet<>(locationTypeNamesForHierarchy); + if (Sets.difference(new HashSet<>(expectedHeaders), new HashSet<>(headerList)).size() == locationTypeNamesForHierarchy.size()) { allErrorMsgs.add(LocationTypesHeaderError); throw new RuntimeException(String.join(", ", allErrorMsgs)); } - return locationTypeNamesForHierachy; + + if (headerList.size() >= locationTypeNamesForHierarchy.size() && !headerList.subList(0, locationTypeNamesForHierarchy.size()).equals(locationTypeNamesForHierarchy)) { + allErrorMsgs.add(LocationTypesHeaderError); + throw new RuntimeException(String.join(", ", allErrorMsgs)); + } + return locationTypeNamesForHierarchy; } private void checkIfHeaderRowHasUnknownHeaders(List additionalHeaders, List allErrorMsgs) { @@ -136,7 +140,7 @@ private void validateRow(Row row, List hierarchicalLocationTypeNames, Li @Transactional(Transactional.TxType.REQUIRES_NEW) public void write(List rows, String idBasedLocationHierarchy) { List allErrorMsgs = new ArrayList<>(); - List hierarchicalLocationTypeNames = validateCreateModeHeaders(rows.get(0).getHeaders(), allErrorMsgs, idBasedLocationHierarchy); + List hierarchicalLocationTypeNames = validateHeaders(rows.get(0).getHeaders(), allErrorMsgs, idBasedLocationHierarchy); for (Row row : rows) { if (skipRow(row, hierarchicalLocationTypeNames)) { continue; diff --git a/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriter.java b/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriter.java index 74be9a2f9..8f5f97609 100644 --- a/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriter.java +++ b/avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriter.java @@ -1,6 +1,5 @@ package org.avni.server.importer.batch.csv.writer; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.avni.server.dao.LocationRepository; import org.avni.server.dao.UserRepository; @@ -10,6 +9,7 @@ import org.avni.server.importer.batch.csv.writer.header.UsersAndCatchmentsHeaders; import org.avni.server.importer.batch.model.Row; import org.avni.server.service.*; +import org.avni.server.util.PhoneNumberUtil; import org.avni.server.util.RegionUtil; import org.avni.server.util.S; import org.avni.server.web.request.syncAttribute.UserSyncSettings; @@ -32,7 +32,6 @@ @Component public class UserAndCatchmentWriter implements ItemWriter, Serializable { - public static final String ERR_MSG_DELIMITER = "\",\""; private final UserService userService; private final CatchmentService catchmentService; private final LocationRepository locationRepository; @@ -44,14 +43,15 @@ public class UserAndCatchmentWriter implements ItemWriter, Serializable { private final Pattern compoundHeaderPattern; private final ResetSyncService resetSyncService; - private static final String ERR_MSG_MANDATORY_FIELD = "Invalid or Empty value specified for mandatory field %s"; - private static final String ERR_MSG_LOCATION_FIELD = "Provided Location does not exist in Avni. Please add it or check for spelling mistakes '%s'"; - private static final String ERR_MSG_LOCALE_FIELD = "Provided value '%s' for Preferred Language is invalid"; - private static final String ERR_MSG_DATE_PICKER_FIELD = "Provided value '%s' for Date picker mode is invalid"; - private static final String ERR_MSG_UNKNOWN_HEADERS = "Unknown headers - %s included in file. Please refer to sample file for valid list of headers"; + private static final String ERR_MSG_MANDATORY_OR_INVALID_FIELD = "Invalid or Empty value specified for mandatory field %s."; + private static final String ERR_MSG_LOCATION_FIELD = "Provided Location does not exist in Avni. Please add it or check for spelling mistakes '%s'."; + private static final String ERR_MSG_LOCALE_FIELD = "Provided value '%s' for Preferred Language is invalid."; + private static final String ERR_MSG_DATE_PICKER_FIELD = "Provided value '%s' for Date picker mode is invalid."; + private static final String ERR_MSG_INVALID_PHONE_NUMBER = "Provided value '%s' for phone number is invalid."; + private static final String ERR_MSG_UNKNOWN_HEADERS = "Unknown headers - %s included in file. Please refer to sample file for valid list of headers."; private static final String ERR_MSG_MISSING_MANDATORY_FIELDS = "Mandatory columns are missing from uploaded file - %s. Please refer to sample file for the list of mandatory headers."; - private static final String ERR_MSG_INVALID_CONCEPT_ANSWER = "'%s' is not a valid value for the concept '%s'" + - "To input this value, add this as an answer to the coded concept '%s'"; + private static final String ERR_MSG_INVALID_CONCEPT_ANSWER = "'%s' is not a valid value for the concept '%s'. " + + "To input this value, add this as an answer to the coded concept '%s'."; private static final String METADATA_ROW_START_STRING = "Mandatory field."; private static final List DATE_PICKER_MODE_OPTIONS = Arrays.asList("calendar", "spinner"); @@ -93,7 +93,7 @@ private void validateHeaders(String[] headers) { checkForMissingHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes); checkForUnknownHeaders(headerList, allErrorMsgs, expectedStandardHeaders, syncAttributeHeadersForSubjectTypes); if (!allErrorMsgs.isEmpty()) { - throw new RuntimeException(String.join(ERR_MSG_DELIMITER, allErrorMsgs)); + throw new RuntimeException(createMultiErrorMessage(allErrorMsgs)); } } @@ -107,11 +107,10 @@ private void checkForUnknownHeaders(List headerList, List allErr } } - private void checkForMissingHeaders(List headerList, List allErrorMsgs, List expectedStandardHeaders, List syncAttributeHeadersForSubjectTypes) { + private void checkForMissingHeaders(List headerList, List allErrorMsgs, List expectedStandardHeaders, List expectedSyncAttributeHeadersForSubjectTypes) { HashSet expectedHeaders = new HashSet<>(expectedStandardHeaders); - expectedHeaders.addAll(syncAttributeHeadersForSubjectTypes); + expectedHeaders.addAll(expectedSyncAttributeHeadersForSubjectTypes); HashSet presentHeaders = new HashSet<>(headerList); - presentHeaders.addAll(syncAttributeHeadersForSubjectTypes); Sets.SetView missingHeaders = Sets.difference(expectedHeaders, presentHeaders); if (!missingHeaders.isEmpty()) { allErrorMsgs.add(String.format(ERR_MSG_MISSING_MANDATORY_FIELDS, String.join(", ", missingHeaders))); @@ -138,7 +137,7 @@ private void write(Row row) throws IDPException { Organisation organisation = UserContextHolder.getUserContext().getOrganisation(); String userSuffix = "@".concat(organisation.getEffectiveUsernameSuffix()); JsonObject syncSettings = constructSyncSettings(row, rowValidationErrorMsgs); - validateRowAndAssimilateErrors(rowValidationErrorMsgs, fullAddress, catchmentName, nameOfUser, username, email, phoneNumber, language, datePickerMode, location, locale, userSuffix); + validateRowAndAssimilateErrors(rowValidationErrorMsgs, fullAddress, catchmentName, nameOfUser, username, email, phoneNumber, language, datePickerMode, location, locale, userSuffix, trackLocation, beneficiaryMode); Catchment catchment = catchmentService.createOrUpdate(catchmentName, location); User user = userRepository.findByUsername(username.trim()); User currentUser = userService.getCurrentUser(); @@ -175,26 +174,34 @@ private void write(Row row) throws IDPException { } } - private void validateRowAndAssimilateErrors(List rowValidationErrorMsgs, String fullAddress, String catchmentName, String nameOfUser, String username, String email, String phoneNumber, String language, String datePickerMode, AddressLevel location, Locale locale, String userSuffix) { - addErrMsgIfValidationFails(!StringUtils.hasLength(catchmentName), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, CATCHMENT_NAME)); - addErrMsgIfValidationFails(!StringUtils.hasLength(username), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, USERNAME)); - addErrMsgIfValidationFails(!StringUtils.hasLength(nameOfUser), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, FULL_NAME_OF_USER)); - addErrMsgIfValidationFails(!StringUtils.hasLength(email), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, EMAIL_ADDRESS)); - addErrMsgIfValidationFails(!StringUtils.hasLength(phoneNumber), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_FIELD, MOBILE_NUMBER)); + private void validateRowAndAssimilateErrors(List rowValidationErrorMsgs, String fullAddress, String catchmentName, String nameOfUser, String username, String email, String phoneNumber, String language, String datePickerMode, AddressLevel location, Locale locale, String userSuffix, Boolean trackLocation, Boolean beneficiaryMode) { + addErrMsgIfValidationFails(!StringUtils.hasLength(catchmentName), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, CATCHMENT_NAME)); + if (!addErrMsgIfValidationFails(!StringUtils.hasLength(username), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, USERNAME))) + extractUserUsernameValidationErrMsg(rowValidationErrorMsgs, username, userSuffix); + if (!addErrMsgIfValidationFails(!StringUtils.hasLength(nameOfUser), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, FULL_NAME_OF_USER))) + extractUserNameValidationErrMsg(rowValidationErrorMsgs, nameOfUser); + if (!addErrMsgIfValidationFails(!StringUtils.hasLength(email), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, EMAIL_ADDRESS))) + extractUserEmailValidationErrMsg(rowValidationErrorMsgs, email); + if (!addErrMsgIfValidationFails(!StringUtils.hasLength(phoneNumber), rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, MOBILE_NUMBER))) + addErrMsgIfValidationFails(!PhoneNumberUtil.isValidPhoneNumber(phoneNumber, RegionUtil.getCurrentUserRegion()), rowValidationErrorMsgs, format(ERR_MSG_INVALID_PHONE_NUMBER, MOBILE_NUMBER)); - addErrMsgIfValidationFails(Objects.isNull(location), rowValidationErrorMsgs, format(ERR_MSG_LOCATION_FIELD, fullAddress)); - addErrMsgIfValidationFails(Objects.isNull(locale), rowValidationErrorMsgs, format(ERR_MSG_LOCALE_FIELD, language)); - addErrMsgIfValidationFails(Objects.isNull(datePickerMode) || !DATE_PICKER_MODE_OPTIONS.contains(datePickerMode), + addErrMsgIfValidationFails(StringUtils.isEmpty(location), rowValidationErrorMsgs, format(ERR_MSG_LOCATION_FIELD, fullAddress)); + addErrMsgIfValidationFails(StringUtils.isEmpty(locale), rowValidationErrorMsgs, format(ERR_MSG_LOCALE_FIELD, language)); + addErrMsgIfValidationFails(StringUtils.isEmpty(datePickerMode) || !DATE_PICKER_MODE_OPTIONS.contains(datePickerMode), rowValidationErrorMsgs, format(ERR_MSG_DATE_PICKER_FIELD, datePickerMode)); - extractUserUsernameValidationErrMsg(rowValidationErrorMsgs, username, userSuffix); - extractUserNameValidationErrMsg(rowValidationErrorMsgs, nameOfUser); - extractUserEmailValidationErrMsg(rowValidationErrorMsgs, email); + addErrMsgIfValidationFails(trackLocation == null, rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, TRACK_LOCATION)); + addErrMsgIfValidationFails(beneficiaryMode == null, rowValidationErrorMsgs, format(ERR_MSG_MANDATORY_OR_INVALID_FIELD, ENABLE_BENEFICIARY_MODE)); + if (!rowValidationErrorMsgs.isEmpty()) { - throw new RuntimeException(String.join(ERR_MSG_DELIMITER, rowValidationErrorMsgs)); + throw new RuntimeException(createMultiErrorMessage(rowValidationErrorMsgs)); } } + private static String createMultiErrorMessage(List errorMsgs) { + return errorMsgs.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining("; ")); + } + private void extractUserNameValidationErrMsg(List rowValidationErrorMsgs, String nameOfUser) { try { User.validateName(nameOfUser); @@ -219,10 +226,11 @@ private void extractUserUsernameValidationErrMsg(List rowValidationError } } - private void addErrMsgIfValidationFails(boolean validationCheckResult, List rowValidationErrorMsgs, String validationErrorMessage) { + private boolean addErrMsgIfValidationFails(boolean validationCheckResult, List rowValidationErrorMsgs, String validationErrorMessage) { if (validationCheckResult) { rowValidationErrorMsgs.add(validationErrorMessage); } + return validationCheckResult; } private JsonObject constructSyncSettings(Row row, List rowValidationErrorMsgs) { @@ -245,7 +253,8 @@ private void updateSyncSettingsFor(String saHeader, Row row, Map { public static final Pattern TRUE_VALUE = Pattern.compile("y|yes|true|1", Pattern.CASE_INSENSITIVE); + public static final Pattern FALSE_VALUE = Pattern.compile("n|no|false|0", Pattern.CASE_INSENSITIVE); private final String[] headers; private final String[] values; public Row(String[] headers, String[] values) { this.headers = headers; this.values = values; - IntStream.range(0, values.length).forEach(index -> this.put(headers[index].trim(), values[index].trim())); + IntStream.range(0, headers.length).forEach(index -> { + this.put(headers[index].trim(), values.length > index ? values[index].trim() : ""); + }); } private String nullSafeTrim(String s) { @@ -52,12 +55,17 @@ public String getOrDefault(Object key, String defaultValue) { @Override public String toString() { return IntStream.range(0, headers.length) - .mapToObj(index -> index < values.length? format("\"%s\"", values[index]): "\"\"") + .mapToObj(index -> index < values.length ? format("\"%s\"", values[index]) : "\"\"") .reduce((c1, c2) -> format("%s,%s", c1, c2)) .get(); } public Boolean getBool(String header) { - return TRUE_VALUE.matcher(String.valueOf(get(header))).matches(); + if (TRUE_VALUE.matcher(String.valueOf(get(header))).matches()) { + return true; + } else if (FALSE_VALUE.matcher(String.valueOf(get(header))).matches()) { + return false; + } + return null; } } diff --git a/avni-server-api/src/main/java/org/avni/server/util/ValidationUtil.java b/avni-server-api/src/main/java/org/avni/server/util/ValidationUtil.java index f22abe941..509f4ef3e 100644 --- a/avni-server-api/src/main/java/org/avni/server/util/ValidationUtil.java +++ b/avni-server-api/src/main/java/org/avni/server/util/ValidationUtil.java @@ -5,14 +5,16 @@ public class ValidationUtil { public static final Pattern COMMON_INVALID_CHARS_PATTERN = Pattern.compile("^.*[<>=\"'].*$"); + public static final Pattern COMMON_INVALID_CHARS = Pattern.compile("<> = \" '"); public static final Pattern NAME_INVALID_CHARS_PATTERN = Pattern.compile("^.*[<>=\"].*$"); + public static final Pattern NAME_INVALID_CHARS = Pattern.compile("<> = \""); public static boolean checkNull(Object checkObject) { return checkObject == null; } public static boolean checkEmptyString(String checkString) { - return checkString.trim().equals(""); + return checkString.trim().isEmpty(); } public static boolean containsDisallowedPattern(String checkString, Pattern pattern) { diff --git a/avni-server-api/src/test/java/org/avni/server/domain/UserTest.java b/avni-server-api/src/test/java/org/avni/server/domain/UserTest.java index dc1806157..f531c00f6 100644 --- a/avni-server-api/src/test/java/org/avni/server/domain/UserTest.java +++ b/avni-server-api/src/test/java/org/avni/server/domain/UserTest.java @@ -6,7 +6,6 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; public class UserTest { @Test diff --git a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BaseCSVImportTest.java b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BaseCSVImportTest.java index e9ef5976b..3a296ddd0 100644 --- a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BaseCSVImportTest.java +++ b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BaseCSVImportTest.java @@ -1,9 +1,6 @@ package org.avni.server.importer.batch.csv.writer; import org.avni.server.common.AbstractControllerIntegrationTest; -import org.avni.server.domain.AddressLevel; - -import static org.junit.Assert.assertNotNull; public abstract class BaseCSVImportTest extends AbstractControllerIntegrationTest { protected String[] header(String... cells) { @@ -21,4 +18,8 @@ protected String[] lineage(String ... lineage) { protected String error(String message) { return message; } + + protected String hasError(String s) { + return s; + } } diff --git a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreatorIntegrationTest.java b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreatorIntegrationTest.java index 8dd8ce855..583bbe568 100644 --- a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreatorIntegrationTest.java +++ b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationCreatorIntegrationTest.java @@ -121,6 +121,26 @@ private void failure(String[] headers, String[] cells, String errorMessage) { assertEquals(before, after); } + private void failsOnMissingHeader(String[] headers, String... errorMessages) { + try { + bulkLocationCreator.write(Collections.singletonList(new Row(headers, new String[0])), hierarchy); + fail(); + } catch (RuntimeException e) { + String message = e.getMessage(); + if (message == null) { + e.printStackTrace(); + fail(); + } else { + Arrays.stream(errorMessages).forEach(s -> { + if (!message.contains(s)) { + e.printStackTrace(); + fail("Expected error message: " + s + " not present in: " + message); + } + }); + } + } + } + private String[] lineageExists(String... lineage) { return lineage; } @@ -241,6 +261,11 @@ public void shouldCreate() { dataRow("Bihar", "Vaishali", " Mahua ", "23.45,43.85"), newLocationsCreated(0)); + // missing headers + failsOnMissingHeader( + header(), + hasError("Location types missing or not in order in header for specified Location Hierarchy. Please refer to sample file for valid list of headers.") + ); treatAsDescriptor(header("State", "District", "Block", "GPS coordinates"), dataRow("Example: state 1", "Ex: distr 1", "Ex: blo 1", "Ex. 23.45,43.85")); diff --git a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationEditorIntegrationTest.java b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationEditorIntegrationTest.java index ad9dc0c90..b7d0b82ae 100644 --- a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationEditorIntegrationTest.java +++ b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/BulkLocationEditorIntegrationTest.java @@ -104,6 +104,54 @@ private String[] verifyNotExists(String... strings) { return strings; } + private void treatAsDescriptor(String[] headers, String... descriptorCells) { + long before = locationRepository.count(); + bulkLocationEditor.write(Collections.singletonList(new Row(headers, descriptorCells))); + long after = locationRepository.count(); + assertEquals(before, after); + } + + private void success(String[] headers, String[] dataRow, String[] exists, String[] ... notExists) { + bulkLocationEditor.editLocation(new Row(headers, dataRow), new ArrayList<>()); + lineageExists(exists); + for (String[] notExist : notExists) { + lineageNotExists(notExist); + } + } + + private void failure(String[] headers, String[] dataRow, String errorMessage, String[] exists, String[] ... notExists) { + try { + bulkLocationEditor.editLocation(new Row(headers, dataRow), new ArrayList<>()); + fail(); + } catch (Exception exception) { + assertEquals(errorMessage, exception.getMessage()); + } + lineageExists(exists); + for (String[] notExist : notExists) { + lineageNotExists(notExist); + } + } + + private void failsOnMissingHeader(String[] headers, String... errorMessages) { + try { + bulkLocationEditor.write(Collections.singletonList(new Row(headers, new String[0]))); + fail(); + } catch (RuntimeException e) { + String message = e.getMessage(); + if (message == null) { + e.printStackTrace(); + fail(); + } else { + Arrays.stream(errorMessages).forEach(s -> { + if (!message.contains(s)) { + e.printStackTrace(); + fail("Expected error message: " + s + " not present in: " + message); + } + }); + } + } + } + @Test public void shouldEdit() { // no change @@ -153,33 +201,16 @@ public void shouldEdit() { "Enter new name here ONLY if it needs to be updated", "Hierarchy of parent location that should contain the child location", "Ex: 23.45,43.85")); - } - - private void treatAsDescriptor(String[] headers, String... descriptorCells) { - long before = locationRepository.count(); - bulkLocationEditor.write(Collections.singletonList(new Row(headers, descriptorCells))); - long after = locationRepository.count(); - assertEquals(before, after); - } - - private void success(String[] headers, String[] dataRow, String[] exists, String[] ... notExists) { - bulkLocationEditor.editLocation(new Row(headers, dataRow), new ArrayList<>()); - lineageExists(exists); - for (String[] notExist : notExists) { - lineageNotExists(notExist); - } - } - private void failure(String[] headers, String[] dataRow, String errorMessage, String[] exists, String[] ... notExists) { - try { - bulkLocationEditor.editLocation(new Row(headers, dataRow), new ArrayList<>()); - fail(); - } catch (Exception exception) { - assertEquals(errorMessage, exception.getMessage()); - } - lineageExists(exists); - for (String[] notExist : notExists) { - lineageNotExists(notExist); - } + // missing header - nothing provided + failsOnMissingHeader( + header(), + hasError("'Location with full hierarchy' is required, At least one of 'New location name', 'GPS coordinates' or 'Parent location with full hierarchy' is required") + ); + // missing header - New location name + failsOnMissingHeader( + header("Location with full hierarchy"), + hasError("At least one of 'New location name', 'GPS coordinates' or 'Parent location with full hierarchy'") + ); } } diff --git a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriterIntegrationTest.java b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriterIntegrationTest.java index 7986a3d55..9b248861e 100644 --- a/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriterIntegrationTest.java +++ b/avni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/UserAndCatchmentWriterIntegrationTest.java @@ -1,14 +1,14 @@ package org.avni.server.importer.batch.csv.writer; -import org.avni.server.application.*; import org.avni.server.dao.CatchmentRepository; -import org.avni.server.dao.LocationRepository; import org.avni.server.dao.UserRepository; import org.avni.server.dao.application.FormRepository; -import org.avni.server.domain.*; +import org.avni.server.domain.AddressLevel; +import org.avni.server.domain.AddressLevelType; +import org.avni.server.domain.Concept; +import org.avni.server.domain.ConceptDataType; import org.avni.server.domain.factory.AddressLevelBuilder; import org.avni.server.domain.factory.AddressLevelTypeBuilder; -import org.avni.server.domain.factory.metadata.TestFormBuilder; import org.avni.server.domain.metadata.SubjectTypeBuilder; import org.avni.server.importer.batch.model.Row; import org.avni.server.service.IDPException; @@ -25,6 +25,7 @@ import java.util.UUID; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; @Sql(value = {"/tear-down.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) @Sql(value = {"/tear-down.sql"}, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) @@ -98,6 +99,50 @@ private void success(String[] headers, String[] cells, boolean catchmentCreated, assertEquals(userRepository.count(), numberOfUsers); } + private void failure(String[] headers, String[] cells, String... errorMessages) throws IDPException { + try { + userAndCatchmentWriter.write(Collections.singletonList(new Row(headers, cells))); + fail(); + } catch (RuntimeException e) { + String message = e.getMessage(); + if (message == null) { + e.printStackTrace(); + fail(); + } else { + Arrays.stream(errorMessages).forEach(s -> { + if (!message.contains(s)) { + e.printStackTrace(); + fail("Expected error message: " + s + " not present in: " + message); + } + }); + } + } + } + + private void failsOnMissingHeader(String[] headers, String... errorMessages) throws IDPException { + try { + userAndCatchmentWriter.write(Collections.singletonList(new Row(headers, new String[]{}))); + fail(); + } catch (RuntimeException e) { + String message = e.getMessage(); + if (message == null) { + e.printStackTrace(); + fail(); + } else { + Arrays.stream(errorMessages).forEach(s -> { + if (!message.contains(s)) { + e.printStackTrace(); + fail("Expected error message: " + s + " not present in: " + message); + } + }); + } + } + } + + private void treatAsDescriptor(String[] headers, String... additionalHeaders) throws IDPException { + success(headers, additionalHeaders, false, false); + } + @Test public void shouldCreateUpdate() throws IDPException { // new catchment, new user @@ -106,5 +151,106 @@ public void shouldCreateUpdate() throws IDPException { dataRow("Bihar, District1, Block11", "Catchment 1", "username1@example", "User 1", "username1@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"), catchmentCreated(true), userCreated(true)); + // existing catchment new user + success( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 1", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"), + catchmentCreated(false), + userCreated(true)); + // new catchment existing user + success( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"), + catchmentCreated(true), + userCreated(false)); + // existing catchment existing user + success( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"), + catchmentCreated(false), + userCreated(false)); + // with spaces + success( + header(" Location with full hierarchy", " Catchment Name", "Username ", " Full Name of User", "Email Address", "Mobile Number", " Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", " Identifier Prefix", " User Groups", " SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow(" Bihar, District1, Block11", " Catchment 4", " username3@example", " User 3", " username3@example.com ", " 9455509147 ", "English ", "true ", " spinner", " false", "", " User Group 1", " Answer 1"), + catchmentCreated(true), + userCreated(true)); + + // wrong - username, email, phone number, language, track location, date picker mode, enable beneficiary mode + failure( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 1", "username1@exmplee", "User 1", "username1@examplecom", "9455047", "Irish", "truee", "spinnerr", "falsee", "", "User Group 1", "Answer 1"), + hasError("Invalid username 'username1@exmplee'. Include correct userSuffix @example at the end"), + hasError("Invalid email address username1@examplecom"), + hasError("Provided value 'Mobile Number' for phone number is invalid."), + hasError("Provided value 'Irish' for Preferred Language is invalid."), + hasError("Provided value 'spinnerr' for Date picker mode is invalid."), + hasError("Invalid or Empty value specified for mandatory field Track Location"), + hasError("Invalid or Empty value specified for mandatory field Enable Beneficiary") + ); + // empty - catchment name, username, Full Name of User, email, phone number, track location, date picker mode, enable beneficiary mode + failure( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", " ", "", " ", " ", " ", " ", " ", "", "", "", "User Group 1", "Answer 1"), + hasError("Invalid or Empty value specified for mandatory field Catchment Name."), + hasError("Invalid or Empty value specified for mandatory field Username."), + hasError("Invalid or Empty value specified for mandatory field Full Name of User."), + hasError("Invalid or Empty value specified for mandatory field Email Address."), + hasError("Invalid or Empty value specified for mandatory field Mobile Number."), + hasError("Provided value '' for Date picker mode is invalid."), + hasError("Invalid or Empty value specified for mandatory field Track Location."), + hasError("Invalid or Empty value specified for mandatory field Enable Beneficiary") + ); + + // invalid User Group Name + failure( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1345", "Answer 1"), + hasError("Group 'User Group 1345' not found") + ); + // same user group twice + success( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username4@example", "User 4", "username4@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1,User Group 1", "Answer 1"), + catchmentCreated(false), + userCreated(true) + ); + + // invalid sync attribute + failure( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1223"), + hasError("'Answer 1223' is not a valid value for the concept 'Sync Concept'.") + ); + + // Wrong location hierarchy + failure( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, NoBlock11", "Catchment 3", "username2@example", "User 2", "username2@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1"), + hasError("Provided Location does not exist in Avni. Please add it or check for spelling mistakes 'Bihar, District1, NoBlock11'.") + ); + + // Missing headers - sync attributes + failsOnMissingHeader( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups"), + hasError("Mandatory columns are missing from uploaded file - SubjectTypeWithSyncAttributeBasedSync->Sync Concept. Please refer to sample file for the list of mandatory headers.") + ); + // Missing headers - all + failsOnMissingHeader( + header(), + hasError("Mandatory columns are missing from uploaded file - Track Location, Identifier Prefix, Catchment Name, Full Name of User, Mobile Number, Enable Beneficiary mode, User Groups, Username, SubjectTypeWithSyncAttributeBasedSync->Sync Concept, Preferred Language, Location with full hierarchy, Date picker mode, Email Address. Please refer to sample file for the list of mandatory headers.") + ); + + // allow additional cells in data row + success( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Bihar, District1, Block11", "Catchment 3", "username5@example", "User 5", "username5@example.com", "9455509147", "English", "true", "spinner", "false", "", "User Group 1", "Answer 1", "Foo"), + catchmentCreated(false), + userCreated(true) + ); + + treatAsDescriptor( + header("Location with full hierarchy", "Catchment Name", "Username", "Full Name of User", "Email Address", "Mobile Number", "Preferred Language", "Track Location", "Date picker mode", "Enable Beneficiary mode", "Identifier Prefix", "User Groups", "SubjectTypeWithSyncAttributeBasedSync->Sync Concept"), + dataRow("Mandatory field. Can be found from Admin -> Locations -> Click Export", "Mandatory field", "Mandatory field", "Mandatory field", "Mandatory field", "Mandatory field. Prefix the mobile number with country code", "Allowed values: {English, Hindi}. Only single value allowed. Default: English", "Allowed values: yes, no. Default: no", "Allowed values: calendar, spinner. Default: calendar", "Allowed values: yes, no. Default: no", "", "Allowed values: {Administrators, Everyone}")); } } diff --git a/avni-server-api/src/test/java/org/avni/server/importer/batch/model/RowTest.java b/avni-server-api/src/test/java/org/avni/server/importer/batch/model/RowTest.java index e8d5d0b92..eb569a6c1 100644 --- a/avni-server-api/src/test/java/org/avni/server/importer/batch/model/RowTest.java +++ b/avni-server-api/src/test/java/org/avni/server/importer/batch/model/RowTest.java @@ -6,13 +6,19 @@ public class RowTest { @Test - public void toStringShouldSerialiseProperly() throws Exception { + public void toStringShouldSerialiseProperly() { String[] headers = {"A", "B"}; assertEquals("\"AA\",\"\"", new Row(headers, new String[]{"AA"}).toString()); assertEquals("\"AA\",\"BB\"", new Row(headers, new String[]{"AA", "BB"}).toString()); assertEquals("\"AB, CD\",\"BB, EE\"", new Row(headers, new String[]{"AB, CD", "BB, EE"}).toString()); } + @Test + public void allowExtraColumnsInData() { + String[] headers = {"A", "B"}; + new Row(headers, new String[]{"AA", "BB", "CC"}); + } + @Test public void trimHeadersAndValues() { String[] headers = {"A", "B"}; diff --git a/avni-server-api/src/test/java/org/avni/server/util/ValidationUtilTest.java b/avni-server-api/src/test/java/org/avni/server/util/ValidationUtilTest.java new file mode 100644 index 000000000..9a41c1de4 --- /dev/null +++ b/avni-server-api/src/test/java/org/avni/server/util/ValidationUtilTest.java @@ -0,0 +1,13 @@ +package org.avni.server.util; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class ValidationUtilTest { + @Test + public void checkChars() { + assertTrue(ValidationUtil.containsDisallowedPattern(">", ValidationUtil.COMMON_INVALID_CHARS_PATTERN)); + assertTrue(ValidationUtil.containsDisallowedPattern("\"", ValidationUtil.COMMON_INVALID_CHARS_PATTERN)); + } +}