Croway commented on PR #26068: URL: https://github.com/apache/camel/pull/26068#issuecomment-5523467348
Thanks for the fix, it is correct as is. A few suggestions before merging: **1. Consider field access instead of `@Transient`** (`KeyValueEntry.java`) The root cause is that the entity uses property access (annotations on getters) while the constructor, `isExpired()` and `toString()` read the fields directly. OpenJPA already warns about this on every run (`"KeyValueEntry" uses property access, but its field "expiresAt" is accessed directly in method "isExpired"`). Since the entity is `@since 4.23` and unreleased, moving `@Id`/`@Column`/`@Lob` onto the fields (as `Customer`/`VersionedItem`/`Address` already do) makes any future helper getter safe by construction, removes the need for `@Transient`, and silences the OpenJPA warning. I verified `JpaKeyValueRepositoryTest` passes 23/23 under both providers with field access and no `@Transient`. **2. Map `MessageProcessed` too** (`KeyValueEntryHibernateMappingTest.java`) `org.apache.camel.processor.idempotent.jpa.MessageProcessed` is the other property-access `@Entity` shipped in the jar, is auto-discovered by Quarkus the same way, and no CI job ever maps it with Hibernate. Adding `.addAnnotatedClass(MessageProcessed.class)` (or mapping every shipped entity) closes the same gap for it. **3. `persistence.xml` comment is now stale** (`src/test/resources/META-INF/persistence.xml`) With `hibernate-core` unconditionally on the test classpath, Hibernate is the first `PersistenceProvider` and claims any unit without `<provider>`. The PR pins the four unpinned units, but the header comment still says the element is "optional if OpenJPA is the only JPA provider". Removing the `keyvalueDb` pin fails all 23 key-value tests with `Unable to determine Dialect without JDBC metadata`. Suggest rewording the comment to say `<provider>` is required because hibernate-core is on the classpath. **4. The javaagent comment states the wrong cause** (`pom.xml`, openjpa profile) "with -Dhibernate the agent jar is not copied" is not accurate: the jar is copied by the `full` profile independently of `-Dhibernate` (`mvn -pl components/camel-jpa validate -Dhibernate` produces `target/openjpa-4.1.1.jar`). The fork actually died because `PCEnhancerAgent.premain` threw `NoClassDefFoundError: org/apache/xbean/asm9/ClassVisitor`, since the OpenJPA transitives live only in the `openjpa` profile. Moving the agent into the profile is the right fix; suggest the comment (and commit body) say "the agent needs the OpenJPA classpath" instead. _Claude Code on behalf of Croway_ -- 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]
