Nulls property metadata not working for records

[cocorossello]

Describe the bug
Nulls.AS_EMPTY does not work for JDK14 records, it deserializes null as null and should be an empty list

Version information
2.12.0

To Reproduce

    record SomeRecord(
            @JsonSetter(contentNulls = Nulls.AS_EMPTY, nulls = Nulls.AS_EMPTY)
            List<String> someCollection
    ) {
    }

    @Test
    void collection_is_null() throws JsonProcessingException {
        var mapper = new ObjectMapper();
        mapper.setDefaultSetterInfo(JsonSetter.Value.construct(Nulls.AS_EMPTY, Nulls.AS_EMPTY));
        assertThat(mapper.readValue("{}", SomeRecord.class).someCollection(), is(nullValue()));
    }

Expected behavior
mapper.readValue("{}", SomeRecord.class).someCollection() should be an empty list

Additional context
I think it's a bug, I've tried to dig a bit in the code but I have no clue. If you can point me in some direction maybe I can try to arrange a PR

[cocorossello]

Sorry, this is how it should work.

However, I wold like to do something like

class SomeClass { List<String> someCollection = new ArrayList<>(); }
So someCollection is never null.

Is there any way to achieve this with records?

[cowtowncoder]

@cocorossello Hmmh. Interesting; since properties are passed via constructor arguments, I think the null coercion should actually work -- if you created POJO with @JsonCreator annotated constructor, that should happen I think. So with quick look this looks like a bug to me.
(note: this only applies to constructor-passed argument -- otherwise the problem is that no setter is called since value is completely missing -- something to ideally improve in future, but for now that's the way it works).

I hope I'll find some time to look into this; code is not necessarily easy to follow but the method to look for in BasicDeserializerFactory -- _addRecordConstructor(...) -- might give some clues.

I would be happy to merge a PR for failing test under src/test-jdk14 (against branch 2.12); esp. if you could contrast POJO equivalent where constructor is used.

[cocorossello]

Sorry, I think I didn't explain well. That test does work and is the same behaviour with regular classes, the issue is not valid (see my second comment), I should have reopened another issue with the question.

The problem is that I would like to have always non null collection fields. I can achieve this easily with regular classes:

class SomeClass { List<String> someCollection = new ArrayList<>(); }

The problem is that I'm not able to achieve this with records. How can I do that?

[cowtowncoder]

@cocorossello Hmmmh. For Collections and Maps (and even POJOs, I think?), use of Nulls.AS_EMPTY should still work -- when passed via constructor, implicit null from missing value should do that. What "empty" means is defined by JsonDeserializer.getEmptyValue(). Many types have such default value, including wrappers... but not everything has.

Or do you mean that Record itself should similarly have default value to use, instead of null (for defaulting)? That one would not work, except for theoretical "no fields" record (which seems like a useless thing), given that it is not clear how to create such "empty" instance.

[cowtowncoder]

Oh, on implementation: Records do allow definitions of constructors and so you could do null replacement there (can use static helper methods for optional null replacement). That is bit more work for sure but seems doable, although I feel I am still missing part of the question.

[cocorossello]

Ok, thanks, I missed the constructor options for records. So I can make it work like:

record SomeRecord( List<String> someCollection ) { SomeRecord { someCollection = someCollection == null ? new ArrayList<>() : someCollection; } }

That's enough for me, sorry for the confusion and thanks for your response.

[cowtowncoder]

@cocorossello No problem! I am just eager to find out if there are edge cases, given that Record handling is brand new feature, and users are pretty good at exploring full set of things one might want to use. Compared to my bare-bones testing. :)

[cocorossello]

Hi, I see that you have pushed a failing test. To be honest I didn't do it because I think it's the same behaviour with classes, so it shouldn't be a bug, right?. This one also fails:

  static class SomeClass {
        @JsonSetter(nulls= Nulls.AS_EMPTY) List<String> names;
        @JsonSetter(nulls=Nulls.FAIL) Map<String, Integer> agesByNames;
    }


    public void testDeserializeWithNullAsEmpty2() throws Exception {
        final ObjectReader r = MAPPER.readerFor(SomeClass.class);
        SomeClass value = value = r.readValue(a2q("{'agesByNames':{'bob':42}}"));
        assertNotNull(value.names);

    }

[cowtowncoder]

@cocorossello Ah. But that is actually not the same. That was what I was trying to say earlier: there is difference between using constructor as setter vs using field or setter method -- only constructor acts on "missing" value (because it must have SOME value to pass anyway, so it keeps track of value).

So if you change the test to have constructor that takes values (annotation may be on field, setter or constructor parameter, as long as names match [and you use parameter-names module or @JsonProperty for constructor parameters)), it should work.
There is a test for POJO case (I just added that before java14 test)

Note that there is #230 to request solving the problem of field/setter method not enforcing required setting: if that gets implemented, null-coercion should also work (or be made to work relatively easily).

I hope this makes more sense? Thank you for reporting the first case!

[cocorossello]

Ok, I understood.

The thing that confused me is that even using parameter-names module (and ensuring that class files has the parameter names), I still need the @JsonCreator annotation, I was trying without it and couldn't get to work (and it is using the constructor)

   static class SomeClass {
        @JsonSetter(nulls= Nulls.AS_EMPTY) List<String> names;
        @JsonSetter(nulls=Nulls.FAIL) Map<String, Integer> agesByNames;

        public SomeClass(List<String> names, Map<String, Integer> agesByNames) {
            this.agesByNames = agesByNames;
            this.names = names;
        }
    }

    @Test
    public void testDeserializeWithNullAsEmpty2() throws Exception {
        var mapper = new ObjectMapper();
        mapper.registerModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES));
        var params = SomeClass.class.getConstructors()[0].getParameters();
        assertEquals(params[0].getName(), "names"); //Works
        var value = mapper.reader().readValue("{\"agesByNames\":{\"bob\":42}}", SomeClass.class);
        assertNotNull(value.names); //Fails
    }

Anyway, thank you very much for the explanation, I'll try to look into it.

[cowtowncoder]

@cocorossello I think I know this one -- if you do NOT annotate constructor, it can still be found BUT since linking between field and constructor does not exist (longer discussion why; it's a bug but probably cannot fix for 2.x; only for 3.0 after full rewrite of introspection), and as such annotations are not transferred.

Because of this, just in this case you MUST add annotation on constructor parameter; they are not merged from field. If you do that, null-handling will work.
Alternative annotation with just plain @JsonCreator will also work: this because constructor is now explicitly found and linkage will exist.
There is an issue filed for this one, old one, and failing test under "failing/" somewhere.

[cocorossello]

I can see that the problem with records is that _beanProperties has the correct nullProvider and creatorProps does not. So one dirty fix (and for sure it will not work in all cases) is to merge that nullProvider. Something like:

BeanDeserializerBase.java:590

        if(this._beanType.isRecordType()) {
            for(int index=0;index<_beanProperties.size();index++) {
                var beanProperty = _beanProperties.find(index);
                for(int j=0;j<creatorProps.length;j++) {
                    if(beanProperty.getName().equals(creatorProps[j].getName())) {
                        creatorProps[j] = creatorProps[j].withNullProvider(beanProperty.getNullValueProvider());
                    }
                }
            }
        }

But of course this does not cover all cases..