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

Check for and eliminate duplicate schema resource registrations #230

Closed
rstoyanchev opened this issue Dec 21, 2021 · 3 comments
Closed

Check for and eliminate duplicate schema resource registrations #230

rstoyanchev opened this issue Dec 21, 2021 · 3 comments
Assignees
Labels
in: core Issues related to config and core support type: enhancement A general enhancement
Milestone

Comments

@rstoyanchev
Copy link
Contributor

A GraphQlSourceBuilderCustomizer might need to replace the default schema resources, e.g. #218 (comment).

@rstoyanchev rstoyanchev added type: enhancement A general enhancement in: core Issues related to config and core support labels Dec 21, 2021
@rstoyanchev rstoyanchev added this to the 1.0.0-M5 milestone Dec 21, 2021
@bclozel bclozel self-assigned this Dec 21, 2021
@bclozel bclozel modified the milestones: 1.0.0-M5, 1.0 Backlog Jan 6, 2022
@bclozel bclozel added the status: blocked An issue that's blocked on an external project change label Jan 6, 2022
@bclozel
Copy link
Member

bclozel commented Jan 7, 2022

I've isolated the PathMatchingResourcePatternResolver in a native application to better understand the issue. This is exactly what Josh hinted at: the resolver is resolving resources from the classpath and then trying to consider them as files. Resources in a native image are not files, folders up in the hierarchy are not added to the image. Even if we add them manually, they're not considered as files so the resolver cannot list resources under them.

I don't think the issue is about replacing resources - as this happens only in JVM mode since both scanned schema resources and resources manually set with a customizer are present, leading to duplicates. In native mode, this doesn't happen since the first set of resources are not discovered.

The code snippet listed in #218 (comment) looks fine to me - and this should actually be quite close to what we should do in the future: an AOT processor should discover those resources at build time, register them as resources against the native image, and then generate code that does such manual registration.

In summary, I don't think we need to change the current behavior.

@bclozel bclozel closed this as completed Jan 7, 2022
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: blocked An issue that's blocked on an external project change labels Jan 7, 2022
@bclozel bclozel removed this from the 1.0 Backlog milestone Jan 7, 2022
@bclozel
Copy link
Member

bclozel commented Jan 7, 2022

I've created spring-projects/spring-boot#29291 to follow up on this.
Note that Spring GraphQL doesn't offer a Jakarta variant for now, so support for this project is not yet available in the Spring Boot 3.x branch.

@rstoyanchev
Copy link
Contributor Author

After a team discussion, I'm reopening this issue to also improve the situation for Boot 2.7.

@joshlong pointed out in #218 (comment) that using GraphqlSourcebuilderCustomizer almost works, except that on the JVM you end up with duplicate resources. We can explicitly protect against this by checking for and ignoring duplicate registrations. This should help for this case, but also shouldn't hurt generally since there should be no reason for the same resource to be added twice.

@rstoyanchev rstoyanchev reopened this Feb 10, 2022
@rstoyanchev rstoyanchev assigned rstoyanchev and unassigned bclozel Feb 10, 2022
@rstoyanchev rstoyanchev removed the status: declined A suggestion or change that we don't feel we should currently apply label Feb 10, 2022
@rstoyanchev rstoyanchev added this to the 1.0.0-M6 milestone Feb 10, 2022
@rstoyanchev rstoyanchev changed the title Provide option to replace schema resources Check for and eliminate duplicate schema resource registrations Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues related to config and core support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants