FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

`MapperFeature` to ignore setter/getter method implicit detection for Record types #4157

Open garretwilson opened 12 months ago

garretwilson commented 12 months ago

Describe your Issue

Because Java is not Delphi ObjectPascal or modern JavaScript or Kotlin, it has no idea of "properties", although the JavaBeans specification was an attempt to shoehorn in the concept by guessing based upon convention. That's why in the wild olden days, Jackson assumed that a method String getDisplayName() was a getter; and if there were no setDisplayName(String), well, it's a getter to an imputed read-only property. If you didn't like it, you added a @JsonIgnore annotation to your class. (Back then we had a heck of a time figuring out how to make an immutable instance if the constructor had String foo, String bar, …, too.)

But now we have the Java record, which (despite its downsides—it's just one step to a lot of other features coming in Java) beautifully encapsulates not only a set of fields, but also the names of the fields and their required order in the constructor! Moreover Java adds some nifty new reflection methods to access records at runtime.

So let's say we have this record:

/**
 * A user's profile information.
 * @param username The user's login identifier.
 * @param firstName The first name of the user.
 * @param lastName The last name of the user.
 */
public record UserProfile(@Nonnull String username, @Nonnull String firstName, @Nonnull String lastName) {

  public UserProfile {
    requireNonNull(username);
    requireNonNull(firstName);
    requireNonNull(lastName);
  }

  /** @return A form of the user name appropriate for displaying in messages. */
  public String getDisplayName() {
    return firstName() + " " + lastName();
  }

}

We know at runtime exactly what the constructor types and names (e.g. username); and which fields they correspond to. We know what the field getters are (e.g. username(). More importantly, we know what methods are not getters, and have nothing to do with the fields—getDisplayName() is an example.

Unfortunately Jackson will generate a JSON object that has a displayName attribute, which will prevent the object from being parsed back round-trip, because displayName doesn't correspond to any of the record fields.

Maybe the user wanted to generate a JSON object with the displayName attribute and never use it for deserializing back to a UserProfile instance. That's a valid use case, but it doesn't seem like it should be the default use case.

I can get around this by adding a @JsonIgnore:

  /** @return A form of the user name appropriate for displaying in messages. */
  @JsonIgnore
  public String getDisplayName() {
    return firstName() + " " + lastName();
  }

But I don't want to dirty the model (even though it's a DTO model) with serialization information unless I have to. As recounted above, at one time we had to. But with Java record, we shouldn't have to.

Is there a way to configure Jackson to automatically ignore non-field methods for Java record? If there is something I could use in JsonMapper.builder() that would be fine. Is there something like JsonMapper.builder().serializationMethodInclusion(JsonInclude.Include.NON_RECORD)? If not, what would you recommend as the best way forward to get this sort of functionality? Is there some little logic I can inject into my JsonMapper to detect and handle this case, for example?

JooHyukKim commented 12 months ago

Is there a way to configure Jackson to automatically ignore non-field methods for Java record? If there is something I could use in JsonMapper.builder() that would be fine. Is there something like JsonMapper.builder().serializationMethodInclusion(JsonInclude.Include.NON_RECORD)? If not, what would you recommend as the best way forward to get this sort of functionality? Is there some little logic I can inject into my JsonMapper to detect and handle this case, for example?

Okay, it seems like a usage question, right?

garretwilson commented 12 months ago

Okay, it seems like a usage question, right?

Well ultimately this is a request to add an option to turn off automatic serialization of additional methods on Java records. I assume there isn't a way already, but I was asking so as to confirm that first. Being none, I'd like to request that such an option be added.

JooHyukKim commented 12 months ago

I see. Since the issue template is for "Something Else", was wondering what the issue would be 😅.

Maybe we can treat this as "Feature Request", but with conditions and maybe add some sort of header, for hint, so others can easily get on helping?

pjfanning commented 12 months ago

@garretwilson have you checked around the web? Plenty of stackoverflow and blogs to look up.

I've never gone near the visibility settings but they look like the kind of settings that might work here.

https://stackoverflow.com/questions/7105745/how-to-specify-jackson-to-only-use-fields-preferably-globally covers the opposite - only using fields - but maybe you could use some trial and error on this?

cowtowncoder commented 12 months ago

@pjfanning I would recommend against trying make use of Fields record types have tho... it requires forcing access and is less than ideal, possibly breaking at some point with later JDKs.

I think I'd be +1 for new MapperFeature -- if need be -- for disabling implicit "getter-discovery" for Record types (probably should also disable setter discovery fwtw). And I think it is quite doable.

... although most likely for 2.17, unless someone can come up with a patch in a day or so; I am trying to close the last issue I really want in for 2.16.

JooHyukKim commented 12 months ago

... although most likely for 2.17, unless someone can come up with a patch

+1 for 2.17 planning 👍🏼 record type has taken lots of battle scars past few minor versions, so we may want to take it slow (unless criticial)

JooHyukKim commented 12 months ago

I think I'd be +1 for new MapperFeature -- if need be -- for disabling implicit "getter-discovery" for Record types (probably should also disable setter discovery fwtw). And I think it is quite doable.

Throwing out some naming ideas,

cowtowncoder commented 12 months ago

Looking at existing MapperFeature, we have AUTO_DETECT_GETTERS. The reason I like referring to auto-detection (or implicit detection) is that this would not ignore explicitly annotated getters; only disable auto-detection -- this so that defining explicit getters is fine since that is often needed (f.ex to change default behavior).

And to refer to both getters and setters we could use term "accessors", so could consider AUTO_DETECT_RECORD_ACCESSORS (default true for backwards compatibility).

One possible complication is that users might expect existing AUTO_DETECT_GETTERS/_SETTERS to still have some effect, so would need to consider semantics there -- for Mapper-/Deserialization-/SerializationFeature we only have on/off setting and no "use default". In that sense, maybe use of DISABLE_RECORD_ACCESSOR_AUTO_DETECTION would make most sense; enabling of which would forcibly prevent auto-detection for Record fields, setters, getters, is-getters (but not Creator detection).

And yes, it does sound like waiting until 2.17 would be prudent, even if this initially seemed like a simple thing to add.

yihtserns commented 12 months ago

I assume there isn't a way already, but I was asking so as to confirm that first.

I've never gone near the visibility settings but they look like the kind of settings that might work here.

I would recommend against trying make use of Fields record types have tho... it requires forcing access and is less than ideal, possibly breaking at some point with later JDKs.

Ignoring the question of whether it is an appropriate method or not, just want to note that using (abusing?) visibility seems to be the typical way of achieving this - see:

cowtowncoder commented 11 months ago

@yihtserns yes. On plus side, there are workarounds. On downside all these workarounds probably make it more not less difficult to fix things (since they are now behavior that arguably needs to be supported, being used somewhat widely).