FasterXML/jackson-module-afterburner

AfterBurner mutates collections rather than adding entires to existing ones

Closed this issue · 10 comments

By default Jackson has USE_GETTERS_AS_SETTERS default.

USE_GETTERS_AS_SETTERS (default: true) Controls whether "getters" that return Collection or Map types can be used for "setting" values (same as how JAXB API works with XML), so that separate "setter" method is not needed. Even if enabled, explicit "setter" method will have precedence over implicit getter-as-setter, if one exists.

However, Afterburner ignores that and tries to set the collection itself. Which leads to IllegalAccessError on final collection fields.

Read more here on StackOverflow.

Affected version: 2.3.0

Thank you for reporting this. We will see what gives; most likely Afterburner is missing override of a method.

Hmmh. Actually, this does not seem to even work for plain databind... need to investigate what I am doing wrong.

Actually, this seems to work exactly the same with or without afterburner. But it means that if either field, or setter exists (not just getter), those have priority and are used instead of getter.

So in your case field name is considered to have precedence over trying to call getter and will be used.

As far as I can see, there is nothing Afterburner-specific here: if behavior itself seems wrong or sub-optimal, you can file an RFE for jackson-databind; it can not be changed just for Afterburner.

One additional comment: I can see how precedence may seem unintuitive; so that in case like:

class Bean {
  private List<String> values;

  public List<String> getValues() { return value; }
}

field will be used, regardless of settings, since it has precedence over getter (but setter would have even higher precedence). However, if field was missing, or had non-matching name, getter would be used.

My main concern is that USE_GETTERS_AS_SETTERS is enabled, so if behavior was changed, it could cause regression.

Anyway: since this is a core databind feature, a new issue would be needed for jackson-databind if behavior was to be changed.

Hi thanks for reply, I think it will be very useful to update the USE_GETTERS_AS_SETTERS config description.

I still am confused about your "precedence" comment. How come field has higher precedence than getter, (Here you were saying opposite). By the way this whole thing about precedence would be a very nice blog topic. This is grey area for most of us, normal precedence, how is it affected by @JsonProperty. Also let me to put forward some scenarios below and see what would be expected behaviour for each:

A) field only

class Bean {
  private List<String> values;
}

B) getter with matching Name

class Bean {
  private List<String> values;

  public List<String> getValues() { return value; }
}

C) getter with non-matching Name

class Bean {
  private List<String> values;

  public List<String> getValuesAlpha() { return value; }
}

D) getter and setter

class Bean {
  private List<String> values;

  public void setValues(List<String> values){this.values.addAll(values);}
  public List<String> getValues() { return value; }
}

E) setter only

class Bean {
  private List<String> values;

 public void setValues(List<String> values){this.values.addAll(values);}
}

in which of these cases it will add to collection, rather than replace it. In which of them I can safely declare List as final?

Where USE_GETTERS_AS_SETTERS would make a difference?

For "normal" use case -- setters for deserialization, getters for deserialization -- matching accessor does have higher precedence than field. But use of getter-for-setting is a special case, and it is not really clear what is the right choice here (ideally intuitive one). To me it seems like it should be the last effort, as it is a really round-about way of modifying things. I am almost thinking that this feature should never have been added in the first place, even if devs with JAXB background might expect it.

But the reason this entry is closed is that this is not Afterburner specific: I am open to discussion on proper logic, documentation, but that should be done under jackson-databind project.

Having said that,

Thanks @cowtowncoder Your explanation makes it clear. No need for followup, unless you want to do something about that.

Just to confirm that I understood correctly, in the situations above only D & E would add to collection, so there i can declare list final, whereas in others list would be replaced. With USE_GETTERS_AS_SETTERS=true also B would allow to make list final.

@husayt Setter has highest precedence for deserialization, so D and E would use setter.
So case B is the only open one, in which precedence between getter and field determines one that gets used.

Great, thank you very much. Jackson is a great piece pf software, it is pleasure to work with and is like treasure island, in a way that there are so many cool features, most of which I still haven't used. But with even very little subset of it I know I can do more than enough.

Thanks again

Thank you for the kind words. I appreciate the feedback, and hope you will all the features you need in future too. :)