OpenAPITools/jackson-databind-nullable

Allow javax.validation for JsonNullable for constraints supported by openapi

Closed this issue · 10 comments

When using a Constraint (e.g. size) on a nullable element, you will get the following exception

javax.validation.UnexpectedTypeException: HV000030: No validator could be found for constraint 'javax.validation.constraints.Size' validating type 'org.openapitools.jackson.nullable.JsonNullable<java.lang.String>'. Check configuration for 'data[0].message'

since there is no Validator for JsonNullable supporting the javax.validation.constraints.Size constraint.

We're currently testing a solution for this, declaring the necessary validators via META-INF/services. Since the JsonNullable is defined in jackson-databind-nullable i would suggest to include appropriate validators (including size and others) also in the jackson-databind-nullable module and expose them via the META-INF/services/javax.validation.ConstraintValidator to automatically support bean validation generated by the openapi-generator.

Before submitting a PR, i'd like to discuss the following questions:
-> is this module the right place for the validators (we would be adding a dependency on the javax.validation api, but without validators for json nullable you cannot use constraints in your api definition together with nullable)
-> which constraints do we need to support with validators (i.e. which constraints are currently supported by the openapi-java-generators)

That's an interesting point. I guess the constraint should be delegated to the wrapped object ? How does it behave with Optional ?

As far as I remeber, Optional is only supported for query-parameters via the useOptional parameter in the spring generator. Not sure if this applies here.

I'm referring to the way validation is done on Optional types generally, not specifically within openapi-generator.

Ah, i understand. So a quick research showed, that unwrapping before validation could be an approach
-> unwrap before validation with spring (see https://stackoverflow.com/questions/29943944/java-8-optional-validation-unwrapper-in-spring)
-> JSR-380 seems to support Optional out of the box ( https://jcp.org/en/jsr/detail?id=380) and they also seem to unwrap before validation

i will do some more research on this and let you know

The unwrapping approach seems to be the better direction, since it avoids duplication of the validation code. Implementation and registration of custom ValueExtractors is documented extensively in the hibernate validator docs (which is the JSR-303 / JSR-380 reference implementation): https://docs.jboss.org/hibernate/stable/validator/reference/en-US/html_single/#chapter-valueextraction

This is also how Optional is handled with javax.validation.
Implementation-wise we have implemented a ValueExtractor:

import org.openapitools.jackson.nullable.JsonNullable;

import javax.validation.valueextraction.ExtractedValue;
import javax.validation.valueextraction.UnwrapByDefault;
import javax.validation.valueextraction.ValueExtractor;

/**
 * Extractor for JsonNullable
 */
@UnwrapByDefault
public class JsonNullableValueExtractor implements ValueExtractor<JsonNullable<@ExtractedValue ?>> {
    @Override
    public void extractValues(JsonNullable<?> originalValue, ValueReceiver receiver) {
        receiver.value(null, originalValue.orElse(null));
    }
}

and registered it via the java service loader mechanism as described in the hibernate-validation docs. The solution above would require to raise the java source and target version to 1.8.

There is still the question if you find it suitable to have this facility here and add a dependency on javax.validation.* in this library. I think it's arguable, since validation will not be functional without this extractor.
Additionally we would avoid reproducing the Extractor above in every project that uses generated code with JsonNullable.

The dependency on JDK 8+ is the biggest problem for me as I would want the lib to be applicable to Android. So it should probably be done in a distinct artifact.

I understand. Unfortunately Extractors have just been introduced with Bean Validation 2.0 (JSR 380) which relies on Java 8.

Regarding Android Support: Java 8 class files/ language features are supported by the android toolchain since version 3.0.0 (see https://developer.android.com/studio/write/java8-support.html and https://developer.android.com/studio/releases/gradle-plugin.html#3-0-0).

The fact that (as said above) javax.validation is not functional out of the box if constraints are used together with nullable in the api-definition is a strong argument for finding a solution that does include the value-extractor by default. I assume this use-case will become rather common as people start utilizing JsonNullable for implementing patch APIs (for which jackson-databind-nullable is a great enhancement).

OK. After discussion with the core team, let's make this lib JDK8+ only.

  • Can you update the README to specify JDK8+ ?
  • Is it possible to set javax.validation and hibernate-validator dependencies as provided ?

Great, the docs are updated accordingly.
Regarding the scope of the javax.validation:validation-api dependency: i've set it to provided and tested it successfully with a simple maven project. Should work since the java service loader mechanism is lazy (encountered no ClassNotFoundException). I've got no Android environment around, maybe you could test the PR with an Android project.

The org.hibernate.validator:hibernate-validator / org.glassfish:javax.el dependencies are only test scoped and thus get not propagated.

PR #6 merged