Skip to content

Commit

Permalink
Made concept source required when searching drug(s) by mapping and re…
Browse files Browse the repository at this point in the history 
…moved includeRetired argument from ConceptService.getDrugByMapping - TRUNK-4214
  • Loading branch information
@wluyima
wluyima committed Mar 3, 2014
1 parent 1ee551c commit bdb9eda
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 49 deletions.
6 changes: 4 additions & 2 deletions api/src/main/java/org/openmrs/api/ConceptService.java
View file
Expand Up @@ -2083,6 +2083,7 @@ public Integer getCountOfConceptReferenceTerms(String query, ConceptSource conce
* @should match on the map types
* @should fail if no code and concept source and withAnyOfTheseTypes are provided
* @should exclude duplicate matches
* @should fail if source is null
*/
@Transactional(readOnly = true)
@Authorized(PrivilegeConstants.VIEW_CONCEPTS)
Expand All @@ -2094,10 +2095,10 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
* getDrugByMapping("12345", rxNorm, Arrays.asList(sameAs, narrowerThan)) If there are multiple
* matches for the highest-priority ConceptMapType, throw an exception
*
*
* @param code the code the reference term code to match on
* @param conceptSource the concept source to match on
* @param withAnyOfTheseTypesOrOrderOfPreference the ConceptMapTypes to match on
* @param includeRetired specifies if a retired drug should be returned as a match
* @since 1.10
* @return the {@link Drug}
* @throws APIException
Expand All @@ -2106,10 +2107,11 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
* @should fail if multiple drugs are found matching the best map type
* @should return null if no match found
* @should fail if no code and concept source and withAnyOfTheseTypes are provided
* @should fail if source is null
*/
@Transactional(readOnly = true)
@Authorized(PrivilegeConstants.VIEW_CONCEPTS)
public Drug getDrugByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference, boolean includeRetired) throws APIException;
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference) throws APIException;

}
5 changes: 2 additions & 3 deletions api/src/main/java/org/openmrs/api/db/ConceptDAO.java
View file
Expand Up @@ -672,9 +672,8 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypes, boolean includeRetired) throws DAOException;

