Add graphql-dgs-extended-validation module #659

hantsy · 2021-09-25T10:00:12Z

address #640

setchy · 2021-09-25T11:30:06Z

Thinking out loud, I suspect a new Advanced documentation entry may be required, too - https://github.com/Netflix/dgs

hantsy · 2021-09-25T13:25:20Z

@setchy I may add such a section later. But I am not a English speaker, writing great English docs is more difficult than coding.

srinivasankavitha · 2021-09-25T15:30:31Z

Thanks for the PR @hantsy. We were just recently looking at adding this. Will review and test it out next week.

…

On Sep 25, 2021, at 6:25 AM, Hantsy Bai ***@***.***> wrote: @setchy I may add such a section later. But I am a none English speaker, writing great English docs is more difficult than coding. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

berngp

Thanks @hantsy, did a quick review and looks great. Will take a deeper dive tomorrow.

...validation/src/test/kotlin/com/netflix/graphql/dgs/autoconfig/BeanValidationSizeSmokeTest.kt

srinivasankavitha · 2021-09-28T23:09:57Z

I see this error when I try it with a spring boot app:

 Exception while executing data fetcher for /createPost: HV000183: Unable to initialize 'jakarta.el.ExpressionFactory'. Check that you have the EL dependencies on the classpath, or use ParameterMessageInterpolator instead

jakarta.validation.ValidationException: HV000183: Unable to initialize 'jakarta.el.ExpressionFactory'. Check that you have the EL dependencies on the classpath, or use ParameterMessageInterpolator instead
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.buildExpressionFactory(ResourceBundleMessageInterpolator.java:217) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.<init>(ResourceBundleMessageInterpolator.java:83) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.<init>(ResourceBundleMessageInterpolator.java:48) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]

You can reproduce it by adding the schema in your test to the graphql-dgs-example-shared module and running the example app. Running the createPost query will produce the error. Looks like the dependencies are not getting pulled in transitively.

srinivasankavitha · 2021-09-28T17:30:15Z

...src/main/kotlin/com/netflix/graphql/dgs/autoconfig/DgsExtendedValidationAutoConfiguration.kt

+import org.springframework.context.annotation.Configuration
+
+@ConditionalOnClass(graphql.validation.rules.ValidationRules::class)
+@ConditionalOnProperty(


Can we get rid of this property? Since the functionality is anyway in a separate module, if the user wants to disable it, he can simply avoid adding it altogether. I'd prefer not having too many configurable properties as much as possible.

I tend to agree, but would that make it inconsistent with the how extended-scalars or micrometer (any others???) are configured?

This is a general approach to enable features in Spring Boot. For example, in the application I need some feature, but in some tests for some specific reason it can be disabled with a property on tests.

Yes, I agree that this is a general approach. However, for this particular feature, I didn't see a use case for having the module be present but disabled via a property. That said, I am fine with keeping it as is.
@setchy - as you also pointed out, we do have the properties for those features that are already in separate modules. So I agree with your point about consistency as well.

hantsy · 2021-09-29T02:54:55Z

I see this error when I try it with a spring boot app:

 Exception while executing data fetcher for /createPost: HV000183: Unable to initialize 'jakarta.el.ExpressionFactory'. Check that you have the EL dependencies on the classpath, or use ParameterMessageInterpolator instead

jakarta.validation.ValidationException: HV000183: Unable to initialize 'jakarta.el.ExpressionFactory'. Check that you have the EL dependencies on the classpath, or use ParameterMessageInterpolator instead
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.buildExpressionFactory(ResourceBundleMessageInterpolator.java:217) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.<init>(ResourceBundleMessageInterpolator.java:83) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]
	at org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator.<init>(ResourceBundleMessageInterpolator.java:48) ~[hibernate-validator-7.0.1.Final.jar:7.0.1.Final]

You can reproduce it by adding the schema in your test to the graphql-dgs-example-shared module and running the example app. Running the createPost query will produce the error. Looks like the dependencies are not getting pulled in transitively.

Currently I only tried 16.0 in my Dgs example(4.8.3). The hibernate validator added required deps, no need extra settings. The SmokeTest did not report this error(executed on the GraphQl engine only). You can add an extra EL 4.0 dependency and try again.

@berngp But from this error, I found another terrible issue. I checked Hibernate Validator 7.x doc, found it was updated to Jakarta EE 9 API level: https://in.relation.to/2021/01/06/hibernate-validator-700-62-final-released/, but Spring 5 is still aligned to Java EE 8/Jakarta EE 8, does support the new namespace.

This maybe cause other issues in a real Spring Boot application. Hope this graphql-java-validation project will release a version variant based on GraphQL 17 and Hiberante Validator 6.2(Jakarta EE 8 level).

hantsy · 2021-09-29T03:43:30Z

There is an issue reported the problem when using GraphQL 17 and Spring Boot 2.5.5 together, graphql-java/graphql-java-extended-validation#52

srinivasankavitha · 2021-09-29T16:57:59Z

There is an issue reported the problem when using GraphQL 17 and Spring Boot 2.5.5 together, graphql-java/graphql-java-extended-validation#52

I think we'll defer this PR until there is a resolution for the above mentioned issue, agree? We just switched the framework to graphql-java-17 and released already.

berngp

This changes works well with the Spring Boot 2.3.x versions that DGS is currently using,
but we will soon be moving to Spring Boot 2.5. There is a blocker on graphql-java-extended-validation when used along with Spring Boot 2.5, see issue below. For this reason we will have to wait for this PR.

graphql-java/graphql-java-extended-validation#52

paulbakker · 2021-09-29T23:38:35Z

Thanks for the work @hantsy. I was pretty excited to see this!

Too bad about the Hibernate Validator issue that seems to be blocking for now. Hopefully that can be resolved in library.

paulbakker · 2021-09-29T23:46:37Z

Thanks to @berngp I found this thread on the Spring Framework that clarifies why this is an issue: spring-projects/spring-framework#25354 (comment)

hantsy · 2021-09-30T05:25:10Z

@berngp Current Spring only recognizes the Jakarta EE 8 API and ignore new Jakarta EE 9 API at all, so I think if using this validation extension in a Spring Boot 2.5 application, it could work, but there are some Jakarta EE 8 and 9 API coexisted in the final jar.

hantsy · 2021-10-02T05:15:50Z

@srinivasankavitha Sent a PR on the upstream project, graphql-java/graphql-java-extended-validation#53, the default source codes are switched back to Jakarta EE 8, and currently I used a jakarta classier for Jakarta EE 9 build.

I would like a different artifactId for the jakarta build(as some Jakarta EE projects used a jakarta postfix for EE 9 and differentiate from EE 8), but I can not set the artifactId dynamically in the maven publish plugin config.

hantsy · 2021-10-05T15:44:53Z

@berngp When adding all the deps (Jakarta EE 9) into build.gradle, the application starts up without any errors. https://github.com/hantsy/spring-graphql-sample/blob/master/dgs-codegen/build.gradle#L26-L32

But it will fail your application, when you are using validation APIs.

sbilello · 2021-11-02T03:42:14Z

Is there a particular validation mechanism outside of this one adopted by Netflix?
I guess that this module is not used but I encountered the problem of upgrading my project and it looks like that it is a problem with its depth dependency hierarchy problem. Is there a different solution adopted or there is no POJO validation at all?

hantsy · 2021-11-02T05:51:25Z

@sbilello you can use Bean Validation directly.

Generally, when using Dgs framework, if you build the models manually, Bean validation is a good match. If you are using codegen plugin to generate models, GraphQL specific validation directives are better. This is only my idea.

sbilello · 2021-11-03T02:22:49Z

Yes, @hantsy but I didn't want to remove the graphql-java-extended-validation module.

hantsy · 2021-11-22T13:28:00Z

@paulbakker There is no response from the upstream maintainers for weeks, if possible create a fork of graphql-java-extended-validation under Netflix? I think only need a Jakarta EE 8 version here.

paulbakker · 2021-11-23T21:00:07Z

Forking would be a bit of a maintenance burden long-term, so I would really prefer to avoid that. I've asked on twitter who's maintaining the repo, hopefully we'll find someone who can merge/release.

paulbakker · 2021-11-24T06:42:36Z

I spoke to Andi today, and he'll look into it after next week. They obviously have a lot on their plate, so let's just be patient. At least we now know the repo isn't abandoned.

hantsy · 2021-11-26T05:26:31Z

@paulbakker Refreshed and update the version.

graphql-dgs-platform/build.gradle.kts

hantsy force-pushed the validation-extensions branch 2 times, most recently from 8800b98 to cd974f7 Compare September 25, 2021 10:12

hantsy force-pushed the validation-extensions branch 4 times, most recently from ceae27f to dbda4e6 Compare September 26, 2021 04:08

berngp reviewed Sep 26, 2021

View reviewed changes

berngp approved these changes Sep 28, 2021

View reviewed changes

srinivasankavitha reviewed Sep 28, 2021

View reviewed changes

...validation/src/test/kotlin/com/netflix/graphql/dgs/autoconfig/BeanValidationSizeSmokeTest.kt Show resolved Hide resolved

srinivasankavitha requested changes Sep 28, 2021

View reviewed changes

feat: add graphql-java-validation module

19c4a10

hantsy force-pushed the validation-extensions branch from 300eda0 to 19c4a10 Compare September 29, 2021 02:46

berngp self-requested a review September 29, 2021 20:32

berngp requested changes Sep 29, 2021

View reviewed changes

paulbakker added blocked-external don't merge yet labels Sep 29, 2021

sbilello mentioned this pull request Nov 2, 2021

Add Jakarta EE 8 support graphql-java/graphql-java-extended-validation#53

Closed

hantsy added 2 commits November 26, 2021 12:09

Merge branch 'master' into validation-extensions

87fd65c

chore: update graphql-java-extended-extension version

c190ecf

hantsy force-pushed the validation-extensions branch from 0cd1286 to c190ecf Compare November 26, 2021 05:21

berngp approved these changes Nov 26, 2021

View reviewed changes

graphql-dgs-platform/build.gradle.kts Show resolved Hide resolved

berngp merged commit a5954a4 into Netflix:master Nov 26, 2021

hantsy deleted the validation-extensions branch November 26, 2021 07:53

jord1e mentioned this pull request Jan 2, 2022

Add graphql-java-extended-scalars dependency management spring-projects/spring-graphql#242

Closed

Prototype-42 mentioned this pull request Nov 27, 2022

documentation: Add documentation for the graphql-dgs-extended-validation module #1336

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add graphql-dgs-extended-validation module #659

Add graphql-dgs-extended-validation module #659

hantsy commented Sep 25, 2021

setchy commented Sep 25, 2021 •

edited

hantsy commented Sep 25, 2021 •

edited

srinivasankavitha commented Sep 25, 2021 via email

berngp left a comment

srinivasankavitha commented Sep 28, 2021

srinivasankavitha Sep 28, 2021

setchy Sep 28, 2021

hantsy Sep 29, 2021

srinivasankavitha Sep 29, 2021

hantsy commented Sep 29, 2021 •

edited

hantsy commented Sep 29, 2021

srinivasankavitha commented Sep 29, 2021

berngp left a comment •

edited

paulbakker commented Sep 29, 2021

paulbakker commented Sep 29, 2021

hantsy commented Sep 30, 2021

hantsy commented Oct 2, 2021 •

edited

hantsy commented Oct 5, 2021 •

edited

sbilello commented Nov 2, 2021

hantsy commented Nov 2, 2021

sbilello commented Nov 3, 2021

hantsy commented Nov 22, 2021

paulbakker commented Nov 23, 2021

paulbakker commented Nov 24, 2021

hantsy commented Nov 26, 2021

Add graphql-dgs-extended-validation module #659

Add graphql-dgs-extended-validation module #659

Conversation

hantsy commented Sep 25, 2021

setchy commented Sep 25, 2021 • edited

hantsy commented Sep 25, 2021 • edited

srinivasankavitha commented Sep 25, 2021 via email

berngp left a comment

Choose a reason for hiding this comment

srinivasankavitha commented Sep 28, 2021

srinivasankavitha Sep 28, 2021

Choose a reason for hiding this comment

setchy Sep 28, 2021

Choose a reason for hiding this comment

hantsy Sep 29, 2021

Choose a reason for hiding this comment

srinivasankavitha Sep 29, 2021

Choose a reason for hiding this comment

hantsy commented Sep 29, 2021 • edited

hantsy commented Sep 29, 2021

srinivasankavitha commented Sep 29, 2021

berngp left a comment • edited

Choose a reason for hiding this comment

paulbakker commented Sep 29, 2021

paulbakker commented Sep 29, 2021

hantsy commented Sep 30, 2021

hantsy commented Oct 2, 2021 • edited

hantsy commented Oct 5, 2021 • edited

sbilello commented Nov 2, 2021

hantsy commented Nov 2, 2021

sbilello commented Nov 3, 2021

hantsy commented Nov 22, 2021

paulbakker commented Nov 23, 2021

paulbakker commented Nov 24, 2021

hantsy commented Nov 26, 2021

setchy commented Sep 25, 2021 •

edited

hantsy commented Sep 25, 2021 •

edited

hantsy commented Sep 29, 2021 •

edited

berngp left a comment •

edited

hantsy commented Oct 2, 2021 •

edited

hantsy commented Oct 5, 2021 •

edited