You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2022/08/19 19:31:31 UTC
[isis] branch master updated: ISIS-3126: [JDO] fixes Stack Overflow on TimestampService#onPreStore
This is an automated email from the ASF dual-hosted git repository.
ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git
The following commit(s) were added to refs/heads/master by this push:
new 368b46bae7 ISIS-3126: [JDO] fixes Stack Overflow on TimestampService#onPreStore
368b46bae7 is described below
commit 368b46bae724f4621f04ef34ac9fbd7c26f387e8
Author: andi-huber <ah...@apache.org>
AuthorDate: Fri Aug 19 21:31:20 2022 +0200
ISIS-3126: [JDO] fixes Stack Overflow on TimestampService#onPreStore
---
.../changetracking/JdoLifecycleListener.java | 21 +++++++++++-
.../entities/DnObjectProviderForIsis.java | 39 ++++++++++++++++++++++
.../timestamping/jdo/JdoTimestampingTest.java | 15 ++++++---
3 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/changetracking/JdoLifecycleListener.java b/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/changetracking/JdoLifecycleListener.java
index 9a13c98b7b..3d009adafd 100644
--- a/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/changetracking/JdoLifecycleListener.java
+++ b/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/changetracking/JdoLifecycleListener.java
@@ -31,11 +31,13 @@ import javax.jdo.listener.StoreLifecycleListener;
import org.datanucleus.enhancement.Persistable;
+import org.apache.isis.commons.internal.base._Casts;
import org.apache.isis.core.metamodel.context.MetaModelContext;
import org.apache.isis.core.metamodel.facets.object.publish.entitychange.EntityChangePublishingFacet;
import org.apache.isis.core.metamodel.objectmanager.ObjectManager.EntityAdaptingMode;
import org.apache.isis.core.metamodel.services.objectlifecycle.ObjectLifecyclePublisher;
import org.apache.isis.core.metamodel.spec.ManagedObject;
+import org.apache.isis.persistence.jdo.datanucleus.entities.DnObjectProviderForIsis;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
@@ -136,8 +138,25 @@ DetachLifecycleListener, DirtyLifecycleListener, LoadLifecycleListener, StoreLif
log.debug("preDirty {}", ()->_Utils.debug(event));
final Persistable pojo = _Utils.persistableFor(event);
- val entity = adaptEntity(pojo, EntityAdaptingMode.MEMOIZE_BOOKMARK);
+ final Runnable doPreDirty = ()->doPreDirty(pojo);
+
+
+ _Casts.castTo(DnObjectProviderForIsis.class, pojo.dnGetStateManager())
+ .ifPresentOrElse(stateManager->
+ stateManager.acquirePreDirtyPropagationLock(pojo.dnGetObjectId())
+ .ifPresent(lock->{
+ try {
+ doPreDirty.run();
+ } finally {
+ lock.release();
+ }
+ }),
+ doPreDirty);
+ }
+
+ private final void doPreDirty(final Persistable pojo) {
+ val entity = adaptEntity(pojo, EntityAdaptingMode.MEMOIZE_BOOKMARK);
objectLifecyclePublisher.onPreUpdate(entity, null);
}
diff --git a/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/entities/DnObjectProviderForIsis.java b/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/entities/DnObjectProviderForIsis.java
index dfb7eee6e8..f52c017ef4 100644
--- a/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/entities/DnObjectProviderForIsis.java
+++ b/persistence/jdo/datanucleus/src/main/java/org/apache/isis/persistence/jdo/datanucleus/entities/DnObjectProviderForIsis.java
@@ -18,6 +18,7 @@
*/
package org.apache.isis.persistence.jdo.datanucleus.entities;
+import java.util.Map;
import java.util.Optional;
import javax.jdo.PersistenceManager;
@@ -30,6 +31,7 @@ import org.datanucleus.state.ReferentialStateManagerImpl;
import org.datanucleus.store.FieldValues;
import org.apache.isis.applib.services.inject.ServiceInjector;
+import org.apache.isis.commons.internal.collections._Maps;
import org.apache.isis.core.metamodel.context.MetaModelContext;
import org.apache.isis.persistence.jdo.spring.integration.TransactionAwarePersistenceManagerFactoryProxy;
@@ -154,6 +156,43 @@ extends ReferentialStateManagerImpl {
}
}
+ // -- PRE-DIRTY NESTED LOOP PREVENTION
+
+ @FunctionalInterface
+ public static interface PreDirtyPropagationLock {
+ void release();
+ }
+
+ // assuming we don't require thread-safety here,
+ // as each thread presumably has its own DN execution context
+ private final Map<Object, PreDirtyPropagationLock> preDirtyPropagationLocks =
+ _Maps.newHashMap();
+
+ private final PreDirtyPropagationLock createPreDirtyPropagationLock(final Object id) {
+ return ()->preDirtyPropagationLocks.remove(id);
+ }
+
+ public Optional<PreDirtyPropagationLock> acquirePreDirtyPropagationLock(final Object id) {
+
+ // this algorithm is not thread-safe
+ // assuming we don't require thread-safety here,
+ // as each thread presumably has its own DN execution context
+
+ val lockIfGranted = preDirtyPropagationLocks.containsKey(id)
+ ? Optional.<PreDirtyPropagationLock>empty()
+ : Optional.of(preDirtyPropagationLocks.computeIfAbsent(id,
+ this::createPreDirtyPropagationLock));
+
+ if(log.isDebugEnabled()) {
+ log.debug("acquirePreDirtyPropagationLock({}) -> {}",
+ id, lockIfGranted.map(lock->"GRANTED").orElse("DENIED"));
+ }
+
+ return lockIfGranted;
+ }
+
+ // --
+
/*
could be used as hooks to detect field changes ...
diff --git a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/timestamping/jdo/JdoTimestampingTest.java b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/timestamping/jdo/JdoTimestampingTest.java
index 4e12934304..c9d9464e49 100644
--- a/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/timestamping/jdo/JdoTimestampingTest.java
+++ b/regressiontests/stable-persistence-jdo/src/test/java/org/apache/isis/testdomain/timestamping/jdo/JdoTimestampingTest.java
@@ -31,6 +31,7 @@ import org.apache.isis.testdomain.jdo.entities.JdoProductComment;
import org.apache.isis.testing.integtestsupport.applib.IsisIntegrationTestAbstract;
import static org.junit.Assert.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
import lombok.val;
@@ -39,8 +40,7 @@ import lombok.val;
Configuration_usingJdo.class
},
properties = {
-// "logging.config=log4j2-debug-persistence.xml",
-// IsisPresets.DebugPersistence,
+ "logging.level.org.apache.isis.persistence.jdo.datanucleus.changetracking.JdoLifecycleListener = DEBUG"
})
@Transactional
class JdoTimestampingTest extends IsisIntegrationTestAbstract {
@@ -62,11 +62,16 @@ class JdoTimestampingTest extends IsisIntegrationTestAbstract {
assertNotNull(comment.getUpdatedAt());
assertNotNull(comment.getUpdatedBy());
- //FIXME[ISIS-3126] see if we can update the persistent entity
+ //[ISIS-3126] see if we can update the persistent entity,
+ // without triggering a nested loop
+ val firstUpdate = comment.getUpdatedAt();
+ comment.setComment("Awesome Book, really!");
+ repository.persist(comment);
- //comment.setComment("Awesome Book, really!");
+ assertNotNull(comment.getUpdatedAt());
+ val secondUpdate = comment.getUpdatedAt();
- //repository.persist(comment);
+ assertNotEquals(firstUpdate.getNanos(), secondUpdate.getNanos());
}