This is an automated email from the ASF dual-hosted git repository. danhaywood pushed a commit to branch CAUSEWAY-3675 in repository https://gitbox.apache.org/repos/asf/causeway.git
commit b3d723badca36c934177ae7e5931bc6a4260ff0b Author: danhaywood <[email protected]> AuthorDate: Tue Jan 16 16:09:07 2024 +0000 CAUSEWAY-3675: adds suppress-auto-flush functionality --- .../ROOT/pages/2024/2.0.0-RC5/mignotes.adoc | 23 ++++- .../core/config/CausewayConfiguration.java | 63 +++++++++++-- .../CausewayModuleCoreRuntimeServices.java | 4 - .../commons/CausewayModulePersistenceCommons.java | 7 ++ .../changetracking/EntityChangeTrackerDefault.java | 100 +++++++++++++-------- .../PreAndPostValueEvaluatorServiceDefault.java | 4 +- .../repository/RepositoryServiceDefault.java | 26 ++++-- 7 files changed, 173 insertions(+), 54 deletions(-) diff --git a/antora/components/relnotes/modules/ROOT/pages/2024/2.0.0-RC5/mignotes.adoc b/antora/components/relnotes/modules/ROOT/pages/2024/2.0.0-RC5/mignotes.adoc index ab856cb7e5..ad2e59b983 100644 --- a/antora/components/relnotes/modules/ROOT/pages/2024/2.0.0-RC5/mignotes.adoc +++ b/antora/components/relnotes/modules/ROOT/pages/2024/2.0.0-RC5/mignotes.adoc @@ -5,9 +5,7 @@ This page will be added to as development progresses. -== AuditTrailEntryRepository and others have simplified the API. - - +== AuditTrailEntryRepository and others have simplified the API (CAUSEWAY-3390) In this release the `AuditTrailEntryRepository` and similar is no longer parameterized by the entity type, and is also an interface. @@ -35,3 +33,22 @@ This applies to: For more details, see link:https://issues.apache.org/jira/browse/CAUSEWAY-3390[CAUSEWAY-3390]. + +== Auto-flush management (CAUSEWAY-3675) + +By default any queries executed by xref:refguide:applib:index/services/repository/RepositoryService.adoc[] are preceded by a flush first. +The rationale behind this is to make sure that everything that might have changed is in the database. + +However, with JDO at least, this behaviour can cause a `ConcurrentModificationException` when coupled with the xref:refguide:persistence:index/commons/integration/changetracking/EntityChangeTrackerDefault.adoc[] internal service, used by auditing. +This grabs the value of properties, including derived properties that are evaluated using a query. + +So now, xref:refguide:persistence:index/commons/integration/changetracking/EntityChangeTrackerDefault.adoc[] suppresses the autoflush. + +Changes: + +* To revert to the original behaviour, use the new `causeway.commons.persistence.entity-change-tracker.suppress-auto-flush` property. + +* The `causeway.core.runtime-services.repository=service.disable-auto-flush` configuration property has been renamed to `causeway.commons.persistence.repository-service.disable-auto-flush`. + +For more details, see link:https://issues.apache.org/jira/browse/CAUSEWAY-3675[CAUSEWAY-3675]. + diff --git a/core/config/src/main/java/org/apache/causeway/core/config/CausewayConfiguration.java b/core/config/src/main/java/org/apache/causeway/core/config/CausewayConfiguration.java index c6bd442731..7f05a9e4e1 100644 --- a/core/config/src/main/java/org/apache/causeway/core/config/CausewayConfiguration.java +++ b/core/config/src/main/java/org/apache/causeway/core/config/CausewayConfiguration.java @@ -57,8 +57,11 @@ import javax.validation.constraints.Min; import javax.validation.constraints.NotEmpty; import javax.validation.constraints.NotNull; +import org.apache.causeway.applib.query.Query; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.boot.info.BuildProperties; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.EnumerablePropertySource; @@ -1925,19 +1928,21 @@ public class CausewayConfiguration { private final RepositoryService repositoryService = new RepositoryService(); @Data public static class RepositoryService { + private boolean disableAutoFlush = false; + /** * Normally any queries are automatically preceded by flushing pending executions. * * <p> * This key allows this behaviour to be disabled. - * - * <p> - * NOTE: this key is redundant for JPA/EclipseLink, which supports its own auto-flush using - * <a href="https://www.eclipse.org/eclipselink/documentation/2.7/jpa/extensions/persistenceproperties_ref.htm#BABDHEEB";>eclipselink.persistence-context.flush-mode</a> * </p> + * + * @deprecated - use instead <code>causeway.persistence.commons.repository-service.disable-auto-flush</code> */ - private boolean disableAutoFlush = false; - + @DeprecatedConfigurationProperty(replacement = "causeway.persistence.commons.repository-service.disable-auto-flush") + public boolean isDisableAutoFlush() { + return disableAutoFlush; + } } private final ExceptionRecognizer exceptionRecognizer = new ExceptionRecognizer(); @@ -2015,6 +2020,52 @@ public class CausewayConfiguration { @Data public static class Persistence { + private final Commons commons = new Commons(); + @Data + public static class Commons { + + private final RepositoryService repositoryService = new RepositoryService(); + @Data + public static class RepositoryService { + + /** + * Normally any queries are automatically preceded by flushing pending executions. + * + * <p> + * This key allows this behaviour to be disabled. + * + * <p> + * NOTE: this key is redundant for JPA/EclipseLink, which supports its own auto-flush using + * <a href="https://www.eclipse.org/eclipselink/documentation/2.7/jpa/extensions/persistenceproperties_ref.htm#BABDHEEB";>eclipselink.persistence-context.flush-mode</a> + * </p> + */ + private boolean disableAutoFlush = false; + } + + private final EntityChangeTracker entityChangeTracker = new EntityChangeTracker(); + @Data + public static class EntityChangeTracker { + /** + * Normally any query submitted to {@link org.apache.causeway.applib.services.repository.RepositoryService#allMatches(Query)} will trigger a + * flush first, unless auto-flush has been {@link Core.RuntimeServices.RepositoryService#isDisableAutoFlush() disabled}. + * + * <p> + * However, this auto-flush behaviour can be troublesome if the query occurs as a side-effect of the evaluation of a derived property, + * whose value in turn is enlisted by an implementation of a subscriber (in particular {@link EntityPropertyChangeSubscriber}) which + * captures the value of all properties (both persisted and derived). However, this behaviour can (at least under JDO), result in a {@link java.util.ConcurrentModificationException}. + * </p> + * + * <p> + * By default, {@link EntityChangeTracker} will therefore temporarily suppress any auto-flushing while this is ongoing. The purpose + * of this configuration property is to never suppress, ie always autoflush. + * </p> + */ + private boolean suppressAutoFlush = true; + } + + } + + private final Schema schema = new Schema(); @Data public static class Schema { diff --git a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/CausewayModuleCoreRuntimeServices.java b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/CausewayModuleCoreRuntimeServices.java index 5810ce7d02..b90d5898bc 100644 --- a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/CausewayModuleCoreRuntimeServices.java +++ b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/CausewayModuleCoreRuntimeServices.java @@ -57,7 +57,6 @@ import org.apache.causeway.core.runtimeservices.publish.LifecycleCallbackNotifie import org.apache.causeway.core.runtimeservices.publish.ObjectLifecyclePublisherDefault; import org.apache.causeway.core.runtimeservices.recognizer.ExceptionRecognizerServiceDefault; import org.apache.causeway.core.runtimeservices.recognizer.dae.ExceptionRecognizerForDataAccessException; -import org.apache.causeway.core.runtimeservices.repository.RepositoryServiceDefault; import org.apache.causeway.core.runtimeservices.routing.RoutingServiceDefault; import org.apache.causeway.core.runtimeservices.scratchpad.ScratchpadDefault; import org.apache.causeway.core.runtimeservices.serializing.SerializingAdapterDefault; @@ -126,9 +125,6 @@ import org.apache.causeway.core.runtimeservices.xmlsnapshot.XmlSnapshotServiceDe // @Controller RoutingServiceDefault.class, - // @Repository's - RepositoryServiceDefault.class, - // @DomainService's TranslationServicePoMenu.class, diff --git a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/CausewayModulePersistenceCommons.java b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/CausewayModulePersistenceCommons.java index ac62cade36..c8da4c1e17 100644 --- a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/CausewayModulePersistenceCommons.java +++ b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/CausewayModulePersistenceCommons.java @@ -18,6 +18,8 @@ */ package org.apache.causeway.persistence.commons; +import org.apache.causeway.persistence.commons.integration.repository.RepositoryServiceDefault; + import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -33,7 +35,12 @@ import org.apache.causeway.persistence.commons.integration.changetracking.PreAnd // @Service's EntityChangeTrackerDefault.class, PreAndPostValueEvaluatorServiceDefault.class, + + // @Repository's + RepositoryServiceDefault.class, + }) public class CausewayModulePersistenceCommons { + public static final String NAMESPACE = "causeway.persistence.commons"; } diff --git a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/EntityChangeTrackerDefault.java b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/EntityChangeTrackerDefault.java index dbf8911f1b..62c70f0941 100644 --- a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/EntityChangeTrackerDefault.java +++ b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/EntityChangeTrackerDefault.java @@ -27,10 +27,16 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.LongAdder; import java.util.function.Function; +import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Provider; +import org.apache.causeway.core.config.CausewayConfiguration; +import org.apache.causeway.persistence.commons.CausewayModulePersistenceCommons; + +import org.apache.causeway.persistence.commons.integration.repository.RepositoryServiceDefault; + import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.Ordered; import org.springframework.lang.Nullable; @@ -90,7 +96,7 @@ import lombok.extern.log4j.Log4j2; */ @Service @TransactionScope -@Named("causeway.persistence.commons.EntityChangeTrackerDefault") +@Named(CausewayModulePersistenceCommons.NAMESPACE + ".EntityChangeTrackerDefault") @Qualifier("default") @RequiredArgsConstructor(onConstructor_ = {@Inject}) @Log4j2 @@ -113,6 +119,7 @@ implements private final EntityChangesPublisher entityChangesPublisher; private final Provider<InteractionProvider> interactionProviderProvider; private final PreAndPostValueEvaluatorService preAndPostValueEvaluatorService; + private final CausewayConfiguration causewayConfiguration; /** * Contains a record for every objectId/propertyId that was changed. @@ -135,6 +142,14 @@ implements private final AtomicBoolean persistentChangesEncountered = new AtomicBoolean(); + private boolean suppressAutoFlush; + + @PostConstruct + public void init() { + this.suppressAutoFlush = causewayConfiguration.getPersistence().getCommons().getEntityChangeTracker().isSuppressAutoFlush(); + } + + @Override public void destroy() throws Exception { enlistedPropertyChangeRecordsById.clear(); @@ -146,6 +161,15 @@ implements persistentChangesEncountered.set(false); } + private void suppressAutoFlushIfRequired(Runnable runnable) { + if (suppressAutoFlush) { + RepositoryServiceDefault.suppressAutoFlush(runnable); + } else { + runnable.run(); + } + } + + Set<PropertyChangeRecord> snapshotPropertyChangeRecords() { // this code path has side-effects, it locks the result for this transaction, // such that cannot enlist on top of it @@ -309,12 +333,14 @@ implements return; } - log.debug("enlist entity's property changes for publishing {}", entity); - enlistForChangeKindPublishing(entity, EntityChangeKind.CREATE); + suppressAutoFlushIfRequired(() -> { + log.debug("enlist entity's property changes for publishing {}", entity); + enlistForChangeKindPublishing(entity, EntityChangeKind.CREATE); - MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) - .filter(pcrId -> ! enlistedPropertyChangeRecordsById.containsKey(pcrId)) // only if not previously seen - .forEach(pcrId -> enlistedPropertyChangeRecordsById.put(pcrId, PropertyChangeRecord.ofNew(pcrId))); + MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) + .filter(pcrId -> ! enlistedPropertyChangeRecordsById.containsKey(pcrId)) // only if not previously seen + .forEach(pcrId -> enlistedPropertyChangeRecordsById.put(pcrId, PropertyChangeRecord.ofNew(pcrId))); + }); } @Override @@ -330,28 +356,30 @@ implements log.debug("enlist entity's property changes for publishing {}", entity); - // we call this come what may; - // additional properties may now have been changed, and the changeKind for publishing might also be modified - enlistForChangeKindPublishing(entity, EntityChangeKind.UPDATE); - - final Can<PropertyChangeRecord> ormPropertyChangeRecords = propertyChangeRecordSupplier!=null - ? propertyChangeRecordSupplier.apply(entity) - : null; - - if(ormPropertyChangeRecords != null) { - // provided by ORM - ormPropertyChangeRecords - .stream() - .filter(pcr -> ! enlistedPropertyChangeRecordsById.containsKey(pcr.getId())) // only if not previously seen - .forEach(pcr -> enlistedPropertyChangeRecordsById.put(pcr.getId(), pcr)); - } else { - // home-grown approach - MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) - .filter(pcrId -> ! enlistedPropertyChangeRecordsById.containsKey(pcrId)) // only if not previously seen - .map(pcrId -> enlistedPropertyChangeRecordsById.put(pcrId, PropertyChangeRecord.ofCurrent(pcrId))) - .filter(Objects::nonNull) // shouldn't happen, just keeping compiler happy - .forEach(PropertyChangeRecord::withPreValueSetToCurrent); - } + suppressAutoFlushIfRequired(() -> { + // we call this come what may; + // additional properties may now have been changed, and the changeKind for publishing might also be modified + enlistForChangeKindPublishing(entity, EntityChangeKind.UPDATE); + + final Can<PropertyChangeRecord> ormPropertyChangeRecords = propertyChangeRecordSupplier !=null + ? propertyChangeRecordSupplier.apply(entity) + : null; + + if(ormPropertyChangeRecords != null) { + // provided by ORM + ormPropertyChangeRecords + .stream() + .filter(pcr -> ! enlistedPropertyChangeRecordsById.containsKey(pcr.getId())) // only if not previously seen + .forEach(pcr -> enlistedPropertyChangeRecordsById.put(pcr.getId(), pcr)); + } else { + // home-grown approach + MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) + .filter(pcrId -> ! enlistedPropertyChangeRecordsById.containsKey(pcrId)) // only if not previously seen + .map(pcrId -> enlistedPropertyChangeRecordsById.put(pcrId, PropertyChangeRecord.ofCurrent(pcrId))) + .filter(Objects::nonNull) // shouldn't happen, just keeping compiler happy + .forEach(PropertyChangeRecord::withPreValueSetToCurrent); + } + }); } @Override @@ -363,14 +391,16 @@ implements return; } - final boolean enlisted = enlistForChangeKindPublishing(entity, EntityChangeKind.DELETE); - if(enlisted) { - log.debug("enlist entity's property changes for publishing {}", entity); + suppressAutoFlushIfRequired(() -> { + final boolean enlisted = enlistForChangeKindPublishing(entity, EntityChangeKind.DELETE); + if(enlisted) { + log.debug("enlist entity's property changes for publishing {}", entity); - MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) - .forEach(pcrId -> enlistedPropertyChangeRecordsById - .computeIfAbsent(pcrId, id -> PropertyChangeRecord.ofDeleting(id))); - } + MmEntityUtils.streamPropertyChangeRecordIdsForChangePublishing(entity) + .forEach(pcrId -> enlistedPropertyChangeRecordsById + .computeIfAbsent(pcrId, id -> PropertyChangeRecord.ofDeleting(id))); + } + }); } /** diff --git a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/PreAndPostValueEvaluatorServiceDefault.java b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/PreAndPostValueEvaluatorServiceDefault.java index 211c828368..cac2e76d5b 100644 --- a/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/PreAndPostValueEvaluatorServiceDefault.java +++ b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/changetracking/PreAndPostValueEvaluatorServiceDefault.java @@ -23,6 +23,8 @@ import javax.annotation.Priority; import javax.inject.Inject; import javax.inject.Named; +import org.apache.causeway.persistence.commons.CausewayModulePersistenceCommons; + import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; @@ -34,7 +36,7 @@ import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; @Service -@Named("causeway.persistence.commons.PreAndPostValueEvaluatorServiceDefault") +@Named(CausewayModulePersistenceCommons.NAMESPACE + ".PreAndPostValueEvaluatorServiceDefault") @Priority(PriorityPrecedence.LATE) @Qualifier("default") @InteractionScope // see note above regarding this diff --git a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/repository/RepositoryServiceDefault.java b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/repository/RepositoryServiceDefault.java similarity index 90% rename from core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/repository/RepositoryServiceDefault.java rename to persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/repository/RepositoryServiceDefault.java index a70776e9f8..65d86d3641 100644 --- a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/repository/RepositoryServiceDefault.java +++ b/persistence/commons/src/main/java/org/apache/causeway/persistence/commons/integration/repository/RepositoryServiceDefault.java @@ -16,19 +16,23 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.causeway.core.runtimeservices.repository; +package org.apache.causeway.persistence.commons.integration.repository; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.concurrent.Callable; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.annotation.Priority; import javax.inject.Named; +import org.apache.causeway.persistence.commons.CausewayModulePersistenceCommons; + import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.lang.Nullable; import org.springframework.stereotype.Service; @@ -53,7 +57,6 @@ import org.apache.causeway.core.metamodel.object.MmEntityUtils; import org.apache.causeway.core.metamodel.object.MmUnwrapUtils; import org.apache.causeway.core.metamodel.objectmanager.ObjectBulkLoader; import org.apache.causeway.core.metamodel.spec.ObjectSpecification; -import org.apache.causeway.core.runtimeservices.CausewayModuleCoreRuntimeServices; import lombok.Getter; import lombok.NonNull; @@ -61,7 +64,7 @@ import lombok.RequiredArgsConstructor; import lombok.val; @Service -@Named(CausewayModuleCoreRuntimeServices.NAMESPACE + ".RepositoryServiceDefault") +@Named(CausewayModulePersistenceCommons.NAMESPACE + ".RepositoryServiceDefault") @Priority(PriorityPrecedence.EARLY) @Qualifier("Default") @RequiredArgsConstructor @@ -81,7 +84,9 @@ implements RepositoryService, HasMetaModelContext { @PostConstruct public void init() { - val disableAutoFlush = causewayConfiguration.getCore().getRuntimeServices().getRepositoryService().isDisableAutoFlush(); + val disableAutoFlush = + causewayConfiguration.getPersistence().getCommons().getRepositoryService().isDisableAutoFlush() || + causewayConfiguration.getCore().getRuntimeServices().getRepositoryService().isDisableAutoFlush(); this.autoFlush = !disableAutoFlush; } @@ -165,9 +170,20 @@ implements RepositoryService, HasMetaModelContext { .collect(Collectors.toCollection(ArrayList::new)); } + private static final ThreadLocal<Boolean> autoFlushSuppressed = ThreadLocal.withInitial(() -> false); + public static void suppressAutoFlush(Runnable runnable) { + Boolean onEntry = autoFlushSuppressed.get(); + try { + autoFlushSuppressed.set(true); + runnable.run(); + } finally { + autoFlushSuppressed.set(onEntry); + } + } + @Override public <T> List<T> allMatches(final Query<T> query) { - if(autoFlush) { + if(autoFlush && !autoFlushSuppressed.get()) { transactionService.flushTransaction(); } return submitQuery(query);
