Make the HTTP feature more flexible to foster easy migrations

[rafaparadela]

As @fedefernandez @raulraja suggested, we should focus not only on greenfield projects and also on existing projects. So in spite of we considered that the HTTP endpoints didn't need parameters (because could be inferred), we want to provide enough flexibility to let other projects be migrated into Mu without too many efforts.

As the first step, we should add these parameters to the @http annotation to let the derived server work as expected:

• HTTP Method: GET, POST, PUT, etc.
• Prefix
• Operation

That along with the hostname and the port set when instantiating the server, would compose the entire endpoint paths.

METHOD http://host:port/prefix/operation

(where the prefix can be composed by several segments)

[fedefernandez]

@rafaparadela instead of adding new configuration params or extra complexity to our http definitions I suggest re-using the http4s types. Let me explain it with an example.

Suppose you have the following service:

  final case class Request1(id: String)
  final case class Response1(access: Boolean)

  final case class Request2()
  final case class Response2()

  trait MyService[F[_]] {
    def op1(request: Request1): F[Response1]
    def op2(request: Request2): F[Response2]
  }

  // ...
  val myService: MyService[F] = ???

We could ask for the following implicits in the macro:

  val pf1: PartialFunction[Request[F], F[Request1]] = ???
  val pf2: PartialFunction[Request[F], F[Request2]] = ???

  val srv1: Kleisli[F, Response1, Response[F]] = ???
  val srv2: Kleisli[F, Response2, Response[F]] = ???

But providing some default values. For example, for the first one could be something like in the macro def:

  val pf1: PartialFunction[Request[F], F[Request1]] = {
    case msg @ POST -> Root / "op1" => msg.as[Request1]
  }

Then, the only thing we need to do is compose those implicits with the service function:

  val s1: PartialFunction[Request[F], F[Response[F]]] = pf1.andThen(_.flatMap(myService.op1)).andThen(_.flatMap(srv1.run))
  val s2: PartialFunction[Request[F], F[Response[F]]] = pf2.andThen(_.flatMap(myService.op2)).andThen(_.flatMap(srv2.run))

  HttpRoutes.of(s1 orElse s2)

This will allow the users to define their owns routes and their own responses, based on the types. For example, for the first case, suppose you want to have a GET operation, receiving the param in the path and returning forbidden when the field in the response is false. All you need to do is to provide the right implicits in the scope:

  implicit val pf1: PartialFunction[Request[F], F[Request1]] = {
    case GET -> Root / "op1" / myId => Applicative[F].pure(Request1(myId))
  }
  implicit val srv1: Kleisli[F, Response1, Response[F]] = Kleisli {
    case Response1(true)  => Ok()
    case Response1(false) => Forbidden()    
  }

Let me know what you think. @juanpedromoreno @raulraja thoughts?

[rafaparadela]

I really like this approach. Especially because we delegate on the users the responsibility to build the routes by themselves. In the current approach that I've been working on, as you can see here, we need to guess what the user wants to do with the response. For instance, in the HEAD case, the body of the response has to be empty, so I added the result of the service as a header. I was not completely happy with this strategy, but as a first iteration, was OK, because RPC and HTTP are different and of course, I assume that we have to make some decisions like that.

But your approach makes more sense to me because they have to define what to do with the entire Response, if they want to discard the body or use the headers, or using a different status code.
Also, this also resolves an issue that I hadn't resolved yet: The query parameters and the header in the response. (It was difficult to solve this using more parameters in the HTTP annotation).

So, in summary. I'm happy to switch the strategy at this moment because although I was about to submit the PR to close the #583, I think that your suggestion worths to delay this.

Note: I'm not sure how to adapt this the derivation of the client, since I was building those pieces considering the request method, and some of them were returning F[Response[F]] (because the body is empty), and others F[MyResponse]. I think that base on your example, all operations in the derived clients will return F[Response[F]], which is still great because we keep being consistent on giving the user the maximum flexibility.

[fedefernandez]

I didn't know you were working on this actively, sorry for not raising this before. As you said, keeping things with the http4s gives us more flexibility and at the same time keeping things easier. I've been working for some time with real APIs and I think we can't cover all the edge cases, even the regular ones (custom authentication, path args, custom headers, custom metrics, ...). This would support all of this.

[rafaparadela]

No worries. I'm going to discard this, and probably will submit an independent PR that closes #574 whose implementation was included in the work I did this weekend.

[SemanticBeeng]

As part of this (?) can we have @service(AvroWithSchema) not imply grpc ?

Could implement this (with json) over http only so we can call mu from web clients that do not support http2.

Learned from @AdrianRaFo about the "client code" generation work in progress but also also that mu rpc really means grpc.
Related questions here #21 (comment).

This means that to call mu services from web clients (was hoping to cross-compile client subs with scalajs) would need to be wrapped in some other form of http endpoints at this time - right?

Still studying this proposed work but does not seem to help with this issue.

Kindly advise.

[SemanticBeeng]

I'm going to discard this

@rafaparadela can you please keep it somewhere ?

[SemanticBeeng]

@fedefernandez this looks very cool.

I am trying to have way to call strongly typed services from web based clients written in Scala and cross compiled with scalajs.

Thinking to implement the client with hammock (see http://pepegar.com/hammock/marshalling.html). These would call these hand-crafted http4s routes you suggest, right?

Please advise if this makes sense.

If all all possible I'd like to ensure it all adds up in the end so kindly consider this use case when building. Also maybe we can partner to make an end to end implementation work?

[fedefernandez]

@SemanticBeeng that sounds reasonable to me.

mu has support for http but is not documented yet. The support was added here:

• Sample http4s REST client/server with client macro derivation #552

What you're proposing is totally feasible. In order to make the call, you need to send a POST with a json of the request to the endpoint root/serviceName/operation. See here for more information but bear in mind that it's still a WIP.

[SemanticBeeng]

See here for more information but bear in mind that it's still a WIP.

@fedefernandez Looks sweet and will go ahead and use it for something I need to build right away.

01 "Wip" but approved and the way to go in terms of design, right ?
Asking because not sure (yet) how it fits with the design proposed in this issue.

Looking at code below with this in mind with the intent of using hammock:

val unaryClient = UnaryGreeter.httpClient[IO](serviceUri)

BlazeClientBuilder[IO](ec).resource.use(unaryClient.getHello(_))

02 In context of "Clients should remain Scala.js compatible" #21 (comment) is UnaryGreeter.httpClient meant to be Scala.js compatible?

03 If it is then could simply replace BlazeClientBuilder with calls from hammock, right?
Should I try to refactor to cross-compile?

04 If it is not at this time but should be then what would it take to make it?

Seems is is the main thing necessary to achieve the cross-compile-able "clients".
Am I missing or conflicting with anything ?

05 Conceptually this httpClient is equivalent with something like this grpc client https://github.com/higherkindness/mu/blob/arf-generated-sample/benchmarks/shared/src/main/scala/higherkindness/mu/rpc/benchmarks/shared/protocols/PersonServiceAvroExpanded.scala (from @AdrianRaFo ) which only works on the server (it seems).

[SemanticBeeng]

04 If it is not at this time but should be then what would it take to make it?

Seems macro generated code is not Scala.js cross-compile-able due to references to http4s (the only lib referenced that is not).

package higherkindness.mu.rpc.http

import cats.effect._
import fs2.Stream
import io.circe.syntax._
import higherkindness.mu.http.implicits._
import org.http4s._
import org.http4s.circe._
import org.http4s.client._

class UnaryGreeterRestClient[F[_]: Sync](uri: Uri) {

  def getHello()(client: Client[F])(
      implicit decoderHelloResponse: io.circe.Decoder[HelloResponse]): F[HelloResponse] = {
    val request = Request[F](Method.GET, uri / "getHello")
    client.expectOr[HelloResponse](request)(handleResponseError)(jsonOf[F, HelloResponse])
  }

...

}

class Fs2GreeterRestClient[F[_]: Sync](uri: Uri) {

  def sayHellos(arg: Stream[F, HelloRequest])(client: Client[F])(
      implicit encoderHelloRequest: io.circe.Encoder[HelloRequest],
      decoderHelloResponse: io.circe.Decoder[HelloResponse]): F[HelloResponse] = {
    val request = Request[F](Method.POST, uri / "sayHellos")
    client.expectOr[HelloResponse](request.withEntity(arg.map(_.asJson)))(handleResponseError)(
      jsonOf[F, HelloResponse])
  }

But if this is all there is we could use hammock instead because it cross-compiles.

@fedefernandez , @pepegar thoughts please?

[pepegar]

Hi @SemanticBeeng

Yep, I think hammock is a good idea if you're planning to
cross-compile here.

Regarding the imports in the macro, currently most of mu is defaulting
to Http4s to do all things HTTP, so I believe introducing hammock for
macros is a bit too disrupting.

What I'd advise in that case is using mu's http functionality (which
would help beta-test it :D) to generate the server and create the
client manually using hammock. That's probably gonna be the easiest
way for you.

[SemanticBeeng]

Oki, good hear to overall idea makes sense, coming from you. :-)

"most of mu is defaulting to Http4s to do all things HTTP"

Yes, can see that.
Is it true that the use of http4s on the client is not key to the design or even implementation?

To test this understanding may I reword this to say this?

In the long run , in the spirit of "Clients should remain Scala.js compatible" #21, mu development will consider having separate macros for generating scala.js compatible clients.

Will proceed as per your suggestion.
Likely will copy / paste / modify the code generated by current macros to use hammock .
Thanks!

[pepegar]

In the long run , in the spirit of "Clients should remain Scala.js compatible" #21, mu development will consider having separate macros for generating scala.js compatible clients.

yep, that's possible. However currently that's a big change to introduce.

Will proceed as per your suggestion.
Likely will copy / paste / modify the code generated by current macros to use hammock .

Awesome! please report back how it goes. And feel free to contact me for hammock related stuff.

[noelmarkham]

I did some work to implement @fedefernandez's suggestion for making the HTTP feature more flexible. I effectively hand-rolled what the macro would do (which really was an exercise for me to understand Mu and the HTTP feature a little better), and got it all working. I put what I did on the branch feature/583-more-http-verbs if you want to have a look, but be warned it's in no shape for public consumption really!

Going down this road would lead to a very small and tidy macro implementation, but more work for the user to wire this all together.

A couple of points I had:

1. In allowing the client to define their own routes, I had to make the F concrete so that it would work with the concrete .route[IO] call. I don't think this is an issue on its own. It may be possible to keep the type constructor abstract, but I couldn't see an immediate way to do this, but I didn't spend much time on it.

2. This felt more important than my previous point: By asking for implicits, by their nature, you can only ever have one type that matches, so if you have multiple routes that have the same signature, it's not immediately clear that this can be done, or at least in a simple way.
   For example, if you have a service made with the following definition (I'm making this up but hopefully it's realistic enough):

trait UserService[F[_]] {
   def getAllUsers(request: Empty.type): F[List[User]]
   def getActiveUsers(request: Empty.type): F[List[User]]
}

Then we'd expect two different pairs of implicits with the same type for these:

PartialFunction[Request[F], F[Empty.type]]
Kleisli[F, List[User], Response[F]]

Every logical parameterless 'get' (lowercase) is going have that same PartialFunction signature.
These would need to be differentiated some way. Ideas such as

• Change the interface
• Have some kind of tagging

could work, but these don't feel natural from a user's point of view.

Similary, a RESTful style for other definitions throw up similar discussions:

trait UserService[F[_]] {
  def createUser(request: User): F[Empty.type]
  def deleteUser(request: User): F[Empty.type]
}

These fit very nicely into the REST model for POST/PUT and DELETE, but there's no direct translaction due to the fact the implicits required would clash.

I'm still very new to this application, so perhaps I've missed something big, but would be interested to get some other thoughts on this. I do wonder if going down this road means hiding too much about the HTTP interface to be useful, and it seems like @rafaparadela's suggested commit may get around some of these limitations.

[L-Lavigne]

Agreed with @noelmarkham that we shouldn't rely on implicits where we might need different implementations for two or more methods with the same signature, which is a reasonable thing for the user to define.

We could try using ordinary parameters with default values. I'm working on a (crude) adaption of Noel's unit test that does this; I expect to push it tomorrow.

[fedefernandez]

@noelmarkham @L-Lavigne I agree, the implicits approach won't work. I'm looking forward to see the approach with default values and how they look with the macro.

[L-Lavigne]

@noelmarkham @fedefernandez Please have a look at 1f9d477. Note that it's a very crude approach as I had to use Option parameters defaulting to None just to get it working quickly, but hopefully it should be a good basis of discussion.

[L-Lavigne]

BTW, not sure why the new localhost server test is passing locally and falling in CI - just saw that the previous version with implicits had the same problem. In my commit I fixed an issue with tests failing even locally because of unstopped server state from previous tests (the fix was to take serverTask.cancel and add .unsafeRunSync()) but this seems to be something different.

[noelmarkham]

That commit looks great. You could get around the None for the PartialFunction by using Map.empty, and for the Kleisli[F, A, B], I suppose as long as the F[B] is a monoid, then you could use its identity? Perhaps that's a bit over the top.
One thing I'm usure about with that custom implementation is that for when the user creates their route:

routeCreator[IO](doPingRoute = Some(CustomRoutesAndResponders.doPingRoute))

In this case, they need to be aware of the name of the name doPingRoute in order to assign a new custom route to it - it doesn't seem clear to me how they'd easily know what's it's called once that name is generated by the macro.

But I am wondering if this is the right approach at all. It feels a little like we're jumping through a lot of hoops just to keep the macro generation down. What is the main intention of this work? If it is to make this as straightforward and intuitive for the clients of the library, then I'm not sure this approach is it. Having the overheads of PartialFunctions and Kleislis (and possible having to explain that) doesn't feel right.

I imagine a user looking at the APIs and seeing the implementation looking something like @http(method = PUT, path = "/my/custom/path") is just going to feel a lot more approachable and natural.

[L-Lavigne]

@noelmarkham Agreed, there's a risk this could end up over-engineered. I think it comes down to whether we need to allow arbitrary code in the user customization or not. If the vast majority of use-cases are just defining a custom HTTP verb and/or endpoint path, we can just use annotations for this.

A good guideline could be the upcoming generation from OpenAPI scenario: anything allowed by the spec should be expressible in the Mu code whether generated or hand-written, but not anything more for now.

[noelmarkham]

Sounds good - I'm going to look at the annotations implementation once I've finished the bug I'm working on.

[noelmarkham]

I'd like to solicit some feedback for the following approach to HTTP server generation for Mu.

I've based this on @rafaparadela's work (https://github.com/higherkindness/mu/pull/586/files) and @BeniVF's notes (https://github.com/47deg/marlow/issues/188#issuecomment-493085319), the code could look something like the following:

@service(Avro) trait ExampleService[F[_]] {

  @http4s(GET, Root / "status") 
  def getStatus(request: Empty.type): F[Response]

  @http4s(GET, Root / "more" / name) 
  def getMore(name: String): F[Response]

  @http4s(GET, Root / "evenMore" / IntVar(userId)) 
  def getEvenMore(userId: Int): F[Response]

  @http4s(GET, Root / "another", List(QueryParameterKey("foo"))) 
  def getAnother(queryParams: List[String]): F[Response]

  @http4s(GET, Root / "everything" / name, List(QueryParameterKey("foo"))) 
  def getAnother(name: String, queryParams: List[String]): F[Response]
}

I've called the annotation @http4s as this is tightly coupled to Http4s, considering the implementation is anyway:

class http4s(
  method:      org.http4s.Method,
  path:        org.http4s.dsl.impl.Path,
  queryParams: List[org.http4s.QueryParameterKey] = Nil
) extends StaticAnnotation

Using the Http4s Path type means it should be possible to extract path parameters and make the macro enforce those parameter types exist in the annotated method.

Having populated queryParams would then make sure the macro enforces the annotated method to have a queryParams: List[String] parameter.

There are also other things that should be added to the @http4s annotation, such as headers, cookies, content types etc, but I feel the above is a good place to start.

I've not given much thought yet to how to handle responses other than 2xx. It feels like, for a response F[A] some kind of A => org.http4s.Response needs to be available somewhere (or maybe just applied).

I'm going to start with trying to implement the path parameter because I think that's probably the most complicated.

[L-Lavigne]

Thanks Noel! Naming the annotation @http4s seems a bit unusual but I can see it justified as that shows the annotation to be a pass-through to that specific library.

Just as a minor improvement I suggest making the annotation's queryParams a vararg of Strings to reduce some clutter in user declarations.

Error mapping does require special care, it's been an issue for me when first trying to generalize services that are exposed to both RPC and REST. Ideally we don't want to rely on user implementations raising errors with a specific transport in mind, so allowing custom mappings here makes sense.

[noelmarkham]

Thanks for that - happy to change anything re the naming, these were just hypotheticals anyway. And yeah, I agree re the error mapping. I think this will all become a bit easier to understand as it is implemented