Skip to content

Commit

Permalink
TRUNK-6265: Bugs in Concept.getPreferredName(Locale) method (#4749)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mogoodrich
mogoodrich authored Sep 19, 2024
1 parent 117b775 commit bc725f2
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 26 deletions.
58 changes: 32 additions & 26 deletions api/src/main/java/org/openmrs/Concept.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public void setPreferredName(ConceptName preferredName) {
}

//first revert the current preferred name(if any) from being preferred
ConceptName oldPreferredName = getPreferredName(preferredName.getLocale());
ConceptName oldPreferredName = getPreferredName(preferredName.getLocale(), true);
if (oldPreferredName != null) {
oldPreferredName.setLocalePreferred(false);
}
Expand Down Expand Up @@ -633,24 +633,26 @@ private ConceptName getNameInLocale(Locale locale) {
return null;
}

public ConceptName getPreferredName(Locale forLocale) {
return getPreferredName(forLocale, false);
}

/**
* Returns the name which is explicitly marked as preferred for a given locale.
*
* @param forLocale locale for which to return a preferred name
* @return preferred name for the locale, or null if no preferred name is specified
* <strong>Should</strong> return the concept name explicitly marked as locale preferred
* <strong>Should</strong> return the fully specified name if no name is explicitly marked as locale preferred
* <strong>Should</strong> return the concept name marked as locale preferred a partial match locale (same language but different country) if no exact match and exact set to false
* <strong>Should</strong> return the fully specified name if no name is explicitly marked as locale preferred and exact set to false
*/
public ConceptName getPreferredName(Locale forLocale) {
public ConceptName getPreferredName(Locale forLocale, Boolean exact) {

if (log.isDebugEnabled()) {
log.debug("Getting preferred conceptName for locale: " + forLocale);
}
// fail early if this concept has no names defined
if (getNames(forLocale).isEmpty()) {
log.debug("there are no names defined for concept with id: {} in the locale: {}", conceptId, forLocale);
return null;
} else if (forLocale == null) {

if (forLocale == null) {
log.warn("Locale cannot be null");
return null;
}
Expand All @@ -661,26 +663,30 @@ public ConceptName getPreferredName(Locale forLocale) {
}
}

// look for partially locale match - any language matches takes precedence over country matches.
ConceptName bestMatch = null;

for (ConceptName nameInLocale : getPartiallyCompatibleNames(forLocale)) {
if (ObjectUtils.nullSafeEquals(nameInLocale.getLocalePreferred(), true)) {
Locale nameLocale = nameInLocale.getLocale();
if (forLocale.getLanguage().equals(nameLocale.getLanguage())) {
return nameInLocale;
} else {
bestMatch = nameInLocale;
if (exact) {
return null;
} else {
// look for partially locale match - any language matches takes precedence over country matches.
ConceptName bestMatch = null;

for (ConceptName nameInLocale : getPartiallyCompatibleNames(forLocale)) {
if (ObjectUtils.nullSafeEquals(nameInLocale.getLocalePreferred(), true)) {
Locale nameLocale = nameInLocale.getLocale();
if (forLocale.getLanguage().equals(nameLocale.getLanguage())) {
return nameInLocale;
} else {
bestMatch = nameInLocale;
}

}

}

if (bestMatch != null) {
return bestMatch;
}

return getFullySpecifiedName(forLocale);
}

if (bestMatch != null) {
return bestMatch;
}

return getFullySpecifiedName(forLocale);
}

/**
Expand Down Expand Up @@ -1014,7 +1020,7 @@ public void addName(ConceptName conceptName) {
conceptName.setConceptNameType(ConceptNameType.FULLY_SPECIFIED);
} else {
if (conceptName.isPreferred() && !conceptName.isIndexTerm() && conceptName.getLocale() != null) {
ConceptName prefName = getPreferredName(conceptName.getLocale());
ConceptName prefName = getPreferredName(conceptName.getLocale(), true);
if (prefName != null) {
prefName.setLocalePreferred(false);
}
Expand Down
57 changes: 57 additions & 0 deletions api/src/test/java/org/openmrs/ConceptTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,42 @@ public void getPreferredName_shouldReturnTheConceptNameExplicitlyMarkedAsLocaleP
assertEquals(preferredNameEN, testConcept.getPreferredName(new Locale("en")));
}

@Test
public void getPreferredName_shouldReturnPreferredNameInLocaleWithCountryIfNoPreferredNameInLocaleWithNoCountry() {
Concept color = new Concept();
// add a name in en but *not* set as preferred
ConceptName preferredNameEN = createConceptName(3, "Color", new Locale("en"), null, false);
color.addName(preferredNameEN);
//preferred name in en_UK
ConceptName preferredNameEN_UK = createConceptName(4, "Colour", Locale.UK, null, true);
color.addName(preferredNameEN_UK);
// we ask for preferred name in en, but since none of the en names are preferred, we should get the en_UK name
assertEquals(preferredNameEN_UK, color.getPreferredName(new Locale("en")));
}

@Test
public void getPreferredName_shouldReturnPreferredNameInLocaleWithCountryIfNoNameInLocaleWithNoCountry() {
Concept color = new Concept();
//preferred name in en_UK
ConceptName preferredNameEN_UK = createConceptName(4, "Colour", Locale.UK, null, true);
color.addName(preferredNameEN_UK);
// we ask for preferred name in en, but since none of the en names are preferred, we should get the en_UK name
assertEquals(preferredNameEN_UK, color.getPreferredName(new Locale("en")));
}

@Test
public void getPreferredName_shouldReturnPreferredNameInLocaleWithoutCountryBeforeLocaleWithCountry() {
Concept color = new Concept();
// preferred name in en_UK
ConceptName preferredNameEN_UK = createConceptName(4, "Colour", Locale.UK, null, true);
color.addName(preferredNameEN_UK);
// preferred name in en
ConceptName preferredNameEN = createConceptName(3, "Color", new Locale("en"), null, true);
color.addName(preferredNameEN);
// we ask for preferred name in en_US; there are no names in en_US, but it should "prefer" "en" over "en_UK"
assertEquals(preferredNameEN, color.getPreferredName(new Locale("en")));
}

/**
* @see Concept#getShortestName(Locale,Boolean)
*/
Expand Down Expand Up @@ -901,6 +937,27 @@ public void setPreferredName_shouldFailIfThePreferredNameToSetToIsAnIndexTerm()
assertThrows(APIException.class, () -> concept.setPreferredName(preferredName));
}

@Test
public void setPreferredName_shouldNotSetPreferredNameInDifferentCountryLocaleToNotPreferred() {
Concept concept = new Concept();
// set non-preferred name in en_US (due to an idiosyncrasy we need to set this to manifest the bug)
ConceptName nonPreferredNameInUS = new ConceptName("Col", Locale.US);
nonPreferredNameInUS.setLocalePreferred(false);
concept.addName(nonPreferredNameInUS);
// set preferred name in en_UK
ConceptName preferredNameInUK = new ConceptName("Colour", Locale.UK);
preferredNameInUK.setLocalePreferred(true);
concept.addName(preferredNameInUK);
// now set preferred name in en_US
ConceptName preferredNameInUS = new ConceptName("Color", Locale.US);
preferredNameInUS.setLocalePreferred(true);
concept.addName(preferredNameInUS);
assertThat(nonPreferredNameInUS.getLocalePreferred(), is(false));
assertThat(preferredNameInUK.getLocalePreferred(), is(true));
assertThat(preferredNameInUS.getLocalePreferred(), is(true));

}

/**
* @see Concept#addName(ConceptName)
*/
Expand Down

0 comments on commit bc725f2

Please sign in to comment.