Copilot commented on code in PR #15772:
URL: https://github.com/apache/grails-core/pull/15772#discussion_r3484355708
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/validation/listener/ValidationEventListener.groovy:
##########
@@ -70,7 +70,7 @@ class ValidationEventListener extends
AbstractPersistenceEventListener {
boolean hasErrors = false
if (source instanceof ConnectionSourcesProvider) {
def connectionSourceName =
((ConnectionSourcesProvider)
source).connectionSources.defaultConnectionSource.name
- GormValidationApi validationApi =
GormEnhancer.findValidationApi((Class<Object>) entityObject.getClass(),
connectionSourceName)
+ GormValidationApi validationApi =
GormRegistry.instance.findValidationApi((Class<Object>)
entityObject.getClass(), connectionSourceName)
Review Comment:
`GormRegistry.findValidationApi(...)` delegates to
`resolveValidationApi(...)` and can return null. That would cause an immediate
NPE on `validationApi.validate(...)`. Prefer using the registry’s
`validationApiRegistry.findValidationApi(...)`, which fails fast with a clear
IllegalStateException when the API is not registered.
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy:
##########
@@ -125,7 +125,7 @@ class UniqueConstraint extends AbstractConstraint {
return
}
// replace with proxy to prevent trying to flush transient
instance
- propertyValue =
GormEnhancer.findStaticApi(association.javaClass).load(associationId)
+ propertyValue =
GormRegistry.instance.findStaticApi(association.javaClass).load(associationId)
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it ultimately uses
`resolveStaticApi(...)`), which would NPE on `.load(...)`. Use
`staticApiRegistry.findStaticApi(...)` here so missing configuration produces a
clear IllegalStateException instead of a NullPointerException.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/MultiTenant.groovy:
##########
@@ -44,7 +44,7 @@ trait MultiTenant<D> extends Entity {
*/
@Generated
static <T> T withTenant(Serializable tenantId, Closure<T> callable) {
- GormEnhancer.findStaticApi(this).withTenant(tenantId, callable)
+ GormRegistry.instance.findStaticApi((Class<D>)
this).withTenant(tenantId, callable)
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE at runtime. Using
`staticApiRegistry.findStaticApi(...)` here produces a clearer
IllegalStateException when GORM isn't initialized for the domain.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/DetachedCriteria.groovy:
##########
@@ -568,7 +568,7 @@ class DetachedCriteria<T> extends
AbstractDetachedCriteria<T> implements GormOpe
* @return The total number updated
*/
Number updateAll(Map properties) {
- GormEnhancer.findStaticApi(targetClass,
connectionName).withDatastoreSession { Session session ->
+ GormRegistry.instance.findStaticApi(targetClass,
connectionName).withDatastoreSession { Session session ->
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE when calling `withDatastoreSession`.
Using `staticApiRegistry.findStaticApi(...)` provides a clear failure mode when
GORM isn't initialized for the target class/connection.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/MultiTenant.groovy:
##########
@@ -66,6 +66,6 @@ trait MultiTenant<D> extends Entity {
*/
@Generated
static <D> GormAllOperations<D> withTenant(Serializable tenantId) {
- (GormAllOperations<D>)
GormEnhancer.findStaticApi(this).withTenant(tenantId)
+ (GormAllOperations<D>) GormRegistry.instance.findStaticApi((Class<D>)
this).withTenant(tenantId)
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE at runtime. Using
`staticApiRegistry.findStaticApi(...)` here produces a clearer
IllegalStateException when GORM isn't initialized for the domain.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/MultiTenant.groovy:
##########
@@ -55,7 +55,7 @@ trait MultiTenant<D> extends Entity {
*/
@Generated
static <D> GormAllOperations eachTenant(Closure callable) {
- GormEnhancer.findStaticApi(this,
ConnectionSource.DEFAULT).eachTenant(callable)
+ GormRegistry.instance.findStaticApi((Class<D>) this,
ConnectionSource.DEFAULT).eachTenant(callable)
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE at runtime. Using
`staticApiRegistry.findStaticApi(...)` here produces a clearer
IllegalStateException when GORM isn't initialized for the domain.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/DetachedCriteria.groovy:
##########
@@ -556,7 +556,7 @@ class DetachedCriteria<T> extends
AbstractDetachedCriteria<T> implements GormOpe
* @return The total number deleted
*/
Number deleteAll() {
- GormEnhancer.findStaticApi(targetClass,
connectionName).withDatastoreSession { Session session ->
+ GormRegistry.instance.findStaticApi(targetClass,
connectionName).withDatastoreSession { Session session ->
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE when calling `withDatastoreSession`.
Using `staticApiRegistry.findStaticApi(...)` provides a clear failure mode when
GORM isn't initialized for the target class/connection.
##########
grails-datamapping-core/src/main/groovy/grails/gorm/DetachedCriteria.groovy:
##########
@@ -737,7 +737,7 @@ class DetachedCriteria<T> extends
AbstractDetachedCriteria<T> implements GormOpe
private withPopulatedQuery(Map args, Closure additionalCriteria, Closure
callable) {
- GormStaticApi staticApi = persistentEntity.isMultiTenant() ?
GormEnhancer.findStaticApi(targetClass) :
GormEnhancer.findStaticApi(targetClass, connectionName)
+ GormStaticApi staticApi =
GormRegistry.instance.findStaticApi(targetClass, connectionName)
Review Comment:
`GormRegistry.findStaticApi(...)` can return null (it delegates to
`resolveStaticApi(...)`), which would NPE on
`staticApi.withDatastoreSession(...)`. Consider using
`GormRegistry.instance.staticApiRegistry.findStaticApi(targetClass,
connectionName)` here to fail fast with a clear IllegalStateException instead.
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java:
##########
@@ -62,6 +65,8 @@
*/
public class AutoTimestampEventListener extends
AbstractPersistenceEventListener implements MappingContext.Listener,
ApplicationContextAware {
+ private static final Logger LOG =
LoggerFactory.getLogger(AutoTimestampEventListener.class);
+
Review Comment:
`LOG` is declared but never used in this class. That typically triggers
static-analysis noise (and in some rule sets, a build failure). Either remove
the logger (and its imports) or add a log statement where it provides value.
--
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]
