Skip to content

Commit

Permalink
TRUNK-4455: All metadata names should be case-insensitive.
Browse files Browse the repository at this point in the history
  • Loading branch information
IamMujuziMoses committed Mar 21, 2024
1 parent 37de9f1 commit af6d0c1
Show file tree
Hide file tree
Showing 18 changed files with 126 additions and 61 deletions.
17 changes: 17 additions & 0 deletions api/src/main/java/org/openmrs/api/AdministrationService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <ul>
* <li><strong>Should</strong> return an object of a given class that matches the exact name</li>
* </ul>
*
* @since 2.7.0
*/
@Authorized(PrivilegeConstants.GET_OPENMRS_OBJECTS)
public <T> T getObjectByFieldValue(Class<T> aClass, String field, String value) throws APIException;
}
5 changes: 5 additions & 0 deletions api/src/main/java/org/openmrs/api/db/AdministrationDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,9 @@ public interface AdministrationDAO {
* @see AdministrationService#updatePostgresSequence()
*/
public void updatePostgresSequence() throws DAOException;

/**
* @see AdministrationService#getObjectByFieldValue(Class, String, String)
*/
public <T> T getObjectByFieldValue(Class<T> aClass, String field, String value) throws DAOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -429,4 +429,19 @@ public void execute(Connection con) throws SQLException {
});
}
}

/**
* @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String)
*/
@Override
public <T> T getObjectByFieldValue(Class<T> aClass, String field, String value) throws DAOException {
Session session = sessionFactory.getCurrentSession();
CriteriaBuilder cb = session.getCriteriaBuilder();
CriteriaQuery<T> cq = cb.createQuery(aClass);
Root<T> root = cq.from(aClass);

cq.where(cb.equal(root.get(field), value));

return session.createQuery(cq).uniqueResult();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -942,4 +942,12 @@ public void updatePostgresSequence() {
dao.updatePostgresSequence();
}

/**
* @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String)
*/
@Override
@Transactional(readOnly = true)
public <T> T getObjectByFieldValue(Class<T> aClass, String field, String value) throws APIException {
return dao.getObjectByFieldValue(aClass, field, value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions api/src/main/java/org/openmrs/util/PrivilegeConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
9 changes: 1 addition & 8 deletions api/src/main/java/org/openmrs/validator/CohortValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<CohortMembership> members = cohort.getMemberships();
if (!CollectionUtils.isEmpty(members)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
Expand Down
10 changes: 1 addition & 9 deletions api/src/main/java/org/openmrs/validator/PrivilegeValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
Expand Down
10 changes: 1 addition & 9 deletions api/src/main/java/org/openmrs/validator/RoleValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions api/src/main/java/org/openmrs/validator/ValidateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <ul>
* <li><strong>Should</strong> fail validation if name is already in use.</li>
* <li><strong>Should</strong> return immediately if validation is disabled and have no errors.</li>
* </ul>
*
* @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");
}
}
}
}
2 changes: 0 additions & 2 deletions api/src/main/resources/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions api/src/test/java/org/openmrs/api/AdministrationServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1106,4 +1107,18 @@ private Cache.ValueWrapper getCacheForCurrentUser(){
private List<Locale> getCachedSearchLocalesForCurrentUser() {
return (List<Locale>) getCacheForCurrentUser().get();
}

/**
* @see org.openmrs.api.AdministrationService#getObjectByFieldValue(Class, String, String)
*/
@Test
public void getObjectByFieldValue_shouldReturnObjectOfGivenClassThatMatchesGivenFieldValue() {
List<ConceptSource> 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());
}
}
4 changes: 2 additions & 2 deletions api/src/test/java/org/openmrs/api/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions api/src/test/java/org/openmrs/validator/ValidateUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,26 @@
*/
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;
import static org.junit.jupiter.api.Assertions.assertThrows;
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;
Expand Down Expand Up @@ -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<ConceptSource> 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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@
<concept_reference_source concept_source_id="1" name="Some Standardized Terminology" description="A made up source" hl7_code="SSTRM" creator="1" date_created="2005-02-24 00:00:00.0" uuid="00001827-639f-4cb4-961f-1e025bf80000" retired="false"/>
<concept_reference_source concept_source_id="2" name="SNOMED CT" description="'Systematized Nomenclature of Medicine--Clinical Terms' is a comprehensive clinical terminology" hl7_code="SCT" unique_id="2.16.840.1.113883.6.96" creator="1" date_created="2005-02-24 00:00:00.0" uuid="j3nfjk33-639f-4cb4-961f-1e025b908433" retired="false"/>
<concept_reference_source concept_source_id="3" name="ICD-10" description="International Classification of Disease, version 10" hl7_code="I10" creator="1" date_created="2009-06-03 13:41:25.0" retired="false" uuid="75f5b378-5065-11de-80cb-001e378eb67e"/>
<concept_reference_source concept_source_id="4" name="ICD-10" description="A made up source which has been retired" hl7_code="I101" creator="1" date_created="2012-04-03 31:41:25.3" retired="true" uuid="63a5b378-5065-11de-80cb-001e378ea23e"/>
<concept_reference_source concept_source_id="5" name="ICD-10" description="A second made up source which has been retired" hl7_code="I102" creator="1" date_created="2012-04-03 13:41:25.9" retired="true" uuid="23a5b378-5065-11de-80cb-001e378ac77e"/>
<concept_reference_source concept_source_id="4" name="ICD-101" description="A made up source which has been retired" hl7_code="I101" creator="1" date_created="2012-04-03 31:41:25.3" retired="true" uuid="63a5b378-5065-11de-80cb-001e378ea23e"/>
<concept_reference_source concept_source_id="5" name="ICD-102" description="A second made up source which has been retired" hl7_code="I102" creator="1" date_created="2012-04-03 13:41:25.9" retired="true" uuid="23a5b378-5065-11de-80cb-001e378ac77e"/>
<concept_reference_term concept_reference_term_id="1" concept_source_id="1" code="WGT234" name="weight term" description="A person's weight in kilograms" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-WGT234"/>
<concept_reference_term concept_reference_term_id="2" concept_source_id="1" code="CD41003" name="cd4 term" description="A person's CD4" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-CD41003"/>
<concept_reference_term concept_reference_term_id="3" concept_source_id="2" code="2332523" name="weight term2" description="A person's weight in kilograms" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SNOMED CT-2332523"/>
Expand Down

0 comments on commit af6d0c1

Please sign in to comment.