You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2010/11/28 14:23:22 UTC

svn commit: r1039880 - in /camel/trunk/components/camel-spring/src: main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java

Author: davsclaus
Date: Sun Nov 28 13:23:21 2010
New Revision: 1039880

URL: http://svn.apache.org/viewvc?rev=1039880&view=rev
Log:
CAMEL-3373: Fixed markRollbackOnlyLast affecting outer transaction. Fixed logging of commit/rollback when markRollbackOnlyLast was used.

Modified:
    camel/trunk/components/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java
    camel/trunk/components/camel-spring/src/test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java

Modified: camel/trunk/components/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java
URL: http://svn.apache.org/viewvc/camel/trunk/components/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java?rev=1039880&r1=1039879&r2=1039880&view=diff
==============================================================================
--- camel/trunk/components/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java (original)
+++ camel/trunk/components/camel-spring/src/main/java/org/apache/camel/spring/spi/TransactionErrorHandler.java Sun Nov 28 13:23:21 2010
@@ -28,6 +28,7 @@ import org.apache.camel.processor.except
 import org.apache.camel.util.ObjectHelper;
 import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.TransactionStatus;
+import org.springframework.transaction.support.TransactionCallback;
 import org.springframework.transaction.support.TransactionCallbackWithoutResult;
 import org.springframework.transaction.support.TransactionTemplate;
 
@@ -125,7 +126,7 @@ public class TransactionErrorHandler ext
         } catch (TransactionRollbackException e) {
             // ignore as its just a dummy exception to force spring TX to rollback
             if (log.isDebugEnabled()) {
-                log.debug("Transaction rollback (" + id + ") for ExchangeId: " + exchange.getExchangeId());
+                log.debug("Transaction rollback (" + id + ") for ExchangeId: " + exchange.getExchangeId() + " due exchange was marked for rollbackOnly");
             }
         } catch (Exception e) {
             log.warn("Transaction rollback (" + id + ") for ExchangeId: " + exchange.getExchangeId() + " due exception: " + e.getMessage());
@@ -134,8 +135,26 @@ public class TransactionErrorHandler ext
             // mark the end of this transaction boundary
             exchange.getUnitOfWork().endTransactedBy(transactionTemplate);
         }
-    }
 
+        // if it was a local rollback only then remove its marker so outer transaction wont see the marker
+        Boolean onlyLast = (Boolean) exchange.removeProperty(Exchange.ROLLBACK_ONLY_LAST);
+        if (onlyLast != null && onlyLast) {
+            if (log.isDebugEnabled()) {
+                // log exception if there was a cause exception so we have the stacktrace
+                Exception cause = exchange.getException();
+                if (cause != null) {
+                    log.debug("Transaction rollback (" + id + ") for ExchangeId: " + exchange.getExchangeId()
+                        + " due exchange was marked for rollbackOnlyLast and due exception: ", cause);
+                } else {
+                    log.debug("Transaction rollback (" + id + ") for ExchangeId: " + exchange.getExchangeId()
+                        + " due exchange was marked for rollbackOnlyLast");
+                }
+            }
+            // remove caused exception due we was marked as rollback only last
+            // so by removing the exception, any outer transaction will not be affected
+            exchange.setException(null);
+        }
+    }
 
     protected void doInTransactionTemplate(final Exchange exchange) {
 
@@ -146,7 +165,7 @@ public class TransactionErrorHandler ext
             protected void doInTransactionWithoutResult(TransactionStatus status) {
                 // wrapper exception to throw if the exchange failed
                 // IMPORTANT: Must be a runtime exception to let Spring regard it as to do "rollback"
-                RuntimeException rce = null;
+                RuntimeException rce;
 
                 // and now let process the exchange by the error handler
                 processByErrorHandler(exchange);
@@ -154,14 +173,10 @@ public class TransactionErrorHandler ext
                 // after handling and still an exception or marked as rollback only then rollback
                 if (exchange.getException() != null || exchange.isRollbackOnly()) {
 
-                    // if it was a local rollback only then remove its marker so outer transaction
-                    // wont rollback as well (Note: isRollbackOnly() also returns true for ROLLBACK_ONLY_LAST)
-                    exchange.removeProperty(Exchange.ROLLBACK_ONLY_LAST);
-
                     // wrap exception in transacted exception
                     if (exchange.getException() != null) {
                         rce = ObjectHelper.wrapRuntimeCamelException(exchange.getException());
-                    } else if (exchange.isRollbackOnly()) {
+                    } else {
                         // create dummy exception to force spring transaction manager to rollback
                         rce = new TransactionRollbackException();
                     }
@@ -170,10 +185,8 @@ public class TransactionErrorHandler ext
                         status.setRollbackOnly();
                     }
 
-                    // rethrow if an exception occurred
-                    if (rce != null) {
-                        throw rce;
-                    }
+                    // throw runtime exception to force rollback (which works best to rollback with Spring transaction manager)
+                    throw rce;
                 }
             }
         });

