#788 jpa: Object-methods like finalize become transactional, if the Type is annotated Transactional

extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java

@@ -43,18 +43,35 @@ class JpaLocalTxnInterceptor implements MethodInterceptor {
   @Transactional
   private static class Internal {}
 
-  // Tracks if the unit of work was begun implicitly by this transaction.
+  /** Tracks if the unit of work was begun implicitly by this transaction. */
   private final ThreadLocal<Boolean> didWeStartWork = new ThreadLocal<Boolean>();
+
+  /** Lenient mode allows interceptors not having a real Transactional annotation.
+      Interceptors created by the standard module won't be lenient. */
+  private boolean lenient;
+
+  public JpaLocalTxnInterceptor() {
+    this.lenient = true;
+  }
+
+  public JpaLocalTxnInterceptor(boolean lenient) {
+    this.lenient = lenient;
+  }
 
   public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+
+    // Check that the method or the class have a Transactional annotation
+    Transactional transactional = readTransactionMetadata(methodInvocation);
+    if (transactional == null) {
+      return methodInvocation.proceed();
+    }
 
     // Should we start a unit of work?
     if (!emProvider.isWorking()) {
       emProvider.begin();
       didWeStartWork.set(true);
     }
 
-    Transactional transactional = readTransactionMetadata(methodInvocation);
     EntityManager em = this.emProvider.get();
 
     // Allow 'joining' of transactions if there is an enclosing @Transactional method.
@@ -105,16 +122,25 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
   private Transactional readTransactionMetadata(MethodInvocation methodInvocation) {
     Transactional transactional;
     Method method = methodInvocation.getMethod();
-    Class<?> targetClass = methodInvocation.getThis().getClass();
-
     transactional = method.getAnnotation(Transactional.class);
+
+    Class<?> targetClass = method.getDeclaringClass();
     if (null == transactional) {
       // If none on method, try the class.
       transactional = targetClass.getAnnotation(Transactional.class);
     }
-    if (null == transactional) {
-      // If there is no transactional annotation present, use the default
-      transactional = Internal.class.getAnnotation(Transactional.class);
+
+    // Lenient mode: for backwards compatibility, try to find one
+    if (lenient && (null == transactional)) {
+      Class<?> calledClass = methodInvocation.getThis().getClass();
+      if (null == transactional) {
+        // try the called class.
+        transactional = calledClass.getAnnotation(Transactional.class);
+      }
+      if (null == transactional) {
+        // if there is no transactional annotation present, use the default
+        transactional = Internal.class.getAnnotation(Transactional.class);
+      }
     }
 
     return transactional;

extensions/persist/src/com/google/inject/persist/jpa/JpaPersistModule.java

@@ -70,7 +70,7 @@ public JpaPersistModule(String jpaUnit) {
     bind(EntityManagerFactory.class)
         .toProvider(JpaPersistService.EntityManagerFactoryProvider.class);
 
-    transactionInterceptor = new JpaLocalTxnInterceptor();
+    transactionInterceptor = new JpaLocalTxnInterceptor(false);
     requestInjection(transactionInterceptor);
 
     // Bind dynamic finders.

extensions/persist/test/com/google/inject/persist/jpa/ClassLevelManagedLocalTransactionsTest.java

@@ -21,6 +21,7 @@
 import com.google.inject.Injector;
 import com.google.inject.persist.PersistService;
 import com.google.inject.persist.Transactional;
+import com.google.inject.persist.jpa.util.TrackedEntityManagerFactory;
 
 import junit.framework.TestCase;
 
@@ -30,6 +31,7 @@
 import java.util.List;
 
 import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
 
 /**
  * This test asserts class level @Transactional annotation behavior.
@@ -145,6 +147,22 @@ 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 trackedEMF = new TrackedEntityManagerFactory(originalEMF);
+    persistService.start(trackedEMF);
+
+    // When toString is invoked in the transactional object
+    FakeTransactionalObject transactionalObject = injector.getInstance(FakeTransactionalObject.class);
+    transactionalObject.toString();
+
+    // No transaction was started
+    assertFalse("transaction has been started for Object method",
+        trackedEMF.hasCreatedSomeEntityManager());
+  }
 
   @Transactional
   public static class TransactionalObject {
@@ -201,4 +219,10 @@ public void runOperationInTxnThrowingChecked() throws IOException {
       throw new IOException();
     }
   }
+
+  @Transactional
+  public static class FakeTransactionalObject {
+    public void fakeTransactionalMethod() {
+    }
+  }
 }

extensions/persist/test/com/google/inject/persist/jpa/util/TrackedEntityManagerFactory.java

@@ -0,0 +1,79 @@
+package com.google.inject.persist.jpa.util;
+
+import java.util.Map;
+
+import javax.persistence.Cache;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.PersistenceUnitUtil;
+import javax.persistence.criteria.CriteriaBuilder;
+import javax.persistence.metamodel.Metamodel;
+
+/**
+ * Proxy EntityManager that adds tracking capabilities, keeping a count of
+ * created objects.
+ */
+public class TrackedEntityManagerFactory implements EntityManagerFactory {
+
+  private EntityManagerFactory originalEMF;
+  private int entityManagerCreatedCount = 0;
+
+  public TrackedEntityManagerFactory(EntityManagerFactory originalEMF) {
+    this.originalEMF = originalEMF;
+  }
+
+  public boolean hasCreatedSomeEntityManager() {
+    return (entityManagerCreatedCount > 0);
+  }
+
+  public int getEntityManagerCreatedCount() {
+    return entityManagerCreatedCount;
+  }
+
+  @Override
+  public boolean isOpen() {
+    return originalEMF.isOpen();
+  }
+
+  @Override
+  public Map<String, Object> getProperties() {
+    return originalEMF.getProperties();
+  }
+
+  @Override
+  public PersistenceUnitUtil getPersistenceUnitUtil() {
+    return originalEMF.getPersistenceUnitUtil();
+  }
+
+  @Override
+  public Metamodel getMetamodel() {
+    return originalEMF.getMetamodel();
+  }
+
+  @Override
+  public CriteriaBuilder getCriteriaBuilder() {
+    return originalEMF.getCriteriaBuilder();
+  }
+
+  @Override
+  public Cache getCache() {
+    return originalEMF.getCache();
+  }
+
+  @Override
+  public EntityManager createEntityManager(Map arg0) {
+    entityManagerCreatedCount++;
+    return originalEMF.createEntityManager(arg0);
+  }
+
+  @Override
+  public EntityManager createEntityManager() {
+    entityManagerCreatedCount++;
+    return originalEMF.createEntityManager();
+  }
+
+  @Override
+  public void close() {
+    originalEMF.close();
+  }
+}