Skip to content

ContextDataFetcherDecorator ignores subscriptions when "subscription" type is renamed #590

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
etienne-sf opened this issue Dec 23, 2022 · 3 comments
Assignees
Labels
status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@etienne-sf
Copy link

etienne-sf commented Dec 23, 2022

Hello,

In GraphQL, it is possible to rename the query, mutation and subscription objects in the schema. When doing so, data fetcher that returns Flux won't work properly in spring-graphql.

Sample schema:

schema
  query: MyQuery
  subscription: MySubscription
}
type MyQuery {
  greetings: [String]
}
type MySubscription {
  greetings: String
}

For subscription, this is due to this line in org.springframework.graphql.execution.ContextDataFetcherDecorator:

boolean handlesSubscription = parent.getName().equals("Subscription");

To check that, I executed the test provided below, after changing Subscription to MySubscription in ContextDataFetcherDecorator: doing this makes the test pass ok.

My first attempt to create a unit test generates wrong failures for Query and Mutation, as the queryFetcher and mutationFetcher hard coded the Query and Mutation name. So I updated the test below which is ok for query and mutation.

I added the test below in the org.springframework.graphql.execution.ContextDataFetcherDecoratorTests. This test is basically a copy/paste of the fluxDataFetcher() and fluxDataFetcherSubscription() tests:

	@Test
	void fluxDataFetcherRenamedSubscription() throws Exception {
		GraphQL graphQl = GraphQlSetup.schemaContent("" +
				"schema {" +
				"  query: MyQuery" +
				"  mutation: MyMutation" +
				"  subscription: MySubscription" +
				"}" +
				"type MyQuery { " +
				"  querySomeData: [String] " +
				"} " +
				"type MyMutation { " +
				"  createSomeData: [String] " +
				"} " +
				"type MySubscription { " +
				"  greetings: String " +
				"}")
			// A query fetcher that returns a Flux (to check Flux behavior on Subscription and non Subscription)
			// (need to use the dataFetcher method as queryFetcher hard coded the query name to "Query")
			.dataFetcher("MyQuery", "querySomeData", (env) ->
				Mono.delay(Duration.ofMillis(50))
						.flatMapMany((aLong) -> Flux.deferContextual((context) -> {
							String name = context.get("name");
							return Flux.just("Hi", "Bonjour", "Hola").map((s) -> s + " " + name);
						})))
			// (need to use the dataFetcher method as queryFetcher hard coded the mutation name to "Mutation")
			.dataFetcher("MyMutation", "createSomeData", (env) ->
				Mono.delay(Duration.ofMillis(50))
						.flatMapMany((aLong) -> Flux.deferContextual((context) -> {
							String name = context.get("name");
							return Flux.just("Hi", "Bonjour", "Hola").map((s) -> s + " " + name);
						})))
			.dataFetcher("MySubscription", "greetings", (env) ->
				Mono.delay(Duration.ofMillis(50))
						.flatMapMany((aLong) -> Flux.deferContextual((context) -> {
							String name = context.get("name");
							return Flux.just("Hi", "Bonjour", "Hola").map((s) -> s + " " + name);
						})))
			.toGraphQl();

		//////////////////////////////////////////////////////////////////////////////////////
		// MyQuery check
		ExecutionInput input = ExecutionInput.newExecutionInput().query("{ querySomeData }").build();
		ReactorContextManager.setReactorContext(Context.of("name", "007"), input.getGraphQLContext());

		ExecutionResult result = graphQl.executeAsync(input).get();

		List<String> data = ResponseHelper.forResult(result).toList("querySomeData", String.class);
		assertThat(data).containsExactly("Hi 007", "Bonjour 007", "Hola 007");

		//////////////////////////////////////////////////////////////////////////////////////
		// MyMutation check
		input = ExecutionInput.newExecutionInput().query("mutation { createSomeData }").build();
		ReactorContextManager.setReactorContext(Context.of("name", "007"), input.getGraphQLContext());

		result = graphQl.executeAsync(input).get();

		data = ResponseHelper.forResult(result).toList("createSomeData", String.class);
		assertThat(data).containsExactly("Hi 007", "Bonjour 007", "Hola 007");

		//////////////////////////////////////////////////////////////////////////////////////
		// MySubscription check
		input = ExecutionInput.newExecutionInput().query("subscription { greetings }").build();
		ReactorContextManager.setReactorContext(Context.of("name", "007"), input.getGraphQLContext());

		result = graphQl.executeAsync(input).get();

		Flux<String> greetingsFlux = ResponseHelper.forSubscription(result)
				.map(response -> response.toEntity("greetings", String.class));

		StepVerifier.create(greetingsFlux)
				.expectNext("Hi 007", "Bonjour 007", "Hola 007")
				.verifyComplete();
	}

I'll try to find how to solve this issue, and propose a PR.

If you have hint, it would be nice: I don't know how to retrieve the GraphQL schema's data from the org.springframework.graphql.execution.ContextDataFetcherDecorator.createVisitor(..) method.

Etienne

@etienne-sf etienne-sf changed the title Flux data fetcher fails when query or subscription is rename Flux data fetcher fails when query, mutation or subscription is renamed Dec 23, 2022
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 23, 2022
@etienne-sf etienne-sf changed the title Flux data fetcher fails when query, mutation or subscription is renamed Flux data fetcher fails when subscription is renamed Dec 23, 2022
@rstoyanchev rstoyanchev self-assigned this Jan 4, 2023
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 4, 2023
@rstoyanchev rstoyanchev added this to the 1.1.2 milestone Jan 4, 2023
@rstoyanchev rstoyanchev changed the title Flux data fetcher fails when subscription is renamed ContextDataFetcherDecorator ignores subscriptions when "subscription" type is renamed Jan 4, 2023
@rstoyanchev
Copy link
Contributor

I didn't know this could be done. Thanks for tracking down the point where it fails. I've added fixed and it should work with the latest 1.1.2 snapshots.

@etienne-sf
Copy link
Author

Thank you

It's quite hard to find (and I didn't while writing this message), but if I remember properly, the 1.1.x branch goes with spring boot 2.8 that needs Java 17.

Is it possible to also push this correction into the 1.0.x branch ?

Étienne

@rstoyanchev
Copy link
Contributor

The 1.1 branch is for Boot 3.0 and Java 17. This is listed on the wiki. We'll backport this to 1.0.x with #593.

rstoyanchev added a commit that referenced this issue Mar 8, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants