From c3289a8722fd2d80fcc832d5e17792eaefb83769 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 9 Jul 2026 21:53:29 -0700 Subject: [PATCH 1/3] Make AutoTimestampEventListener temporary disabling thread-safe Fixes #14510. The withoutTimestamps, withoutDateCreated and withoutLastUpdated methods disabled timestamping by mutating the listener-wide entity maps (capturing each entry and setting it to Optional.empty()), which is JVM-global state. Overlapping windows on concurrent threads corrupted each other: a thread leaving its window re-enabled timestamping for every other thread still inside one, so historical dateCreated/lastUpdated values were silently overwritten. An exception inside the closure also left timestamping permanently disabled because the previous state was restored without try/finally. Suppression is now tracked per thread: the property-name getters consult a ThreadLocal holding an all-entities nesting depth plus the set of entity names disabled by per-class scopes. Each scope only re-enables the names it added, so nested and overlapping scopes restore correctly, and try/finally guarantees restoration when the closure throws. The shared metadata maps are no longer mutated. The CreatedBy/UpdatedBy auditing maps are untouched, as no withoutXxx API covers them. Behavioral note: disabling is now scoped to the calling thread; threads spawned inside the closure are no longer affected. The GORM auto-timestamping guide documents this. The spec now exercises the listener through beforeInsert/beforeUpdate and covers cross-thread isolation, overlapping windows, nested and overlapping same-class scopes, exception restoration and concurrent use. Forward-port of the 7.0.x fix (#15952) to 8.0.x, identical to the 7.1.x port (#15953), as this class is identical on 7.1.x through 8.0.x. --- .../eventsAutoTimestamping.adoc | 2 + .../events/AutoTimestampEventListener.java | 133 +++-- .../AutoTimestampEventListenerSpec.groovy | 510 ++++++++++++------ 3 files changed, 439 insertions(+), 206 deletions(-) diff --git a/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc b/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc index d71e8b52519..fdc6ae9904a 100644 --- a/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc +++ b/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc @@ -408,3 +408,5 @@ autoTimestampEventListener.withoutTimestamps { ---- WARNING: Because the timestamp handling is only disabled for the duration of the closure, you must flush the session during the closure execution! + +NOTE: The timestamp handling is only disabled for the thread executing the closure. Other threads persisting entities at the same time are unaffected and will continue to have their timestamps applied. 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 f8757121a74..c635fef29b7 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 @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -79,6 +78,9 @@ public class AutoTimestampEventListener extends AbstractPersistenceEventListener protected Map>> entitiesWithUpdatedBy = new ConcurrentHashMap<>(); protected Collection uninitializedEntities = new ConcurrentLinkedQueue<>(); + private final ThreadLocal disabledDateCreated = new ThreadLocal<>(); + private final ThreadLocal disabledLastUpdated = new ThreadLocal<>(); + private TimestampProvider timestampProvider = new DefaultTimestampProvider(); private AuditorAware auditorAware; @@ -219,11 +221,17 @@ public boolean beforeUpdate(PersistentEntity entity, EntityAccess ea) { } protected Set getLastUpdatedPropertyNames(String entityName) { + if (isDisabled(disabledLastUpdated, entityName)) { + return null; + } Optional> properties = entitiesWithLastUpdated.get(entityName); return properties == null ? null : properties.orElse(null); } protected Set getDateCreatedPropertyNames(String entityName) { + if (isDisabled(disabledDateCreated, entityName)) { + return null; + } Optional> properties = entitiesWithDateCreated.get(entityName); return properties == null ? null : properties.orElse(null); } @@ -311,53 +319,83 @@ public void setAuditorAware(AuditorAware auditorAware) { this.auditorAware = auditorAware; } - private void processAllEntries(final Set>>> entries, final Runnable runnable) { - Map>> originalValues = new LinkedHashMap<>(); - for (Map.Entry>> entry: entries) { - originalValues.put(entry.getKey(), entry.getValue()); - entry.setValue(Optional.empty()); + private static boolean isDisabled(final ThreadLocal disabledTimestamps, final String entityName) { + DisabledTimestamps disabled = disabledTimestamps.get(); + return disabled != null && disabled.isDisabled(entityName); + } + + private static DisabledTimestamps getOrCreateDisabled(final ThreadLocal disabledTimestamps) { + DisabledTimestamps disabled = disabledTimestamps.get(); + if (disabled == null) { + disabled = new DisabledTimestamps(); + disabledTimestamps.set(disabled); } - runnable.run(); - for (Map.Entry>> entry: entries) { - entry.setValue(originalValues.get(entry.getKey())); + return disabled; + } + + private static void removeIfEmpty(final ThreadLocal disabledTimestamps, final DisabledTimestamps disabled) { + if (disabled.isEmpty()) { + disabledTimestamps.remove(); } } - private void processEntries(final List classes, Map>> entities, final Runnable runnable) { - Set>>> entries = new HashSet<>(); - final List classNames = new ArrayList<>(classes.size()); - for (Class clazz: classes) { - classNames.add(clazz.getName()); + private static void runWithAllDisabled(final ThreadLocal disabledTimestamps, final Runnable runnable) { + DisabledTimestamps disabled = getOrCreateDisabled(disabledTimestamps); + disabled.allDisabledDepth++; + try { + runnable.run(); + } finally { + disabled.allDisabledDepth--; + removeIfEmpty(disabledTimestamps, disabled); } - for (Map.Entry>> entry: entities.entrySet()) { - if (classNames.contains(entry.getKey())) { - entries.add(entry); + } + + 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) { + String entityName = clazz.getName(); + if (disabled.entityNames.add(entityName)) { + added.add(entityName); + } } + runnable.run(); + } finally { + disabled.entityNames.removeAll(added); + removeIfEmpty(disabledTimestamps, disabled); } - processAllEntries(entries, runnable); } /** - * Temporarily disables the last updated processing during the execution of the runnable + * Temporarily disables the last updated processing during the execution of the runnable. + * The processing is only disabled for the calling thread; concurrently executing threads + * are unaffected. * * @param runnable The code to execute while the last updated listener is disabled */ public void withoutLastUpdated(final Runnable runnable) { - processAllEntries(entitiesWithLastUpdated.entrySet(), runnable); + runWithAllDisabled(disabledLastUpdated, runnable); } /** - * Temporarily disables the last updated processing only on the provided classes during the execution of the runnable + * Temporarily disables the last updated processing only on the provided classes during the + * execution of the runnable. The processing is only disabled for the calling thread; + * concurrently executing threads are unaffected. * * @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) { - processEntries(classes, entitiesWithLastUpdated, runnable); + runWithDisabled(disabledLastUpdated, classes, runnable); } /** - * Temporarily disables the last updated processing only on the provided class during the execution of the runnable + * Temporarily disables the last updated processing only on the provided class during the + * execution of the runnable. The processing is only disabled for the calling thread; + * concurrently executing threads are unaffected. * * @param clazz Which class to disable the last updated processing for * @param runnable The code to execute while the last updated listener is disabled @@ -369,26 +407,32 @@ public void withoutLastUpdated(final Class clazz, final Runnable runnable) { } /** - * Temporarily disables the date created processing during the execution of the runnable + * Temporarily disables the date created processing during the execution of the runnable. + * The processing is only disabled for the calling thread; concurrently executing threads + * are unaffected. * * @param runnable The code to execute while the date created listener is disabled */ public void withoutDateCreated(final Runnable runnable) { - processAllEntries(entitiesWithDateCreated.entrySet(), runnable); + runWithAllDisabled(disabledDateCreated, runnable); } /** - * Temporarily disables the date created processing only on the provided classes during the execution of the runnable + * Temporarily disables the date created processing only on the provided classes during the + * execution of the runnable. The processing is only disabled for the calling thread; + * concurrently executing threads are unaffected. * * @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) { - processEntries(classes, entitiesWithDateCreated, runnable); + runWithDisabled(disabledDateCreated, classes, runnable); } /** - * Temporarily disables the date created processing only on the provided class during the execution of the runnable + * Temporarily disables the date created processing only on the provided class during the + * execution of the runnable. The processing is only disabled for the calling thread; + * concurrently executing threads are unaffected. * * @param clazz Which class to disable the date created processing for * @param runnable The code to execute while the date created listener is disabled @@ -400,7 +444,9 @@ public void withoutDateCreated(final Class clazz, final Runnable runnable) { } /** - * Temporarily disables the timestamp processing during the execution of the runnable + * Temporarily disables the timestamp processing during the execution of the runnable. + * The processing is only disabled for the calling thread; concurrently executing threads + * are unaffected. * * @param runnable The code to execute while the timestamp listeners are disabled */ @@ -409,7 +455,9 @@ public void withoutTimestamps(final Runnable runnable) { } /** - * Temporarily disables the timestamp processing only on the provided classes during the execution of the runnable + * Temporarily disables the timestamp processing only on the provided classes during the + * execution of the runnable. The processing is only disabled for the calling thread; + * concurrently executing threads are unaffected. * * @param classes Which classes to disable the timestamp processing for * @param runnable The code to execute while the timestamp listeners are disabled @@ -419,7 +467,9 @@ public void withoutTimestamps(final List classes, final Runnable runnable } /** - * Temporarily disables the timestamp processing during the execution of the runnable + * Temporarily disables the timestamp processing during the execution of the runnable. + * The processing is only disabled for the calling thread; concurrently executing threads + * are unaffected. * * @param clazz Which class to disable the timestamp processing for * @param runnable The code to execute while the timestamp listeners are disabled @@ -428,4 +478,25 @@ public void withoutTimestamps(final Class clazz, final Runnable runnable) { withoutDateCreated(clazz, () -> withoutLastUpdated(clazz, runnable)); } + /** + * The entities for which timestamp processing is temporarily disabled on the current thread. + * All entities are disabled while {@code allDisabledDepth} is greater than zero; otherwise + * only the entities named in {@code entityNames} are disabled. + */ + private static final class DisabledTimestamps { + + private int allDisabledDepth; + + private final Set entityNames = new HashSet<>(); + + private boolean isDisabled(String entityName) { + return allDisabledDepth > 0 || entityNames.contains(entityName); + } + + private boolean isEmpty() { + return allDisabledDepth == 0 && entityNames.isEmpty(); + } + + } + } diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy index 289351b7253..f05d7466596 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy @@ -19,270 +19,437 @@ package org.grails.datastore.gorm.timestamp +import java.util.concurrent.Callable +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.CountDownLatch +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.util.concurrent.TimeUnit + import groovy.transform.InheritConstructors +import spock.lang.Specification + import org.grails.datastore.gorm.events.AutoTimestampEventListener import org.grails.datastore.mapping.core.Datastore +import org.grails.datastore.mapping.engine.EntityAccess import org.grails.datastore.mapping.model.MappingContext -import spock.lang.Specification +import org.grails.datastore.mapping.model.PersistentEntity class AutoTimestampEventListenerSpec extends Specification { + private static final Set BOTH_TIMESTAMPS = ['dateCreated', 'lastUpdated'] as Set + private static final Set DATE_CREATED_ONLY = ['dateCreated'] as Set + private static final Set LAST_UPDATED_ONLY = ['lastUpdated'] as Set + TestEventListener listener - Map>> lastUpdatedBaseState = [:] - Map>> dateCreatedBaseState = [:] void setup() { listener = new TestEventListener(Stub(Datastore) { getMappingContext() >> null }) - [Foo, Bar, FooBar].each { - lastUpdatedBaseState.put(it.getName(), Optional.of(['lastUpdated'] as Set)) - dateCreatedBaseState.put(it.getName(), Optional.of(['dateCreated'] as Set)) - } } - void updateBaseStates() { - listener.dateCreated.entrySet().each { - dateCreatedBaseState.put(it.key, it.value) - } - listener.lastUpdated.entrySet().each { - lastUpdatedBaseState.put(it.key, it.value) - } + void "timestamps are applied on insert and update by default"() { + expect: + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY } - void "test withoutLastUpdated"() { + void "withoutLastUpdated disables lastUpdated for all entities only while the closure runs"() { + given: + Map fooInsertInside = null + Map barUpdateInside = null + when: listener.withoutLastUpdated { - updateBaseStates() + fooInsertInside = appliedOnInsert(Foo) + barUpdateInside = appliedOnUpdate(Bar) } then: - lastUpdatedBaseState[Foo.getName()].isEmpty() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isEmpty() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + fooInsertInside.keySet() == DATE_CREATED_ONLY + barUpdateInside.isEmpty() - when: - updateBaseStates() - - then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + and: 'timestamping resumes once the closure completes' + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Bar).keySet() == LAST_UPDATED_ONLY } - void "test withoutLastUpdated(Class)"() { + void "withoutLastUpdated(Class) only affects the given class"() { + given: + Map barInside = null + Map fooInside = null + when: listener.withoutLastUpdated(Bar) { - updateBaseStates() + barInside = appliedOnInsert(Bar) + fooInside = appliedOnInsert(Foo) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() - - when: - updateBaseStates() + barInside.keySet() == DATE_CREATED_ONLY + fooInside.keySet() == BOTH_TIMESTAMPS - then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS } - void "test withoutLastUpdated(Class[])"() { + void "withoutLastUpdated(List) only affects the given classes"() { + given: + Map barInside = null + Map fooBarInside = null + Map fooInside = null + when: listener.withoutLastUpdated([Bar, FooBar]) { - updateBaseStates() + barInside = appliedOnInsert(Bar) + fooBarInside = appliedOnInsert(FooBar) + fooInside = appliedOnInsert(Foo) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isEmpty() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + barInside.keySet() == DATE_CREATED_ONLY + fooBarInside.keySet() == DATE_CREATED_ONLY + fooInside.keySet() == BOTH_TIMESTAMPS + + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS + appliedOnInsert(FooBar).keySet() == BOTH_TIMESTAMPS + } + + void "withoutDateCreated disables dateCreated for all entities only while the closure runs"() { + given: + Map fooInside = null + Map barInside = null when: - updateBaseStates() + listener.withoutDateCreated { + fooInside = appliedOnInsert(Foo) + barInside = appliedOnInsert(Bar) + } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + fooInside.keySet() == LAST_UPDATED_ONLY + barInside.keySet() == LAST_UPDATED_ONLY + + and: + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS } - void "test withoutDateCreated"() { + void "withoutDateCreated(Class) only affects the given class"() { + given: + Map barInside = null + Map fooInside = null + when: - listener.withoutDateCreated() { - updateBaseStates() + listener.withoutDateCreated(Bar) { + barInside = appliedOnInsert(Bar) + fooInside = appliedOnInsert(Foo) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isEmpty() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isEmpty() + barInside.keySet() == LAST_UPDATED_ONLY + fooInside.keySet() == BOTH_TIMESTAMPS + + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS + } + + void "withoutDateCreated(List) only affects the given classes"() { + given: + Map barInside = null + Map fooBarInside = null + Map fooInside = null when: - updateBaseStates() + listener.withoutDateCreated([Bar, FooBar]) { + barInside = appliedOnInsert(Bar) + fooBarInside = appliedOnInsert(FooBar) + fooInside = appliedOnInsert(Foo) + } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + barInside.keySet() == LAST_UPDATED_ONLY + fooBarInside.keySet() == LAST_UPDATED_ONLY + fooInside.keySet() == BOTH_TIMESTAMPS + + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS + appliedOnInsert(FooBar).keySet() == BOTH_TIMESTAMPS } - void "test withoutDateCreated(Class)"() { + void "withoutTimestamps disables all timestamp handling only while the closure runs"() { + given: + Map fooInsertInside = null + Map fooUpdateInside = null + Map barInsertInside = null + when: - listener.withoutDateCreated(Bar) { - updateBaseStates() + listener.withoutTimestamps { + fooInsertInside = appliedOnInsert(Foo) + fooUpdateInside = appliedOnUpdate(Foo) + barInsertInside = appliedOnInsert(Bar) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isPresent() + fooInsertInside.isEmpty() + fooUpdateInside.isEmpty() + barInsertInside.isEmpty() + + and: + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY + } + + void "withoutTimestamps(Class) only affects the given class"() { + given: + Map barInside = null + Map fooInside = null when: - updateBaseStates() + listener.withoutTimestamps(Bar) { + barInside = appliedOnInsert(Bar) + fooInside = appliedOnInsert(Foo) + } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + barInside.isEmpty() + fooInside.keySet() == BOTH_TIMESTAMPS + + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS } - void "test withoutDateCreated(Class[])"() { + void "withoutTimestamps(List) only affects the given classes"() { + given: + Map barInside = null + Map fooBarInside = null + Map fooInside = null + when: - listener.withoutDateCreated([Bar, FooBar]) { - updateBaseStates() + listener.withoutTimestamps([Bar, FooBar]) { + barInside = appliedOnInsert(Bar) + fooBarInside = appliedOnInsert(FooBar) + fooInside = appliedOnInsert(Foo) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isEmpty() + barInside.isEmpty() + fooBarInside.isEmpty() + fooInside.keySet() == BOTH_TIMESTAMPS + + and: + appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS + appliedOnInsert(FooBar).keySet() == BOTH_TIMESTAMPS + } + + void "nested disabling restores the enclosing scope"() { + given: + Map fooInner = null + Map fooOuter = null + Map barOuter = null when: - updateBaseStates() + listener.withoutLastUpdated(Foo) { + listener.withoutTimestamps { + fooInner = appliedOnInsert(Foo) + } + fooOuter = appliedOnInsert(Foo) + barOuter = appliedOnInsert(Bar) + } - then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + then: 'everything is disabled inside the nested closure' + fooInner.isEmpty() + + and: 'only the outer suppression remains after the nested closure completes' + fooOuter.keySet() == DATE_CREATED_ONLY + barOuter.keySet() == BOTH_TIMESTAMPS + + and: 'all timestamping resumes after the outer closure completes' + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS } + void "overlapping scopes disabling the same class restore the enclosing scope"() { + given: + Map fooInner = null + Map barInner = null + Map fooBetween = null + Map barBetween = null - void "test withoutTimestamps"() { when: - listener.withoutTimestamps() { - updateBaseStates() + listener.withoutDateCreated(Foo) { + listener.withoutDateCreated([Foo, Bar]) { + fooInner = appliedOnInsert(Foo) + barInner = appliedOnInsert(Bar) + } + fooBetween = appliedOnInsert(Foo) + barBetween = appliedOnInsert(Bar) } - then: - lastUpdatedBaseState[Foo.getName()].isEmpty() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isEmpty() - dateCreatedBaseState[Foo.getName()].isEmpty() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isEmpty() + then: 'both classes are disabled inside the inner scope' + fooInner.keySet() == LAST_UPDATED_ONLY + barInner.keySet() == LAST_UPDATED_ONLY - when: - updateBaseStates() + and: 'Foo stays disabled in the outer scope after the inner scope exits' + fooBetween.keySet() == LAST_UPDATED_ONLY + barBetween.keySet() == BOTH_TIMESTAMPS + + and: 'everything is restored after the outer scope exits' + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + } + + void "no suppression remains when registering a class list fails part way"() { + when: 'the second element blows up after the first has already been registered' + listener.withoutTimestamps([Foo, null]) { + throw new IllegalStateException('never reached') + } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + thrown(NullPointerException) + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY } - void "test withoutTimestamps(Class)"() { + void "timestamp processing is restored when the closure throws"() { when: - listener.withoutTimestamps(Bar) { - updateBaseStates() + listener.withoutTimestamps { + throw new IllegalStateException('failure inside withoutTimestamps') } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isPresent() + thrown(IllegalStateException) + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY + } - when: - updateBaseStates() + void "disabling timestamps on one thread does not affect other threads"() { + given: + CountDownLatch entered = new CountDownLatch(1) + CountDownLatch release = new CountDownLatch(1) + Thread worker = new Thread({ + listener.withoutTimestamps { + entered.countDown() + release.await(10, TimeUnit.SECONDS) + } + }) + + when: 'this thread persists while the worker holds an open withoutTimestamps window' + worker.start() + boolean workerEntered = entered.await(10, TimeUnit.SECONDS) + Map insertApplied = appliedOnInsert(Foo) + Map updateApplied = appliedOnUpdate(Foo) + release.countDown() + worker.join(10000) then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + workerEntered + insertApplied.keySet() == BOTH_TIMESTAMPS + updateApplied.keySet() == LAST_UPDATED_ONLY + } + + void "overlapping windows on different threads restore independently"() { + given: + CountDownLatch aEntered = new CountDownLatch(1) + CountDownLatch bEntered = new CountDownLatch(1) + CountDownLatch aExited = new CountDownLatch(1) + Map insideB = null + Map insideBAfterAExited = null + Map afterA = null + + Thread threadA = new Thread({ + listener.withoutTimestamps(Foo) { + aEntered.countDown() + bEntered.await(10, TimeUnit.SECONDS) + } + afterA = appliedOnInsert(Foo) + aExited.countDown() + }) + Thread threadB = new Thread({ + aEntered.await(10, TimeUnit.SECONDS) + listener.withoutTimestamps(Foo) { + insideB = appliedOnInsert(Foo) + bEntered.countDown() + aExited.await(10, TimeUnit.SECONDS) + insideBAfterAExited = appliedOnInsert(Foo) + } + }) + + when: + threadA.start() + threadB.start() + threadA.join(15000) + threadB.join(15000) + + then: 'thread B stays disabled for its whole window, even after thread A restores' + insideB.isEmpty() + insideBAfterAExited.isEmpty() + + and: 'thread A is re-enabled as soon as its own window closes' + afterA.keySet() == BOTH_TIMESTAMPS + + and: 'the test thread was never affected' + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS } - void "test withoutTimestamps(Class[])"() { + void "suppression stays thread local under concurrent use"() { + given: + int threadCount = 8 + int iterations = 25 + ExecutorService pool = Executors.newFixedThreadPool(threadCount) + CountDownLatch start = new CountDownLatch(1) + when: - listener.withoutTimestamps([Bar, FooBar]) { - updateBaseStates() + List> outcomes = (1..threadCount).collect { int n -> + pool.submit({ + start.await(10, TimeUnit.SECONDS) + boolean consistent = true + iterations.times { + if (n % 2 == 0) { + listener.withoutTimestamps { + consistent &= appliedOnInsert(Foo).isEmpty() + } + consistent &= appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + } else { + consistent &= appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + consistent &= appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY + } + } + consistent + } as Callable) } + start.countDown() + List results = outcomes.collect { it.get(30, TimeUnit.SECONDS) } then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isEmpty() - lastUpdatedBaseState[FooBar.getName()].isEmpty() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isEmpty() - dateCreatedBaseState[FooBar.getName()].isEmpty() + results.every { it } - when: - updateBaseStates() + cleanup: + pool.shutdownNow() + } - then: - lastUpdatedBaseState[Foo.getName()].isPresent() - lastUpdatedBaseState[Bar.getName()].isPresent() - lastUpdatedBaseState[FooBar.getName()].isPresent() - dateCreatedBaseState[Foo.getName()].isPresent() - dateCreatedBaseState[Bar.getName()].isPresent() - dateCreatedBaseState[FooBar.getName()].isPresent() + private Map appliedOnInsert(Class clazz) { + Map applied = new ConcurrentHashMap<>() + listener.beforeInsert(entityFor(clazz), recordingAccess(applied)) + applied + } + + private Map appliedOnUpdate(Class clazz) { + Map applied = new ConcurrentHashMap<>() + listener.beforeUpdate(entityFor(clazz), recordingAccess(applied)) + applied + } + + private static PersistentEntity entityFor(Class clazz) { + [getName: { -> clazz.name }] as PersistentEntity + } + + private static EntityAccess recordingAccess(Map applied) { + [ + getPropertyValue: { String name -> null }, + getPropertyType : { String name -> Date }, + setProperty : { String name, Object value -> applied.put(name, value) } + ] as EntityAccess } } @@ -301,17 +468,10 @@ class FooBar { @InheritConstructors class TestEventListener extends AutoTimestampEventListener { - Map>> getDateCreated() { - this.entitiesWithDateCreated - } - Map>> getLastUpdated() { - this.entitiesWithLastUpdated - } - protected void initForMappingContext(MappingContext mappingContext) { [Foo, Bar, FooBar].each { entitiesWithLastUpdated.put(it.getName(), Optional.of(['lastUpdated'] as Set)) entitiesWithDateCreated.put(it.getName(), Optional.of(['dateCreated'] as Set)) } } -} \ No newline at end of file +} From 18e9fa3a8a20f93a4f50b742bef214174a5e63f9 Mon Sep 17 00:00:00 2001 From: James Daugherty Date: Fri, 10 Jul 2026 08:28:59 -0400 Subject: [PATCH 2/3] docs, multi-thread documentation, and removing dead code --- .../eventsAutoTimestamping.adoc | 16 ++ .../eventsAutoTimestamping.adoc | 18 ++ .../events/AutoTimestampEventListener.java | 104 +++++++++-- .../AutoTimestampEventListenerSpec.groovy | 163 ++++++++++++++++++ 4 files changed, 287 insertions(+), 14 deletions(-) diff --git a/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc b/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc index fdc6ae9904a..aa77c905a0f 100644 --- a/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc +++ b/grails-data-hibernate5/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc @@ -410,3 +410,19 @@ autoTimestampEventListener.withoutTimestamps { WARNING: Because the timestamp handling is only disabled for the duration of the closure, you must flush the session during the closure execution! NOTE: The timestamp handling is only disabled for the thread executing the closure. Other threads persisting entities at the same time are unaffected and will continue to have their timestamps applied. + +If work inside the closure runs on other threads (for example an executor-based import), the suppression must be propagated to those threads explicitly. Capture the state with `captureTimestampSuppression` and apply it on the executing thread with `withTimestampSuppression`: + +[source,groovy] +---- +autoTimestampEventListener.withoutTimestamps { + def suppression = autoTimestampEventListener.captureTimestampSuppression() + executor.submit { + autoTimestampEventListener.withTimestampSuppression(suppression) { + new Foo(dateCreated: new Date() - 2, lastUpdated: new Date() - 1).save(flush: true) + } + }.get() +} +---- + +The captured state is immutable and may be applied on any number of threads, even after the capturing closure has completed. diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc index 31c6305bfac..06677831b1e 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/advancedGORMFeatures/eventsAutoTimestamping.adoc @@ -408,3 +408,21 @@ autoTimestampEventListener.withoutTimestamps { ---- WARNING: Because the timestamp handling is only disabled for the duration of the closure, you must flush the session during the closure execution! + +NOTE: The timestamp handling is only disabled for the thread executing the closure. Other threads persisting entities at the same time are unaffected and will continue to have their timestamps applied. + +If work inside the closure runs on other threads (for example an executor-based import), the suppression must be propagated to those threads explicitly. Capture the state with `captureTimestampSuppression` and apply it on the executing thread with `withTimestampSuppression`: + +[source,groovy] +---- +autoTimestampEventListener.withoutTimestamps { + def suppression = autoTimestampEventListener.captureTimestampSuppression() + executor.submit { + autoTimestampEventListener.withTimestampSuppression(suppression) { + new Foo(dateCreated: new Date() - 2, lastUpdated: new Date() - 1).save(flush: true) + } + }.get() +} +---- + +The captured state is immutable and may be applied on any number of threads, even after the capturing closure has completed. 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 c635fef29b7..8ecf4e12bb9 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 @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -277,25 +278,15 @@ protected void storeDateCreatedAndLastUpdatedInfo(PersistentEntity persistentEnt protected void storeTimestampAvailability(Map>> timestampAvailabilityMap, PersistentEntity persistentEntity, PersistentProperty property) { if (property != null && timestampProvider.supportsCreating(property.getType())) { - Optional> timestampProperties = timestampAvailabilityMap.computeIfAbsent(persistentEntity.getName(), k -> Optional.of(new HashSet<>())); - if (timestampProperties.isPresent()) { - timestampProperties.get().add(property.getName()); - } - else { - throw new IllegalStateException("Timestamp properties for entity [" + persistentEntity.getName() + "] have been disabled. Cannot add property [" + property.getName() + "]"); - } + timestampAvailabilityMap.computeIfAbsent(persistentEntity.getName(), k -> Optional.of(new HashSet<>())) + .ifPresent(propertyNames -> propertyNames.add(property.getName())); } } protected void storeAuditorAvailability(Map>> auditorAvailabilityMap, PersistentEntity persistentEntity, PersistentProperty property) { if (property != null) { - Optional> auditorProperties = auditorAvailabilityMap.computeIfAbsent(persistentEntity.getName(), k -> Optional.of(new HashSet<>())); - if (auditorProperties.isPresent()) { - auditorProperties.get().add(property.getName()); - } - else { - throw new IllegalStateException("Auditor properties for entity [" + persistentEntity.getName() + "] have been disabled. Cannot add property [" + property.getName() + "]"); - } + auditorAvailabilityMap.computeIfAbsent(persistentEntity.getName(), k -> Optional.of(new HashSet<>())) + .ifPresent(propertyNames -> propertyNames.add(property.getName())); } } @@ -478,6 +469,83 @@ public void withoutTimestamps(final Class clazz, final Runnable runnable) { withoutDateCreated(clazz, () -> withoutLastUpdated(clazz, runnable)); } + /** + * Captures the timestamp suppression state currently active on the calling thread, so that + * it can be re-applied on another thread with + * {@link #withTimestampSuppression(TimestampSuppression, Runnable)}. Because the + * {@code withoutTimestamps}, {@code withoutDateCreated} and {@code withoutLastUpdated} + * windows only affect the calling thread, work handed off to other threads (an executor, + * a reactive scheduler) is not suppressed unless the captured state is propagated to the + * executing thread. + * + *

