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

Serialize by reference - used to work with 1.9.11 #539

Closed
fabienrenaud opened this issue Sep 12, 2014 · 15 comments
Closed

Serialize by reference - used to work with 1.9.11 #539

fabienrenaud opened this issue Sep 12, 2014 · 15 comments
Milestone

Comments

@fabienrenaud
Copy link

Code:

@Test
public void testNoReadingFullWriting() {
    String json = "{\"a\":100,\"b\":\"bb\",\"x\":-100,\"y\":\"YY\",\"z\":314,\"obj\":{\"id\":100}}";
    BigData0 obj = JsonUtils.fromJson(json, BigData0.class);
    assertNotNull(obj);
    assertNotNull(obj.getTree());
    assertEquals(6, obj.getTree().size());

    String str = JsonUtils.toJson(obj);
    assertEquals(json, str);
    assertNotNull(obj.getTree());
}

private static class BigData0 extends JsonContainer {
}

public abstract class JsonContainer {

    private ObjectNode tree;

    protected ObjectNode getTree() {
        return tree;
    }

    protected void setTree(ObjectNode tree) {
        this.tree = tree;
    }
}

Then i have a jackson module and method (in JsonUtils) that check when an object is an instance of JsonContainer and when it is, it is supposed to serialize and deserialize in a special (deserialization set the tree attribute, serialization uses the module that extends BeanSerializer and overrides serializeFields...

Problem is that with jackson 2 now, when the class doesn't have any attributes, it just uses the unknown deserializer by default... and skips checking whether a bean serializer might be available...

I understand this is a very brief, possibly incomprehensible explanation... The entire thing solution i had to write in jackson 1 is on stackoverflow though:
http://stackoverflow.com/questions/17967531/jackson-api-partially-update-a-string

In short: the whole problem was to deserialize a huge json blob with only a partial knowledge/class representation of its content but still be able to save the entire thing back.
I wish jackson supports that in the first place.

@cowtowncoder
Copy link
Member

One problem I see is this: there is no public accessor, nor @JsonProperty, so no properties are discovered. Adding @JsonProperty for getter, setter or field should let tree property be discovered.
Behavior in 1.9 should be the same as with 2.x, although I am not sure why earlier versions would differ (property detection code was, however, completely rewritten between 1.7 and 1.9 versions).

@cowtowncoder
Copy link
Member

Another problem is that JsonUtils is missing, so I can not fully reproduce code above.
But assuming it would use simple readValue/writeValue, problem is that incoming JSON does not match expected structure; JsonContainer would expect tree as property for main JSON Object, and only under that have the tree.

So I guess what I really need is more complete test case; plus, above comment for missing @JsonProperty (or public getter) is also relevant here.

@fabienrenaud
Copy link
Author

Full executable example:

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonGenerationException;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.Module;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationConfig;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.ser.BeanPropertyWriter;
import com.fasterxml.jackson.databind.ser.BeanSerializer;
import com.fasterxml.jackson.databind.ser.BeanSerializerModifier;
import com.fasterxml.jackson.databind.ser.std.BeanSerializerBase;
import java.io.IOException;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class JsonBug1 {

    public static void main(String[] args) {
        String json = "{\"a\":100,\"b\":\"bb\",\"x\":-100,\"y\":\"YY\",\"z\":314,\"obj\":{\"id\":100}}";
        Foo obj = JsonUtils.fromJson(json, Foo.class);
        assert obj != null;
        assert obj.getTree() != null;
        assert obj.getTree().size() == 6;

        String str = JsonUtils.toJson(obj);
        if (!json.equals(str)) {
            throw new IllegalStateException();
        }
        assert obj.getTree() != null;
    }

    private static class Foo extends JsonContainer {
    }

    private static final class JsonUtils {

        private static final ObjectMapper JACKSON_MAPPER = new ObjectMapper();
        private static final JsonSerializer<JsonContainer> JSON_EMTPY_CONTAINER_SERIALIZER = new JsonSerializer<JsonContainer>() {
            @Override
            public void serialize(JsonContainer value, JsonGenerator jgen, SerializerProvider provider) throws IOException {
                jgen.writeTree(value.getTree());
            }
        };

        static {
            JACKSON_MAPPER.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);

            JACKSON_MAPPER.disable(
                    MapperFeature.AUTO_DETECT_CREATORS,
                    MapperFeature.AUTO_DETECT_FIELDS,
                    MapperFeature.AUTO_DETECT_GETTERS,
                    MapperFeature.AUTO_DETECT_IS_GETTERS,
                    MapperFeature.AUTO_DETECT_SETTERS,
                    MapperFeature.USE_GETTERS_AS_SETTERS
            );

            JACKSON_MAPPER.setSerializationInclusion(JsonInclude.Include.NON_NULL);

            JACKSON_MAPPER.registerModule(new JsonContainerModule());
        }

        private JsonUtils() {
        }

        public static ObjectMapper getMapper() {
            return JACKSON_MAPPER;
        }

        /**
         * Maps the given Java Object to a JSON string
         */
        public static String toJson(final Object obj) {
            String json = null;
            try {
                json = JACKSON_MAPPER.writeValueAsString(obj);
            } catch (IOException ex) {
                ex.printStackTrace();
            }
            return json;
        }

        public static <T> T fromJson(final String text, final Class<T> clazz) {
            T obj = null;
            try {
                Class superClass = clazz.getSuperclass();
                if (superClass != null && JsonContainer.class.isAssignableFrom(superClass)) {
                    JsonNode node = JACKSON_MAPPER.readTree(text);
                    obj = JACKSON_MAPPER.convertValue(node, clazz);
                    ((JsonContainer) obj).setTree((ObjectNode) node);
                } else {
                    obj = JACKSON_MAPPER.readValue(text, clazz);
                }
            } catch (Exception ex) {
                ex.printStackTrace();
            }

            return obj;
        }

        public static <T> T fromJson(final String text, final Type type) {
            T obj = null;
            try {
                obj = JACKSON_MAPPER.readValue(text, getJavaType(type));
            } catch (IOException ex) {
                ex.printStackTrace();
            }
            return obj;
        }

        private static JavaType getJavaType(final Type type) {
            if (type.getClass() == Class.class) {
                Class clazz = (Class) type;
                if (clazz.isArray()) {
                    throw new IllegalArgumentException("JSON arrays as Java arrays[] are not supported. Use List or Set instead.");
                } else {
                    return JACKSON_MAPPER.getTypeFactory().constructType(type);
                }
            } else if (type instanceof ParameterizedType) {
                ParameterizedType pType = (ParameterizedType) type;
                if (pType.getRawType() == List.class) {
                    return JACKSON_MAPPER.getTypeFactory().constructCollectionType(List.class, (Class) pType.getActualTypeArguments()[0]);
                } else if (pType.getRawType() == Set.class) {
                    return JACKSON_MAPPER.getTypeFactory().constructCollectionType(Set.class, (Class) pType.getActualTypeArguments()[0]);
                } else {
                    throw new IllegalArgumentException("Unknown/unsupported parametrized type: " + pType.getRawType().getClass().getCanonicalName());
                }
            } else {
                throw new IllegalArgumentException("Unknown/unsupported type: " + type.getClass().getCanonicalName());
            }
        }

        private static class JsonContainerModule extends Module {

            @Override
            public String getModuleName() {
                return "JsonContainerModule";
            }

            @Override
            public Version version() {
                return new Version(0, 1, 0, "SNAPSHOT");
            }

            @Override
            public void setupModule(Module.SetupContext context) {
                context.addBeanSerializerModifier(new BeanSerializerModifier() {
                    @Override
                    public JsonSerializer<?> modifySerializer(SerializationConfig config, BeanDescription beanDesc, JsonSerializer<?> serializer) {
                        if (JsonContainer.class.isAssignableFrom(beanDesc.getBeanClass())) {
                            return serializer == null
                                    ? JSON_EMTPY_CONTAINER_SERIALIZER
                                    : new JsonContainerSerializer((BeanSerializerBase) serializer);
                        }
                        return serializer;
                    }
                });
            }
        }

        private static class JsonContainerSerializer extends BeanSerializer {

            public JsonContainerSerializer(BeanSerializerBase src) {
                super(src);
            }

            @Override
            protected void serializeFields(Object b, JsonGenerator jgen, SerializerProvider provider) throws IOException, JsonGenerationException {
                JsonContainer bean = (JsonContainer) b;
                final BeanPropertyWriter[] props;
                if (_filteredProps != null && provider.getActiveView() != null) {
                    props = _filteredProps;
                } else {
                    props = _props;
                }
                int i = 0;
                try {
                    for (final int len = props.length; i < len; ++i) {
                        BeanPropertyWriter prop = props[i];
                        if (prop != null) { // can have nulls in filtered list
                            prop.serializeAsField(bean, jgen, provider);
                            if (bean.getTree() != null) {
                                bean.getTree().remove(prop.getName());
                            }
                        }
                    }
                    if (_anyGetterWriter != null) {
                        _anyGetterWriter.getAndSerialize(bean, jgen, provider);
                    }
                    if (bean.getTree() != null) {
                        Iterator<Map.Entry<String, JsonNode>> it = bean.getTree().fields();
                        while (it.hasNext()) {
                            Map.Entry<String, JsonNode> e = it.next();
                            jgen.writeFieldName(e.getKey());
                            jgen.writeObject(e.getValue());
                        }
                    }
                } catch (Exception e) {
                    String name = (i == props.length) ? "[anySetter]" : props[i].getName();
                    wrapAndThrow(provider, e, b, name);
                } catch (StackOverflowError e) {
                    /* 04-Sep-2009, tatu: Dealing with this is tricky, since we do not
                     *   have many stack frames to spare... just one or two; can't
                     *   make many calls.
                     */
                    JsonMappingException mapE = new JsonMappingException("Infinite recursion (StackOverflowError)", e);
                    String name = (i == props.length) ? "[anySetter]" : props[i].getName();
                    mapE.prependPath(new JsonMappingException.Reference(bean, name));
                    throw mapE;
                }
            }
        }
    }

    private abstract class JsonContainer {

        private ObjectNode tree;

        protected ObjectNode getTree() {
            return tree;
        }

        protected void setTree(ObjectNode tree) {
            this.tree = tree;
        }
    }

}

I can't annotate getTree or tree because it's actually not a json property at all but the cache of the entire json blob which I deserialized before.
For more context on why I had to write all that crap and that was the problem I had to solve, see my post on stackoverflow:
http://stackoverflow.com/questions/17967531/jackson-api-partially-update-a-string

Before attempting to upgrade to Jackson 2.4, i was on 1.9.11.

@fabienrenaud fabienrenaud changed the title Serialize by reference - used to work with 1.x Serialize by reference - used to work with 1.9.11 Sep 12, 2014
@cowtowncoder
Copy link
Member

I am sorry but example is big enough that I can't follow on what it is really trying to achieve, or where it goes wrong. I don't understand reference to "just uses empty deserializer": empty bean handling (as special case) only applies to serialization; but even there, custom serializers are checked first.

So at this point I don't quite understand what your problem is, or what to do with it.

@fabienrenaud
Copy link
Author

I described the original problem in a different ticket: #549

The link on stackoverflow didn't help?

@cowtowncoder
Copy link
Member

I can understand the other issue to some degree (although it took quite a bit of time).
But here, executable example is so long that I do need a condensed example of pinpointing specific problem to be able to help. It is not necessary to understand full use case or goal, but I do need to understand what is the thing that you think should work, and how.

@fabienrenaud
Copy link
Author

I guess you need to see the code running through the module and why it was built in the first place. With the above code, replace the main function with:

public static void main(String[] args) {
    stillWorks();
    stoppedWorking();
}

private static void stillWorks() {
    String json     = "{\"a\":100,\"b\":\"bb\",\"x\":-100,\"y\":\"YY\",\"z\":314,\"obj\":{\"id\":100}}";
    String expected = "{\"b\":\"__\",\"a\":100,\"x\":-100,\"y\":\"YY\",\"z\":314,\"obj\":{\"id\":100}}"; // note: "b" comes now first because the module is ghetto and ignores the serialization config
    Bar obj = JsonUtils.fromJson(json, Bar.class);
    assert obj != null;
    assert obj.getTree() != null;
    assert obj.getTree().size() == 6;

    obj.b = "__";

    String str = JsonUtils.toJson(obj);
    if (!expected.equals(str)) {
        throw new IllegalStateException();
    }
    assert obj.getTree() != null;
    System.out.println("StillWorks: passed");
}

private static void stoppedWorking() {
    String json = "{\"a\":100,\"b\":\"bb\",\"x\":-100,\"y\":\"YY\",\"z\":314,\"obj\":{\"id\":100}}";
    Foo obj = JsonUtils.fromJson(json, Foo.class);
    assert obj != null;
    assert obj.getTree() != null;
    assert obj.getTree().size() == 6;

    String str = JsonUtils.toJson(obj);
    if (!json.equals(str)) {
        throw new IllegalStateException();
    }
    assert obj.getTree() != null;
    System.out.println("StoppedWorking: passed");
}

