Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestHeader should be implicitly available for Action Bodyparser #8218

Open
mvdstam opened this issue Feb 9, 2018 · 1 comment
Open

RequestHeader should be implicitly available for Action Bodyparser #8218

mvdstam opened this issue Feb 9, 2018 · 1 comment

Comments

@mvdstam
Copy link

mvdstam commented Feb 9, 2018

Hello,

Please regard the following use case:

def indexSubmit = myActionHandler(parse.form(MyForm, onErrors = indexSubmitErrorHandler)).async { implicit request =>
    // request.body is available and valid here
  }

private def indexSubmitErrorHandler(formWithErrors: Form[MyModel]) = {
  BadRequest(myFormView(formWithErrors))
}

I really like this concept, since this would decouple the handling of errors completely from the action body of a HTTP endpoint. However, in my case this won't compile, since my template expects an implicit RequestHeader. Looking at BodyParsers.scala, I see the following:

def form[A](form: Form[A], maxLength: Option[Long] = None, onErrors: Form[A] => Result = (formErrors: Form[A]) => Results.BadRequest): BodyParser[A] =
    BodyParser { requestHeader =>
      import play.core.Execution.Implicits.trampoline
      val parser = anyContent(maxLength)
      parser(requestHeader).map { resultOrBody =>
        resultOrBody.right.flatMap { body =>
          form
            .bindFromRequest()(Request[AnyContent](requestHeader, body))
            .fold(formErrors => Left(onErrors(formErrors)), a => Right(a))
        }
      }
    }

I'd like to suggest marking requestHeader here as implicit to solve this issue, since any templates that expect RequestHeader values won't work.

As a workaround, I've moved the parsing to the body of the action:

def indexSubmit = myActionHandler.async { implicit request =>
    MyForm.bindFromRequest.fold(
      formWithErrors => Future.successful(BadRequest(myFormView(formWithErrors))),
      formData => {
        // formData is available here
      }
    )
  }

In addition to this, I think it would be useful to allow asynchronous errorhandlers as well, since it's not always possible (or even desirable) to have completely synchronous code within an error handler:

def form[A](form: Form[A], maxLength: Option[Long] = None, onErrors: Form[A] => Future[Result] = (formErrors: Form[A]) => Future.successful(Results.BadRequest)): BodyParser[A]
@mvdstam
Copy link
Author

mvdstam commented Feb 10, 2018

After looking into this, I realized that solving this will require more than simply marking requestHeader as implicit. What I originally had in mind, was this:

A method within a controller used for onErrors:

private def indexSubmitErrorHandler(formWithErrors: Form[MyModel])(implicit request: RequestHeader) = {
  BadRequest(myFormView(formWithErrors))
}

And then simply in BodyParsers.scala:

def form[A](form: Form[A], maxLength: Option[Long] = None, onErrors: Form[A] => Result = (formErrors: Form[A]) => Results.BadRequest): BodyParser[A] =
    BodyParser { implicit requestHeader =>
      import play.core.Execution.Implicits.trampoline
      val parser = anyContent(maxLength)
      parser(requestHeader).map { resultOrBody =>
        resultOrBody.right.flatMap { body =>
          form
            .bindFromRequest()(Request[AnyContent](requestHeader, body))
            .fold(formErrors => Left(onErrors(formErrors)), a => Right(a))
        }
      }
    }

Unless I'm missing something here, this will never work. Overloading this method like so will also not work due to type erasure, since onErrors will still be a Function1 and won't compile:

def form[A](form: Form[A], maxLength: Option[Long] = None, onErrors: (Form[A], RequestHeader) => Result = (formErrors: Form[A], requestHeader: RequestHeader) => Results.BadRequest): BodyParser[A] =
    BodyParser { requestHeader =>
      import play.core.Execution.Implicits.trampoline
      val parser = anyContent(maxLength)
      parser(requestHeader).map { resultOrBody =>
        resultOrBody.right.flatMap { body =>
          form
            .bindFromRequest()(Request[AnyContent](requestHeader, body))
            .fold(formErrors => Left(onErrors(formErrors, requestHeader)), a => Right(a))
        }
      }
    }

Any thoughts about this? 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants