Skip to content

#292 - Error loading locations on OpenMRS 2.7 #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ See the [documentation on Initializer's logging properties](readme/rtprops.md#lo

#### Version 2.9.0
* Fix for InitializerSerializer to ensure compatibility with OpenMRS version 2.7.0+
* Fix for processing attributes to ensure compatibility with OpenMRS version 2.7.0+

#### Version 2.8.0
* Ampath forms translation files will now generate checksums.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/**
* This Source Code Form is subject to the terms of the Mozilla Public License,
* v. 2.0. If a copy of the MPL was not distributed with this file, You can
* obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under
* the terms of the Healthcare Disclaimer located at http://openmrs.org/license.
*
* Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS
* graphic logo is a trademark of OpenMRS Inc.
*/
package org.openmrs.module.initializer.api.loaders;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.openmrs.Location;
import org.openmrs.LocationAttribute;
import org.openmrs.LocationTag;
import org.openmrs.api.LocationService;
import org.openmrs.api.context.Context;
import org.openmrs.customdatatype.datatype.DateDatatype;
import org.openmrs.module.initializer.DomainBaseModuleContextSensitive_2_7_Test;
import org.openmrs.module.initializer.api.loc.LocationsLoader;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;

import java.util.Collection;
import java.util.Date;
import java.util.Locale;
import java.util.Set;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;

