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.

Sign up for GitHub

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

Jump to bottom

Use @JsonProperty and lowercase feature when serializing Enums despite write using toString() #4039

Merged
merged 5 commits into from
Jul 15, 2023
Merged

Use @JsonProperty and lowercase feature when serializing Enums despite write using toString() #4039

merged 5 commits into from
Jul 15, 2023

Conversation

iProdigy
Copy link
Contributor

Serialization complement of #4036

@iProdigy
Copy link
Contributor Author

@JooHyukKim Could you also review this PR please?

JooHyukKim
JooHyukKim reviewed Jul 15, 2023
View reviewed changes
src/main/java/com/fasterxml/jackson/databind/ser/std/EnumSerializer.java
*
* @since 2.16
*/
protected final EnumValues _valuesByToString;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not confident about readability perspective, but is it possible to use the same/similar structure as other EnumValues members' javadoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the review - can you elaborate on what you mean here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally thought it will be easier to read if javadoc of _values, _valuesByEnumNaming, and _valuesByToString had similarly structured paragraph and use similar terms. But can skip my comment, thanks 👍🏻

@iProdigy iProdigy mentioned this pull request Jul 15, 2023
Merged
2 tasks
cowtowncoder
cowtowncoder reviewed Jul 15, 2023
View reviewed changes
src/main/java/com/fasterxml/jackson/databind/ser/std/EnumSerializer.java
@@ -82,6 +92,20 @@ public EnumSerializer(EnumValues v, Boolean serializeAsIndex, EnumValues valuesB
_values = v;
_serializeAsIndex = serializeAsIndex;
_valuesByEnumNaming = valuesByEnumNaming;
_valuesByToString = null;
Copy link
Member

@cowtowncoder cowtowncoder Jul 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should just use this(v, serializeAsIndex, valuesByEnumNaming, null); ?

Also let's add @Deprecated (and Javadoc) here

cowtowncoder
cowtowncoder reviewed Jul 15, 2023
View reviewed changes
src/main/java/com/fasterxml/jackson/databind/ser/std/EnumSerializer.java
@@ -71,6 +80,7 @@ public EnumSerializer(EnumValues v, Boolean serializeAsIndex)
_values = v;
_serializeAsIndex = serializeAsIndex;
_valuesByEnumNaming = null;
_valuesByToString = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can just delegate (with this(v, serializeAsIndex, null, null)).

Also, unless called from non-deprecated code, let's also mark as @Deprecated // since 2.16.

cowtowncoder
cowtowncoder approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; just need to add some more Deprecation markers & could chain from deprecated constructors.

Actually, I can do these post-merge, so approving & merging now.

@cowtowncoder cowtowncoder merged commit 3c48461 into FasterXML:2.16 Jul 15, 2023
@cowtowncoder cowtowncoder changed the title feat(enum): use JsonProperty and lowercase feature despite write using toString Use @JsonProperty and lowercase feature when serializing Enums despite write using toString Jul 15, 2023
@cowtowncoder cowtowncoder changed the title Use @JsonProperty and lowercase feature when serializing Enums despite write using toString Use @JsonProperty and lowercase feature when serializing Enums despite write using toString() Jul 15, 2023
cowtowncoder added a commit that referenced this pull request Jul 15, 2023
@cowtowncoder
Update release notes wrt #4039
ca47ea3
@iProdigy iProdigy deleted the fix/enum-write-tostring-jsonproperty branch July 15, 2023 23:31
cowtowncoder added a commit that referenced this pull request Jul 16, 2023
@cowtowncoder
Minor additions post #4039
1ae7dc4
@JooHyukKim JooHyukKim mentioned this pull request Jul 14, 2023
Closed
@YutaHiguchi-bsn YutaHiguchi-bsn mentioned this pull request Feb 1, 2024
Closed
1 task
@pjfanning pjfanning mentioned this pull request Jun 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JooHyukKim JooHyukKim JooHyukKim left review comments

@cowtowncoder cowtowncoder cowtowncoder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iProdigy @cowtowncoder @JooHyukKim