Possible problem with `NON_EMPTY` exclusion, `int`s, `Strings`

[cowtowncoder]

(from FasterXML/jackson-module-afterburner#55)

It appears like default handling might not work as expected with 2.5.4, whereas Afterburner does seem to handle things better. Need to investigate, and also see if 2.6.0-rc3 works better.

[cowtowncoder]

Yes, looks like empty checking does work for Strings, but not for numbers.

[JoergM]

Although this is a closed issue I comment here to provide the context.

Can you please motivate why Zero as a number is considered empty? This just caused a production bug at our side after an update to 2.6.2. Zero can be a perfectly valid value in a business context. But because of this change it got filtered out.

We now changed the JsonInclude option to ALWAYS. This fixes the issue but does not feel like a proper solution. Are there any alternatives to handle a situation where Zero is valid but I want to filter out other empty values like an empty string?

[cowtowncoder]

@JoergM First of all, apologies for breakage: we did not anticipate this to lead to issues, since in absence of value, such default values should be used anyway: for numbers, 0 is the default value for int for example.

The original intent of NON_EMPTY definition includes empty Collections, Strings, and primitives with default values (numbers as boolean value of false, but not all serializers implemented these rules as originally intended.
Note too that there is nothing invalid in "empty"; empty Collections may be perfectly valid as well: the main use for NON_EMPTY is to reduce size of JSON Objects by removing what are thought to be unnecessary entries (ones for which default value: for Objects, null, for primitives Java default -- Collections and arrays are sort of outliers actually)

As a work-around, note that you can annotate both classes (to give default inclusion for its properties) and individual properties (as overrides). This may make most sense on short term.

For longer term there is a plan to also allow use of custom inclusion definition, since it is impossible to come up with a short list of criteria that would work for all of users.

[Sovietaced]

I will hold off on updating our pom WRT jackson until the longer term solution is available.

[talawahtech]

I just got hit by this as well. It also seems counter-intuitive to me to treat zero valued ints and doubles as "empty". Furthermore based on my tests a zero valued Double (object not primitive) is also treated as empty which is very perplexing since Double doesn't have a default value (other than null).

So if you serialize a Double whose value is 0.0 using JsonInclude.Include.NON_EMPTY the value will disappear entirely in serialized form, and then when you attempt to deserialize the output, the deserialized value will be null.

Also consider a corner case where the field is a string, but the getter returns a double.

private String riskScore;

public double getRiskScore(){
    return Double.parseDouble(riskScore);
}

Using the default serialization (or a pre-2.6 version of jackson) a string value of "0.0" would be serialized to a Json Number (0.0) and then deserialized back to the original string without issue. However now the 0.0 double is thrown away during serialization and then after serialization you end up with a null string.

I am all for making things more flexible with a custom inclusion definition, but these new defaults of discarding ints and doubles definitely don't seem intuitive to me. I feel like it could lead to a lot of head scratching and trigger bugs that don't show up right away.

[cowtowncoder]

At this point I think the best course for 2.7 would be to see if it is possible to make NON_DEFAULT work so that it would take on current behavior of empty (with wide applicability), and revert NON_EMPTY to only include null (like NON_NULL), empty String (length 0), Map (size 0), Collection (size 0) and arrays (length 0).

While I understand that the change is non-intuitive to many, due to missing handling in earlier versions, behavior is what it is for 2.6.x and I do not want to risk further confusion by variation within patch levels.

I will file a new issue (with reference to this one), since this issue is for original fix for missing handling, and partial revert needs to be a separate change (as it also contains related change to NON_DEFAULT, as per above).

[cowtowncoder]

As per above, created #952 to track work for 2.7.

[knightweb]

Hi @cowtowncoder - I think this issue has to be considered a critical bug. Ideally a version 2.6.x would come out with a fix, reverting to the previous behaviour of NON_EMPTY. I think the fundemental concept of NON_EMPTY has been misunderstood. NON_EMTPY is just not a concept that applies to real primitives, they always have a default value. So with auto-unboxing it is imperitive to not apply NON_EMPTY to the boxed versions. Version prior to 2.6.0 cope with this correctly.

Below is a fully runnable example, hopefully illustrating the seriousness of this change. I've also included some additional fields, that show why simply removing the NON_EMPTY serialisation option is not the solution - as you can't keep the same payload structure between version 2.5.4 and 2.6.

package com.testcomp;

import java.util.ArrayList;
import java.util.List;

import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.databind.ObjectMapper;

public class PrimitiveHandlingIssue {

    public static class IntegerIssue {      

        public IntegerIssue(){}

        private List<String> shouldntSerialise;
        private String alsoShouldntSerialise;

        private Integer mustSerialise;

        public Integer getMustSerialise() {
            return mustSerialise;
        }

        public void setMustSerialise(Integer mustSerialise) {
            this.mustSerialise = mustSerialise;
        }

        public List<String> getShouldntSerialise() {
            return shouldntSerialise;
        }

        public void setShouldntSerialise(List<String> shouldntSerialise) {
            this.shouldntSerialise = shouldntSerialise;
        }

        public String getAlsoShouldntSerialise() {
            return alsoShouldntSerialise;
        }

        public void setAlsoShouldntSerialise(String alsoShouldntSerialise) {
            this.alsoShouldntSerialise = alsoShouldntSerialise;
        }

    }   

    /**
     * Run this test on both version 2.5.4 and 2.6.0 to see the behaviour difference.
     * @param args
     * @throws Exception
     */
    public static void main(String[] args) throws Exception {

        ObjectMapper mapper = new ObjectMapper();
        //Comment the line below to see the behaviour of this field across version 2.5.4 and 2.6.0.
        //see how it behaves correctly in 2.5.4.  In 2.6.0 a fundemental error has been made.
        //NON_EMPTY is not a concept that can apply to real primitives, as they always have a default, and you can't tell whether they where set to the default or not.
        mapper.setSerializationInclusion(Include.NON_EMPTY);

        PrimitiveHandlingIssue.IntegerIssue phiBefore = new PrimitiveHandlingIssue.IntegerIssue();
        phiBefore.setMustSerialise(0);

        phiBefore.setShouldntSerialise(new ArrayList<String>());


        String js = mapper.writeValueAsString(phiBefore);
        System.out.println(js);

        PrimitiveHandlingIssue.IntegerIssue phiAfter = mapper.readValue(js, PrimitiveHandlingIssue.IntegerIssue.class);
        //Here is the now unavoidable NULL POINTER caused by this bug.
        System.out.println(phiAfter.getMustSerialise() * 2);
    }

}

Currently I'm struggle to upgrade from version 2.4.1 to 2.6.+ - as I can't find a version that doesn't have some form of bug/odd feature. I'm trying to upgrade to get the JsonTypeInfo.As.EXISTING_PROPERTY but have so far hit 2 major reasons not to use 2.6.+. Unfortunately version 2.5+ are also unusable as they do not allow multiple named types refering to the same subclass (something fixed in version 2.6).

Would it be possible to have a 2.6.x patch for this issues (too many other issues/major changes with 2.7 at the moment to conisder making that leap).

[cowtowncoder]

@knightweb Unfortunately that is the behavior for 2.6 and will not be changed at this point. There are multiple 2.6.x patch versions, and at this point there would be more confusion here than seems worth.

One potential possibility would be a patch for 2.5, if anyone has time to backport fix for allowing multiple named types; I could release 2.5.5-1 micro-patch for that. I don't have time to investigate that myself.

[knightweb]

Yes perhaps backporting that JsonTypeInfo fix would be the best solution as there are other blockers to a 2.6 upgrade for me. I'll take a look at the code, if small enough I could perhaps apply it as an aspect. (i'm guessing I couldn't contribute just now).

I see version 2.7 has a fix for this issue but no longer supports Java 1.6 when deserializing. I assume this is intentional and a move to JDK1.7 will be mandatory for anyone upgrading to jackson-core 2.7?

[cowtowncoder]

@knightweb Thank you for your help here -- and sorry for the combination of blockers. It is unfortunate that your use case happens to touch these disparate items, making upgrade much more difficult than it should be.

As to 2.7, it is sort of half-way version. 2 core components -- jackson-annotations and jackson-core should still only require Java 6, whereas others will require Java 7. But even with that:

1. Java 7 should only be needed to build Jackson 2.7 components (other than annotations, streaming)
2. Java 6 runtime should be enough to use jackson-databind, and most other modules; exceptions are modules for datatypes not included in Java 6 (like Java 8 datetime (jsr-310)) or that require Java 7 (HPPC)
3. No Java 7 language constructs are yet used (they will be used in 2.8)
4. Java 7 datatypes supported by jackson-databind are all dynamically loaded, and should downgrade gracefully (there is just 1 new value type, and 2 annotation types)

So... it's bit unclean from Maven perspective, but the intent is for 2.7 to still run on Java 6, but include Java 7 support dynamically.

[crumbpicker]

I have updated my Jackson dependency to 2.6.5 and still have an issue with the serialization of zero values when using the JsonInclude.Include.NON_EMPTY config property.
Could you please notify here from which version of Jackson the behavior of version 2.5.x has been restored ?
Thanks !