The returned snapshot is immutable and may be applied on any number of threads, + * concurrently, including after the originating window has closed. + * + * @return the suppression state active on the calling thread at the time of the call + */ + public TimestampSuppression captureTimestampSuppression() { + return new TimestampSuppression(disabledDateCreated.get(), disabledLastUpdated.get()); + } + + /** + * Executes the runnable with the given captured suppression state installed on the calling + * thread, restoring the thread's previous suppression state afterwards, even when the + * runnable throws. The snapshot replaces any suppression already active on the calling + * thread for the duration of the runnable. + * + * @param suppression The state captured by {@link #captureTimestampSuppression()} + * @param runnable The code to execute with the captured suppression state applied + */ + public void withTimestampSuppression(final TimestampSuppression suppression, final Runnable runnable) { + Objects.requireNonNull(suppression, "suppression must not be null"); + DisabledTimestamps previousDateCreated = installSuppression(disabledDateCreated, suppression.dateCreated); + DisabledTimestamps previousLastUpdated = installSuppression(disabledLastUpdated, suppression.lastUpdated); + try { + runnable.run(); + } finally { + restoreSuppression(disabledLastUpdated, previousLastUpdated); + restoreSuppression(disabledDateCreated, previousDateCreated); + } + } + + private static DisabledTimestamps installSuppression(final ThreadLocal disabledTimestamps, final DisabledTimestamps snapshot) { + DisabledTimestamps previous = disabledTimestamps.get(); + if (snapshot == null) { + disabledTimestamps.remove(); + } else { + // install a copy: nested scopes inside the runnable mutate their thread's instance, + // which must not corrupt the shared snapshot or other threads applying it + disabledTimestamps.set(new DisabledTimestamps(snapshot)); + } + return previous; + } + + private static void restoreSuppression(final ThreadLocal disabledTimestamps, final DisabledTimestamps previous) { + if (previous == null) { + disabledTimestamps.remove(); + } else { + disabledTimestamps.set(previous); + } + } + + /** + * An immutable snapshot of a thread's temporary timestamp suppression state, captured with + * {@link AutoTimestampEventListener#captureTimestampSuppression()} and applied on another + * thread with {@link AutoTimestampEventListener#withTimestampSuppression(TimestampSuppression, Runnable)}. + */ + public static final class TimestampSuppression { + + private final DisabledTimestamps dateCreated; + + private final DisabledTimestamps lastUpdated; + + private TimestampSuppression(DisabledTimestamps dateCreated, DisabledTimestamps lastUpdated) { + this.dateCreated = dateCreated == null || dateCreated.isEmpty() ? null : new DisabledTimestamps(dateCreated); + this.lastUpdated = lastUpdated == null || lastUpdated.isEmpty() ? null : new DisabledTimestamps(lastUpdated); + } + + } + /** * The entities for which timestamp processing is temporarily disabled on the current thread. * All entities are disabled while {@code allDisabledDepth} is greater than zero; otherwise @@ -489,6 +557,14 @@ private static final class DisabledTimestamps { private final Set entityNames = new HashSet<>(); + private DisabledTimestamps() { + } + + private DisabledTimestamps(DisabledTimestamps source) { + allDisabledDepth = source.allDisabledDepth; + entityNames.addAll(source.entityNames); + } + private boolean isDisabled(String entityName) { return allDisabledDepth > 0 || entityNames.contains(entityName); } diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy index f05d7466596..390ce3843e7 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy @@ -428,6 +428,169 @@ class AutoTimestampEventListenerSpec extends Specification { pool.shutdownNow() } + void "captured suppression can be applied on another thread"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + listener.withoutTimestamps(Foo) { + suppression = listener.captureTimestampSuppression() + } + Map fooInside = null + Map barInside = null + Map fooAfter = null + + when: 'a worker thread applies the state captured inside the window' + Thread worker = new Thread({ + listener.withTimestampSuppression(suppression) { + fooInside = appliedOnInsert(Foo) + barInside = appliedOnInsert(Bar) + } + fooAfter = appliedOnInsert(Foo) + }) + worker.start() + worker.join(10000) + + then: 'the captured Foo suppression applies on the worker thread' + fooInside.isEmpty() + barInside.keySet() == BOTH_TIMESTAMPS + + and: 'the worker thread is restored once the block completes' + fooAfter.keySet() == BOTH_TIMESTAMPS + + and: 'the capturing thread is unaffected' + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + } + + void "captured suppression outlives the originating window and is reusable"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + listener.withoutLastUpdated { + suppression = listener.captureTimestampSuppression() + } + Map firstUse = null + Map secondUse = null + + when: + listener.withTimestampSuppression(suppression) { + firstUse = appliedOnInsert(Foo) + } + listener.withTimestampSuppression(suppression) { + secondUse = appliedOnInsert(Foo) + } + + then: + firstUse.keySet() == DATE_CREATED_ONLY + secondUse.keySet() == DATE_CREATED_ONLY + + and: + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + } + + void "an empty captured suppression temporarily replaces the thread's own suppression"() { + given: + AutoTimestampEventListener.TimestampSuppression noSuppression = listener.captureTimestampSuppression() + Map insideCapture = null + Map afterCapture = null + + when: + listener.withoutTimestamps { + listener.withTimestampSuppression(noSuppression) { + insideCapture = appliedOnInsert(Foo) + } + afterCapture = appliedOnInsert(Foo) + } + + then: 'the empty snapshot re-enables timestamping while installed' + insideCapture.keySet() == BOTH_TIMESTAMPS + + and: 'the enclosing window is restored once the snapshot block completes' + afterCapture.isEmpty() + } + + void "the previous suppression state is restored when the propagated runnable throws"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + listener.withoutTimestamps { + suppression = listener.captureTimestampSuppression() + } + + when: + listener.withTimestampSuppression(suppression) { + throw new IllegalStateException('failure inside withTimestampSuppression') + } + + then: + thrown(IllegalStateException) + appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + appliedOnUpdate(Foo).keySet() == LAST_UPDATED_ONLY + } + + void "nested scopes inside an applied suppression do not corrupt the snapshot"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + listener.withoutDateCreated(Foo) { + suppression = listener.captureTimestampSuppression() + } + Map barNested = null + Map secondUseFoo = null + Map secondUseBar = null + + when: 'the first application opens and closes a nested scope for another class' + listener.withTimestampSuppression(suppression) { + listener.withoutDateCreated(Bar) { + barNested = appliedOnInsert(Bar) + } + } + listener.withTimestampSuppression(suppression) { + secondUseFoo = appliedOnInsert(Foo) + secondUseBar = appliedOnInsert(Bar) + } + + then: + barNested.keySet() == LAST_UPDATED_ONLY + + and: 'the second application still reflects only the captured state' + secondUseFoo.keySet() == LAST_UPDATED_ONLY + secondUseBar.keySet() == BOTH_TIMESTAMPS + } + + void "one captured suppression can be applied concurrently on multiple threads"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + listener.withoutTimestamps(Foo) { + suppression = listener.captureTimestampSuppression() + } + int threadCount = 8 + ExecutorService pool = Executors.newFixedThreadPool(threadCount) + CountDownLatch start = new CountDownLatch(1) + + when: + List> outcomes = (1..threadCount).collect { + pool.submit({ + start.await(10, TimeUnit.SECONDS) + boolean consistent = true + 25.times { + listener.withTimestampSuppression(suppression) { + consistent &= appliedOnInsert(Foo).isEmpty() + listener.withoutTimestamps(Bar) { + consistent &= appliedOnInsert(Bar).isEmpty() + } + consistent &= appliedOnInsert(Bar).keySet() == BOTH_TIMESTAMPS + } + consistent &= appliedOnInsert(Foo).keySet() == BOTH_TIMESTAMPS + } + consistent + } as Callable) + } + start.countDown() + List results = outcomes.collect { it.get(30, TimeUnit.SECONDS) } + + then: + results.every { it } + + cleanup: + pool.shutdownNow() + } + private Map appliedOnInsert(Class clazz) { Map applied = new ConcurrentHashMap<>() listener.beforeInsert(entityFor(clazz), recordingAccess(applied)) From 43e59b4593e802ed339417a36b5b57ab8c99218d Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Fri, 10 Jul 2026 09:27:08 -0700 Subject: [PATCH 3/3] Cover the codecov-flagged branches in AutoTimestampEventListener Two additions to the spec: - Registering auditor properties through persistentEntityAdded and asserting CreatedBy/LastModifiedBy population on insert and update, which executes the storeAuditorAvailability body. - Capturing a suppression snapshot inside a window opened with an empty class list, where the thread-local state is present but empty, which takes the isEmpty() branch of both TimestampSuppression constructor ternaries. Also pins that an empty class list disables nothing and that the resulting snapshot suppresses nothing. --- .../AutoTimestampEventListenerSpec.groovy | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy index 390ce3843e7..743cbe2346f 100644 --- a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy +++ b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/timestamp/AutoTimestampEventListenerSpec.groovy @@ -30,11 +30,17 @@ import java.util.concurrent.TimeUnit import groovy.transform.InheritConstructors import spock.lang.Specification +import grails.gorm.annotation.CreatedBy +import grails.gorm.annotation.LastModifiedBy import org.grails.datastore.gorm.events.AutoTimestampEventListener +import org.grails.datastore.mapping.config.Property import org.grails.datastore.mapping.core.Datastore import org.grails.datastore.mapping.engine.EntityAccess +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 class AutoTimestampEventListenerSpec extends Specification { @@ -591,6 +597,68 @@ class AutoTimestampEventListenerSpec extends Specification { pool.shutdownNow() } + void "a snapshot captured inside an empty class-list window suppresses nothing"() { + given: + AutoTimestampEventListener.TimestampSuppression suppression = null + Map insideWindow = null + Map applied = null + + when: + listener.withoutTimestamps([]) { + insideWindow = appliedOnInsert(Foo) + suppression = listener.captureTimestampSuppression() + } + listener.withTimestampSuppression(suppression) { + applied = appliedOnInsert(Foo) + } + + then: 'an empty class list disables nothing inside the window' + insideWindow.keySet() == BOTH_TIMESTAMPS + + and: 'the captured snapshot is empty and suppresses nothing when applied' + applied.keySet() == BOTH_TIMESTAMPS + } + + void "auditor properties registered via persistentEntityAdded are populated on insert and update"() { + given: + listener.setAuditorAware([getCurrentAuditor: { -> Optional.of('testUser') }] as AuditorAware) + listener.persistentEntityAdded(auditedEntity()) + + when: + Map inserted = appliedOnInsert(Audited) + Map updated = appliedOnUpdate(Audited) + + then: + inserted == [createdBy: 'testUser', lastModifiedBy: 'testUser'] + updated == [lastModifiedBy: 'testUser'] + } + + private PersistentEntity auditedEntity() { + PersistentEntity entity = null + PersistentProperty createdBy = auditedProperty('createdBy') { -> entity } + PersistentProperty lastModifiedBy = auditedProperty('lastModifiedBy') { -> entity } + ClassMapping classMapping = [getMappedForm: { -> null }] as ClassMapping + entity = [ + getName : { -> Audited.name }, + isInitialized : { -> true }, + getMapping : { -> classMapping }, + getJavaClass : { -> Audited }, + getPersistentProperties: { -> [createdBy, lastModifiedBy] } + ] as PersistentEntity + entity + } + + private static PersistentProperty auditedProperty(String name, Closure owner) { + Property mappedForm = new Property() + PropertyMapping mapping = [getMappedForm: { -> mappedForm }] as PropertyMapping + [ + getName : { -> name }, + getType : { -> String }, + getMapping: { -> mapping }, + getOwner : { -> owner.call() } + ] as PersistentProperty + } + private Map appliedOnInsert(Class clazz) { Map applied = new ConcurrentHashMap<>() listener.beforeInsert(entityFor(clazz), recordingAccess(applied)) @@ -620,6 +688,16 @@ class Foo { } +class Audited { + + @CreatedBy + String createdBy + + @LastModifiedBy + String lastModifiedBy + +} + class Bar { }