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

DeserializationFeature.READ_ENUMS_USING_TO_STRING not dynamically changeable with 2.7 #1161

Closed
asa-git opened this issue Mar 13, 2016 · 6 comments
Milestone

Comments

@asa-git
Copy link

asa-git commented Mar 13, 2016

Hiya,
There seems to be an issue with the ObjectReader objects not taking into account the configuration set in them instead of the main object mapper instance.

Here is a simple example:

Having this inner class:

    static class Data {
        enum Type {
            A, B, C;

            @Override
            public String toString() {
                return this.name().toLowerCase();
            };
        };

        private final Type type;
        private final String value;

        @JsonCreator
        public Data(//
                @JsonProperty("type") Type type, //
                @JsonProperty("value") String value) {
            this.type = type;
            this.value = value;
        }

        @Override
        public String toString() {
            return "Data [type=" + type + ", value=" + value + "]";
        }
    }

The following will fail with an exception when parsing the enum:

        final String src = "[ { \"type\": \"a\", \"value\": \"1\" }, { \"type\": \"b\", \"value\": \"2\" }]";
        final ObjectMapper mapper = new ObjectMapper();

        MappingIterator<Data> iterator = mapper //
                .readerFor(Data.class) //
                .with(DeserializationFeature.READ_ENUMS_USING_TO_STRING) //
                .readValues(src);
        while (iterator.hasNext()) {
            System.out.println(iterator.next());
        }

to get it to work it seems we need to move the DeserializationFeature configuration to the ObjectMapper itself:

        final String src = "[ { \"type\": \"a\", \"value\": \"1\" }, { \"type\": \"b\", \"value\": \"2\" }]";
        final ObjectMapper mapper = new ObjectMapper() //
                .configure(DeserializationFeature.READ_ENUMS_USING_TO_STRING, true);

        MappingIterator<Data> iterator = mapper //
                .readerFor(Data.class) //
                .readValues(src);
        while (iterator.hasNext()) {
            System.out.println(iterator.next());
        }

Tested with both version 2.7.1-1 and 2.7.2.

@cowtowncoder
Copy link
Member

That certainly sounds like a bug: the whole idea of ObjectReader is that instances would allow reconfiguration.
Thank you for providing the test case, I hope to resolve this for 2.7.3.

@cowtowncoder
Copy link
Member

This is a tricky problem to resolve. The underlying problem is not so much that the setting was not propagated, but rather combination of this feature being used during construction of deserializer (i.e. not changeable once setting first observed, and deserialized cached), and auto-fetching of said deserializer.

For the specific case you have, a work-around would be to change construction order to:

        MappingIterator<Data1161> iterator = mapper
                .reader()
                .with(DeserializationFeature.READ_ENUMS_USING_TO_STRING)
                .forType(Data1161.class)
                .readValues(src);

which would prevent pre-fetch of deserializer before configuration values are complete.

I will keep this issue open and try to figure out proper fix.

@cowtowncoder cowtowncoder changed the title ObjectReader and DeserializationFeature DeserializationFeature.READ_ENUMS_USING_TO_STRING not dynamically changeable with 2.7 Mar 14, 2016
cowtowncoder added a commit that referenced this issue Mar 14, 2016
@asa-git
Copy link
Author

asa-git commented Mar 14, 2016

hiya,
Thanks for the workaround, through if I'm not wrong there might be a quick way to resolve it.
While tracing the code, I was able to see that the internal _config object was properly updated.
The exception being generated in the method com.fasterxml.jackson.databind.deser.std.EnumDeserializer._deserializeAltString(JsonParser, DeserializationContext, String) would that not only require to add the following (or close to) :

if (ctxt.isEnabled(DeserializationFeature.READ_ENUMS_USING_TO_STRING)) {
    for (Enum e: ((Enum[]) this._enumsByIndex)) {
        if (e.toString().equals(name)) {
            return e;
        }
    }
}

UPDATE: I cloned the master branch and tried the above proposal. it seems to have solved the issue without any regression that I could see. If you need a PR, just let me know against which branch I should send it.

@cowtowncoder
Copy link
Member

@asa-git Functionally, yes, but performance is rather bad, so deserializer uses lookup tables.
In this case I think change would just involve allowing use of lazily-constructed table that would be populated as needed from names.

@cowtowncoder cowtowncoder added this to the 2.7.3 milestone Mar 15, 2016
@asa-git
Copy link
Author

asa-git commented Mar 15, 2016

Tks a lot for the quick patch.
The alternate implementation will for fine for the moment until the release of 2.7.3.

@cowtowncoder
Copy link
Member

@asa-git yes, absolutely, it is better to have a work-around than failure so it's great you have one.

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