TRUNK-6218: Concept Validations should be more lenient with retired c…

…oncepts (#4555)

api/src/main/java/org/openmrs/api/ConceptService.java

@@ -1596,7 +1596,22 @@ public List<Drug> getDrugs(String drugName, Concept concept, boolean searchKeywo
 	 */
 	@Authorized(PrivilegeConstants.GET_CONCEPT_REFERENCE_TERMS)
 	public ConceptReferenceTerm getConceptReferenceTermByCode(String code, ConceptSource conceptSource) throws APIException;
-
+
+	/**
+	 * Gets a list of concept reference terms with the specified code from the specified concept source
+	 *
+	 * @param code the code to match against
+	 * @param conceptSource the concept source to match against
+	 * @param includeRetired specifies if retired concept reference terms should be included
+	 * @return concept reference term object
+	 * @since 2.7
+	 * @throws APIException
+	 * <strong>Should</strong> return a list of concept reference terms that matches the given code from the given source
+	 */
+	@Authorized(PrivilegeConstants.GET_CONCEPT_REFERENCE_TERMS)
+	public List<ConceptReferenceTerm> getConceptReferenceTermByCode(String code, ConceptSource conceptSource, boolean includeRetired) throws APIException;
+
+
 	/**
 	 * Stores the specified concept reference term to the database
 	 * 

api/src/main/java/org/openmrs/api/db/ConceptDAO.java

@@ -520,7 +520,12 @@ public List<Drug> getDrugs(String drugName, Concept concept, boolean searchOnPhr
 	 * @see ConceptService#getConceptReferenceTermByCode(String, ConceptSource)
 	 */
 	public ConceptReferenceTerm getConceptReferenceTermByCode(String code, ConceptSource conceptSource) throws DAOException;
-
+
+	/**
+	 * @see ConceptService#getConceptReferenceTermByCode(String, ConceptSource, boolean)
+	 */
+	public List<ConceptReferenceTerm> getConceptReferenceTermByCode(String code, ConceptSource conceptSource, boolean includeRetired) throws DAOException;
+
 	/**
 	 * @see ConceptService#saveConceptReferenceTerm(ConceptReferenceTerm)
 	 */

api/src/main/java/org/openmrs/api/db/hibernate/HibernateConceptDAO.java

@@ -9,6 +9,8 @@
  */
 package org.openmrs.api.db.hibernate;
 
+import static java.util.stream.Collectors.toList;
+
 import javax.persistence.Query;
 import javax.persistence.TypedQuery;
 import javax.persistence.criteria.CriteriaBuilder;
@@ -1117,7 +1119,7 @@ public List<Concept> getConceptsByMapping(String code, String sourceName, boolea
 		}
 
 		return session.createQuery(cq).getResultList()
-			.stream().distinct().collect(Collectors.toList());
+			.stream().distinct().collect(toList());
 	}
 
 	/**
@@ -1142,7 +1144,7 @@ public List<Integer> getConceptIdsByMapping(String code, String sourceName, bool
 		}
 
 		return session.createQuery(cq).getResultList()
-			.stream().distinct().collect(Collectors.toList());
+			.stream().distinct().collect(toList());
 	}
 
 	/**
@@ -1741,32 +1743,55 @@ public ConceptReferenceTerm getConceptReferenceTermByName(String name, ConceptSo
 		}
 		return terms.get(0);
 	}
-
+	
 	/**
 	 * @see org.openmrs.api.db.ConceptDAO#getConceptReferenceTermByCode(java.lang.String,
 	 *      org.openmrs.ConceptSource)
 	 */
 	@Override
 	public ConceptReferenceTerm getConceptReferenceTermByCode(String code, ConceptSource conceptSource) throws DAOException {
+		List<ConceptReferenceTerm> conceptReferenceTerms = getConceptReferenceTermByCode(code, conceptSource, true);
+
+		if (conceptReferenceTerms.isEmpty()) {
+			return null;
+		} else if (conceptReferenceTerms.size() > 1) {
+			List<ConceptReferenceTerm> unretiredConceptReferenceTerms = conceptReferenceTerms.stream()
+			        .filter(term -> !term.getRetired())
+					.collect(toList());
+			if (unretiredConceptReferenceTerms.size() == 1) {
+				return unretiredConceptReferenceTerms.get(0);
+			}
+
+			// either more than one unretired concept term or more than one retired concept term
+			throw new APIException("ConceptReferenceTerm.foundMultipleTermsWithCodeInSource",
+			        new Object[] { code, conceptSource.getName() });
+		}
+
+		return conceptReferenceTerms.get(0);
+	}
+
+	/**
+	 * @see org.openmrs.api.db.ConceptDAO#getConceptReferenceTermByCode(java.lang.String,
+	 *      org.openmrs.ConceptSource, boolean)
+	 */
+	@Override
+	public List<ConceptReferenceTerm> getConceptReferenceTermByCode(String code, ConceptSource conceptSource,
+	        boolean includeRetired) throws DAOException {
 		Session session = sessionFactory.getCurrentSession();
 		CriteriaBuilder cb = session.getCriteriaBuilder();
 		CriteriaQuery<ConceptReferenceTerm> cq = cb.createQuery(ConceptReferenceTerm.class);
 		Root<ConceptReferenceTerm> root = cq.from(ConceptReferenceTerm.class);
 
-		Predicate codePredicate = cb.equal(root.get("code"), code);
-		Predicate sourcePredicate = cb.equal(root.get("conceptSource"), conceptSource);
-
-		cq.where(cb.and(codePredicate, sourcePredicate));
-
-		List<ConceptReferenceTerm> terms = session.createQuery(cq).getResultList();
-
-		if (terms.isEmpty()) {
-			return null;
-		} else if (terms.size() > 1) {
-			throw new APIException("ConceptReferenceTerm.foundMultipleTermsWithCodeInSource",
-				new Object[] { code, conceptSource.getName() });
+		List<Predicate> predicates = new ArrayList<>();
+		predicates.add(cb.equal(root.get("code"), code));
+		predicates.add(cb.equal(root.get("conceptSource"), conceptSource));
+
+		if (!includeRetired) {
+			predicates.add(cb.isFalse(root.get("retired")));
 		}
-		return terms.get(0);
+		cq.where(predicates.toArray(new Predicate[]{}));
+
+		return session.createQuery(cq).getResultList();
 	}
 
 	/**
@@ -2121,7 +2146,7 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
 
 		cq.where(basePredicates.toArray(new Predicate[]{}));
 
-		return session.createQuery(cq).getResultList().stream().distinct().collect(Collectors.toList());
+		return session.createQuery(cq).getResultList().stream().distinct().collect(toList());
 	}
 
 	/**

api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java

@@ -1785,7 +1785,13 @@ public ConceptReferenceTerm getConceptReferenceTermByName(String name, ConceptSo
 	public ConceptReferenceTerm getConceptReferenceTermByCode(String code, ConceptSource conceptSource) throws APIException {
 		return dao.getConceptReferenceTermByCode(code, conceptSource);
 	}
-
+
+	@Override
+	@Transactional(readOnly = true)
+	public List<ConceptReferenceTerm> getConceptReferenceTermByCode(String code, ConceptSource conceptSource, boolean includeRetired) throws APIException {
+		return dao.getConceptReferenceTermByCode(code, conceptSource, includeRetired);
+	}
+
 	/**
 	 * @see org.openmrs.api.ConceptService#saveConceptReferenceTerm(org.openmrs.ConceptReferenceTerm)
 	 */

api/src/main/java/org/openmrs/validator/ConceptReferenceTermValidator.java

@@ -62,6 +62,7 @@ public boolean supports(Class<?> c) {
 	 * <strong>Should</strong> fail if a term is mapped multiple times to the same term
 	 * <strong>Should</strong> pass validation if field lengths are correct
 	 * <strong>Should</strong> fail validation if field lengths are not correct
+	 * <strong>Should</strong> pass validation if the duplicate concept reference term is retired
 	 */
 	@Override
 	public void validate(Object obj, Errors errors) throws APIException {
@@ -93,7 +94,7 @@ public void validate(Object obj, Errors errors) throws APIException {
 		//Ensure that there are no terms with the same code in the same source
 		ConceptReferenceTerm termWithDuplicateCode = Context.getConceptService().getConceptReferenceTermByCode(code,
 		    conceptReferenceTerm.getConceptSource());
-		if (termWithDuplicateCode != null
+		if (termWithDuplicateCode != null && !termWithDuplicateCode.getRetired()
 		        && !OpenmrsUtil.nullSafeEquals(termWithDuplicateCode.getUuid(), conceptReferenceTerm.getUuid())) {
 			errors.rejectValue("code", "ConceptReferenceTerm.duplicate.code",
 			    "Duplicate concept reference term code in its concept source: " + code);

api/src/main/java/org/openmrs/validator/ConceptValidator.java

@@ -87,6 +87,8 @@ public boolean supports(Class<?> c) {
 	 * <strong>Should</strong> pass if different concepts have the same short name
 	 * <strong>Should</strong> fail if the concept datatype is null
 	 * <strong>Should</strong> fail if the concept class is null
+	 * <strong>Should</strong> pass if the concept is retired and the only validation failures would be in ConceptName 
+	 * or ConceptMap, as a retired Concept bypasses ConceptName and ConceptMap validation.
 	 */
 	@Override
 	public void validate(Object obj, Errors errors) throws APIException, DuplicateConceptNameException {
@@ -104,9 +106,13 @@ public void validate(Object obj, Errors errors) throws APIException, DuplicateCo
 
 		ValidationUtils.rejectIfEmpty(errors, "datatype", "Concept.datatype.empty");
 		ValidationUtils.rejectIfEmpty(errors, "conceptClass", "Concept.conceptClass.empty");
-
+		
 		boolean hasFullySpecifiedName = false;
 		for (Locale conceptNameLocale : conceptToValidate.getAllConceptNameLocales()) {
+			if (conceptToValidate.getRetired()) {
+				continue;
+			}
+
 			boolean fullySpecifiedNameForLocaleFound = false;
 			boolean preferredNameForLocaleFound = false;
 			boolean shortNameForLocaleFound = false;
@@ -199,12 +205,12 @@ else if (nameInLocale.getLocalePreferred() && preferredNameForLocaleFound) {
 		}
 
 		//Ensure that each concept has at least a fully specified name
-		if (!hasFullySpecifiedName) {
+		if (!hasFullySpecifiedName && !conceptToValidate.getRetired()) {
 			log.debug("Concept has no fully specified name");
 			errors.reject("Concept.error.no.FullySpecifiedName");
 		}
 
-		if (CollectionUtils.isNotEmpty(conceptToValidate.getConceptMappings())) {
+		if (CollectionUtils.isNotEmpty(conceptToValidate.getConceptMappings()) && !conceptToValidate.getRetired()) {
 			//validate all the concept maps
 			int index = 0;
 			Set<Integer> mappedTermIds = null;

api/src/test/java/org/openmrs/api/ConceptServiceTest.java

@@ -42,6 +42,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import net.sf.ehcache.Ehcache;
 import org.apache.commons.collections.CollectionUtils;
@@ -2173,6 +2174,49 @@ public void getConceptReferenceTermByCode_shouldReturnAConceptReferenceTermThatM
 		assertNotNull(term);
 		assertEquals("2332523", term.getCode());
 	}
+
+	/**
+	 * @see ConceptService#getConceptReferenceTermByCode(String,ConceptSource)
+	 */
+	@Test
+	public void getConceptReferenceTermByCode_shouldReturnUnretiredTermIfRetiredAlsoExists()
+	{
+		ConceptReferenceTerm term = Context.getConceptService().getConceptReferenceTermByCode("898989",
+			new ConceptSource(1));
+		assertNotNull(term);
+		assertEquals("898989", term.getCode());
+		assertFalse(term.getRetired());
+	}
+
+	/**
+	 * @see ConceptService#getConceptReferenceTermByCode(String,ConceptSource, boolean)
+	 */
+	@Test
+	public void getConceptReferenceTermByCode_shouldReturnBothRetiredAndUnretiredTerms() {
+		List<ConceptReferenceTerm> terms = Context.getConceptService().getConceptReferenceTermByCode(
+			"898989", new ConceptSource(1), true);
+
+		assertEquals(2, terms.size());
+
+		List<Boolean> termStates = terms.stream().map(ConceptReferenceTerm::getRetired)
+			.sorted().collect(Collectors.toList());
+
+        List<Boolean> expectedStates = Arrays.asList(false, true);
+
+		assertEquals(expectedStates, termStates);
+	}
+
+	/**
+	 * @see ConceptService#getConceptReferenceTermByCode(String,ConceptSource, boolean)
+	 */
+	@Test
+	public void getConceptReferenceTermByCode_shouldExcludeRetiredConcepts() {
+		List<ConceptReferenceTerm> terms = Context.getConceptService().getConceptReferenceTermByCode(
+			"898989", new ConceptSource(1), false);
+
+		assertEquals(1, terms.size());
+		assertFalse(terms.get(0).getRetired());
+	}
 
 	/**
 	 * @see ConceptService#getConceptMapTypes(null,null)
@@ -2332,7 +2376,7 @@ public void retireConceptMapType_shouldShouldSetTheDefaultRetireReasonIfNoneIsGi
 	@Test
 	public void getConceptReferenceTerms_shouldReturnAllTheConceptReferenceTermsIfIncludeRetiredIsSetToTrue()
 	{
-		assertEquals(11, Context.getConceptService().getConceptReferenceTerms(true).size());
+		assertEquals(13, Context.getConceptService().getConceptReferenceTerms(true).size());
 	}
 
 	/**
@@ -2341,7 +2385,7 @@ public void getConceptReferenceTerms_shouldReturnAllTheConceptReferenceTermsIfIn
 	@Test
 	public void getConceptReferenceTerms_shouldReturnOnlyUnRetiredConceptReferenceTermsIfIncludeRetiredIsSetToFalse()
 	{
-		assertEquals(10, Context.getConceptService().getConceptReferenceTerms(false).size());
+		assertEquals(11, Context.getConceptService().getConceptReferenceTerms(false).size());
 	}
 
 	/**
@@ -2359,7 +2403,7 @@ public void getConceptReferenceTermByUuid_shouldReturnTheConceptReferenceTermTha
 	@Test
 	public void getConceptReferenceTerms_shouldReturnOnlyTheConceptReferenceTermsFromTheGivenConceptSource()
 	{
-		assertEquals(9, conceptService.getConceptReferenceTerms(null, conceptService.getConceptSource(1), 0, null,
+		assertEquals(11, conceptService.getConceptReferenceTerms(null, conceptService.getConceptSource(1), 0, null,
 		    true).size());
 	}
 
@@ -2475,7 +2519,7 @@ public void unretireConceptReferenceTerm_shouldUnretireTheSpecifiedConceptRefere
 	 */
 	@Test
 	public void getAllConceptReferenceTerms_shouldReturnAllConceptReferenceTermsInTheDatabase() {
-		assertEquals(11, Context.getConceptService().getAllConceptReferenceTerms().size());
+		assertEquals(13, Context.getConceptService().getAllConceptReferenceTerms().size());
 	}
 
 	/**
@@ -2591,15 +2635,15 @@ public void getConceptsByClass_shouldReturnAnEmptyListIfNoneWasFound() {
 	 */
 	@Test
 	public void getCountOfConceptReferenceTerms_shouldIncludeRetiredTermsIfIncludeRetiredIsSetToTrue() {
-		assertEquals(11, conceptService.getCountOfConceptReferenceTerms("", null, true).intValue());
+		assertEquals(13, conceptService.getCountOfConceptReferenceTerms("", null, true).intValue());
 	}
 
 	/**
 	 * @see ConceptService#getCountOfConceptReferenceTerms(String,ConceptSource,null)
 	 */
 	@Test
 	public void getCountOfConceptReferenceTerms_shouldNotIncludeRetiredTermsIfIncludeRetiredIsSetToFalse() {
-		assertEquals(10, conceptService.getCountOfConceptReferenceTerms("", null, false).intValue());
+		assertEquals(11, conceptService.getCountOfConceptReferenceTerms("", null, false).intValue());
 	}
 
 	/**

api/src/test/java/org/openmrs/validator/ConceptReferenceTermValidatorTest.java

@@ -310,4 +310,24 @@ public void validate_shouldFailValidationIfFieldLengthsAreNotCorrect() {
 		assertTrue(errors.hasFieldErrors("description"));
 		assertTrue(errors.hasFieldErrors("retireReason"));
 	}
+
+	@Test
+	public void validate_shouldPassIfTheConceptReferenceTermCodeIsDuplicateButRetired() {
+		ConceptReferenceTerm term = new ConceptReferenceTerm();
+		term.setName("name");
+		term.setCode("WGT234");
+		term.setConceptSource(Context.getConceptService().getConceptSource(1));
+		Errors errors = new BindException(term, "term");
+
+		ConceptReferenceTerm termWithDuplicateCode = Context.getConceptService()
+			.getConceptReferenceTermByCode(term.getCode(), term.getConceptSource());
+
+		if (termWithDuplicateCode != null) {
+			termWithDuplicateCode.setRetired(true);
+		}
+
+		new ConceptReferenceTermValidator().validate(term, errors);
+
+		assertFalse(errors.hasFieldErrors("code"));
+	}
 }

api/src/test/java/org/openmrs/validator/ConceptValidatorTest.java

@@ -32,6 +32,7 @@
 import org.openmrs.ConceptDatatype;
 import org.openmrs.ConceptDescription;
 import org.openmrs.ConceptMap;
+import org.openmrs.ConceptMapType;
 import org.openmrs.ConceptName;
 import org.openmrs.ConceptReferenceTerm;
 import org.openmrs.api.ConceptNameType;
@@ -534,4 +535,29 @@ public void validate_shouldNotFailIfBlankConceptDescriptionIsPassed() {
 
 		assertThat(errors, not(hasFieldErrors("description")));
 	}
+
+	@Test
+	public void validate_shouldSkipConceptNameValidationForRetiredConcepts() {
+		concept.setRetired(true);
+		concept.addName(new ConceptName("", Context.getLocale()));
+		concept.setDatatype(new ConceptDatatype());
+		concept.setConceptClass(new ConceptClass());
+
+		validator.validate(concept, errors);
+
+		assertFalse(errors.hasErrors(), "Expected no validation errors for a retired concept");
+	}
+
+	@Test
+	public void validate_shouldSkipConceptMapValidationForRetiredConcepts() {
+		concept.setRetired(true);
+		concept.addName(new ConceptName("some name", Context.getLocale()));
+		concept.setConceptClass(new ConceptClass());
+		concept.setDatatype(new ConceptDatatype());
+		concept.addConceptMapping(new ConceptMap(null, new ConceptMapType()));
+
+		validator.validate(concept, errors);
+
+		assertFalse(errors.hasErrors(), "Expected no validation errors for a retired concept");
+	}
 }

api/src/test/resources/org/openmrs/include/standardTestDataset.xml

@@ -142,6 +142,8 @@
   <concept_reference_term concept_reference_term_id="9" concept_source_id="1" code="127cd4689" name="cd4died term2" description="died" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-127689_3"/>
   <concept_reference_term concept_reference_term_id="10" concept_source_id="1" code="454545" name="no term name3" description="" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-454545"/>
   <concept_reference_term concept_reference_term_id="11" concept_source_id="1" code="retired code" name="retired name" description="test duplicate term that is retired" retired="1" retired_by="1" retire_reason="test reason" date_retired="2004-08-12 12:00:00" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-retired code"/>
+  <concept_reference_term concept_reference_term_id="12" concept_source_id="1" code="898989" name="sample non retired" description="sample retired term" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-898989_1"/>
+  <concept_reference_term concept_reference_term_id="13" concept_source_id="1" code="898989" name="sample retired" description="sample non retired term" retired="1" creator="1" date_created="2004-08-12 00:00:00.0" uuid="SSTRM-898989_2"/>
   <concept_map_type concept_map_type_id="1" name="is a" is_hidden="0" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="1ce7a784-7d8f-11e1-909d-c80aa9edcf4e"/>
   <concept_map_type concept_map_type_id="2" name="same-as" description="Indicates similarity" is_hidden="0" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="35543629-7d8c-11e1-909d-c80aa9edcf4e"/>
   <concept_map_type concept_map_type_id="3" name="broader-than" is_hidden="0" retired="0" creator="1" date_created="2004-08-12 00:00:00.0" uuid="4b9d9421-7d8c-11e1-909d-c80aa9edcf4e"/>