Add support for Collection<JsonNullable<T>> in the JsonNullableValueExtractor by tofi86 · Pull Request #35 · OpenAPITools/jackson-databind-nullable

tofi86 · 2022-09-09T11:03:36Z

This PR adds support for Collection<JsonNullable> in the JsonNullableValueExtractor.

closes #34

…sonNullableValueExtractor

wing328 · 2022-10-24T02:49:38Z

@tofi86 thanks for the PR, which looks good to me.

tofi86 · 2022-10-28T10:58:31Z

@wing328 @vrnsky Thanks for approving and merging this patch.

Unfortunately, right after your approval, we stumbled upon a severe issue with this earlier this week:

Adding a container validation constraint like @Size to the JsonNullable<List<?>> field crashes the Hibernate Validator when using JsonNullable with my patch:

@Valid
@Size(min = 2)
private JsonNullable<Set<Person>> persons = JsonNullable.undefined();

fails the testUnwrapList() test:

Okt 28, 2022 11:43:03 AM org.hibernate.validator.internal.util.Version <clinit>
INFO: HV000001: Hibernate Validator 6.1.0.Final

javax.validation.ValidationException: HV000028: Unexpected exception during isValid call.

	at org.hibernate.validator.internal.engine.constraintvalidation.ConstraintTree.validateSingleConstraint(ConstraintTree.java:186)
	at org.hibernate.validator.internal.engine.constraintvalidation.SimpleConstraintTree.validateConstraints(SimpleConstraintTree.java:62)
	at org.hibernate.validator.internal.engine.constraintvalidation.ConstraintTree.validateConstraints(ConstraintTree.java:75)
	at org.hibernate.validator.internal.metadata.core.MetaConstraint.doValidateConstraint(MetaConstraint.java:130)
	at org.hibernate.validator.internal.metadata.core.MetaConstraint.access$100(MetaConstraint.java:39)
	at org.hibernate.validator.internal.metadata.core.MetaConstraint$TypeParameterValueReceiver.doValidate(MetaConstraint.java:246)
	at org.hibernate.validator.internal.metadata.core.MetaConstraint$TypeParameterValueReceiver.indexedValue(MetaConstraint.java:212)
	at org.openapitools.jackson.nullable.JsonNullableValueExtractor.extractValues(JsonNullableValueExtractor.java:21)
	at org.openapitools.jackson.nullable.JsonNullableValueExtractor.extractValues(JsonNullableValueExtractor.java:11)
	at org.hibernate.validator.internal.engine.valueextraction.ValueExtractorHelper.extractValues(ValueExtractorHelper.java:42)
	at org.hibernate.validator.internal.metadata.core.MetaConstraint.validateConstraint(MetaConstraint.java:117)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validateMetaConstraint(ValidatorImpl.java:555)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validateConstraintsForSingleDefaultGroupElement(ValidatorImpl.java:518)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validateConstraintsForDefaultGroup(ValidatorImpl.java:488)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validateConstraintsForCurrentGroup(ValidatorImpl.java:450)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validateInContext(ValidatorImpl.java:400)
	at org.hibernate.validator.internal.engine.ValidatorImpl.validate(ValidatorImpl.java:172)
	at org.openapitools.jackson.nullable.JsonNullableValueExtractorTest.testUnwrapList(JsonNullableValueExtractorTest.java:71)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.lang.ClassCastException: org.openapitools.jackson.nullable.JsonNullableValueExtractorTest$Person cannot be cast to java.util.Collection
	at org.hibernate.validator.internal.constraintvalidators.bv.size.SizeValidatorForCollection.isValid(SizeValidatorForCollection.java:23)
	at org.hibernate.validator.internal.engine.constraintvalidation.ConstraintTree.validateSingleConstraint(ConstraintTree.java:180)
	... 45 more

The interesting part here is the Caused by:

Person cannot be cast to java.util.Collection

It seems like adding the list elements manually to the validation processor (as this PR does) causes the Hibernate validator to apply the @Size container validation constraint to the list child elements - which is of course totally wrong.

I did some debugging and research the past few days but cannot find a proper solution for this problem.

Reverting my PullRequest leaves the list elements unvalidated (which, at least for me, isn't a proper solution) – and keeping the patch as it is causes container constraints like @Size to fail badly (which isn't a good solution either).

What are your thoughts about this problem?

Maybe the original authors @hatzlj and @cbornet who discussed and implemented #2 have an opionion about this as well?

Just to recap: The initial motivation for opening issue #34 and submitting this patch was the fact, that list elements wrapped in JsonNullable were not validated, although the field was annotated with @Valid – whereas list elements are validated when the list is not wrapped.

I would have expected that unwrapping doesn't prevent further list item validation, but as we see it does. And there also seem to exist a couple of restrictions with that as written in the "Known issues" section of the validator docs. Maybe this is a limitation we cannot work around? Maybe we need to find another solution for that problem?

Sorry for the trouble! Looking forward to hearing your ideas about this...

Best regards,
Tobias

MelleD · 2023-02-15T14:12:04Z

No it crashes also without the @Size

This also crash

  @JsonProperty("groups")
  @Valid
  private JsonNullable<Set<Long>> groups = JsonNullable.undefined();

Without this change the @SiZe is working on the get method

  @Valid @Size(min=1, max = 10) 
  public JsonNullable<Set<Long>> getGroups() {
    return groups;
  }

OpenAPITools#34 adds support for Collection<JsonNullable<T>> in the J…

fbf920e

…sonNullableValueExtractor

tofi86 force-pushed the master branch from 3948b78 to fbf920e Compare September 9, 2022 11:12

vrnsky approved these changes Oct 23, 2022

View reviewed changes

wing328 merged commit 8bab929 into OpenAPITools:master Oct 24, 2022

bf2robot mentioned this pull request Feb 13, 2023

Update dependency org.openapitools:jackson-databind-nullable to v0.2.5 bf2fc6cc711aee1a0c2a/e2e-test-suite#475

Merged

1 task

MelleD mentioned this pull request Feb 15, 2023

JsonNullable with Collection is broken #49

Closed

This was referenced Feb 17, 2023

Fix broken unwrap bean validation and adapt tests #50

Merged

[BUG][Java-Spring] Collection Bean Validation is not working with JsonNullable and Optional OpenAPITools/openapi-generator#14723

Closed

renovate Bot mentioned this pull request Nov 20, 2024

Update all non-major maven dependencies valitydev/swag-fraudbusters#39

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Collection<JsonNullable<T>> in the JsonNullableValueExtractor#35

Add support for Collection<JsonNullable<T>> in the JsonNullableValueExtractor#35
wing328 merged 1 commit intoOpenAPITools:masterfrom
goldflam-gmbh:master

tofi86 commented Sep 9, 2022

Uh oh!

wing328 commented Oct 24, 2022

Uh oh!

tofi86 commented Oct 28, 2022

Uh oh!

MelleD commented Feb 15, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tofi86 commented Sep 9, 2022

Uh oh!

wing328 commented Oct 24, 2022

Uh oh!

tofi86 commented Oct 28, 2022

Uh oh!

MelleD commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MelleD commented Feb 15, 2023 •

edited

Loading