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

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

Closed
VictorKrapivin opened this issue Sep 9, 2019 · 10 comments
Closed

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

VictorKrapivin opened this issue Sep 9, 2019 · 10 comments

Comments

@VictorKrapivin
Copy link

VictorKrapivin commented Sep 9, 2019

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
Copy link
Member

cbornet commented Sep 9, 2019

Thanks for reporting. Would you do the fix ?

@VictorKrapivin
Copy link
Author

@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
Copy link
Member

cbornet commented Sep 10, 2019

@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 pushed a commit to VictorKrapivin/jackson-databind-nullable that referenced this issue Sep 10, 2019
@VictorKrapivin
Copy link
Author

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

cbornet added a commit that referenced this issue Sep 10, 2019
…defined

Issue #12 - No need to validate values for JsonNullable.undefined()
@cbornet
Copy link
Member

cbornet commented Sep 10, 2019

Closed by #13

@cbornet cbornet closed this as completed Sep 10, 2019
@VictorKrapivin
Copy link
Author

@cbornet Any plans for release?

@VictorKrapivin
Copy link
Author

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

@jmini
Copy link
Member

jmini commented Dec 18, 2019

@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
Copy link
Member

jmini commented Dec 19, 2019

Version 0.2.1 was released!

@VictorKrapivin
Copy link
Author

@jmini Thanks!!

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

No branches or pull requests

3 participants