ME-31: Support Address hierarchy

[Bawanthathilan]

Description of what I changed

• Added a new Address Hierarchy domain exporter
• addressConfiguration.xml - rebuilt from the ordered hierarchy levels and the live address template, then serialized via the module's own AddressConfigurationLoader.writeToString(...) so the XML
  matches the import format.

Issue I worked on

see https://openmrs.atlassian.net/browse/ME-31

Checklist: I completed these to help reviewers :)

• My IDE is configured to follow the code style of this project.

  No? Unsure? -> configure your IDE, format the code and add the changes with git add . && git commit --amend

• I have added tests to cover my changes. (If you refactored
  existing code that was well tested you do not have to add tests)

  No? -> write tests and add them to this commit git add . && git commit --amend

• I ran mvn clean package right before creating this pull request and
  added all formatting changes to my commit.

  No? -> execute above command

• All new and existing tests passed.

  No? -> figure out why and add the fix to your commit. It is your responsibility to make sure your code works.

• My pull request is based on the latest changes of the master branch.

  No? Unsure? -> execute command git pull --rebase upstream master

[wikumChamith]

What happens if getSizeMappings or getElementDefaults returns null?

[github-actions]

getElementDefaults() returns null on a stock install, so line 124 is the one that goes first. openmrs-core's DEFAULT_ADDRESS_TEMPLATE (OpenmrsConstants, 2.8.0) seeds layout.address.format with <nameMappings>, <sizeMappings> and <lineByLineFormat> and no <elementDefaults> element at all, and LayoutTemplate never initializes the field, so elementDefaults.get(token) throws on the very first level.

That is not an exotic state: it is every database whose address template GP has not been rewritten by an addressConfiguration.xml load, which includes any site that set its levels up through the addresshierarchy admin pages or via importAddressHierarchyFile. Those sites have levels and entries, so the domain is selected, and the export job then dies with an NPE having written neither file.

Extending the guard you already have for a null template to the two maps individually (template.getSizeMappings() == null ? Collections.emptyMap() : template.getSizeMappings(), same for element defaults) covers it, and a test passing a non-null AddressTemplate with unset maps would pin it, which the current HashMap-based test cannot.

[Bawanthathilan]

What happens if getSizeMappings or getElementDefaults returns null?

the guard only checked template == null, but getSizeMappings() . getElementDefaults() can themselves return null. ill update this PR @wikumChamith

[wikumChamith]

Won't the NPE be on levels without an address field?

[github-actions]

Yes, and it lands on this line rather than on field.getName(), which is already null-guarded on line 118. token goes into the map as null, and sizeMappings is whatever came out of the GP: the core default declares <sizeMappings class="properties">, and XStream aliases properties to java.util.Properties, which extends Hashtable. Hashtable.get(null) throws NPE instead of returning null (I checked on Java 17). The unit test misses it because it builds a HashMap, which swallows a null key happily.

Levels with no address field are reachable rather than theoretical: AddressHierarchyServiceImpl.addAddressHierarchyLevel() creates one with addressField left unset, and AddressHierarchyImportUtil calls it for every CSV column beyond the configured levels, so any hierarchy grown that way has them.

Worth knowing before picking the fix: a plain null check on the lookup would get you past the export but leave a <field>-less <addressComponent> in the file, and that blows up on the way back in. AddressConfiguration.getAddressTemplate() calls c.getField().getName() unconditionally for every component, and isMatchableLevelConfig looks each level up by field. So skipping a level that has no address field, with a warning, produces something loadable where guarding the map lookup alone does not.

[Bawanthathilan]

Won't the NPE be on levels without an address field?

already gurded

[dkayiwa]

@claude review

[github-actions]

A bare ^ will not survive the round trip. The addresshierarchy module hands this delimiter straight to String.split, so it is a Java regex, and ^ there is the zero-width start anchor rather than a literal caret. I ran it on Java 17: "Cambodia^00".split("^") returns a single element, ["Cambodia^00"], so AddressHierarchyImportUtil.splitIntoNameAndUserGeneratedId (2.17.0) never separates the id out.

If merged as-is, re-importing an export taken from any database that populates user_generated_id renames every entry it touches: what should come back as Cambodia with id 00 becomes an entry literally named Cambodia^00, with user_generated_id left null. Nothing throws on either side, so neither the export nor the Iniz load reports anything wrong.

It is the module's own default, which is presumably where it came from, but the sibling default is broken the same way ("a|b".split("|") gives ["a", "|", "b"]), so those two defaults look like they have simply never been exercised. The configs in the wild all avoid the bare caret: openmrs/openmrs-content-referenceapplication-demo, mekomsolutions/openmrs-config-haiti and mekomsolutions/ozone-distro-cambodia all pair <entryDelimiter>,</entryDelimiter> with <identifierDelimiter>%</identifierDelimiter> (the demo's own addresshierarchy.csv next to that file reads Cambodia%00,Banteay Meanchey%01,Mongkol Borei%0101,...), and PIH/openmrs-config-zl escapes rather than swaps, \^ and \|. I would go with %, since it matches the reference application demo and sits naturally alongside the comma you already chose for entries.

Suggested change

	public static final String IDENTIFIER_DELIMITER = "^";
	public static final String IDENTIFIER_DELIMITER = "%";

buildEntriesCsv_appendsUserGeneratedIdWithIdentifierDelimiter pins the current character, so it needs the same edit. Asserting that a produced cell splits back into two parts on IDENTIFIER_DELIMITER would keep a future change to it honest.

[Bawanthathilan]

yes. good catch. this file use "% this " https://github.com/openmrs/openmrs-content-referenceapplication-demo/blob/main/configuration/backend_configuration/addresshierarchy/addresshierarchy.csv

[github-actions]

These check that the generated string looks right, not that the addresshierarchy module can read it back, which is the property that actually matters for an exporter whose only job is producing input for that module's loader. AddressConfigurationLoader.readFromString is the exact inverse of the writeToString you call on line 139 and it needs no OpenMRS context (it just builds a plain XStream), so the stronger check is cheap:

AddressConfiguration parsed = AddressConfigurationLoader.readFromString(xml);
AddressComponent first = parsed.getAddressComponents().get(0);
assertEquals(AddressField.COUNTRY, first.getField());
assertEquals(40, first.getSizeMapping());
assertEquals("addresshierarchy.csv", parsed.getAddressHierarchyFile().getFilename());

Your call whether it is worth the churn. The reason I would take it is that it keeps holding when an addresshierarchy bump renames a field or changes an XStream alias, where ten contains checks would still pass on nine of them and the tenth failure would not tell you the file had stopped being loadable.