From a0bb027bcd3c7609b457f272f2b3b710a0f6e930 Mon Sep 17 00:00:00 2001 From: Sam Berlin Date: Thu, 20 Apr 2023 14:33:42 -0700 Subject: [PATCH] Don't automatically create an EntityManager outside of a UnitOfWork, because it leads to leaks. Fixes #739, fixes #997, fixes #730, fixes #985, fixes #959, fixes #731. This also adds an optional Options to the JpaPersistModule constructor, if users wish to use the legacy behavior. This is a breaking change, but to a better default value. Users who want to keep the dangerous form can opt back in with the options. PiperOrigin-RevId: 525852009 --- .../inject/persist/jpa/JpaPersistModule.java | 7 +++ .../inject/persist/jpa/JpaPersistOptions.java | 61 +++++++++++++++++++ .../inject/persist/jpa/JpaPersistService.java | 12 +++- .../test/com/google/inject/persist/BUILD | 1 + ...lassLevelManagedLocalTransactionsTest.java | 18 ++++-- ...ropsEntityManagerFactoryProvisionTest.java | 2 + .../inject/persist/jpa/DynamicFinderTest.java | 7 +-- .../EntityManagerFactoryProvisionTest.java | 2 + .../jpa/EntityManagerProvisionTest.java | 10 --- .../jpa/JoiningLocalTransactionsTest.java | 15 +++-- .../persist/jpa/JpaPersistServiceTest.java | 3 +- ...gedLocalTransactionsAcrossRequestTest.java | 22 ++++--- .../jpa/ManagedLocalTransactionsTest.java | 29 +++++---- ...ManualLocalTransactionsConfidenceTest.java | 4 +- ...ocalTransactionsWithCustomMatcherTest.java | 6 +- 15 files changed, 149 insertions(+), 50 deletions(-) create mode 100644 extensions/persist/src/com/google/inject/persist/jpa/JpaPersistOptions.java diff --git a/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java index 6ff74ad5c6..8f4e812f9e 100644 --- a/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java +++ b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java @@ -44,11 +44,17 @@ */ public final class JpaPersistModule extends PersistModule { private final String jpaUnit; + private final JpaPersistOptions options; public JpaPersistModule(String jpaUnit) { + this(jpaUnit, JpaPersistOptions.builder().build()); + } + + public JpaPersistModule(String jpaUnit, JpaPersistOptions options) { Preconditions.checkArgument( null != jpaUnit && jpaUnit.length() > 0, "JPA unit name must be a non-empty string."); this.jpaUnit = jpaUnit; + this.options = options; } private Map properties; @@ -57,6 +63,7 @@ public JpaPersistModule(String jpaUnit) { @Override protected void configurePersistence() { bindConstant().annotatedWith(Jpa.class).to(jpaUnit); + bind(JpaPersistOptions.class).annotatedWith(Jpa.class).toInstance(options); bind(JpaPersistService.class).in(Singleton.class); diff --git a/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistOptions.java b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistOptions.java new file mode 100644 index 0000000000..e157eac6a2 --- /dev/null +++ b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistOptions.java @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2023 Google, Inc. + * + * 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 com.google.inject.persist.jpa; + +/** Options that configure how the JPA persist service will work. */ +public final class JpaPersistOptions { + + private final boolean autoBeginWorkOnEntityManagerCreation; + + private JpaPersistOptions(JpaPersistOptions.Builder builder) { + this.autoBeginWorkOnEntityManagerCreation = builder.autoBeginWorkOnEntityManagerCreation; + } + + /** + * Returns true if the work unit should automatically begin when the EntityManager is created, if + * it hasn't already begun. + * + *

This defaults to false because it's not safe, as careless usage can lead to leaking + * sessions. + */ + public boolean getAutoBeginWorkOnEntityManagerCreation() { + return autoBeginWorkOnEntityManagerCreation; + } + + /** Returns a builder to set options. */ + public static Builder builder() { + return new Builder(); + } + + /** A builder to create the options. */ + public static final class Builder { + private boolean autoBeginWorkOnEntityManagerCreation; + + private Builder() {} + + public JpaPersistOptions build() { + return new JpaPersistOptions(this); + } + + /** Sets the {@link JpaPersistOptions#getAutoBeginWorkOnEntityManagerCreation} property. */ + public Builder setAutoBeginWorkOnEntityManagerCreation( + boolean autoBeginWorkOnEntityManagerCreation) { + this.autoBeginWorkOnEntityManagerCreation = autoBeginWorkOnEntityManagerCreation; + return this; + } + } +} diff --git a/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistService.java b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistService.java index 957ccec929..72ac2d8354 100644 --- a/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistService.java +++ b/extensions/persist/src/com/google/inject/persist/jpa/JpaPersistService.java @@ -33,24 +33,30 @@ import javax.persistence.EntityManagerFactory; import javax.persistence.Persistence; -/** @author Dhanji R. Prasanna (dhanji@gmail.com) */ +/** + * @author Dhanji R. Prasanna (dhanji@gmail.com) + */ @Singleton class JpaPersistService implements Provider, UnitOfWork, PersistService { private final ThreadLocal entityManager = new ThreadLocal<>(); private final String persistenceUnitName; private final Map persistenceProperties; + private final JpaPersistOptions options; @Inject public JpaPersistService( - @Jpa String persistenceUnitName, @Nullable @Jpa Map persistenceProperties) { + @Jpa JpaPersistOptions options, + @Jpa String persistenceUnitName, + @Nullable @Jpa Map persistenceProperties) { + this.options = options; this.persistenceUnitName = persistenceUnitName; this.persistenceProperties = persistenceProperties; } @Override public EntityManager get() { - if (!isWorking()) { + if (options.getAutoBeginWorkOnEntityManagerCreation() && !isWorking()) { begin(); } diff --git a/extensions/persist/test/com/google/inject/persist/BUILD b/extensions/persist/test/com/google/inject/persist/BUILD index cc01cfb963..20f26bdcba 100644 --- a/extensions/persist/test/com/google/inject/persist/BUILD +++ b/extensions/persist/test/com/google/inject/persist/BUILD @@ -27,6 +27,7 @@ java_library( "//third_party/java/hibernate:hibernate5", "//third_party/java/hsqldb:hsqldb2", "//third_party/java/javax_persistence", + "//third_party/java/jsr330_inject", "//third_party/java/junit", "//third_party/java/mockito", ], diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ClassLevelManagedLocalTransactionsTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ClassLevelManagedLocalTransactionsTest.java index 93fa30c835..30e3df47f4 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ClassLevelManagedLocalTransactionsTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ClassLevelManagedLocalTransactionsTest.java @@ -21,10 +21,12 @@ import com.google.inject.Injector; import com.google.inject.persist.PersistService; import com.google.inject.persist.Transactional; +import com.google.inject.persist.UnitOfWork; import java.io.FileNotFoundException; import java.io.IOException; import java.util.Date; import java.util.List; +import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import junit.framework.TestCase; @@ -62,6 +64,7 @@ public void tearDown() { public void testSimpleTransaction() { injector.getInstance(TransactionalObject.class).runOperationInTxn(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager session = injector.getInstance(EntityManager.class); assertFalse( "EntityManager was not closed by transactional service", @@ -92,6 +95,7 @@ public void testSimpleTransactionRollbackOnChecked() { // ignore } + injector.getInstance(UnitOfWork.class).begin(); EntityManager session = injector.getInstance(EntityManager.class); assertFalse( "EntityManager was not closed by transactional service (rollback didnt happen?)", @@ -119,6 +123,7 @@ public void testSimpleTransactionRollbackOnCheckedExcepting() { // ignored } + injector.getInstance(UnitOfWork.class).begin(); EntityManager session = injector.getInstance(EntityManager.class); assertFalse( "Txn was not closed by transactional service (commit didnt happen?)", @@ -144,6 +149,7 @@ public void testSimpleTransactionRollbackOnUnchecked() { // ignore } + injector.getInstance(UnitOfWork.class).begin(); EntityManager session = injector.getInstance(EntityManager.class); assertFalse( "EntityManager was not closed by transactional service (rollback didnt happen?)", @@ -184,9 +190,10 @@ public void testTransactionalDoesntAffectObjectMethods() { @Transactional public static class TransactionalObject { - @Inject EntityManager session; + @Inject Provider sessionProvider; public void runOperationInTxn() { + EntityManager session = sessionProvider.get(); assertTrue(session.getTransaction().isActive()); JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT); @@ -196,10 +203,11 @@ public void runOperationInTxn() { @Transactional public static class TransactionalObject4 { - @Inject EntityManager session; + @Inject Provider sessionProvider; @Transactional public void runOperationInTxnThrowingUnchecked() { + EntityManager session = sessionProvider.get(); assertTrue(session.getTransaction().isActive()); JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); @@ -211,9 +219,10 @@ public void runOperationInTxnThrowingUnchecked() { @Transactional(rollbackOn = IOException.class, ignore = FileNotFoundException.class) public static class TransactionalObject3 { - @Inject EntityManager session; + @Inject Provider sessionProvider; public void runOperationInTxnThrowingCheckedExcepting() throws IOException { + EntityManager session = sessionProvider.get(); assertTrue(session.getTransaction().isActive()); JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT_2); @@ -225,9 +234,10 @@ public void runOperationInTxnThrowingCheckedExcepting() throws IOException { @Transactional(rollbackOn = IOException.class) public static class TransactionalObject2 { - @Inject EntityManager session; + @Inject Provider sessionProvider; public void runOperationInTxnThrowingChecked() throws IOException { + EntityManager session = sessionProvider.get(); assertTrue(session.getTransaction().isActive()); JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); diff --git a/extensions/persist/test/com/google/inject/persist/jpa/CustomPropsEntityManagerFactoryProvisionTest.java b/extensions/persist/test/com/google/inject/persist/jpa/CustomPropsEntityManagerFactoryProvisionTest.java index 7f1cd712ad..61a9f3016f 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/CustomPropsEntityManagerFactoryProvisionTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/CustomPropsEntityManagerFactoryProvisionTest.java @@ -54,6 +54,8 @@ public void testSessionCreateOnInjection() { //startup persistence injector.getInstance(PersistService.class).start(); + injector.getInstance(UnitOfWork.class).begin(); + //obtain em assertTrue(injector.getInstance(EntityManager.class).isOpen()); } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/DynamicFinderTest.java b/extensions/persist/test/com/google/inject/persist/jpa/DynamicFinderTest.java index f6fe254fcf..3f84099cd7 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/DynamicFinderTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/DynamicFinderTest.java @@ -22,6 +22,7 @@ import com.google.inject.Provider; import com.google.inject.persist.PersistService; import com.google.inject.persist.Transactional; +import com.google.inject.persist.UnitOfWork; import com.google.inject.persist.finder.Finder; import java.util.ArrayList; import java.util.Date; @@ -62,11 +63,7 @@ public void testDynamicFinderListAll() { dao.persist(te); - //im not sure this hack works... - assertFalse( - "Duplicate entity managers crossing-scope", - dao.lastEm.equals(injector.getInstance(EntityManager.class))); - + injector.getInstance(UnitOfWork.class).begin(); List list = injector.getInstance(JpaFinder.class).listAll(); assertNotNull(list); assertFalse(list.isEmpty()); diff --git a/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerFactoryProvisionTest.java b/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerFactoryProvisionTest.java index 73bc7c95c4..e1aaae0107 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerFactoryProvisionTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerFactoryProvisionTest.java @@ -50,6 +50,8 @@ public void testSessionCreateOnInjection() { //startup persistence injector.getInstance(PersistService.class).start(); + injector.getInstance(UnitOfWork.class).begin(); + //obtain em assertTrue(injector.getInstance(EntityManager.class).isOpen()); } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerProvisionTest.java b/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerProvisionTest.java index b432d4b6bd..2c5d5ba3b8 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerProvisionTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/EntityManagerProvisionTest.java @@ -57,11 +57,6 @@ public void testEntityManagerLifecyclePerTxn() { dao.persist(te); - //im not sure this hack works... - assertFalse( - "Duplicate entity managers crossing-scope", - dao.lastEm.equals(injector.getInstance(EntityManager.class))); - //try to start a new em in a new txn dao = injector.getInstance(JpaDao.class); @@ -80,11 +75,6 @@ public void testEntityManagerLifecyclePerTxn2() { dao.persist(te); - //im not sure this hack works... - assertFalse( - "Duplicate entity managers crossing-scope", - dao.lastEm.equals(injector.getInstance(EntityManager.class))); - //try to start a new em in a new txn dao = injector.getInstance(JpaDao.class); diff --git a/extensions/persist/test/com/google/inject/persist/jpa/JoiningLocalTransactionsTest.java b/extensions/persist/test/com/google/inject/persist/jpa/JoiningLocalTransactionsTest.java index ecd48badbd..df61ee5d93 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/JoiningLocalTransactionsTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/JoiningLocalTransactionsTest.java @@ -24,6 +24,7 @@ import com.google.inject.persist.UnitOfWork; import java.io.IOException; import java.util.Date; +import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.NoResultException; @@ -58,6 +59,7 @@ public void testSimpleTransaction() { .getInstance(JoiningLocalTransactionsTest.TransactionalObject.class) .runOperationInTxn(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse("txn was not closed by transactional service", em.getTransaction().isActive()); @@ -86,6 +88,7 @@ public void testSimpleTransactionRollbackOnChecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( @@ -114,6 +117,7 @@ public void testSimpleTransactionRollbackOnUnchecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( "Session was not closed by transactional service (rollback didnt happen?)", @@ -131,11 +135,11 @@ public void testSimpleTransactionRollbackOnUnchecked() { } public static class TransactionalObject { - private final EntityManager em; + private final Provider emProvider; @Inject - public TransactionalObject(EntityManager em) { - this.em = em; + public TransactionalObject(Provider emProvider) { + this.emProvider = emProvider; } @Transactional @@ -145,6 +149,7 @@ public void runOperationInTxn() { @Transactional(rollbackOn = IOException.class) public void runOperationInTxnInternal() { + EntityManager em = emProvider.get(); JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT); em.persist(entity); @@ -159,7 +164,7 @@ public void runOperationInTxnThrowingChecked() throws IOException { private void runOperationInTxnThrowingCheckedInternal() throws IOException { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IOException(); } @@ -173,7 +178,7 @@ public void runOperationInTxnThrowingUnchecked() { public void runOperationInTxnThrowingUncheckedInternal() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IllegalStateException(); } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/JpaPersistServiceTest.java b/extensions/persist/test/com/google/inject/persist/jpa/JpaPersistServiceTest.java index 67b9288cf4..6919f74098 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/JpaPersistServiceTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/JpaPersistServiceTest.java @@ -34,7 +34,8 @@ public class JpaPersistServiceTest extends TestCase { private static final Properties PERSISTENCE_PROPERTIES = new Properties(); private final JpaPersistService sut = - new JpaPersistService(PERSISTENCE_UNIT_NAME, PERSISTENCE_PROPERTIES); + new JpaPersistService( + JpaPersistOptions.builder().build(), PERSISTENCE_UNIT_NAME, PERSISTENCE_PROPERTIES); private final PersistenceProvider provider = mock(PersistenceProvider.class); private final EntityManagerFactory factory = mock(EntityManagerFactory.class); private final EntityManager entityManager = mock(EntityManager.class); diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsAcrossRequestTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsAcrossRequestTest.java index 1c31fe41d5..e5e65b79d5 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsAcrossRequestTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsAcrossRequestTest.java @@ -26,6 +26,7 @@ import com.google.inject.persist.finder.Finder; import java.io.IOException; import java.util.Date; +import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.NoResultException; @@ -57,6 +58,7 @@ public final void tearDown() { public void testSimpleTransaction() { injector.getInstance(TransactionalObject.class).runOperationInTxn(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse(em.getTransaction().isActive()); @@ -77,6 +79,7 @@ public void testSimpleTransaction() { } public void testSimpleTransactionWithMerge() { + injector.getInstance(UnitOfWork.class).begin(); EntityManager emOrig = injector.getInstance(EntityManager.class); JpaTestEntity entity = injector.getInstance(TransactionalObject.class).runOperationInTxnWithMerge(); @@ -107,6 +110,7 @@ public void testSimpleTransactionWithMerge() { } public void disabled_testSimpleTransactionWithMergeAndDF() { + injector.getInstance(UnitOfWork.class).begin(); EntityManager emOrig = injector.getInstance(EntityManager.class); JpaTestEntity entity = injector.getInstance(TransactionalObject.class).runOperationInTxnWithMergeForDf(); @@ -140,6 +144,7 @@ public void testSimpleTransactionRollbackOnChecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( @@ -168,6 +173,7 @@ public void testSimpleTransactionRollbackOnUnchecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( "Session was not closed by transactional service (rollback didnt happen?)", @@ -187,39 +193,39 @@ public void testSimpleTransactionRollbackOnUnchecked() { } public static class TransactionalObject { - private final EntityManager em; + private final Provider emProvider; @Inject - public TransactionalObject(EntityManager em) { - this.em = em; + public TransactionalObject(Provider emProvider) { + this.emProvider = emProvider; } @Transactional public void runOperationInTxn() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); } @Transactional public JpaTestEntity runOperationInTxnWithMerge() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT_MERGE); - return em.merge(entity); + return emProvider.get().merge(entity); } @Transactional public JpaTestEntity runOperationInTxnWithMergeForDf() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT_MERGE_FORDF); - return em.merge(entity); + return emProvider.get().merge(entity); } @Transactional(rollbackOn = IOException.class) public void runOperationInTxnThrowingChecked() throws IOException { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IOException(); } @@ -228,7 +234,7 @@ public void runOperationInTxnThrowingChecked() throws IOException { public void runOperationInTxnThrowingUnchecked() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IllegalStateException(); } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java index 343c66dee4..b36b6a88e9 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java @@ -26,6 +26,7 @@ import com.google.inject.persist.UnitOfWork; import java.io.IOException; import java.util.Date; +import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.NoResultException; @@ -58,6 +59,7 @@ public final void tearDown() { public void testSimpleTransaction() { injector.getInstance(TransactionalObject.class).runOperationInTxn(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse("txn was not closed by transactional service", em.getTransaction().isActive()); @@ -80,6 +82,7 @@ public void testSimpleTransactionWithMerge() { JpaTestEntity entity = injector.getInstance(TransactionalObject.class).runOperationInTxnWithMerge(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse("txn was not closed by transactional service", em.getTransaction().isActive()); @@ -108,6 +111,7 @@ public void testSimpleTransactionRollbackOnChecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( @@ -132,6 +136,7 @@ public void testSimpleTransactionRollbackOnUnchecked() { injector.getInstance(UnitOfWork.class).end(); } + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( "Session was not closed by transactional service (rollback didnt happen?)", @@ -149,6 +154,7 @@ public void testSimpleTransactionRollbackOnUnchecked() { public void testSimpleTransactionRollbackPerformedManuallyWithoutException() { injector.getInstance(TransactionalObject.class).runOperationInTxnWithManualRollback(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( "Session was not closed by transactional service (rollback didnt happen?)", @@ -166,6 +172,7 @@ public void testSimpleTransactionRollbackPerformedManuallyWithoutException() { public void testSimpleTransactionRollbackOnlySetWithoutException() { injector.getInstance(TransactionalObject.class).runOperationInTxnWithRollbackOnlySet(); + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); assertFalse( "Session was not closed by transactional service (rollback didnt happen?)", @@ -181,32 +188,32 @@ public void testSimpleTransactionRollbackOnlySetWithoutException() { } public static class TransactionalObject { - private final EntityManager em; + private final Provider emProvider; @Inject - public TransactionalObject(EntityManager em) { - this.em = em; + public TransactionalObject(Provider emProvider) { + this.emProvider = emProvider; } @Transactional public void runOperationInTxn() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); } @Transactional public JpaTestEntity runOperationInTxnWithMerge() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(UNIQUE_TEXT_MERGE); - return em.merge(entity); + return emProvider.get().merge(entity); } @Transactional(rollbackOn = IOException.class) public void runOperationInTxnThrowingChecked() throws IOException { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IOException(); } @@ -215,7 +222,7 @@ public void runOperationInTxnThrowingChecked() throws IOException { public void runOperationInTxnThrowingUnchecked() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); throw new IllegalStateException(); } @@ -224,18 +231,18 @@ public void runOperationInTxnThrowingUnchecked() { public void runOperationInTxnWithManualRollback() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); - em.getTransaction().rollback(); + emProvider.get().getTransaction().rollback(); } @Transactional public void runOperationInTxnWithRollbackOnlySet() { JpaTestEntity entity = new JpaTestEntity(); entity.setText(TRANSIENT_UNIQUE_TEXT); - em.persist(entity); + emProvider.get().persist(entity); - em.getTransaction().setRollbackOnly(); + emProvider.get().getTransaction().setRollbackOnly(); } } } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsConfidenceTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsConfidenceTest.java index cad873481e..16e230c21a 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsConfidenceTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsConfidenceTest.java @@ -22,6 +22,7 @@ import com.google.inject.persist.PersistService; import com.google.inject.persist.Transactional; import java.util.Date; +import javax.inject.Provider; import javax.persistence.EntityManager; import javax.persistence.PersistenceException; import junit.framework.TestCase; @@ -71,10 +72,11 @@ public void testThrowingCleanupInterceptorConfidence() { } public static class TransactionalObject { - @Inject EntityManager em; + @Inject Provider emProvider; @Transactional public void runOperationInTxn() { + EntityManager em = emProvider.get(); JpaParentTestEntity entity = new JpaParentTestEntity(); JpaTestEntity child = new JpaTestEntity(); diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsWithCustomMatcherTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsWithCustomMatcherTest.java index 41991f5aed..3f310b1b78 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsWithCustomMatcherTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ManualLocalTransactionsWithCustomMatcherTest.java @@ -56,7 +56,8 @@ public void tearDown() { } public void testSimpleCrossTxnWork() { - //pretend that the request was started here + // pretend that the request was started here + injector.getInstance(UnitOfWork.class).begin(); EntityManager em = injector.getInstance(EntityManager.class); JpaTestEntity entity = @@ -76,7 +77,8 @@ public void testSimpleCrossTxnWork() { injector.getInstance(UnitOfWork.class).end(); - //try to query them back out + // try to query them back out + injector.getInstance(UnitOfWork.class).begin(); em = injector.getInstance(EntityManager.class); assertNotNull( em.createQuery("from JpaTestEntity where text = :text")