persistence@glassfish.java.net

Code Review for 2278 Query.executeUpdate() method doesn't throw TransactionRequiredException

From: Wonseok Kim <guruwons_at_gmail.com>
Date: Fri, 18 May 2007 19:49:15 +0900

Hi Tom, Andrei

Please review this really easy fix for the issue - just inserted tx check
:-)
https://glassfish.dev.java.net/issues/show_bug.cgi?id=2278

To confirm I also added a new test and fixed one existing test which calls
Query.executeUpdate() without tx.
Let me know if you see any problem.

Thanks,
-Wonseok

Index:
entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java,v
retrieving revision 1.37
diff -c -w -r1.37 EJBQueryImpl.java
***
entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java
 6 May 2007 02:16:02 -0000 1.37
---
entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java
 18 May 2007 09:17:27 -0000
***************
*** 380,385 ****
--- 380,389 ----
              if ( !(getDatabaseQuery() instanceof ModifyQuery) ){
                  throw new IllegalStateException(
ExceptionLocalization.buildMessage("incorrect_query_for_execute_update"));
              }
+
+             // need to throw TransactionRequiredException if there is no
active transaction
+             entityManager.checkForTransaction(true);
+
              //fix for bug:4288845, did not add the parameters to the query
              Vector parameterValues = processParameters();
              if(isFlushModeAUTO()) {
Index:
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/advanced/EntityManagerJUnitTestSuite.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/advanced/EntityManagerJUnitTestSuite.java,v
retrieving revision 1.49
diff -c -w -r1.49 EntityManagerJUnitTestSuite.java
***
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/advanced/EntityManagerJUnitTestSuite.java
 15 May 2007 18:10:27 -0000    1.49
---
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/advanced/EntityManagerJUnitTestSuite.java
 18 May 2007 09:17:29 -0000
***************
*** 2306,2311 ****
--- 2306,2312 ----
              return;
          }
          String seqName = "testOracleSequenceDefinition";
+         em.getTransaction().begin();
          try {
              // first param is preallocationSize, second is startValue
              // both should be positive
***************
*** 2313,2318 ****
--- 2314,2322 ----
              internalTestOracleSequenceDefinition(10, 5, seqName, em);
              internalTestOracleSequenceDefinition(10, 15, seqName, em);
          } finally {
+             if(em.getTransaction().isActive()) {
+                 em.getTransaction().rollback();
+             }
              em.close();
          }
      }
Index:
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLValidationTestSuite.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLValidationTestSuite.java,v
retrieving revision 1.29
diff -c -w -r1.29 JUnitEJBQLValidationTestSuite.java
***
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLValidationTestSuite.java
 26 Mar 2007 15:06:47 -0000    1.29
---
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/ejb/ejbqltesting/JUnitEJBQLValidationTestSuite.java
 18 May 2007 09:17:29 -0000
***************
*** 136,141 ****
--- 136,142 ----
          suite.addTest(new
JUnitEJBQLValidationTestSuite("illegalArgumentExceptionTest"));
          suite.addTest(new
JUnitEJBQLValidationTestSuite("createNamedQueryThrowsIllegalArgumentExceptionTest"));
          suite.addTest(new
JUnitEJBQLValidationTestSuite("flushTxExceptionTest"));
+         suite.addTest(new
JUnitEJBQLValidationTestSuite("testExecuteUpdateTxException"));
          suite.addTest(new
JUnitEJBQLValidationTestSuite("noResultExceptionTest"));
          suite.addTest(new
JUnitEJBQLValidationTestSuite("testGetSingleResultOnUpdate"));
          suite.addTest(new
JUnitEJBQLValidationTestSuite("testGetSingleResultOnDelete"));
***************
*** 956,961 ****
--- 957,989 ----
          }
      }
+     public void testExecuteUpdateTxException()
+     {
+         boolean testPass=false;
+         String ejbqlString = "DELETE FROM Employee e WHERE e.lastName=\
"doesNotExist\"";
+
+         EntityManager em = createEntityManager();
+         try
+         {
+             Object result = em.createQuery(ejbqlString).executeUpdate();
+
+             //rollback for clean-up if above call does not fail, otherwise
this may affect other tests
+             if(!em.getTransaction().isActive()){
+                 em.getTransaction().begin();
+             }
+             em.getTransaction().rollback();
+         }
+         catch (TransactionRequiredException e)
+         {
+             testPass = true;
+         }
+         finally
+         {
+             em.close();
+         }
+         Assert.assertTrue("TransactionRequiredException is expected",
testPass);
+     }
+
      public void createNamedQueryThrowsIllegalArgumentExceptionTest()
      {
          try