improvements to multipart file handling

core/src/main/scala/io/fintrospect/parameters/Body.scala

@@ -47,16 +47,26 @@ object Body {
     * for server-server communications when you want the server to reject with a BadRequest.
     * This method simply takes a set of form fields.
     */
-  def form(fields: FormField[_] with Extractor[Form, _]*): UrlEncodedFormBody = new UrlEncodedFormBody(fields, new StrictFormCodec())
+  def form(fields: FormField[_] with Extractor[Form, _]*): UrlEncodedFormBody = new UrlEncodedFormBody(fields, StrictFormValidator, StrictFormFieldExtractor)
 
   /**
     * HTML encoded form HTTP message body which deserializes even if fields are missing/invalid. Use this
     * for browser-server communications where you want to give feedback to the user.
     * This method takes a set of form fields, combined with their relevant error messages in case of validation failure.
     */
-  def webForm(fields: (FormField[_] with Extractor[Form, _], String)*): UrlEncodedFormBody = new UrlEncodedFormBody(fields.map(_._1), new WebFormCodec(Map(fields: _*)))
+  def webForm(fields: (FormField[_] with Extractor[Form, _], String)*): UrlEncodedFormBody = new UrlEncodedFormBody(fields.map(_._1), new WebFormValidator(Map(fields: _*)), WebFormFieldExtractor)
 
-  def multiPartForm(fields: FormField[_] with Extractor[Form, _]*): MultiPartFormBody = new MultiPartFormBody(fields, new StrictFormCodec())
+  /**
+    * MultiPart encoded form HTTP message body which will fail to deserialize if a single field is missing/invalid. Use this
+    * for server-server communications when you want the server to reject with a BadRequest.
+    * This method simply takes a set of form fields.
+    */
+  def multiPartForm(fields: FormField[_] with Extractor[Form, _]*): MultiPartFormBody = new MultiPartFormBody(fields, StrictFormValidator, StrictFormFieldExtractor)
 
-  def multiPartWebForm(fields: (FormField[_] with Extractor[Form, _], String)*): MultiPartFormBody = new MultiPartFormBody(fields.map(_._1), new StrictFormCodec())
+  /**
+    * MultiPart encoded form HTTP message body which deserializes even if fields are missing/invalid. Use this
+    * for browser-server communications where you want to give feedback to the user.
+    * This method takes a set of form fields, combined with their relevant error messages in case of validation failure.
+    */
+  def multiPartWebForm(fields: (FormField[_] with Extractor[Form, _], String)*): MultiPartFormBody = new MultiPartFormBody(fields.map(_._1), StrictFormValidator, WebFormFieldExtractor)
 }

core/src/main/scala/io/fintrospect/parameters/FormField.scala

@@ -63,10 +63,11 @@ object FormField {
     override def apply[T](spec: ParameterSpec[T]) = new SingleParameter(spec, FormFieldExtractAndRebind) with FormField[T] with Mandatory[T]
 
     def file(inName: String, inDescription: String = null) = new SingleFile(inName, inDescription) with MandatoryFile {
-      override def <--?(form: Form): Extraction[MultiPartFile] = form.files.get(inName) match {
-        case Some(s) => Extracted(s.headOption)
-        case None => ExtractionFailed(Missing(this))
-      }
+      override def <--?(form: Form): Extraction[MultiPartFile] =
+        form.files.get(inName) match {
+          case Some(files) => Extracted(files.headOption)
+          case None => ExtractionFailed(Missing(this))
+        }
     }
 
     override val multi = new Parameters[MultiFormField, MandatorySeq] {
@@ -75,7 +76,7 @@ object FormField {
       def file(inName: String, inDescription: String = null) =
         new MultiFile(inName, inDescription) with MandatoryFileSeq {
           override def <--?(form: Form): Extraction[Seq[MultiPartFile]] = form.files.get(inName) match {
-            case Some(s) => Extracted(Option(s.toSeq))
+            case Some(files) => Extracted(Option(files.toSeq))
             case None => ExtractionFailed(Missing(this))
           }
         }
@@ -95,7 +96,7 @@ object FormField {
       def file(inName: String, inDescription: String = null) =
         new MultiFile(inName, inDescription) with OptionalFileSeq {
           override def <--?(form: Form): Extraction[Seq[MultiPartFile]] = form.files.get(inName) match {
-            case Some(s) => Extracted(Option(s.toSeq))
+            case Some(files) => Extracted(Option(files.toSeq))
             case None => Extracted(None)
           }
         }

core/src/main/scala/io/fintrospect/parameters/FormFieldExtractor.scala

@@ -0,0 +1,19 @@
+package io.fintrospect.parameters
+
+import io.fintrospect.util.{Extracted, Extraction, ExtractionFailed, Extractor}
+
+sealed trait FormFieldExtractor {
+  def apply(fields: Seq[Extractor[Form, _]], f: Form): Extraction[Form]
+}
+
+object WebFormFieldExtractor extends FormFieldExtractor {
+  override def apply(fields: Seq[Extractor[Form, _]], t: Form): Extraction[Form] = Extracted(Some(t))
+}
+
+object StrictFormFieldExtractor extends FormFieldExtractor {
+  override def apply(fields: Seq[Extractor[Form, _]], form: Form): Extraction[Form] = Extraction.combine(fields.map(_.extract(form))) match {
+    case failed@ExtractionFailed(_) => failed
+    case _ => Extracted(Some(form))
+  }
+}
+

core/src/main/scala/io/fintrospect/parameters/FormValidator.scala

@@ -0,0 +1,35 @@
+package io.fintrospect.parameters
+
+import io.fintrospect.util._
+
+/**
+  * A strategy for validating the fields in a form
+  */
+sealed trait FormValidator {
+  def apply(fields: Seq[Extractor[Form, _]], form: Form): Form
+}
+
+/**
+  * Web-forms are a less harsh version of forms, which report both a collection of received fields and a set of invalid fields.
+  * This form-type is to be used for web forms (where feedback is desirable and the user can be redirected back to the form page).
+  * As such, extracting an invalid webform from a request will not fail unless the body encoding itself is invalid.
+  */
+class WebFormValidator(messages: Map[Parameter, String]) extends FormValidator {
+  override def apply(fields: Seq[Extractor[Form, _]], rawForm: Form): Form = {
+    new Form(rawForm.fields, rawForm.files, fields.flatMap {
+      _ <--? rawForm match {
+        case ExtractionFailed(e) => e.map(er => ExtractionError(er.param, messages.getOrElse(er.param, er.reason)))
+        case _ => Nil
+      }
+    })
+  }
+}
+
+/**
+  * Strict Forms fail when failing even a single field fails.
+  * This form is used for non-web forms (where the posted form is merely an url-encoded set of form parameters) and
+  * will auto-reject requests with a BadRequest.
+  */
+object StrictFormValidator extends FormValidator {
+  override def apply(fields: Seq[Extractor[Form, _]], rawForm: Form): Form = rawForm
+}

core/src/main/scala/io/fintrospect/parameters/MultiPartFormBody.scala

@@ -0,0 +1,49 @@
+package io.fintrospect.parameters
+
+import com.twitter.finagle.http._
+import com.twitter.finagle.http.exp.Multipart.{FileUpload, InMemoryFileUpload, OnDiskFileUpload}
+import com.twitter.io.Buf
+import io.fintrospect.ContentTypes.MULTIPART_FORM
+import io.fintrospect.util.{Extraction, ExtractionError, ExtractionFailed, Extractor}
+
+import scala.util.{Failure, Success, Try}
+
+class MultiPartFormBody(fields: Seq[FormField[_] with Extractor[Form, _]],
+                        validator: FormValidator, extractor: FormFieldExtractor)
+  extends Body[Form] {
+
+  override val contentType = MULTIPART_FORM
+
+  override def iterator = fields.iterator
+
+  def -->(value: Form): Seq[RequestBinding] =
+    Seq(new RequestBinding(null, req => {
+      val fields = value.fields.flatMap(f => f._2.map(g => SimpleElement(f._1, g))).toSeq
+      val files = value.files.flatMap(f => f._2.map(g => FileElement(f._1, g.content, g.contentType, g.filename))).toSeq
+
+      val next = RequestBuilder()
+        .url("http://notreallyaserver")
+        .addHeaders(Map(req.headerMap.toSeq: _*))
+        .add(fields ++ files)
+        .buildFormPost(multipart = true)
+      next.uri = req.uri
+      next
+    }))
+
+  override def <--?(message: Message): Extraction[Form] = message.asInstanceOf[Request].multipart
+    .map(m => {
+      val multipart = message.asInstanceOf[Request].multipart.get
+      Try(validator(fields, Form(multipart.attributes.mapValues(_.toSet), multipart.files.mapValues(_.map(toMultipartFile).toSet)))) match {
+        case Success(form) => extractor(fields, form)
+        case Failure(e) => ExtractionFailed(fields.filter(_.required).map(param => ExtractionError(param, "Could not parse")))
+      }
+    }).getOrElse(ExtractionFailed(fields.filter(_.required).map(param => ExtractionError(param, "Could not parse"))))
+
+  private def toMultipartFile(f: FileUpload) = {
+    f match {
+      case InMemoryFileUpload(content, fileType, name, _) => MultiPartFile(content, Option(fileType), Option(name))
+      // FIXME - OnDiskUploads!
+      case OnDiskFileUpload(_, fileType, name, _) => MultiPartFile(Buf.Empty, Option(fileType), Option(name))
+    }
+  }
+}

core/src/main/scala/io/fintrospect/parameters/Retrieval.scala

@@ -27,7 +27,8 @@ trait Mandatory[-From, T] extends Retrieval[From, T] with Extractor[From, T] {
   override def <--(from: From): T = extract(from) match {
     case Extracted(Some(t)) => t
     case Extracted(None) => throw new IllegalStateException("Extraction failed: Missing")
-    case _ => throw new IllegalStateException("Extraction failed: Invalid")
+    case e =>
+      throw new IllegalStateException("Extraction failed: Invalid")
   }
 }
 

core/src/main/scala/io/fintrospect/parameters/UrlEncodedFormBody.scala

@@ -1,60 +1,52 @@
 package io.fintrospect.parameters
 
+import java.net.{URLDecoder, URLEncoder}
+
 import com.twitter.finagle.http._
-import com.twitter.io.Buf
-import io.fintrospect.ContentTypes.{APPLICATION_FORM_URLENCODED, MULTIPART_FORM}
+import io.fintrospect.ContentTypes.APPLICATION_FORM_URLENCODED
 import io.fintrospect.util.{Extraction, ExtractionError, ExtractionFailed, Extractor}
 import org.jboss.netty.handler.codec.http.HttpHeaders.Names
 
 import scala.util.{Failure, Success, Try}
 
-class UrlEncodedFormBody(val fields: Seq[FormField[_] with Extractor[Form, _]], encodeDecode: FormCodec)
+class UrlEncodedFormBody(fields: Seq[FormField[_] with Extractor[Form, _]],
+                         validator: FormValidator, extractor: FormFieldExtractor)
   extends Body[Form] {
 
   override val contentType = APPLICATION_FORM_URLENCODED
 
   override def iterator = fields.iterator
 
+  private def decodeFields(content: String): Map[String, Set[String]] = {
+    content
+      .split("&")
+      .filter(_.contains("="))
+      .map(nvp => {
+        val parts = nvp.split("=")
+        (URLDecoder.decode(parts(0), "UTF-8"), if (parts.length > 1) URLDecoder.decode(parts(1), "UTF-8") else "")
+      })
+      .groupBy(_._1)
+      .mapValues(_.map(_._2))
+      .mapValues(_.toSet)
+  }
+
+  private def encode(form: Form): String = form.fields.flatMap {
+    case (name, values) => values.map(value => URLEncoder.encode(name, "UTF-8") + "=" + URLEncoder.encode(value, "UTF-8"))
+  }.mkString("&")
+
+
   def -->(value: Form): Seq[RequestBinding] =
     Seq(new RequestBinding(null, req => {
-      val contentString = encodeDecode.encode(value)
+      val contentString = encode(value)
       req.headerMap.add(Names.CONTENT_TYPE, contentType.value)
       req.headerMap.add(Names.CONTENT_LENGTH, contentString.length.toString)
       req.contentString = contentString
       req
     })) ++ fields.map(f => new FormFieldBinding(f, ""))
 
   override def <--?(message: Message): Extraction[Form] =
-    Try(encodeDecode.decode(fields, message)) match {
-      case Success(form) => encodeDecode.extract(fields, form)
+    Try(validator(fields, new Form(decodeFields(message.contentString), Map.empty, Nil))) match {
+      case Success(form) => extractor(fields, form)
       case Failure(e) => ExtractionFailed(fields.filter(_.required).map(param => ExtractionError(param, "Could not parse")))
     }
 }
-
-class MultiPartFormBody(val fields: Seq[FormField[_] with Extractor[Form, _]], encodeDecode: FormCodec)
-  extends Body[Form] {
-
-  override val contentType = MULTIPART_FORM
-
-  override def iterator = fields.iterator
-
-  def -->(value: Form): Seq[RequestBinding] =
-    Seq(new RequestBinding(null, req => {
-      val fields = value.fields.flatMap(f => f._2.map(g => SimpleElement(f._1, g))).toSeq
-      val files = value.files.flatMap(f => f._2.map(g => FileElement(f._1, null, null, null))).toSeq
-      val headers = Map(req.headerMap.toSeq: _*)
-      RequestBuilder().url(req.uri).addHeaders(headers).add(fields).add(files).buildFormPost(multipart = true)
-    }))
-
-  override def <--?(message: Message): Extraction[Form] = message.asInstanceOf[Request].multipart
-    .map(m => {
-      val multipart = message.asInstanceOf[Request].multipart.get
-      Try(Form(
-        multipart.attributes.mapValues(_.toSet),
-        multipart.files.mapValues(_.map(f => MultiPartFile(Buf.Empty, Option(f.contentType), Option(f.fileName))).toSet)
-      )) match {
-        case Success(form) => encodeDecode.extract(fields, form)
-        case Failure(e) => ExtractionFailed(fields.filter(_.required).map(param => ExtractionError(param, "Could not parse")))
-      }
-    }).getOrElse(ExtractionFailed(fields.filter(_.required).map(param => ExtractionError(param, "Could not parse"))))
-}