Copilot commented on code in PR #15731:
URL: https://github.com/apache/grails-core/pull/15731#discussion_r3398332363
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/HibernateGormStaticApi.groovy:
##########
@@ -353,22 +354,42 @@ class HibernateGormStaticApi<D> extends GormStaticApi<D> {
if (!queryMap) return null
Map coercedMap = queryMap.collectEntries { k, v -> [k.toString(), v] }
String hql = buildWhereHql(coercedMap)
- doSingleInternal(hql, coercedMap, [], args, false)
+ doSingleInternal(hql, buildWhereParams(coercedMap), [],
buildFindWhereArgs(args), false)
}
@Override
List<D> findAllWhere(Map queryMap, Map args) {
if (!queryMap) return null
Map coercedMap = queryMap.collectEntries { k, v -> [k.toString(), v] }
String hql = buildWhereHql(coercedMap)
- doListInternal(hql, coercedMap, [], args, false)
+ doListInternal(hql, buildWhereParams(coercedMap), [], args, false)
}
private String buildWhereHql(Map queryMap) {
- String whereClause = queryMap.keySet().collect { Object key -> "$key =
:$key" }.join(' and ')
+ String whereClause = queryMap.collect { Object key, Object value ->
+ String propertyName = validateWherePropertyName(key.toString())
+ value == null ? "$propertyName is null" : "$propertyName =
:$propertyName"
+ }.join(' and ')
return "from ${persistentEntity.name} where $whereClause"
}
+ private String validateWherePropertyName(String propertyName) {
+ if (persistentEntity.getPropertyByName(propertyName) == null) {
+ throw new IllegalArgumentException("Property [$propertyName] is
not a valid property of ${persistentEntity.name}")
+ }
+ return propertyName
+ }
Review Comment:
`validateWherePropertyName` uses `persistentEntity.getPropertyByName(...)`,
which can resolve mapped `targetName` values (for example custom column names)
via `AbstractPersistentEntity`’s `mappedPropertiesByName`. That means
non-property keys may be treated as “valid” and then interpolated into HQL,
contradicting the intent to only allow domain property names and potentially
producing invalid HQL at runtime.
##########
grails-test-examples/hyphenated/grails-app/conf/application.yml:
##########
@@ -62,9 +62,14 @@ grails:
hibernate:
cache:
queries: false
- use_second_level_cache: true
+ use_second_level_cache: false
use_query_cache: false
region.factory_class:
'org.hibernate.cache.ehcache.EhCacheRegionFactory'
Review Comment:
`region.factory_class` still points at
`org.hibernate.cache.ehcache.EhCacheRegionFactory` even though both
second-level and query caching are disabled. This is misleading and can become
a hard failure in the Hibernate 7 functional lane where `hibernate-ehcache` is
excluded (leaving the class unavailable).
--
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]
