TRUNK-6516: Provide CDC mechanism with Debezium by rkorytkowski · Pull Request #6151 · openmrs/openmrs-core

rkorytkowski · 2026-06-02T09:51:24Z

Description of what I changed

Please see openmrs/openmrs-module-debezium@6fdf039 for a module that produces CDC events.

Issue I worked on

see https://issues.openmrs.org/browse/TRUNK-6516

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

rkorytkowski · 2026-06-02T09:54:44Z

 * @since 2.9.x
 */
-class BaseEvent extends ApplicationEvent implements OutboxableEvent {
+class BaseEvent implements OutboxableEvent {


According to Spring docs extending ApplicationEvent is no longer recommended or needed for events.

rkorytkowski · 2026-06-02T09:56:17Z

+ * 
+ * @since 2.9.x
+ */
+public abstract class BaseSessionEvent extends BaseEvent {


Moving sessionId to BaseSessionEvent since BaseEvent is now extended by CDCEvent, for which sessionId does not make any sense as it is published by an asynchronous task.

rkorytkowski · 2026-06-02T09:59:27Z

+
+	private boolean shapshot = false;
+
+	private Class<T> entityType;


It makes it easier to listen to events for certain entity types without a need for conditions in code.

rkorytkowski · 2026-06-05T09:02:06Z

-import org.springframework.test.context.TestExecutionListeners;
-import org.springframework.test.context.junit.jupiter.SpringExtension;
 import org.springframework.transaction.annotation.Transactional;
-import org.xml.sax.InputSource;


rkorytkowski · 2026-06-05T09:03:42Z

+
+	<jdbc:initialize-database data-source="dataSource" ignore-failures="DROPS">
+		<jdbc:script location="classpath:create_shedlock_table.sql"/>
+	</jdbc:initialize-database>


Fix for tests in modules with custom DB instead of in-memory one.

dkayiwa

Reviewed the full diff plus the surrounding event/outbox/test infrastructure, and verified behavior locally:

OutboxEventIT passes 4/4 on both the base branch and this PR's branch (run locally) — covering POJO events → PayloadApplicationEvent wrapping → generic @EventListener/@TransactionalEventListener(AFTER_COMMIT) matching → aggregator → outbox JSON round-trip → shedlock-backed polling. The new create_shedlock_table.sql initializer works, and is strictly more robust than the old inline creation (it also covers @SkipBaseSetup and JUnit-4 paths that previously missed the table).
Round-tripped the new CDCEvent through the outbox-configured ObjectMapper (works) and a plain ObjectMapper (fails — see inline comment on getResolvableType()).
Checked the referenced debezium module commit against this API (entityClass may be null for unmapped tables — getResolvableType() handles that correctly).

Overall the direction looks right: POJO events per current Spring guidance, sessionId separated into BaseSessionEvent so CDC events don't carry a meaningless session id, and the EventPublisher change is what makes publishing from background threads possible at all. The test-infra changes unblock module-supplied containers.

Beyond the inline comments:

Tests: the diff adds no core tests for the new API. Two cheap, high-value ones: typed-listener matching for CDCEvent<T> (the ResolvableTypeProvider dispatch is its main feature), and EventPublisher publishing a BaseSessionEvent with no session bound (the new catch branch is untested).
Follow-up ticket worth filing (pre-existing, surfaced while verifying): outbox serialization silently depends on JobRunrConfig mutating the shared ObjectMapper bean (JobRunr's JacksonJsonMapper constructor switches it to field visibility and activates @class default typing — which is also the only reason abstract List<EntityEvent<?>> elements deserialize at all). If that coupling ever changes, outbox serialization of ResolvableTypeProvider events breaks outright; and a context-wide mapper with default typing enabled is a deserialization-gadget hazard if any module reuses the bean on untrusted input. A dedicated, explicitly-configured outbox mapper would remove both risks.
45 files still carry @since 2.9.x after this PR fixes 5 — worth a one-shot sweep before 2.9.0 ships.
Trivia: OpenmrsPropertyConfig is a whitespace-only hunk; CDCTransactionStartedEvent/CDCTransactionCompletedEvent are identical and could share a small base.

I'd treat the shapshot typo as the one blocker while this API is still unreleased; everything else is should-fix or at your discretion.

🤖 Review prepared with Claude Code

dkayiwa · 2026-06-05T12:15:00Z

+ */
+public class CDCEvent<T> extends BaseEvent implements ResolvableTypeProvider {
+
+	private boolean shapshot = false;


Typo (the one thing I'd block on): shapshot → snapshot. This ships in 2.9.0 as public API (isShapshot()/setShapshot()), and since outbox serialization is field-based, the misspelling also lands in persisted payloads ({"shapshot":false,...} — verified locally). Renaming after release would be breaking; renaming now is free — code search shows openmrs-module-debezium doesn't reference it yet.

Related design question: the debezium module currently never calls setShapshot(...) (it maps Debezium op="r" to Operation.READ instead), so as merged the flag can never be true. Either wire it to payload.source.snapshot in the module or drop it from core.

dkayiwa · 2026-06-05T12:15:00Z

+	}
+
+	public enum Operation {
+		READ, CREATE, UPDATE, DELETE 


Debezium also emits op="t" (truncate) — the module's getOperation() currently maps that to a null operation. Worth deciding explicitly whether to add TRUNCATE here or document that truncates are ignored.

dkayiwa · 2026-06-05T12:15:00Z

+	}
+
+	@Override
+	public @Nullable ResolvableType getResolvableType() {


Suggest @JsonIgnore here (and on the pre-existing EntityEvent.getResolvableType()): serializing this class with a plain ObjectMapper fails with InvalidDefinitionException: Direct self-reference leading to cycle (...resolvableType->ResolvableType["componentType"]->...) — reproduced locally against this branch. It only works through the outbox today because JobRunrConfig's JacksonJsonMapper mutates the shared ObjectMapper bean to field-based visibility, so getters are never serialized. Since the class javadoc recommends forwarding events to message brokers, the first consumer to use a standard mapper will hit this.

Added @JsonIgnore to getResolvableType

dkayiwa · 2026-06-05T12:15:00Z

+ * blocked for too long, and it keeps up with the events queue.
+ * <p>
+ * If you combine {@link org.springframework.context.event.EventListener} with {@link org.springframework.scheduling.annotation.Async}
+ * you may run longer processing, but you loose the ability to re-try if anything fails, and in worst case you may lose the event.


Javadoc typos: "you loose the ability" → "lose"; and above: "not happening inside an application transaction rather are fetched" → "rather they are fetched".

dkayiwa · 2026-06-05T12:15:00Z

+ */
+public abstract class BaseSessionEvent extends BaseEvent {
+	private static final long serialVersionUID = 1L;
+


Consistency nit: BaseEvent is Serializable (via OutboxableEvent), but only this class declares serialVersionUID — BaseEvent, CDCEvent and the two CDCTransaction*Events don't. Add it everywhere or drop it here.

Also: double blank line before getSessionId(), and the (String sessionId, Set<String> tags) constructor has no callers (fine if intended as API).

dkayiwa · 2026-06-05T12:15:00Z

+				BaseSessionEvent sessionEvent = (BaseSessionEvent) event;
+				sessionEvent.setSessionId(sessionId);
+			} catch (HibernateException e) {
+				log.warn("No session bound to the current thread", e);


Minor: this warns with a full stack trace on every session-less publish — if a scheduled task routinely publishes entity events, it gets noisy. Consider log.debug, or warn without the throwable. Otherwise the catch itself is the right call — it's what lets session events be published from background threads.

dkayiwa · 2026-06-05T12:15:00Z

+	/**
+	 * @since 2.9.0
+	 */
+	public void ensureDatabaseRunning() {


This overridable method is invoked from the constructor, so a subclass override runs before subclass instance fields are initialized — it can only safely reference static state. That's the usual testcontainers idiom, but easy to trip over; suggest documenting the constraint in the javadoc. Also protected seems more fitting than public for an override hook.

dkayiwa · 2026-06-05T12:15:00Z

 		}
 	}

+	public static void ensureDatabaseRunning(JdbcDatabaseContainer<?> dbContainer, String database) {


Unlike the legacy paths, the custom container isn't recorded in this class's static state — so if a default-container test later runs in the same JVM, ensureDatabaseRunning() will start a second container and repoint the database* system properties mid-suite. Probably fine for homogeneous module suites, but worth a comment. (Also a couple of trailing-whitespace lines crept into this method.)

dkayiwa · 2026-06-05T12:15:00Z

+				System.setProperty("databaseName", dbContainer.getDatabaseName());
+				System.setProperty("databaseUsername", dbContainer.getUsername());
+				System.setProperty("databasePassword", dbContainer.getPassword());
+				if (database.equals("mariadb")) {


database.equals("mariadb") NPEs if database is null — prefer "mariadb".equals(database). And com.mysql.jdbc.Driver below is the deprecated alias (works on mysql-connector-java 8.0.30 but logs a deprecation notice) — com.mysql.cj.jdbc.Driver is the proper name.

dkayiwa · 2026-06-05T12:15:00Z

-public abstract class BaseModuleContextSensitiveTest extends BaseContextSensitiveTest {
+@Transactional
+@Rollback
+public abstract class BaseModuleContextSensitiveTest extends BaseModuleContextSensitiveNonTransactionalTest {


Compatibility note: this class no longer extends BaseContextSensitiveTest. Moving the implementation up to BaseContextSensitiveNonTransactionalTest is binary-compatible and @Transactional/@Rollback are re-applied, so behavior is preserved — but module test utilities doing instanceof BaseContextSensitiveTest (or typed against it) will need adjusting. Worth a release note.

rkorytkowski · 2026-06-08T07:18:47Z

@dkayiwa addressed all review comments

sonarqubecloud · 2026-06-08T07:46:54Z

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dkayiwa · 2026-06-08T10:11:52Z

+	}
+
+	public enum Operation {
+		READ, CREATE, UPDATE, DELETE, TRUNCATE 


What is TRUNCATE used for?

See https://debezium.io/documentation/reference/stable/connectors/mysql.html#mysql-truncate-events not useful to us at the moment, but added as suggested by AI for completeness.

dkayiwa · 2026-06-08T10:16:37Z

+ * 
+ * @since 2.9.0
+ */
+public class CDCTransactionCompletedEvent extends BaseEvent {


Do we also need one for transaction rolled back?

No CDC for roll back. The DB log contains only committed data.

Oh i see! 😊

rkorytkowski · 2026-06-08T11:32:10Z

@dkayiwa thanks for reviewing! As soon as this is merged I'll be able to issue a PR with post-commit review fixes for TRUNK-6429. Also have a pending PR for TRUNK-6515 that depends on this one :)

dkayiwa · 2026-06-08T11:43:49Z


 	public EntityEvent(T entity, Set<String> tags) {
-		super(entity, tags);
+		super(tags);


What do you think about the above sonar suggestions? 😊

I don't think we ever followed this, but fine with me to start doing that. Technically no difference.

dkayiwa · 2026-06-08T15:10:59Z

As per my initial review, this looks good. So i have merged it to unblock you. In the meantime, i will continue with testing it out by using it for events for the querystore/chartsearchai module. If i experience any issues, i will raise them here.

(cherry picked from commit 57f4018)

* TRUNK-6429: Create application events for service method calls and entity changes (#6084) (cherry picked from commit 144fee3) * TRUNK-6516: Provide CDC mechanism with Debezium (#6151) (cherry picked from commit 57f4018) * TRUNK-6429: Addressing post commit review (#6190) (cherry picked from commit a4331e2)

rkorytkowski requested review from dkayiwa, ibacher and mseaton June 2, 2026 09:53

rkorytkowski commented Jun 2, 2026

View reviewed changes

rkorytkowski force-pushed the TRUNK-6516 branch 5 times, most recently from 141a5d7 to d443cde Compare June 5, 2026 09:01

rkorytkowski commented Jun 5, 2026

View reviewed changes

dkayiwa reviewed Jun 5, 2026

View reviewed changes

rkorytkowski force-pushed the TRUNK-6516 branch from d443cde to cd249e9 Compare June 8, 2026 07:17

rkorytkowski force-pushed the TRUNK-6516 branch 2 times, most recently from 3020ad1 to 0440345 Compare June 8, 2026 07:41

TRUNK-6516: Provide CDC mechanism with Debezium

67fdf41

rkorytkowski force-pushed the TRUNK-6516 branch from 0440345 to 67fdf41 Compare June 8, 2026 07:44

dkayiwa reviewed Jun 8, 2026

View reviewed changes

dkayiwa merged commit 57f4018 into openmrs:2.9.x Jun 8, 2026
13 checks passed

dkayiwa mentioned this pull request Jun 16, 2026

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

Open

rkorytkowski added a commit to rkorytkowski/openmrs-core that referenced this pull request Jun 30, 2026

TRUNK-6516: Provide CDC mechanism with Debezium (openmrs#6151)

1428924

(cherry picked from commit 57f4018)

dkayiwa mentioned this pull request Jul 10, 2026

Events/CDC (TRUNK-6429/6515/6516) sign-off: 13 verified findings across core + artemis/camel/debezium #6289

Open


		private boolean shapshot = false;

		private Class<T> entityType;

Uh oh!

Conversation

rkorytkowski commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of what I changed

Issue I worked on

Checklist: I completed these to help reviewers :)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkorytkowski Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkayiwa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkorytkowski commented Jun 8, 2026

Uh oh!

sonarqubecloud Bot commented Jun 8, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkorytkowski commented Jun 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkayiwa commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

rkorytkowski commented Jun 2, 2026 •

edited

Loading

rkorytkowski Jun 2, 2026 •

edited

Loading

dkayiwa commented Jun 8, 2026 •

edited

Loading