Jackson JSON Processor
  1. Jackson JSON Processor
  2. JACKSON-189

Beans with overloaded setters aren't handled well

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 1.3
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      2

      Description

      It looks like BasicBeanDescription throws an exception if multiple setters are found for a property:

      Conflicting setter definitions for property "value": OverloadedBean#setValue(1 params) vs OverloadedBean#setValue(1 params)

      I've been able to get around this limitation by subclassing BasicBeanDescription and overriding okNameForSetter to check not just for a reasonable name but also that there is a getter that has a return type the same as the setter's argument. Since okNameForSetter is called so frequently it would be much more efficient to be done only when a potential problem is detected (in BasicBeanDescription.findSetters).

      1. OverloadedSettersTest.java
        0.9 kB
        Bryan Barkley
      2. patch.txt
        3 kB
        Bryan Barkley

        Activity

        Hide
        Tatu Saloranta added a comment -

        Hmmh. Interesting. So basically it would be good to use conflict resolution if there is enough information to do that.
        One additional question: if you explicitly specify one with @JsonProperty annotation, do you still get an exception (I suspect that would be the case).

        Anyway, yes, I agree that it'd be good to improve handling for such cases; and although generally read/write sides are loosely coupled with Jackson, it would make sense to use extra information if such is available.

        For what it's worth, expected work-around here would be tagging other setter(s) with @JsonIgnore.

        Show
        Tatu Saloranta added a comment - Hmmh. Interesting. So basically it would be good to use conflict resolution if there is enough information to do that. One additional question: if you explicitly specify one with @JsonProperty annotation, do you still get an exception (I suspect that would be the case). Anyway, yes, I agree that it'd be good to improve handling for such cases; and although generally read/write sides are loosely coupled with Jackson, it would make sense to use extra information if such is available. For what it's worth, expected work-around here would be tagging other setter(s) with @JsonIgnore.
        Hide
        Bryan Barkley added a comment -

        Hmm, I hadn't thought about the cases with annotations. I'm trying to use Jackson to replace XStream as the transport between multiple services, so the annotation based tagging isn't really a viable option, as there are hundreds of different types of objects being passed around, and we don't want to revisit each one and tie our business objects to a specific framework. We want something that just works out of the box (Jackson doesn't so far but I'm hoping I can get it there - polymorphic deserialization is a big part of that).

        Attached is a patch using some code written to get things working at a basic level for me. It's not production ready by any means, but demonstrates a basic approach.

        Show
        Bryan Barkley added a comment - Hmm, I hadn't thought about the cases with annotations. I'm trying to use Jackson to replace XStream as the transport between multiple services, so the annotation based tagging isn't really a viable option, as there are hundreds of different types of objects being passed around, and we don't want to revisit each one and tie our business objects to a specific framework. We want something that just works out of the box (Jackson doesn't so far but I'm hoping I can get it there - polymorphic deserialization is a big part of that). Attached is a patch using some code written to get things working at a basic level for me. It's not production ready by any means, but demonstrates a basic approach.
        Hide
        Tatu Saloranta added a comment -

        Thanks. This should be useful, and it should be possible to handle some common cases.

        One more thought: perhaps correct way would actually to fully handle overloading: already it is for example possible to use both single-string and single-int/long constructors for instantiation. So why not allow alternative setters, depending on which JSON type is found – currently this is done by coercion (int-setter works for Strings, iff one can parse String as a number etc).

        Show
        Tatu Saloranta added a comment - Thanks. This should be useful, and it should be possible to handle some common cases. One more thought: perhaps correct way would actually to fully handle overloading: already it is for example possible to use both single-string and single-int/long constructors for instantiation. So why not allow alternative setters, depending on which JSON type is found – currently this is done by coercion (int-setter works for Strings, iff one can parse String as a number etc).
        Hide
        Bryan Barkley added a comment -

        That makes sense in some cases, including the test case. The actual code where I first encountered this problem had overloaded setters for enums:

        setFoo(MyEnum val)

        setFoo(int val)

        Where we can convert the int to an enum in the setter. Type coercion wouldn't work in that case, and you'd need to know which setter to call based on the JSON value you have (presumably from the getter and of the same type).

        Show
        Bryan Barkley added a comment - That makes sense in some cases, including the test case. The actual code where I first encountered this problem had overloaded setters for enums: setFoo(MyEnum val) setFoo(int val) Where we can convert the int to an enum in the setter. Type coercion wouldn't work in that case, and you'd need to know which setter to call based on the JSON value you have (presumably from the getter and of the same type).
        Hide
        Tatu Saloranta added a comment -

        True – coercion from JSON String (default serialization type for Java enums) would need to be dynamically introspected, but deserializers currently do not offer such metadata. And conveivably someone could change serialization to use ints or such.
        So most likely this should only allow handling of overleading of "native" types – ones used that are used when nominal type is Object.class; and only when there are multiple choices.
        And the very first step can be along your original suggestion: bean deserializer factory can just gather all alternatives, and see if it can find out clear winner.

        Show
        Tatu Saloranta added a comment - True – coercion from JSON String (default serialization type for Java enums) would need to be dynamically introspected, but deserializers currently do not offer such metadata. And conveivably someone could change serialization to use ints or such. So most likely this should only allow handling of overleading of "native" types – ones used that are used when nominal type is Object.class; and only when there are multiple choices. And the very first step can be along your original suggestion: bean deserializer factory can just gather all alternatives, and see if it can find out clear winner.
        Hide
        Tatu Saloranta added a comment -

        Will have to defer to 1.5.0 (1.4.0 will be released any day now), will keep in progress.

        Show
        Tatu Saloranta added a comment - Will have to defer to 1.5.0 (1.4.0 will be released any day now), will keep in progress.
        Hide
        Tatu Saloranta added a comment -

        I was hoping to get this in 1.5.0, but after working through issues with AnnotationIntrospector design, I realized that there are bigger changes that should be made first (see JACKSON-242), before this issue can be resolved.
        This means that I will defer this issue until 1.6, to be done as part of bigger overhaul of property/annotation handling.

        Show
        Tatu Saloranta added a comment - I was hoping to get this in 1.5.0, but after working through issues with AnnotationIntrospector design, I realized that there are bigger changes that should be made first (see JACKSON-242 ), before this issue can be resolved. This means that I will defer this issue until 1.6, to be done as part of bigger overhaul of property/annotation handling.
        Hide
        David Tanner added a comment -

        Will this be getting any attention? Or is it essentially dead.

        Show
        David Tanner added a comment - Will this be getting any attention? Or is it essentially dead.
        Hide
        Tatu Saloranta added a comment -

        No work is on-going with this.
        This is not to say there isn't still strong desire to improve this area, just that AFAIK no one is actively working on it.
        Patches would be most welcome.

        I may also need to merge some entries as there seem to be duplicate entries (esp. if considering github bug tracker too).

        Show
        Tatu Saloranta added a comment - No work is on-going with this. This is not to say there isn't still strong desire to improve this area, just that AFAIK no one is actively working on it. Patches would be most welcome. I may also need to merge some entries as there seem to be duplicate entries (esp. if considering github bug tracker too).
        Hide
        Petar Tahchiev added a comment -

        Any update here? Do you think this will be fixed soon?

        Show
        Petar Tahchiev added a comment - Any update here? Do you think this will be fixed soon?
        Hide
        Tatu Saloranta added a comment -

        Codehaus Jira will be closed: all bug tracking now via github:

        https://github.com/FasterXML/jackson-databind/issues

        There are two known issues reported there,

        https://github.com/FasterXML/jackson-databind/issues/193
        https://github.com/FasterXML/jackson-databind/issues/327

        which will be worked on and hopefully cover same underlying problem.
        If root cause is different, please file a new bug at github.

        Show
        Tatu Saloranta added a comment - Codehaus Jira will be closed: all bug tracking now via github: https://github.com/FasterXML/jackson-databind/issues There are two known issues reported there, https://github.com/FasterXML/jackson-databind/issues/193 https://github.com/FasterXML/jackson-databind/issues/327 which will be worked on and hopefully cover same underlying problem. If root cause is different, please file a new bug at github.

          People

          • Assignee:
            Tatu Saloranta
            Reporter:
            Bryan Barkley
          • Votes:
            3 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: