Skip to content

Commit

Permalink
Search for patient by identifier fails when identifier is made of only
Browse files Browse the repository at this point in the history 
letters - TRUNK-3822

Follow up - TRUNK-3822

Follow up to fix some issues - TRUNK-3822

Fixed patientService.getPatientCount method - TRUNK-3822

Follow up to fix failing unit test - TRUNK-3822

Apply auto format changes and fixed HibernateEncounterDAO - TRUNK-3822
  • Loading branch information
@wluyima
wluyima authored and wluyima committed May 15, 2013
1 parent a9d96c5 commit abe78c5
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 61 deletions.
2 changes: 2 additions & 0 deletions api/src/main/java/org/openmrs/api/PatientService.java
View file
Expand Up @@ -566,6 +566,7 @@ public PatientIdentifierType unretirePatientIdentifierType(PatientIdentifierType
* @return a list of matching Patients
* @throws APIException
* @since 1.8
* @should find a patients with a matching identifier with no digits
*/
@Authorized( { PrivilegeConstants.VIEW_PATIENTS })
public List<Patient> getPatients(String query, Integer start, Integer length) throws APIException;
Expand Down Expand Up @@ -971,6 +972,7 @@ public void saveCauseOfDeathObs(Patient patient, Date dateDied, Concept causeOfD
* @return the number of patients matching the given search phrase
* @since 1.8
* @should return the right count when a patient has multiple matching person names
* @should return the right count of patients with a matching identifier with no digits
*/
@Authorized( { PrivilegeConstants.VIEW_PATIENTS })
public Integer getCountOfPatients(String query);
Expand Down
10 changes: 8 additions & 2 deletions api/src/main/java/org/openmrs/api/db/PatientDAO.java
View file
Expand Up @@ -55,6 +55,8 @@ public interface PatientDAO {
public List<Patient> getAllPatients(boolean includeVoided) throws DAOException;

/**
* @param searchOnNamesOrIdentifiers specifies if the logic should find patients that match the
* name or identifier otherwise find patients that match both the name and identifier
* @see org.openmrs.api.PatientService#getPatients(String, String, List, boolean, Integer,
* Integer)
* @should escape percentage character in name phrase
Expand All @@ -63,9 +65,11 @@ public interface PatientDAO {
* @should escape percentage character in identifier phrase
* @should escape underscore character in identifier phrase
* @should escape an asterix character in identifier phrase
* @should get patients with a matching identifier and type
*/
public List<Patient> getPatients(String name, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly, Integer start, Integer length) throws DAOException;
boolean matchIdentifierExactly, Integer start, Integer length, boolean searchOnNamesOrIdentifiers)
throws DAOException;

/**
* @see org.openmrs.api.PatientService#getPatientIdentifiers(java.lang.String, java.util.List,
Expand Down Expand Up @@ -155,8 +159,10 @@ public List<PatientIdentifierType> getPatientIdentifierTypes(String name, String
public void deletePatientIdentifier(PatientIdentifier patientIdentifier) throws DAOException;

/**
* @param searchOnNamesOrIdentifiers specifies if the logic should find patients that match the
* name or identifier otherwise find patients that match both the name and identifier
* @see PatientService#getCountOfPatients(String)
*/
public Long getCountOfPatients(String name, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly);
boolean matchIdentifierExactly, boolean searchOnNamesOrIdentifiers);
}
2 changes: 1 addition & 1 deletion api/src/main/java/org/openmrs/api/db/hibernate/HibernateEncounterDAO.java
View file
Expand Up @@ -409,7 +409,7 @@ private Criteria createEncounterByQueryCriteria(String query, Integer patientId,
name = query;
}
criteria = new PatientSearchCriteria(sessionFactory, criteria).prepareCriteria(name, identifier,
new ArrayList<PatientIdentifierType>(), false, orderByNames);
new ArrayList<PatientIdentifierType>(), false, orderByNames, false);
}

