JsonStreamContext "currentValue" wrongly references to @JsonTypeInfo annotated object

[aritzbastida]

Describe the bug
When a POJO attribute contains an object of a class annotated with @JsonTypeInfo, the method serializeWithType() changes the current value, so that it references that inner object when processing it (such as MailAttachment, in the example below). This behavior was introduced due to #631.

However, this current value also has a side-effect on subsequent sibling attributes. In the following sample, the JsonGenerator would state MailStatement as current value when processing "actions" and "properties". This behaviour breaks Jackson Property Filters, such as Squiggly (https://github.com/bohnman/squiggly/issues/62)

Note that if the Attachment class has no @JsonTypeInfo annotation, then "actions" and "properties" correctly state Issue object as current value.

{
	"id": "ISSUE-1",
	"issueSummary": "Dragons Need Fed",
	"issueDetails": "I need my dragons fed pronto.",
	"reporter": {
		"firstName": "Daenerys",
		"lastName": "Targaryen",
		"entityType": "User"
	},
	"assignee": {
		"firstName": "Jorah",
		"lastName": "Mormont",
		"entityType": "User"
	},
	"attachment": {
		"@class": ".MailAttachment",
		"id": "1234",
		"name": "Mail",
		"from": "from@mail.com",
		"to": "to@mail.com"
	},
	"actions": [
		{
			"id": null,
			"type": "COMMENT",
			"text": "I'm going to let Daario get this one..",
			"user": {
				"firstName": "Jorah",
				"lastName": "Mormont",
				"entityType": "User"
			}
		},
		{
			"id": null,
			"type": "CLOSE",
			"text": "All set.",
			"user": {
				"firstName": "Daario",
				"lastName": "Naharis",
				"entityType": "User"
			}
		}
	],
	"properties": {
		"email": "motherofdragons@got.com",
		"priority": "1"
	}
}

Attribute processing, together with their JsonStreamContext "currentValue":

== id: com.github.bohnman.squiggly.model.Issue@222114ba
== issueSummary: com.github.bohnman.squiggly.model.Issue@222114ba
== issueDetails: com.github.bohnman.squiggly.model.Issue@222114ba
== reporter: com.github.bohnman.squiggly.model.Issue@222114ba
== firstName: com.github.bohnman.squiggly.model.User@1283bb96
===== reporter: com.github.bohnman.squiggly.model.Issue@222114ba
== lastName: com.github.bohnman.squiggly.model.User@1283bb96
===== reporter: com.github.bohnman.squiggly.model.Issue@222114ba
== entityType: com.github.bohnman.squiggly.model.User@1283bb96
===== reporter: com.github.bohnman.squiggly.model.Issue@222114ba
== assignee: com.github.bohnman.squiggly.model.Issue@222114ba
== firstName: com.github.bohnman.squiggly.model.User@74f0ea28
===== assignee: com.github.bohnman.squiggly.model.Issue@222114ba
== lastName: com.github.bohnman.squiggly.model.User@74f0ea28
===== assignee: com.github.bohnman.squiggly.model.Issue@222114ba
== entityType: com.github.bohnman.squiggly.model.User@74f0ea28
===== assignee: com.github.bohnman.squiggly.model.Issue@222114ba
== attachment: com.github.bohnman.squiggly.model.Issue@222114ba
== id: com.github.bohnman.squiggly.model.MailAttachment@409bf450
===== attachment: com.github.bohnman.squiggly.model.MailAttachment@409bf450
== name: com.github.bohnman.squiggly.model.MailAttachment@409bf450
===== attachment: com.github.bohnman.squiggly.model.MailAttachment@409bf450
== from: com.github.bohnman.squiggly.model.MailAttachment@409bf450
===== attachment: com.github.bohnman.squiggly.model.MailAttachment@409bf450
== to: com.github.bohnman.squiggly.model.MailAttachment@409bf450
===== attachment: com.github.bohnman.squiggly.model.MailAttachment@409bf450
== actions: com.github.bohnman.squiggly.model.MailAttachment@409bf450
== properties: com.github.bohnman.squiggly.model.MailAttachment@409bf450

Version information
2.9 - 2.12.3

[cowtowncoder]

Quick question: is this during serialization (writing)?

I think I'd need a minimal reproduction: I trust there is an issue, but fixing it requires careful verification as testing of current value stability is bit limited.

[aritzbastida]

Thanks for answering!!

Yes, it happens during serialization. In fact, the issue is caused by the change made in serializeWithType() (#631). I didn't check whether there is a similar problem during deserialization, though.

As for the test case, I extended testIssue2475 (see below) so that it breaks with the error: java.lang.Error: Field 'set', context not that of Item2475 instance. Once strategy attribute is processed, the next attribute (set) does no longer refer to the Item2475 class. Note that, if we remove @JsonTypeInfo annotation from Strategy, the filter works just OK.

To fix this test, we can revert the changes in #631; that is, remove the statement gen.setCurrentValue(bean); in serializeWithType() method. As a result, we would have a consistent behavior for filters, not depending on @JsonTypeInfo any more. I checked that the whole jackson-databind test suite passes without errors. Note, however, this will break (external) custom serializers which demanded this specific behavior for polymorphic types.

package com.fasterxml.jackson.databind.ser;

public class TestCustomSerializers extends BaseMapTest
{

    @JsonFilter("myFilter")
    @JsonPropertyOrder({ "id", "strategy", "set" })
    public static class Item2475 {
    	
        private Collection<String> set;
        private Strategy strategy;
		private String id;

        public Item2475(Collection<String> set, String id) {
            this.set = set;
            this.strategy = new Foo(42);
            this.id = id;
        }

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

        public String getId() {
            return id;
        }
    }
      
    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")    
    @JsonSubTypes({ @JsonSubTypes.Type(name = "Foo", value = Foo.class) })
    interface Strategy { }

    static class Foo implements Strategy {
        public int foo;

        @JsonCreator
        Foo(@JsonProperty("foo") int foo) {
            this.foo = foo;
        }
    }     

    // [databind#2475]
    public void testIssue2475() throws Exception {
        SimpleFilterProvider provider = new SimpleFilterProvider().addFilter("myFilter", new MyFilter2475());
        ObjectWriter writer = MAPPER.writer(provider);
        
        // contents don't really matter that much as verification within filter but... let's
        // check anyway
        assertEquals(aposToQuotes("{'id':'ID-1','strategy':{'type':'Foo','foo':42},'set':[]}"),
        		writer.writeValueAsString(new Item2475(new ArrayList<String>(), "ID-1")));

        assertEquals(aposToQuotes("{'id':'ID-2','strategy':{'type':'Foo','foo':42},'set':[]}"),
        		writer.writeValueAsString(new Item2475(new HashSet<String>(), "ID-2")));
        
    }    
}

[cowtowncoder]

Thank you for adding a test case! I'll probably try to trim it down further.

As to specific place(s), I'll have to look; but serializeWithType() is defined in rather a lot of places.
In case of, say, BeanSerializerBase, I am guessing that just moving setting (which is needed in general, it cannot be just removed) after call to writeTypePrefix() might work.

[aritzbastida]

Exactly, I referred to BeanSerializerBase, specifically the lines added as a result of [databind#631].

    @Override
    public void serializeWithType(Object bean, JsonGenerator gen, SerializerProvider provider, 
       TypeSerializer typeSer) throws IOException {

        if (_objectIdWriter != null) {
            gen.setCurrentValue(bean); // [databind#631]
            _serializeWithObjectId(bean, gen, provider, typeSer);
            return;
        }
        gen.setCurrentValue(bean); // [databind#631]
        ...
    }

I look forward to your analysis. Thanks!

[cowtowncoder]

Finally found time to check this and yes, ordering of that call is incorrect -- it needs to occur after START_OBJECT gets called, as that is the context in which current value should be updated. Test verifies this.
Will be included in 2.13.0-rc1.