[RFC] Parameterize context type of decorators #137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jodersky
commented
Jul 23, 2024
| @@ -14,10 +15,10 @@ import cask.model.{Request, Response} | |||
| * to `wrapFunction`, which takes a `Map` representing any additional argument | |||
| * lists (if any). | |||
| */ | |||
| trait Decorator[OuterReturned, InnerReturned, Input] extends scala.annotation.Annotation { | |||
| trait Decorator[OuterReturned, InnerReturned, Input, InputContext] extends scala.annotation.Annotation { | |||
There was a problem hiding this comment.
this is the main change here, along with adding an extra parameter to delegate and changing invocation to accommodate these changes
jodersky
commented
Jul 23, 2024
jodersky
commented
Jul 24, 2024
Comment on lines
+1
to
+55
| package app | ||
|
|
||
| case class Context( | ||
| session: Session | ||
| ) | ||
|
|
||
| case class Session(data: collection.mutable.Map[String, String]) | ||
|
|
||
| trait CustomParser[T] extends cask.router.ArgReader[Any, T, Context] | ||
| object CustomParser: | ||
| given CustomParser[Context] with | ||
| def arity = 0 | ||
| def read(ctx: Context, label: String, input: Any): Context = ctx | ||
| given CustomParser[Session] with | ||
| def arity = 0 | ||
| def read(ctx: Context, label: String, input: Any): Session = ctx.session | ||
| given literal[Literal]: CustomParser[Literal] with | ||
| def arity = 1 | ||
| def read(ctx: Context, label: String, input: Any): Literal = input.asInstanceOf[Literal] | ||
|
|
||
| object DecoratedContext extends cask.MainRoutes{ | ||
|
|
||
| class custom extends cask.router.Decorator[cask.Response.Raw, cask.Response.Raw, Any, Context]{ | ||
|
|
||
| override type InputParser[T] = CustomParser[T] | ||
|
|
||
| def wrapFunction(req: cask.Request, delegate: Delegate) = { | ||
| // Create a custom context out of the request. Custom contexts are useful | ||
| // to group an expensive operation that may be used by multiple | ||
| // parameter readers or that carry state. This example focuses on carrying | ||
| // state. | ||
| val ctx = Context(Session(collection.mutable.Map.empty)) // this would typically be populated from a signed cookie | ||
|
|
||
| delegate(ctx, Map("user" -> 1337)).map{ response => | ||
| val extraCookies = ctx.session.data.map( | ||
| (k, v) => cask.Cookie(k, v) | ||
| ) | ||
|
|
||
| response.copy( | ||
| cookies = response.cookies ++ extraCookies | ||
| ) | ||
| } | ||
|
|
||
| } | ||
| } | ||
|
|
||
| @custom() | ||
| @cask.get("/hello/:world") | ||
| def hello(world: String, req: cask.Request)(session: Session, user: Int) = { | ||
| session.data("hello") = "world" | ||
| world + user | ||
| } | ||
|
|
||
| initialize() | ||
| } |
There was a problem hiding this comment.
See this as a motivating example
|
I think this looks reasonable |
jodersky
force-pushed
the
jo/inputcontext
branch
from
7253b61
to
cbfe275
Compare
jodersky
force-pushed
the
jo/inputcontext
branch
from
a4e9441
to
1f7549c
Compare
|
@lihaoyi, I ported the changes to also work with scala 2, and included the example in the published ones. Let me know if you think this is now good to merge. |
lihaoyi
approved these changes
Nov 3, 2024
|
@jodersky looks good, you have commit access so go ahead and merge this and the other PRs when you're happy and tag a release |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows customizing the way by-type parameters can be read: instead of always reading data from a
cask.Request, this allows a decorator to parameterize the context type and pass it in explicitly from thewrapFunctionmethod.Essentially, this makes the context parameter of
ArgReaders (which are used to translate data from a HTTP request to a scala parameter) customizable for every decorator.Motivation
The idea behind this proposal is to uniformize the way "named" and "by-type" parameters within the same endpoint are handled. By "named" parameters I mean parameters which are set from the
Map[String, Input]in the delegate function, and byby-typeparameters I mean parameters which are computed from an arity zeroArgReader, and thus use thecask.Requestcontext to compute their value.E.g.
In this case,
fooandbarand "named" andcookie1andreqare "by-type".Right now, the
wrapFunctionmethod handles how named parameters are set, and therefore can centrally do arbitrary pre- and post-processing. However, by-type parameters must always use acask.Requestto compute any values, which can be problematic if the computation is expensive or if any kind of state is maintained.This pull request removes this asymmetry, by allowing the
wrapFunctionto compute a custom context which is computed once before being passed to allInputReadersImplementation approach
InputContexttype parameter to decoratorsInputContexttodelegate.type Delegate = (InputContext, Map[String, Input)) => ...Decorator.invoke()to carry a list of input contexts, one for each parameter listEntrypointmacros to use these input contexts rather than hardcodingcask.Request(currently I've only done the work to support Scala 3 for this)This is a proof of concept PR. As of this writing, the code only works for Scala 3.