Modified: camel/trunk/components/camel-spring/src/test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java
URL: http://svn.apache.org/viewvc/camel/trunk/components/camel-spring/src/test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java?rev=1039880&r1=1039879&r2=1039880&view=diff
==============================================================================
--- camel/trunk/components/camel-spring/src/test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java (original)
+++ camel/trunk/components/camel-spring/src/test/java/org/apache/camel/spring/interceptor/MixedTransactionPropagationTest.java Sun Nov 28 13:23:21 2010
@@ -81,11 +81,32 @@ public class MixedTransactionPropagation
         assertEquals("Number of books", 1, count);
     }
 
-    public void testMixed() throws Exception {
+    public void testMixedRollbackOnlyLast() throws Exception {
         template.sendBody("direct:mixed", "Hello World");
 
         int count = jdbc.queryForInt("select count(*) from books");
-        assertEquals("Number of books", 4, count);
+        assertEquals("Number of books", 3, count);
+
+        // assert correct books in database
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Camel in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Tiger in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Elephant in Action'"));
+        assertEquals(0, jdbc.queryForInt("select count(*) from books where title = 'Lion in Action'"));
+        assertEquals(0, jdbc.queryForInt("select count(*) from books where title = 'Donkey in Action'"));
+    }
+
+    public void testMixedCommit() throws Exception {
+        template.sendBody("direct:mixed3", "Hello World");
+
+        int count = jdbc.queryForInt("select count(*) from books");
+        assertEquals("Number of books", 5, count);
+
+        // assert correct books in database
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Camel in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Tiger in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Elephant in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Lion in Action'"));
+        assertEquals(1, jdbc.queryForInt("select count(*) from books where title = 'Crocodile in Action'"));
     }
 
     protected RouteBuilder createRouteBuilder() throws Exception {
@@ -108,21 +129,40 @@ public class MixedTransactionPropagation
                     // all these steps will be okay
                     .setBody(constant("Tiger in Action")).beanRef("bookService")
                     .setBody(constant("Elephant in Action")).beanRef("bookService")
-                    .setBody(constant("Lion in Action")).beanRef("bookService")
                     // continue on route 2
                     .to("direct:mixed2");
 
                 from("direct:mixed2")
-                    // using a different propagation which is requires new
-                    .transacted("PROPAGATION_REQUIRES_NEW")
                     // tell Camel that if this route fails then only rollback this last route
                     // by using (rollback only *last*)
                     .onException(Exception.class).markRollbackOnlyLast().end()
+                    // using a different propagation which is requires new
+                    .transacted("PROPAGATION_REQUIRES_NEW")
                     // this step will be okay
-                    .setBody(constant("Giraffe in Action")).beanRef("bookService")
+                    .setBody(constant("Lion in Action")).beanRef("bookService")
                     // this step will fail with donkey
                     .setBody(constant("Donkey in Action")).beanRef("bookService");
                 // END SNIPPET: e1
+
+                from("direct:mixed3")
+                    // using required
+                    .transacted("PROPAGATION_REQUIRED")
+                    // all these steps will be okay
+                    .setBody(constant("Tiger in Action")).beanRef("bookService")
+                    .setBody(constant("Elephant in Action")).beanRef("bookService")
+                    // continue on route 4
+                    .to("direct:mixed4");
+
+                from("direct:mixed4")
+                    // tell Camel that if this route fails then only rollback this last route
+                    // by using (rollback only *last*)
+                    .onException(Exception.class).markRollbackOnlyLast().end()
+                    // using a different propagation which is requires new
+                    .transacted("PROPAGATION_REQUIRES_NEW")
+                    // this step will be okay
+                    .setBody(constant("Lion in Action")).beanRef("bookService")
+                    // this step will be okay
+                    .setBody(constant("Crocodile in Action")).beanRef("bookService");
             }
         };
     }