Merge pull request #1235 from lift/dpp_issue_1235

Transaction S.addAround is broken in some cases
lift · Mar 12, 2012 · df9b69d · df9b69d
2 parents 33bf011 + 380ccf0
commit df9b69d
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 26 deletions.
diff --git a/persistence/db/src/main/scala/net/liftweb/db/DB.scala b/persistence/db/src/main/scala/net/liftweb/db/DB.scala
@@ -21,6 +21,8 @@ import common._
 import util._
 import Helpers._
 
+import net.liftweb.http.S
+
 import javax.sql.{DataSource}
 import java.sql.ResultSetMetaData
 import java.sql.{Statement, ResultSet, Types, PreparedStatement, Connection, DriverManager}
@@ -240,13 +242,13 @@ trait DB extends Loggable {
               try {
                 try {
                   val ret = f
-                  success = true
+                  success = !S.exceptionThrown_?
                   ret
                 } catch {
                   // this is the case when we want to commit the transaction
                   // but continue to throw the exception
                   case e: LiftFlowOfControlException => {
-                    success = true
+                    success = !S.exceptionThrown_?
                     throw e
                   }
                 }
@@ -263,13 +265,13 @@ trait DB extends Loggable {
             try {
               try {
                 val ret = f
-                success = true
+                success = !S.exceptionThrown_?
                 ret
               } catch {
                 // this is the case when we want to commit the transaction
                 // but continue to throw the exception
                 case e: LiftFlowOfControlException => {
-                  success = true
+                  success = !S.exceptionThrown_?
                   throw e
                 }
               }
@@ -676,13 +678,13 @@ trait DB extends Loggable {
       var rollback = true
       try {
         val ret = f(conn)
-        rollback = false
+        rollback = S.exceptionThrown_?
         ret
       } catch {
         // this is the case when we want to commit the transaction
         // but continue to throw the exception
         case e: LiftFlowOfControlException => {
-          rollback = false
+          rollback = S.exceptionThrown_?
           throw e
         }
       } finally {

diff --git a/project/Build.scala b/project/Build.scala
@@ -109,7 +109,7 @@ object BuildDef extends Build {
 
   lazy val db =
     persistenceProject("db")
-        .dependsOn(util)
+        .dependsOn(util, webkit)
         .settings(libraryDependencies += mockito_all)
 
   lazy val proto =

diff --git a/web/webkit/src/main/scala/net/liftweb/http/LiftRules.scala b/web/webkit/src/main/scala/net/liftweb/http/LiftRules.scala
@@ -1299,7 +1299,7 @@ class LiftRules() extends Factory with FormVendor with LazyLoggable {
    * a default implementation is already appended.
    *
    */
-  @volatile var exceptionHandler = RulesSeq[ExceptionHandlerPF].append {
+  val exceptionHandler = RulesSeq[ExceptionHandlerPF].append {
     case (Props.RunModes.Development, r, e) =>
       logger.error("Exception being returned to browser when processing " + r.uri.toString + ": " + showException(e))
       XhtmlResponse((<html> <body>Exception occured while processing {r.uri}<pre>{showException(e)}</pre> </body> </html>), S.htmlProperties.docType, List("Content-Type" -> "text/html; charset=utf-8"), Nil, 500, S.ieMode)

diff --git a/web/webkit/src/main/scala/net/liftweb/http/LiftServlet.scala b/web/webkit/src/main/scala/net/liftweb/http/LiftServlet.scala
@@ -294,6 +294,7 @@ class LiftServlet extends Loggable {
       case foc: LiftFlowOfControlException => throw foc
       case e: Exception if !e.getClass.getName.endsWith("RetryRequest") => {
         val bundle = (Props.mode, req, e)
+        S.assertExceptionThrown()
         NamedPF.applyBox(bundle, LiftRules.exceptionHandler.toList)
       }
     }
@@ -328,7 +329,6 @@ class LiftServlet extends Loggable {
           LiftSession.onBeginServicing.foreach(_(liftSession, req))
           val ret: (Boolean, Box[LiftResponse]) =
             try {
-              try {
                 // run the continuation in the new session
                 // if there is a continuation
                 continuation match {
@@ -353,21 +353,6 @@ class LiftServlet extends Loggable {
                       (true, Full(liftSession.checkRedirect(req.createNotFound(f))))
                   }
                 }
-              } catch {
-                case ContinueResponseException(cre) => throw cre
-
-                case ite: java.lang.reflect.InvocationTargetException if (ite.getCause.isInstanceOf[ResponseShortcutException]) =>
-                  (true, Full(liftSession.handleRedirect(ite.getCause.asInstanceOf[ResponseShortcutException], req)))
-
-                case rd: net.liftweb.http.ResponseShortcutException => (true, Full(liftSession.handleRedirect(rd, req)))
-
-                case e if (e.getClass.getName.endsWith("RetryRequest")) => throw e
-
-                case e: LiftFlowOfControlException => throw e
-
-                case e => (true, NamedPF.applyBox((Props.mode, req, e), LiftRules.exceptionHandler.toList))
-
-              }
 
             } finally {
               if (S.functionMap.size > 0) {
@@ -489,7 +474,7 @@ class LiftServlet extends Loggable {
         }
       } catch {
         case foc: LiftFlowOfControlException => throw foc
-        case e => NamedPF.applyBox((Props.mode, requestState, e), LiftRules.exceptionHandler.toList);
+        case e => S.assertExceptionThrown() ; NamedPF.applyBox((Props.mode, requestState, e), LiftRules.exceptionHandler.toList);
       }
       tryo {
         LiftSession.onEndServicing.foreach(_(liftSession, requestState, ret))

diff --git a/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala b/web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala
@@ -1145,7 +1145,7 @@ class LiftSession(private[http] val _contextPath: String, val uniqueId: String,
 
       case e: LiftFlowOfControlException => throw e
 
-      case e => NamedPF.applyBox((Props.mode, request, e), LiftRules.exceptionHandler.toList);
+      case e => S.assertExceptionThrown() ; NamedPF.applyBox((Props.mode, request, e), LiftRules.exceptionHandler.toList);
 
     }
 

diff --git a/web/webkit/src/main/scala/net/liftweb/http/S.scala b/web/webkit/src/main/scala/net/liftweb/http/S.scala
@@ -338,6 +338,8 @@ trait S extends HasParams with Loggable {
   private val _oneShot = new ThreadGlobal[Boolean]
   private val _disableTestFuncNames = new ThreadGlobal[Boolean]
 
+  private object _exceptionThrown extends TransientRequestVar(false)
+
   private object postFuncs extends TransientRequestVar(new ListBuffer[() => Unit])
 
   /**
@@ -386,6 +388,18 @@ trait S extends HasParams with Loggable {
 
   def location: Box[sitemap.Loc[_]] = CurrentLocation.is
 
+
+  /**
+   * An exception was thrown during the processing of this request.
+   * This is tested to see if the transaction should be rolled back
+   */
+  def assertExceptionThrown() {_exceptionThrown.set(true)}
+
+  /**
+   * Was an exception thrown during the processing of the current request?
+   */
+  def exceptionThrown_? : Boolean = _exceptionThrown.get
+
   /**
    * @return a List of any Cookies that have been set for this Response. If you want
    * a specific cookie, use findCookie.