Skip to content

Commit

Permalink
TRUNK-3992: PersonByNameComparator should be case-insensitive
Browse files Browse the repository at this point in the history
  • Loading branch information
mogoodrich committed May 22, 2013
1 parent 76d93ba commit 8bf5387
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 36 deletions.
10 changes: 5 additions & 5 deletions api/src/main/java/org/openmrs/aop/LoggingAdvice.java
Expand Up @@ -13,10 +13,6 @@
*/
package org.openmrs.aop;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.commons.logging.Log;
Expand All @@ -26,6 +22,10 @@
import org.openmrs.api.context.Context;
import org.openmrs.util.OpenmrsUtil;

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;

/**

This comment has been minimized.

Copy link
@mogoodrich

mogoodrich May 22, 2013

Author Member

Okay, I'll take the crap for reformatting these includes... I thought I had that feature turned off... but the difference between Eclipse and IntelliJ is really annoying...

* This class provides the log4j aop around advice for our service layer. This advice is placed on
* all services and daos via the spring application context. See
Expand Down Expand Up @@ -123,7 +123,7 @@ else if (logSetter)
}
catch (Throwable t) {
if (logGetter || logSetter) {
String username;
String username;
User user = Context.getAuthenticatedUser();
if (user == null) {
username = "Guest (Not logged in)";
Expand Down
25 changes: 17 additions & 8 deletions api/src/main/java/org/openmrs/util/PersonByNameComparator.java
Expand Up @@ -13,11 +13,11 @@
*/
package org.openmrs.util;

import java.util.Comparator;

import org.openmrs.Person;
import org.openmrs.PersonName;

import java.util.Comparator;

/**
* A simple person comparator for sorting persons by name. Sorts names based on the following
* precedence: FamilyName, FamilyName2, GivenName, MiddleName, FamilyNamePrefix, FamilyNameSuffix
Expand All @@ -40,6 +40,7 @@ public int compare(Person person1, Person person2) {
* @should return negative if personName for person1 comes before that of person2
* @should return positive if personName for person1 comes after that of person2
* @should return zero if the givenName middleName and familyName match
* @should be case insensitive
* @since 1.8
*/
public static int comparePersonsByName(Person person1, Person person2) {
Expand All @@ -55,26 +56,34 @@ public static int comparePersonsByName(Person person1, Person person2) {
PersonName name1 = person1.getPersonName();
PersonName name2 = person2.getPersonName();

int ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyName(), name2.getFamilyName());
int ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyName() != null ? name1.getFamilyName().toLowerCase()
: null, name2.getFamilyName() != null ? name2.getFamilyName().toLowerCase() : null);

if (ret == 0) {
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyName2(), name2.getFamilyName2());
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyName2() != null ? name1.getFamilyName().toLowerCase()
: null, name2.getFamilyName2() != null ? name2.getFamilyName2().toLowerCase() : null);
}

if (ret == 0) {
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getGivenName(), name2.getGivenName());
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getGivenName() != null ? name1.getGivenName().toLowerCase()
: null, name2.getGivenName() != null ? name2.getGivenName().toLowerCase() : null);
}

if (ret == 0) {
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getMiddleName(), name2.getMiddleName());
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getMiddleName() != null ? name1.getMiddleName().toLowerCase()
: null, name2.getMiddleName() != null ? name2.getMiddleName().toLowerCase() : null);
}

if (ret == 0) {
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyNamePrefix(), name2.getFamilyNamePrefix());
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyNamePrefix() != null ? name1.getFamilyNamePrefix()
.toLowerCase() : null, name2.getFamilyNamePrefix() != null ? name2.getFamilyNamePrefix().toLowerCase()
: null);
}

if (ret == 0) {
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyNameSuffix(), name2.getFamilyNameSuffix());
ret = OpenmrsUtil.compareWithNullAsGreatest(name1.getFamilyNameSuffix() != null ? name1.getFamilyNameSuffix()
.toLowerCase() : null, name2.getFamilyNameSuffix() != null ? name2.getFamilyNameSuffix().toLowerCase()
: null);
}

return ret;
Expand Down
11 changes: 11 additions & 0 deletions api/src/test/java/org/openmrs/util/PersonByNameComparatorTest.java
Expand Up @@ -65,4 +65,15 @@ public void comparePersonsByName_shouldReturnZeroIfTheGivenNameMiddleNameAndFami
int actualValue = PersonByNameComparator.comparePersonsByName(person1, person2);
Assert.assertTrue("Expected zero but it was: " + actualValue, actualValue == 0);
}

@Test
@Verifies(value = "should not be case-sensitive", method = "comparePersonsByName(Person,Person)")
public void comparePersonsByName_shouldNotBeCaseSensitive() throws Exception {
Person person1 = new Person();
person1.addName(new PersonName("GIVENNAME", "MIDDLENAME", "FAMILYNAME"));
Person person2 = new Person();
person2.addName(new PersonName("givenName", "middleName", "familyName"));
int actualValue = PersonByNameComparator.comparePersonsByName(person1, person2);
Assert.assertTrue("Expected zero but it was: " + actualValue, actualValue == 0);
}
}
45 changes: 22 additions & 23 deletions web/src/main/java/org/openmrs/web/Listener.java
Expand Up @@ -13,28 +13,6 @@
*/

This comment has been minimized.

Copy link
@mogoodrich

mogoodrich May 22, 2013

Author Member

This file is auto-formatting noise and can be ignored as well.

package org.openmrs.web;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.StringReader;
import java.sql.Driver;
import java.sql.DriverManager;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;

import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.ServletException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.log4j.LogManager;
Expand Down Expand Up @@ -65,6 +43,27 @@
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.ServletException;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
import java.io.StringReader;
import java.sql.Driver;
import java.sql.DriverManager;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;

/**
* Our Listener class performs the basic starting functions for our webapp. Basic needs for starting
* the API: 1) Get the runtime properties 2) Start Spring 3) Start the OpenMRS APi (via
Expand Down Expand Up @@ -597,7 +596,7 @@ public static void performWebStartOfModules(ServletContext servletContext) throw
catch (Throwable t) {
Throwable rootCause = getActualRootCause(t, true);
if (rootCause != null)
log.fatal("Unable to refresh the spring application context. Root Cause was:", rootCause);
log.fatal("Unable to refresh the spring application context. Root Cause was:", rootCause);
else
log.fatal("Unable to refresh the spring application context. Unloading all modules, Error was:", t);

Expand Down

0 comments on commit 8bf5387

Please sign in to comment.