StringCollectionSerializer calls JsonGenerator.setCurrentValue(value), which messes up current value for sibling properties

[bohnman]

Versions: 2.9.9 and 2.10.0.pr3

Given the following class definition:

public static class Item {
        private Collection<String> set;
        private String id;

        public Item(String id) {
            this.set = new HashSet<>();
            this.id = id;
        }

        public Collection<String> getSet() {
            return set;
        }

        public String getId() {
            return id;
        }
    }

You'll notice that when the serializer writes out the set and id properties, the current value on jgen is HashSet not the Item. I write a Jackson filtering library and I count on the current value being correct.

Doing some digging, I noticed that the StringCollectionSerializer sets the currentValue in the serialize method like so:

@Override
    public void serialize(Collection<String> value, JsonGenerator g,
            SerializerProvider provider) throws IOException
    {
        g.setCurrentValue(value);
        final int len = value.size();
        if (len == 1) {
            if (((_unwrapSingle == null) &&
                    provider.isEnabled(SerializationFeature.WRITE_SINGLE_ELEM_ARRAYS_UNWRAPPED))
                    || (_unwrapSingle == Boolean.TRUE)) {
                serializeContents(value, g, provider);
                return;
            }
        }
        g.writeStartArray(len);
        serializeContents(value, g, provider);
        g.writeEndArray();
    }

However, the current value is not explicity set in IndexedStringListSerializer nor CollectionSerializer, and consequently they both don't exhibit this problem

I have attached a java class that exhibits this behavior as long as you're using Jackson version 2.9.9 or later
Test.java.zip

[cowtowncoder]

Thank you for reporting the issue. Sounds like a bug.

Since 2.10.0 is now being released (and 2.10.0.pr3 having been available), it would be interesting to see if there is any change in behavior: I hope to add a unit test once I have time to investigate this.

[cowtowncoder]

Ok. I must admit I am not sure I fully follow what would be the expected behavior here, at every given point. I can see there is a discrepancy, but I am not sure which serializers are doing the right thing. Perhaps StringCollectionSerializer is calling method too early (and should do it after writeStartArray()? Conversely it seems that other serializer(s) are missing needed call...

[cowtowncoder]

Yes, I think call needs to be right after writeStartArray(), to nest as expected. "current value" is, I think, more accurately "current structured value" I guess...

[bohnman]

@cowtowncoder Can the call to setCurrentValue be moved to serializeContents? This is what IndexedStringListSerializer and CollectionSerializer do. I agree that it needs to be after writeStartArray because that call creates a new context just for the array and doesn't overwrite the current context for the array's "parent", which in my example is Item

[cowtowncoder]

I think I'll just start using new (in 2.10) writeStartArray() methods that will do setting.

My main concern is adding unit tests to try to sort of define expected behavior: the whole "current value" concept is (alas) quite undefined; so while fix itself looks easy enough, it'd be good to both find other cases (I'm sure they exist) and to guard against otherwise possible regressions from refactoring.