Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TRUNK-4455: All metadata names should be case-insensitive. #4579

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
TRUNK-4455: All metadata names should be case-insensitive.
  • Loading branch information
IamMujuziMoses committed Mar 21, 2024
commit 9f8851cab2aecde03f6b3243c9496571af59c524
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,13 @@ 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
Loading