persistence@glassfish.java.net

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

From: Tom Ware <tom.ware_at_oracle.com>
Date: Fri, 18 May 2007 10:34:48 -0400

Hi Wonseok,

  The changes looks good to me.

-Tom

Wonseok Kim wrote:

> 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
> <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
>