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

Optionally allow unknown arguments #34

Closed
wants to merge 1 commit into from

Conversation

jodersky
Copy link
Member

This adds the ability for ArgumentReaders to control whther or not unknown arguments encountered in the entrypoint are considered an error.

The motivating use case is to allow parsing arbitrary query parameters.

@jodersky jodersky requested a review from lihaoyi August 22, 2020 09:49
@jodersky
Copy link
Member Author

jodersky commented Aug 22, 2020

In the end, I'm not so sure what to think of this solution. I find that having one argument reader change the parsing model for all other readers leads to surprising behavior: i.e. with the presence of an extra "magic" parameter type, all of a sudden the failure condition changes.

I also experimented with using a dedicated annotation to allow extra args (similar to the @doc annotation), but that also feels clunky.

Maybe, at the end of the day, the correct way to allow extra args is to create a custom endpoint? But that also reduces flexibility. Wdyt, @lihaoyi ?

This adds the ability for ArgumentReaders to control whther or not
unknown arguments encountered in the entrypoint are considered an
error.

The motivating use case is to allow parsing arbitrary query
parameters.
@jodersky
Copy link
Member Author

Closing this, as I really think the behavior is too surprising

@jodersky jodersky closed this Aug 22, 2020
@jodersky
Copy link
Member Author

jodersky commented Aug 22, 2020

Btw, in case anyone else come across the need to read an unknown number of query parameters, you can do this by defining a custom endpoint.

import cask.router.HttpEndpoint
import cask.model.{Request, Response}
import cask.router.{ArgReader, Result}

class rawQuery(val path: String, val methods: Seq[String] = Seq("get"), override val subpath: Boolean = false)
    extends HttpEndpoint[Response.Raw, String] {

  type InputParser[T] = rawQuery.Parser[T]

  // This is left empty on purpose, since query parameters will be accessed over the request context.
  // Note that path arguments are not passed through the "wrapFunction" delegation mechanism;
  // they are handled specifically via the Decorator's invoke() method
  def wrapFunction(ctx: Request, delegate: Delegate): Result[Response.Raw] = delegate(Map.empty) 
  def wrapPathSegment(s: String): String = s

}

object rawQuery {
  trait Parser[T] extends ArgReader[String, T, Request]

  // query param readers are used for paths (which are handled outside of wrapFunction)
  // this follows the convention used by cask's standard WebEndpoints
  implicit def qparamReader[T: QueryParamReader] = new Parser[T] {
    override def arity = implicitly[QueryParamReader[T]].arity
    override def read(ctx: cask.model.Request, label: String, v: String) = {
      implicitly[QueryParamReader[T]].read(ctx, label, Seq(v))
    }
  }

}
object ExtraParams extends cask.MainRoutes{

  // this only checks parameter matches on the path, it allows
  // an unknown number of query parameters
  @rawQuery("/echo/:param1")
  def echo1(param1: String, ctx: cask.Request) = ctx.exchange.getQueryString


  initialize()
}

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

Successfully merging this pull request may close these issues.

1 participant