htp akka#19681 MalformedRequestContentRejection cause traceability (a…

…kka#20346) * improved cause tracking for MalformedRequestContentRejection * unwrapping RejectionErrors when unmarshalling * amended old project name in CONTRIBUTING.md
drewhk · May 6, 2016 · 7d40c1d · 7d40c1d
1 parent 92a08ab
commit 7d40c1d
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 6 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -59,7 +59,7 @@ The Akka build includes a special task called `validatePullRequest` which invest
 then running tests only on those projects.
 
 For example changing something in `akka-http-core` would cause tests to be run in all projects which depend on it
-(e.g. `akka-http-core-tests`, `akka-http-marshallers-*`, `akka-docs` etc.).
+(e.g. `akka-http-tests`, `akka-http-marshallers-*`, `akka-docs` etc.).
 
 To use the task simply type, and the output should include entries like shown below:
 

diff --git a/...tests/src/test/scala/akka/http/scaladsl/server/directives/MarshallingDirectivesSpec.scala b/...tests/src/test/scala/akka/http/scaladsl/server/directives/MarshallingDirectivesSpec.scala
@@ -16,6 +16,9 @@ import spray.json.DefaultJsonProtocol._
 import MediaTypes._
 import HttpCharsets._
 import headers._
+import org.xml.sax.SAXParseException
+
+import scala.util.{ Failure, Try }
 
 class MarshallingDirectivesSpec extends RoutingSpec with Inside {
   import ScalaXmlSupport._
@@ -26,6 +29,12 @@ class MarshallingDirectivesSpec extends RoutingSpec with Inside {
       case NodeSeq.Empty ⇒ throw Unmarshaller.NoContentException
       case x             ⇒ { val i = x.text.toInt; require(i >= 0); i }
     }
+  implicit val TryIntUnmarshaller: FromEntityUnmarshaller[Try[Int]] =
+    IntUnmarshaller map {
+      Try(_).recoverWith {
+        case e: IllegalArgumentException ⇒ Failure(RejectionError(ValidationRejection(e.getMessage, Option(e))))
+      }
+    }
 
   val `text/xxml` = MediaType.customWithFixedCharset("text", "xxml", `UTF-8`)
   implicit val IntMarshaller: ToEntityMarshaller[Int] =
@@ -71,12 +80,23 @@ class MarshallingDirectivesSpec extends RoutingSpec with Inside {
         }
       }
     }
+    "unwrap a RejectionError and return its exception" in {
+      Put("/", HttpEntity(ContentType(`text/xml`, iso88592), "<int>-3</int>")) ~> {
+        entity(as[Try[Int]]) { _ ⇒ completeOk }
+      } ~> check {
+        inside(rejection) {
+          case ValidationRejection("requirement failed", Some(_: IllegalArgumentException)) ⇒
+        }
+      }
+    }
     "return a MalformedRequestContentRejection if unmarshalling failed due to a not further classified error" in {
       Put("/", HttpEntity(ContentTypes.`text/xml(UTF-8)`, "<foo attr='illegal xml'")) ~> {
         entity(as[NodeSeq]) { _ ⇒ completeOk }
       } ~> check {
-        rejection shouldEqual MalformedRequestContentRejection(
-          "XML document structures must start and end within the same entity.", None)
+        inside(rejection) {
+          case MalformedRequestContentRejection(
+            "XML document structures must start and end within the same entity.", _: SAXParseException) ⇒
+        }
       }
     }
     "extract an Option[T] from the requests entity using the in-scope Unmarshaller" in {

diff --git a/akka-http/src/main/scala/akka/http/impl/server/RejectionHandlerWrapper.scala b/akka-http/src/main/scala/akka/http/impl/server/RejectionHandlerWrapper.scala
@@ -48,7 +48,7 @@ private[http] class RejectionHandlerWrapper(javaHandler: server.RejectionHandler
         case TooManyRangesRejection(maxRanges) ⇒
           handleTooManyRangesRejection(ctx, maxRanges)
         case MalformedRequestContentRejection(message, cause) ⇒
-          handleMalformedRequestContentRejection(ctx, message, cause.orNull)
+          handleMalformedRequestContentRejection(ctx, message, cause)
         case RequestEntityExpectedRejection ⇒
           handleRequestEntityExpectedRejection(ctx)
         case UnacceptedResponseContentTypeRejection(supported) ⇒

diff --git a/akka-http/src/main/scala/akka/http/scaladsl/server/Rejection.scala b/akka-http/src/main/scala/akka/http/scaladsl/server/Rejection.scala
@@ -99,7 +99,7 @@ case class TooManyRangesRejection(maxRanges: Int) extends Rejection
  * Note that semantic issues with the request content (e.g. because some parameter was out of range)
  * will usually trigger a `ValidationRejection` instead.
  */
-case class MalformedRequestContentRejection(message: String, cause: Option[Throwable] = None) extends Rejection
+case class MalformedRequestContentRejection(message: String, cause: Throwable) extends Rejection
 
 /**
  * Rejection created by unmarshallers.

diff --git a/akka-http/src/main/scala/akka/http/scaladsl/server/directives/MarshallingDirectives.scala b/akka-http/src/main/scala/akka/http/scaladsl/server/directives/MarshallingDirectives.scala
@@ -33,10 +33,11 @@ trait MarshallingDirectives {
       import ctx.materializer
       onComplete(um(ctx.request)) flatMap {
         case Success(value) ⇒ provide(value)
+        case Failure(RejectionError(r)) ⇒ reject(r)
         case Failure(Unmarshaller.NoContentException) ⇒ reject(RequestEntityExpectedRejection)
         case Failure(Unmarshaller.UnsupportedContentTypeException(x)) ⇒ reject(UnsupportedRequestContentTypeRejection(x))
         case Failure(x: IllegalArgumentException) ⇒ reject(ValidationRejection(x.getMessage.nullAsEmpty, Some(x)))
-        case Failure(x) ⇒ reject(MalformedRequestContentRejection(x.getMessage.nullAsEmpty, Option(x.getCause)))
+        case Failure(x) ⇒ reject(MalformedRequestContentRejection(x.getMessage.nullAsEmpty, x))
       }
     } & cancelRejections(RequestEntityExpectedRejection.getClass, classOf[UnsupportedRequestContentTypeRejection])