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

[BUG][SPRING] Using useBeanValidation together with openApiNullable creates model classes that validate null values incorrectly #11969

Open
2 of 6 tasks
WIStudent opened this issue Mar 25, 2022 · 5 comments · May be fixed by OpenAPITools/jackson-databind-nullable#53

Comments

@WIStudent
Copy link

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

I have a spec that contains a repsonse that contains a json object with a nullable required field (see below). When I use the spring generator with useBeanValidation: true and openApiNullable: true (they are enabled per default) it produces a model that contains the following:

public class Config {
  @JsonProperty("endpoint")
  private JsonNullable<String> endpoint = JsonNullable.undefined();

  public Config endpoint(String endpoint) {
    this.endpoint = JsonNullable.of(endpoint);
    return this;
  }

  @ApiModelProperty(required = true, value = "")
  @NotNull
  public JsonNullable<String> getEndpoint() {
    return endpoint;
  }

  public void setEndpoint(JsonNullable<String> endpoint) {
    this.endpoint = endpoint;
}

When I create an instance of this model where the field endpoint is null and try to validate it, the validator says it is invalid because endpoint must not be null:

var config = new Config().setEndpoint(null);
var validator = Validation.buildDefaultValidatorFactory().getValidator();
var validationResult = validator.validate(config);

I think this happens because JsonNullable is implemented in a way that the validation constraint is applied to the wrapped value and not to the JsonNullable object itself (see https://github.com/OpenAPITools/jackson-databind-nullable)

openapi-generator version

5.3.1
I cannot upgrade to 5.4.0 because of #11506

OpenAPI declaration file content or url
components:
  responses:
    Config:
      description: success
      content:
        application/json:
          schema:
            title: Config
            type: object
            properties:
              endpoint:
                type: string
                nullable: true
            required:
              - endpoint
Generation Details

Using the spring generator with useBeanValidation: true and openApiNullable: true

Suggest a fix

Do not apply @NotNull annotation to getters that return a JsonNullable object.

@WIStudent
Copy link
Author

Seems like someone else also ran into this issue here OpenAPITools/jackson-databind-nullable#17 (comment)

@WIStudent
Copy link
Author

I took a look into the template and I think the intention of the @NotNull annotation is to make sure that required fields are present, but because JsonNullable applies the annotation differently this does not work.

If isPresent inside the JsonNullable instance is true, the validation is applied on the value, and according to this PR the validation isn't applied at all when isPresent is false. So in the case of JsonNullable @NotNull will not be able to ensure that a required field is present.

@WIStudent
Copy link
Author

As a workaround I created a custom validator that checks that the JsonNullable is present:

import javax.validation.Constraint;
import javax.validation.Payload;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Constraint(validatedBy = JsonNullableIsPresentValidator.class)
@Target( {ElementType.METHOD, ElementType.FIELD })
@Retention(RetentionPolicy.RUNTIME)
public @interface JsonNullableIsPresent {
  String message() default "JsonNullable is not present";
  Class<?>[] groups() default {};
  Class<? extends Payload>[] payload() default {};
}
import org.jetbrains.annotations.Nullable;
import org.openapitools.jackson.nullable.JsonNullable;
import javax.validation.ConstraintValidator;
import javax.validation.ConstraintValidatorContext;

public class JsonNullableIsPresentValidator implements ConstraintValidator<JsonNullableIsPresent, @Nullable JsonNullable<?>> {
  @Override
  public void initialize(JsonNullableIsPresent constraintAnnotation) {}

  @Override
  public boolean isValid(@Nullable JsonNullable<?> jsonNullable, ConstraintValidatorContext constraintValidatorContext) {
    return jsonNullable != null && jsonNullable.isPresent();
  }
}

Then I created a customized template and modified the required block of the beanValidation.mustache file:

{{#required}}{{#openApiNullable}}{{#isNullable}}  @JsonNullableIsPresent(payload = javax.validation.valueextraction.Unwrapping.Skip.class){{/isNullable}}{{^isNullable}}  @NotNull{{/isNullable}}{{/openApiNullable}}{{^openApiNullable}}  @NotNull{{/openApiNullable}}{{/required}}

@giovanniberti
Copy link

Also reproduced with 6.0.0

@tofi86
Copy link

tofi86 commented Jul 27, 2022

We also ran into this issue and went with a custom beanValidation.mustache template for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants