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

Finch in Action #204

Closed
vkostyukov opened this issue Feb 23, 2015 · 23 comments
Closed

Finch in Action #204

vkostyukov opened this issue Feb 23, 2015 · 23 comments

Comments

@vkostyukov
Copy link
Collaborator

This is a big ticket that related to #190, #172 and PR #184. For now we have amazing and highly-composable basic blocks but it's still not really clear how to use them together. One and simple example is shown in the demo project, although I was wondering if we could do better.

There is nothing wrong with using Finch in a straightforward way (via Service[HttpRequest, HttpResponse]). But we also should think about using it in a more convenient manner. The PR #184 is a good example of this effort. My hight-level idea is to provide users a way to write their microservices as functions:

def getUser(userId: Long): Future[User] = ???
def getUsers(fromId: Int, toId: Int): Future[Seq[User]] = ???

and then use Finch basic blocks to serve those functions on Finagle. It might look as follows:

val e1: Endpoint[User] = Get / "users" / int /> getUser
val e2: Endpoint[Seq[User]] Get / "users" /> { fetchFromAndTo map getUsers }

We don't really need to convert request reader into service (per #184) or create an explicit service instance. We do have everything is needed in RequestReader so we can just map it:

val pagination: RequestReader[Int ~ Int] = ???
val users: RequestReader[Seq[User]] = pagination map { case from ~ to => getUsers(from, to) }

Thus, we may define an Endpoint as type Endpoint[+A] = Router[RequestReader[A]]. Therefore, we're replacing Service with RequestReader, which sounds reasonable for me:

  • They look similar: Req => Future[A]
  • RequestReaders are composable as hell (we did a great job), Services are not (! is an ugly hack).
@vkostyukov
Copy link
Collaborator Author

Would be nice to hear others opinion on this direction. /cc @BenWhitehead, @jenshalm, @rpless, @rodrigopr, @benjumanji.

@benjumanji
Copy link
Contributor

