OpenAPITools / jackson-databind-nullable

JsonNullable wrapper class and Jackson module to support meaningful null values
Apache License 2.0
103 stars 30 forks source link

JsonNullable with Collection is broken #49

Closed MelleD closed 1 year ago

MelleD commented 1 year ago

With the version >0.25 the Collection handling is broken.

When you use for example JsonNullable<Set> the extractor (JsonNullableValueExtractorHelper) throws a error.

Caused by: java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.util.Collection (java.lang.Long and java.util.Collection are in module java.base of loader 'bootstrap')
    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)
    ... 156 more
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.JsonNullableValueExtractorHelper.extractValues(JsonNullableValueExtractorHelper.java:13)
    at org.openapitools.jackson.nullable.JsonNullableValueExtractor.extractValues(JsonNullableValueExtractor.java:14)
    at org.openapitools.jackson.nullable.JsonNullableValueExtractor.extractValues(JsonNullableValueExtractor.java:10)
    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.springframework.validation.beanvalidation.SpringValidatorAdapter.validate(SpringValidatorAdapter.java:109)

Example:


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

@tofi86 Think it is related to you change JsonNullable handling

EDIT: Ok i see there is a longer message, but no solution. https://github.com/OpenAPITools/jackson-databind-nullable/pull/35

But currently this crashes the open api generator with spring boot.

tofi86 commented 1 year ago

@wing328 Maybe now it's time to discuss my PR feedback in https://github.com/OpenAPITools/jackson-databind-nullable/pull/35#issuecomment-1294854904?

MelleD commented 1 year ago

@tofi86 I'm not sure what did not work for you?

But here is a example with output that the Bean validation is working with 0.24 when the JsonNullable is not undefined

public class JsonNullableTest extends WebBaseTest {

   public static class MyBean {

      @Valid
      private JsonNullable<Set<Long>> setJsonNullable = JsonNullable.of( Set.of( 1L ) );
      @Valid
      private final JsonNullable<Set<MyBean2>> beans2 = JsonNullable.of( Set.of( new MyBean2() ) );
      @Valid
      private final JsonNullable<MyBean2> bean2 = JsonNullable.of( new MyBean2() );

      public void setSetJsonNullable( final JsonNullable<Set<Long>> setJsonNullable ) {
         this.setJsonNullable = setJsonNullable;
      }

      @Size( min = 2 )
      public JsonNullable<Set<Long>> getSetJsonNullable() {
         return setJsonNullable;
      }

      @Valid
      public JsonNullable<MyBean2> getBean2() {
         return bean2;
      }

      @Valid
      @Size( min = 2 )
      public JsonNullable<Set<MyBean2>> getBeans2() {
         return beans2;
      }

   }

   public static class MyBean2 {
      @NotNull
      private String name;

      private JsonNullable<String> name2 = JsonNullable.of( "b" );

      public void setName( final String name ) {
         this.name = name;
      }

      public String getName() {
         return name;
      }

      @Size( min = 5 )
      @Pattern( regexp = "a*" )
      public JsonNullable<String> getName2() {
         return name2;
      }

      public void setName2( final JsonNullable<String> name2 ) {
         this.name2 = name2;
      }
   }

   @Autowired
   ValidationService validationService;

   @Test
   void test() {
      final MyBean bean = new MyBean();

      try {
         validationService.validate( bean );
      } catch ( final ConstraintViolationException e ) {
         e.getConstraintViolations().stream()
               .forEach( constraintViolation -> System.out.println( constraintViolation.getPropertyPath() + ": " + constraintViolation.getMessage() ) );
      }
   }

}

}
setJsonNullable: size must be between 2 and 2147483647

beans2: size must be between 2 and 2147483647

bean2.name2: size must be between 5 and 2147483647
bean2.name: must not be null
bean2.name2: must match "a*"
MelleD commented 1 year ago

@tofi86 Or is this the root cause?

 @Valid
      private final JsonNullable<Set<MyBean2>> beans2 = JsonNullable.of( Set.of( new MyBean2() ) );

That the elements in MyBean2(NotNull, Pattern etc) are not checked?

But is the same for Optional, correct?

      @Valid
      private final Optional<@Valid Set<MyBean2>> beansOptional = Optional.of( Set.of( new MyBean2() ) );
MelleD commented 1 year ago

@tofi86 me again :)

IMHO this is a Bug into the generator because the validation works. I think you can revert you change. The @Valid have to inside the Set then wit works.

Example:

public class JsonNullableTest extends WebBaseTest {

   public static class MyBean {

      private final JsonNullable<Set<@Valid MyBean2>> beansNullable = JsonNullable.of( Set.of( new MyBean2() ) );

      private final Optional<Set<@Valid MyBean2>> beansOptional = Optional.of( Set.of( new MyBean2() ) );

      public JsonNullable<Set<MyBean2>> getBeansNullable() {
         return beansNullable;
      }

      public Optional<Set<MyBean2>> getBeansOptional() {
         return beansOptional;
      }
   }

   public static class MyBean2 {
      @NotNull
      private String name;

      private JsonNullable<String> name2 = JsonNullable.of( "b" );

      public void setName( final String name ) {
         this.name = name;
      }

      public String getName() {
         return name;
      }

      @Size( min = 5 )
      @Pattern( regexp = "a*" )
      public JsonNullable<String> getName2() {
         return name2;
      }

      public void setName2( final JsonNullable<String> name2 ) {
         this.name2 = name2;
      }
   }

   @Autowired
   ValidationService validationService;

   @Test
   void test() {
      final MyBean bean = new MyBean();

      try {
         validationService.validate( bean );
      } catch ( final ConstraintViolationException e ) {
         e.getConstraintViolations().stream()
               .forEach( constraintViolation -> System.out.println( constraintViolation.getPropertyPath() + ": " + constraintViolation.getMessage() ) );
      }
   }

}
beansNullable[].name2: must match "a*"
beansNullable[].name2: size must be between 5 and 2147483647
beansNullable[].name: must not be null

beansOptional[].name2: size must be between 5 and 2147483647
beansOptional[].name: must not be null
beansOptional[].name2: must match "a*"

So IMHO we have just to change the generator to set the @Valid on the right position.

@wing328 what do you think?

MelleD commented 1 year ago

@wing328 & @tofi86

IMHO we should revert the change and discuss the solution on the generator side https://github.com/OpenAPITools/openapi-generator/issues/14723

tofi86 commented 1 year ago

IMHO this is a Bug into the generator because the validation works. I think you can revert you change. The @Valid have to inside the Set then wit works.

Sounds good to me. Thanks for taking the lead on this issue @MelleD!

tofi86 commented 1 year ago

@tofi86 Or is this the root cause?

 @Valid
      private final JsonNullable<Set<MyBean2>> beans2 = JsonNullable.of( Set.of( new MyBean2() ) );

That the elements in MyBean2(NotNull, Pattern etc) are not checked?

Yes exactly, that was one of the issues PR #35 tried to solve. Also getting the exact validation path of the failed item.