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

JacksonSingleton should not force FAIL_ON_UNKNOWN_PROPERTIES on us #486

Closed
magnet opened this Issue Apr 24, 2015 · 3 comments

Comments

Projects
None yet
2 participants
@magnet
Contributor

magnet commented Apr 24, 2015

    private void setMappers(ObjectMapper mapper, XmlMapper xml) {
        synchronized (lock) {
            this.mapper = mapper;
            this.xml = xml;
            if (mapper != null) {
                this.mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
            }
            if (xml != null) {
                this.xml.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
            }
        }
    }

I'd argue to keep Jackson's own defaults and have a configuration option to set extra deserialization/serialization features.

I understand being lenient might look like a better default for some applications, and that there might be existing Wisdom projects that now rely on that default. But it's not a sensible default, and it should at least be possible to disable this setting in the configuration.

(as an argument on why Wisdom shouldn't redefine Jackson's defaults: we just spent one hour determining why exactly Jackson was behaving differently between our Wisdom routes and our unit tests).

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 24, 2015

Member

Oh I see the issue.
However it's a recommended default to support evolution. So, let's add a configuration option to disable this default.

Member

cescoffier commented Apr 24, 2015

Oh I see the issue.
However it's a recommended default to support evolution. So, let's add a configuration option to disable this default.

@cescoffier cescoffier added this to the 0.8.1 milestone Apr 24, 2015

@cescoffier cescoffier changed the title from JacksonSingleton should not force FAIL_ON_UNKNOWN_PROPERTIES on us to Allow disabling the JacksonSingleton FAIL_ON_UNKNOWN_PROPERTIES option Apr 24, 2015

@cescoffier cescoffier changed the title from Allow disabling the JacksonSingleton FAIL_ON_UNKNOWN_PROPERTIES option to JacksonSingleton should not force FAIL_ON_UNKNOWN_PROPERTIES on us Apr 24, 2015

@cescoffier cescoffier added the planned label Apr 24, 2015

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 24, 2015

Member

Let's have a jackson configuration to enable / disable any of the Jackson feature. IF not set, it would use current default. If set, it applied the configuration.

Member

cescoffier commented Apr 24, 2015

Let's have a jackson configuration to enable / disable any of the Jackson feature. IF not set, it would use current default. If set, it applied the configuration.

@magnet

This comment has been minimized.

Show comment
Hide comment
@magnet

magnet Apr 25, 2015

Contributor

Thanks :)

Contributor

magnet commented Apr 25, 2015

Thanks :)

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