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

[tofi86]

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

closes #34

[wing328]

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

[tofi86]

@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]

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;
  }