fix(transport): Make Server::layer() support more than one layer

[Arcterus]

The current behavior is to replace the previous layer with the newly specified layer. The new behavior is to chain the layers together. The current behavior might actually be intentional, but IMO it's not very intuitive given how services are added by chaining method calls.

I can adjust the docs and/or tests as necessary, so let me know if you want me to do so.

Motivation

We were using Server::layer() and assumed it supported more than one layer (without using Stack from tower) given the example in the docs (which has an interceptor for authentication that, with the current behavior, gets replaced with effectively an identity interceptor afaict).

Solution

This change basically just uses Stack internally rather than requiring users to use it themselves manually.

[davidpdrsn]

I've actually been thinking that we should consider changing Server::layer so it can be called after services have been added, and applies the middleware to the services already added. This is how axum does it and I personally find it easier to understand. Adding the middleware first, then the services, seems a bit backwards to me.

In general we're not really making much use of the fact that tonic's uses axum internally for its routing. We could add more flexible APIs such as merging routers. That is probably outside the scope of this PR but thought I would mention it.

I think this changes makes sense on its own though.

[Arcterus]

I haven't used axum much, so ¯\_(ツ)_/¯.

When you say the middleware applies to the services you specified previously, does that mean you can layer the middleware so some only apply to a subset of services? Like, you could add more services after adding a middleware and those new services would not be affected by the middleware? If so, I'd definitely prefer that approach. We currently have an ad hoc mechanism to do something like this, which I'd prefer to remove if possible.

[LucioFranco]

It should be possible to achieve this right now by just passing in your own service builder correct? I honestly want to just nix the entire server code and just use axum at this point 😆

[davidpdrsn]

It should be possible to achieve this right now by just passing in your own service builder correct?

Yeah this would just be a convenience.

I honestly want to just nix the entire server code and just use axum at this point 😆

👌

[LucioFranco]

LGTM just one question

[LucioFranco]

I feel like we shouldn't have to implement this manually, is there something missing in tower?

[Arcterus]

Apparently Default is only implemented for ServiceBuilder<Identity>, so the derive fails.

[LucioFranco]

Ah okay huh I guess derive doesn't take into account default bounds