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

Add Date and UUID deserialization support in nullSafeValue method #42956

Closed
wants to merge 2 commits into from

Conversation

minwoo1999
Copy link
Contributor

This pull request introduces enhancements to the JsonObjectDeserializer class to support the deserialization of Date and UUID types.

Changes Made:

Added Support for Date:

Implemented logic to handle Date objects by converting a long timestamp from the JSON node to a Date instance.
java

if (type == Date.class) {
    return (D) new Date(jsonNode.longValue());
}

Added Support for UUID:

Added functionality to convert a string representation of a UUID from the JSON node into a UUID instance.
java

if (type == UUID.class) {
    return (D) UUID.fromString(jsonNode.textValue());
}

New Unit Tests:

Implemented two unit tests to validate the new functionalities:
nullSafeValueWhenClassIsDateShouldReturnDate: Confirms that a JsonNode containing a timestamp correctly deserializes into a Date.
nullSafeValueWhenClassIsUUIDShouldReturnUUID: Validates that a JsonNode containing a UUID string deserializes into a UUID.
Benefits: These enhancements simplify the deserialization process by allowing the JsonObjectDeserializer to directly handle Date and UUID types, improving the usability of the class in applications that require these types.

@pivotal-cla
Copy link

@minwoo1999 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@minwoo1999 Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 31, 2024
@philwebb
Copy link
Member

Thanks for the pull-request, but I'm not sure that we should make such a change. Currently the nullSafeValue is intentionally limited to JSON primitives, and doesn't really do any conversion. I think generally the conversion should be a user concern. For example, you are converting a date using milliseconds from "the epoch", but a different representation may be equally valid.

Can you provide a bit more background about the reasons behind these proposed changes?

@philwebb philwebb added status: waiting-for-feedback We need additional information before we can continue status: on-hold We can't start working on this issue yet labels Oct 31, 2024
@minwoo1999
Copy link
Contributor Author

Thank you for your feedback!

I think the intention behind the proposed changes to the nullSafeValue method is to allow it to handle Date and UUID types. While it currently focuses on basic JSON primitives, many developers frequently work with dates and UUIDs in JSON data and expect seamless support for these types.

For instance, JSON APIs often use timestamps as milliseconds since the epoch, and being able to convert these directly into Date objects would simplify development. Similarly, since UUIDs are usually represented as strings, providing direct conversion would make working with them more convenient.

Ultimately, the goal is to enhance flexibility and usability, enabling developers to manage commonly used data types directly within the nullSafeValue method, leading to cleaner code and a more efficient workflow. Thank you for considering this perspective!

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 1, 2024
@philwebb
Copy link
Member

philwebb commented Nov 1, 2024

Thanks for the additional info @minwoo1999. I'm afraid I'm still not keen to merge this, but I do think you have a valid use-case so instead I've added a nullSafeValue method that accepts a mapper function (see #42972). That will allow you to write something like this:

Date date = nullSafeValue(tree.get("date"), Integer.class, Date::new);
UUID uuid = nullSafeValue(tree.get("uuid"), String.class, UUID::fromString);

@philwebb philwebb closed this Nov 1, 2024
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: on-hold We can't start working on this issue yet status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants