Upgrade build to Java 21 #26
Conversation
WalkthroughProject modernized for Java 21 and Jakarta Persistence: build tooling and dependencies updated; local Changes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (11)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGeneralGenericDao.java (2)
49-66: Pagination without ORDER BY is nondeterministic; add stable ordering.Offset/limit with no order can return inconsistent slices across runs. Prefer ordering by the entity’s single-id attribute when present.
Apply:
CriteriaQuery<T> criteriaQuery = criteriaBuilder.createQuery(persistentClass); Root<T> root = criteriaQuery.from(persistentClass); - criteriaQuery.select(root); + criteriaQuery.select(root); + // Ensure deterministic pagination + var entityType = session.getSessionFactory().getMetamodel().entity(persistentClass); + if (entityType.hasSingleIdAttribute()) { + var idAttr = entityType.getId(entityType.getIdType().getJavaType()); + criteriaQuery.orderBy(criteriaBuilder.asc(root.get(idAttr.getName()))); + }And add imports if needed:
+import jakarta.persistence.metamodel.EntityType; +import jakarta.persistence.metamodel.SingularAttribute;
82-103: Narrow exception scope and add light batch flush/clear.Catching HibernateException is too broad; use TransientObjectException for the transient-id path. Also flush/clear periodically for large sets to limit memory.
Apply:
- execute(session -> { - for (T t : entities) { + execute(session -> { + int i = 0; + for (T t : entities) { if (t == null) { continue; } boolean persisted = false; try { Object identifier = session.getIdentifier(t); if (identifier == null) { session.persist(t); persisted = true; } - } catch (HibernateException e) { + } catch (org.hibernate.TransientObjectException e) { session.persist(t); persisted = true; } if (!persisted) { session.merge(t); } + if (++i % 50 == 0) { + session.flush(); + session.clear(); + } } return null; });src/main/java/org/sakaiproject/genericdao/hibernate/HibernateSessionFactorySupport.java (1)
32-38: currentSession requires an active, thread-bound sessiongetCurrentSession() throws if no transactional session is bound. If callers may use this DAO outside managed transactions, consider SessionFactoryUtils.getSession(getSessionFactory(), false) or document the requirement clearly.
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (3)
94-118: Support nested property paths (e.g., "user.name")root.get(property) fails for dotted paths. Add a resolver that walks segments; use it for predicates and sorting.
Apply this helper and replacements:
@@ - private Expression<String> asString(Path<?> path) { + private <T> Path<?> resolvePath(Root<T> root, String property) { + Path<?> p = root; + for (String part : property.split("\\.")) { + p = p.get(part); + } + return p; + } + + private Expression<String> asString(Path<?> path) { return path.as(String.class); } @@ - predicates.add(criteriaBuilder.equal(root.get(property), value)); + predicates.add(criteriaBuilder.equal(resolvePath(root, property), value)); @@ - predicates.add(buildComparablePredicate(criteriaBuilder, root.get(property), true, value)); + predicates.add(buildComparablePredicate(criteriaBuilder, resolvePath(root, property), true, value)); @@ - predicates.add(buildComparablePredicate(criteriaBuilder, root.get(property), false, value)); + predicates.add(buildComparablePredicate(criteriaBuilder, resolvePath(root, property), false, value)); @@ - predicates.add(criteriaBuilder.like(asString(root.get(property)), value.toString())); + predicates.add(criteriaBuilder.like(asString(resolvePath(root, property)), value.toString())); @@ - predicates.add(criteriaBuilder.isNull(root.get(property))); + predicates.add(criteriaBuilder.isNull(resolvePath(root, property))); @@ - predicates.add(criteriaBuilder.isNotNull(root.get(property))); + predicates.add(criteriaBuilder.isNotNull(resolvePath(root, property))); @@ - predicates.add(criteriaBuilder.notEqual(root.get(property), value)); + predicates.add(criteriaBuilder.notEqual(resolvePath(root, property), value)); @@ - Path<Object> path = root.get(order.property); + Path<?> path = resolvePath(root, order.property);Caveat: collection-valued joins still need Join handling; if your app uses one-to-many sorting/filtering (e.g., "roles.name"), we should extend resolvePath to join collection attributes. Want a follow-up patch?
Also applies to: 151-166
104-106: LIKE semantics: wildcards and case sensitivityCurrent code requires callers to supply wildcards. Consider normalizing to contains-match (wrap with %value%) and/or case-insensitive variants via lower() to reduce surprises.
240-248: Avoid mutating Search in baseFindOneBySearch (optional)Setting search.setLimit(1) mutates caller input. Consider copying or using a local Search to avoid side effects.
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGenericDao.java (5)
121-121: Count via execute: LGTM; consider null-safe single result.Optionally use
getSingleResultOrNull()(JPA 3.1) and default to 0 for symmetry with the overload below.
309-333: Metamodel-based id property: OK; remove raw types.Works for simple ids; returns null for embedded/composite. Prefer generics to drop raw casts:
- @SuppressWarnings("rawtypes") - Set idAttributes = entityType.getIdClassAttributes(); - if (idAttributes != null && idAttributes.size() == 1) { - return ((SingularAttribute) idAttributes.iterator().next()).getName(); - } + Set<SingularAttribute<?, ?>> idAttributes = entityType.getIdClassAttributes(); + if (idAttributes != null && idAttributes.size() == 1) { + return idAttributes.iterator().next().getName(); + }Consider documenting behavior for
@EmbeddedId/@IdClass.
343-357: Avoid relying on PersistenceUnitUtil via SessionFactory.Safer to use the Hibernate Session API you already depend on. This also avoids ambiguity about
getSessionFactory()type.Suggested change:
- PersistenceUnitUtil persistenceUnitUtil = getSessionFactory().getPersistenceUnitUtil(); - Object identifier; - try { - identifier = persistenceUnitUtil.getIdentifier(object); - } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Could not get identifier for this object, it may not be persistent: " + object, e); - } + Object identifier = execute(session -> { + try { + return session.getIdentifier(object); + } catch (HibernateException e) { + throw new IllegalArgumentException("Could not get identifier for this object, it may not be persistent: " + object, e); + } + });Keep the serializable checks as-is.
372-391: Create flow: id retrieval may be null before flush.For some generators (IDENTITY/SEQUENCE), id might not be available until flush. Optionally flush if
newIdis null:- Object newId = session.getIdentifier(object); + Object newId = session.getIdentifier(object); + if (newId == null) { + session.flush(); + newId = session.getIdentifier(object); + }Or tolerate null and document.
165-169: Minor: HQL uses “order by”, not “sort by”.
buildCountHQLsearches for"sort by". This likely never matches; use"order by".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
README.md(1 hunks)pom.xml(3 hunks)src/main/java/javax/persistence/Column.java(0 hunks)src/main/java/javax/persistence/Entity.java(0 hunks)src/main/java/javax/persistence/Id.java(0 hunks)src/main/java/javax/persistence/Table.java(0 hunks)src/main/java/javax/persistence/Transient.java(0 hunks)src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentColumnName.java(1 hunks)src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentId.java(1 hunks)src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentTableName.java(1 hunks)src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentTransient.java(1 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java(3 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java(2 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGeneralGenericDao.java(4 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGenericDao.java(8 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateSessionFactorySupport.java(1 hunks)src/test/resources/database.properties(1 hunks)src/test/resources/hibernate/GenericTestObject.hbm.xml(1 hunks)src/test/resources/hibernate/GenericTestParentObject.hbm.xml(1 hunks)src/test/resources/spring-hibernate.xml(1 hunks)
💤 Files with no reviewable changes (5)
- src/main/java/javax/persistence/Transient.java
- src/main/java/javax/persistence/Id.java
- src/main/java/javax/persistence/Entity.java
- src/main/java/javax/persistence/Table.java
- src/main/java/javax/persistence/Column.java
✅ Files skipped from review due to trivial changes (2)
- README.md
- src/test/resources/hibernate/GenericTestObject.hbm.xml
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGeneralGenericDao.java (2)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
SuppressWarnings(40-96)src/test/java/org/sakaiproject/genericdao/hibernate/CompleteGenericDaoTest.java (1)
SuppressWarnings(35-355)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (2)
src/main/java/org/sakaiproject/genericdao/api/search/Restriction.java (1)
Restriction(28-207)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
SuppressWarnings(40-96)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGenericDao.java (1)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
SuppressWarnings(40-96)
🔇 Additional comments (22)
src/test/resources/database.properties (1)
13-13: LGTM! Hibernate 6.x configuration aligned.The addition of
hibernate.transform_hbm_xml.enabled=trueis appropriate for Hibernate 6.x to support legacy HBM XML mappings during the migration period.src/test/resources/spring-hibernate.xml (1)
16-25: LGTM! Property placeholder wiring is correct.The addition of the
hibernate.transform_hbm_xml.enabledplaceholder correctly wires the property fromdatabase.propertiesto support Hibernate 6.x legacy mapping transformations.src/test/resources/hibernate/GenericTestParentObject.hbm.xml (1)
4-4: LGTM! DTD URL updated to official Hibernate source.The DOCTYPE system URL correctly points to
www.hibernate.org, reflecting the Hibernate project's current hosting.pom.xml (3)
164-168: LGTM! Java 21 enforcement aligned with PR objectives.The enforcer plugin correctly requires Java 21+, matching the stated PR goal.
176-183: LGTM! Compiler release set to Java 21.The compiler plugin correctly targets Java 21 release, consistent with the enforcer rules.
92-107: Verify new dependency declarations.The migration to
jakarta.persistence-api(provided scope) andorg.hibernate.orm:hibernate-corewith Spring ORM is architecturally sound. Confirm that the provided scope for Jakarta Persistence API is appropriate for your deployment environment (i.e., the container supplies it).src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentTransient.java (1)
23-23: LGTM! Jakarta Persistence namespace migration complete.The import correctly migrates from
javax.persistence.Transienttojakarta.persistence.Transient, aligning with Jakarta EE 9+ and the overall PR migration strategy.src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentId.java (1)
23-23: Jakarta import swap looks good.Import now aligns with jakarta.persistence.Id; no functional changes.
src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentTableName.java (2)
23-23: Jakarta import swap looks good.Import now aligns with jakarta.persistence.Table; no behavioral changes.
36-41: Verify annotation usages before expanding @target
Thergsearch returned no occurrences of@PersistentTableNamein the codebase; please confirm there are no direct or reflective uses (including in external modules) before addingElementType.TYPEto the@Target.src/main/java/org/sakaiproject/genericdao/api/annotations/PersistentColumnName.java (1)
23-23: Jakarta import swap looks good.Import now aligns with jakarta.persistence.Column; no functional changes.
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGeneralGenericDao.java (2)
24-27: Imports: OK for Criteria API and Hibernate Query.These support the Criteria-based refactor and typed queries.
118-125: Delete batch via session.remove: LGTM.Loads, de-dupes, and removes in-session; return value matches removed count.
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
69-76: Example semantics changed vs legacy Hibernate Example; confirm intentIncluding primitives at default values (0/false) and empty strings will now filter on them. Legacy Example often excluded default/empty values. If unintended, add exclusions for empty strings and primitive defaults.
Do you want to preserve legacy Example semantics (exclude zero/false/empty string), or is strict equality on all non-null values desired?
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (3)
175-183: toInt pagination clamp is solidSaturating to [0, Integer.MAX_VALUE] avoids overflow and negatives. LGTM.
191-205: Criteria API migration looks correctcountBySearch/findBySearch build and execute criteria with optional where, sorting, and pagination. Good use of execute(...) wrapper.
Also applies to: 210-235
416-484: Backward-compat methods delegate through Search correctlyThe deprecated findByProperties/countByProperties paths are preserved and reimplemented through Search—good for compatibility.
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGenericDao.java (5)
23-23: Jakarta imports: looks right; verify dependency alignment.Good move to
jakarta.persistence.*. Please confirm runtime is on Hibernate 6.x and JPA 3.1+ to avoid package mismatch at runtime. Mixingorg.hibernate.query.Querywith JPA metamodel is fine under Hibernate 6.Also applies to: 40-43
55-55: Confirm HibernateSessionFactorySupport contract.Code now calls both
getCurrentSession()andgetMetamodel()/getPersistenceUnitUtil()viagetSessionFactory(). Ensure this returns an object exposing both surfaces (or proxies appropriately), or compilation/runtime will fail.
364-364: findById via execute: LGTM.
397-401: Update via merge: LGTM.
410-413: Delete via remove: LGTM.
| <properties> | ||
| <hibernate.version>6.5.2.Final</hibernate.version> | ||
| <spring.version>6.1.6</spring.version> | ||
| <jakarta.persistence.version>3.1.0</jakarta.persistence.version> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| </properties> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify dependency version compatibility and security.
The major version upgrades (Hibernate 5.6→6.5, Spring 5.3→6.1, Jakarta Persistence 3.1.0) represent significant API changes. Ensure compatibility between these versions and check for security advisories.
Additionally, search for official compatibility documentation:
🏁 Script executed:
#!/bin/bash
# Verify Hibernate 6.5.2.Final and Spring 6.1.6 compatibility and check for advisories
echo "=== Checking Hibernate compatibility with Spring 6.1.6 ==="
curl -s "https://docs.spring.io/spring-framework/docs/6.1.6/javadoc-api/" | grep -i hibernate || echo "Unable to verify from Spring docs"
echo -e "\n=== Checking for security advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "org.hibernate.orm:hibernate-core") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
echo -e "\n=== Checking Spring Framework advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "org.springframework:spring-core") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Length of output: 3346
🌐 Web query:
Hibernate 6.5.2.Final Spring 6.1.6 compatibility
💡 Result:
Short answer: Yes — but only via JPA. Spring Framework 6.1.x supports Hibernate 6 as a JPA provider (use HibernateJpaVendorAdapter / EntityManagerFactory). Spring’s native Hibernate-specific support (the orm.hibernate5 package, LocalSessionFactoryBuilder, HibernateTemplate, etc.) is intended for Hibernate 5.x and should not be relied on with Hibernate 6.x. Hibernate 6.5.2.Final itself targets Jakarta Persistence 3.1 / Jakarta EE 10, which is compatible with the Spring 6/Jakarta-based platform. [1][2][3]
What to do in practice
- Use JPA configuration (LocalContainerEntityManagerFactoryBean / HibernateJpaVendorAdapter or Spring Boot’s JPA auto-config). [2]
- Do not expect Spring’s old native SessionFactory helpers (orm.hibernate5 package) to work reliably with Hibernate 6 — migrate code to JPA (EntityManager) or adapt to provider-specific APIs through the JPA extension points. [2][5]
- Verify your Jakarta API versions and dependencies (Hibernate 6.5.2.Final uses jakarta.persistence 3.1). [4]
References
- Spring Framework ORM guide (notes about Hibernate 6 support via JPA). [1]
- HibernateJpaVendorAdapter Javadoc — lists compatibility with Hibernate 6.x. [2]
- Spring Framework GitHub discussion (explanation that Hibernate 6.x is supported only via JPA). [5]
- Hibernate 6.5.2.Final / hibernate-core metadata (release info). [4]
Sources
[1] https://docs.spring.io/spring-framework/reference/data-access/orm/hibernate.html
[2] https://docs.spring.io/spring-framework/docs/6.1.x/javadoc-api/org/springframework/orm/jpa/vendor/HibernateJpaVendorAdapter.html
[3] spring-projects/spring-framework#30288
[4] https://mvnrepository.com/artifact/org.hibernate.orm/hibernate-core/6.5.2.Final
🌐 Web query:
Jakarta Persistence 3.1.0 Hibernate 6.5 compatibility
💡 Result:
Short answer: Yes — Hibernate ORM 6.5 is compatible with Jakarta Persistence 3.1. See the Hibernate 6.5 compatibility section and the Jakarta Persistence 3.1 spec page. [1][2]
References:
[1] Hibernate ORM 6.5 — Compatibility (Jakarta Persistence: 3.1).
[2] Jakarta Persistence 3.1 specification — list of compatible implementations (includes Hibernate ORM).
Upgrade Spring and migrate Hibernate integration
- In pom.xml, bump
<spring.version>to ≥ 6.2.11 to remediate the high-severity annotation detection vulnerability (affects 6.0.0–6.1.22). - Replace any use of
org.springframework.orm.hibernate5(LocalSessionFactoryBean, HibernateTemplate, etc.) with JPA integration viaLocalContainerEntityManagerFactoryBean/HibernateJpaVendorAdapterfor Hibernate 6.5.2.Final. - Hibernate 6.5.2.Final + Jakarta Persistence 3.1.0 compatibility is confirmed; no advisories found for Hibernate ORM.
🤖 Prompt for AI Agents
In pom.xml around lines 12 to 17, the spring.version must be bumped to at least
6.2.11 and the project must stop using Spring's Hibernate 5 integration; update
the <spring.version> property to 6.2.11 (or later) and remove/replace any
imports/usages of org.springframework.orm.hibernate5 (LocalSessionFactoryBean,
HibernateTemplate, etc.). Replace those integrations with JPA-based
configuration: add/adjust a LocalContainerEntityManagerFactoryBean configured
with a HibernateJpaVendorAdapter, set the appropriate JPA properties for
Hibernate 6.5.2.Final and Jakarta Persistence 3.1.0, and move
session/transaction code to use EntityManager / Spring's JpaTransactionManager.
Ensure pom dependencies include spring-jdbc/spring-orm (if needed) and the
JPA/Hibernate artifacts compatible with Jakarta namespaces, and update spring
configuration classes/beans and any DAO code to use EntityManager APIs instead
of HibernateTemplate/SessionFactory.
| if (property == null || value == null) { | ||
| throw new IllegalArgumentException("restrictions property and value cannot be null or empty"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: NULL/NOT_NULL restrictions are rejected
This throws when value == null, breaking valid cases like new Restriction("prop", null, Restriction.NULL/NOT_NULL).
Apply this fix:
- if (property == null || value == null) {
- throw new IllegalArgumentException("restrictions property and value cannot be null or empty");
- }
+ if (property == null) {
+ throw new IllegalArgumentException("restriction property cannot be null");
+ }
+ int cmp = restriction.comparison;
+ if (cmp != Restriction.NULL && cmp != Restriction.NOT_NULL && value == null) {
+ throw new IllegalArgumentException("restriction value cannot be null for comparison: " + cmp);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (property == null || value == null) { | |
| throw new IllegalArgumentException("restrictions property and value cannot be null or empty"); | |
| } | |
| if (property == null) { | |
| throw new IllegalArgumentException("restriction property cannot be null"); | |
| } | |
| int cmp = restriction.comparison; | |
| if (cmp != Restriction.NULL && cmp != Restriction.NOT_NULL && value == null) { | |
| throw new IllegalArgumentException("restriction value cannot be null for comparison: " + cmp); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java
around lines 63-66, the current guard throws when value == null which
incorrectly rejects valid Restriction instances that use NULL or NOT_NULL
operators; modify the validation so it still rejects a null/empty property but
only throws for a null value when the restriction operator is not NULL or
NOT_NULL (i.e., allow null values when operator == NULL or operator ==
NOT_NULL); update the conditional to check the restriction/operator before
throwing so NULL/NOT_NULL cases pass through.
| if (arrayValues != null) { | ||
| if (arrayValues.length == 0) { | ||
| continue; | ||
| } | ||
| if (value.getClass().isArray()) { | ||
| // special handling for "in" type comparisons | ||
| Object[] objectArray = (Object[]) value; | ||
| if (objectArray.length == 1) { | ||
| value = objectArray[0]; | ||
| } else if (objectArray.length > 1) { | ||
| if (Restriction.NOT_EQUALS == search.getRestrictions()[i].comparison) { | ||
| junction.add( Restrictions.not( Restrictions.in(property, objectArray) ) ); | ||
| } else { | ||
| junction.add(Restrictions.in(property, objectArray)); | ||
| } | ||
| } else { | ||
| // do nothing for now, this is slightly invalid but not worth dying over | ||
| if (arrayValues.length == 1) { | ||
| value = arrayValues[0]; | ||
| } else { | ||
| In<Object> inClause = criteriaBuilder.in(root.get(property)); | ||
| for (Object element : arrayValues) { | ||
| inClause.value(element); | ||
| } | ||
| } | ||
|
|
||
| if (! value.getClass().isArray()) { | ||
| switch (search.getRestrictions()[i].comparison) { | ||
| case Restriction.EQUALS: | ||
| junction.add(Restrictions.eq(property, value)); | ||
| break; | ||
| case Restriction.GREATER: | ||
| junction.add(Restrictions.gt(property, value)); | ||
| break; | ||
| case Restriction.LESS: | ||
| junction.add(Restrictions.lt(property, value)); | ||
| break; | ||
| case Restriction.LIKE: | ||
| junction.add(Restrictions.like(property, value)); | ||
| break; | ||
| case Restriction.NULL: | ||
| junction.add(Restrictions.isNull( property )); | ||
| break; | ||
| case Restriction.NOT_NULL: | ||
| junction.add(Restrictions.isNotNull( property )); | ||
| break; | ||
| case Restriction.NOT_EQUALS: | ||
| junction.add(Restrictions.ne(property, value)); | ||
| break; | ||
| if (restriction.comparison == Restriction.NOT_EQUALS) { | ||
| predicates.add(criteriaBuilder.not(inClause)); | ||
| } else { | ||
| predicates.add(inClause); | ||
| } | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Array/collection handling: sanitize nulls and limit IN/NOT IN to EQUALS/NOT_EQUALS
- IN/NOT IN should only apply to EQUALS/NOT_EQUALS; currently it overrides other comparisons.
- Null elements inside the array/collection will error in Criteria in-clause.
Apply this refactor:
- if (arrayValues != null) {
+ if (arrayValues != null) {
if (arrayValues.length == 0) {
continue;
}
- if (arrayValues.length == 1) {
- value = arrayValues[0];
- } else {
- In<Object> inClause = criteriaBuilder.in(root.get(property));
- for (Object element : arrayValues) {
- inClause.value(element);
- }
- if (restriction.comparison == Restriction.NOT_EQUALS) {
- predicates.add(criteriaBuilder.not(inClause));
- } else {
- predicates.add(inClause);
- }
- continue;
- }
+ // remove nulls
+ arrayValues = java.util.Arrays.stream(arrayValues).filter(java.util.Objects::nonNull).toArray(Object[]::new);
+ if (arrayValues.length == 0) { continue; }
+ if (arrayValues.length == 1) {
+ value = arrayValues[0];
+ } else {
+ if (restriction.comparison != Restriction.EQUALS && restriction.comparison != Restriction.NOT_EQUALS) {
+ throw new IllegalArgumentException("Arrays/collections supported only for EQUALS/NOT_EQUALS");
+ }
+ In<Object> inClause = criteriaBuilder.in(resolvePath(root, property));
+ for (Object element : arrayValues) {
+ inClause.value(element);
+ }
+ predicates.add(restriction.comparison == Restriction.NOT_EQUALS
+ ? criteriaBuilder.not(inClause)
+ : inClause);
+ continue;
+ }
}Note: adds imports java.util.Arrays and java.util.Objects.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (arrayValues != null) { | |
| if (arrayValues.length == 0) { | |
| continue; | |
| } | |
| if (value.getClass().isArray()) { | |
| // special handling for "in" type comparisons | |
| Object[] objectArray = (Object[]) value; | |
| if (objectArray.length == 1) { | |
| value = objectArray[0]; | |
| } else if (objectArray.length > 1) { | |
| if (Restriction.NOT_EQUALS == search.getRestrictions()[i].comparison) { | |
| junction.add( Restrictions.not( Restrictions.in(property, objectArray) ) ); | |
| } else { | |
| junction.add(Restrictions.in(property, objectArray)); | |
| } | |
| } else { | |
| // do nothing for now, this is slightly invalid but not worth dying over | |
| if (arrayValues.length == 1) { | |
| value = arrayValues[0]; | |
| } else { | |
| In<Object> inClause = criteriaBuilder.in(root.get(property)); | |
| for (Object element : arrayValues) { | |
| inClause.value(element); | |
| } | |
| } | |
| if (! value.getClass().isArray()) { | |
| switch (search.getRestrictions()[i].comparison) { | |
| case Restriction.EQUALS: | |
| junction.add(Restrictions.eq(property, value)); | |
| break; | |
| case Restriction.GREATER: | |
| junction.add(Restrictions.gt(property, value)); | |
| break; | |
| case Restriction.LESS: | |
| junction.add(Restrictions.lt(property, value)); | |
| break; | |
| case Restriction.LIKE: | |
| junction.add(Restrictions.like(property, value)); | |
| break; | |
| case Restriction.NULL: | |
| junction.add(Restrictions.isNull( property )); | |
| break; | |
| case Restriction.NOT_NULL: | |
| junction.add(Restrictions.isNotNull( property )); | |
| break; | |
| case Restriction.NOT_EQUALS: | |
| junction.add(Restrictions.ne(property, value)); | |
| break; | |
| if (restriction.comparison == Restriction.NOT_EQUALS) { | |
| predicates.add(criteriaBuilder.not(inClause)); | |
| } else { | |
| predicates.add(inClause); | |
| } | |
| continue; | |
| } | |
| if (arrayValues != null) { | |
| if (arrayValues.length == 0) { | |
| continue; | |
| } | |
| // remove nulls | |
| arrayValues = java.util.Arrays.stream(arrayValues) | |
| .filter(java.util.Objects::nonNull) | |
| .toArray(Object[]::new); | |
| if (arrayValues.length == 0) { | |
| continue; | |
| } | |
| if (arrayValues.length == 1) { | |
| value = arrayValues[0]; | |
| } else { | |
| if (restriction.comparison != Restriction.EQUALS | |
| && restriction.comparison != Restriction.NOT_EQUALS) { | |
| throw new IllegalArgumentException( | |
| "Arrays/collections supported only for EQUALS/NOT_EQUALS"); | |
| } | |
| In<Object> inClause = criteriaBuilder.in(resolvePath(root, property)); | |
| for (Object element : arrayValues) { | |
| inClause.value(element); | |
| } | |
| predicates.add( | |
| restriction.comparison == Restriction.NOT_EQUALS | |
| ? criteriaBuilder.not(inClause) | |
| : inClause); | |
| continue; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java
around lines 74 to 91, the current handling builds IN/NOT IN for any comparison
and passes null elements to Criteria in-clause; change logic so IN/NOT IN is
only used when restriction.comparison is EQUALS or NOT_EQUALS, and before
building the In clause filter the array/collection to remove nulls and ignore
empty results (skip when no non-null values, or fall back to a different
predicate for single-value cases). Add imports java.util.Arrays and
java.util.Objects as needed. Ensure behavior: if array/collection has no
non-null elements, continue (or produce appropriate predicate), if one non-null
element treat as single-value comparison, otherwise build In<Object> with only
non-null elements and wrap with criteriaBuilder.not(...) when comparison is
NOT_EQUALS.
| List<Predicate> predicates = new ArrayList<Predicate>(); | ||
| try { | ||
| PropertyDescriptor[] descriptors = Introspector.getBeanInfo(persistentClass).getPropertyDescriptors(); | ||
| for (PropertyDescriptor descriptor : descriptors) { | ||
| if (descriptor.getReadMethod() == null || "class".equals(descriptor.getName())) { | ||
| continue; | ||
| } | ||
| Object value = descriptor.getReadMethod().invoke(exampleObject); | ||
| if (value != null) { | ||
| predicates.add(criteriaBuilder.equal(root.get(descriptor.getName()), value)); | ||
| } | ||
| } | ||
| } catch (IntrospectionException | IllegalAccessException | InvocationTargetException e) { | ||
| throw new IllegalStateException("Unable to evaluate example object", e); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Guard against unmapped/collection properties in example introspection
Introspecting all bean properties and blindly calling root.get(name) will throw at runtime for unmapped getters or collection-valued attributes. Filter to mapped singular attributes via the JPA metamodel and skip collections.
Apply this diff to constrain evaluated properties:
@@
- List<Predicate> predicates = new ArrayList<Predicate>();
+ List<Predicate> predicates = new ArrayList<>();
+ // Only consider mapped, non-collection attributes
+ var entityType = session.getMetamodel().entity(targetClass);
+ var mappedAttrNames = entityType.getAttributes().stream()
+ .filter(a -> !a.isCollection())
+ .map(jakarta.persistence.metamodel.Attribute::getName)
+ .collect(java.util.stream.Collectors.toSet());
try {
PropertyDescriptor[] descriptors = Introspector.getBeanInfo(persistentClass).getPropertyDescriptors();
for (PropertyDescriptor descriptor : descriptors) {
- if (descriptor.getReadMethod() == null || "class".equals(descriptor.getName())) {
+ String name = descriptor.getName();
+ if (descriptor.getReadMethod() == null || "class".equals(name) || !mappedAttrNames.contains(name)) {
continue;
}
Object value = descriptor.getReadMethod().invoke(exampleObject);
if (value != null) {
- predicates.add(criteriaBuilder.equal(root.get(descriptor.getName()), value));
+ predicates.add(criteriaBuilder.equal(root.get(name), value));
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java
around lines 65 to 79, the introspection loop currently calls
root.get(propertyName) for every bean property which will fail for unmapped or
collection-valued attributes; replace that behavior by consulting the JPA
Metamodel (entityManager.getMetamodel()) to obtain the EntityType for
persistentClass and only evaluate properties that are mapped as
SingularAttribute (skip Collection/Plural attributes and unmapped names), and
skip any attribute lookups that throw IllegalArgumentException; use the
metamodel check before invoking root.get(...) so only mapped singular attributes
are turned into predicates and continue on others.
| return execute(session -> { | ||
| Query<Number> query = session.createQuery(newHqlQuery, Number.class); | ||
| if (params != null) { | ||
| for (int i = 0; i < params.length; i++) { | ||
| query.setParameter(i, params[i]); | ||
| } | ||
| } | ||
| Number result = query.getSingleResult(); | ||
| return result == null ? 0 : result.intValue(); | ||
| }); |
There was a problem hiding this comment.
Positional parameters: likely off-by-one under JPA/Hibernate 6.
JPQL/HQL positional params are 1-based (?1, ?2, …). Current loop uses 0-based indices and the comment still references bare ?, which is not valid JPQL. Recommend switching callers to named or ?1-style params and adjust indexing.
Proposed minimal fix (assuming ?1-style in HQL):
- for (int i = 0; i < params.length; i++) {
- query.setParameter(i, params[i]);
- }
+ for (int i = 0; i < params.length; i++) {
+ query.setParameter(i + 1, params[i]);
+ }Also update any HQL using bare ? to ?1, ?2, … or use named parameters.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return execute(session -> { | |
| Query<Number> query = session.createQuery(newHqlQuery, Number.class); | |
| if (params != null) { | |
| for (int i = 0; i < params.length; i++) { | |
| query.setParameter(i, params[i]); | |
| } | |
| } | |
| Number result = query.getSingleResult(); | |
| return result == null ? 0 : result.intValue(); | |
| }); | |
| return execute(session -> { | |
| Query<Number> query = session.createQuery(newHqlQuery, Number.class); | |
| if (params != null) { | |
| for (int i = 0; i < params.length; i++) { | |
| query.setParameter(i + 1, params[i]); | |
| } | |
| } | |
| Number result = query.getSingleResult(); | |
| return result == null ? 0 : result.intValue(); | |
| }); |
🤖 Prompt for AI Agents
In src/main/java/org/sakaiproject/genericdao/hibernate/HibernateGenericDao.java
around lines 134 to 143, the code sets positional parameters using 0-based
indices while JPA/Hibernate 6 requires 1-based positional parameters (e.g. ?1,
?2) and bare '?' is invalid; update the loop to call query.setParameter(i + 1,
params[i]) so positions start at 1, and revise any HQL using bare '?' to use ?1,
?2… or, preferably, switch callers to named parameters and bind by name instead.
| import org.hibernate.SessionFactory; | ||
| import org.springframework.beans.factory.InitializingBean; | ||
| import org.springframework.dao.DataAccessException; | ||
| import org.springframework.orm.hibernate5.SessionFactoryUtils; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Spring/Hibernate compatibility for SessionFactoryUtils package
Import is org.springframework.orm.hibernate5.SessionFactoryUtils. Ensure this matches your Spring ORM module for the Hibernate version in the POM (Spring 6 + Hibernate 6 support differs by module).
Run this to inspect declared versions:
Also applies to: 45-46
🏁 Script executed:
#!/bin/bash
rg -nP '<artifactId>(spring-orm|hibernate-core)</artifactId>.*\n\s*<version>.*</version>' -C2 pom.xmlLength of output: 0
🏁 Script executed:
#!/bin/bash
find . -name "pom.xml" -print0 | xargs -0 grep -nE '<artifactId>(spring-orm|hibernate-core)</artifactId>'Length of output: 130
🏁 Script executed:
#!/bin/bash
find . -name "pom.xml" -print0 | xargs -0 grep -nC2 '<artifactId>spring-orm</artifactId>'
find . -name "pom.xml" -print0 | xargs -0 grep -nC2 '<artifactId>hibernate-core</artifactId>'Length of output: 525
🏁 Script executed:
#!/bin/bash
find . -name "pom.xml" -print0 | xargs -0 grep -nE '<spring.version>|<hibernate.version>'Length of output: 127
Update import to Hibernate 6 support package
Replace
import org.springframework.orm.hibernate5.SessionFactoryUtils;with
import org.springframework.orm.hibernate6.SessionFactoryUtils;to align Spring ORM 6.1.6 (spring.version) with Hibernate 6.5.2.Final (hibernate.version).
🤖 Prompt for AI Agents
In
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateSessionFactorySupport.java
around line 10, the class imports Spring's Hibernate 5 SessionFactoryUtils but
the project uses Spring ORM 6.x with Hibernate 6.x; replace the import
org.springframework.orm.hibernate5.SessionFactoryUtils with
org.springframework.orm.hibernate6.SessionFactoryUtils so the correct Spring ORM
package for Hibernate 6 is used and then rebuild to ensure no other API changes
are required.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pom.xml (1)
14-15: Upgrade Spring Framework to a patched release.
spring.version6.1.6 is affected by multiple published vulnerabilities, including CVE-2025-41249 (annotation detection bypass) and CVE-2025-22233 (DataBinder bypass). The Spring team’s advisory directs users to upgrade to 6.2.11 (OSS) to remediate CVE-2025-41249 and to 6.1.20+/6.2.7+ for CVE-2025-22233; remaining on 6.1.6 leaves known security holes.(spring.io)- <spring.version>6.1.6</spring.version> + <spring.version>6.2.11</spring.version>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml(1 hunks)AGENTS.md(1 hunks)README.md(1 hunks)pom.xml(3 hunks)src/test/resources/database.properties(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/resources/database.properties
|
|
||
| on: | ||
| push: | ||
| branches: [ main ] |
There was a problem hiding this comment.
Fix branch filter to cover master pushes.
The repo’s default branch is master (this PR targets it), so pushes there will skip CI with the current branches: [ main ] filter. Update the trigger to include master so release pushes actually run tests.
push:
- branches: [ main ]
+ branches: [ master ]
+ # keep main if you plan to switch defaults
+ # branches: [ master, main ]🤖 Prompt for AI Agents
.github/workflows/ci.yml around line 5: the workflow currently only triggers on
pushes to the "main" branch, but this repository uses "master" as the default
branch so pushes there skip CI; update the push branch filter to include
"master" (for example change branches: [ main ] to include master as well or use
a pattern that covers both) so that pushes to master trigger the workflow.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
67-87: Skip unmapped/collection properties infindByExamplepredicatesThe introspection loop still feeds every bean property into
root.get(name). For entities with@Transientgetters or collection-valued associations that default to non-null (very common), this call throwsIllegalArgumentException, sofindByExamplenow hard-fails for legitimate entities. Please filter the descriptors against the JPA metamodel and ignore plural attributes before building predicates. (Same issue noted previously.)- List<Predicate> predicates = new ArrayList<Predicate>(); + var entityType = session.getMetamodel().entity(targetClass); + var mappedAttrNames = entityType.getAttributes().stream() + .filter(a -> !a.isCollection()) + .map(jakarta.persistence.metamodel.Attribute::getName) + .collect(java.util.stream.Collectors.toSet()); + List<Predicate> predicates = new ArrayList<>(); try { PropertyDescriptor[] descriptors = Introspector.getBeanInfo(persistentClass).getPropertyDescriptors(); for (PropertyDescriptor descriptor : descriptors) { - if (descriptor.getReadMethod() == null || "class".equals(descriptor.getName())) { + String name = descriptor.getName(); + if (descriptor.getReadMethod() == null || "class".equals(name) || !mappedAttrNames.contains(name)) { continue; } Object value = descriptor.getReadMethod().invoke(exampleObject); if (value != null) { - predicates.add(criteriaBuilder.equal(root.get(descriptor.getName()), value)); + predicates.add(criteriaBuilder.equal(root.get(name), value)); }src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (2)
63-65: Critical bug: NULL/NOT_NULL restrictions incorrectly rejected.This validation throws when
value == null, which breaks validRestriction.NULLandRestriction.NOT_NULLqueries that don't require a value parameter.Apply this fix:
- if (property == null || value == null) { - throw new IllegalArgumentException("restrictions property and value cannot be null or empty"); - } + if (property == null) { + throw new IllegalArgumentException("restriction property cannot be null"); + } + int cmp = restriction.comparison; + if (cmp != Restriction.NULL && cmp != Restriction.NOT_NULL && value == null) { + throw new IllegalArgumentException("restriction value cannot be null for comparison: " + cmp); + }
74-92: Major issue: Array/collection handling needs sanitization and operator restrictions.Current implementation has two problems:
- Null elements in arrays/collections will cause runtime errors in JPA Criteria
Inclause.- IN/NOT IN logic applies to any comparison operator, but should only apply to EQUALS/NOT_EQUALS.
Apply this refactor:
if (arrayValues != null) { if (arrayValues.length == 0) { continue; } + // remove nulls + arrayValues = java.util.Arrays.stream(arrayValues).filter(java.util.Objects::nonNull).toArray(Object[]::new); + if (arrayValues.length == 0) { continue; } if (arrayValues.length == 1) { value = arrayValues[0]; } else { + if (restriction.comparison != Restriction.EQUALS && restriction.comparison != Restriction.NOT_EQUALS) { + throw new IllegalArgumentException("Arrays/collections supported only for EQUALS/NOT_EQUALS"); + } In<Object> inClause = criteriaBuilder.in(root.get(property)); for (Object element : arrayValues) { inClause.value(element); } - if (restriction.comparison == Restriction.NOT_EQUALS) { - predicates.add(criteriaBuilder.not(inClause)); - } else { - predicates.add(inClause); - } + predicates.add(restriction.comparison == Restriction.NOT_EQUALS + ? criteriaBuilder.not(inClause) + : inClause); continue; } }Note: Requires imports
java.util.Arraysandjava.util.Objects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/sakaiproject/genericdao/api/CompleteGenericDao.java(1 hunks)src/main/java/org/sakaiproject/genericdao/api/finders/ByExampleFinder.java(3 hunks)src/main/java/org/sakaiproject/genericdao/api/finders/ByPropsFinder.java(8 hunks)src/main/java/org/sakaiproject/genericdao/base/BaseBasicGenericDao.java(7 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java(10 hunks)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java(3 hunks)src/main/java/org/sakaiproject/genericdao/springjdbc/JdbcBasicGenericDao.java(7 hunks)src/main/java/org/sakaiproject/genericdao/springjdbc/JdbcGeneralGenericDao.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/org/sakaiproject/genericdao/api/finders/ByExampleFinder.java (1)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
SuppressWarnings(40-98)
src/main/java/org/sakaiproject/genericdao/api/finders/ByPropsFinder.java (2)
src/main/java/org/sakaiproject/genericdao/api/search/Restriction.java (1)
Restriction(28-207)src/main/java/org/sakaiproject/genericdao/base/BaseBasicGenericDao.java (1)
SuppressWarnings(33-306)
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (3)
src/main/java/org/sakaiproject/genericdao/api/search/Restriction.java (1)
Restriction(28-207)src/main/java/org/sakaiproject/genericdao/base/BaseBasicGenericDao.java (1)
SuppressWarnings(33-306)src/main/java/org/sakaiproject/genericdao/hibernate/HibernateCompleteGenericDao.java (1)
SuppressWarnings(40-98)
🔇 Additional comments (10)
src/main/java/org/sakaiproject/genericdao/base/BaseBasicGenericDao.java (1)
200-274: LGTM: Deprecation annotations added correctly.The
@Deprecatedannotations properly formalize the existing javadoc@deprecatedcomments, making the deprecation compiler-visible and consistent with the broader API evolution towardSearch-based query methods.src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java (5)
19-30: Jakarta Persistence migration looks correct.The imports properly use
jakarta.persistence.criteria.*APIs andorg.hibernate.query.Query, aligning with the Java 21 and Hibernate 7.x upgrade objectives.
137-149: LGTM: Helper methods are well-implemented.
buildComparablePredicatecorrectly enforcesComparableconstraint and handles GREATER/LESS comparisons.applySortingproperly translates customOrderobjects to JPAOrderobjects with correct ascending/descending logic.toIntprovides safe bounds checking for pagination parameters.Also applies to: 151-183
191-205: LGTM: Count query migration to JPA Criteria API is correct.The refactored
baseCountBySearchproperly uses JPA Criteria API withCriteriaQuery<Long>andcriteriaBuilder.count(root). The session-based execution pattern aligns with Hibernate 7.x best practices.
210-235: LGTM: Find query migration to JPA Criteria API is well-structured.The refactored
baseFindBySearchcorrectly:
- Constructs a typed
CriteriaQuery<T>with proper select and where clauses.- Applies sorting via
applySorting.- Handles pagination safely using
toInt()conversion andsetFirstResult/setMaxResults.
387-461: LGTM: Deprecation annotations are consistent.The
@Deprecatedannotations properly match existing javadoc comments, completing the API deprecation pattern consistently across allcountByPropertiesandfindByPropertiesoverloads.src/main/java/org/sakaiproject/genericdao/springjdbc/JdbcBasicGenericDao.java (1)
301-375: LGTM: Deprecation annotations complete the API evolution.The
@Deprecatedannotations properly complement the existing javadoc@deprecatedcomments across all property-based finder method overloads, maintaining consistency with the codebase-wide migration toSearch-based APIs.src/main/java/org/sakaiproject/genericdao/api/finders/ByPropsFinder.java (3)
30-30: LGTM: Interface deprecation signals API migration.Marking the entire
ByPropsFinderinterface as@Deprecatedcorrectly formalizes the documented migration path toBySearchFinderandSearch-based query methods.
36-80: LGTM: Constant deprecations guide users to canonical definitions.The
@Deprecatedannotations on comparison and sort constants correctly redirect developers to the canonical constants inRestrictionand to usingOrderobjects, eliminating duplication in the API surface.
100-261: LGTM: Method deprecations complete the API surface evolution.All
findByPropertiesandcountByPropertiesmethod signatures are correctly marked@Deprecatedwith clear migration guidance toBySearchFindermethods, completing the coordinated deprecation across the API and implementation layers.
| private Expression<String> asString(Path<?> path) { | ||
| return path.as(String.class); | ||
| } |
There was a problem hiding this comment.
Potential runtime issue: Type safety in string conversions.
asString performs an unchecked cast to String.class which will fail at runtime if the path doesn't point to a String property. This could cause ClassCastException when LIKE comparisons are applied to non-string fields.
Consider validating the property type or wrapping in a try-catch to provide a clearer error message:
private Expression<String> asString(Path<?> path) {
- return path.as(String.class);
+ try {
+ return path.as(String.class);
+ } catch (ClassCastException e) {
+ throw new IllegalArgumentException("LIKE comparison requires string property, got: " + path.getJavaType(), e);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Expression<String> asString(Path<?> path) { | |
| return path.as(String.class); | |
| } | |
| private Expression<String> asString(Path<?> path) { | |
| try { | |
| return path.as(String.class); | |
| } catch (ClassCastException e) { | |
| throw new IllegalArgumentException( | |
| "LIKE comparison requires string property, got: " + path.getJavaType(), e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/sakaiproject/genericdao/hibernate/HibernateBasicGenericDao.java
around lines 133 to 135, the asString method does an unchecked cast to String
which can throw ClassCastException at runtime; update it to first inspect the
Path's Java type (path.getJavaType()) and if it is String (or assignable from
String) return path.as(String.class), otherwise either perform a safe SQL cast
using the CriteriaBuilder function API (e.g. criteriaBuilder.function("cast",
String.class, path) or equivalent available in your environment) or throw an
IllegalArgumentException with a clear message describing the field type and that
LIKE operations require string conversion; ensure the method uses the
CriteriaBuilder instance rather than blind casting and includes a descriptive
error when conversion is not possible.
Summary
Testing
mvn testhttps://chatgpt.com/codex/tasks/task_e_68dd511382988328bc9232b416abec29
Summary by CodeRabbit
Documentation
New Features
Chores
Upgrades
Tests
Deprecations