return criteria;
Expand Down
13 changes: 7 additions & 6 deletions api/src/main/java/org/openmrs/api/db/hibernate/HibernatePatientDAO.java
View file
Expand Up @@ -192,19 +192,20 @@ private void insertPatientStubIfNeeded(Patient patient) {

/**
* @see org.openmrs.api.db.PatientDAO#getPatients(String, String, List, boolean, Integer,
* Integer)
* Integer, boolean)
*/
@SuppressWarnings("unchecked")
public List<Patient> getPatients(String name, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly, Integer start, Integer length) throws DAOException {
boolean matchIdentifierExactly, Integer start, Integer length, boolean searchOnNamesOrIdentifiers)
throws DAOException {
if (StringUtils.isBlank(name) && StringUtils.isBlank(identifier)
&& (identifierTypes == null || identifierTypes.isEmpty())) {
return Collections.emptyList();
}

Criteria criteria = sessionFactory.getCurrentSession().createCriteria(Patient.class);
criteria = new PatientSearchCriteria(sessionFactory, criteria).prepareCriteria(name, identifier, identifierTypes,
matchIdentifierExactly, true);
matchIdentifierExactly, true, searchOnNamesOrIdentifiers);
// restricting the search to the max search results value
if (start != null)
criteria.setFirstResult(start);
Expand Down Expand Up @@ -575,16 +576,16 @@ public void deletePatientIdentifier(PatientIdentifier patientIdentifier) throws
}

