#292 - Error loading locations on OpenMRS 2.7 (#293)

mseaton · web-flow · commit b5a6cabd5bcc · 2025-02-04T05:54:39.000-05:00
diff --git a/README.md b/README.md
@@ -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.
diff --git a/api-2.7/src/test/java/org/openmrs/module/initializer/api/loaders/GlobalPropertiesLoaderIntegration_2_7_Test.java b/api-2.7/src/test/java/org/openmrs/module/initializer/api/loaders/GlobalPropertiesLoaderIntegration_2_7_Test.java
@@ -18,9 +18,9 @@
 import org.springframework.beans.factory.annotation.Autowired;
 
 /**
- * This test is intended to be a copy of the GlobalPropertiesLoaderIntegrationTest from the api module,
- * but included within the api-2.7 module to test that global properties loading from xml works successfully
- * in an OpenMRS 2.7 environment
+ * This test is intended to be a copy of the GlobalPropertiesLoaderIntegrationTest from the api
+ * module, but included within the api-2.7 module to test that global properties loading from xml
+ * works successfully in an OpenMRS 2.7 environment
  */
 public class GlobalPropertiesLoaderIntegration_2_7_Test extends DomainBaseModuleContextSensitive_2_7_Test {
 	
diff --git a/api-2.7/src/test/java/org/openmrs/module/initializer/api/loaders/LocationsLoader_2_7_IntegrationTest.java b/api-2.7/src/test/java/org/openmrs/module/initializer/api/loaders/LocationsLoader_2_7_IntegrationTest.java
@@ -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));
+		}
+	}
+}
diff --git a/api-2.7/src/test/resources/testAppDataDir/configuration/locations/locations.csv b/api-2.7/src/test/resources/testAppDataDir/configuration/locations/locations.csv
@@ -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,,,,,,,,,,,,,,,,,,
diff --git a/api-2.7/src/test/resources/testAppDataDir/configuration/locationtags/locationtags.csv b/api-2.7/src/test/resources/testAppDataDir/configuration/locationtags/locationtags.csv
@@ -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
diff --git a/api/src/main/java/org/openmrs/module/initializer/api/BaseAttributeLineProcessor.java b/api/src/main/java/org/openmrs/module/initializer/api/BaseAttributeLineProcessor.java
@@ -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
@@ -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: '"
+					        + attributeTypeIdentifier + "'");
+				}
+				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;
 	}
diff --git a/api/src/test/resources/testdata/test-metadata.xml b/api/src/test/resources/testdata/test-metadata.xml
@@ -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"/>
@@ -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"/>
