From 29f75717dfd3a43c1adc4293e26f68e3f8e232e4 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 10:12:12 -0500 Subject: [PATCH 01/10] Add test coverage for org.grails.datastore.gorm.events (core) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DomainEventListener, DefaultApplicationEventPublisher, and ConfigurableApplicationContextEventPublisher had zero test coverage. Adds mock-based Spock specs driving each class through its public constructor/method/onApplicationEvent surface only. Coverage: DomainEventListener 0% -> 92% lines / 91% branches / 96% methods; DefaultApplicationEventPublisher 0% -> 95% / 81% / 100%; ConfigurableApplicationContextEventPublisher 0% -> 100% / n/a / 100%. Three branches are documented as intentionally left uncovered in DomainEventListenerSpec: invokeEvent's ea==null path is unreachable via the public API, and its eventMethod.getParameterTypes().length==1 path is confirmed (via decompiling spring-core) structurally dead — findAndCacheEvent only ever caches zero-argument hook methods, so event-argument-accepting hooks can never be invoked. --- ...pplicationContextEventPublisherSpec.groovy | 71 +++ ...efaultApplicationEventPublisherSpec.groovy | 163 +++++ .../events/DomainEventListenerSpec.groovy | 556 ++++++++++++++++++ 3 files changed, 790 insertions(+) create mode 100644 grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationContextEventPublisherSpec.groovy create mode 100644 grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisherSpec.groovy create mode 100644 grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationContextEventPublisherSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationContextEventPublisherSpec.groovy new file mode 100644 index 00000000000..f28f519144d --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationContextEventPublisherSpec.groovy @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.grails.datastore.gorm.events + +import spock.lang.Specification + +import org.springframework.context.ApplicationEvent +import org.springframework.context.ApplicationListener +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.context.PayloadApplicationEvent + +class ConfigurableApplicationContextEventPublisherSpec extends Specification { + + ConfigurableApplicationContext applicationContext = Mock(ConfigurableApplicationContext) + ConfigurableApplicationContextEventPublisher publisher = new ConfigurableApplicationContextEventPublisher(applicationContext) + + void "exposes the ApplicationContext it was constructed with"() { + expect: + publisher.applicationContext.is(applicationContext) + } + + void "addApplicationListener delegates to the wrapped ApplicationContext"() { + given: + ApplicationListener listener = Mock(ApplicationListener) + + when: + publisher.addApplicationListener(listener) + + then: + 1 * applicationContext.addApplicationListener(listener) + } + + void "publishEvent(ApplicationEvent) delegates to the wrapped ApplicationContext"() { + given: + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 1 * applicationContext.publishEvent(event) + } + + void "publishEvent(Object) delegates to the wrapped ApplicationContext"() { + given: + Object payload = 'a plain payload' + + when: + publisher.publishEvent(payload) + + then: + 1 * applicationContext.publishEvent(payload) + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisherSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisherSpec.groovy new file mode 100644 index 00000000000..2c4e5fc689d --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisherSpec.groovy @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.grails.datastore.gorm.events + +import spock.lang.Specification + +import org.springframework.context.ApplicationEvent +import org.springframework.context.ApplicationListener +import org.springframework.context.PayloadApplicationEvent +import org.springframework.context.event.SmartApplicationListener + +class DefaultApplicationEventPublisherSpec extends Specification { + + DefaultApplicationEventPublisher publisher = new DefaultApplicationEventPublisher() + + void "publishEvent(ApplicationEvent) does nothing when no listeners are registered"() { + given: + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + noExceptionThrown() + } + + void "publishEvent(Object) does nothing when no listeners are registered"() { + when: + publisher.publishEvent('a plain payload') + + then: + noExceptionThrown() + } + + void "publishEvent notifies a plain ApplicationListener regardless of event or source type"() { + given: + ApplicationListener listener = Mock(ApplicationListener) + publisher.addApplicationListener(listener) + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 1 * listener.onApplicationEvent(event) + } + + void "publishEvent notifies every registered listener in order"() { + given: + ApplicationListener first = Mock(ApplicationListener) + ApplicationListener second = Mock(ApplicationListener) + publisher.addApplicationListener(first) + publisher.addApplicationListener(second) + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 1 * first.onApplicationEvent(event) + 1 * second.onApplicationEvent(event) + } + + void "publishEvent notifies a SmartApplicationListener when it supports both the event type and source type"() { + given: + SmartApplicationListener listener = Mock(SmartApplicationListener) { + supportsEventType(_) >> true + supportsSourceType(_) >> true + } + publisher.addApplicationListener(listener) + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 1 * listener.onApplicationEvent(event) + } + + void "publishEvent skips a SmartApplicationListener that does not support the event type"() { + given: + SmartApplicationListener listener = Mock(SmartApplicationListener) { + supportsEventType(_) >> false + } + publisher.addApplicationListener(listener) + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 0 * listener.onApplicationEvent(_) + } + + void "publishEvent skips a SmartApplicationListener that supports the event type but not the source type"() { + given: + SmartApplicationListener listener = Mock(SmartApplicationListener) { + supportsEventType(_) >> true + supportsSourceType(_) >> false + } + publisher.addApplicationListener(listener) + ApplicationEvent event = new PayloadApplicationEvent<>(this, 'payload') + + when: + publisher.publishEvent(event) + + then: + 0 * listener.onApplicationEvent(_) + } + + void "publishEvent(Object) wraps the payload in a PayloadApplicationEvent sourced from the publisher itself"() { + given: + ApplicationListener listener = Mock(ApplicationListener) + publisher.addApplicationListener(listener) + + when: + publisher.publishEvent('a plain payload') + + then: + 1 * listener.onApplicationEvent({ ApplicationEvent e -> + e instanceof PayloadApplicationEvent && + e.source.is(publisher) && + e.payload == 'a plain payload' + }) + } + + void "publishEvent(Object) applies the same SmartApplicationListener filtering as publishEvent(ApplicationEvent)"() { + given: + SmartApplicationListener supported = Mock(SmartApplicationListener) { + supportsEventType(_) >> true + supportsSourceType(_) >> true + } + SmartApplicationListener unsupported = Mock(SmartApplicationListener) { + supportsEventType(_) >> false + } + publisher.addApplicationListener(supported) + publisher.addApplicationListener(unsupported) + + when: + publisher.publishEvent('a plain payload') + + then: + 1 * supported.onApplicationEvent(_) + 0 * unsupported.onApplicationEvent(_) + } +} diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy new file mode 100644 index 00000000000..3ddf2d0118d --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy @@ -0,0 +1,556 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.grails.datastore.gorm.events + +import java.sql.Timestamp + +import spock.lang.Specification +import spock.lang.Unroll + +import org.springframework.beans.factory.config.AutowireCapableBeanFactory +import org.springframework.context.ApplicationEvent +import org.springframework.context.ConfigurableApplicationContext +import org.springframework.context.PayloadApplicationEvent + +import org.grails.datastore.mapping.config.Entity +import org.grails.datastore.mapping.core.Datastore +import org.grails.datastore.mapping.core.connections.ConnectionSource +import org.grails.datastore.mapping.core.connections.ConnectionSourceSettings +import org.grails.datastore.mapping.core.connections.ConnectionSources +import org.grails.datastore.mapping.core.connections.ConnectionSourcesProvider +import org.grails.datastore.mapping.dirty.checking.DirtyCheckable +import org.grails.datastore.mapping.engine.EntityAccess +import org.grails.datastore.mapping.engine.event.MergeEvent +import org.grails.datastore.mapping.engine.event.PersistEvent +import org.grails.datastore.mapping.engine.event.PostDeleteEvent +import org.grails.datastore.mapping.engine.event.PostInsertEvent +import org.grails.datastore.mapping.engine.event.PostLoadEvent +import org.grails.datastore.mapping.engine.event.PostUpdateEvent +import org.grails.datastore.mapping.engine.event.PreDeleteEvent +import org.grails.datastore.mapping.engine.event.PreInsertEvent +import org.grails.datastore.mapping.engine.event.PreLoadEvent +import org.grails.datastore.mapping.engine.event.PreUpdateEvent +import org.grails.datastore.mapping.engine.event.SaveOrUpdateEvent +import org.grails.datastore.mapping.engine.event.ValidationEvent +import org.grails.datastore.mapping.model.ClassMapping +import org.grails.datastore.mapping.model.MappingContext +import org.grails.datastore.mapping.model.PersistentEntity +import org.grails.datastore.mapping.model.PersistentProperty +import org.grails.datastore.mapping.model.config.GormProperties + +/** + * Note on coverage gaps left deliberately untested: + * - {@code invokeEvent}'s {@code ea != null} branch is always true through every public before- + * and after-hook method, which never passes a null {@code EntityAccess}; the {@code ea == null} + * path is unreachable via the public API. + * - {@code invokeEvent}'s {@code eventMethod.getParameterTypes().length == 1} branch can never be + * taken: {@code findAndCacheEvent} caches hooks via Spring's {@code ReflectionUtils.findMethod(Class, String)}, + * which (confirmed via decompiling spring-core) only ever matches zero-argument methods, so a + * cached {@code eventMethod} can never have one parameter. Event-argument-accepting hooks appear + * to be an unreachable, effectively dead capability. + * - The protected {@code DomainEventListener(ConnectionSourcesProvider, MappingContext)} + * constructor exists solely for subclassing (e.g. {@code grails.gorm.rx.events.DomainEventListener}), + * which is covered by its own module's spec; exercising it here would duplicate that coverage. + */ +class DomainEventListenerSpec extends Specification { + + void "registers itself as a mapping context listener and creates event caches for entities present at construction time"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + MappingContext mappingContext = Mock(MappingContext) { + getPersistentEntities() >> [entity] + } + Datastore datastore = plainDatastore(mappingContext) + + when: + DomainEventListener listener = new DomainEventListener(datastore) + + then: + 1 * mappingContext.addMappingContextListener(_) + + when: 'the pre-existing entity\'s hook is invoked' + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + listener.beforeInsert(entity, ea) + + then: 'it fires immediately, proving the cache was created eagerly at construction time' + domain.invoked == ['beforeInsert'] + } + + void "persistentEntityAdded creates event caches for a newly discovered entity"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + expect: 'the hook is not yet wired up before the entity is added' + listener.beforeInsert(entity, ea) + domain.invoked.isEmpty() + + when: + listener.persistentEntityAdded(entity) + listener.beforeInsert(entity, ea) + + then: + domain.invoked == ['beforeInsert'] + } + + void "supportsEventType accepts AbstractPersistenceEvent subtypes and rejects unrelated ApplicationEvents"() { + given: + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + + expect: + listener.supportsEventType(PreInsertEvent) + !listener.supportsEventType(PayloadApplicationEvent) + } + + void "beforeInsert sets an initial numeric version to 0 when the entity is versioned"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain, true, Long) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> new NoHooksDomain() + getPersistentEntity() >> entity + } + + when: + listener.beforeInsert(entity, ea) + + then: + 1 * ea.setProperty(GormProperties.VERSION, 0) + } + + void "beforeInsert sets an initial java.sql.Timestamp version when the version type is a Timestamp"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain, true, Timestamp) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> new NoHooksDomain() + getPersistentEntity() >> entity + } + + when: + listener.beforeInsert(entity, ea) + + then: + 1 * ea.setProperty(GormProperties.VERSION, { it instanceof Timestamp }) + } + + void "beforeInsert sets an initial java.util.Date version when the version type is a plain Date"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain, true, Date) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> new NoHooksDomain() + getPersistentEntity() >> entity + } + + when: + listener.beforeInsert(entity, ea) + + then: + 1 * ea.setProperty(GormProperties.VERSION, { it.class == Date }) + } + + void "beforeInsert does not set a version when the version type is neither Number, Timestamp, nor Date"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain, true, String) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> new NoHooksDomain() + getPersistentEntity() >> entity + } + + when: + listener.beforeInsert(entity, ea) + + then: + 0 * ea.setProperty(*_) + } + + void "beforeInsert does not set a version when the entity is not versioned"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain, false) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Mock(EntityAccess) { getEntity() >> new NoHooksDomain() } + + when: + listener.beforeInsert(entity, ea) + + then: + 0 * ea.setProperty(*_) + } + + void "beforeInsert returns true without error when the entity was never registered with the listener"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> new NoHooksDomain() } + + expect: + listener.beforeInsert(entity, ea) + } + + void "beforeInsert returns true without error when the domain class defines no beforeInsert hook"() { + given: + PersistentEntity entity = entityFor(NoHooksDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + listener.persistentEntityAdded(entity) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> new NoHooksDomain() } + + expect: + listener.beforeInsert(entity, ea) + } + + @Unroll + void "the 2-arg #methodName(entity, ea) convenience overload dispatches to the corresponding hook"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + listener.persistentEntityAdded(entity) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener."$methodName"(entity, ea) + + then: + domain.invoked == [hookName] + + where: + methodName | hookName + 'beforeInsert' | 'beforeInsert' + 'beforeUpdate' | 'beforeUpdate' + 'beforeDelete' | 'beforeDelete' + 'beforeLoad' | 'beforeLoad' + 'afterInsert' | 'afterInsert' + 'afterUpdate' | 'afterUpdate' + 'afterDelete' | 'afterDelete' + 'afterLoad' | 'afterLoad' + } + + void "afterInsert activates dirty checking on entities that implement DirtyCheckable"() { + given: + DirtyCheckableDomain domain = Spy(DirtyCheckableDomain) + PersistentEntity entity = entityFor(DirtyCheckableDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterInsert(entity, ea) + + then: + 1 * domain.trackChanges() + } + + void "afterUpdate re-activates dirty checking on entities that implement DirtyCheckable"() { + given: + DirtyCheckableDomain domain = Spy(DirtyCheckableDomain) + PersistentEntity entity = entityFor(DirtyCheckableDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterUpdate(entity, ea) + + then: + 1 * domain.trackChanges() + } + + void "afterLoad activates dirty checking on entities that implement DirtyCheckable"() { + given: + DirtyCheckableDomain domain = Spy(DirtyCheckableDomain) + PersistentEntity entity = entityFor(DirtyCheckableDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterLoad(entity, ea) + + then: + 1 * domain.trackChanges() + } + + void "afterDelete does not activate dirty checking since the entity is no longer trackable"() { + given: + DirtyCheckableDomain domain = Spy(DirtyCheckableDomain) + PersistentEntity entity = entityFor(DirtyCheckableDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterDelete(entity, ea) + + then: + 0 * domain.trackChanges() + } + + void "afterLoad autowires the entity when the datastore's default connection source is configured to autowire"() { + given: + NoHooksDomain domain = new NoHooksDomain() + PersistentEntity entity = entityFor(NoHooksDomain, false, null, false) + AutowireCapableBeanFactory beanFactory = Mock(AutowireCapableBeanFactory) + ConfigurableApplicationContext appContext = Stub(ConfigurableApplicationContext) { + getAutowireCapableBeanFactory() >> beanFactory + } + Datastore datastore = connectionAwareDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }, true, appContext) + DomainEventListener listener = new DomainEventListener(datastore) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterLoad(entity, ea) + + then: + 1 * beanFactory.autowireBeanProperties(domain, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, false) + } + + void "afterLoad autowires the entity when the entity's own mapping requests autowire even though the datastore default does not"() { + given: + NoHooksDomain domain = new NoHooksDomain() + PersistentEntity entity = entityFor(NoHooksDomain, false, null, true) + AutowireCapableBeanFactory beanFactory = Mock(AutowireCapableBeanFactory) + ConfigurableApplicationContext appContext = Stub(ConfigurableApplicationContext) { + getAutowireCapableBeanFactory() >> beanFactory + } + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }, appContext) + DomainEventListener listener = new DomainEventListener(datastore) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterLoad(entity, ea) + + then: + 1 * beanFactory.autowireBeanProperties(domain, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, false) + } + + void "afterLoad does not autowire the entity when neither the datastore default nor the entity's mapping request it"() { + given: + NoHooksDomain domain = new NoHooksDomain() + PersistentEntity entity = entityFor(NoHooksDomain, false, null, false) + AutowireCapableBeanFactory beanFactory = Mock(AutowireCapableBeanFactory) + ConfigurableApplicationContext appContext = Stub(ConfigurableApplicationContext) { + getAutowireCapableBeanFactory() >> beanFactory + } + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }, appContext) + DomainEventListener listener = new DomainEventListener(datastore) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterLoad(entity, ea) + + then: + 0 * beanFactory.autowireBeanProperties(*_) + } + + void "afterLoad requests autowiring without error when the datastore has no ApplicationContext to autowire through"() { + given: + NoHooksDomain domain = new NoHooksDomain() + PersistentEntity entity = entityFor(NoHooksDomain, false, null, false) + Datastore datastore = connectionAwareDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }, true, null) + DomainEventListener listener = new DomainEventListener(datastore) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.afterLoad(entity, ea) + + then: + notThrown(NullPointerException) + } + + @Unroll + void "onApplicationEvent dispatches a #eventType.simpleName to the #hookName hook"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + listener.persistentEntityAdded(entity) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + ApplicationEvent event = eventType.newInstance(datastore, entity, ea) + + when: + listener.onApplicationEvent(event) + + then: + domain.invoked == [hookName] + + where: + eventType | hookName + PreInsertEvent | 'beforeInsert' + PostInsertEvent | 'afterInsert' + PreUpdateEvent | 'beforeUpdate' + PostUpdateEvent | 'afterUpdate' + PreDeleteEvent | 'beforeDelete' + PostDeleteEvent | 'afterDelete' + PreLoadEvent | 'beforeLoad' + PostLoadEvent | 'afterLoad' + } + + @Unroll + void "onApplicationEvent silently ignores a #eventType.simpleName since domain events define no hook for it"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + listener.persistentEntityAdded(entity) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener.onApplicationEvent(eventType.newInstance(datastore, entity, ea)) + + then: + noExceptionThrown() + domain.invoked.isEmpty() + + where: + eventType << [SaveOrUpdateEvent, ValidationEvent, MergeEvent, PersistEvent] + } + + @Unroll + void "onApplicationEvent cancels a #eventType.simpleName when its before-hook returns false"() { + given: + CancellingDomain domain = new CancellingDomain() + PersistentEntity entity = entityFor(CancellingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + listener.persistentEntityAdded(entity) + EntityAccess ea = Mock(EntityAccess) { getEntity() >> domain } + ApplicationEvent event = eventType.newInstance(datastore, entity, ea) + + when: + listener.onApplicationEvent(event) + + then: + event.cancelled + 0 * ea.refresh() + + where: + eventType << [PreInsertEvent, PreUpdateEvent, PreDeleteEvent] + } + + void "onApplicationEvent refreshes the entity access after a successful beforeInsert hook since beforeInsert is a refresh event"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + listener.persistentEntityAdded(entity) + EntityAccess ea = Mock(EntityAccess) { getEntity() >> domain } + + when: + listener.onApplicationEvent(new PreInsertEvent(datastore, entity, ea)) + + then: + 1 * ea.refresh() + } + + void "onApplicationEvent does not refresh the entity access after a successful beforeLoad hook since beforeLoad is not a refresh event"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + Datastore datastore = plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] }) + DomainEventListener listener = new DomainEventListener(datastore) + listener.persistentEntityAdded(entity) + EntityAccess ea = Mock(EntityAccess) { getEntity() >> domain } + + when: + listener.onApplicationEvent(new PreLoadEvent(datastore, entity, ea)) + + then: + 0 * ea.refresh() + } + + private PersistentEntity entityFor(Class javaClass, boolean versioned = false, Class versionType = null, + boolean mappedAutowire = false) { + PersistentProperty version = versioned ? Stub(PersistentProperty) { getType() >> versionType } : null + ClassMapping mapping = Stub(ClassMapping) { + getMappedForm() >> new Entity(autowire: mappedAutowire) + } + Stub(PersistentEntity) { + getJavaClass() >> javaClass + isVersioned() >> versioned + getVersion() >> version + getMapping() >> mapping + } + } + + private Datastore plainDatastore(MappingContext mappingContext, ConfigurableApplicationContext appContext = null) { + Stub(Datastore) { + getMappingContext() >> mappingContext + getApplicationContext() >> appContext + } + } + + private Datastore connectionAwareDatastore(MappingContext mappingContext, boolean autowire, + ConfigurableApplicationContext appContext = null) { + ConnectionSource connectionSource = Stub(ConnectionSource) { + getSettings() >> new ConnectionSourceSettings(autowire: autowire) + } + ConnectionSources connectionSources = Stub(ConnectionSources) { + getDefaultConnectionSource() >> connectionSource + } + Stub(Datastore, additionalInterfaces: [ConnectionSourcesProvider]) { + getMappingContext() >> mappingContext + getApplicationContext() >> appContext + getConnectionSources() >> connectionSources + } + } +} + +class RecordingDomain { + + List invoked = [] + + void beforeInsert() { invoked << 'beforeInsert' } + + void beforeUpdate() { invoked << 'beforeUpdate' } + + void beforeDelete() { invoked << 'beforeDelete' } + + void beforeLoad() { invoked << 'beforeLoad' } + + void afterInsert() { invoked << 'afterInsert' } + + void afterUpdate() { invoked << 'afterUpdate' } + + void afterDelete() { invoked << 'afterDelete' } + + void afterLoad() { invoked << 'afterLoad' } +} + +class CancellingDomain { + + boolean beforeInsert() { false } + + boolean beforeUpdate() { false } + + boolean beforeDelete() { false } +} + +class NoHooksDomain { + +} + +class DirtyCheckableDomain implements DirtyCheckable { + +} From da3a9fe60a6e8b16c704a41dd2cb61170f9cc48e Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 10:25:09 -0500 Subject: [PATCH 02/10] Add construction/dispatch test coverage for AutoTimestampEventListener The existing AutoTimestampEventListenerSpec only drove the suppression logic and beforeInsert/beforeUpdate directly via a test subclass that bypassed initForMappingContext entirely, leaving construction, real entity/property scanning (storeDateCreatedAndLastUpdatedInfo, including @CreatedDate/@LastModifiedDate/@CreatedBy/@LastModifiedBy annotation detection), the deferred-initialization path, setApplicationContext, supportsEventType, and onApplicationEvent dispatch untested. Coverage: 84% -> 98% lines, 61% -> 83% branches, 76% -> 96% methods. --- ...estampEventListenerConstructionSpec.groovy | 347 ++++++++++++++++++ 1 file changed, 347 insertions(+) create mode 100644 grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListenerConstructionSpec.groovy diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListenerConstructionSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListenerConstructionSpec.groovy new file mode 100644 index 00000000000..38eb5eace57 --- /dev/null +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListenerConstructionSpec.groovy @@ -0,0 +1,347 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.grails.datastore.gorm.events + +import spock.lang.Specification + +import org.springframework.beans.factory.NoSuchBeanDefinitionException +import org.springframework.context.ApplicationContext + +import grails.gorm.annotation.CreatedBy +import grails.gorm.annotation.CreatedDate +import grails.gorm.annotation.LastModifiedBy +import grails.gorm.annotation.LastModifiedDate +import org.grails.datastore.gorm.timestamp.AuditorAware +import org.grails.datastore.gorm.timestamp.TimestampProvider +import org.grails.datastore.mapping.config.Entity +import org.grails.datastore.mapping.core.Datastore +import org.grails.datastore.mapping.engine.EntityAccess +import org.grails.datastore.mapping.engine.event.PostInsertEvent +import org.grails.datastore.mapping.engine.event.PreInsertEvent +import org.grails.datastore.mapping.engine.event.PreUpdateEvent +import org.grails.datastore.mapping.model.ClassMapping +import org.grails.datastore.mapping.model.MappingContext +import org.grails.datastore.mapping.model.PersistentEntity +import org.grails.datastore.mapping.model.PersistentProperty +import org.grails.datastore.mapping.model.PropertyMapping +import org.grails.datastore.mapping.config.Property + +/** + * Covers the surface of {@link AutoTimestampEventListener} that + * {@code org.grails.datastore.gorm.timestamp.AutoTimestampEventListenerSpec} does not exercise: + * construction via a real {@link Datastore}/{@link MappingContext} (which drives the real + * {@code storeDateCreatedAndLastUpdatedInfo} scanning logic, including annotation detection), + * event dispatch via the public {@code onApplicationEvent}/{@code supportsEventType} surface, and + * {@code setApplicationContext}. The protected {@code AutoTimestampEventListener(MappingContext)} + * constructor is exercised only through {@code org.grails.gorm.rx.events.AutoTimestampEventListener}, + * its sole subclass, covered by that module's own spec. + */ +class AutoTimestampEventListenerConstructionSpec extends Specification { + + void "construction scans already-initialized entities and registers a dateCreated/lastUpdated-by-name property"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true) + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == ['dateCreated'] as Set + listener.getLastUpdatedPropertyNames(WithConventionalNames.name) == ['lastUpdated'] as Set + } + + void "construction registers @CreatedDate/@LastModifiedDate annotated properties regardless of their name"() { + given: + PersistentEntity entity = entityFor(WithDateAnnotations, true) + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getDateCreatedPropertyNames(WithDateAnnotations.name) == ['whenCreated'] as Set + listener.getLastUpdatedPropertyNames(WithDateAnnotations.name) == ['whenModified'] as Set + } + + void "construction registers @CreatedBy/@LastModifiedBy annotated properties as auditor fields"() { + given: + PersistentEntity entity = entityFor(WithAuditorAnnotations, true) + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getCreatedByPropertyNames(WithAuditorAnnotations.name) == ['createdBy'] as Set + listener.getUpdatedByPropertyNames(WithAuditorAnnotations.name) == ['lastModifiedBy'] as Set + } + + void "construction ignores plain properties that carry no auto-timestamp or auditor annotation"() { + given: + PersistentEntity entity = entityFor(PlainDomain, true) + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getDateCreatedPropertyNames(PlainDomain.name) == null + listener.getLastUpdatedPropertyNames(PlainDomain.name) == null + listener.getCreatedByPropertyNames(PlainDomain.name) == null + listener.getUpdatedByPropertyNames(PlainDomain.name) == null + } + + void "construction skips scanning an entity whose mapping explicitly disables autoTimestamp"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true, false) + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == null + listener.getLastUpdatedPropertyNames(WithConventionalNames.name) == null + } + + void "construction scans an entity that has no mapped form at all as if autoTimestamp were enabled"() { + given: + PersistentEntity entity = Stub(PersistentEntity) { + getName() >> WithConventionalNames.name + isInitialized() >> true + getMapping() >> Stub(ClassMapping) { getMappedForm() >> null } + getPersistentProperties() >> propertiesFor(WithConventionalNames) + } + + when: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + then: + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == ['dateCreated'] as Set + } + + void "storeTimestampAvailability does not register a dateCreated/lastUpdated property whose type the TimestampProvider does not support"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true) + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([])) + listener.setTimestampProvider(Stub(TimestampProvider) { + supportsCreating(_) >> false + }) + + when: + listener.persistentEntityAdded(entity) + + then: + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == null + listener.getLastUpdatedPropertyNames(WithConventionalNames.name) == null + } + + void "an uninitialized entity is deferred and only scanned once beforeInsert is actually invoked for it"() { + given: 'an entity that starts out not initialized, but becomes initialized by the time its hook fires' + boolean initialized = false + PersistentEntity entity = Stub(PersistentEntity) { + getName() >> WithConventionalNames.name + isInitialized() >> { initialized } + getMapping() >> Stub(ClassMapping) { getMappedForm() >> new Entity() } + getPersistentProperties() >> propertiesFor(WithConventionalNames) + } + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([entity])) + + expect: 'nothing was scanned yet, since the entity reported itself as not initialized' + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == null + + when: + initialized = true + WithConventionalNames domain = new WithConventionalNames() + EntityAccess ea = Stub(EntityAccess) { + getEntity() >> domain + getPropertyValue(_) >> null + getPropertyType(_) >> Date + } + listener.beforeInsert(entity, ea) + + then: 'the deferred scan ran, so the property is now known and was applied to the entity access' + listener.getDateCreatedPropertyNames(WithConventionalNames.name) == ['dateCreated'] as Set + } + + void "setApplicationContext wires the AuditorAware bean when one is present"() { + given: + AuditorAware auditorAware = Stub(AuditorAware) + ApplicationContext applicationContext = Stub(ApplicationContext) { + getBean(AuditorAware) >> auditorAware + } + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([])) + + when: + listener.setApplicationContext(applicationContext) + + then: + listener.auditorAware.is(auditorAware) + } + + void "setApplicationContext silently leaves auditorAware unset when no AuditorAware bean is registered"() { + given: + ApplicationContext applicationContext = Stub(ApplicationContext) { + getBean(AuditorAware) >> { throw new NoSuchBeanDefinitionException(AuditorAware) } + } + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([])) + + when: + listener.setApplicationContext(applicationContext) + + then: + noExceptionThrown() + listener.auditorAware == null + } + + void "supportsEventType accepts only PreInsertEvent and PreUpdateEvent"() { + given: + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastoreFor([])) + + expect: + listener.supportsEventType(PreInsertEvent) + listener.supportsEventType(PreUpdateEvent) + !listener.supportsEventType(PostInsertEvent) + } + + void "onApplicationEvent dispatches a PreInsertEvent to beforeInsert"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true) + Datastore datastore = datastoreFor([entity]) + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastore) + WithConventionalNames domain = new WithConventionalNames() + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> domain + getPropertyType(_) >> Date + } + + when: + listener.onApplicationEvent(new PreInsertEvent(datastore, entity, ea)) + + then: + 2 * ea.setProperty(_, _) + } + + void "onApplicationEvent dispatches a PreUpdateEvent to beforeUpdate"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true) + Datastore datastore = datastoreFor([entity]) + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastore) + WithConventionalNames domain = new WithConventionalNames() + EntityAccess ea = Mock(EntityAccess) { + getEntity() >> domain + getPropertyType(_) >> Date + } + + when: + listener.onApplicationEvent(new PreUpdateEvent(datastore, entity, ea)) + + then: + 1 * ea.setProperty('lastUpdated', _) + } + + void "onApplicationEvent ignores an event whose entity is null"() { + given: + Datastore datastore = datastoreFor([]) + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastore) + EntityAccess ea = Mock(EntityAccess) + + when: + listener.onApplicationEvent(new PreInsertEvent(datastore, null, ea)) + + then: + noExceptionThrown() + 0 * ea.setProperty(*_) + } + + void "onApplicationEvent ignores event types other than PreInsert and PreUpdate"() { + given: + PersistentEntity entity = entityFor(WithConventionalNames, true) + Datastore datastore = datastoreFor([entity]) + AutoTimestampEventListener listener = new AutoTimestampEventListener(datastore) + EntityAccess ea = Mock(EntityAccess) { getEntity() >> new WithConventionalNames() } + + when: + listener.onApplicationEvent(new PostInsertEvent(datastore, entity, ea)) + + then: + noExceptionThrown() + 0 * ea.setProperty(*_) + } + + private Datastore datastoreFor(List entities) { + Stub(Datastore) { + getMappingContext() >> Stub(MappingContext) { + getPersistentEntities() >> entities + } + } + } + + private PersistentEntity entityFor(Class javaClass, boolean initialized, Boolean autoTimestamp = null) { + Entity mappedForm = autoTimestamp == null ? new Entity() : new Entity(autoTimestamp: autoTimestamp) + Stub(PersistentEntity) { + getName() >> javaClass.name + isInitialized() >> initialized + getMapping() >> Stub(ClassMapping) { getMappedForm() >> mappedForm } + getPersistentProperties() >> propertiesFor(javaClass) + } + } + + private List propertiesFor(Class javaClass) { + javaClass.declaredFields.findAll { !it.synthetic }.collect { field -> + Property mappedForm = new Property() + PersistentEntity owner = Stub(PersistentEntity) { + getName() >> javaClass.name + getJavaClass() >> javaClass + } + Stub(PersistentProperty) { + getName() >> field.name + getType() >> field.type + getOwner() >> owner + getMapping() >> Stub(PropertyMapping) { getMappedForm() >> mappedForm } + } + } + } +} + +class WithConventionalNames { + + Date dateCreated + Date lastUpdated +} + +class WithDateAnnotations { + + @CreatedDate + Date whenCreated + + @LastModifiedDate + Date whenModified +} + +class WithAuditorAnnotations { + + @CreatedBy + String createdBy + + @LastModifiedBy + String lastModifiedBy +} + +class PlainDomain { + + String name +} From 36e717a8462230a0791aef7474f54262cd44a8aa Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 10:30:16 -0500 Subject: [PATCH 03/10] Remove dead event-argument-hook branch from DomainEventListener invokeEvent's eventMethod.getParameterTypes().length == 1 branch could never be taken: findAndCacheEvent caches hooks via Spring's ReflectionUtils.findMethod(Class, String), which only ever matches zero-argument methods (confirmed by decompiling spring-core), so a cached eventMethod can never have one parameter. Dropped the branch and the now-unused ApplicationEvent parameter it required, along with the argument at all 8 call sites. No behavior change; full module suite, jacoco, and codeStyle all clean. --- .../gorm/events/DomainEventListener.java | 31 +++++++------------ .../events/DomainEventListenerSpec.groovy | 10 +++--- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java index 4f800d779bd..fca82d846fb 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java @@ -163,7 +163,7 @@ public boolean beforeInsert(final PersistentEntity entity, final EntityAccess ea } } - return invokeEvent(EVENT_BEFORE_INSERT, entity, ea, event); + return invokeEvent(EVENT_BEFORE_INSERT, entity, ea); } protected void setVersion(final EntityAccess ea) { @@ -180,19 +180,19 @@ else if (Date.class.isAssignableFrom(versionType)) { } public boolean beforeUpdate(final PersistentEntity entity, final EntityAccess ea) { - return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea, null); + return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea); } public boolean beforeUpdate(final PersistentEntity entity, final EntityAccess ea, PreUpdateEvent event) { - return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea, event); + return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea); } public boolean beforeDelete(final PersistentEntity entity, final EntityAccess ea) { - return invokeEvent(EVENT_BEFORE_DELETE, entity, ea, null); + return invokeEvent(EVENT_BEFORE_DELETE, entity, ea); } public boolean beforeDelete(final PersistentEntity entity, final EntityAccess ea, PreDeleteEvent event) { - return invokeEvent(EVENT_BEFORE_DELETE, entity, ea, event); + return invokeEvent(EVENT_BEFORE_DELETE, entity, ea); } public void beforeLoad(final PersistentEntity entity, final EntityAccess ea) { @@ -200,7 +200,7 @@ public void beforeLoad(final PersistentEntity entity, final EntityAccess ea) { } public void beforeLoad(final PersistentEntity entity, final EntityAccess ea, PreLoadEvent event) { - invokeEvent(EVENT_BEFORE_LOAD, entity, ea, event); + invokeEvent(EVENT_BEFORE_LOAD, entity, ea); } public void afterDelete(final PersistentEntity entity, final EntityAccess ea) { @@ -208,7 +208,7 @@ public void afterDelete(final PersistentEntity entity, final EntityAccess ea) { } public void afterDelete(final PersistentEntity entity, final EntityAccess ea, PostDeleteEvent event) { - invokeEvent(EVENT_AFTER_DELETE, entity, ea, event); + invokeEvent(EVENT_AFTER_DELETE, entity, ea); } public void afterInsert(final PersistentEntity entity, final EntityAccess ea) { @@ -217,7 +217,7 @@ public void afterInsert(final PersistentEntity entity, final EntityAccess ea) { public void afterInsert(final PersistentEntity entity, final EntityAccess ea, PostInsertEvent event) { activateDirtyChecking(ea); - invokeEvent(EVENT_AFTER_INSERT, entity, ea, event); + invokeEvent(EVENT_AFTER_INSERT, entity, ea); } private void activateDirtyChecking(EntityAccess ea) { @@ -233,7 +233,7 @@ public void afterUpdate(final PersistentEntity entity, final EntityAccess ea) { public void afterUpdate(final PersistentEntity entity, final EntityAccess ea, PostUpdateEvent event) { activateDirtyChecking(ea); // reset dirty checking - invokeEvent(EVENT_AFTER_UPDATE, entity, ea, event); + invokeEvent(EVENT_AFTER_UPDATE, entity, ea); } public void afterLoad(final PersistentEntity entity, final EntityAccess ea) { @@ -245,7 +245,7 @@ public void afterLoad(final PersistentEntity entity, final EntityAccess ea, Post if (autowireEntities || (entity != null && entity.getMapping().getMappedForm().isAutowire())) { autowireBeanProperties(ea.getEntity()); } - invokeEvent(EVENT_AFTER_LOAD, entity, ea, event); + invokeEvent(EVENT_AFTER_LOAD, entity, ea); } protected void autowireBeanProperties(final Object entity) { @@ -274,7 +274,7 @@ public boolean supportsEventType(Class eventType) { return AbstractPersistenceEvent.class.isAssignableFrom(eventType); } - private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAccess ea, ApplicationEvent event) { + private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAccess ea) { final Map events = entityEvents.get(entity); if (events == null) { return true; @@ -287,14 +287,7 @@ private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAcc final Object result; if (ea != null) { - final Object o = ea.getEntity(); - - if (eventMethod.getParameterTypes().length == 1) { - result = ReflectionUtils.invokeMethod(eventMethod, o, event); - } - else { - result = ReflectionUtils.invokeMethod(eventMethod, o); - } + result = ReflectionUtils.invokeMethod(eventMethod, ea.getEntity()); } else { result = null; diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy index 3ddf2d0118d..319f27100c8 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy @@ -60,14 +60,14 @@ import org.grails.datastore.mapping.model.config.GormProperties * - {@code invokeEvent}'s {@code ea != null} branch is always true through every public before- * and after-hook method, which never passes a null {@code EntityAccess}; the {@code ea == null} * path is unreachable via the public API. - * - {@code invokeEvent}'s {@code eventMethod.getParameterTypes().length == 1} branch can never be - * taken: {@code findAndCacheEvent} caches hooks via Spring's {@code ReflectionUtils.findMethod(Class, String)}, - * which (confirmed via decompiling spring-core) only ever matches zero-argument methods, so a - * cached {@code eventMethod} can never have one parameter. Event-argument-accepting hooks appear - * to be an unreachable, effectively dead capability. * - The protected {@code DomainEventListener(ConnectionSourcesProvider, MappingContext)} * constructor exists solely for subclassing (e.g. {@code grails.gorm.rx.events.DomainEventListener}), * which is covered by its own module's spec; exercising it here would duplicate that coverage. + * + * {@code invokeEvent} previously also branched on {@code eventMethod.getParameterTypes().length == 1} + * to invoke a hook with the triggering event as an argument. That branch was confirmed dead (via + * decompiling spring-core's {@code ReflectionUtils.findMethod(Class, String)}, which only ever + * matches zero-argument methods) and removed. */ class DomainEventListenerSpec extends Specification { From 623be84e7126607de7f0b92e4a579f9f58803e52 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 10:44:29 -0500 Subject: [PATCH 04/10] Fix DomainEventListener IntelliJ warnings - entityEvents field made final (never reassigned) - Removed ZERO_PARAMS constant (zero references anywhere in the repo) - Parameterized raw ConnectionSourcesProvider/Class usages - supportsEventType now null-safe: Spring 7's SmartApplicationListener.supportsEventType declares its parameter @Nullable, and this override would NPE via Class.isAssignableFrom(null) - invokeEvent's ea.refresh() call is now guarded by the same ea != null check already used earlier in the method, closing a latent NPE path - Removed the two switch branches in onPersistenceEvent (SaveOrUpdate, Validation) that were duplicates of the default branch - Reordered each before/after method pair so the 2-arg overload holds the real logic and is canonical; the 3-arg overload (whose event parameter has been unused since the dead 1-arg-hook branch was removed) is now @Deprecated, forwards to the 2-arg overload, and is scheduled for removal in 9.0. onPersistenceEvent now calls the 2-arg overloads directly instead of the newly-deprecated 3-arg ones. Added tests for the deprecated overloads' delegation and the null eventType case. Full suite, jacoco, and codeStyle all clean. --- .../gorm/events/DomainEventListener.java | 123 ++++++++++-------- .../events/DomainEventListenerSpec.groovy | 36 +++++ 2 files changed, 108 insertions(+), 51 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java index fca82d846fb..75899037c93 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java @@ -58,10 +58,8 @@ public class DomainEventListener extends AbstractPersistenceEventListener implements MappingContext.Listener { - private Map> entityEvents = new ConcurrentHashMap<>(); + private final Map> entityEvents = new ConcurrentHashMap<>(); - @SuppressWarnings("rawtypes") - public static final Class[] ZERO_PARAMS = {}; public static final String EVENT_BEFORE_INSERT = "beforeInsert"; private static final String EVENT_BEFORE_UPDATE = "beforeUpdate"; private static final String EVENT_BEFORE_DELETE = "beforeDelete"; @@ -84,15 +82,15 @@ public DomainEventListener(final Datastore datastore) { } datastore.getMappingContext().addMappingContextListener(this); - if (datastore instanceof ConnectionSourcesProvider) { - autowireEntities = ((ConnectionSourcesProvider) datastore).getConnectionSources().getDefaultConnectionSource().getSettings().isAutowire(); + if (datastore instanceof ConnectionSourcesProvider) { + autowireEntities = ((ConnectionSourcesProvider) datastore).getConnectionSources().getDefaultConnectionSource().getSettings().isAutowire(); } else { autowireEntities = false; } } - protected DomainEventListener(ConnectionSourcesProvider connectionSourcesProvider, final MappingContext mappingContext) { + protected DomainEventListener(ConnectionSourcesProvider connectionSourcesProvider, final MappingContext mappingContext) { super(null); for (PersistentEntity entity : mappingContext.getPersistentEntities()) { @@ -106,54 +104,41 @@ protected DomainEventListener(ConnectionSourcesProvider connectionSourcesProvide protected void onPersistenceEvent(final AbstractPersistenceEvent event) { switch (event.getEventType()) { case PreInsert: - if (!beforeInsert(event.getEntity(), event.getEntityAccess(), (PreInsertEvent) event)) { + if (!beforeInsert(event.getEntity(), event.getEntityAccess())) { event.cancel(); } break; case PostInsert: - afterInsert(event.getEntity(), event.getEntityAccess(), (PostInsertEvent) event); + afterInsert(event.getEntity(), event.getEntityAccess()); break; case PreUpdate: - if (!beforeUpdate(event.getEntity(), event.getEntityAccess(), (PreUpdateEvent) event)) { + if (!beforeUpdate(event.getEntity(), event.getEntityAccess())) { event.cancel(); } break; case PostUpdate: - afterUpdate(event.getEntity(), event.getEntityAccess(), (PostUpdateEvent) event); + afterUpdate(event.getEntity(), event.getEntityAccess()); break; case PreDelete: - if (!beforeDelete(event.getEntity(), event.getEntityAccess(), (PreDeleteEvent) event)) { + if (!beforeDelete(event.getEntity(), event.getEntityAccess())) { event.cancel(); } break; case PostDelete: - afterDelete(event.getEntity(), event.getEntityAccess(), (PostDeleteEvent) event); + afterDelete(event.getEntity(), event.getEntityAccess()); break; case PreLoad: - beforeLoad(event.getEntity(), event.getEntityAccess(), (PreLoadEvent) event); + beforeLoad(event.getEntity(), event.getEntityAccess()); break; case PostLoad: - afterLoad(event.getEntity(), event.getEntityAccess(), (PostLoadEvent) event); - break; - case SaveOrUpdate: - break; - case Validation: + afterLoad(event.getEntity(), event.getEntityAccess()); break; default: break; } } - /** - * @deprecated Use {@link #beforeInsert(org.grails.datastore.mapping.model.PersistentEntity, org.grails.datastore.mapping.engine.EntityAccess, org.grails.datastore.mapping.engine.event.PreInsertEvent)} instead - */ - @Deprecated public boolean beforeInsert(final PersistentEntity entity, final EntityAccess ea) { - return beforeInsert(entity, ea, null); - } - - public boolean beforeInsert(final PersistentEntity entity, final EntityAccess ea, PreInsertEvent event) { - if (entity.isVersioned()) { try { setVersion(ea); @@ -166,8 +151,16 @@ public boolean beforeInsert(final PersistentEntity entity, final EntityAccess ea return invokeEvent(EVENT_BEFORE_INSERT, entity, ea); } + /** + * @deprecated the {@code event} parameter is unused; use {@link #beforeInsert(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public boolean beforeInsert(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PreInsertEvent event) { + return beforeInsert(entity, ea); + } + protected void setVersion(final EntityAccess ea) { - final Class versionType = ea.getPersistentEntity().getVersion().getType(); + final Class versionType = ea.getPersistentEntity().getVersion().getType(); if (Number.class.isAssignableFrom(versionType)) { ea.setProperty(GormProperties.VERSION, 0); } @@ -183,43 +176,63 @@ public boolean beforeUpdate(final PersistentEntity entity, final EntityAccess ea return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea); } - public boolean beforeUpdate(final PersistentEntity entity, final EntityAccess ea, PreUpdateEvent event) { - return invokeEvent(EVENT_BEFORE_UPDATE, entity, ea); + /** + * @deprecated the {@code event} parameter is unused; use {@link #beforeUpdate(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public boolean beforeUpdate(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PreUpdateEvent event) { + return beforeUpdate(entity, ea); } public boolean beforeDelete(final PersistentEntity entity, final EntityAccess ea) { return invokeEvent(EVENT_BEFORE_DELETE, entity, ea); } - public boolean beforeDelete(final PersistentEntity entity, final EntityAccess ea, PreDeleteEvent event) { - return invokeEvent(EVENT_BEFORE_DELETE, entity, ea); + /** + * @deprecated the {@code event} parameter is unused; use {@link #beforeDelete(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public boolean beforeDelete(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PreDeleteEvent event) { + return beforeDelete(entity, ea); } public void beforeLoad(final PersistentEntity entity, final EntityAccess ea) { - beforeLoad(entity, ea, null); - } - - public void beforeLoad(final PersistentEntity entity, final EntityAccess ea, PreLoadEvent event) { invokeEvent(EVENT_BEFORE_LOAD, entity, ea); } - public void afterDelete(final PersistentEntity entity, final EntityAccess ea) { - afterDelete(entity, ea, null); + /** + * @deprecated the {@code event} parameter is unused; use {@link #beforeLoad(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public void beforeLoad(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PreLoadEvent event) { + beforeLoad(entity, ea); } - public void afterDelete(final PersistentEntity entity, final EntityAccess ea, PostDeleteEvent event) { + public void afterDelete(final PersistentEntity entity, final EntityAccess ea) { invokeEvent(EVENT_AFTER_DELETE, entity, ea); } - public void afterInsert(final PersistentEntity entity, final EntityAccess ea) { - afterInsert(entity, ea, null); + /** + * @deprecated the {@code event} parameter is unused; use {@link #afterDelete(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public void afterDelete(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PostDeleteEvent event) { + afterDelete(entity, ea); } - public void afterInsert(final PersistentEntity entity, final EntityAccess ea, PostInsertEvent event) { + public void afterInsert(final PersistentEntity entity, final EntityAccess ea) { activateDirtyChecking(ea); invokeEvent(EVENT_AFTER_INSERT, entity, ea); } + /** + * @deprecated the {@code event} parameter is unused; use {@link #afterInsert(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public void afterInsert(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PostInsertEvent event) { + afterInsert(entity, ea); + } + private void activateDirtyChecking(EntityAccess ea) { Object e = ea.getEntity(); if (e instanceof DirtyCheckable) { @@ -228,19 +241,19 @@ private void activateDirtyChecking(EntityAccess ea) { } public void afterUpdate(final PersistentEntity entity, final EntityAccess ea) { - afterUpdate(entity, ea, null); - } - - public void afterUpdate(final PersistentEntity entity, final EntityAccess ea, PostUpdateEvent event) { activateDirtyChecking(ea); // reset dirty checking invokeEvent(EVENT_AFTER_UPDATE, entity, ea); } - public void afterLoad(final PersistentEntity entity, final EntityAccess ea) { - afterLoad(entity, ea, null); + /** + * @deprecated the {@code event} parameter is unused; use {@link #afterUpdate(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public void afterUpdate(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PostUpdateEvent event) { + afterUpdate(entity, ea); } - public void afterLoad(final PersistentEntity entity, final EntityAccess ea, PostLoadEvent event) { + public void afterLoad(final PersistentEntity entity, final EntityAccess ea) { activateDirtyChecking(ea); if (autowireEntities || (entity != null && entity.getMapping().getMappedForm().isAutowire())) { autowireBeanProperties(ea.getEntity()); @@ -248,6 +261,14 @@ public void afterLoad(final PersistentEntity entity, final EntityAccess ea, Post invokeEvent(EVENT_AFTER_LOAD, entity, ea); } + /** + * @deprecated the {@code event} parameter is unused; use {@link #afterLoad(PersistentEntity, EntityAccess)} instead. Scheduled for removal in 9.0. + */ + @Deprecated + public void afterLoad(final PersistentEntity entity, final EntityAccess ea, @SuppressWarnings("unused") PostLoadEvent event) { + afterLoad(entity, ea); + } + protected void autowireBeanProperties(final Object entity) { ConfigurableApplicationContext applicationContext = datastore.getApplicationContext(); if (applicationContext != null) { @@ -271,7 +292,7 @@ public void persistentEntityAdded(PersistentEntity entity) { * java.lang.Class) */ public boolean supportsEventType(Class eventType) { - return AbstractPersistenceEvent.class.isAssignableFrom(eventType); + return eventType != null && AbstractPersistenceEvent.class.isAssignableFrom(eventType); } private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAccess ea) { @@ -294,7 +315,7 @@ private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAcc } boolean booleanResult = (result instanceof Boolean) ? (Boolean) result : true; - if (booleanResult && REFRESH_EVENTS.contains(eventName)) { + if (ea != null && booleanResult && REFRESH_EVENTS.contains(eventName)) { ea.refresh(); } return booleanResult; diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy index 319f27100c8..01e6f61ee8f 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy @@ -123,6 +123,14 @@ class DomainEventListenerSpec extends Specification { !listener.supportsEventType(PayloadApplicationEvent) } + void "supportsEventType rejects a null event type rather than throwing, matching SmartApplicationListener's nullable contract"() { + given: + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + + expect: + !listener.supportsEventType(null) + } + void "beforeInsert sets an initial numeric version to 0 when the entity is versioned"() { given: PersistentEntity entity = entityFor(NoHooksDomain, true, Long) @@ -248,6 +256,34 @@ class DomainEventListenerSpec extends Specification { 'afterLoad' | 'afterLoad' } + @Unroll + @SuppressWarnings('deprecation') + void "the deprecated 3-arg #methodName(entity, ea, event) overload delegates to the 2-arg overload, ignoring the event argument"() { + given: + RecordingDomain domain = new RecordingDomain() + PersistentEntity entity = entityFor(RecordingDomain) + DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) + listener.persistentEntityAdded(entity) + EntityAccess ea = Stub(EntityAccess) { getEntity() >> domain } + + when: + listener."$methodName"(entity, ea, null) + + then: + domain.invoked == [hookName] + + where: + methodName | hookName + 'beforeInsert' | 'beforeInsert' + 'beforeUpdate' | 'beforeUpdate' + 'beforeDelete' | 'beforeDelete' + 'beforeLoad' | 'beforeLoad' + 'afterInsert' | 'afterInsert' + 'afterUpdate' | 'afterUpdate' + 'afterDelete' | 'afterDelete' + 'afterLoad' | 'afterLoad' + } + void "afterInsert activates dirty checking on entities that implement DirtyCheckable"() { given: DirtyCheckableDomain domain = Spy(DirtyCheckableDomain) From d841627d9a93e4692449a18ad1ccb7883b0e1870 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 11:07:56 -0500 Subject: [PATCH 05/10] Assert supportsEventType's eventType parameter as @NonNull Rather than defensively guarding against null, declare the contract explicitly via jspecify's @NonNull and let a null argument fail fast with an NPE. Updated the corresponding spec to assert the NPE instead of a graceful false return. --- .../datastore/gorm/events/DomainEventListener.java | 6 ++++-- .../datastore/gorm/events/DomainEventListenerSpec.groovy | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java index 75899037c93..b47a5616540 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DomainEventListener.java @@ -26,6 +26,8 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.NonNull; + import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.context.ApplicationEvent; import org.springframework.context.ConfigurableApplicationContext; @@ -291,8 +293,8 @@ public void persistentEntityAdded(PersistentEntity entity) { * @see org.springframework.context.event.SmartApplicationListener#supportsEventType( * java.lang.Class) */ - public boolean supportsEventType(Class eventType) { - return eventType != null && AbstractPersistenceEvent.class.isAssignableFrom(eventType); + public boolean supportsEventType(@NonNull Class eventType) { + return AbstractPersistenceEvent.class.isAssignableFrom(eventType); } private boolean invokeEvent(String eventName, PersistentEntity entity, EntityAccess ea) { diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy index 01e6f61ee8f..0f2eb864222 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/events/DomainEventListenerSpec.groovy @@ -123,12 +123,15 @@ class DomainEventListenerSpec extends Specification { !listener.supportsEventType(PayloadApplicationEvent) } - void "supportsEventType rejects a null event type rather than throwing, matching SmartApplicationListener's nullable contract"() { + void "supportsEventType throws on a null event type, per its @NonNull contract"() { given: DomainEventListener listener = new DomainEventListener(plainDatastore(Stub(MappingContext) { getPersistentEntities() >> [] })) - expect: - !listener.supportsEventType(null) + when: + listener.supportsEventType(null) + + then: + thrown(NullPointerException) } void "beforeInsert sets an initial numeric version to 0 when the entity is versioned"() { From 7b5204bcc2998c073d5b826968abae621aaae908 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 11:08:03 -0500 Subject: [PATCH 06/10] Fix AutoTimestampEventListener IntelliJ warnings - supportsEventType is now null-safe, matching Spring 7's SmartApplicationListener.supportsEventType @Nullable contract - Parameterized every raw Class/List usage across the withoutLastUpdated/withoutDateCreated/withoutTimestamps overloads and their shared runWithDisabled helper - Replaced disabled.entityNames.removeAll(added) (a HashSet.removeAll of a List, which can fall into an O(n*m) path depending on relative collection sizes) with a direct per-element remove, guaranteeing O(1) removals regardless of size getTimestampProvider() was also flagged as unused but left as-is: it's the getter half of a real getter/setter bean-property pair (the setter is the intended extension point for injecting a custom TimestampProvider), not dead code. Full suite, jacoco, and codeStyle all clean. --- .../events/AutoTimestampEventListener.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java index 8ecf4e12bb9..04ebaf8e31b 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/AutoTimestampEventListener.java @@ -29,6 +29,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; +import org.jspecify.annotations.NonNull; + import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; @@ -124,9 +126,8 @@ protected void onPersistenceEvent(final AbstractPersistenceEvent event) { } } - public boolean supportsEventType(Class eventType) { - return PreInsertEvent.class.isAssignableFrom(eventType) || - PreUpdateEvent.class.isAssignableFrom(eventType); + public boolean supportsEventType(@NonNull Class eventType) { + return PreInsertEvent.class.isAssignableFrom(eventType) || PreUpdateEvent.class.isAssignableFrom(eventType); } public boolean beforeInsert(PersistentEntity entity, EntityAccess ea) { @@ -341,13 +342,13 @@ private static void runWithAllDisabled(final ThreadLocal dis } } - private static void runWithDisabled(final ThreadLocal disabledTimestamps, final List classes, final Runnable runnable) { + private static void runWithDisabled(final ThreadLocal disabledTimestamps, final List> classes, final Runnable runnable) { // only the names this scope newly disables may be re-enabled on exit; a name already // disabled by an enclosing scope on this thread must survive this scope's finally List added = new ArrayList<>(classes.size()); DisabledTimestamps disabled = getOrCreateDisabled(disabledTimestamps); try { - for (Class clazz : classes) { + for (Class clazz : classes) { String entityName = clazz.getName(); if (disabled.entityNames.add(entityName)) { added.add(entityName); @@ -355,7 +356,7 @@ private static void runWithDisabled(final ThreadLocal disabl } runnable.run(); } finally { - disabled.entityNames.removeAll(added); + added.forEach(disabled.entityNames::remove); removeIfEmpty(disabledTimestamps, disabled); } } @@ -379,7 +380,7 @@ public void withoutLastUpdated(final Runnable runnable) { * @param classes Which classes to disable the last updated processing for * @param runnable The code to execute while the last updated listener is disabled */ - public void withoutLastUpdated(final List classes, final Runnable runnable) { + public void withoutLastUpdated(final List> classes, final Runnable runnable) { runWithDisabled(disabledLastUpdated, classes, runnable); } @@ -391,8 +392,8 @@ public void withoutLastUpdated(final List classes, final Runnable runnabl * @param clazz Which class to disable the last updated processing for * @param runnable The code to execute while the last updated listener is disabled */ - public void withoutLastUpdated(final Class clazz, final Runnable runnable) { - ArrayList list = new ArrayList<>(1); + public void withoutLastUpdated(final Class clazz, final Runnable runnable) { + ArrayList> list = new ArrayList<>(1); list.add(clazz); withoutLastUpdated(list, runnable); } @@ -416,7 +417,7 @@ public void withoutDateCreated(final Runnable runnable) { * @param classes Which classes to disable the date created processing for * @param runnable The code to execute while the date created listener is disabled */ - public void withoutDateCreated(final List classes, final Runnable runnable) { + public void withoutDateCreated(final List> classes, final Runnable runnable) { runWithDisabled(disabledDateCreated, classes, runnable); } @@ -428,8 +429,8 @@ public void withoutDateCreated(final List classes, final Runnable runnabl * @param clazz Which class to disable the date created processing for * @param runnable The code to execute while the date created listener is disabled */ - public void withoutDateCreated(final Class clazz, final Runnable runnable) { - ArrayList list = new ArrayList<>(1); + public void withoutDateCreated(final Class clazz, final Runnable runnable) { + ArrayList> list = new ArrayList<>(1); list.add(clazz); withoutDateCreated(list, runnable); } @@ -453,7 +454,7 @@ public void withoutTimestamps(final Runnable runnable) { * @param classes Which classes to disable the timestamp processing for * @param runnable The code to execute while the timestamp listeners are disabled */ - public void withoutTimestamps(final List classes, final Runnable runnable) { + public void withoutTimestamps(final List> classes, final Runnable runnable) { withoutDateCreated(classes, () -> withoutLastUpdated(classes, runnable)); } @@ -465,7 +466,7 @@ public void withoutTimestamps(final List classes, final Runnable runnable * @param clazz Which class to disable the timestamp processing for * @param runnable The code to execute while the timestamp listeners are disabled */ - public void withoutTimestamps(final Class clazz, final Runnable runnable) { + public void withoutTimestamps(final Class clazz, final Runnable runnable) { withoutDateCreated(clazz, () -> withoutLastUpdated(clazz, runnable)); } From 7ba8d626a56da2fdf285f8ea3bbbeb7f840aed3b Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 11:08:09 -0500 Subject: [PATCH 07/10] Deduplicate DefaultApplicationEventPublisher.publishEvent overloads publishEvent(ApplicationEvent) and publishEvent(Object) both iterated applicationListeners and applied the same SmartApplicationListener event/source-type filtering before dispatching; only the event-wrapping step differed. Extracted the shared iterate-filter-dispatch logic into a private dispatch(ApplicationEvent) method both overloads now call. Also narrows ConfigurableApplicationEventPublisher.addApplicationListener's parameter to ApplicationListener, matching ConfigurableApplicationContextEventPublisher's already-narrower signature. No behavior change; existing DefaultApplicationEventPublisherSpec coverage (95%/81%) verifies both overloads unchanged. --- ...nfigurableApplicationEventPublisher.groovy | 3 +- .../DefaultApplicationEventPublisher.groovy | 28 +++++++------------ 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationEventPublisher.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationEventPublisher.groovy index 57ea9a82440..7687be7319c 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationEventPublisher.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/ConfigurableApplicationEventPublisher.groovy @@ -19,6 +19,7 @@ package org.grails.datastore.gorm.events +import org.springframework.context.ApplicationEvent import org.springframework.context.ApplicationEventPublisher import org.springframework.context.ApplicationListener @@ -35,5 +36,5 @@ interface ConfigurableApplicationEventPublisher extends ApplicationEventPublishe * * @param listener The application listener */ - void addApplicationListener(ApplicationListener listener) + void addApplicationListener(ApplicationListener listener) } diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy index 4101072e38f..43fa9303b58 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy @@ -36,42 +36,34 @@ import org.springframework.context.event.SmartApplicationListener class DefaultApplicationEventPublisher implements ConfigurableApplicationEventPublisher { private List applicationListeners = [] + @Override void publishEvent(ApplicationEvent event) { - for (listener in applicationListeners) { - if (listener instanceof SmartApplicationListener) { - SmartApplicationListener smartApplicationListener = (SmartApplicationListener) listener - if (!smartApplicationListener.supportsEventType((Class) event.getClass())) { - continue - } - else if (!smartApplicationListener.supportsSourceType(event.source.getClass())) { - continue - } - } - listener.onApplicationEvent(event) - } + dispatch(event) } @Override void publishEvent(Object event) { + dispatch(new PayloadApplicationEvent(this, event)) + } + + private void dispatch(ApplicationEvent event) { for (listener in applicationListeners) { - def eventObject = new PayloadApplicationEvent(this, event) if (listener instanceof SmartApplicationListener) { SmartApplicationListener smartApplicationListener = (SmartApplicationListener) listener - if (!smartApplicationListener.supportsEventType((Class) eventObject.getClass())) { + if (!smartApplicationListener.supportsEventType((Class) event.getClass())) { continue } - else if (!smartApplicationListener.supportsSourceType(eventObject.source.getClass())) { + else if (!smartApplicationListener.supportsSourceType(event.source.getClass())) { continue } } - - listener.onApplicationEvent(eventObject) + listener.onApplicationEvent(event) } } @Override - void addApplicationListener(ApplicationListener listener) { + void addApplicationListener(ApplicationListener listener) { applicationListeners.add(listener) } } From 1ca4940ed4438b27faa77a9cafcbe84494361eaf Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 11:15:37 -0500 Subject: [PATCH 08/10] Fix MultiTenantEventListener IntelliJ warnings Renamed the local RxDatastoreClient variable in onApplicationEvent from datastoreClient to sourceClient, since it shadowed the class's own datastoreClient field (the source of the "might not be assigned" confusion) and made the two equals() calls read ambiguously. Both this.datastoreClient.equals(datastoreClient) calls are now datastoreClient == sourceClient, Groovy's idiomatic null-safe equals. No behavior change; full suite, jacoco, and codeStyle clean. --- .../gorm/rx/events/MultiTenantEventListener.groovy | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy index a7905c1cc88..e01ef112b7a 100644 --- a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy +++ b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/MultiTenantEventListener.groovy @@ -62,18 +62,18 @@ class MultiTenantEventListener implements PersistenceEventListener { @Override void onApplicationEvent(ApplicationEvent event) { if (supportsEventType(event.getClass())) { - RxDatastoreClient datastoreClient = (RxDatastoreClient) event.getSource() - Assert.notNull(datastoreClient, 'Datastore client should never be null from source event') + RxDatastoreClient sourceClient = (RxDatastoreClient) event.getSource() + Assert.notNull(sourceClient, 'Datastore client should never be null from source event') if (event instanceof PreQueryEvent) { PreQueryEvent preQueryEvent = (PreQueryEvent) event Query query = preQueryEvent.getQuery() PersistentEntity entity = query.getEntity() if (entity.isMultiTenant()) { - if (supportsSourceType(datastoreClient.getClass()) && this.datastoreClient.equals(datastoreClient)) { + if (supportsSourceType(sourceClient.getClass()) && datastoreClient == sourceClient) { TenantId tenantId = entity.getTenantId() if (tenantId != null) { - Serializable currentId = Tenants.currentId(datastoreClient.getClass()) + Serializable currentId = Tenants.currentId(sourceClient.getClass()) query.eq(tenantId.getName(), currentId) } } @@ -85,8 +85,8 @@ class MultiTenantEventListener implements PersistenceEventListener { if (entity.isMultiTenant()) { TenantId tenantId = entity.getTenantId() EntityReflector reflector = entity.getReflector() - if (supportsSourceType(datastoreClient.getClass()) && this.datastoreClient.equals(datastoreClient)) { - Serializable currentId = Tenants.currentId(datastoreClient.getClass()) + if (supportsSourceType(sourceClient.getClass()) && datastoreClient == sourceClient) { + Serializable currentId = Tenants.currentId(sourceClient.getClass()) if (currentId != null) { try { if (currentId == ConnectionSource.DEFAULT) { From 339a861c38104d5193bc2482e32d7490fa9e3049 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 14 Aug 2026 21:22:06 -0500 Subject: [PATCH 09/10] Replace .equals() with == in rx AutoTimestampEventListener/DomainEventListener Same idiomatic Groovy null-safe equals fix already applied to MultiTenantEventListener. No behavior change; full suite, jacoco, and codeStyle clean. --- .../grails/gorm/rx/events/AutoTimestampEventListener.groovy | 4 ++-- .../org/grails/gorm/rx/events/DomainEventListener.groovy | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/AutoTimestampEventListener.groovy b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/AutoTimestampEventListener.groovy index 9b0d377fa64..a7fc820c476 100644 --- a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/AutoTimestampEventListener.groovy +++ b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/AutoTimestampEventListener.groovy @@ -41,11 +41,11 @@ class AutoTimestampEventListener extends org.grails.datastore.gorm.events.AutoTi @Override protected boolean isValidSource(AbstractPersistenceEvent event) { Object source = event.getSource() - return (source instanceof RxDatastoreClient) && source.equals(datastoreClient) + return (source instanceof RxDatastoreClient) && source == datastoreClient } @Override boolean supportsSourceType(Class sourceType) { - datastoreClient.getClass().equals(sourceType) + datastoreClient.getClass() == sourceType } } diff --git a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/DomainEventListener.groovy b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/DomainEventListener.groovy index 6fba3f84e27..d43a7f75e55 100644 --- a/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/DomainEventListener.groovy +++ b/grails-datamapping-rx/src/main/groovy/org/grails/gorm/rx/events/DomainEventListener.groovy @@ -41,12 +41,12 @@ class DomainEventListener extends org.grails.datastore.gorm.events.DomainEventLi @Override protected boolean isValidSource(AbstractPersistenceEvent event) { Object source = event.getSource() - return (source instanceof RxDatastoreClient) && source.equals(datastoreClient) + return (source instanceof RxDatastoreClient) && source == datastoreClient } @Override boolean supportsSourceType(Class sourceType) { - datastoreClient.getClass().equals(sourceType) + datastoreClient.getClass() == sourceType } } From dcdb6144f7c6671bb427589022ab868a8254ef34 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Sun, 16 Aug 2026 10:34:56 -0500 Subject: [PATCH 10/10] Remove unnecessary cast in DefaultApplicationEventPublisher.dispatch event.getClass() already statically types as Class here, matching supportsEventType's parameter type, so the explicit cast was redundant. Co-Authored-By: Claude Sonnet 5 --- .../gorm/events/DefaultApplicationEventPublisher.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy index 43fa9303b58..b1d534a60ae 100644 --- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy +++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/events/DefaultApplicationEventPublisher.groovy @@ -51,7 +51,7 @@ class DefaultApplicationEventPublisher implements ConfigurableApplicationEventPu for (listener in applicationListeners) { if (listener instanceof SmartApplicationListener) { SmartApplicationListener smartApplicationListener = (SmartApplicationListener) listener - if (!smartApplicationListener.supportsEventType((Class) event.getClass())) { + if (!smartApplicationListener.supportsEventType(event.getClass())) { continue } else if (!smartApplicationListener.supportsSourceType(event.source.getClass())) {