This seems like a nice idea. I was just writing some code (not yet finch'd) that basically involved a bunch of filters to turn Service[Request, Respose] to Service[C,D]. This kind of thing would make that pretty pleasant, so once we have an impl, I have some code I can test this on.

@vkostyukov
Copy link
Collaborator Author

Although, it's not clear yet how to fetch the details from a custom request type using this approach.

case class MyReq(http: HttpRequest, currentUserId: Long)

@vkostyukov
Copy link
Collaborator Author

I've came up with an interesting idea. The answer is in an a cornerstone abstraction that should glue all the basic blocks together. This approach doesn't involve changes of the current abstractions but adds new one that wires everything together.

I'm thinking about adding io.finch.Microservice that looks as follows:

trait Microservice1[-A, +B] {
  def apply(a: A): Future[B]
}
...
trait Microservice5[-A, -B, -C, -D, -E, +F] {
  def apply(a: A, b: B, c: C, d: D, e: E): Future[F]
}

type Microservice[-Req, +Rep] = Microservice1[Req, Rep]

Microservice is a lightweight and domain-specific version of a Finagle Service. It might be considered as a hight-level Service. Inside Finch application, everything might be done with microservices rather than with services. Finch will take care about converting microservices into services and serve them with finagle-http.

Microservices are composable with both request readers and response builders. Request readers change its input type to Req while response builders change its output type to HttpResponse. The API is inspired by functions composition:

val r: RequestReader[A ~ B ~ C] = ???
val m1: Microservice[A, B, C, D] = ???
val m2: Microservice[HttpRequest, D] = m1 compose r
val m3: Microservice[HttpRequest, HttpResponse] = m2 andThen Ok

Thus, the data-flow is something like this (rr - request reader, m - microservice, rb - response builder): rr1 -> .. rrN -> m -> rb1 -> .. -> rbN.

It's obvious, that microservcies are composable with both simple functions and other microservices in a familiar way: andThen and compose.

trait Microservice1[-A, +B] {
  def apply(a: A): Future[B] = ???

  def compose(f: A => C): Microservice[C, B] = ???
  def andThen(f: B => C): Microservice[A, C] = ???

  def compose[C](before: Microservice[C, A]): Microservice[C, B] = ???
  def andThen[C](after: Microservice[B, C]): Microservice[A, C] = ???
}

Finally, the problem with custom request type might be easy resolved with a version of a compose function that takes function Req => RequestReader.

trait Microservice1[-A, +B] { self
  def apply(a: A): Future[B]
  def compose[Req](f: Req => RequestReader[A])(implicit ev: Req => HttpRequest) = 
    new Microservice1[Req, B] {
      def apply(req: Req) = f(req)(req) flatMap self
    }
}

case class MyReq(http: HttpRequest, currentUserId: Int)
val getUserName: Microservice[Int, String] = ???

val m1: Microservice[MyReq, Int] = m.compose { req: MyReq => RequestReader.value(req.currentUserId) }
val m2: Microservice[MyReq, HttpResponse] = m andThen Ok

We've seen before how microservices are glued request readers and response builders together. The last thing we have to think about is routers. We might redefine an endpoint in term of microservices to allow them be implicitly converted into Finagle services.

type Endpoint[-A, +B] = Router[Microservcie[A, B]]
implicit def[A, B](e: Endpoint[A, B]): Service[A, B] = ???

Would love to hear any suggestions and comments. What I like more about this approach is that it doesnt't break anything (except for Endpoint). So, we can safely introduce it in the next version.

@BenWhitehead
Copy link
Contributor

I'll try and spend some more time digging into this tomorrow, but I'm a bit worried about Endpoint breaking. I realize finch is evolving to try and offer more expressive ways of doing things that what endpoint provides but I have several services that were built on finagle 0.1 and it's becoming more involved to update to newer versions.

There's also the matter that I think is mentioned in #190 Composing routers with Finagle filters/services. The services that I have written are using Finagle Filters to decorate the incoming request and perform authorization before passing the authorized request onto an endpoint (ultimately made up of other endpoints).

I like that the request readers have become more friendly to error messaging and the expressiveness has improved, but losing finagle filters is a deal breaker in my current use cases.

I also don't know if anything has been done to allow for the pattern matching over combinators mentioned in #147 (comment)

@vkostyukov
Copy link
Collaborator Author

Thanks for the feedback Ben!

Pattern matching over combinators is on this milestone #149.

This approach doesn't prevent us from using filters. You can always implicitly convert Microservice to Service and apply filter to it. Although, we may come up with a new name for endpoint and do not break the current API. So, you can keep using Services and old-style endpoints (or even new route combinators API).

@BenWhitehead
Copy link
Contributor

Thanks for the link to #149.

If we come up with a new name that makes sense it might be good to use that so that we can distance ourselves from the old name that is going to be deprecated.

I'll try and make sure to carve out some time to see what it will take to update one of my projects (unfortunately I can't make any guarantees as the next few weeks are going to be very busy for me).

@jenshalm
Copy link
Contributor

I've only just seen it and I haven't put too much thought into it yet. But my initial gut feeling is that I'm not too fond of the Microservice type. One reason is consistency. For Router and RequestReader we introduced combinator types (A ~ B and A | B respectively), in the case of the RequestReader mainly to avoid a lot of plumbing in our impl (we could have made it more convenient without the combinator type). Now with the Microservice we suddenly go the Xx1, Xx2, Xx3 route, which is something I usually find fairly ugly in Scala. And I'm not sure I prefer Micoservice3[A, B, C] over Service[A ~ B, C].

I don't have any brilliant alternative idea yet, but I'd love to first think about what is really missing in the existing types for optimal composability. The main reason why I've put #184 on hold was that I did not know how to combine a custom request type with the result of a reader. If I have a filter/service that does Auth and then a reader, how can I have the "real" service deal with a domain object that contains the userId and the result from the reader? I'm not sure the Microservice API solves this (I don't fully understand the corresponding example as it produces a RequestReader, not a Future). It might be that an additional combinator might already help:

(auth ~ reader) ! service ! TurnIntoHttp

So, in short, I'd prefer to first define the gaps in composability that RequestReader, Router, Service and Filter leave and try to think about filling these gaps with something of lighter weight than MicroserviceN.

@jenshalm
Copy link
Contributor

Ok, I thought about it a bit more and I'm afraid I would really recommend against introducing the Microservice trait. Apart from all the things I already wrote above, these additional concerns come to mind:

  1. Speed of API evolution. We just introduced a lot of new APIs in 0.5.0. Normally it would be best to first let users try these out and report back and get a better feeling about all the use cases. This would allow us to really clearly define what problem we are trying to solve. With this proposal it is not 100% clear to me.

  2. Complexity. I find introducing a Microservice on top of RequestReader, Router, Service and Filter starts to blur the semantics and makes everything start to feel a bit bloated.

  3. There might be neater alternatives for making working with the combinator types more convenient. Just out of curiosity, I just tried the following:

case class ~[A,B](a: A, b: B)

implicit class Foo2[A,B] (in: A ~ B) {
  def ! = println("2")
}
implicit class Foo3[A,B,C] (in: A ~ B ~ C) {
  def ! = println("3")
}
implicit class Foo4[A,B,C,D] (in: A ~ B ~ C ~ D) {
  def ! = println("4")
}

val x: String ~ Int ~ String = new ~(new ~("foo", 2), "bar")

x !                                            //> 3

so some of the problems could potentially be solved with implicits without introducing a new public API.

@vkostyukov
Copy link
Collaborator Author

We don't have to solve this ticket in this milestone but we definitely must do it before 1.0.0. I wish we could solve this w/o introducing new API. @jenshalm could you please elaborate how the ! is going to help us?

In my understanding the problem we're trying to solve is to allow users write domain-specific services rather than HttpRequest => HttpResponse. We can define a mock problem here to use it as an a counter-example.

Let's say, we want to define a service that answers how many photos user has in the given album. The endpoint looks like GET /user/albums/:id/photos.count?format=json. Three parameters are passed one should be returned: Int ~ String ~ String => Int. userId: Int is available via custom request, album: String is available via URI and format: String is a query-string param. We might want to define the following Service:

val getNumberOfPhotos = new Service[Int ~ String ~ String, Int] {
 ...
}

But in this case we will have to use pattern-matching to extract input arguments. We can avoid this defining a simple factory method Microservice.

object Microservice {
  def apply[A, B, C, D](f: (A, B, C) => Future[D]) = new Service[A ~ B ~ C, D] {
    def apply(req: A ~ B ~ C): Future[D] = req match {
      case a ~ b ~ c => f(a, b, c)
    }
  }
}

And then use is like this:

val getNumberOfPhotos = Microservice { userId: Int, album: String, format: String =>
 ...
}

@jenshalm
Copy link
Contributor

The ! in my example was only a demonstration that implicits can distinguish between A ~ B ~ C and A ~ B, so that we could potentially use implicit conversions, e.g. to take a RequestReader[A ~ B] and chain a plain (A, B) => C to it and promote everything to a full service as a final step. I just wasn't sure the implicits would work, as A ~ B ~ C is also an A ~ B, but the former is probably seen as more specific when resolving implicits.

Ok, I'm clearer now about the goals. I'm still a bit confused about where the custom request types came from. Are we sure they are the right pattern? They seem to complicate things a bit in that they A) prevent implicit conversion of a reader to a service B) get lost when applied to a reader anyway. So there is always the extra challenge of passing the extra info from the custom request through. That's why I thought it might be more convenient to combine all parts that need a Request instead of chaining them, e.g. authenticate ~ params producing User ~ MyInput. Which could then be chained to any (User, MyInput) => MyOutput without pattern matching.

Note that I don't have the time to apply any sort of deeper thinking to it before the weekend, so some things I write might not make much sense. I just want to try to find the simplest solution (which is in line with finch philosophy), and the Microservice trait did not feel simple enough.

The Micoservice object looks better (although some scenarios might still benefit from additional implicits). The name feels slightly unfortunate, but a Service companion object already exists in Finagle, so might be the only choice here.

@jenshalm
Copy link
Contributor

Maybe we can build a mini-demo app for just the sample scenario you mentioned? Having a custom request carrying user info (coming from a filter?), a router extracting one param, a request reader extracting one param, a service function that only knows domain instances and a json encoder for the result. All wired with the existing 0.5.0 API as a fully working demo app and as a starting point to discuss improvements?

@vkostyukov
Copy link
Collaborator Author

I finally got what you mean! Instead of changing the request type with auth filter we apply a request reader auth that fetches the current user id. Like this:

val auth: RequestReader[Int] = RequiredHeader("X-User-Id").map(_.toInt)

So we can solve our counter-example as follows (~> is an alias for embedFlatMap):

