Alternate proposal

[alexsnaps]

Since @eguzki was very much against the idea of splitting the responsibility of sending the message from the actual Action underneath all of the layers in between, I came up with something that I thing would let you do that, while composing the state rather than folding it all together in a ball of mud.

proposal_context::Filter only responsibility is to hold on to the state relative to each phase of the request being handled:

• on_http_request_headers: where a possible start of a "flow" happens, otherwise Status::Continue;
• on_grpc_call_response: where an Operation is pending a response from a (single here, but it could expose the token associated with all of the) request(s) it's made;
• on_http_response_headers: where possible additional headers added by some service need to be added

Most of the coordination now happens in handle_operation which will either associate the Operation with the proper field, or let the request die, i.e. deny it...

"FallibleOperations", tho not an explicit type, do expose a .fail() -> Operation method. So to handle a GrpcMessage sending failure for instance.

Code in the no_implicit_dep module, take an explicit &mut T: HttpContext in, which in production would be the Filter itself, while in testing can be a mock of it, so no implicit dependency, mostly funtion calls, to the proxy wasm sdk exists. They are all explicit... Coupling to the proxy wasm API remains, but not to the "runtime".

This comes with the downside there is now a ever so thin dependency on the proxy wasm sdk types in our implementation. It also will need to be passed along all the way down thru Core layers to the individual Auth_Module & RateLimiting_Module from @eguzki 's design, but still makes this all cleaner than what we have today...

Anyways, throwing this out there... This is very obviously not to be merged ever. At best, if someone thinks this can be a useful basis for further improvement, possibly this single commit could be reused. But I was mostly trying to highlight different options...

[alexsnaps]

wrt achieving "a better encapsulation (bringing separation of concerns)", encapsulation is a mean to good cohesion... this, does achieve somewhat of the former, but keeping things coupled together nonetheless. It's is obvious already with this example, but it'll get even pervasive when implementing the code below that layer and how &mut ctx will be passed through all layers to eventually reach the Modules that need to do gRPC calls... Arguably that's all of them right now (even if conditionally) so maybe that's not necessarily bad... yet leaking the fact that wait for that gRPC response all the way back up. This two facts (API & conceptual coupling) clearly indicates high coupling. But to counter the point... this is fairly small code base (and can get slimmer still), so it isn't necessary surprising. And too abstractions aren't good neither.

Anyways... You all know my perspective. Tried to make myself a little useful by exploring a possible middle ground.

[alexsnaps]

One last thought, probably mostly interesting to @adam-cattermole, I would possibly extract a Request struct that would act as a memoizing wrapper so to avoid looking the same properties up over and over again, e.g. have request.time only resolved once from the host for instance. That probably would either be passed around alongside the ctx or be part of the context itself... either coupling it to the Filter impl. (i.e. override the default impl of these methods) or having a dedicated accessor to it, but then specializing <T: HttpContext> to something else on the genericized fns.