Non-proxied / non-pattern service mutators bypass #6084 *ServiceEvents (transferEncounter, stopVisits, removeAllergy, setAllergies, …) — systemic gap

[dkayiwa]

Summary

The OpenmrsServiceEventAdvice from #6084 emits *ServiceEvents only for public service methods whose name matches save*/create*/void*/unvoid*/retire*/unretire*/purge* and that are invoked through the Spring proxy. A class of service operations mutates indexed clinical data while defeating both conditions, so they emit no event and event-driven consumers (read stores, caches, analytics, FHIR projections) silently go stale. This issue collects the confirmed instances and proposes a systemic fix; the individual high-impact cases are filed separately (#6194, #6195, #6196, #6197) and are instances of the same root pattern.

Environment

• OpenMRS Platform 2.9.0-SNAPSHOT (bytecode verified; behaviour cross-checked on referenceapplication-standalone 3.7.0-SNAPSHOT).
• Found while building a CQRS read store on the TRUNK-6429: Create application events for service method calls and en… #6084 events. The audit below covers every core service that mutates a type the read store indexes (Obs, Condition, Diagnosis, Encounter, Visit, Patient, Allergy, Orders, PatientProgram, MedicationDispense).

The two ways a mutation escapes #6084

1. Non-pattern method name — merge*, discontinue*, transfer*, remove*, bulk stop*, set* are never matched by the advice pointcut.
2. Self-invocation / direct-DAO / private *Internal — even a pattern-named write bypasses the proxy when a service mutates a record via this.x(...), dao.x(...), or a private xInternal(...) rather than the proxied public service method.

Both defeat the advice because it depends on AspectJ interception of an externally-called, proxied public method.

Confirmed instances (bytecode-verified, 2.9.0-SNAPSHOT)

Operation	Why no event	Indexed state left stale
EncounterService.transferEncounter	non-pattern; internally voidEncounter(...) and saveEncounter(...) are self-invoked (invokevirtual this, not Context.getEncounterService())	encounter's patient — the encounter stays under the wrong patient and the transfer is invisible
VisitService.stopVisits (bulk close; the scheduled "close stale visits" task)	writes via VisitDAO.saveVisit(...) directly	stopDatetime / active flag — closed visits still appear active
PatientService.removeAllergy	self-invoked voidAllergy(...)	allergy not removed downstream (stale)
PatientService.setAllergies (bulk replace)	PatientDAO.saveAllergies(...) + self-invoked voidAllergy(...)	allergy set stale
PatientService.mergePatients	non-pattern; conditions/allergies/dispenses not reassigned	see #6194 / #6195
OrderService.discontinueOrder / order stop	private stopOrder(...) → saveOrderInternal(...)	dateStopped / active — see #6197

Contrast (works correctly): VisitService.endVisit sets stopDatetime then calls Context.getVisitService().saveVisit(...) through the proxy, so it does fire a SaveServiceEvent. This is the pattern the others should follow.

OrderService.updateOrderFulfillerStatus also bypasses the advice but mutates a field most consumers don't track, so it is lower priority.

Suggested systemic fix

Rather than patching each method, address the structural cause so future mutators don't silently regress:

• Option A — route internal mutations through the proxied service. Have services call Context.getXService().saveX(...) / voidX(...) (as endVisit already does) instead of this.xInternal(...) / dao.x(...), and add the missing pattern-named methods (or have the advice recognise merge*/discontinue*/transfer*). Lowest-risk, but easy to regress again.
• Option B — emit below the proxy. Publish entity events from a layer that can't be bypassed by call-path — e.g. a Hibernate Interceptor/PostInsert/PostUpdate listener, or the CDC pipeline (TRUNK-6516: Provide CDC mechanism with Debezium #6151). This makes coverage immune to self-invocation and method naming, at the cost of losing the typed service-operation semantics the current advice provides.

A hybrid (advice for typed service events + a DAO/CDC backstop for completeness) may be the most robust.

Scope boundary

Even with all of the above fixed, direct-database writes that bypass the service layer entirely — Liquibase changesets that rewrite indexed columns, raw SQL, bulk admin scripts — can never emit a *ServiceEvent. Those are catchable only by the CDC path (#6151 + the debezium producer). #6084 events should be understood as covering service-layer writes only.

Related

• mergePatients does not reassign Conditions, Allergies, or MedicationDispenses to the surviving patient #6194, No dedicated patient-merge event; consumers must rely on the incidental SaveServiceEvent<PersonMergeLog> #6195 — mergePatients reassignment + no merge event
• purgePatient fails with FK violations when patient has Conditions (purge cascade incomplete for newer clinical tables) #6196 — incomplete purge cascade for newer tables
• Order stop/discontinue does not fire a *ServiceEvent for the stopped order (event-driven consumers see it as still active) #6197 — order stop/discontinue not evented

[rkorytkowski]

The fix should be the same as for other issues with service method names that do not follow the save.../void.../... pattern. We should publish events manually at the beginning of such methods.

[priyanshuvishwakarma273403]

"Hi, I'd like to work on this issue. Is anyone currently working on it?"

[Tarekchehahde]

PR: #6295 — fixes the confirmed stopVisits instance by routing through proxied saveVisit (same as endVisit). Added StopVisitsServiceEventIT; full suite not run locally.