Skip to content
This repository has been archived by the owner on Jan 22, 2019. It is now read-only.

Generic type information for Optional wrapped generic type lost in visitor #83

Closed
vzx opened this issue Sep 21, 2015 · 2 comments
Closed
Milestone

Comments

@vzx
Copy link

vzx commented Sep 21, 2015

This is kind of a strange issue, which I can't really describe well except by the unit test below. When a Guava Optional wraps a generic type (eg. a Collection), the generic type information seems to be lost somewhere along the way, and the visitor does not actually visit the properties of the generic type.

In the following code, the test fails using 2.6.0-rc3 and higher (2.6.2 included), but succeeds with 2.6.0-rc2. The expected output is [values.data, values, values.data.value], but the actual output is [values.data, values].

While debugging, I found out that in the method GuavaOptionalSerializer#acceptJsonFormatVisitor the generic type information of Collection is lost, and it thinks the type is Object, not ValueHolder, but the code was a bit too complex for me to figure out why or how. It seems to me the issue might have been in changeset eb6b98a but I'm not sure.

By the way, this works fine with Jdk8Module and java.util.Optional.

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.jsonFormatVisitors.*;
import com.fasterxml.jackson.datatype.guava.GuavaModule;
import com.google.common.base.Optional;
import org.junit.Assert;
import org.junit.Test;

import java.util.Collection;
import java.util.HashSet;
import java.util.Set;

public class OptionalGenericTest {
    @Test
    public void testGeneric() throws Exception {
        VisitorWrapper wrapper = new VisitorWrapper(null, "", new HashSet<>());
        new ObjectMapper()
                .registerModule(new GuavaModule())
                .acceptJsonFormatVisitor(TopLevel.class, wrapper);
        Set<String> properties = wrapper.getTraversedProperties();

        System.out.println(properties);

        Assert.assertTrue(properties.contains("values.data.value"));
    }

    public class TopLevel {
        @JsonProperty("values")
        public Optional<CollectionHolder<ValueHolder>> values;
    }

    public class ValueHolder {
        @JsonProperty("value")
        public String value;
    }

    public class CollectionHolder<T> {
        @JsonProperty("data")
        public Collection<T> data;
    }

    public static class VisitorWrapper implements JsonFormatVisitorWrapper {
        private SerializerProvider serializerProvider;
        private final String baseName;
        private final Set<String> traversedProperties;

        public VisitorWrapper(SerializerProvider serializerProvider, String baseName, Set<String> traversedProperties) {
            this.serializerProvider = serializerProvider;
            this.baseName = baseName;
            this.traversedProperties = traversedProperties;
        }

        private VisitorWrapper createSubtraverser(String baseName) {
            return new VisitorWrapper(getProvider(), baseName, traversedProperties);
        }

        public Set<String> getTraversedProperties() {
            return traversedProperties;
        }

        @Override
        public JsonObjectFormatVisitor expectObjectFormat(JavaType type) throws JsonMappingException {
            return new JsonObjectFormatVisitor.Base(serializerProvider) {
                @Override
                public void property(BeanProperty prop) throws JsonMappingException {
                    anyProperty(prop);
                }

                @Override
                public void optionalProperty(BeanProperty prop) throws JsonMappingException {
                    anyProperty(prop);
                }

                private void anyProperty(BeanProperty prop) throws JsonMappingException {
                    final String propertyName = prop.getFullName().toString();
                    traversedProperties.add(baseName + propertyName);
                    serializerProvider.findValueSerializer(prop.getType(), prop)
                            .acceptJsonFormatVisitor(createSubtraverser(baseName + propertyName + "."), prop.getType());
                }
            };
        }

        @Override
        public JsonArrayFormatVisitor expectArrayFormat(JavaType type) throws JsonMappingException {
            serializerProvider.findValueSerializer(type.getContentType())
                    .acceptJsonFormatVisitor(createSubtraverser(baseName), type.getContentType());
            return new JsonArrayFormatVisitor.Base(serializerProvider);
        }

        @Override
        public JsonStringFormatVisitor expectStringFormat(JavaType type) throws JsonMappingException {
            return new JsonStringFormatVisitor.Base();
        }

        @Override
        public JsonNumberFormatVisitor expectNumberFormat(JavaType type) throws JsonMappingException {
            return new JsonNumberFormatVisitor.Base();
        }

        @Override
        public JsonIntegerFormatVisitor expectIntegerFormat(JavaType type) throws JsonMappingException {
            return new JsonIntegerFormatVisitor.Base();
        }

        @Override
        public JsonBooleanFormatVisitor expectBooleanFormat(JavaType type) throws JsonMappingException {
            return new JsonBooleanFormatVisitor.Base();
        }

        @Override
        public JsonNullFormatVisitor expectNullFormat(JavaType type) throws JsonMappingException {
            return new JsonNullFormatVisitor.Base();
        }

        @Override
        public JsonAnyFormatVisitor expectAnyFormat(JavaType type) throws JsonMappingException {
            return new JsonAnyFormatVisitor.Base();
        }

        @Override
        public JsonMapFormatVisitor expectMapFormat(JavaType type) throws JsonMappingException {
            return new JsonMapFormatVisitor.Base(serializerProvider);
        }

        @Override
        public SerializerProvider getProvider() {
            return serializerProvider;
        }

        @Override
        public void setProvider(SerializerProvider provider) {
            this.serializerProvider = provider;
        }
    }
}
@cowtowncoder
Copy link
Member

This does sound complicated, thank you for reporting it.
I am not sure whether expected behavior can be fully supported because generic parameters for "custom" types (that is, something other than parameterization of recognized types like Map, Collection, Iterable, Optional)) are difficult to handle and pass; and the reason earlier versions worked may actually have been lucky coincidence. But I hope I am wrong and it turns out it can be supported just fine. :-)

@cowtowncoder cowtowncoder added this to the 2.6.3 milestone Sep 29, 2015
@cowtowncoder
Copy link
Member

Thank you for the test -- problem was actually easy enough to solve; serializer access was not using full JavaType, but type-erased (raw) version. Fix will be in 2.6.3.

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

No branches or pull requests

2 participants