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

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

Open
garretwilson opened this issue Oct 12, 2023 · 10 comments
Labels
2.17 Issues planned at earliest for 2.17 Record Issue related to JDK17 java.lang.Record support to-evaluate Issue that has been received but not yet evaluated

Comments

@garretwilson
Copy link

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?

@garretwilson garretwilson added the to-evaluate Issue that has been received but not yet evaluated label Oct 12, 2023
@JooHyukKim
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

@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?

@garretwilson garretwilson changed the title Option to ignore Java record non-field methods by default. Request option to ignore Java record non-field methods by default. Oct 13, 2023
@cowtowncoder cowtowncoder added Record Issue related to JDK17 java.lang.Record support 2.17 Issues planned at earliest for 2.17 labels Oct 13, 2023
@cowtowncoder
Copy link
Member

@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.

@cowtowncoder cowtowncoder changed the title Request option to ignore Java record non-field methods by default. MapperFeature to ignore setter/getter method implicit detection for Record types Oct 13, 2023
@JooHyukKim
Copy link
Member

... 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
Copy link
Member

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,

  • MapperFeature.IGNORE_SETTER_METHODS_IN_RECORD_TYPE,
  • MapperFeature.IGNORE_GETTER_METHODS_IN_RECORD_TYPE,
  • MapperFeature.IGNORE_GETTER_AND_SETTER_METHODS_IN_RECORD_TYPE,

@cowtowncoder
Copy link
Member

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
Copy link
Contributor

yihtserns commented Oct 13, 2023

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
Copy link
Member

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 Issues planned at earliest for 2.17 Record Issue related to JDK17 java.lang.Record support to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

5 participants