Service chaining

[raelg]

Hi,

I think the following method is missing from the Service class, which will enable straight-forward chained service calls.

 def andThen[Rep2](service: Service[Rep, Rep2]) : Service[Req, Rep2] = {
    new Service[Req, Rep2] {
      override def apply(request: Req): Future[Rep2] = {
        Service.this.apply(request).flatMap{ rep =>
          service.apply(rep)
        }
      }
    }
  }

e.g, this will enable the following service hierarchy:

val importantService: Service[Request, JsonModel] = {...} // invokes the service and returns a JsonModel 
val jsonService: Service[JsonModel, Response] = {...} // converts the JsonModel to a valid response
val myService: Service[Request, Response] = importantService andThen jsonService 

This simplifies the responsibility and testability of each service, while enabling reuse.

This would also enable a Service to be positioned before a Filter, as in the example below:

e.g,

val importantService: Service[Request, [Option[JsonModel]] = {...} // invokes the service and returns a optional JsonModel 
val optionalMapperFilter: Filter[Option[JsonModel], Response, JsonModel, Response]  = {..} // unrwap the option and returns 404 if None, or invoke the service with the value otherwise 
val jsonService: Service[JsonModel, Response] = {...} // convert the JsonModel to a valid response
val myService: Service[Request, Response] = importantService andThen (optionalMapperFilter andThen jsonService)

[luciferous]

I'm not saying this is a bad idea, but in your example, could a plain Future.flatMap work?

def jsonService(model: JsonModel): Response = { ... }
val myService = importantService.flatMap(jsonService)

On Jun 15, 2015 1:47 AM, "raelg" notifications@github.com wrote:

Hi,

I think the following method is missing from the Service class, which will enable straight-forward chained service calls.

def andThen[Rep2](service: Service[Rep, Rep2]) : Service[Req, Rep2] = { new Service[Req, Rep2] { override def apply(request: Req): Future[Rep2] = { Service.this.apply(request).flatMap{ rep => service.apply(rep) } } } }

e.g, this will enable the following service hierarchy:

val importantService: Service[Request, JsonModel] = {...} // invokes the service and returns a JsonModel val jsonService: Service[JsonModel, Response] = {...} // converts the JsonModel to a valid responseval myService: Service[Request, Response] = importantService andThen jsonService

This simplifies the responsibility and testability of each service, while enabling reuse.

This would also enable a Service to be positioned before a Filter, as in the example below:

e.g,

val importantService: Service[Request, [Option[JsonModel]] = {...} // invokes the service and returns a optional JsonModel val optionalMapperFilter: Filter[Option[JsonModel], Response, JsonModel, Response] = {..} // unrwap the option and returns 404 if None, or invoke the service with the value otherwise val jsonService: Service[JsonModel, Response] = {...} // convert the JsonModel to a valid responseval myService: Service[Request, Response] = importantService andThen (optionalMapperFilter andThen jsonService)

— Reply to this email directly or view it on GitHub https://github.com/twitter/finagle/issues/385.

[mosesn]

Historically we've often used Filters for solving this kind of problem. NB: this Service acts like a special kind of Filter, which only mutates input, not output, like:

trait ServiceFilter[Req, SvcRep, FilterRep](first: Service[Req, Rep]) extends Filter[Req, FilterRep, SvcRep, FilterRep] {
  def apply(req: Req, second: Service[SvcRep, FilterRep]): Future[FilterRep] =
    first(req).flatMap(second)
}

The one difference is that we need to early-bind FilterRep. Most folks don't have that many clients with different types (thrift services which talk to an enormous number of endpoints / other services being a notable exception), and we could provide an API which works for most use cases via:

class Service[Req, Rep] {
  ...
  def toFilter[Out]: Filter[Req, Out, Rep, Out] = new ServiceFilter[Req, Out, Rep, Out](this)
}

I think keeping Filters and flatMap as the base tools of composition is powerful, and I'm hesitant to allow new combinators too quickly.

[raelg]

flatMap itself doesn't work, because the method is not defined on the Service trait. I initially used the Filter approach above, but decided that subclassing Service with a new trait with the andThen method is the cleanest, and most composable approach. I'm building a microservice with a handful of RESTful JSON end-points, and think this approach fits in nicely with Finagle's functional design.

[mosesn]

Could you elaborate on what you don't like about the Filter approach?

[raelg]

Well, I guess for one, it's slightly less code with my the new service subclass, and personally I think it's more readable.

[vkostyukov]

@raelg what you can also do is to use the "pimp my library" pattern to wrap the andThen function with implicit class like this.

[luciferous]

FWIW I agree with @raelg, but I only have abstract reasons for why I want it. @raelg may have a concrete use case, which might be nice to know about, and why he thinks Filters are not ideal.

My reasons:

1. We already have map[Req1](f: Req1 => Req): Service[Req1, Rep] which adapts the input type of the Service. It maintains symmetry if we have some way of also mapping the output type.
2. Four type parameters means Filters can be quite complicated.
3. Mapping just the output shrinks the scope of effects, which might be nice, e.g. we know that a composition of Service.andThen(b => c) will never retry the Service dispatch; the closure doesn't have access to it. We don't have this guarantee with Filter.

Oh, one more thing. The ability to compose to the right (i.e. adapt the output type) completes the Profunctor instance for Service.

[mosesn]

@raelg I'm a little hesitant to do add just anything for the sake of small improvement in concision. I feel like a lot of the power of the Service abstraction is how simple it is, and increasing the surface area means that finagle ends up even more difficult to learn, and Service becomes more difficult to reason about.

I'm not sure I buy the readability argument–since Filters are the base unit of composition, it seems to me like it's more readable.

@luciferous I'm having trouble getting behind 1. because it means map in a totally different way than map normally works, but I can buy it if I just call it prologue or something different in my head.

1. is true, this is why we normally use SimpleFilter.
2. is also a good point, especially if we end up ever wanting to structure a filter / service chain as a DAG that gets compiled later

OK, I'm slowly getting sold, but can we have a few other folks weigh in? I'm curious what @roanta and @mariusae think, since they have the most context.

[mariusae]

This can easily be done through an external combinator, and even be combined with syntax enrichment. I'm not convinced it's useful enough to warrant inclusion in the standard Service trait.

[luciferous]

@mosesn I just spoke with Travis, and I think maybe I should clarify a little what I'm looking for, then people can critique it as they feel is right.

So we have a map already in Service, and this corresponds to a contravariant map which adapts the Service's request type. This map is poorly named, imo, but maybe it made sense at one time. It's misleading for those familiar with map expecting it to adapt the output (response) type.

trait Service {
  // This is the contravariant map, which is already defined in Service.
  def map[Req1](f: Req1 => Req): Service[Req1, Rep]

  // This is the covariant map, which we don't have.
  def rmap[Rep1](f: Rep => Rep1): Service[Req, Rep1]
}

@mosesn I wasn't very clear before on the map I was talking about – I'm talking about rmap, which works exactly like how every other map works, no?

@mariusae yeah, I don't know how useful it is either, maybe @raelg can compel with his use case.

@travisbrown What do you think of this?:

  def andThen[Rep1](f: Rep => Future[Rep1]): Service[Req, Rep1]

[vkostyukov]

@luciferous I'm not sure we would be able to add andThen[Rep1](f: Req => Future[Rep1]) since service already extends Req => Future[Rep], which provides andThen[B](f: Future[Rep] => B). Both functions take the same argument type: Function1[A, B], so we can't override it due to type erasure.

Perhaps, I'm missing something. Correct me if I'm wrong.

Isn't contrvariant map called comap?

[luciferous]

@vkostyukov right we'd call it something else

[travisbrown]

@luciferous I'm not too worried about the input and output mapping functions—if someone really wants these, they're probably open to something like a Cats or Scalaz dependency, and providing a Profunctor[Service] instance will give them lmap and rmap. Adding these to Service would just make things more complicated.

It would be nice to have a clean way to compose two services—we've wanted this in a few places in Finch, for example. I'd personally like to see compose and andThen with reasonable signatures on Service:

def andThen[Rep1](f: Service[Rep, Rep1]): Service[Req, Rep1]
def compose[Req1](f: Service[Req1, Req]): Service[Req1, Rep]

It's not possible to add these via Cats's Compose (at least at the moment), since the syntax for Compose uses the same names, and the compiler won't look past the methods that are actually on Service.

I don't feel terribly strongly about this, though. We could easily add aliases in ComposeOps that wouldn't collide with the methods on Service. Leaving the Service API as it is and letting people who want improvements make them via type classes and enrichment seems reasonable to me.

[vkostyukov]

:+1: for andThen and compose. I would vote for adding these functions into the Service trait (as well as deprecating map).

[mosesn]

We maybe shouldn't use andThen or compose because Service is-a Function, and the overloading with something semantically different could get confusing. Getting back to the point, is this useful enough to merit inclusion in Service specifically?

[raelg]

Having used Finagle for a couple of weeks, I'm only familiar with it on the surface level, and it may well be possible my services are too granular and transformation would be more natural in a different layer, especially as each of my routes essentially uses it's own filter chain.

Using a Filter to unwrap and short circuit Options does seem like appropriate usage of Filter though (and is easily testable). However, the reason I wouldn't use a Filter to accomplish changing services is since when reading the code, it wouldn't be obvious what the extra Filter did without looking at the implementation, as opposed to the Service#andThen approach. I'm not sure I agree with the comment above, as the usage is similar to Scala's Function1#andThen: Composes two instances of Function1 in a new Function1, with this function applied first

[travisbrown]

@raelg The problem is that since Service extends Function1, there are two different ways to think about composition. Viewing Service[I, O] as just a function I => Future[O], you can compose it with other functions that return an I or take a Future[O]. Viewing it as a service, you want to be able to compose Service[A, B] and Service[B, C].

The current andThen and compose methods support the first kind of composition, and therefore aren't very useful (to my way of thinking, at least), but they come from Function1, so they can't be deprecated away. We could overload these method names to provide the more meaningful kind of composition as well, but as @mosesn says, that would be kind of confusing.

[raelg]

Point taken. It could well be that this is specific to my use-case and anyway adding the implementation has been straight forward enough. I would agree that the current andThen and compose methods confused my initial understanding and approach.

[Gabriella439]

For what it's worth, I'm in favor of this since it's analogous to Haskell's (>=>) operator (specialized to Futures):

(>=>) :: (a -> Future b) -> (b -> Future c) -> (a -> Future c)

Unlike flatMap, this operator has the nice property that it is "compositional". Informally, "compositional" means that you get back what you put in: two "Kleisli arrows" go in, one "Kleisli arrow" comes out. Formally, "compositional" means that it is the composition operator in a category (specifically the "Kleisli category" for the Future monad).

flatMap is close to being compositional, but not quite. The issue is that flatMap requires dealing with two separate types: values of shape Future[B] and values of shape A -> Future[B]. With (>=>) you only have to think in terms of values of type A -> Future[B].

This might seem like a minor distinction but from a user's standpoint it means that they don't need to think in terms of Future at all. They can think purely in terms of Services if they use the (>=>) operator exclusively. All you would need is a function that promotes a Future to a service with a () input:

type Service a b = a -> Future b

fromFuture :: Future b -> Service () b

[travisbrown]

For what it's worth I started a little project a couple of months ago that provides category and profunctor instances (from cats) for Service, and last night I implemented an idea that I've been wanting to try for a while that makes it possible to use enrichment methods even if they collide with methods on the original type. This means you get lmap and rmap for the price of an implicit class wrapping, with compose and andThen (for services, not functions) only adding a small extra syntactic cost.

All these things are pretty straightforwardly available to people who really want to use them, which is probably an argument for not meddling with Service.

[dschobel]

Consensus seems to be that no one feels strongly that this belongs in Service, if no one objects I'll close this.