vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 535 forks source link

Ability to supply a different GraphQL java based upon the request context. #2680

Closed NavidMitchell closed 1 day ago

NavidMitchell commented 1 week ago

Motivation:

I need to use a different GraphQL java object to process each request. This is because I currently am providing a different GraphQL schema based on a path parameter that is configured for my handler.

Ex:

        GraphQLProvider graphQlProvider = new GraphQlProvider();
        router.post(properties.getGraphqlPath()+":"+NAMESPACE_PATH_PARAMETER)
              .consumes("application/json")
              .consumes("application/graphql")
              .produces("application/json")
              .handler(BodyHandler.create(false)
                                  .setBodyLimit(properties.getMaxHttpBodySize()))
              .handler(new GqlHandler(graphQlProvider));

The design I used in the commits is probably not the cleanest. Maybe the GraphQL Provider Function should be added with a fluent method like the queryContext ext. I am not sure what makes the most sense, since the GraphQL object is currently a constructor parameter. I can change the code to whatever the Vertx team wants. But I wanted to make sure there was interest in supporting this before spending too much time.

tsegismont commented 3 days ago

How many different schemas do you need to serve? Are they dynamically loaded?

It there are only a few and statically configured, it seems configuring different routes would do the job.

NavidMitchell commented 3 days ago

Hello @tsegismont,

The routes are dynamic and not known beforehand. This is for our opensource software here. https://github.com/Kinotic-Foundation/structures For vertx 3.9.16 I just copied the GraphQL handler and made the modifications I needed. I would like to avoid that going forward so I get bug fixes and enhancements.

tsegismont commented 2 days ago

I'm reluctant to do this because, to be consistent, we would also have to change the GraphQLWSHandler, and all options or fields in both handlers that are not already dependent on the the routing context.

Since routes can be added/removed on the fly, how about dynamically registering a handler with the exact route path?

NavidMitchell commented 2 days ago

Hello @tsegismont , Thank you for looking into and thinking about this. What I can do instead is create a Delegating RoutingContext Handler that delegates to the correct Vertx GraphQLHandler per namespace. I need to manage dynamic async creation and tear down of the GraphQL java class but I can create the GraphQLHandler as well then delegate accordingly.

Sorry to bug you about this. If would have thought of this in the first place I wouldn't have needed to.

tsegismont commented 1 day ago

Hello @tsegismont , Thank you for looking into and thinking about this. What I can do instead is create a Delegating RoutingContext Handler that delegates to the correct Vertx GraphQLHandler per namespace. I need to manage dynamic async creation and tear down of the GraphQL java class but I can create the GraphQLHandler as well then delegate accordingly.

You're welcome. Yes, delegating to handler stored in a map (path=>handler) is another option. I believe you have to manage async creation and tear down of the corresponding GraphQL object in any case.

Sorry to bug you about this. If would have thought of this in the first place I wouldn't have needed to.

No problem. We can't think about any use case upfront, and we may not want to support them all anyway.

NavidMitchell commented 1 day ago

I used a Caffeine cache vs a Map given my requirement.
If anyone in interested the implementation can be found here.

https://github.com/Kinotic-Foundation/structures/blob/25814e3a93293f54873a4a26e54c83ec5fa83731/structures-core/src/main/java/org/kinotic/structures/internal/endpoints/graphql/DefaultDelegatingGqlHandler.java#L41