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

Contextual data for proxied request #71

Open
tsegismont opened this issue Mar 15, 2024 · 6 comments · May be fixed by ramanadas/vertx-http-proxy#2
Open

Contextual data for proxied request #71

tsegismont opened this issue Mar 15, 2024 · 6 comments · May be fixed by ramanadas/vertx-http-proxy#2
Labels
enhancement New feature or request gsoc2024

Comments

@tsegismont
Copy link
Contributor

tsegismont commented Mar 15, 2024

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
Copy link

Might be related #50

@tsegismont
Copy link
Contributor Author

Thanks for the heads-up @fbuetler

@wzy1935
Copy link
Contributor

wzy1935 commented Jul 31, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

wzy1935 commented Aug 6, 2024

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.

(I think I'm running out of ideas)

@tsegismont
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc2024
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants