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

No need to validate values for JsonNullable.undefined() #12

Closed VictorKrapivin closed 5 years ago

VictorKrapivin commented 5 years ago

At the moment value initialized as undefined() is treated as null within validation.

Following code demonstrates issue:

    public class TestObject {
        @NotNull @Size(max = 3)
        private JsonNullable<String> restrictedString = JsonNullable.undefined();

        public void setRestrictedString(String value) {
            restrictedString = JsonNullable.of(value);
        }
    }
TestObject testObject = new TestObject();
Set<ConstraintViolation<TestObject>> violations = validator.validate(testObject);
assertEquals(0, violations.size());

Here violations should be empty, because JSR380 constraints address value within JsonNullable container, not container itself. So testObject is valid. But actual behavior gives violations.

cbornet commented 5 years ago

Thanks for reporting. Would you do the fix ?

VictorKrapivin commented 5 years ago

@cbornet

Would you do the fix ?

Hi, sure. But I'm not able to create branch in this repository. Actually my suggestion is to fix it like that:

@UnwrapByDefault
public class JsonNullableValueExtractor implements ValueExtractor<JsonNullable<@ExtractedValue ?>> {
    @Override
    public void extractValues(JsonNullable<?> originalValue, ValueReceiver receiver) {
        if (originalValue.isPresent()) {
            receiver.value(null, originalValue.get());
        }
    }
}
cbornet commented 5 years ago

@VictorKrapivin to submit a PR, you need to fork the repo, create a branch in the fork, then you can PR with the help of GitHub. See details here.

VictorKrapivin commented 5 years ago

@cbornet Ok, thanks for explanation! PR #13 ready for review

cbornet commented 5 years ago

Closed by #13

VictorKrapivin commented 5 years ago

@cbornet Any plans for release?

VictorKrapivin commented 4 years ago

@cbornet Hi! Is it possible to release this fix in 0.2.1?

jmini commented 4 years ago

@VictorKrapivin, @cbornet:

If I remember well, I have published the last release to maven central.

I am waiting for feedback, just to be sure. The idea is to create a 0.2.1 release and bump master to 0.2.2-SNAPSHOT?

jmini commented 4 years ago

Version 0.2.1 was released!

VictorKrapivin commented 4 years ago

@jmini Thanks!!