CAMEL-24130: camel-jpa - JpaMessageIdRepository should not reuse EntityManager from exchange header#24792
Conversation
…tyManager from exchange header Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Review: CAMEL-24130 — JpaMessageIdRepository should not reuse EntityManager from exchange header
Verdict: Approve ✅
The fix correctly changes usePassedInEntityManager from true to false in all four getTargetEntityManager() calls (add, contains, remove, clear).
Why this is correct:
JpaMessageIdRepository manages its own EntityManager lifecycle — each method creates an EM, uses it within transactionStrategy.executeInTransaction(), and closes it in the finally block. When usePassedInEntityManager=true, getTargetEntityManager() first checks the exchange header for CamelEntityManager, which the JPA consumer populates with its poll-scoped EM. The repository then closes that EM in its finally block, leaving the consumer with a closed EM for its subsequent flush/clear — causing IllegalStateException: EntityManager is closed.
The usePassedInEntityManager parameter was set to true when CAMEL-18450 refactored the repository to use getTargetEntityManager() (commit bbbb47efd8f3), but this was likely unintentional — the repository previously created its own EM directly via the factory and never reused one from the exchange. Setting it to false restores that self-contained lifecycle.
Notes:
- The remaining EM resolution path (exchange property → transaction context → shared EM → create new) still works correctly. The
allowRecreate=trueparameter ensures a closed EM from the exchange property is replaced with a fresh one. - Change is minimal and focused — 4 identical boolean flips across the 4 public methods.
Reviewed with Claude Code (claude-code/1.0.3) on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 9 tested, 29 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
2 participants
Summary
JpaMessageIdRepositoryhardcodesusePassedInEntityManager=truein allgetTargetEntityManager()calls, which causes it to grab theEntityManagerfrom the exchange'sCamelEntityManagerheader. When used downstream of a JPA consumer (which puts its own poll-scoped EM in that header), the repository then closes the consumer's EM in itsfinallyblock, causingIllegalStateException: EntityManager is closedduring the consumer's subsequent flush/clear.Fix
Changed
usePassedInEntityManagerfromtruetofalsein all fourgetTargetEntityManager()calls (add,contains,remove,clear). This makes the repository always create its own EM via its ownentityManagerFactory, matchingJpaEndpoint's default behavior. The existingfinallyclose logic is then correct — the repository only closes EMs it created itself.Claude Code on behalf of davsclaus