zio-http icon indicating copy to clipboard operation
zio-http copied to clipboard

Refactor middleware to use declarative encoding based on new machinery in zio.http.api

Open jdegoes opened this issue 2 years ago • 17 comments

Is your feature request related to a problem? Please describe.

The middleware uses an executable encoding, which makes introspection impossible. In addition to having some performance implications, this means that it is not known in advance which headers, query parameters, or route segments a piece of middleware will inspect, or if and how a piece of middleware will modify the response headers or bodies.

This means that middleware will not form a part of the documentation generated for API endpoints created using zio.http.api. So, for example, an authentication middleware will, if used on an endpoint defined using zio.http.api, not contribute any relevant details to the documentation for this endpoint when it is generated. Similarly, the automatic client generated "for free" by APIExecutor will not be able to provide middleware with whatever it requires.

Describe the solution you'd like

In order to solve this problem, as well as create many more possibilities for optimization, the design of middleware needs to be refactored to be declarative. Previously, this would not have been possible, because a declarative description of what headers, query parameters, or route segments needed by middleware did not exist.

Now, thanks to the new machinery in zio.http.api, it is possible to declaratively describe the inputs to a middleware. This is a necessary but not sufficient step toward a declarative encoding for middleware.

The following shows an early (NOT suitable) design for a declarative encoding:

sealed trait Middleware[-R, +E, +AIn, -BIn, -AOut, +BOut]
object Middleware {
  final case class Http[A](in: In[A], incoming: (A, Request) => Request, outgoing: Response => Response) extends Middleware[Any, Nothing, Request, Response, Request, Response]
  ...
}

Describe alternatives you've considered

None.

jdegoes avatar Sep 18 '22 16:09 jdegoes

I would like to take this up

afsalthaj avatar Sep 18 '22 21:09 afsalthaj

@afsalthaj Let's work on it together, if you don't mind! Will be done twice as fast. 😆

jdegoes avatar Sep 20 '22 18:09 jdegoes

For sure. :)

On Tue, 20 Sep 2022 at 11:37 pm, John A. De Goes @.***> wrote:

@afsalthaj https://github.com/afsalthaj Let's work on it together, if you don't mind! Will be done twice as fast. 😆

— Reply to this email directly, view it on GitHub https://github.com/zio/zio-http/issues/1501#issuecomment-1252723550, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABY2QJIFNGW4PJRFWSSHJO3V7H4PPANCNFSM6AAAAAAQPQBN6A . You are receiving this because you were mentioned.Message ID: @.***>

afsalthaj avatar Sep 21 '22 04:09 afsalthaj

In our first collaboration, we discussed making the following changes:

  1. Adding a new type parameter to In to track how the In is generated. This would allow us to impose constraints such as:
    1. This In can only be composed with that In (e.g. path1 / path2).
    2. This In has already set the body and cannot be composed with another one that has also set the body.
    3. This In may NOT consume path segments (i.e. have a Route constructor inside it).
  2. Augmenting Out with two additional constructors:
    1. One constructor to append a header to the response.
    2. Another constructor to combine two Out into a single Out, similar to In.Combine.
  3. Leveraging the new type parameter in Out to improve type safety:
    1. Do NOT allow combining two Out that have both independently set the response body.
  4. Augmenting API with two new type parameters: MiddlewareIn, and MiddlewareOut, which are used to form two new fields: middlewareIn: In[MiddlewareInput], and middlewareOut: Out[MiddlewareOutput], thus producing a declarative description of how middleware processes requests and how it augments responses.
  5. Propagating these new type parameters into Service, and preserving them during service composition.

This is only half of the design problem: we did NOT have a chance to look at how middleware should be redesigned using a declarative encoding. However, we do have some general notes on a more declarative middleware, composed of four things:

  1. What does the middleware need from the request? (In[MiddlewareInput], above)
  2. How does the middleware modify the response? (Out[MiddlewareOutput], above)
  3. How does the middleware modify the request?
  4. What does the middleware need from the response?
  5. What effect do we execute upon middleware input?
  6. What effect do we execute using the response?

A very naive model of this is something like:

trait Middleware[R, E, Input, Output] {
  def preaction(in: Input): ZIO[R, E, Unit]
  def postaction(response: Response): ZIO[R, E, Output]
}

This is not complete (by far!), because it lacks the capabilities of existing middleware (and type parameters), and it is not clear how a single middleware concept would apply to both a service as well as an http. In addition, feeding all of Response to middleware seems overkill and impossible to optimize versus a more constrained approach (but yet, API does not need to know about that).

In the next session, we will attempt to resolve these issues.

cc @afsalthaj @adamgfraser

jdegoes avatar Sep 21 '22 22:09 jdegoes

One idea:

object Example {

  // executable middleware preserves existing functionality when needed
  trait Middleware[-R, +E, +AIn, -BIn, -AOut, +BOut] {
    def apply[R1 <: R, E1 >: E](http: Http[R1, E1, AIn, BIn]): Http[R1, E1, AOut, BOut]
  }

  // subset of middleware that can be declaratively described
  sealed trait APIMiddleware[-R, +E, In, Out] extends Middleware[R, E, Request, Request, Response, Response]

  object APIMiddleware {

    // possible to "embed" some executable middleware into API though without docs
    final case class Executable[-R, +E](executable: Middleware[R, E, Request, Request, Response, Response])
        extends APIMiddleware[R, E, Unit, Unit] {
      def apply[R1 <: R, E1 >: E](http: Http[R1, E1, Request, Request]): Http[R1, E1, Response, Response] =
        executable(http)
    }

    // fixed hierarchy of fully declarative middleware
    sealed trait Declarative[-R, +E, In, Out] extends APIMiddleware[R, E, In, Out] {

      // can "interpret" any declarative descripton to update an appropriate app
      def apply[R1 <: R, E1 >: E](http: Http[R1, E1, Request, Response]): Http[R1, E1, Request, Response] =
        ???
    }

    object Declarative {

      sealed trait Input[-R, +E, In] extends Declarative[R, E, In, Unit]
      object Input {
        final case class AddHeader(header: Header) extends Input[Any, Nothing, Unit]
      }

      sealed trait Output[-R, +E, Out] extends Declarative[R, E, Unit, Out]

      sealed trait Execution[-R, +E] extends Declarative[R, E, Unit, Unit]
    }
  }
}

I'm worried about declarative middleware having an R and an E type parameter since API doesn't so it seems like we may need to get rid of that. Also need to think about how codecs would work with this because it seems like they can transform the In and Out types rather than just adding constraints to it.

adamgfraser avatar Sep 22 '22 03:09 adamgfraser

This might be super close to what Adam said. That we could add two types of Middleware, one is Executable Middleware, and the other an IntrospectableMiddleware.

Conceptually


type HttpMiddleware[R, E] = HttpApp[R, E] => HttpApp[R, E]
sealed trait  IntrospectableMiddleware[A, B], 
which is a ADT with terms representing transformation from API[A, B] to API[A, B]

Do we need any other middleware other than HttpMiddleware ?

Hopefully the answer is yes. In fact the very notion of HttpMiddleware (executable) can be simplified for developers. Example:

Instead of

val middlewares: HttpMiddleware[Any, IOException] =
    // print debug info about request and response
    Middleware.debug ++
      // close connection if request takes more than 3 seconds
      Middleware.timeout(3 seconds) ++
      // add static header
      Middleware.addHeader("X-Environment", "Dev") ++
      // add dynamic header
      serverTime

  // Run it like any simple app
  val run = Server.serve(app @@ middlewares).provide(Server.default)

We could simply do

val app: HttpApp = ???

app
  .withTimeOut(3.seconds)
  .withDebug
  .addResponseHeader(response => Clock.now.map(t => response.addHeader("time" -> t.toString))

afsalthaj avatar Sep 27 '22 03:09 afsalthaj

I had to edit the above snippet a couple of times :)

afsalthaj avatar Sep 27 '22 03:09 afsalthaj

Here's the changes I was envisioning being made to API:

final case class API[MiddlewareIn, MiddlewareOut, HandlerIn, HandlerOut](
  middlewareIn: In[Query & Headers, MiddlewareIn],
  middlewareOut: Out[Headers, Unit],
  handlerIn: In[Route & Query & Headers & Body, HandlerIn],
  handlerOut: Out[Headers & Body, HandlerOut],
  doc: Doc
)

(In addition to the changes described above to In and Out.)

Now we have a precise description of what middleware requires.

Then we can further divide middleware into: MiddlewareSpec[In, Out], which is a bundle of middlewareIn / middlewareOut (in fact, we should store MiddlewareSpec in API as a single logical entity, probably), and Middleware, which is the "handler" or "implementation" of a MiddlewareSpec. I don't know what that would look like, but I know the goal: the middleware can only do what the spec says.

I think it's necessary to decrease the power of Middleware: it cannot alter the input or output type anymore.

I think that's fine as for performance reasons we'll be pushing people to API which already handles decoding / encoding.

jdegoes avatar Sep 27 '22 13:09 jdegoes

cc @adamgfraser @afsalthaj

jdegoes avatar Sep 27 '22 13:09 jdegoes

@adamgfraser I like that direction. To use my names, MiddlewareSpec cannot use E or R, and may only describe:

  1. What the middleware requires from the request
  2. What the middleware adds to the request (does this make sense? Maybe for middleware chaining???)
  3. What the middleware requires from the response (does this make sense? Maybe for middelware chaining??)
  4. What the middleware adds to the response

In the above code sketch, API can do both (1) and (4). We have to decide on (2) and (3).

I think we delete A/B in/out from Middleware (deleting its ability to do transcoding) so it becomes Middleware[R, E], or maybe: Middleware[R, E, In, Out] (???).