def getNumberOfPhotos(album: String)(userId: Int, format: String): Future[Int] = ???

val route: Router[RequestReader[Int]] = Get / "user" / "albums" / string /> { album =>
  auth ~ RequiredParam("format") ~> getNumberOfPhotos(album)
}

Yes. We can start with simple demo to have a sandbox for experiments.

Although, I like the idea of using RR instead filters for authorization. I don't think that changing request type is a good pattern. Finagle-httpx doesn't even provide API for extending request (that's why we have implicit ev: Req => HttpRequest everywhere). Finagle is moving to immutable HTTP types (req might be a case class). Neither composition nor inheritance is a good practice in this case but request reader that transforms immutable request to a reasonable domain-specific objects sounds like a solid approach.

@jenshalm
Copy link
Contributor

Yes, that's kind of what I meant. The only downside I see is that the auth reader has to be chained explicitly for each route. I guess there is no easy way to say authFilter ~ router and then magically get access to the result of the filter inside all the individual routes. But in general it would be beneficial if the RR could be reduced back to extending HttpRequest => Future[A], it would make the RR more easily composable. As long as we don't make other scenarios more cumbersome.

Where do you want to create the demo app/playground? It might be good to have the original version here in this repo so that users could create PRs to demonstrate different approaches for improved APIs.

@vkostyukov
Copy link
Collaborator Author

Yeah. I will create the single-file mini-demo project here as a sub-project. Probably it make sense to name it a quickstart or bootstrap.

@vkostyukov
Copy link
Collaborator Author

Playground is on review #205. My plan for this weekend is to play with it and come up with PR for the Finch in Action problem.

@vkostyukov
Copy link
Collaborator Author

I've put a gist that describes the approach suggested in #206. Please, have a look.

@vkostyukov
Copy link
Collaborator Author

I want to merge #206 PR as an experimental feature of 0.6.0 release. The gist that describes experimental features is here: https://gist.github.com/vkostyukov/e0e952c28b87563b2383. Please let me know if you have any concerns.

@sergeykolbasov
Copy link
Collaborator

I'm really excited about the job you did there guys!

We've started using finch in our project and made few abstractions over Service[HttpRequest, HttpResponse].

At first I wrote some implicit conversions from Function2[A, B, R] to PartialFunction[A/B, R] so now we are able to map simple functions like def someResponse(a: String, b: Int) with />.

Then we've came with an idea to hide Service[HttpRequest, HttpResponse] implementation since content of someResponse in 95% of cases is nothing more but Service.apply. We've called dat abstraction action (just like Play Framework) and it's simple as fck:

def apply(f: (RequestContext)=>FResult): Service[HttpRequest, HttpResponse] = {
    ... service implementation ...
  }

Now we're able to do whatever we want with HttpRequest (lazy auth, json decode from payload, wrap it to our own RequestContext, etc) before passing it to someResponse.

I've just read your gist about microservices and it reminds me my attempts to do the same thing - skip controller "proxy" functions and use straight service's methods. I tried to make an action builder around passed service method but droped this idea because of another tasks.

Hope you will do it!

@vkostyukov
Copy link
Collaborator Author

Thanks @imliar! That is super cool what are you doing with Finch. Hopefully, something similar will be shipped in 0.6.0 release. BTW, feel free to add your company into the adopters list.

What you described first two paragraphs: RequestContext and implicit conversion to / type is exactly what has been implemented in #206. The PR is on review, a couple of things should be finished but the general idea is similar to what you was trying to do: hide meaningless HttpRequest/HttpResponse types and substitue them with domain types. See mentioned PR for the details.

@sergeykolbasov
Copy link
Collaborator

@vkostyukov

There are still some important cases to think about.
For example: what about custom error handling? How route should react to different failures in Future[A] and where should I handle em and describe failure behaviour.

@vkostyukov
Copy link
Collaborator Author

The typical use case is have a top-level filter that handles all the possible errors from the stack. All the different failures in Future[A] is nothing more than just Future[Nothing]. See an example here.

The scenario is following: you write your micro-services that respond some Future[A] or Future.exception. So, you can wrap your services with a filter that handles all the errors in a one place.

@vkostyukov
Copy link
Collaborator Author

This is solved in #206.

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

No branches or pull requests

5 participants