Merge pull request #2517 from apache/pre-merge
Pre merge (3788)
diff --git a/api/applib/src/main/java/org/apache/causeway/applib/services/metrics/MetricsService.java b/api/applib/src/main/java/org/apache/causeway/applib/services/metrics/MetricsService.java
index 64ec4f7..94ce3cc 100644
--- a/api/applib/src/main/java/org/apache/causeway/applib/services/metrics/MetricsService.java
+++ b/api/applib/src/main/java/org/apache/causeway/applib/services/metrics/MetricsService.java
@@ -18,7 +18,11 @@
*/
package org.apache.causeway.applib.services.metrics;
+import java.util.Collections;
+import java.util.Set;
+
import org.apache.causeway.applib.annotation.DomainObject;
+import org.apache.causeway.applib.services.bookmark.Bookmark;
import org.apache.causeway.applib.services.iactn.InteractionProvider;
import org.apache.causeway.schema.ixn.v2.MemberExecutionDto;
@@ -27,33 +31,61 @@
* numbers of object loaded, dirtied etc.
*
* <p>
- * Only entities with {@link DomainObject#entityChangePublishing()} enabled
+ * Only entities with {@link DomainObject#entityChangePublishing()} enabled (in other words, auditing)
* are counted.
* </p>
*
+ * @implNote Implementation depends upon ORM callbacks.
* @since 1.x {@index}
*/
public interface MetricsService {
/**
- * The number of entities that have, so far in this request, been loaded from the database.
- * <p>
- * Corresponds to the number of times that <code>javax.jdo.listener.LoadLifecycleListener#postLoad(InstanceLifecycleEvent)</code> (or equivalent) is fired.
+ * The number of entities that were loaded (but not necessarily modified) from the database.
+ *
* <p>
* Is captured within {@link MemberExecutionDto#getMetrics()} (accessible from {@link InteractionProvider#currentInteraction()}).
+ *
+ * @see #entitiesLoaded()
+ * @see #numberEntitiesDirtied()
*/
int numberEntitiesLoaded();
/**
- * The number of objects that have, so far in this request, been dirtied/will need updating in the database); a
- * good measure of the footprint of the interaction.
+ * Returns the bookmarks of the entities loaded within the transaction.
+ *
* <p>
- * Corresponds to the number of times that <code>javax.jdo.listener.DirtyLifecycleListener#preDirty(InstanceLifecycleEvent)</code> (or equivalent) callback is fired.
+ * Requires detailed metrics to be enabled using config param.
+ *
+ * @see #numberEntitiesLoaded()
+ * @see #entitiesDirtied()
+ */
+ default Set<Bookmark> entitiesLoaded() {
+ return Collections.emptySet();
+ }
+
+ /**
+ * The number of entities that were modified within the transaction.
+ *
* <p>
* Is captured within {@link MemberExecutionDto#getMetrics()} (accessible from {@link InteractionProvider#currentInteraction()}.
+ *
+ * @see #numberEntitiesLoaded()
+ * @see #entitiesDirtied()
*/
int numberEntitiesDirtied();
+ /**
+ * Returns the bookmarks of the entities that were modified within the transaction.
+ *
+ * <p>
+ * Requires detailed metrics to be enabled using config param.
+ *
+ * @see #numberEntitiesDirtied()
+ * @see #entitiesLoaded()
+ */
+ default Set<Bookmark> entitiesDirtied() {
+ return Collections.emptySet();
+ }
}
-
diff --git a/api/applib/src/main/java/org/apache/causeway/applib/services/publishing/log/PageRenderLogger.java b/api/applib/src/main/java/org/apache/causeway/applib/services/publishing/log/PageRenderLogger.java
index dd379d4..8cbc181 100644
--- a/api/applib/src/main/java/org/apache/causeway/applib/services/publishing/log/PageRenderLogger.java
+++ b/api/applib/src/main/java/org/apache/causeway/applib/services/publishing/log/PageRenderLogger.java
@@ -24,8 +24,11 @@
import java.util.stream.Collectors;
import javax.annotation.Priority;
+import javax.inject.Inject;
import javax.inject.Named;
+import org.apache.causeway.applib.services.metrics.MetricsService;
+
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Service;
@@ -36,6 +39,7 @@
import org.apache.causeway.applib.services.publishing.spi.PageRenderSubscriber;
import org.apache.causeway.commons.internal.base._NullSafe;
+import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
/**
@@ -48,9 +52,12 @@
@Named(PageRenderLogger.LOGICAL_TYPE_NAME)
@Priority(PriorityPrecedence.LATE)
@Qualifier("Logging")
+@RequiredArgsConstructor(onConstructor_ = {@Inject})
@Log4j2
public class PageRenderLogger implements PageRenderSubscriber {
+ private final MetricsService metricsService;
+
static final String LOGICAL_TYPE_NAME = CausewayModuleApplib.NAMESPACE + ".PageRenderLogger";
@Override
@@ -60,7 +67,9 @@
@Override
public void onRenderingDomainObject(final Bookmark bookmark) {
- log.debug("rendering object: [ {} ]", doubleQuoted(bookmark.stringify()));
+ if(log.isDebugEnabled()) {
+ log.debug("rendering object: [ {} ]", doubleQuoted(bookmark.stringify()));
+ }
}
@Override
@@ -68,18 +77,26 @@
final var bookmarksStringified = bookmarksStringified(bookmarkSupplier);
- log.debug("rendering collection: [ {} ]", bookmarksStringified);
+ if (log.isDebugEnabled()) {
+ log.debug("rendering collection: [ {} ]", bookmarksStringified);
+ }
}
@Override
public void onRenderingValue(final Object value) {
- log.debug("rendering value: [ {} ]", doubleQuoted(value));
+ if(log.isDebugEnabled()) {
+ log.debug("rendering value: [ {} ]", doubleQuoted(value));
+ }
}
@Override
public void onRenderedDomainObject(final Bookmark bookmark) {
- log.debug("rendered object: [ {} ]", doubleQuoted(bookmark.stringify()));
+ if(log.isDebugEnabled()) {
+ // until @ActionLayout#redirectPolicy is reintroduced (if it ever is), there's no point in querying for the numberEntitiesDirtied,
+ // because (for Wicket viewer at least), the rendering is in a separate request to any modifying action.
+ log.debug("rendered object: [ {} ] numEntitiesLoaded: {}", doubleQuoted(bookmark.stringify()), metricsService.numberEntitiesLoaded());
+ }
}
@Override
@@ -87,13 +104,17 @@
final var bookmarksStringified = bookmarksStringified(bookmarkSupplier);
- log.debug("rendered collection: [ {} ]", bookmarksStringified);
+ if (log.isDebugEnabled()) {
+ log.debug("rendered collection: [ {} ]", bookmarksStringified);
+ }
}
@Override
public void onRenderedValue(final Object value) {
- log.debug("rendered value: [ {} ]", doubleQuoted(value));
+ if(log.isDebugEnabled()) {
+ log.debug("rendered value: [ {} ]", doubleQuoted(value));
+ }
}
// -- HELPER
@@ -109,5 +130,4 @@
private static String doubleQuoted(final @Nullable Object obj) {
return "\"" + obj + "\"";
}
-
}
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 02459ce..c8191e4 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
@@ -1547,6 +1547,28 @@
}
}
+
+ private final Service service = new Service();
+ @Data
+ public static class Service {
+
+ private final MetricsService metricsService = new MetricsService();
+ @Data
+ public static class MetricsService {
+ public enum Level {
+ COUNTERS_ONLY,
+ COUNTERS_AND_DETAIL;
+
+ public boolean isCountersOnly() { return this == COUNTERS_ONLY; }
+ public boolean isCountersAndDetail() { return this == COUNTERS_AND_DETAIL; }
+ }
+
+ /**
+ * What level of detail the MetricsService should capture.
+ */
+ private Level level = Level.COUNTERS_ONLY;
+ }
+ }
}
private final Core core = new Core();
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 154ba3d..4aec4ea 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
@@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.Map;
@@ -87,6 +88,7 @@
import org.apache.causeway.core.transaction.changetracking.HasEnlistedEntityChanges;
import org.apache.causeway.persistence.commons.CausewayModulePersistenceCommons;
+import lombok.Getter;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.val;
@@ -171,30 +173,60 @@
enlistedPropertyChangeRecordsById.computeIfAbsent(pcrId, func);
}
- /**
- * Contains pre- and post- values of every property of every object that actually changed. A lazy snapshot.
- */
- private final _Lazy<Set<PropertyChangeRecord>> entityPropertyChangeRecordsForPublishing
- = _Lazy.of(() -> {
- Set<PropertyChangeRecord> records;
- try {
- records = changedRecords(enlistedPropertyChangeRecordsById.values());
- } catch(ConcurrentModificationException ex) {
- log.warn(
- "A concurrent modification exception, one of these properties seemed to change as we looked at it :\n" +
- enlistedPropertyChangeRecordsById.keySet()
+ private Changes evaluateChanges() {
+ val changedProperties = evaluateChangedProperties();
+
+ val isCountersAndDetail = causewayConfiguration.getApplib().getService().getMetricsService().getLevel().isCountersAndDetail();
+ Set<Bookmark> loadedBookmarks =
+ isCountersAndDetail
+ ? enlistedPropertyChangeRecordsById.keySet()
.stream()
- .map(PropertyChangeRecordId::toString)
- .collect(Collectors.joining("\n"))
- );
- // instead, we take a copy
- records = changedRecords(new ArrayList<PropertyChangeRecord>(enlistedPropertyChangeRecordsById.values()));
- }
+ .map(PropertyChangeRecordId::getBookmark)
+ .collect(Collectors.<Bookmark>toSet())
+ : Collections.emptySet();
+
+ Set<Bookmark> dirtiedBookmarks =
+ isCountersAndDetail
+ ? changedProperties
+ .stream()
+ .map(PropertyChangeRecord::getBookmark)
+ .collect(Collectors.<Bookmark>toSet())
+ : Collections.emptySet();
enlistedPropertyChangeRecordsById.clear();
- return records;
- });
+ return new Changes(changedProperties, loadedBookmarks, dirtiedBookmarks);
+ }
+
+ private Set<PropertyChangeRecord> evaluateChangedProperties() {
+ Set<PropertyChangeRecord> dirtiedProperties;
+ try {
+ dirtiedProperties = changedRecords(enlistedPropertyChangeRecordsById.values());
+ } catch (ConcurrentModificationException ex) {
+ log.warn(
+ "A concurrent modification exception, one of these properties seemed to change as we looked at it :\n" +
+ enlistedPropertyChangeRecordsById.keySet()
+ .stream()
+ .map(PropertyChangeRecordId::toString)
+ .collect(Collectors.joining("\n"))
+ );
+ // instead, we take a copy
+ dirtiedProperties = changedRecords(new ArrayList<>(enlistedPropertyChangeRecordsById.values()));
+ }
+ return dirtiedProperties;
+ }
+
+ @RequiredArgsConstructor
+ static class Changes {
+ @Getter private final Set<PropertyChangeRecord> dirtiedProperties;
+ @Getter private final Set<Bookmark> loadedBookmarks;
+ @Getter private final Set<Bookmark> dirtiedBookmarks;
+ }
+
+ /**
+ * Contains pre- and post- values of every property of every object that actually changed. A lazy snapshot.
+ */
+ private final _Lazy<Changes> changes = _Lazy.of(this::evaluateChanges);
/**
* when iterating over the original value set, if any of the properties causes the entity to change state as it is
@@ -215,12 +247,11 @@
.collect(_Sets.toUnmodifiable());
}
- private Set<PropertyChangeRecord> memoizePropertyChangeRecordsIfRequired() {
- return entityPropertyChangeRecordsForPublishing.get();
+ private Changes memoizeChangesIfRequired() {
+ return changes.get();
}
-
/**
* @implNote access to this {@link Map} must be thread-safe (insertion order preservation is not required)
*/
@@ -232,10 +263,10 @@
@Override
- public void destroy() throws Exception {
+ public void destroy() {
if(log.isDebugEnabled()) {
- val interactionId = interactionProviderProvider.get().currentInteraction().map(Interaction::getInteractionId).orElseGet(null);
+ val interactionId = interactionProviderProvider.get().currentInteraction().map(Interaction::getInteractionId).orElse(null);
log.debug("EntityChangeTrackerDefault.destroy xactn={} interactionId={} thread={}", transactionCounter.get(), interactionId, Thread.currentThread().getName());
}
@@ -281,7 +312,7 @@
// guard against transient
if(ManagedObjects.bookmark(entity).isEmpty()) return true;
- if(entityPropertyChangeRecordsForPublishing.isMemoized()) {
+ if(changes.isMemoized()) {
throw _Exceptions.illegalState("Cannot enlist additional changes for auditing, "
+ "since changedObjectPropertiesRef was already prepared (memoized) for auditing.");
}
@@ -297,7 +328,7 @@
log.debug("about to publish entity changes");
// we memoize the property changes to (hopefully) avoid ConcurrentModificationExceptions with ourselves later
- memoizePropertyChangeRecordsIfRequired();
+ memoizeChangesIfRequired();
entityPropertyChangePublisher.publishChangedProperties();
entityChangesPublisher.publishChangingEntities(this);
@@ -311,7 +342,7 @@
private void resetState(LongAdder entityChangeEventCount, LongAdder numberEntitiesLoaded) {
enlistedPropertyChangeRecordsById.clear();
- entityPropertyChangeRecordsForPublishing.clear();
+ changes.clear();
changeKindByEnlistedAdapter.clear();
entityChangeEventCount.reset();
@@ -323,7 +354,8 @@
private void enableCommandPublishing() {
val alreadySet = persistentChangesEncountered.getAndSet(true);
if(!alreadySet) {
- currentInteraction().getCommand(); //TODO does this call have side-effects? if so explain, else remove
+ // has side effects
+ val command = currentInteraction().getCommand();
}
}
@@ -343,7 +375,7 @@
// this code path has side-effects, it locks the result for this transaction,
// such that cannot enlist on top of it
- final int numberEntityPropertiesModified = memoizePropertyChangeRecordsIfRequired().size();
+ final int numberEntityPropertiesModified = memoizeChangesIfRequired().dirtiedProperties.size();
val interactionId = interaction.getInteractionId();
final int nextEventSequence = ((InteractionInternal) interaction).getThenIncrementTransactionSequence();
@@ -423,7 +455,7 @@
// this code path has side-effects, it locks the result for this transaction,
// such that cannot enlist on top of it
- Set<PropertyChangeRecord> propertyChangeRecords = memoizePropertyChangeRecordsIfRequired();
+ Set<PropertyChangeRecord> propertyChangeRecords = memoizeChangesIfRequired().getDirtiedProperties();
return propertyChangeRecords.stream()
.map(propertyChangeRecord -> propertyChangeRecord.toEntityPropertyChange(timestamp, userName, txId))
@@ -590,6 +622,16 @@
return changeKindByEnlistedAdapter.size();
}
+ @Override
+ public Set<Bookmark> entitiesLoaded() {
+ return memoizeChangesIfRequired().getLoadedBookmarks();
+ }
+
+ @Override
+ public Set<Bookmark> entitiesDirtied() {
+ return memoizeChangesIfRequired().getDirtiedBookmarks();
+ }
+
// -- HELPER
/**
@@ -602,7 +644,6 @@
boolean isEnabled();
}
- @Inject private Configuration configuration;
@Component
@Priority(PriorityPrecedence.LATE)
@@ -621,4 +662,8 @@
return causewayConfiguration.getPersistence().getCommons().getEntityChangeTracker().isEnabled();
}
}
+
+ @Inject private Configuration configuration;
+ @Inject private CausewayConfiguration causewayConfiguration;
+
}
\ No newline at end of file
