ramu11 commented on PR #27314:
URL: https://github.com/apache/camel/pull/27314#issuecomment-5998313552

   > Thanks for the continued work on this - several points are now addressed 
(per-exchange failure handling, header docs, stream lifetime, stateless tenant, 
docs listing, option labels). Some blockers remain:
   > 
   > 1. **Parent pom dependencyManagement (not addressed)**: `parent/pom.xml` 
still adds `jakarta.persistence-api`, `hibernate-core`, `spring-orm` and `h2`. 
These are not existing entries used by camel-jpa - they are added by this PR 
(camel-jpa declares its versions locally), `spring-orm` is no longer used, and 
managing them in the parent pins transitive versions for every module. Please 
declare the versions in the component pom only.
   > 2. **Pre-release dependencies**: Hibernate 8.0.0.Beta3 / 
jakarta.persistence 4.0.0-M7. We cannot release against beta/milestone 
artifacts; this needs to wait for GA (or target Hibernate 7).
   > 3. **Session shared via exchange property** (inline): passing the consumer 
`Session` as the `CamelHibernateSession` exchange property means
   >    
   >    * a producer with a different `sessionFactory`/`tenantIdentifier` 
silently uses the consumer session (wrong database/tenant),
   >    * the producer's `setDefaultReadOnly(...)` / `enableFilter(...)` mutate 
the consumer session for the rest of the batch,
   >    * exchange properties are copied to wireTap / seda / parallel 
split/multicast exchanges, so a non thread-safe Session can be used from other 
threads or after the poll closed it.
   >      Please scope reuse to the same SessionFactory and thread, and do not 
mutate a session the producer does not own. Also `HIBERNATE_SESSION` is 
documented as a consumer/producer header but used as an exchange property.
   > 4. **Consumer re-reads the same rows on every poll** (not 
addressed/answered): there is no mark-processed / delete / update step, unlike 
camel-jpa's `consumeDelete`/`@Consumed`.
   > 5. **Stateless insert/upsert** still open their own session, so the 
`skipLocked` self-deadlock remains possible there.
   > 6. **Tests**: the new skipLocked / streaming / session-reuse tests are 
Mockito-only; an H2-backed test proving the deadlock is gone would be valuable.
   > 7. **AI assistance**: the question about AI use is still unanswered - 
please confirm and add `Co-authored-by` trailers if applicable.
   > 
   > _Claude Code on behalf of davsclaus. This review was generated by an AI 
agent and may contain inaccuracies. Please verify all suggestions before 
applying. It does not replace specialized review tools or static analysis._
   
   ### Review items addressed
   
   * **Parent dependency management:** Removed Hibernate, Jakarta Persistence, 
H2, and Spring ORM dependency management from the parent; versions are now 
managed by `camel-hibernate`.
   * **H2 version:** Added the H2 version directly to `camel-hibernate/pom.xml`.
   * **Session exchange-property handling:** Replaced the shared `Hibernate 
Session` exchange property with `HibernateSessionContext` using Camel's 
`SafeCopyProperty`.
   * **Session scope safety:** Session reuse is validated against the 
`SessionFactory`, tenant, owning thread, and active state.
   * **Consumer row reprocessing:** Added `consumeDelete=true` by default, with 
`consumeDelete=false` available when rows should be retained.
   * **`skipLocked`:** Added a real H2-backed locking test to verify that 
locked rows are skipped without deadlocking.
   * **Stateless operations:** Kept stateless insert/upsert operations 
independently session-scoped and isolated from consumer session reuse.
   * **Reused consumer sessions:** Producer avoids modifying session-wide 
filters or default read-only state when reusing a consumer-owned session.
   * **Streaming:** Existing stream/session cleanup remains covered, including 
completion and failure handling.
   * **Generated metadata:** Regenerated endpoint config/URI factory/metadata 
for the new `consumeDelete` option.
   * **Formatting:** `git diff --check` passes.
   * **Tests:** `camel-hibernate` tests pass: **35/35, 0 failures, 0 errors**.
   
   **Review assisted by GPT-5.6.**
   


-- 
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]

Reply via email to