/**
* This is a copy of the LocationsLoaderIntegration test in order to test the same test against
* OpenMRS 2.7
*/
public class LocationsLoader_2_7_IntegrationTest extends DomainBaseModuleContextSensitive_2_7_Test {

@Autowired
@Qualifier("locationService")
private LocationService ls;

@Autowired
private LocationsLoader loader;

@Autowired
private DateDatatype dateDatatype;

private Locale localeEn = Locale.ENGLISH;

private Locale localeKm = new Locale("km", "KH");

@Before
public void setup() throws Exception {
executeDataSet("testdata/test-metadata.xml");
}

@Test
public void load_shouldLoadAccordingToCsvFiles() {
// Pre-load verif
{
Location loc = ls.getLocation(4089);
Assert.assertEquals("a03e395c-b881-49b7-b6fc-983f6bddc7fc", loc.getUuid());
Assert.assertEquals("Acme Clinic", loc.getName());
Assert.assertThat(loc.getDescription(), nullValue());
Assert.assertThat(loc.getParentLocation(), nullValue());
Assert.assertThat(loc.getTags().size(), is(0));

Collection<LocationAttribute> attributes = loc.getActiveAttributes();
Assert.assertThat(attributes.size(), is(1));
Assert.assertEquals("2016-04-14",
dateDatatype.serialize((Date) ((LocationAttribute) attributes.toArray()[0]).getValue()));
}
{
Location loc = ls.getLocation(4090);
Assert.assertEquals("cbaaaab4-d960-4ae9-9b6a-8983fbd947b6", loc.getUuid());
Assert.assertEquals("Legacy Location", loc.getName());
Assert.assertEquals("Legacy location that must be retired", loc.getDescription());
Assert.assertThat(loc.getRetired(), is(false));
}

// Replay
loader.load();

// Verify fetch by name
{
Location loc = ls.getLocation("LOCATION_NO_UUID");
Assert.assertEquals("Main Street", loc.getAddress1());
Assert.assertEquals("fdddc31a-3930-11ea-9712-a73c3c19744f", loc.getUuid());
}
// Verify creation with dynamic tag creation
{
Location loc = ls.getLocation("The Lake Clinic-Cambodia");
Assert.assertEquals("Paradise Street", loc.getAddress1());
Assert.assertEquals("Siem Reap", loc.getCountyDistrict());
Assert.assertEquals("Siem Reap", loc.getStateProvince());
Assert.assertEquals("Cambodia", loc.getCountry());

Set<LocationTag> tags = loc.getTags();
Assert.assertThat(tags, notNullValue());
Assert.assertThat(tags.size(), is(2));
Assert.assertThat(tags.contains(ls.getLocationTagByName("Login Location")), is(true));
Assert.assertThat(tags.contains(ls.getLocationTagByName("Another Location Tag")), is(true));

Collection<LocationAttribute> attributes = loc.getActiveAttributes();
Assert.assertThat(attributes.size(), is(2));
Assert.assertEquals("CODE-TLC-123", ((LocationAttribute) attributes.toArray()[0]).getValue());
Assert.assertEquals("2017-05-15",
dateDatatype.serialize((Date) ((LocationAttribute) attributes.toArray()[1]).getValue()));
}
// Verify creation with Tag| headers
{
Location loc = ls.getLocation("OPD Room");
Assert.assertEquals(ls.getLocation("The Lake Clinic-Cambodia"), loc.getParentLocation());

Set<LocationTag> tags = loc.getTags();
Assert.assertThat(tags, notNullValue());
Assert.assertThat(tags.size(), is(1));
Assert.assertThat(tags.contains(ls.getLocationTagByName("Facility Location")), is(true));
}
// Verify that the provided UUID is correctly assigned
{
Location loc = ls.getLocationByUuid("1cb58794-3c49-11ea-b3eb-f7801304f314");
Assert.assertNotNull(loc);
Assert.assertEquals("New Location", loc.getName());
}
// Verify edit
{
Location loc = ls.getLocationByUuid("a03e395c-b881-49b7-b6fc-983f6bddc7fc");
Assert.assertEquals("Acme Clinic", loc.getName());
Assert.assertEquals("This now becomes a child of TLC", loc.getDescription());
Assert.assertEquals(ls.getLocation("The Lake Clinic-Cambodia"), loc.getParentLocation());

Set<LocationTag> tags = loc.getTags();
Assert.assertThat(tags, notNullValue());
Assert.assertThat(tags.size(), is(1));
Assert.assertThat(tags.contains(ls.getLocationTagByName("Login Location")), is(true));

Collection<LocationAttribute> attributes = loc.getActiveAttributes();
Assert.assertThat(attributes.size(), is(1));
Assert.assertEquals("2019-03-13",
dateDatatype.serialize((Date) ((LocationAttribute) attributes.toArray()[0]).getValue()));

}
// Verify retire
{
Location loc = ls.getLocationByUuid("cbaaaab4-d960-4ae9-9b6a-8983fbd947b6");
Assert.assertThat(loc.getRetired(), is(true));
}
// Verify that location with an invalid parent isn't created
{
Location loc = ls.getLocationByUuid("2b9824a3-92f0-4966-8f34-1b105624b267");
Assert.assertNull(loc);
}

// verify Locations domain i18n on entries with display:xy fields
{
Assert.assertEquals("Acme Clinic (translated)", Context.getMessageSourceService()
.getMessage("ui.i18n.Location.name.a03e395c-b881-49b7-b6fc-983f6bddc7fc", null, localeEn));
Assert.assertEquals("គ្លីនិកអាមី", Context.getMessageSourceService()
.getMessage("ui.i18n.Location.name.a03e395c-b881-49b7-b6fc-983f6bddc7fc", null, localeKm));
Assert.assertEquals("Acme Clinic (translated)", Context.getMessageSourceService()
.getMessage("org.openmrs.Location.a03e395c-b881-49b7-b6fc-983f6bddc7fc", null, localeEn));
Assert.assertEquals("គ្លីនិកអាមី", Context.getMessageSourceService()
.getMessage("org.openmrs.Location.a03e395c-b881-49b7-b6fc-983f6bddc7fc", null, localeKm));
}
// verify no Locations domain i18n on entries without display:xy fields and entries without pre-filled uuids
{
Assert.assertNotNull(ls.getLocationByUuid("1cb58794-3c49-11ea-b3eb-f7801304f314"));
Assert.assertEquals("ui.i18n.Location.name.1cb58794-3c49-11ea-b3eb-f7801304f314",
Context.getMessageSourceService().getMessage("ui.i18n.Location.name.1cb58794-3c49-11ea-b3eb-f7801304f314",
null, localeEn));
Assert.assertEquals("ui.i18n.Location.name.1cb58794-3c49-11ea-b3eb-f7801304f314",
Context.getMessageSourceService().getMessage("ui.i18n.Location.name.1cb58794-3c49-11ea-b3eb-f7801304f314",
null, localeKm));

Assert.assertEquals("org.openmrs.Location.1cb58794-3c49-11ea-b3eb-f7801304f314",
Context.getMessageSourceService().getMessage("org.openmrs.Location.1cb58794-3c49-11ea-b3eb-f7801304f314",
null, localeEn));
Assert.assertEquals("org.openmrs.Location.1cb58794-3c49-11ea-b3eb-f7801304f314",
Context.getMessageSourceService().getMessage("org.openmrs.Location.1cb58794-3c49-11ea-b3eb-f7801304f314",
null, localeKm));

Location loc = ls.getLocation("The Lake Clinic-Cambodia");
Assert.assertNotNull(loc);

String uuid = loc.getUuid();
Assert.assertEquals("ui.i18n.Location.name." + uuid,
Context.getMessageSourceService().getMessage("ui.i18n.Location.name." + uuid, null, localeEn));
Assert.assertEquals("ui.i18n.Location.name." + uuid,
Context.getMessageSourceService().getMessage("ui.i18n.Location.name." + uuid, null, localeKm));
Assert.assertEquals("org.openmrs.Location." + uuid,
Context.getMessageSourceService().getMessage("org.openmrs.Location." + uuid, null, localeEn));
Assert.assertEquals("org.openmrs.Location." + uuid,
Context.getMessageSourceService().getMessage("org.openmrs.Location." + uuid, null, localeKm));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Uuid,Void/Retire,Name,Description,Parent,Tags,Tag|Facility Location,Attribute|9eca4f4e-707f-4bb8-8289-2f9b6e93803c,Attribute|Last Audit Date,Address 1,Address 2,Address 3,Address 4,Address 5,Address 6,City/Village,County/District,State/Province,Postal Code,Country,display:en,display:km_KH,_order:1000
,,The Lake Clinic-Cambodia,,,Login Location; Another Location Tag,,CODE-TLC-123,2017-05-15,Paradise Street,,,,,,,Siem Reap,Siem Reap,,Cambodia,The Lake Clinic-Cambodia (translated),គ្លីនីកគ្លីនិក - ប្រទេសកម្ពុជា,
,,OPD Room,,The Lake Clinic-Cambodia,,TRUE,,,,,,,,,,,,,,,,
a03e395c-b881-49b7-b6fc-983f6bddc7fc,,Acme Clinic,This now becomes a child of TLC,The Lake Clinic-Cambodia,Login Location,,,2019-03-13,,,,,,,,,,,,Acme Clinic (translated),គ្លីនិកអាមី,
cbaaaab4-d960-4ae9-9b6a-8983fbd947b6,TRUE,Legacy Location,Legacy location that must be retired,,,,,,,,,,,,,,,,,,,
,,LOCATION_NO_UUID,,,,,,,Main Street,,,,,,,Siem Reap,Siem Reap,,Cambodia,,,
1cb58794-3c49-11ea-b3eb-f7801304f314,,New Location,,,,,,,,,,,,,,,,,,,,
2b9824a3-92f0-4966-8f34-1b105624b267,,Invalid parent location,This location shouldn not be loaded as it has an invalid parent,Invalid parent,,,,,,,,,,,,,,,,,,
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Uuid,Void/Retire,Name,Description
,,Sparse,
b03e395c-b881-49b7-b6fc-983f6befc7fc,,Filled in,A tag with all its fields
a2327745-2970-4752-ac8a-dd0ba131f40e,TRUE,Facility Location,
a1417745-1170-5752-fc8a-dd0ba131f40e,,Supply Room,Don't call it a shed
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
package org.openmrs.module.initializer.api;

import java.util.AbstractMap;
import java.util.AbstractMap.SimpleEntry;
import java.util.Arrays;
import java.util.Collection;
import java.util.function.Consumer;

import org.apache.commons.lang3.StringUtils;
import org.openmrs.BaseOpenmrsObject;
import org.openmrs.api.context.Context;
import org.openmrs.attribute.Attribute;
import org.openmrs.attribute.BaseAttribute;
import org.openmrs.attribute.BaseAttributeType;
import org.openmrs.customdatatype.CustomDatatype;
import org.openmrs.customdatatype.CustomDatatypeUtil;
import org.openmrs.customdatatype.Customizable;
import org.openmrs.module.initializer.InitializerConstants;

import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;

/**
* Base class to any <code>AttributeLineProcessor</code> that processes all attributes of a domain
Expand All @@ -28,32 +28,64 @@ public T fill(T instance, CsvLine line) throws IllegalArgumentException {

Customizable<A> attributable = (Customizable<A>) instance;

Consumer<? super SimpleEntry<String, String>> processAttributeData = attData -> {

AT attType = getAttributeType(attData.getKey());
if (attType == null) {
throw new IllegalArgumentException("An attribute value is specified ('" + attData.getValue()
+ "') for an attribute type that cannot be resolved by the following identifier: '"
+ attData.getKey() + "'");
// First, retrieve all attributes that are defined in the CSV and the values for the given row
Map<AT, Object> rowValues = new LinkedHashMap<>();
for (String header : line.getHeaderLine()) {
if (header.toLowerCase().startsWith(HEADER_ATTRIBUTE_PREFIX)) {
String attributeTypeIdentifier = StringUtils.removeStartIgnoreCase(header, HEADER_ATTRIBUTE_PREFIX);
AT attributeType = getAttributeType(attributeTypeIdentifier);
Object attributeValue = null;
String attributeValueStr = line.getString(header, "");
if (attributeType == null) {
throw new IllegalArgumentException("An attribute value is specified ('" + attributeValueStr
+ "') for an attribute type that cannot be resolved by the following identifier: '"
+ attributeType + "'");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this was meant to point to attributeTypeIdentifier? Would be nice having a unit test covering this - maybe not strictly necessary?

Suggested change
+ attributeType + "'");
+ attributeTypeIdentifier + "'");

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @Ruhanga . I've updated that. This isn't new functionality, my main goal was to ensure all existing unit and integration tests pass, as well as any that I copied into the api-2.7 project, so my assumption is that the existing unit test coverage is sufficiently robust to handle a refactor.

}
if (StringUtils.isNotBlank(attributeValueStr)) {
String dtClass = attributeType.getDatatypeClassname();
String dtConfig = attributeType.getDatatypeConfig();
CustomDatatype<?> datatype = CustomDatatypeUtil.getDatatype(dtClass, dtConfig);
attributeValue = datatype.fromReferenceString(attributeValueStr);
}
rowValues.put(attributeType, attributeValue);
}

attributable.getAttributes().removeIf(att -> att.getAttributeType().equals(attType));

CustomDatatype<?> datatype = CustomDatatypeUtil.getDatatype(attType.getDatatypeClassname(),
attType.getDatatypeConfig());
Object value = datatype.fromReferenceString(attData.getValue());

A attribute = newAttribute();
attribute.setAttributeType(attType);
attribute.setValue(value);

attributable.addAttribute(attribute);
};
}

// process the attribute value from each attribute header
Arrays.stream(line.getHeaderLine()).filter(h -> StringUtils.startsWithIgnoreCase(h, HEADER_ATTRIBUTE_PREFIX)).map(
h -> new AbstractMap.SimpleEntry<>(StringUtils.removeStartIgnoreCase(h, HEADER_ATTRIBUTE_PREFIX), line.get(h)))
.filter(attData -> StringUtils.isNotBlank(attData.getValue())).forEach(processAttributeData);
// Next, iterate over the existing attributes on the instance, and void and recreate if any have changed
for (A existingAttribute : attributable.getActiveAttributes()) {
AT type = existingAttribute.getAttributeType();
Object existingValue = existingAttribute.getValue();
// If the CSV does not define a column for an existing attribute type, do not modify it on the instance
if (rowValues.containsKey(type)) {
Object newValue = rowValues.remove(type);
// Only process a change if the new value is different from the existing value
if (!existingValue.equals(newValue)) {
// Void existing attribute
existingAttribute.setVoided(true);
existingAttribute.setDateVoided(new Date());
existingAttribute.setVoidedBy(Context.getAuthenticatedUser());
existingAttribute.setVoidReason(InitializerConstants.DEFAULT_VOID_REASON);
// Add new attribute, if there is a value defined for it
if (newValue != null) {
A newAttribute = newAttribute();
newAttribute.setAttributeType(type);
newAttribute.setValue(newValue);
attributable.addAttribute(newAttribute);
}
}
}
}

// Finally, add any remaining attributes that did not match any existing types on the instance
for (AT type : rowValues.keySet()) {
Object newValue = rowValues.get(type);
if (newValue != null) {
A newAttribute = newAttribute();
newAttribute.setAttributeType(type);
newAttribute.setValue(newValue);
attributable.addAttribute(newAttribute);
}
}

return instance;
}
Expand Down
14 changes: 7 additions & 7 deletions api/src/test/resources/testdata/test-metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<location_attribute_type LOCATION_ATTRIBUTE_TYPE_ID="1089" NAME="Location Code" DATATYPE="org.openmrs.customdatatype.datatype.FreeTextDatatype" MIN_OCCURS="0" CREATOR="1" DATE_CREATED="2019-12-11 09:31:48.0" RETIRED="false" UUID="9eca4f4e-707f-4bb8-8289-2f9b6e93803c"/>
<location_attribute_type LOCATION_ATTRIBUTE_TYPE_ID="1090" NAME="Last Audit Date" DATATYPE="org.openmrs.customdatatype.datatype.DateDatatype" CREATOR="1" DATE_CREATED="2005-01-01 00:00:00.0" MIN_OCCURS="0" RETIRED="false" UUID="9516cc50-6f9f-11e0-8414-001e378eb67e"/>

<location_attribute location_attribute_id="1089" location_id="4089" attribute_type_id="1090" value_reference="2016-04-14" creator="1" date_created="2005-01-01 00:00:00.0" voided="false" uuid="3a2bdb18-6faa-11e0-8414-001e378eb67e"/>

<location_tag_map location_id="4092" location_tag_id="4089"/>
Expand All @@ -41,18 +41,18 @@
<privilege privilege="Add Apples" description="Able to add apples." uuid="4604e928-96bf-4e2c-be08-cbbc40dd000c"/>
<privilege privilege="Add Reports" description="Able to add analytic reports." uuid="cf68a296-2700-102b-80cb-0017a47871b2"/>
<privilege privilege="Add Hens" description="Able to add poultry." uuid="36404041-c255-4c5b-9b47-0d757d2afa95"/>

<role ROLE="test-entity-role" UUID="3f8f0ab7-c240-4b68-8951-bb7020be01f6"/>

<patient_identifier_type patient_identifier_type_id="1" name="OpenMRS ID" description="OpenMRS Identification Number" creator="1" date_created="2020-03-1 15:59:20.0" required="false" format="" format_description="" validator="" retired="false" uuid="b0d10dc0-d8ce-11e3-9c1a-0jhg89c9a66" />
<patient_identifier_type patient_identifier_type_id="2" name="Legacy ID" description="Legacy Identification Number" creator="1" date_created="2020-03-1 15:59:20.0" required="false" format="" format_description="" validator="" retired="false" uuid="8aaacb14-5ac0-4226-9a6e-fee8e9d92aeb" />
<idgen_identifier_source id="1" name="OpenMRS ID Generator" identifier_type="1" creator="1" date_created="2009-11-30 11:28:30.489" retired="false" uuid="0d47284f-9e9b-4a81-a88b-8bb42bc0a901" />

<idgen_identifier_source id="1" name="OpenMRS ID Generator" identifier_type="1" creator="1" date_created="2009-11-30 11:28:30.489" retired="false" uuid="0d47284f-9e9b-4a81-a88b-8bb42bc0a901" />
<idgen_seq_id_gen id="1" first_identifier_base="1" next_sequence_value="6" base_character_set="abcdefghijklmnopqrstuvwxyz0123456789" prefix="" min_length="0" max_length="0" />

<idgen_identifier_source id="2" name="Legacy ID Generator" identifier_type="2" creator="1" date_created="2009-11-30 11:28:30.489" retired="false" uuid="9eca4f4e-707f-4lb8-8289-2f9b6e93803f" />
<idgen_seq_id_gen id="2" first_identifier_base="1" next_sequence_value="2" base_character_set="0123456789" prefix="" min_length="0" max_length="0" />

<idgen_auto_generation_option id="15000" source="1" identifier_type="1" manual_entry_enabled="false" automatic_generation_enabled="true" uuid="eade77b6-3365-47ed-9ee3-2324598629eb" />

<relationship_type relationship_type_id="15001" a_is_to_b="Aunt" b_is_to_a="Niece" preferred="false" weight="0" description="A relationship of an aunt and her niece" creator="1" date_created="2007-05-04 00:00:00.0" retired="false" uuid="3982f469-cedc-4b2d-91ea-fe38f881e1a0"/>
Expand Down