/**
* @see org.openmrs.api.ConceptService#getDrugByMapping(String, ConceptSource, Collection,
* boolean)
* @see org.openmrs.api.ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
Drug getDrugByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference, boolean includeRetired) throws DAOException;
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference) throws DAOException;
}
6 changes: 3 additions & 3 deletions api/src/main/java/org/openmrs/api/db/hibernate/HibernateConceptDAO.java
View file
Expand Up @@ -2101,8 +2101,8 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
*/
@Override
public Drug getDrugByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference, boolean includeRetired) throws DAOException {
Criteria criteria = createSearchDrugByMappingCriteria(code, conceptSource, includeRetired);
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference) throws DAOException {
Criteria criteria = createSearchDrugByMappingCriteria(code, conceptSource, true);

// match with any of the supplied collection or order of preference of conceptMapTypes
if (withAnyOfTheseTypesOrOrderOfPreference.size() > 0) {
Expand All @@ -2115,7 +2115,7 @@ public Drug getDrugByMapping(String code, ConceptSource conceptSource,
return drugs.get(0);
}
//reset for the next execution to avoid unwanted AND clauses on every found map type
criteria = createSearchDrugByMappingCriteria(code, conceptSource, includeRetired);
criteria = createSearchDrugByMappingCriteria(code, conceptSource, true);
}
} else {
List<Drug> drugs = criteria.list();
Expand Down
16 changes: 8 additions & 8 deletions api/src/main/java/org/openmrs/api/impl/ConceptServiceImpl.java
View file
Expand Up @@ -2204,8 +2204,9 @@ public List<Drug> getDrugs(String searchPhrase, Locale locale, boolean exactLoca
@Override
public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypes, boolean includeRetired) throws APIException {
if (code == null && conceptSource == null && CollectionUtils.isEmpty(withAnyOfTheseTypes)) {
throw new APIException("Please provide a code or concept source or map types");

if (conceptSource == null) {
throw new APIException("ConceptSource is required");
}
if (withAnyOfTheseTypes == null) {
withAnyOfTheseTypes = Collections.EMPTY_LIST;
Expand All @@ -2214,18 +2215,17 @@ public List<Drug> getDrugsByMapping(String code, ConceptSource conceptSource,
}

/**
* @see org.openmrs.api.ConceptService#getDrugByMapping(String, ConceptSource, Collection,
* boolean)
* @see org.openmrs.api.ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Override
public Drug getDrugByMapping(String code, ConceptSource conceptSource,
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference, boolean includeRetired) throws APIException {
if (code == null && conceptSource == null && CollectionUtils.isEmpty(withAnyOfTheseTypesOrOrderOfPreference)) {
throw new APIException("Please provide a code or concept source or map types");
Collection<ConceptMapType> withAnyOfTheseTypesOrOrderOfPreference) throws APIException {
if (conceptSource == null) {
throw new APIException("ConceptSource is required");
}
if (withAnyOfTheseTypesOrOrderOfPreference == null) {
withAnyOfTheseTypesOrOrderOfPreference = Collections.EMPTY_LIST;
}
return dao.getDrugByMapping(code, conceptSource, withAnyOfTheseTypesOrOrderOfPreference, includeRetired);
return dao.getDrugByMapping(code, conceptSource, withAnyOfTheseTypesOrOrderOfPreference);
}
}
92 changes: 59 additions & 33 deletions api/src/test/java/org/openmrs/api/ConceptServiceTest.java
View file
Expand Up @@ -40,7 +40,9 @@
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.openmrs.Concept;
import org.openmrs.ConceptAnswer;
import org.openmrs.ConceptClass;
Expand Down Expand Up @@ -74,7 +76,6 @@
import org.openmrs.util.ConceptMapTypeComparator;
import org.openmrs.util.LocaleUtility;
import org.openmrs.util.OpenmrsConstants;
import org.springframework.test.annotation.ExpectedException;

/**
* This test class (should) contain tests for all of the ConcepService methods TODO clean up and
Expand All @@ -92,6 +93,9 @@ public class ConceptServiceTest extends BaseContextSensitiveTest {

protected static final String GET_DRUG_MAPPINGS = "org/openmrs/api/include/ConceptServiceTest-getDrugMappings.xml";

@Rule
public ExpectedException expectedException = ExpectedException.none();

/**
* Run this before each unit test in this class. The "@Before" method in
* {@link BaseContextSensitiveTest} is run right before this method.
Expand Down Expand Up @@ -1449,9 +1453,8 @@ public void saveConceptStopWord_shouldSaveReturnConceptStopWordWithId() throws E
/**
* @see {@link ConceptService#saveConceptStopWord(ConceptStopWord)}
*/
@Test
@Test(expected = ConceptStopWordException.class)
@Verifies(value = "should fail if a duplicate conceptStopWord in a locale is added", method = "saveConceptStopWord(ConceptStopWord)")
@ExpectedException(ConceptStopWordException.class)
public void saveConceptStopWord_shouldFailIfADuplicateConceptStopWordInALocaleIsAdded() throws Exception {
ConceptStopWord conceptStopWord = new ConceptStopWord("A");
try {
Expand Down Expand Up @@ -2727,7 +2730,8 @@ public void getDrugsByMapping_shouldExcludeDuplicateMatches() throws Exception {
List<ConceptMapType> conceptMapTypeList = conceptService.getConceptMapTypes(false, true);
//the expected matching drug has two mappings to different concept sources but same code
//so this test also ensure that we can never get back duplicates
List<Drug> drugs = conceptService.getDrugsByMapping("WGT234", null, conceptMapTypeList, false);
ConceptSource source = conceptService.getConceptSource(1);
List<Drug> drugs = conceptService.getDrugsByMapping("WGT234", source, conceptMapTypeList, false);
assertEquals(1, drugs.size());
assertTrue(containsId(drugs, 2));
}
Expand Down Expand Up @@ -2769,7 +2773,8 @@ public void getDrugsByMapping_shouldReturnEmptyListIfNoMatchesAreFound() throws
@Test
public void getDrugsByMapping_shouldMatchOnTheCode() throws Exception {
executeDataSet(GET_DRUG_MAPPINGS);
List<Drug> drugs = conceptService.getDrugsByMapping("WGT234", null, null, false);
ConceptSource source = conceptService.getConceptSource(1);
List<Drug> drugs = conceptService.getDrugsByMapping("WGT234", source, null, false);
assertEquals(1, drugs.size());
assertTrue(containsId(drugs, 2));
}
Expand All @@ -2794,22 +2799,44 @@ public void getDrugsByMapping_shouldMatchOnTheConceptSource() throws Exception {
public void getDrugsByMapping_shouldMatchOnTheMapTypes() throws Exception {
executeDataSet(GET_DRUG_MAPPINGS);
List<ConceptMapType> conceptMapTypeList = conceptService.getConceptMapTypes(false, true);
List<Drug> drugs = conceptService.getDrugsByMapping(null, null, conceptMapTypeList, false);
ConceptSource source = conceptService.getConceptSource(1);
List<Drug> drugs = conceptService.getDrugsByMapping(null, source, conceptMapTypeList, false);
assertEquals(2, drugs.size());
assertTrue(containsId(drugs, 2));
assertTrue(containsId(drugs, 3));

drugs = conceptService.getDrugsByMapping(null, null, conceptMapTypeList, true);
drugs = conceptService.getDrugsByMapping(null, source, conceptMapTypeList, true);
assertEquals(3, drugs.size());
assertTrue(containsId(drugs, 2));
assertTrue(containsId(drugs, 3));
assertTrue(containsId(drugs, 11));
}

/**
* @verifies fail if no code and concept source and withAnyOfTheseTypes are provided
* @see ConceptService#getDrugsByMapping(String, org.openmrs.ConceptSource,
* java.util.Collection, boolean)
*/
@Test(expected = APIException.class)
public void getDrugsByMapping_shouldFailIfNoCodeAndConceptSourceAndWithAnyOfTheseTypesAreProvided() throws Exception {
conceptService.getDrugByMapping(null, null, null);
}

/**
* @verifies fail if source is null
* @see ConceptService#getDrugsByMapping(String, org.openmrs.ConceptSource,
* java.util.Collection, boolean)
*/
@Test
public void getDrugsByMapping_shouldFailIfSourceIsNull() throws Exception {
expectedException.expect(APIException.class);
expectedException.expectMessage("ConceptSource is required");
conceptService.getDrugsByMapping("random", null, null, false);
}

/**
* @verifies return a drug that matches the code and source and the best map type
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection,
* boolean)
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test
public void getDrugByMapping_shouldReturnADrugThatMatchesTheCodeAndSourceAndTheBestMapType() throws Exception {
Expand All @@ -2821,74 +2848,73 @@ public void getDrugByMapping_shouldReturnADrugThatMatchesTheCodeAndSourceAndTheB
List<ConceptMapType> conceptMapTypeList = new ArrayList<ConceptMapType>();
conceptMapTypeList.add(mapTypeWithMatch);
conceptMapTypeList.add(mapTypeWithNoMatch);
Drug drug = conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList, false);
Drug drug = conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList);
assertEquals(expectedDrugId, drug.getDrugId());

//Lets switch the order is the map types in the list to make sure that
//if there is no match on the first map type, the logic matches on the second
//sanity check that actually there will be no match on the first map type in the list
conceptMapTypeList.clear();
conceptMapTypeList.add(mapTypeWithNoMatch);
assertNull(conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList, false));
assertNull(conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList));

conceptMapTypeList.add(mapTypeWithMatch);
drug = conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList, false);
drug = conceptService.getDrugByMapping("WGT234", source, conceptMapTypeList);
assertEquals(expectedDrugId, drug.getDrugId());
}

/**
* @verifies fail if multiple drugs are found matching the best map type
* @see ConceptService#getDrugByMapping(String, ConceptSource, Collection, boolean)
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test(expected = DAOException.class)
public void getDrugByMapping_shouldFailIfMultipleDrugsAreFoundMatchingTheBestMapType() throws Exception {
executeDataSet(GET_DRUG_MAPPINGS);
conceptService.getDrugByMapping("CD41003", null, Collections.singleton(conceptService.getConceptMapType(2)), false);
ConceptSource source = conceptService.getConceptSource(1);
conceptService.getDrugByMapping("CD41003", source, Collections.singleton(conceptService.getConceptMapType(2)));
}

/**
* @verifies return null if no match found
* @see ConceptService#getDrugByMapping(String, ConceptSource, Collection, boolean)
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test
public void getDrugByMapping_shouldReturnNullIfNoMatchFound() throws Exception {
executeDataSet(GET_DRUG_MAPPINGS);
List<ConceptMapType> conceptMapTypeList = conceptService.getConceptMapTypes(false, true);
Drug drug = conceptService.getDrugByMapping("random code", conceptService.getConceptSource(1), conceptMapTypeList,
false);
Drug drug = conceptService.getDrugByMapping("random code", conceptService.getConceptSource(1), conceptMapTypeList);
assertNull(drug);
}

/**
* @verifies fail if no code and concept source and withAnyOfTheseTypes are provided
* @see ConceptService#getDrugsByMapping(String, org.openmrs.ConceptSource,
* java.util.Collection, boolean)
*/
@Test(expected = APIException.class)
public void getDrugsByMapping_shouldFailIfNoCodeAndConceptSourceAndWithAnyOfTheseTypesAreProvided() throws Exception {
conceptService.getDrugByMapping(null, null, null, false);
}

/**
* @verifies fail if no code and concept source and withAnyOfTheseTypes are provided
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection,
* boolean)
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test(expected = APIException.class)
public void getDrugByMapping_shouldFailIfNoCodeAndConceptSourceAndWithAnyOfTheseTypesAreProvided() throws Exception {
conceptService.getDrugByMapping(null, null, Collections.EMPTY_LIST, false);
conceptService.getDrugByMapping(null, null, Collections.EMPTY_LIST);
}

/**
* @verifies return a drug that matches the code and source
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection,
* boolean)
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test
public void getDrugByMapping_shouldReturnADrugThatMatchesTheCodeAndSource() throws Exception {
executeDataSet(GET_DRUG_MAPPINGS);
final Integer expectedDrugId = 2;
Drug drug = conceptService.getDrugByMapping("WGT234", conceptService.getConceptSource(2), null, false);
Drug drug = conceptService.getDrugByMapping("WGT234", conceptService.getConceptSource(2), null);
assertEquals(expectedDrugId, drug.getDrugId());
}

/**
* @verifies fail if source is null
* @see ConceptService#getDrugByMapping(String, org.openmrs.ConceptSource, java.util.Collection
*/
@Test
public void getDrugByMapping_shouldFailIfSourceIsNull() throws Exception {
expectedException.expect(APIException.class);
expectedException.expectMessage("ConceptSource is required");
conceptService.getDrugByMapping("random", null, null);
}
}

0 comments on commit bdb9eda

Please sign in to comment.