Croway commented on PR #27314:
URL: https://github.com/apache/camel/pull/27314#issuecomment-5991022638
Since Hibernate 8 is still in beta and the runtimes won't adopt it soon,
I've reviewed the PR from a transaction point of view, and there are some
improvements and bug fixes that can be done now. That way, once it's merged,
the component is ready to work with Camel transactions
(`from(...).transacted().to("hibernate:...")`) without renaming options or
changing behaviour users already rely on.
Full transaction joining doesn't need to land in this PR; the description
already defers it. Below is what I measured, then what I'd ask for in this PR.
### What happens today
The producer and the consumer always do `openSession()` +
`beginTransaction()` + `commit()` per call, and never look at
`exchange.isTransacted()`. I wrote a probe test (Narayana + Agroal + H2 for
JTA, Spring 7.0.9 for the Spring cases):
| Setup | Result after the route fails past the hibernate step |
|---|---|
| `SpringTransactionPolicy` + `JpaTransactionManager` over the component's
`SessionFactory` | hibernate rows **survive** the rollback |
| `SpringTransactionPolicy` + `DataSourceTransactionManager`, JDBC insert
then hibernate insert | JDBC row rolled back, hibernate row **committed**
(split writes). A hibernate query in the same route can't see the JDBC row
written earlier in the same transaction |
| `JtaTransactionPolicy` (Narayana) + Agroal, component bootstrapped as
documented | **every exchange fails**: `TransactionException: Unable to commit
against JDBC Connection` / `Attempting to commit while taking part in a
transaction`. The DataSource is always registered as
`JAKARTA_NON_JTA_DATASOURCE`, so Hibernate commits a connection that Agroal has
enlisted in JTA |
| Same, plus `hibernate.transaction.coordinator_class=jta` and
`hibernate.transaction.jta.platform=Narayana` | **correct**: rollback removes
the rows; on success nothing is visible outside the route until the JTA commit |
The last row works only because of how Hibernate itself behaves. With a JTA
coordinator, `Transaction.begin()` skips the begin when a JTA transaction is
already active, and `commit()`/`rollback()` only commit, or mark rollback-only,
when Hibernate did not start the transaction. Nothing in the component
documents or tests this.
### What to settle in this PR
1. **Move session and transaction handling into one place.** Put session
acquisition and begin/commit/rollback behind a small strategy (like
`TransactionStrategy` in camel-jpa), with today's resource-local behaviour as
the default. Producer, consumer and the stateless path then call it instead of
`beginTransaction()`/`commit()` directly, and JTA and Spring become extra
implementations later, with no change to the endpoint options.
2. **Don't break JTA out of the box.** The default bootstrap fails on any
DataSource enlisted in JTA, which is the default on Quarkus. Either:
- add a `transactionType` component option now (`RESOURCE_LOCAL` default,
`JTA`). With `JTA` it registers the DataSource as `JAKARTA_JTA_DATASOURCE` and
sets the coordinator, plus the platform when one is given. The option name is
then fixed from the first release; or
- at least don't force `JAKARTA_NON_JTA_DATASOURCE` when
`hibernateProperties` set a JTA coordinator.
3. **Fail fast instead of splitting writes.** Until Spring joining exists, a
`transacted()` exchange that reaches the producer with a non-JTA
`SessionFactory` should throw a clear error rather than quietly commit on its
own. You can detect this with
`sessionFactory.unwrap(SessionFactoryImplementor.class).getServiceRegistry().requireService(TransactionCoordinatorBuilder.class).isJta()`.
Allowing it later is not a breaking change; silently committing now would be
one to change later.
4. **Don't commit or roll back a transaction the component didn't start.**
When the coordinator is JTA, use `sessionFactory.getCurrentSession()` (JTA
session context) when a transaction is active, and leave the outcome to the
`TransactedPolicy`. Store the session in `Exchange.TRANSACTION_CONTEXT_DATA`
(Splitter, Multicast and Aggregate already pass it to sub-exchanges) so several
hibernate steps in one exchange share a session. Close it on exchange
completion, not in a `finally` around each call.
5. **Reject `streaming=true` inside a transacted exchange.** The stream's
session and transaction would outlive the route's transaction.
6. **Consumer.** The poll currently runs the whole downstream route inside
the consumer's own private transaction. So rows locked with `SKIP_LOCKED`, and
a later `.transacted()`, commit independently of each other. Run the poll
through the strategy from point 1 so that later, a downstream `transacted()`
(propagation REQUIRED) joins the same transaction, as `JpaConsumer` does.
7. **Spring support optional, not a hard dependency.** Later, a Spring
implementation of the strategy can join
`JpaTransactionManager`/`HibernateTransactionManager`
(`EntityManagerFactoryUtils.getTransactionalEntityManager(sf).unwrap(Session.class)`)
and `DataSourceTransactionManager`
(`sf.withOptions().connection(DataSourceUtils.getConnection(ds))`, and
`withStatelessOptions()` for insert/upsert). Keeping it behind the interface
from point 1 with an optional `spring-orm` keeps the native path free of
Spring, as requested earlier in this review.
8. **Tests.** Add a JTA test with Narayana + Agroal (same test dependencies
as `camel-jta`) covering commit, rollback, and the "not visible before commit"
case. Add a test that pins down the behaviour of a transacted exchange with a
non-JTA `SessionFactory` (fail fast, per point 3).
9. **Docs.** Add a *Transactions* section to `hibernate-component.adoc`
stating today's behaviour: each operation commits on its own unless the
`SessionFactory` uses JTA. Then the intended setup per runtime:
- **Camel / Camel Spring Boot**: Spring transaction managers and Narayana
JTA.
- **Camel Quarkus**: Narayana JTA only, XA and non-XA. There the
`SessionFactory` would normally come from Quarkus's Hibernate ORM extension
through the registry, which is already JTA-managed.
10. **Hibernate version.** On Camel Spring Boot and Camel Quarkus, the
`SessionFactory` that joins transactions is normally the one the runtime
builds. Until they move to Hibernate 8, the component can't reuse it, so keep
the registry lookup and the component's own bootstrap independent of each other.
I can share the probe test if it helps; it is a single self-contained class.
_Claude Code on behalf of Croway. This comment was generated by an AI agent
and may contain inaccuracies. Please verify before applying._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