private static class Foo extends JsonContainer {
}

private static class Bar extends JsonContainer {
    @JsonProperty("b")
    private String b;
}

The interest of the module -- I hope you can see it above -- is to get back the full blob with my change when i reserialize Bar.

Thing is, even if the POJO is empty, has no properties, for some dumb reason (config issue, dev problem, some bad code clean up), i still want to get back the whole blob (which is cached in JsonContainer). It used to work in 1.9 because no matter what, my Pojo was running through the module regardless of whether it has defined properties. Now, it skips the module which may have special treatment for that type because the pojo has no properties.

The module above is a ghetto / quick support for #549 and only works for 1st level properties.
Now, talking about #549, this should not return an empty blob:

mapper.writerForUpdating(node).write(new Object());

... stoppedWorking() would be equivalent to that line above... but now returns me an empty blob with jackson 2.

Is this helping?

@cowtowncoder
Copy link
Member

Would it be possible to write a simple reproducible case that only shows the part of skipping module: it need not to actual functionality you have or anything. I just don't see why serializer construction was not working as expected.

On the other hand, sub-classing of BeanSerializer could be the problem part. Even if custom serializer was being used, if method getting called is different, behavior would look like serialization was not working.

Another thing I notice in serializer is that it modifies the tree given as input (removing entries). This means that serialization is not idempotent. It would be safer to keep Set of property names that are already output, and skip serialization of those entries from tree. This may not be the reason for failure, but seems bit dangerous practice.

@fabienrenaud
Copy link
Author

Here you go, smallest possible and comparative example:

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.Module;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationConfig;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.ser.BeanSerializerModifier;
import java.io.IOException;

public class JsonBug {

    static boolean invokedForBar;
    static boolean invokedForFoo;

    public static void main(String[] args) {
        new JsonBug().run();
    }
    private final ObjectMapper mapper;

    JsonBug() {
        mapper = new ObjectMapper();

        mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
        mapper.disable(
                MapperFeature.AUTO_DETECT_CREATORS,
                MapperFeature.AUTO_DETECT_FIELDS,
                MapperFeature.AUTO_DETECT_GETTERS,
                MapperFeature.AUTO_DETECT_IS_GETTERS,
                MapperFeature.AUTO_DETECT_SETTERS,
                MapperFeature.USE_GETTERS_AS_SETTERS
        );
        mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
        mapper.registerModule(new MyModule());
    }

    void run() {
        testA();
        testB();
    }

    private void testA() {
        invokedForBar = false;
        Bar b = new Bar();
        b.b = "asd";
        try {
            mapper.writeValueAsString(b);
        } catch (IOException ex) {
            System.err.println(ex.getClass() + " " +ex.getMessage());
        }
        System.out.println("Test A: " + (invokedForBar ? "Passed" : "Failed"));
    }

    private void testB() {
        invokedForFoo = false;
        Foo b = new Foo();
        try {
            mapper.writeValueAsString(b);
        } catch (IOException ex) {
            System.err.println(ex.getClass() + " " +ex.getMessage());
        }
        System.out.println("Test B: " + (invokedForFoo ? "Passed" : "Failed"));
    }

    private static class Foo {
    }

    private static class Bar {

        @JsonProperty("b")
        private String b;
    }

    private static class MyModule extends Module {

        @Override
        public String getModuleName() {
            return "MyModule";
        }

        @Override
        public Version version() {
            return new Version(0, 1, 0, "SNAPSHOT");
        }

        @Override
        public void setupModule(Module.SetupContext context) {
            context.addBeanSerializerModifier(new BeanSerializerModifier() {
                @Override
                public JsonSerializer<?> modifySerializer(SerializationConfig config, BeanDescription beanDesc, JsonSerializer<?> serializer) {
                    if (beanDesc.getBeanClass() == Bar.class) {
                        invokedForBar = true;
                    }
                    if (beanDesc.getBeanClass() == Foo.class) {
                        invokedForFoo = true;
                    }
                    return serializer;
                }
            });
        }
    }

}

@cowtowncoder
Copy link
Member

Ok I'll try the code and see if it makes more sense.

@fabienrenaud
Copy link
Author

And just to prove i'm not bluffing, here's the version I use with Jackson 1.9.11 ;)

import java.io.IOException;
import org.codehaus.jackson.Version;
import org.codehaus.jackson.annotate.JsonProperty;
import org.codehaus.jackson.map.DeserializationConfig;
import org.codehaus.jackson.map.JsonSerializer;
import org.codehaus.jackson.map.Module;
import org.codehaus.jackson.map.ObjectMapper;
import org.codehaus.jackson.map.SerializationConfig;
import org.codehaus.jackson.map.introspect.BasicBeanDescription;
import org.codehaus.jackson.map.ser.BeanSerializerModifier;

public class JsonBugV1 {

    static boolean invokedForBar;
    static boolean invokedForFoo;

    public static void main(String[] args) {
        new JsonBugV1().run();
    }
    private final ObjectMapper mapper;

    JsonBugV1() {
        mapper = new ObjectMapper();

        mapper.configure(DeserializationConfig.Feature.FAIL_ON_UNKNOWN_PROPERTIES, false);
        mapper.registerModule(new MyModule());
    }

    void run() {
        testA();
        testB();
    }

    private void testA() {
        invokedForBar = false;
        Bar b = new Bar();
        b.b = "asd";
        try {
            mapper.writeValueAsString(b);
        } catch (IOException ex) {
            System.err.println(ex.getClass() + " " + ex.getMessage());
        }
        System.out.println("Test A: " + (invokedForBar ? "Passed" : "Failed"));
    }

    private void testB() {
        invokedForFoo = false;
        Foo b = new Foo();
        try {
            mapper.writeValueAsString(b);
        } catch (IOException ex) {
            System.err.println(ex.getClass() + " " + ex.getMessage());
        }
        System.out.println("Test B: " + (invokedForFoo ? "Passed" : "Failed"));
    }

    private static class Foo {
    }

    private static class Bar {

        @JsonProperty("b")
        private String b;
    }

    private static class MyModule extends Module {

        @Override
        public String getModuleName() {
            return "MyModule";
        }

        @Override
        public Version version() {
            return new Version(0, 1, 0, "SNAPSHOT");
        }

        @Override
        public void setupModule(Module.SetupContext context) {
            context.addBeanSerializerModifier(new BeanSerializerModifier() {
                @Override
                public JsonSerializer<?> modifySerializer(SerializationConfig config, BasicBeanDescription beanDesc, JsonSerializer<?> serializer) {
                    if (beanDesc.getBeanClass() == Bar.class) {
                        invokedForBar = true;
                    }
                    if (beanDesc.getBeanClass() == Foo.class) {
                        invokedForFoo = true;
                    }
                    return serializer;
                }
            });
        }
    }

}

This outputs:

Test A: Passed
class org.codehaus.jackson.map.JsonMappingException No serializer found for class test.JsonBugV1$Foo and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationConfig.Feature.FAIL_ON_EMPTY_BEANS) )
Test B: Passed

While the version for jackson 2 outputs:

Test A: Passed
class com.fasterxml.jackson.databind.JsonMappingException No serializer found for class test.JsonBugV2$Foo and no properties discovered to create BeanSerializer (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) )
Test B: Failed

@cowtowncoder
Copy link
Member

Ok, so failure I saw was with "can't serialize POJO with no properties"; and that can be disabled with:

mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);

would this help? With this, the code passes.

But I guess even without this setting, modifySerializer() should be called (because default error-throwing serializer does exist). I can try to see if there is something to fix in this codepath.

@cowtowncoder
Copy link
Member

... and indeed, that does not work, somewhat surprisingly. Why surprisingly? Because there's unit test TestBeanSerializer that tests for post-creation handling of empty serializer.

Alas, it only tests changeProperties handling, but NOT `modifySerializer'.

So I think I finally see the underlying bug here. :)

@fabienrenaud
Copy link
Author

Yeeaeaaaahhh :)

@cowtowncoder cowtowncoder modified the milestones: 2.2, 2.5.0 Sep 18, 2014
@cowtowncoder
Copy link
Member

Unfortunately change is non-trivial, so I think this should only go in 2.5.

Work-around with earlier versions is to use:

mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);

which will result in expected behavior as well (i.e. modifySerializer()) getting called.

Note that with the fix, type of JsonSerializer being returned will not be BeanSerializer (but dummy serializer that would be used to throw exception if it gets used), so modifySerializer() needs to be careful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants