eclipse-vertx / vertx-http-proxy

vertx http proxy
Eclipse Public License 2.0
55 stars 36 forks source link

Contextual data for proxied request #71

Open tsegismont opened 8 months ago

tsegismont commented 8 months ago

We should have the ability to provide some contextual data when handing over a request to the proxy.

This data should be accessible to all the different proxy components (e.g. origin selector, interceptors).

Possibly related to #35 and #44

fbuetler commented 5 months ago

Might be related https://github.com/eclipse-vertx/vertx-http-proxy/issues/50

tsegismont commented 5 months ago

Thanks for the heads-up @fbuetler

wzy1935 commented 3 months ago

I pushed an example implementation #96, using a wrapper (indeed I think HttpServerRequestWrapper is the most decent approach) so that context can bind to each request.

Usage example:

// Before
vertx.createHttpServer().requestHandler(proxy);

// After
vertx.createHttpServer().requestHandler(request -> {
  ContextedHttpServerRequest contextedRequest = ContextedHttpServerRequest.from(request);
  contextedRequest.set("key", "value");
  proxy.handle(contextedRequest);
});

What do you think? @tsegismont

tsegismont commented 3 months ago

Thanks for your proposal @wzy1935

I'm not sure we should pursue in this direction. We already have a mechanism for extended HTTP request in Vert.x Web. I don't think it's a good thing to do the same in Vert.x core (which doesn't have the need for this).

wzy1935 commented 3 months ago

I think the additional context data must be passed through the HttpProxy#handle(HttpServerRequest request) method (since we want it to be per request), but I also think that HttpProxy should still extends Handler<HttpServerRequest>, so It's kinda troublesome. Can we based on this one https://github.com/eclipse-vertx/vertx-http-proxy/pull/96/commits/f95ac839bb776209e26c64cda33701d482652b9d ? It will also allow user to create/pass their own context instance.

(I think I'm running out of ideas)

tsegismont commented 3 months ago

I think the additional context data must be passed through the HttpProxy#handle(HttpServerRequest request) method (since we want it to be per request), but I also think that HttpProxy should still extends Handler<HttpServerRequest>, so It's kinda troublesome. Can we based on this one f95ac83 ? It will also allow user to create/pass their own context instance.

This looks to be going in the right direction, in my opinion. We can still have HttpProxy extending Handler<HttpServerRequest>. By default, the contextual data map will be empty.

And then, like in your prototype, the HttpProxy can have an overloaded method:

  /**
   * Handle the <i><b>outbound</b></i> {@code HttpServerRequest}.
   *
   * @param request the outbound {@code HttpServerRequest}
   */
  void handle(HttpServerRequest request, Map<String, Object> attachments);

We can use the existing concept of attachments to the ProxyContext.

Next step is to overoload (and maybe deprecate) the originSelector and originRequestProvider methods so that the user can retrieve an attachment when the callback is invoked.