Perhaps, to convert a API to a Service, you need to specify TWO things:

  1. The handler function, which handles the input to produce the output.
  2. The middleware, which handles the middleware input to produce the middleware output.

If you did not define a middleware spec, then maybe you don't need to provide middleware.

Or maybe it should work a bit different: you can implement a MiddlewareSpec to produce a Middleware, which can independently be applied to the Http you get back from Service#toHttp.

jdegoes avatar Sep 27 '22 14:09 jdegoes

Actually, In is already sufficient for Middleware by itself because it's invertible. So we can simplify to:

final case class API[MiddlewareIn, MiddlewareOut, HandlerIn, HandlerOut](
  middlewareIn: In[Query & Headers, MiddlewareIn],
  middlewareOut: In[Headers, MiddlewareOut],
  handlerIn: In[Route & Query & Headers & Body, HandlerIn],
  handlerOut: Out[HandlerOut]
  doc: Doc
)

In fact maybe we don't need Out if we have constrained In:

final case class API[MiddlewareIn, MiddlewareOut, HandlerIn, HandlerOut](
  middlewareIn: In[Query & Headers, MiddlewareIn],
  middlewareOut: In[Headers, MiddlewareOut],
  handlerIn: In[Route & Query & Headers & Body, HandlerIn],
  handlerOut: In[Body, HandlerOut]
  doc: Doc
)

Now a handler for middleware must accept MiddlewareIn (which we can construct from query and headers) and produce MiddlewareOut, which we can translate to appropriate headers.

Factoring out MiddlewareSpec:

final case class MiddlewareSpec[MiddlewareIn, MiddlewareOut] {
  middlewareIn: In[Query & Headers, MiddlewareIn],
  middlewareOut: In[Headers, MiddlewareOut]
)

final case class API[MiddlewareIn, MiddlewareOut, HandlerIn, HandlerOut](
  middlewareSpec: MiddlewareSpec[MiddlewareIn, MiddlewareOut],
  handlerIn: In[Route & Query & Headers & Body, HandlerIn],
  handlerOut: In[Body, HandlerOut]
  doc: Doc
)

jdegoes avatar Sep 27 '22 19:09 jdegoes

Working with these models already

afsalthaj avatar Sep 28 '22 05:09 afsalthaj

Question: will this usecase be described using the new encoding?

An authentication middleware

  1. Reads the auth token from the header
  2. Attempts to load a login session from the session storage with the token
  3. If the session is invalid, reject the request with a 401 response
  4. Otherwise produce the retrieved Session (or User,) so that the handler can use the valid session information in a type safe and ergonomic way

The existing executable encoding does 1,2,3 successfully, but not 4. There are a workaround using (Fiber)Ref with R type, but it doesn't guarantee the user is authenticated in type level.

guersam avatar Sep 28 '22 05:09 guersam

@guersam To accomodate this use case cleanly (i.e. without fiber ref), a middleware would have to be able to produce some value that can be consumed by the handler.

I am not sure what that would look like, but we can think about it.

jdegoes avatar Sep 28 '22 20:09 jdegoes

A very early stage trying to validate just the Out realm https://github.com/zio/zio-http/pull/1598

afsalthaj avatar Oct 01 '22 09:10 afsalthaj

Here is an example in the draft PR on how this looks like in terms of usage

https://github.com/zio/zio-http/blob/fb9fe69b5ff61e99798b0de574607b67bb5acd4e/zio-http-example/src/main/scala/example/APIExamples.scala

  val getUser =
    API.get(literal("users") / int).out[Int] @@ MiddlewareSpec.addHeader("key", "value")

It would be even ideal to have @@ at service level that inspects every API and add the middleware.

i.e

  val addHeader = 
    MiddlewareSpec.addHeader("key", "value")
    
  val getUser =
    API.get(literal("users") / int).out[Int]

  val getUsersService =
    getUser.handle[Any, Nothing] { case (id: Int) =>
      ZIO.succeedNow(1)
    }

  val getUserPosts =
    API
      .get(literal("users") / int / literal("posts") / query("name") / int)

  val getUserPostsService =
    getUserPosts.handle[Any, Nothing] { case (id1, query, id2) => ??? }

  val services = (getUsersService ++ getUserPostsService) @@ addHeader // which delegates to `@@` in `API` case class

However this implies the documentation need to rely on the API that is accessible from the service, and not the raw APIs. I hope that's the case anyway

cc @jdegoes @adamgfraser

afsalthaj avatar Oct 02 '22 03:10 afsalthaj

@afsalthaj I think it makes sense, to offer docs based on the combination of endpoints and middleware. That said, I think that it should be possible to combine docs on a higher level. But I don't see this conflicting.

987Nabil avatar Oct 02 '22 15:10 987Nabil

Can we close this now that we have the Middleware/ MiddlewareSpec in zio.http.api?

vigoo avatar Jan 21 '23 13:01 vigoo