Fix duplicate patient creation on MPI import

[mherman22]

Summary

When importing a patient from the MPI (OpenCR) into iSantePlus, two local patient records are created instead of one. This happens because:

1. importMpiPatient() → savePatient() triggers PatientSynchronizationAdvice AOP
2. The AOP spawns PatientUpdateWorker which re-queries the MPI
3. The worker discovers golden record seealso links and triggers a second importMpiPatient() call
4. The second import creates a duplicate patient with a new iSantePlus ID

Changes

• PatientSynchronizationAdvice: Add ThreadLocal<Boolean> SUPPRESS flag — when set, the AOP advice skips spawning PatientUpdateWorker, breaking the re-entrant import loop
• FhirMpiClientServiceImpl.importPatient(): Implement the method (was returning null), with duplicate detection via matchWithExistingPatient() and AOP suppression during savePatient()
• HL7MpiClientServiceImpl.importPatient(): Add the same AOP suppression to prevent duplicates on the HL7 code path
• PatientUpdateWorker: Add dedup lock (HashSet) to prevent concurrent exports of the same patient (matching existing PatientSyncWorker pattern)
• PatientUpdateWorker + PatientSyncWorker: Add proxy privileges to prevent APIAuthenticationException in background threads
• MpiClientService / MpiClientWorker / MpiClientServiceImpl: Remove duplicate getPatientList() method definitions and fix parseFhirPatient() call signature

Test plan

• Import a patient from the MPI into one iSantePlus instance — verify only ONE local record is created
• Verify the imported patient is correctly exported back to OpenCR
• Import the same patient again — verify it updates the existing record instead of creating a new one
• Verify PatientUpdateWorker still runs normally for non-import savePatient() calls (e.g. editing a patient in the UI)
• Verify cross-reference identifiers are still resolved after patient creation
• Check OpenHIM transactions — no duplicate POST to /CR/fhir/Patient during import

[ibacher]

This is slightly too clever. You actually want:

Suggested change

	hasId |= eid.getIdentifier().equals(id.getIdentifier())
	hasId = hasId || eid.getIdentifier().equals(id.getIdentifier())

The |= is a bitwise or and assignment operator similar to +=. The thing is that while the bytecode for hasId |= ... is somewhat more efficient byte-code wise, it will always evaluate both operands whereas hasId = hasId || ... will short-circuit, i.e., once hasId has been set to true it will no longer invoke the eid.getIdentifier().equals(id.getIdentifier()) part.

To demonstrate, a simplified version based on:

boolean first = true;
boolean second = true;

first |= second;

Results in something like this:

1 iload_1  // load the first "true"
2 iload_2 // load the second "true"
3 ior         // bitwise "or" of 1 and 2

Whereas:

boolean first = true;
boolean second = true;

first = first || second;

Is much more verbose, but in a key way:

1 iload_1   // load the first "true"
2 ifne 5    // if the first value != 0 jump to line 5
3 iload_2 // load the second "true"
4 ifeq 7    // if the second value = 0 jump to line 7
5 iconst_1 // the value 1, i.e., false
6 return
7 iconst_0 // the value 0, i.e., true
8 return

Which is more bytecodes but in the real example, every where we have iload_2 in the dummy code, we'd actually have something like:

1 aload_0         // load eid
2 invokevirtual // run getIdentifier()
3 astore_2       // save the result to register 2
4 aload_1         // load id
5 invokevirtual // run getIdentifier(), result on stack
6 aload_2        // load result of first identifier call
7 invokevirtual // call equals()

In the second case we can skip those steps. And if none of that makes any sense, the short version is || short-circuits, i.e., only evaluates the left-side if the left-side is true, but | does not, i.e., it always evaluates both sides.

[mherman22]

Nice, thanks! this was just copied from

openmrs-module-mpi-client/api/src/main/java/org/openmrs/module/santedb/mpiclient/api/impl/HL7MpiClientServiceImpl.java

Line 393 in ef4a6bc

public Patient importPatient(MpiPatient patient) throws MpiClientException

so perhaps a rewrite of that is required. Bringing this method here is somewhat not the fix. I noticed registration-core module has https://github.com/IsantePlus/openmrs-module-registrationcore/blob/5af0ddfc383379b0d42de352b9643e0e6aab6a0c/api/src/main/java/org/openmrs/module/registrationcore/api/impl/RegistrationCoreServiceImpl.java#L417-L487 and i am still trying to figure out these two calls and how they complement each other.

For now i will revert this!

[ibacher]

IIRC, checkout is currently on v6 or something like that...

[ibacher]

Maybe use mvn install:install-file here to install everest-core? This will ensure that the Maven metadata is also updated.

[ibacher]

Is this value set anywhere?

[mherman22]

IsantePlus/openmrs-module-registrationcore#49

[ibacher]

I see... Ok... Weird, but ok

[ibacher]

I'd wrap these in try...catch blocks to ignore any exceptions...

[ibacher]

Probably same above.

[ibacher]

I might make this lock removal the first thing we do in the finally block since it's the least likely to fail.

[ibacher]

Instead of this, we can use the Maven repo https://oscarmcmaster.sourceforge.net/m2/ to find Everest-Core. However, it's weird to do this in settings instead of just... adding a repo in the POM