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

Adding pluggable SPI for schema resolution. #218

Closed
wants to merge 1 commit into from

Conversation

joshlong
Copy link
Member

Hi - i'd like to be able to source the schema for the GraphQLSource from different places. And, specifically, I'd like to be able to hard code the resolution of the resources so that things work well in a GraalVM context. This adds an interface to support the strategy

@jvalkeal
Copy link
Contributor

@joshlong There's already GraphQlSourceBuilderCustomizer which allows you to add additional sources.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 14, 2021

Indeed, that has been possible since M3, after #124. Closing for now, but if you run into issues with it, feel free to comment and we'll re-open if needed.

@rstoyanchev rstoyanchev added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 14, 2021
@joshlong
Copy link
Member Author

joshlong commented Dec 18, 2021

Hi - the GraphqlSourcebuilderCustomizer is close to, but not quite, what I want.

It adds to the GraphQLBuilder after the default locations have already been added. What I want is to replace the defaults, not add to them. The ResourcePatternResolver doesn't work on GraalVM. So nothing gets added. It fails on startup. I'd like to use the GraphQlSourceBuilderCustomizer to provide the Resources i want directly. If I do the following, it fails when running on the JRe because it finds the schema with the ResourcePatternResolver, and then finds my addition, contributed through the GraphQLSourceBuilderCustomizer. It thinks I'm trying to redefine the types. It does work on Spring Native however, since in the end there is only one Resource contributed to the final application.

        @Bean
	GraphQlSourceBuilderCustomizer graphQlSourceBuilderCustomizer() {
		return builder ->  builder.schemaResources(new ClassPathResource("graphql/schema.graphqls")) ;
		 
	}

I can do the following hacky thing to detect if I'm running inside a GraalVM native image, and therefore only contribute the Resource in a GraalVM native image context, but it is super hacky.

	@Bean
	GraphQlSourceBuilderCustomizer graphQlSourceBuilderCustomizer() {
		return builder -> {
			if ( NativeDetector.inNativeImage()) {
                           builder.schemaResources(new ClassPathResource("graphql/schema.graphqls")) ;
                         }
		};
	}

@rstoyanchev
Copy link
Contributor

Okay, I see. We can add an option on GraphQlSource.Builder to replace schema resources, perhaps:

Builder schemaResources(Consume<List<Resource>> resourcesConsumer);

@rstoyanchev
Copy link
Contributor

I've created #230 for now, although @bclozel, I do wonder if there needs to be some improvement in Spring's ResourceResolver to better accommodate resources for native apps.

@bclozel
Copy link
Member

bclozel commented Dec 21, 2021

To me this should be something native images should support (listing resources under a particular location). I'll need to experiment a bit. This might be a problem with our own ResourceResolver, in Spring Native - I'm not sure #230 is really necessary, it's too early to tell.

@rstoyanchev
Copy link
Contributor

Okay, it makes sense to get to the bottom of the problem. I figured, having more control over the list of resources makes sense on its own, but better to see if it is really needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants