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

@JsonDeserialize.contentConverter does not work for non-builtin collections #92

Closed
mwisnicki opened this issue Aug 4, 2021 · 7 comments
Milestone

Comments

@mwisnicki
Copy link

I'm trying to use content converter with Guava collections like so:

@JsonDeserialize(contentConverter = DoublingConverter.class)
public ImmutableList<Integer> ints;

However this does not seem to do anything despite Jackson having deserializers for Guava.
Changing collection to one of built-in types makes the program work.

I would expect that if Jackson can deserialize given collection type, it will also be able to apply converters to it.
Posting here rather than to datatypes project since this is not related to specific collection implementation but rather a generic problem.

TestImmutableListContentConverter .java
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.util.StdConverter;
import com.fasterxml.jackson.datatype.guava.GuavaModule;
import com.google.common.collect.ImmutableList;

public class TestImmutableListContentConverter {

    static class Holder {
        @JsonDeserialize(contentConverter = DoublingConverter.class)
        public ImmutableList<Integer> ints;
    }

    static class DoublingConverter extends StdConverter<Integer, Integer> {
        @Override
        public Integer convert(Integer value) {
            return value != null ? value * 2 : null;
        }
    }

    public static void main(String[] args) throws Exception {
        var json = """
                { "ints": [1,2,3] }
                """;
        var om = new ObjectMapper();
        om.registerModule(new GuavaModule());
        var value = om.readValue(json, Holder.class);
        System.out.println(value.ints);
    }

}
pom.xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example</groupId>
    <artifactId>test-jackson-guava</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>16</maven.compiler.source>
        <maven.compiler.target>16</maven.compiler.target>
    </properties>

    <dependencies>
        <dependency>
            <groupId>com.fasterxml.jackson.datatype</groupId>
            <artifactId>jackson-datatype-guava</artifactId>
            <version>2.13.0-rc1</version>
        </dependency>
    </dependencies>

</project>
@cowtowncoder
Copy link
Member

Belongs under Guava datatype library since this requires support by deserializers provided there; will transfer.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-core Aug 18, 2021
@cowtowncoder cowtowncoder added 2.13 guava need-test-case To work on issue, a reproduction (ideally unit test) needed labels Aug 18, 2021
@cowtowncoder
Copy link
Member

Would be great to have a test case, if anyone wants something easy to do. Implementation should not be difficult either, need to have a look at default CollectionDeserializer, lift necessary handling.

@mwisnicki
Copy link
Author

But isn't this a bug in common code?

@mwisnicki
Copy link
Author

Would be great to have a test case, if anyone wants something easy to do. Implementation should not be difficult either, need to have a look at default CollectionDeserializer, lift necessary handling.

Should I submit my testcase in a form of PR adding unit test?

@cowtowncoder
Copy link
Member

@mwisnicki Unlikely based on your mentioning that it works for built-in collections? Guava datatype provides its own Immutable-type deserializers which need to handle a few aspects wrt customizations and configuration.

If it turns out the problem happens to be in jackson-databind we can transfer it back of course.

@cowtowncoder
Copy link
Member

Yes, PR for the unit test would be a great start! It does not have to be separate if you (or anyone else) can figure out the fix too but it can be.

@cowtowncoder cowtowncoder added 2.15 and removed need-test-case To work on issue, a reproduction (ideally unit test) needed 2.13 labels Apr 23, 2023
@cowtowncoder cowtowncoder changed the title JsonDeserialize contentConverter does not work for non-builtin collections @JsonDeserialize.contentConverter does not work for non-builtin collections Apr 23, 2023
cowtowncoder added a commit that referenced this issue Apr 23, 2023
@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Apr 23, 2023
@cowtowncoder
Copy link
Member

Fixed by @JooHyukKim -- will be in 2.15.0. (and if anyone wants to backport fix in 2.14 that'd be fine too, looks safe enough)

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

No branches or pull requests

2 participants