Skip to content

Commit

Permalink
Don't wrap methods declared by Object in transactions. Fixes #958, fixes
Browse files Browse the repository at this point in the history
 #788.

PiperOrigin-RevId: 526028062
  • Loading branch information
sameb authored and Guice Team committed Apr 21, 2023
1 parent 2d78877 commit 6fb1a1c
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@

import com.google.inject.AbstractModule;
import com.google.inject.internal.InternalFlags;
import com.google.inject.matcher.AbstractMatcher;
import com.google.inject.matcher.Matcher;
import java.lang.reflect.Method;
import org.aopalliance.intercept.MethodInterceptor;

/**
Expand All @@ -38,7 +41,8 @@ protected final void configure() {
requireBinding(UnitOfWork.class);
if (InternalFlags.isBytecodeGenEnabled()) {
// class-level @Transacational
bindInterceptor(annotatedWith(Transactional.class), any(), getTransactionInterceptor());
bindInterceptor(
annotatedWith(Transactional.class), NOT_OBJECT_METHOD, getTransactionInterceptor());
// method-level @Transacational
bindInterceptor(any(), annotatedWith(Transactional.class), getTransactionInterceptor());
}
Expand All @@ -47,4 +51,12 @@ protected final void configure() {
protected abstract void configurePersistence();

protected abstract MethodInterceptor getTransactionInterceptor();

private static final Matcher<Method> NOT_OBJECT_METHOD =
new AbstractMatcher<Method>() {
@Override
public boolean matches(Method m) {
return !Object.class.equals(m.getDeclaringClass());
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.Date;
import java.util.List;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import junit.framework.TestCase;

/**
Expand All @@ -48,7 +49,7 @@ public class ClassLevelManagedLocalTransactionsTest extends TestCase {
public void setUp() {
injector = Guice.createInjector(new JpaPersistModule("testUnit"));

//startup persistence
// startup persistence
injector.getInstance(PersistService.class).start();
}

Expand All @@ -66,7 +67,7 @@ public void testSimpleTransaction() {
"EntityManager was not closed by transactional service",
session.getTransaction().isActive());

//test that the data has been stored
// test that the data has been stored
session.getTransaction().begin();
Object result =
session
Expand All @@ -88,15 +89,15 @@ public void testSimpleTransactionRollbackOnChecked() {
try {
injector.getInstance(TransactionalObject2.class).runOperationInTxnThrowingChecked();
} catch (IOException e) {
//ignore
// ignore
}

EntityManager session = injector.getInstance(EntityManager.class);
assertFalse(
"EntityManager was not closed by transactional service (rollback didnt happen?)",
session.getTransaction().isActive());

//test that the data has been stored
// test that the data has been stored
session.getTransaction().begin();
List<?> result =
session
Expand All @@ -115,15 +116,15 @@ public void testSimpleTransactionRollbackOnCheckedExcepting() {
injector.getInstance(TransactionalObject3.class).runOperationInTxnThrowingCheckedExcepting();
fail("Exception was not thrown by test txn-al method!");
} catch (IOException e) {
//ignored
// ignored
}

EntityManager session = injector.getInstance(EntityManager.class);
assertFalse(
"Txn was not closed by transactional service (commit didnt happen?)",
session.getTransaction().isActive());

//test that the data has been stored
// test that the data has been stored
session.getTransaction().begin();
Object result =
session
Expand All @@ -140,15 +141,15 @@ public void testSimpleTransactionRollbackOnUnchecked() {
try {
injector.getInstance(TransactionalObject4.class).runOperationInTxnThrowingUnchecked();
} catch (RuntimeException re) {
//ignore
// ignore
}

EntityManager session = injector.getInstance(EntityManager.class);
assertFalse(
"EntityManager was not closed by transactional service (rollback didnt happen?)",
session.getTransaction().isActive());

//test that the data has been stored
// test that the data has been stored
session.getTransaction().begin();
List<?> result =
session
Expand All @@ -161,6 +162,26 @@ public void testSimpleTransactionRollbackOnUnchecked() {
assertTrue("a result was returned! rollback sure didnt happen!!!", result.isEmpty());
}

public void testTransactionalDoesntAffectObjectMethods() {
// Given a persist service that tracks when it's called
JpaPersistService persistService = injector.getInstance(JpaPersistService.class);
EntityManagerFactory originalEMF = injector.getInstance(EntityManagerFactory.class);
TrackedEntityManagerFactory trackingEMF = new TrackedEntityManagerFactory(originalEMF);
persistService.start(trackingEMF);

FakeTransactionalObject txnObj = injector.getInstance(FakeTransactionalObject.class);

String unused = txnObj.toString();
assertFalse(
"Should not have created a transaction for toString method",
trackingEMF.hasCreatedSomeEntityManager());

txnObj.fakeTransactionalMethod();
assertTrue(
"Transaction should have been created for normal method",
trackingEMF.hasCreatedSomeEntityManager());
}

@Transactional
public static class TransactionalObject {
@Inject EntityManager session;
Expand Down Expand Up @@ -215,4 +236,9 @@ public void runOperationInTxnThrowingChecked() throws IOException {
throw new IOException();
}
}

@Transactional
public static class FakeTransactionalObject {
public void fakeTransactionalMethod() {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package com.google.inject.persist.jpa;

import java.util.Map;
import javax.persistence.Cache;
import javax.persistence.EntityGraph;
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.PersistenceUnitUtil;
import javax.persistence.Query;
import javax.persistence.SynchronizationType;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.metamodel.Metamodel;

/** Proxy EntityManager that adds tracking capabilities, keeping a count of created objects. */
class TrackedEntityManagerFactory implements EntityManagerFactory {

private final EntityManagerFactory delegate;
private int entityManagerCreatedCount = 0;

public TrackedEntityManagerFactory(EntityManagerFactory delegate) {
this.delegate = delegate;
}

public boolean hasCreatedSomeEntityManager() {
return (entityManagerCreatedCount > 0);
}

public int getEntityManagerCreatedCount() {
return entityManagerCreatedCount;
}

@Override
public boolean isOpen() {
return delegate.isOpen();
}

@Override
public Map<String, Object> getProperties() {
return delegate.getProperties();
}

@Override
public PersistenceUnitUtil getPersistenceUnitUtil() {
return delegate.getPersistenceUnitUtil();
}

@Override
public Metamodel getMetamodel() {
return delegate.getMetamodel();
}

@Override
public CriteriaBuilder getCriteriaBuilder() {
return delegate.getCriteriaBuilder();
}

@Override
public Cache getCache() {
return delegate.getCache();
}

@SuppressWarnings("rawtypes")
@Override
public EntityManager createEntityManager(Map arg0) {
entityManagerCreatedCount++;
return delegate.createEntityManager(arg0);
}

@Override
public EntityManager createEntityManager() {
entityManagerCreatedCount++;
return delegate.createEntityManager();
}

@Override
public EntityManager createEntityManager(SynchronizationType synchronizationType) {
entityManagerCreatedCount++;
return delegate.createEntityManager(synchronizationType);
}

@SuppressWarnings("rawtypes")
@Override
public EntityManager createEntityManager(SynchronizationType synchronizationType, Map map) {
entityManagerCreatedCount++;
return delegate.createEntityManager(synchronizationType, map);
}

@Override
public void close() {
delegate.close();
}

@Override
public void addNamedQuery(String name, Query query) {
delegate.addNamedQuery(name, query);
}

@Override
public <T> T unwrap(Class<T> cls) {
return delegate.unwrap(cls);
}

@Override
public <T> void addNamedEntityGraph(String graphName, EntityGraph<T> entityGraph) {
delegate.addNamedEntityGraph(graphName, entityGraph);
}
}

0 comments on commit 6fb1a1c

Please sign in to comment.