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

QueryDSL does not work with Extended Validation #240

Closed
Thinkenterprise opened this issue Jan 1, 2022 · 6 comments
Closed

QueryDSL does not work with Extended Validation #240

Thinkenterprise opened this issue Jan 1, 2022 · 6 comments
Assignees
Labels
in: core Issues related to config and core support status: superseded Issue is superseded by another type: enhancement A general enhancement

Comments

@Thinkenterprise
Copy link

I work with Spring Boot 2.6 and Spring GraphQL 1.0.0-M4.

Spring GraphQL supports Jakarta Bean Validation, and it works too.

I still tried the validation via DIRECTIVES and their implementation. That works too!!

<dependency>
	<groupId>com.graphql-java</groupId>
	<artifactId>graphql-java-extended-validation</artifactId>
	<version>17.0-hibernate-validator-6.2.0.Final</version>
</dependency>

However, if I add the following configuration, the QueryDSL repositories no longer work and return a value of null.

@Profile("validation")
@Configuration
public class GraphQLValidationConfiguration implements RuntimeWiringConfigurer {

    @Override
    public void configure(Builder builder) {
        ValidationRules validationRules = ValidationRules.newValidationRules()
                                                         .onValidationErrorStrategy(OnValidationErrorStrategy.RETURN_NULL)
                                                        .build();
       ValidationSchemaWiring validationSchemaWiring = new  ValidationSchemaWiring(validationRules);
       builder.directiveWiring(validationSchemaWiring);
    }
}

I also added the configuration and dependency to the Spring GraphQL sample and there are problems there too. So, it seems to be a general problem and not a problem in my implementation!?

I am not sure whether this is a mistake in the implementation of the various QueryDSLDataFetcher or whether it is due to the graphql-java-extended-validation

I just want to communicate the problem and let the Spring GraphQL team decide. Thanks

@bclozel bclozel added the status: waiting-for-triage An issue we've not yet triaged label Jan 3, 2022
@bclozel
Copy link
Member

bclozel commented Jan 3, 2022

Could you provide a sample application showing the problem? Ideally, something we can download or a project on github we could git clone. So far, I don't see anything specific to Spring GraphQL so we'd need to see the problem for ourselves to figure things out.

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Jan 3, 2022
@Thinkenterprise
Copy link
Author

Yes very much. I have pushed the small example based on your webmvc-http-sample into my github account. If you start the application the query

query {
  artifactRepositories {
    id
  }
}

doese not return any data. If you disable the GraphQLConfiguration the same query works.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 4, 2022
@rstoyanchev rstoyanchev added the in: core Issues related to config and core support label Jan 5, 2022
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 5, 2022

Looking at extended validation, ValidationSchemaWiring decorates the DataFetcher for each field, but the auto-registration for QueryDSL and QBE happens late, through a TypeVisitor after each RuntimeWiringConfigurer, and only if there isn't a registration already, that is other than a PropertyDataFetcher. As a result of this, such a decorated PropertyDataFetcher likely causes the QueryDSL auto-registration to back off.

I don't know yet what a solution might be but we can review the order in which we perform registrations in GraphQlSource.Builder in light of this.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 18, 2022

@Thinkenterprise, the way auto registration works has been updated to be more reliable and work better with other ways of registerting data fetchers. It's now based on a WiringFactory which is applied when deciding on the DataFetcher for a field, and before Extended Validation's decoration via SchemaDirectiveWiring.

So this should be fixed, if you'd like you can give it a try with 1.0.0-SNAPSHOT and Boot 2.7.0-SNAPSHOT for the starter.

@rstoyanchev rstoyanchev removed this from the 1.0.0-M5 milestone Jan 18, 2022
@rstoyanchev rstoyanchev added the status: superseded Issue is superseded by another label Jan 18, 2022
@Thinkenterprise
Copy link
Author

@rstoyanchev I checked it works. Thanks!!

@rstoyanchev
Copy link
Contributor

Good to hear and thanks for the quick confirmation!

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 status: superseded Issue is superseded by another type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants