From af6d0c155eb82abc2d489ee33cce41066f3a4717 Mon Sep 17 00:00:00 2001 From: IamMujuziMoses Date: Tue, 19 Mar 2024 11:50:10 +0300 Subject: [PATCH] TRUNK-4455: All metadata names should be case-insensitive. --- .../openmrs/api/AdministrationService.java | 17 ++++++++++++ .../org/openmrs/api/db/AdministrationDAO.java | 5 ++++ .../hibernate/HibernateAdministrationDAO.java | 15 +++++++++++ .../api/impl/AdministrationServiceImpl.java | 8 ++++++ .../openmrs/api/impl/ConceptServiceImpl.java | 2 +- .../org/openmrs/util/PrivilegeConstants.java | 3 +++ .../openmrs/validator/CohortValidator.java | 9 +------ .../validator/ConceptDatatypeValidator.java | 11 +------- .../validator/ConceptSourceValidator.java | 10 +------ .../validator/LocationTagValidator.java | 10 +------ .../openmrs/validator/PrivilegeValidator.java | 10 +------ .../org/openmrs/validator/RoleValidator.java | 10 +------ .../org/openmrs/validator/ValidateUtil.java | 27 +++++++++++++++++++ api/src/main/resources/messages.properties | 2 -- .../api/AdministrationServiceTest.java | 15 +++++++++++ .../java/org/openmrs/api/UserServiceTest.java | 4 +-- .../openmrs/validator/ValidateUtilTest.java | 25 +++++++++++++++++ .../openmrs/include/standardTestDataset.xml | 4 +-- 18 files changed, 126 insertions(+), 61 deletions(-) diff --git a/api/src/main/java/org/openmrs/api/AdministrationService.java b/api/src/main/java/org/openmrs/api/AdministrationService.java index de1466f38e8d..4ce42df45545 100644 --- a/api/src/main/java/org/openmrs/api/AdministrationService.java +++ b/api/src/main/java/org/openmrs/api/AdministrationService.java @@ -392,4 +392,21 @@ public interface AdministrationService extends OpenmrsService { * @since 2.4 */ public void updatePostgresSequence(); + + /** + * Returns an object of a given class that matches a given field value + * + * @param aClass The class to which the objects belong + * @param field the field of the object to retrieve + * @param value the field value of the object to retrieve + * @return object of a given class that matches a given field value + * @throws APIException + * + * + * @since 2.7.0 + */ + @Authorized(PrivilegeConstants.GET_OPENMRS_OBJECTS) + public T getObjectByFieldValue(Class aClass, String field, String value) throws APIException; } diff --git a/api/src/main/java/org/openmrs/api/db/AdministrationDAO.java b/api/src/main/java/org/openmrs/api/db/AdministrationDAO.java index 2cfc9710aded..c2b81986a0f6 100644 --- a/api/src/main/java/org/openmrs/api/db/AdministrationDAO.java +++ b/api/src/main/java/org/openmrs/api/db/AdministrationDAO.java @@ -85,4 +85,9 @@ public interface AdministrationDAO { * @see AdministrationService#updatePostgresSequence() */ public void updatePostgresSequence() throws DAOException; + + /** + * @see AdministrationService#getObjectByFieldValue(Class, String, String) + */ + public T getObjectByFieldValue(Class aClass, String field, String value) throws DAOException; } diff --git a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java index 3a0a6a217541..34e6dce29b21 100644 --- a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java +++ b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateAdministrationDAO.java @@ -429,4 +429,19 @@ public void execute(Connection con) throws SQLException { }); } } + + /** + * @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String) + */ + @Override + public T getObjectByFieldValue(Class aClass, String field, String value) throws DAOException { + Session session = sessionFactory.getCurrentSession(); + CriteriaBuilder cb = session.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(aClass); + Root root = cq.from(aClass); + + cq.where(cb.equal(root.get(field), value)); + + return session.createQuery(cq).uniqueResult(); + } } diff --git a/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java index 91f9b5180b40..7ae89b01ea19 100644 --- a/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java +++ b/api/src/main/java/org/openmrs/api/impl/AdministrationServiceImpl.java @@ -942,4 +942,12 @@ public void updatePostgresSequence() { dao.updatePostgresSequence(); } + /** + * @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String) + */ + @Override + @Transactional(readOnly = true) + public T getObjectByFieldValue(Class aClass, String field, String value) throws APIException { + return dao.getObjectByFieldValue(aClass, field, value); + } } diff --git a/api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java index 44c40413f2a0..7f73acf1b122 100644 --- a/api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java +++ b/api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java @@ -1017,7 +1017,7 @@ public ConceptSource purgeConceptSource(ConceptSource cs) throws APIException { @Override public ConceptSource retireConceptSource(ConceptSource cs, String reason) throws APIException { // retireReason is automatically set in BaseRetireHandler - return dao.saveConceptSource(cs); + return Context.getConceptService().saveConceptSource(cs); } /** diff --git a/api/src/main/java/org/openmrs/util/PrivilegeConstants.java b/api/src/main/java/org/openmrs/util/PrivilegeConstants.java index 9312f4d41c3a..5898bc89cbbd 100644 --- a/api/src/main/java/org/openmrs/util/PrivilegeConstants.java +++ b/api/src/main/java/org/openmrs/util/PrivilegeConstants.java @@ -112,6 +112,9 @@ private PrivilegeConstants() { @AddOnStartup(description = "Able to get provider attribute types") public static final String GET_PROVIDER_ATTRIBUTE_TYPES = "Get Provider Attribute Types"; + @AddOnStartup(description = "Able to get openmrs objects") + public static final String GET_OPENMRS_OBJECTS = "Get Openmrs Objects"; + @AddOnStartup(description = "Able to get person objects") public static final String GET_PERSONS = "Get People"; diff --git a/api/src/main/java/org/openmrs/validator/CohortValidator.java b/api/src/main/java/org/openmrs/validator/CohortValidator.java index 32f58a26d7a7..ef3d309bd5de 100644 --- a/api/src/main/java/org/openmrs/validator/CohortValidator.java +++ b/api/src/main/java/org/openmrs/validator/CohortValidator.java @@ -12,7 +12,6 @@ import java.util.Collection; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.lang3.StringUtils; import org.openmrs.Cohort; import org.openmrs.CohortMembership; import org.openmrs.Patient; @@ -51,13 +50,7 @@ public void validate(Object obj, Errors errors) { Cohort cohort = (Cohort) obj; - if (StringUtils.isNotBlank(cohort.getName())) { - Cohort existingCohort = Context.getCohortService().getCohortByName(cohort.getName()); - if (existingCohort != null && !existingCohort.getUuid().equals(cohort.getUuid())) { - errors.rejectValue("name", "general.error.nameAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(cohort, "name", cohort.getName(), errors); if (!cohort.getVoided()) { Collection members = cohort.getMemberships(); if (!CollectionUtils.isEmpty(members)) { diff --git a/api/src/main/java/org/openmrs/validator/ConceptDatatypeValidator.java b/api/src/main/java/org/openmrs/validator/ConceptDatatypeValidator.java index 97bbf44cfde7..4afaad93b60a 100644 --- a/api/src/main/java/org/openmrs/validator/ConceptDatatypeValidator.java +++ b/api/src/main/java/org/openmrs/validator/ConceptDatatypeValidator.java @@ -9,10 +9,8 @@ */ package org.openmrs.validator; -import org.apache.commons.lang3.StringUtils; import org.openmrs.ConceptDatatype; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.springframework.validation.Errors; import org.springframework.validation.ValidationUtils; import org.springframework.validation.Validator; @@ -53,14 +51,7 @@ public void validate(Object obj, Errors errors) { if (cd == null) { errors.rejectValue("conceptDatatype", "error.general"); } else { - ConceptDatatype conceptDatatype = (ConceptDatatype) obj; - if (StringUtils.isNotBlank(conceptDatatype.getName())) { - ConceptDatatype existingDatatype = Context.getConceptService().getConceptDatatypeByName(conceptDatatype.getName()); - if (existingDatatype != null && !existingDatatype.getUuid().equals(conceptDatatype.getUuid())) { - errors.rejectValue("name", "general.error.nameAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(cd, "name", cd.getName(), errors); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "name", "error.name"); ValidateUtil.validateFieldLengths(errors, obj.getClass(), "name", "hl7Abbreviation", "description", "retireReason"); diff --git a/api/src/main/java/org/openmrs/validator/ConceptSourceValidator.java b/api/src/main/java/org/openmrs/validator/ConceptSourceValidator.java index 03f54f97cb29..b4e53472e9cc 100644 --- a/api/src/main/java/org/openmrs/validator/ConceptSourceValidator.java +++ b/api/src/main/java/org/openmrs/validator/ConceptSourceValidator.java @@ -9,10 +9,8 @@ */ package org.openmrs.validator; -import org.apache.commons.lang3.StringUtils; import org.openmrs.ConceptSource; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.springframework.validation.Errors; import org.springframework.validation.ValidationUtils; import org.springframework.validation.Validator; @@ -54,13 +52,7 @@ public void validate(Object obj, Errors errors) throws IllegalArgumentException + ConceptSource.class); } else { ConceptSource conceptSource = (ConceptSource) obj; - if (StringUtils.isNotBlank(conceptSource.getName())) { - ConceptSource existingConceptSource = Context.getConceptService().getConceptSourceByName(conceptSource.getName()); - if (existingConceptSource != null && !existingConceptSource.getUuid().equals(conceptSource.getUuid())) { - errors.rejectValue("name", "general.error.nameAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(conceptSource, "name", conceptSource.getName(), errors); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "name", "error.name"); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "description", "error.null"); ValidateUtil.validateFieldLengths(errors, obj.getClass(), "name", "hl7Code", "uniqueId", "description", diff --git a/api/src/main/java/org/openmrs/validator/LocationTagValidator.java b/api/src/main/java/org/openmrs/validator/LocationTagValidator.java index 63acc59fb6ab..f5984bb9ba92 100644 --- a/api/src/main/java/org/openmrs/validator/LocationTagValidator.java +++ b/api/src/main/java/org/openmrs/validator/LocationTagValidator.java @@ -9,10 +9,8 @@ */ package org.openmrs.validator; -import org.apache.commons.lang3.StringUtils; import org.openmrs.LocationTag; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.springframework.validation.Errors; import org.springframework.validation.ValidationUtils; import org.springframework.validation.Validator; @@ -44,13 +42,7 @@ public boolean supports(Class c) { public void validate(Object target, Errors errors) { if (target != null) { LocationTag locationTag = (LocationTag) target; - if (StringUtils.isNotBlank(locationTag.getName())) { - LocationTag existingLocationTag = Context.getLocationService().getLocationTagByName(locationTag.getName()); - if (existingLocationTag != null && !existingLocationTag.getUuid().equals(locationTag.getUuid())) { - errors.rejectValue("name", "general.error.nameAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(locationTag, "name", locationTag.getName(), errors); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "name", "LocationTag.error.name.required"); ValidateUtil.validateFieldLengths(errors, target.getClass(), "name", "description", "retireReason"); } diff --git a/api/src/main/java/org/openmrs/validator/PrivilegeValidator.java b/api/src/main/java/org/openmrs/validator/PrivilegeValidator.java index 5334f5daa045..145f4e098c7b 100644 --- a/api/src/main/java/org/openmrs/validator/PrivilegeValidator.java +++ b/api/src/main/java/org/openmrs/validator/PrivilegeValidator.java @@ -9,10 +9,8 @@ */ package org.openmrs.validator; -import org.apache.commons.lang3.StringUtils; import org.openmrs.Privilege; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.springframework.validation.Errors; import org.springframework.validation.ValidationUtils; import org.springframework.validation.Validator; @@ -53,13 +51,7 @@ public void validate(Object obj, Errors errors) { if (privilege == null) { errors.rejectValue("privilege", "error.general"); } else { - if (StringUtils.isNotBlank(privilege.getPrivilege())) { - Privilege existingPrivilege = Context.getUserService().getPrivilege(privilege.getPrivilege()); - if (existingPrivilege != null && !existingPrivilege.getUuid().equals(privilege.getUuid())) { - errors.rejectValue("privilege", "error.privilege.privilegeAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(privilege, "privilege", privilege.getPrivilege(), errors); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "privilege", "error.privilege"); ValidateUtil.validateFieldLengths(errors, obj.getClass(), "privilege", "description"); } diff --git a/api/src/main/java/org/openmrs/validator/RoleValidator.java b/api/src/main/java/org/openmrs/validator/RoleValidator.java index 85ed1463e81f..cb3eecc1a748 100644 --- a/api/src/main/java/org/openmrs/validator/RoleValidator.java +++ b/api/src/main/java/org/openmrs/validator/RoleValidator.java @@ -9,10 +9,8 @@ */ package org.openmrs.validator; -import org.apache.commons.lang3.StringUtils; import org.openmrs.Role; import org.openmrs.annotation.Handler; -import org.openmrs.api.context.Context; import org.springframework.validation.Errors; import org.springframework.validation.ValidationUtils; import org.springframework.validation.Validator; @@ -55,13 +53,7 @@ public void validate(Object obj, Errors errors) { if (role == null) { errors.rejectValue("role", "error.general"); } else { - if (StringUtils.isNotBlank(role.getRole())) { - Role existingRole = Context.getUserService().getRole(role.getRole()); - if (existingRole != null && !existingRole.getUuid().equals(role.getUuid())) { - errors.rejectValue("role", "error.role.roleAlreadyInUse"); - return; - } - } + ValidateUtil.rejectDuplicateName(role, "role", role.getRole(), errors); ValidationUtils.rejectIfEmptyOrWhitespace(errors, "role", "error.role"); // reject any role that has a leading or trailing space diff --git a/api/src/main/java/org/openmrs/validator/ValidateUtil.java b/api/src/main/java/org/openmrs/validator/ValidateUtil.java index 3a7038c5c1e5..5e7bad52852b 100644 --- a/api/src/main/java/org/openmrs/validator/ValidateUtil.java +++ b/api/src/main/java/org/openmrs/validator/ValidateUtil.java @@ -174,4 +174,31 @@ public static void disableValidationForThread() { public static void resumeValidationForThread() { disableValidationForThread.remove(); } + + /** + * Tests if given field value is already in use by another object of the same class + * + * @param obj the object being tested + * @param field the field of the object to be tested + * @param value the field value of the object being tested + * @param errors + *
    + *
  • Should fail validation if name is already in use.
    • + *
  • Should return immediately if validation is disabled and have no errors.
    • + *
+ * + * @since 2.7.0 + */ + public static void rejectDuplicateName(OpenmrsObject obj, String field, String value, Errors errors) { + if (disableValidation) { + return; + } + + if (StringUtils.isNotBlank(value)) { + OpenmrsObject existingObject = Context.getAdministrationService().getObjectByFieldValue(obj.getClass(), field, value); + if (existingObject != null && !existingObject.getUuid().equals(obj.getUuid())) { + errors.rejectValue(field, "general.error.nameAlreadyInUse"); + } + } + } } diff --git a/api/src/main/resources/messages.properties b/api/src/main/resources/messages.properties index 3fee620e382b..7d5561d5118a 100644 --- a/api/src/main/resources/messages.properties +++ b/api/src/main/resources/messages.properties @@ -71,7 +71,6 @@ errors.patientId.cannotBeNull=Patient Id cannot be null error.foundValidationErrors=Found Validation Errors error.invalid=Invalid error.privilegesRequired=Privileges required: {0} -error.privilege.privilegeAlreadyInUse=Specified Privilege name already exists, please specify another error.extraPrivilegesRequired=Extra privileges required error.insufficientPrivileges=Insufficient Privileges error.aunthenticationRequired=Basic authentication required @@ -2037,7 +2036,6 @@ Role.manage.header=Role Management Role.title=Role Form Role.role=Role error.role=Role cannot be empty -error.role.roleAlreadyInUse=Specified Role name already exists, please specify another Role.save=Save Role Role.add=Add Role Role.inheritedPrivileges.description=Greyed out checkboxes represent privileges inherited from other roles, these cannot be removed individually. diff --git a/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java b/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java index 27c59f3f63ac..530e9495bcdb 100644 --- a/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java +++ b/api/src/test/java/org/openmrs/api/AdministrationServiceTest.java @@ -33,6 +33,7 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import org.openmrs.ConceptSource; import org.openmrs.GlobalProperty; import org.openmrs.ImplementationId; import org.openmrs.Privilege; @@ -1106,4 +1107,18 @@ private Cache.ValueWrapper getCacheForCurrentUser(){ private List getCachedSearchLocalesForCurrentUser() { return (List) getCacheForCurrentUser().get(); } + + /** + * @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String) + */ + @Test + public void getObjectByFieldValue_shouldReturnObjectOfGivenClassThatMatchesGivenFieldValue() { + List allSources = Context.getConceptService().getAllConceptSources(false); + assertNotNull(allSources); + assertEquals("SNOMED CT", allSources.get(1).getName()); + + ConceptSource conceptSource = adminService.getObjectByFieldValue(ConceptSource.class, "name", "snomed ct"); + assertNotNull(conceptSource); + assertEquals("SNOMED CT", conceptSource.getName()); + } } diff --git a/api/src/test/java/org/openmrs/api/UserServiceTest.java b/api/src/test/java/org/openmrs/api/UserServiceTest.java index df0993c4c884..0dffa29c38b4 100644 --- a/api/src/test/java/org/openmrs/api/UserServiceTest.java +++ b/api/src/test/java/org/openmrs/api/UserServiceTest.java @@ -1210,7 +1210,7 @@ public void saveRole_shouldThrowErrorWhenCurrentUserLacksPrivilegeAssignedToRole currentUser.addRole(adminRole); Privilege myPrivilege = new Privilege("custom privilege"); - + Context.addProxyPrivilege(PrivilegeConstants.GET_OPENMRS_OBJECTS); APIException exception = assertThrows(APIException.class, () -> withCurrentUserAs(currentUser, () -> { Role newRole = new Role("another role"); newRole.addPrivilege(myPrivilege); @@ -1230,7 +1230,7 @@ public void saveRole_shouldThrowErrorWhenCurrentUserLacksAPrivilegeAssignedToRol User currentUser = new User(); currentUser.addRole(adminRole); - + Context.addProxyPrivilege(PrivilegeConstants.GET_OPENMRS_OBJECTS); APIException exception = assertThrows(APIException.class, () -> withCurrentUserAs(currentUser, () -> { Role newRole = new Role("another role"); newRole.addPrivilege(myFirstPrivilege); diff --git a/api/src/test/java/org/openmrs/validator/ValidateUtilTest.java b/api/src/test/java/org/openmrs/validator/ValidateUtilTest.java index 08a87c8d245f..5ab8897bb967 100644 --- a/api/src/test/java/org/openmrs/validator/ValidateUtilTest.java +++ b/api/src/test/java/org/openmrs/validator/ValidateUtilTest.java @@ -9,6 +9,7 @@ */ package org.openmrs.validator; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -16,13 +17,18 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import java.util.List; + import org.junit.jupiter.api.Test; import org.openmrs.Concept; +import org.openmrs.ConceptSource; import org.openmrs.Drug; import org.openmrs.Location; +import org.openmrs.OpenmrsObject; import org.openmrs.Patient; import org.openmrs.PatientIdentifierType; import org.openmrs.api.ValidationException; +import org.openmrs.api.context.Context; import org.openmrs.test.jupiter.BaseContextSensitiveTest; import org.springframework.validation.BindException; import org.springframework.validation.Errors; @@ -208,4 +214,23 @@ public void validate_shouldReturnThrowExceptionAlongWithAppropriateMessageIfTheO ValidationException exception = assertThrows(ValidationException.class, () -> ValidateUtil.validate(drug)); assertTrue(exception.getMessage().contains("failed to validate with reason: name: This value exceeds the maximum length of 255 permitted for this field.")); } + + /** + * @see org.openmrs.validator.ValidateUtil#rejectDuplicateName(OpenmrsObject, String, String, Errors) + */ + @Test + public void rejectDuplicateName_shouldRejectNameIfItsAlreadyInUse() { + List allSources = Context.getConceptService().getAllConceptSources(false); + assertNotNull(allSources); + assertEquals("SNOMED CT", allSources.get(1).getName()); + + ConceptSource conceptSource = new ConceptSource(); + conceptSource.setName("snomed ct"); + conceptSource.setDescription("a concept source to add for testing"); + + BindException errors = new BindException(conceptSource, "conceptSource"); + assertFalse(errors.hasErrors()); + ValidateUtil.rejectDuplicateName(conceptSource, "name", conceptSource.getName(), errors); + assertTrue(errors.hasErrors()); + } } diff --git a/api/src/test/resources/org/openmrs/include/standardTestDataset.xml b/api/src/test/resources/org/openmrs/include/standardTestDataset.xml index aec298d8af7a..f518d79c7f38 100644 --- a/api/src/test/resources/org/openmrs/include/standardTestDataset.xml +++ b/api/src/test/resources/org/openmrs/include/standardTestDataset.xml @@ -129,8 +129,8 @@ - - + +