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

Required primitive properties #3392

Closed
acanthite1855 opened this issue Feb 4, 2022 · 9 comments
Closed

Required primitive properties #3392

acanthite1855 opened this issue Feb 4, 2022 · 9 comments
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed

Comments

@acanthite1855
Copy link

acanthite1855 commented Feb 4, 2022

Is your feature request related to a problem? Please describe.
I have a request model containing IDs which are numbers (long),
In order to make them required I need to make them nullable (Long) and add @JsonSettter(nulls = Nulls.FAIL)

There is DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES, but it only works when property in JSON explicitly set to null. But when the property itself is missing it does not work, but simply sets the value to 0

Describe the solution you'd like
DeserializationFeature.FAIL_ON_MISSING_PROPERTIES and/or @JsonProperty(required = true) to work for primitives as well.

@acanthite1855 acanthite1855 added the to-evaluate Issue that has been received but not yet evaluated label Feb 4, 2022
@cowtowncoder
Copy link
Member

One quick note: FAIL_ON_MISSING_PROPERTIES only works when passing things via Creator method (constructor or factory method) -- but if you do that, it does work anything required.
Conversely "required" check is never made for properties set via setter or field; there is a highly voted issue (#230 I think)) to implement support for those checks too but no current activity in implementing it.

There is nothing particular about primitives as far as I know, however.

@k163377
Copy link
Contributor

k163377 commented Sep 10, 2023

This behavior is also a problem in KotlinModule.
Is it possible to prevent PropertyValueBuffer::getParameter from returning a default value if the SettableBeanProperty is a primitive type and the input is null or undefined?

I think a failure should occur for unexpected input.

@k163377
Copy link
Contributor

k163377 commented Sep 10, 2023

I will share the workaround I have found.
If the argument is a primitive type, tell Jackson to use the corresponding wrapper type, so that if the input is nullish the read result will be null.

The following is an example of how this can be accomplished by implementing an AnnotationIntrospector (note that this is just a Java adaptation of a prototype for a KotlinModule, and has not been tested to work for all use cases).

import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.introspect.Annotated;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;
import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector;
import java.lang.Class;

public class AI extends NopAnnotationIntrospector {
    @Override
    public JavaType refineDeserializationType(
            final MapperConfig<?> config,
            final Annotated a,
            final JavaType baseType
    ) {
        if (a instanceof AnnotatedParameter && baseType.isPrimitive()) {
            Class<?> primitiveType = baseType.getRawClass();
            return config.getTypeFactory().constructType(TODO("convert primitiveType to wrapperType"));
        } else {
            return baseType;
        }
    }
}

@cowtowncoder
Copy link
Member

@k163377 I think your problem is slightly different from original authors.

I am not sure I follow request on PropertyValueBuffer, at least not completely.
For case of required property, missing value will trigger failure.
For other cases, a usable default is indeed given.

But I guess one possible corner-case that could be addressed is that of:

  1. Primitive valued-constructor parameter
  2. DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES enabled
  3. No value passed to (1)

which should result in failure. If not, that's a bug.

Otherwise I think you mentioned that supporting Kotlin's nullability aspects is difficult -- this is true. But I think separate issue(s) are needed; I think you have already filed some?

@k163377
Copy link
Contributor

k163377 commented Sep 19, 2023

Oh, I must have been mistaken.
I thought the problem with the deserialization of dto1 in the sample code below was that no exception was thrown.

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;

public class Github3392 {
    public static class Dto {
        public final int foo;
        public final int bar;
        public final String baz;
        public final String qux;

        @JsonCreator
        public Dto(
                @JsonProperty(value = "foo", required = false) int foo,
                @JsonProperty(value = "bar", required = true) int bar,
                @JsonProperty(value = "baz", required = false) String baz,
                @JsonProperty(value = "qux", required = true) String qux
        ) {
            this.foo = foo;
            this.bar = bar;
            this.baz = baz;
            this.qux = qux;
        }
    }

    @Test
    public void test() throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();

        // not fail, foo:0, bar:0, baz:null, qux: null
        Dto dto1 = mapper.readValue("{\"foo\":null,\"bar\":null,\"baz\":null,\"qux\":null}", Dto.class);
        // fails
        Dto dto2 = mapper.readValue("{}", Dto.class);
    }
}

On the other hand, this isssue is about setters and not about creators, right?

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 20, 2023

It seems to me that the second case should (and does) throw an exception, but the first case has values for all properties so it should pass. Nothing in settings forbids nulls, unless DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES is enabled (it isn't by default).

And when enabling that setting, exception is indeed thrown.
So this code example does not show a problem as far as I can see.

But the original report is too vague to specify actual problem -- there is no reproduction, just general description lacking info on whether setters or constructor is used (for example).
My guess is that it may well be using setter or field, in which case absent value will not be notice (mutators are only called if there IS a value to set -- nothing tracks potential properties that are not found, except when using Creators when such tracking is needed anyway).

@cowtowncoder cowtowncoder added the need-test-case To work on issue, a reproduction (ideally unit test) needed label Sep 20, 2023
@cowtowncoder
Copy link
Member

Ok, since I can not reproduce reported issue, will close -- may be re-opened with actual reproduction.

But anyone finding this issue please note that you need to use Creator (with @JsonCreator) for checks to work as expected.

@Boeing1337
Copy link

Kotlin
data class DTO (
val value: Long
)
with request boby = {} I Receive 0 in the value even with FAIL_ON_NULL_FOR_PRIMITIVES

@cowtowncoder
Copy link
Member

@Boeing1337 for Kotlin-specific issues, please file on jackson-module-kotlin issue tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed
Projects
None yet
Development

No branches or pull requests

4 participants