From 153a1793c08f28f4eba31fefe6304d380a13bbfe Mon Sep 17 00:00:00 2001 From: Vlad Mihalcea Date: Tue, 19 Jun 2018 12:07:30 +0300 Subject: [PATCH] SPR-16956 - Propagate read-only status as FlushMode.MANUAL to Hibernate Session --- .../HibernateTransactionManager.java | 3 + .../orm/hibernate5/SessionHolder.java | 9 ++ .../orm/jpa/vendor/HibernateJpaDialect.java | 18 +++- .../orm/HibernateSessionFlushingTests.java | 22 ++++- .../ejb/ReadOnlyJpaTxDaoTests.java | 88 +++++++++++++++++++ .../ejb/dao/TestEntityService.java | 28 ++++++ .../ejb/dao/TestEntityServiceImpl.java | 37 ++++++++ .../transaction/ejb/required-tx-config.xml | 2 + 8 files changed, 201 insertions(+), 6 deletions(-) create mode 100644 spring-test/src/test/java/org/springframework/test/context/transaction/ejb/ReadOnlyJpaTxDaoTests.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityService.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityServiceImpl.java diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java b/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java index 4fa239aee5ce..97401207e9b3 100644 --- a/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java +++ b/spring-orm/src/main/java/org/springframework/orm/hibernate5/HibernateTransactionManager.java @@ -495,6 +495,8 @@ protected void doBegin(Object transaction, TransactionDefinition definition) { if (definition.isReadOnly() && txObject.isNewSession()) { // Just set to MANUAL in case of a new Session for this transaction. session.setFlushMode(FlushMode.MANUAL); + txObject.getSessionHolder().setPreviousReadOnly( session.isDefaultReadOnly() ); + session.setDefaultReadOnly(true); } if (!definition.isReadOnly() && !txObject.isNewSession()) { @@ -717,6 +719,7 @@ protected void doCleanupAfterCompletion(Object transaction) { } if (txObject.getSessionHolder().getPreviousFlushMode() != null) { session.setFlushMode(txObject.getSessionHolder().getPreviousFlushMode()); + session.setDefaultReadOnly(txObject.getSessionHolder().isPreviousReadOnly()); } if (!this.allowResultAccessAfterCompletion && !this.hibernateManagedSession) { disconnectOnCompletion(session); diff --git a/spring-orm/src/main/java/org/springframework/orm/hibernate5/SessionHolder.java b/spring-orm/src/main/java/org/springframework/orm/hibernate5/SessionHolder.java index 591a19fad2e7..46f7dd45cf34 100644 --- a/spring-orm/src/main/java/org/springframework/orm/hibernate5/SessionHolder.java +++ b/spring-orm/src/main/java/org/springframework/orm/hibernate5/SessionHolder.java @@ -46,6 +46,7 @@ public class SessionHolder extends ResourceHolderSupport { @Nullable private FlushMode previousFlushMode; + private boolean previousReadOnly; public SessionHolder(Session session) { Assert.notNull(session, "Session must not be null"); @@ -75,12 +76,20 @@ public FlushMode getPreviousFlushMode() { return this.previousFlushMode; } + public boolean isPreviousReadOnly() { + return previousReadOnly; + } + + public void setPreviousReadOnly(boolean previousReadOnly) { + this.previousReadOnly = previousReadOnly; + } @Override public void clear() { super.clear(); this.transaction = null; this.previousFlushMode = null; + this.previousReadOnly = false; } } diff --git a/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java b/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java index 84cc031dd5e4..e375770bfbce 100644 --- a/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java +++ b/spring-orm/src/main/java/org/springframework/orm/jpa/vendor/HibernateJpaDialect.java @@ -165,7 +165,8 @@ else if (isolationLevelNeeded) { // Adapt flush mode and store previous isolation level, if any. FlushMode previousFlushMode = prepareFlushMode(session, definition.isReadOnly()); - return new SessionTransactionData(session, previousFlushMode, preparedCon, previousIsolationLevel); + boolean previousReadOnly = prepareReadOnly(session, definition.isReadOnly()); + return new SessionTransactionData(session, previousFlushMode, preparedCon, previousIsolationLevel, previousReadOnly); } @Override @@ -174,7 +175,8 @@ public Object prepareTransaction(EntityManager entityManager, boolean readOnly, Session session = getSession(entityManager); FlushMode previousFlushMode = prepareFlushMode(session, readOnly); - return new SessionTransactionData(session, previousFlushMode, null, null); + boolean previousReadOnly = prepareReadOnly(session, readOnly); + return new SessionTransactionData(session, previousFlushMode, null, null, previousReadOnly); } @SuppressWarnings("deprecation") @@ -200,6 +202,12 @@ protected FlushMode prepareFlushMode(Session session, boolean readOnly) throws P return null; } + protected boolean prepareReadOnly(Session session, boolean readOnly) { + boolean previousReadOnly = session.isDefaultReadOnly(); + session.setDefaultReadOnly(readOnly); + return previousReadOnly; + } + @Override public void cleanupTransaction(@Nullable Object transactionData) { if (transactionData instanceof SessionTransactionData) { @@ -332,13 +340,16 @@ private static class SessionTransactionData { @Nullable private final Integer previousIsolationLevel; + private final boolean previousReadOnly; + public SessionTransactionData(Session session, @Nullable FlushMode previousFlushMode, - @Nullable Connection preparedCon, @Nullable Integer previousIsolationLevel) { + @Nullable Connection preparedCon, @Nullable Integer previousIsolationLevel, boolean previousReadOnly) { this.session = session; this.previousFlushMode = previousFlushMode; this.preparedCon = preparedCon; this.previousIsolationLevel = previousIsolationLevel; + this.previousReadOnly = previousReadOnly; } @SuppressWarnings("deprecation") @@ -346,6 +357,7 @@ public void resetSessionState() { if (this.previousFlushMode != null) { this.session.setFlushMode(this.previousFlushMode); } + session.setDefaultReadOnly(previousReadOnly); if (this.preparedCon != null && this.session.isConnected()) { Connection conToReset = HibernateConnectionHandle.doGetConnection(this.session); if (conToReset != this.preparedCon) { diff --git a/spring-test/src/test/java/org/springframework/test/context/junit4/orm/HibernateSessionFlushingTests.java b/spring-test/src/test/java/org/springframework/test/context/junit4/orm/HibernateSessionFlushingTests.java index f6d688f01fc0..edf3ec87fef1 100644 --- a/spring-test/src/test/java/org/springframework/test/context/junit4/orm/HibernateSessionFlushingTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/junit4/orm/HibernateSessionFlushingTests.java @@ -18,8 +18,10 @@ import javax.persistence.PersistenceException; +import org.hibernate.Session; import org.hibernate.SessionFactory; import org.hibernate.exception.ConstraintViolationException; + import org.junit.Before; import org.junit.Test; @@ -29,9 +31,12 @@ import org.springframework.test.context.junit4.orm.domain.DriversLicense; import org.springframework.test.context.junit4.orm.domain.Person; import org.springframework.test.context.junit4.orm.service.PersonService; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; -import static org.junit.Assert.*; -import static org.springframework.test.transaction.TransactionTestUtils.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.springframework.test.transaction.TransactionTestUtils.assertInTransaction; /** * Transactional integration tests regarding manual session flushing with @@ -52,7 +57,6 @@ public class HibernateSessionFlushingTests extends AbstractTransactionalJUnit4Sp @Autowired private SessionFactory sessionFactory; - protected int countRowsInPersonTable() { return countRowsInTable("person"); } @@ -77,6 +81,18 @@ public void findSam() { assertEquals("Verifying Sam's driver's license number", Long.valueOf(1234), driversLicense.getNumber()); } + @Test + @Transactional(readOnly = true) + public void readOnlySession() { + Session session = sessionFactory.getCurrentSession(); + Person sam = personService.findByName(SAM); + sam.setName("Vlad"); + //By setting setDefaultReadOnly(true), the user can no longer modify any entity. + session.flush(); + session.refresh(sam); + assertEquals("Sam", sam.getName()); + } + @Test public void saveJuergenWithDriversLicense() { DriversLicense driversLicense = new DriversLicense(2L, 2222L); diff --git a/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/ReadOnlyJpaTxDaoTests.java b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/ReadOnlyJpaTxDaoTests.java new file mode 100644 index 000000000000..7db398e3de16 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/ReadOnlyJpaTxDaoTests.java @@ -0,0 +1,88 @@ +/* + * Copyright 2002-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.test.context.transaction.ejb; + +import java.util.concurrent.atomic.AtomicReference; +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.orm.jpa.JpaTransactionManager; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests; +import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; +import org.springframework.test.context.transaction.ejb.dao.TestEntityService; +import org.springframework.test.context.transaction.ejb.model.TestEntity; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionCallback; +import org.springframework.transaction.support.TransactionTemplate; + +import static org.junit.Assert.assertEquals; + +/** + * + * @author Vlad MIhalcea + */ +@ContextConfiguration("required-tx-config.xml") +public class ReadOnlyJpaTxDaoTests extends AbstractJUnit4SpringContextTests { + + protected static final String TEST_NAME = "test-name"; + + @PersistenceContext + private EntityManager em; + + @Autowired + private TestEntityService testEntityService; + + @Autowired + protected JpaTransactionManager transactionManager; + + @Before + public void init() { + new TransactionTemplate(transactionManager).execute( + (TransactionCallback) status -> { + TestEntity testEntity = new TestEntity(); + testEntity.setName(TEST_NAME); + + em.persist(testEntity); + return null; + } ); + } + + @Test + public void testReadOnlyTx() { + AtomicReference entityHolder = new AtomicReference<>(); + assertEquals(0, em.find(TestEntity.class, TEST_NAME).getCount()); + + testEntityService.onReadOnly( em -> { + TestEntity entity = em.find(TestEntity.class, TEST_NAME); + entity.setCount(100); + //By setting setDefaultReadOnly(true), the user can no longer modify any entity. + em.flush(); + em.refresh(entity); + entityHolder.set(entity); + }); + + assertEquals(0, entityHolder.get().getCount()); + } + +} \ No newline at end of file diff --git a/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityService.java b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityService.java new file mode 100644 index 000000000000..f22babfc0811 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityService.java @@ -0,0 +1,28 @@ +/* + * Copyright 2002-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.test.context.transaction.ejb.dao; + +import java.util.function.Consumer; +import javax.persistence.EntityManager; + +/** + * @author Vlad Mihalcea + */ +public interface TestEntityService { + + void onReadOnly(Consumer callback); +} diff --git a/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityServiceImpl.java b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityServiceImpl.java new file mode 100644 index 000000000000..9a7c66528663 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/transaction/ejb/dao/TestEntityServiceImpl.java @@ -0,0 +1,37 @@ +/* + * Copyright 2002-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.test.context.transaction.ejb.dao; + +import java.util.function.Consumer; +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; + +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class TestEntityServiceImpl implements TestEntityService { + + @PersistenceContext + protected EntityManager em; + + @Override + @Transactional(readOnly = true) + public void onReadOnly(Consumer callback) { + callback.accept(em); + } +} diff --git a/spring-test/src/test/resources/org/springframework/test/context/transaction/ejb/required-tx-config.xml b/spring-test/src/test/resources/org/springframework/test/context/transaction/ejb/required-tx-config.xml index a159597bc6c1..e8f57e3bab6f 100644 --- a/spring-test/src/test/resources/org/springframework/test/context/transaction/ejb/required-tx-config.xml +++ b/spring-test/src/test/resources/org/springframework/test/context/transaction/ejb/required-tx-config.xml @@ -6,4 +6,6 @@ + + \ No newline at end of file