spring-projects / spring-graphql

Spring Integration for GraphQL
https://spring.io/projects/spring-graphql
Apache License 2.0
1.52k stars 300 forks source link

Adding URI path segment to HttpGraphqlClient builder #937

Closed tvaneerten closed 5 months ago

tvaneerten commented 6 months ago

Problem

When building the HttpGraphqlClient, or when building the request, there is no option to add a URI path segment to the underlying WebClient instance's baseUrl. In situations where a WebClient may be used for different endpoints on the same service, and the GraphQL service is located at /graphql, for example, it would be nice to be able to configure that URI at the HttpGraphqlClient level without needing to build a new WebClient for the Graphql-specific endpoint.

I'm thinking of how the UrlSpec.url method works with WebClient:

webClient.post().uri("/service") ...
bclozel commented 5 months ago

Thanks for reaching out @tvaneerten ! We have discussed this today and we don't think we are going to introduce this change. The GraphQlClient should, once built, provide an API that's transport agnostic. Introducing a uri(String) method would go against that since we're getting at the transport level here.

Still, we would like to learn more about your use case and concerns:

  1. it seems that you think that creating several client instances has drawbacks. If you have memory usage concerns, most of the actual resources should be shared here. If this is about duplicating code, have you considered using the mutate method to create another instance out of an existing one?
  2. as far as we understand, targeting different endpoints most likely means targeting different schemas. In this case, we think that using separate instances (with different concerns) is a good idea. Maybe your use case is different?

Thanks!

tvaneerten commented 5 months ago

Hey thanks for the response. I understand the transport-agnostic reasoning once the GraphQLClient instance is built. That makes total sense to me. However, I still see some benefit to having this at the GraphQLClient.Builder level.

My situation is I have a REST service and a GraphQL service on the same host. When using a WebClient for my REST service, I set up my base URL on the WebClient,

WebClient webClient = WebClient.builder().baseUrl("http://localhost:8080") ...

And then per call, specify the path:

webClient.get().uri("/users") ...

But now I want to build my GraphQLClient using the same WebClient, since I'm requesting from the same host. I want to share all of that configuration. However, my GraphQL service is at http://localhost:8080/graphql, not http://localhost:8080.

As you mentioned, I could use mutate and create a copied WebClient, and set the full URL:

WebClient graphQLWebClient = webClient.mutate().baseUrl("http://localhost:8080/graphql").build();
GraphQLClient graphQLClient = HttpGraphQLClient.Builder(graphQLWebClient).build();

But this seems much cleaner (and also doesn't require me to know the base URL of the underlying WebClient):

GraphQLClient graphQLClient = HttpGraphQLClient.Builder(webClient).uri("/graphql").build();
bclozel commented 5 months ago

But this seems much cleaner (and also doesn't require me to know the base URL of the underlying WebClient)

I think it's the opposite. If the WebClient has been created like this:

WebClient webClient = WebClient.create("http://localhost:8080/restapi/");

Doing the following should point to "http://localhost:8080/restapi/graphql" as a base url if we concatenate, or "/graphql" if we override. In both cases I can see developers being confused by the result.

GraphQLClient graphQLClient = HttpGraphQLClient.Builder(webClient).uri("/graphql").build();

Thanks for the proposal, but I don't see a way forward without making things more confusing.