/**
* @see PatientDAO#getCountOfPatients(String, String, List, boolean)
* @see PatientDAO#getCountOfPatients(String, String, List, boolean, boolean)
*/
public Long getCountOfPatients(String name, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly) {
boolean matchIdentifierExactly, boolean searchOnNamesOrIdentifiers) {
Criteria criteria = sessionFactory.getCurrentSession().createCriteria(Patient.class);
//Skip the ordering of names because H2(and i think PostgreSQL) will require one of the ordered
//columns to be in the resultset which then contradicts with the combination of
//(Projections.rowCount() and Criteria.uniqueResult()) that expect back only one row with one column
criteria = new PatientSearchCriteria(sessionFactory, criteria).prepareCriteria(name, identifier, identifierTypes,
matchIdentifierExactly, false);
matchIdentifierExactly, false, searchOnNamesOrIdentifiers);
criteria.setProjection(Projections.countDistinct("patientId"));
return (Long) criteria.uniqueResult();
}
Expand Down
78 changes: 56 additions & 22 deletions api/src/main/java/org/openmrs/api/db/hibernate/PatientSearchCriteria.java
View file
Expand Up @@ -20,6 +20,8 @@
import org.hibernate.Criteria;
import org.hibernate.Hibernate;
import org.hibernate.SessionFactory;
import org.hibernate.criterion.Conjunction;
import org.hibernate.criterion.Criterion;
import org.hibernate.criterion.Expression;
import org.hibernate.criterion.LogicalExpression;
import org.hibernate.criterion.MatchMode;
Expand Down Expand Up @@ -58,10 +60,22 @@ public PatientSearchCriteria(SessionFactory sessionFactory, Criteria criteria) {
* @param identifier
* @param identifierTypes
* @param matchIdentifierExactly
* @param searchOnNamesOrIdentifiers specifies if the logic should find patients that match the
* name or identifier otherwise find patients that match both the name and identifier
* @return {@link Criteria}
*/
public Criteria prepareCriteria(String name, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly, boolean orderByNames) {
boolean matchIdentifierExactly, boolean orderByNames, boolean searchOnNamesOrIdentifiers) {

//Find patients that match either the name or identifier if only
//the name or identifier was passed in to this method
if (searchOnNamesOrIdentifiers && (name == null || identifier == null)) {
if (name == null)
name = identifier;
else
identifier = name;
}

name = HibernateUtil.escapeSqlWildcards(name, sessionFactory);
identifier = HibernateUtil.escapeSqlWildcards(identifier, sessionFactory);

Expand All @@ -74,20 +88,33 @@ public Criteria prepareCriteria(String name, String identifier, List<PatientIden

// get only distinct patients
criteria.setResultTransformer(Criteria.DISTINCT_ROOT_ENTITY);

Criterion nameCriterion = null;
if (name != null) {
addNameCriterias(criteria, name);
nameCriterion = prepareNameCriterion(name);
}

Criterion identifierCriterion = null;
// do the restriction on either identifier string or types
if (identifier != null || identifierTypes.size() > 0) {
addIdentifierCriterias(criteria, identifier, identifierTypes, matchIdentifierExactly);
identifierCriterion = prepareIdentifierCriterion(identifier, identifierTypes, matchIdentifierExactly,
searchOnNamesOrIdentifiers);
}

if (searchOnNamesOrIdentifiers) {
criteria.add(Restrictions.or(nameCriterion, identifierCriterion));
} else {
if (nameCriterion != null) {
criteria.add(nameCriterion);
}
if (identifierCriterion != null) {
criteria.add(identifierCriterion);
}
}

// TODO add junit test for searching on voided patients

// make sure the patient object isn't voided
criteria.add(Expression.eq("voided", false));
criteria.add(Restrictions.eq("voided", false));

return criteria;
}
Expand All @@ -100,19 +127,21 @@ public Criteria prepareCriteria(String name, String identifier, List<PatientIden
* @param identifierTypes
* @param matchIdentifierExactly
*/
private void addIdentifierCriterias(Criteria criteria, String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly) {
private Criterion prepareIdentifierCriterion(String identifier, List<PatientIdentifierType> identifierTypes,
boolean matchIdentifierExactly, boolean searchOnNamesOrIdentifiers) {

Conjunction conjuction = Restrictions.conjunction();
// TODO add junit test for searching on voided identifiers

// add the join on the identifiers table
criteria.createAlias("identifiers", "ids");
criteria.add(Expression.eq("ids.voided", false));

conjuction.add(Restrictions.eq("ids.voided", false));
// do the identifier restriction
if (identifier != null) {
// if the user wants an exact search, match on that.
if (matchIdentifierExactly) {
criteria.add(Expression.eq("ids.identifier", identifier));
conjuction.add(Restrictions.eq("ids.identifier", identifier).ignoreCase());
} else {
AdministrationService adminService = Context.getAdministrationService();
String regex = adminService.getGlobalProperty(OpenmrsConstants.GLOBAL_PROPERTY_PATIENT_IDENTIFIER_REGEX, "");
Expand All @@ -125,18 +154,18 @@ private void addIdentifierCriterias(Criteria criteria, String identifier, List<P
}

if (StringUtils.hasLength(patternSearch)) {
splitAndAddSearchPattern(criteria, identifier, patternSearch);
conjuction.add(splitAndGetSearchPattern(identifier, patternSearch));
}
// if the regex is empty, default to a simple "like" search or if
// we're in hsql world, also only do the simple like search (because
// hsql doesn't know how to deal with 'regexp'
else if (regex.equals("") || HibernateUtil.isHSQLDialect(sessionFactory)) {
addCriterionForSimpleSearch(criteria, identifier, adminService);
conjuction.add(getCriterionForSimpleSearch(identifier, adminService));
}
// if the regex is present, search on that
else {
regex = replaceSearchString(regex, identifier);
criteria.add(Restrictions.sqlRestriction("identifier regexp ?", regex, Hibernate.STRING));
conjuction.add(Restrictions.sqlRestriction("identifier regexp ?", regex, Hibernate.STRING));
}
}
}
Expand All @@ -145,39 +174,38 @@ else if (regex.equals("") || HibernateUtil.isHSQLDialect(sessionFactory)) {

// do the type restriction
if (identifierTypes.size() > 0) {
criteria.add(Expression.in("ids.identifierType", identifierTypes));
criteria.add(Restrictions.in("ids.identifierType", identifierTypes));
}

return conjuction;
}

/**
* Utility method to add prefix and suffix like expression
*
* @param criteria
* @param identifier
* @param adminService
*/
private void addCriterionForSimpleSearch(Criteria criteria, String identifier, AdministrationService adminService) {
private Criterion getCriterionForSimpleSearch(String identifier, AdministrationService adminService) {
String prefix = adminService.getGlobalProperty(OpenmrsConstants.GLOBAL_PROPERTY_PATIENT_IDENTIFIER_PREFIX, "");
String suffix = adminService.getGlobalProperty(OpenmrsConstants.GLOBAL_PROPERTY_PATIENT_IDENTIFIER_SUFFIX, "");
StringBuffer likeString = new StringBuffer(prefix).append(identifier).append(suffix);
criteria.add(Expression.like("ids.identifier", likeString.toString()));
return Restrictions.ilike("ids.identifier", likeString.toString());
}

/**
* Utility method to add search pattern expression to identifier.
*
* @param criteria
* @param identifier
* @param patternSearch
*/
private void splitAndAddSearchPattern(Criteria criteria, String identifier, String patternSearch) {
private Criterion splitAndGetSearchPattern(String identifier, String patternSearch) {
// split the pattern before replacing in case the user searched on a comma
List<String> searchPatterns = new ArrayList<String>();
// replace the @SEARCH@, etc in all elements
for (String pattern : patternSearch.split(","))
searchPatterns.add(replaceSearchString(pattern, identifier));
criteria.add(Expression.in("ids.identifier", searchPatterns));
return Restrictions.in("ids.identifier", searchPatterns);
}

/**
Expand All @@ -200,7 +228,10 @@ private String removePadding(String identifier, String regex) {
* @param criteria
* @param name
*/
private void addNameCriterias(Criteria criteria, String name) {
private Criterion prepareNameCriterion(String name) {

Conjunction conjuction = Restrictions.conjunction();

// TODO simple name search to start testing, will need to make "real"
// name search
// i.e. split on whitespace, guess at first/last name, etc
Expand All @@ -220,12 +251,14 @@ private void addNameCriterias(Criteria criteria, String name) {
if (i > 0) {
nameSoFar += " " + n;
LogicalExpression fullNameSearch = getNameSearch(nameSoFar);
searchExpression = Expression.or(oneNameSearch, fullNameSearch);
searchExpression = Restrictions.or(oneNameSearch, fullNameSearch);
}
criteria.add(searchExpression);
conjuction.add(searchExpression);
}
}
}

return conjuction;
}

/**
Expand All @@ -237,6 +270,7 @@ private void addNameCriterias(Criteria criteria, String name) {
* <pre>
* ... where voided = false &amp;&amp; name in (familyName2, familyName, middleName, givenName)
* </pre>
*
* Except when the name provided is less than min characters (usually 3) then we will look for
* an EXACT match by default
*
Expand Down
21 changes: 4 additions & 17 deletions api/src/main/java/org/openmrs/api/impl/PatientServiceImpl.java
View file
Expand Up @@ -1646,14 +1646,7 @@ public Integer getCountOfPatients(String query) {
if (StringUtils.isBlank(query))
return count;
List<PatientIdentifierType> emptyList = new Vector<PatientIdentifierType>();
// if there is a number in the query string
if (query.matches(".*\\d+.*")) {
log.debug("[Identifier search] Query: " + query);
return OpenmrsUtil.convertToInteger(dao.getCountOfPatients(null, query, emptyList, false));
} else {
// there is no number in the string, search on name
return OpenmrsUtil.convertToInteger(dao.getCountOfPatients(query, null, emptyList, false));
}
return OpenmrsUtil.convertToInteger(dao.getCountOfPatients(null, query, emptyList, false, true));
}

/**
Expand All @@ -1679,21 +1672,15 @@ private int getMinSearchCharacters() {
/**
* @see PatientService#getPatients(String, Integer, Integer)
*/
@SuppressWarnings("unchecked")
@Override
@Transactional(readOnly = true)
public List<Patient> getPatients(String query, Integer start, Integer length) throws APIException {
List<Patient> patients = new Vector<Patient>();
if (StringUtils.isBlank(query))
return patients;

// if there is a number in the query string
if (query.matches(".*\\d+.*")) {
log.debug("[Identifier search] Query: " + query);
return getPatients(null, query, null, false, start, length);
} else {
// there is no number in the string, search on name
return getPatients(query, null, null, false, start, length);
}
return dao.getPatients(query, null, Collections.EMPTY_LIST, false, start, length, true);
}

/**
Expand All @@ -1706,6 +1693,6 @@ public List<Patient> getPatients(String name, String identifier, List<PatientIde
if (identifierTypes == null)
identifierTypes = Collections.emptyList();

return dao.getPatients(name, identifier, identifierTypes, matchIdentifierExactly, start, length);
return dao.getPatients(name, identifier, identifierTypes, matchIdentifierExactly, start, length, false);
}
}

0 comments on commit abe78c5

Please sign in to comment.