From e155fc551e3a61cd98062c3fd17e59758f310766 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Tue, 9 Nov 2021 12:54:44 -0500 Subject: [PATCH 1/2] HHH-14540 Don't share session-scoped interceptors with temp session --- .../internal/SessionFactoryImpl.java | 15 +++- .../synchronization/AuditProcess.java | 1 + ...sionFactoryInterceptorTransactionTest.java | 76 +++++++++++++++++++ .../tm/SessionInterceptorTransactionTest.java | 73 ++++++++++++++++++ .../test/integration/tm/TestInterceptor.java | 43 +++++++++++ 5 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionFactoryInterceptorTransactionTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionInterceptorTransactionTest.java create mode 100644 hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/TestInterceptor.java diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java index ef50a2b8ec..c99724a10a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java @@ -1123,7 +1123,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { } } - public static Interceptor configuredInterceptor(Interceptor interceptor, SessionFactoryOptions options) { + public static Interceptor configuredInterceptor(Interceptor interceptor, boolean explicitNoInterceptor, SessionFactoryOptions options) { // NOTE : DO NOT return EmptyInterceptor.INSTANCE from here as a "default for the Session" // we "filter" that one out here. The return from here should represent the // explicitly configured Interceptor (if one). Return null from here instead; Session @@ -1139,6 +1139,12 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { return optionsInterceptor; } + // If explicitly asking for no interceptor and there is no SessionFactory-scoped interceptors, then + // no need to inherit from the configured stateless session ones. + if ( explicitNoInterceptor ) { + return null; + } + // then check the Session-scoped interceptor prototype final Class statelessInterceptorImplementor = options.getStatelessInterceptorImplementor(); final Supplier statelessInterceptorImplementorSupplier = options.getStatelessInterceptorImplementorSupplier(); @@ -1181,6 +1187,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { private String tenantIdentifier; private TimeZone jdbcTimeZone; private boolean queryParametersValidationEnabled; + private boolean explicitNoInterceptor; // Lazy: defaults can be built by invoking the builder in fastSessionServices.defaultSessionEventListeners // (Need a fresh build for each Session as the listener instances can't be reused across sessions) @@ -1269,7 +1276,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { @Override public Interceptor getInterceptor() { - return configuredInterceptor( interceptor, sessionFactory.getSessionFactoryOptions() ); + return configuredInterceptor( interceptor, explicitNoInterceptor, sessionFactory.getSessionFactoryOptions() ); } @Override @@ -1316,6 +1323,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { @SuppressWarnings("unchecked") public T interceptor(Interceptor interceptor) { this.interceptor = interceptor; + this.explicitNoInterceptor = false; return (T) this; } @@ -1323,6 +1331,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { @SuppressWarnings("unchecked") public T noInterceptor() { this.interceptor = EmptyInterceptor.INSTANCE; + this.explicitNoInterceptor = true; return (T) this; } @@ -1494,7 +1503,7 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { @Override public Interceptor getInterceptor() { - return configuredInterceptor( EmptyInterceptor.INSTANCE, sessionFactory.getSessionFactoryOptions() ); + return configuredInterceptor( EmptyInterceptor.INSTANCE, false, sessionFactory.getSessionFactoryOptions() ); } diff --git a/hibernate-envers/src/main/java/org/hibernate/envers/internal/synchronization/AuditProcess.java b/hibernate-envers/src/main/java/org/hibernate/envers/internal/synchronization/AuditProcess.java index ac47d29ee8..def0caab20 100644 --- a/hibernate-envers/src/main/java/org/hibernate/envers/internal/synchronization/AuditProcess.java +++ b/hibernate-envers/src/main/java/org/hibernate/envers/internal/synchronization/AuditProcess.java @@ -160,6 +160,7 @@ public class AuditProcess implements BeforeTransactionCompletionProcess { .connection() .autoClose( false ) .connectionHandlingMode( PhysicalConnectionHandlingMode.DELAYED_ACQUISITION_AND_RELEASE_AFTER_TRANSACTION ) + .noInterceptor() .openSession(); executeInSession( temporarySession ); temporarySession.flush(); diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionFactoryInterceptorTransactionTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionFactoryInterceptorTransactionTest.java new file mode 100644 index 0000000000..1f6fd56464 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionFactoryInterceptorTransactionTest.java @@ -0,0 +1,76 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.envers.test.integration.tm; + +import static org.junit.Assert.assertEquals; + +import java.util.Map; + +import javax.persistence.EntityManager; +import javax.transaction.TransactionManager; + +import org.hibernate.FlushMode; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.envers.test.Priority; +import org.hibernate.envers.test.entities.StrTestEntity; +import org.hibernate.internal.SessionImpl; +import org.junit.Test; + +import org.hibernate.testing.jta.TestingJtaBootstrap; +import org.hibernate.testing.jta.TestingJtaPlatformImpl; + +/** + * @author Chris Cranford + */ +public class SessionFactoryInterceptorTransactionTest extends BaseEnversJPAFunctionalTestCase { + + private TestInterceptor interceptor; + private TransactionManager tm; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { StrTestEntity.class }; + } + + @Override + protected void addConfigOptions(Map options) { + super.addConfigOptions( options ); + + TestInterceptor.reset(); + + this.interceptor = new TestInterceptor(); + options.put( AvailableSettings.INTERCEPTOR, interceptor ); + options.put( AvailableSettings.ALLOW_JTA_TRANSACTION_ACCESS, true ); + + TestingJtaBootstrap.prepare( options ); + tm = TestingJtaPlatformImpl.INSTANCE.getTransactionManager(); + } + + @Test + @Priority(10) + public void initData() throws Exception { + // Revision 1 + EntityManager em = getEntityManager(); + // Explicitly use manual flush to trigger separate temporary session write via Envers + em.unwrap( SessionImpl.class ).setHibernateFlushMode( FlushMode.MANUAL ); + tm.begin(); + StrTestEntity entity = new StrTestEntity( "Test" ); + em.persist( entity ); + em.flush(); + tm.commit(); + } + + @Test + public void testInterceptorInvocations() throws Exception { + // Expect the interceptor to have been created once and invoked twice, once for the original session + // and follow-up for the Envers temporary session. + final Map invocationMap = TestInterceptor.getBeforeCompletionCallbacks(); + assertEquals( 1, invocationMap.size() ); + assertEquals( invocationMap.values().stream().filter( v -> v == 2 ).count(), 1 ); + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionInterceptorTransactionTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionInterceptorTransactionTest.java new file mode 100644 index 0000000000..921fb08c2e --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/SessionInterceptorTransactionTest.java @@ -0,0 +1,73 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.envers.test.integration.tm; + +import static org.junit.Assert.assertEquals; + +import java.util.Map; + +import javax.persistence.EntityManager; +import javax.transaction.TransactionManager; + +import org.hibernate.FlushMode; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.envers.test.BaseEnversJPAFunctionalTestCase; +import org.hibernate.envers.test.Priority; +import org.hibernate.envers.test.entities.StrTestEntity; +import org.hibernate.internal.SessionImpl; +import org.junit.Test; + +import org.hibernate.testing.jta.TestingJtaBootstrap; +import org.hibernate.testing.jta.TestingJtaPlatformImpl; + +/** + * @author Chris Cranford + */ +public class SessionInterceptorTransactionTest extends BaseEnversJPAFunctionalTestCase { + + private TransactionManager tm; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { StrTestEntity.class }; + } + + @Override + protected void addConfigOptions(Map options) { + super.addConfigOptions( options ); + + TestInterceptor.reset(); + + options.put( AvailableSettings.SESSION_SCOPED_INTERCEPTOR, TestInterceptor.class.getName() ); + options.put( AvailableSettings.ALLOW_JTA_TRANSACTION_ACCESS, "true" ); + + TestingJtaBootstrap.prepare( options ); + tm = TestingJtaPlatformImpl.INSTANCE.getTransactionManager(); + } + + @Test + @Priority(10) + public void initData() throws Exception { + // Revision 1 + EntityManager em = getEntityManager(); + // Explicitly use manual flush to trigger separate temporary session write via Envers + em.unwrap( SessionImpl.class ).setHibernateFlushMode( FlushMode.MANUAL ); + tm.begin(); + StrTestEntity entity = new StrTestEntity( "Test" ); + em.persist( entity ); + em.flush(); + tm.commit(); + } + + @Test + public void testInterceptorInvocations() throws Exception { + // The interceptor should only be created once and should only be invoked once. + final Map invocationMap = TestInterceptor.getBeforeCompletionCallbacks(); + assertEquals( 1, invocationMap.size() ); + assertEquals( invocationMap.values().stream().filter( v -> v == 1 ).count(), 1 ); + } +} diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/TestInterceptor.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/TestInterceptor.java new file mode 100644 index 0000000000..b8e7a27404 --- /dev/null +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/tm/TestInterceptor.java @@ -0,0 +1,43 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.envers.test.integration.tm; + +import java.util.HashMap; +import java.util.Map; + +import org.hibernate.EmptyInterceptor; +import org.hibernate.Transaction; + +import org.jboss.logging.Logger; + +/** + * @author Chris Cranford + */ +public class TestInterceptor extends EmptyInterceptor { + + private static final Logger LOGGER = Logger.getLogger( TestInterceptor.class ); + private static Map interceptorInvocations = new HashMap<>(); + + public TestInterceptor() { + interceptorInvocations.put( this, 0 ); + } + + @Override + public void beforeTransactionCompletion(Transaction tx) { + super.beforeTransactionCompletion(tx); + interceptorInvocations.put( this, interceptorInvocations.get( this ) + 1 ); + LOGGER.info( "Interceptor beforeTransactionCompletion invoked" ); + } + + public static Map getBeforeCompletionCallbacks() { + return interceptorInvocations; + } + + public static void reset() { + interceptorInvocations.clear(); + } +} From 1dc49271c18afa355d5ea331cd7c91123cfd0361 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Tue, 16 Nov 2021 15:26:48 +0000 Subject: [PATCH 2/2] HHH-14540 Maintain strict API backwards compatibility --- .../java/org/hibernate/internal/SessionFactoryImpl.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java index c99724a10a..87d51e7c51 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionFactoryImpl.java @@ -1123,6 +1123,14 @@ public class SessionFactoryImpl implements SessionFactoryImplementor { } } + /** + * @deprecated use {@link #configuredInterceptor(Interceptor, boolean, SessionFactoryOptions)} + */ + @Deprecated + public static Interceptor configuredInterceptor(Interceptor interceptor, SessionFactoryOptions options) { + return configuredInterceptor( interceptor, false, options ); + } + public static Interceptor configuredInterceptor(Interceptor interceptor, boolean explicitNoInterceptor, SessionFactoryOptions options) { // NOTE : DO NOT return EmptyInterceptor.INSTANCE from here as a "default for the Session" // we "filter" that one out here. The return from here should represent the