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 over EnumNamingStrategy for Enum serialization #4040

Merged
merged 1 commit into from
Jul 16, 2023
Merged

Use @JsonProperty over EnumNamingStrategy for Enum serialization #4040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/fasterxml/jackson/databind/ser/std/EnumSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,6 @@ protected static EnumValues constructEnumNamingStrategyValues(SerializationConfi
EnumNamingStrategy enumNamingStrategy = EnumNamingStrategyFactory.createEnumNamingStrategyInstance(
namingDef, config.canOverrideAccessModifiers());
return enumNamingStrategy == null ? null : EnumValues.constructUsingEnumNamingStrategy(
config, enumClass, enumNamingStrategy);
config, annotatedClass, enumNamingStrategy);
}
}
38 changes: 38 additions & 0 deletions src/main/java/com/fasterxml/jackson/databind/util/EnumValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,52 @@ public static EnumValues constructFromToString(MapperConfig<?> config, Class<Enu
return construct(config, enumClass, external);
}

/**
* Returns String serializations of Enum name using an instance of {@link EnumNamingStrategy}.
* <p>
* The output {@link EnumValues} should contain values that are symmetric to
* {@link EnumResolver#constructUsingEnumNamingStrategy(DeserializationConfig, AnnotatedClass, EnumNamingStrategy)}.
*
* @since 2.16
*/
public static EnumValues constructUsingEnumNamingStrategy(MapperConfig<?> config, AnnotatedClass annotatedClass,
EnumNamingStrategy namingStrategy)
{
// prepare data
final AnnotationIntrospector ai = config.getAnnotationIntrospector();
final Class<?> enumCls0 = annotatedClass.getRawType();
final Class<Enum<?>> enumCls = _enumClass(enumCls0);
final Enum<?>[] enumConstants = _enumConstants(enumCls0);

// introspect
String[] names = new String[enumConstants.length];
if (ai != null) {
ai.findEnumValues(config, annotatedClass, enumConstants, names);
}

// build
SerializableString[] textual = new SerializableString[enumConstants.length];
for (int i = 0, len = enumConstants.length; i < len; i++) {
Enum<?> enumValue = enumConstants[i];
String name = names[i];
if (name == null) {
name = namingStrategy.convertEnumToExternalName(enumValue.name());
}
textual[i] = config.compileString(name);
Copy link
Contributor Author

@iProdigy iProdigy Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
textual[i] = config.compileString(name);
if (config.isEnabled(EnumFeature.WRITE_ENUMS_TO_LOWERCASE)) {
name = name.toLowerCase();
}
textual[i] = config.compileString(name);

Should WRITE_ENUMS_TO_LOWERCASE apply here? (my opinion is no when naming strategy is used... however @JsonProperty is lowercased in constructFromName)

Copy link
Member

Choose a reason for hiding this comment

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

Depends on how PropertyNamingStategy version does it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is a lowercase SerializationFeature for the general case

Copy link
Member

Choose a reason for hiding this comment

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

Correct: lower-casing is datatype-specific, no SerializationFeature (nor will there ever be).

The main question is interesting one; I can see arguments either way.
I think that since EnumFeature.WRITE_ENUMS_TO_LOWERCASE is slightly more dynamic (Enum naming strategy being defined as annotations in class definition or mix-in), it should have precedence and, when enabled, force lower-casing.

Copy link
Member

Choose a reason for hiding this comment

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

So I think code as-is does what is my leaning. Great!

Copy link
Contributor Author

@iProdigy iProdigy Jul 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@cowtowncoder the current code doesn't force lower-casing, I think you're looking at the suggested diff

Edit: created a PR for this - if I misunderstood your comment, feel free to close it

Copy link
Member

Choose a reason for hiding this comment

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

@iProdigy Ok I must have thought suggestion was applied and not looked at actual change. Thank you for following through, I just merged follow-up PR!

}
return construct(enumCls, textual);
}

/**
* Returns String serializations of Enum name using an instance of {@link EnumNamingStrategy}.
*
* The output {@link EnumValues} should contain values that are symmetric to
* {@link EnumResolver#constructUsingEnumNamingStrategy(DeserializationConfig, Class, EnumNamingStrategy)}.
*
* @since 2.15
* @deprecated Since 2.16; use {@link #constructUsingEnumNamingStrategy(MapperConfig, AnnotatedClass, EnumNamingStrategy)} instead.
*/
@Deprecated
public static EnumValues constructUsingEnumNamingStrategy(MapperConfig<?> config, Class<Enum<?>> enumClass, EnumNamingStrategy namingStrategy) {
Class<? extends Enum<?>> cls = ClassUtil.findEnumType(enumClass);
Enum<?>[] values = cls.getEnumConstants();
Expand Down
15 changes: 15 additions & 0 deletions src/test/java/com/fasterxml/jackson/databind/ser/jdk/EnumNamingSerializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ static enum EnumFlavorD {
PEANUT_BUTTER
}

@EnumNaming(EnumNamingStrategies.CamelCaseStrategy.class)
static enum EnumFlavorE {
PEANUT_BUTTER,
@JsonProperty("almond")
ALMOND_BUTTER
}

static class EnumFlavorWrapperBean {
public EnumSauceB sauce;

Expand Down Expand Up @@ -102,4 +109,12 @@ public void testDesrEnumWithEnumMap() throws Exception {

assertEquals(a2q("{'mayoNezz':'value'}"), str);
}

public void testEnumNamingStrategyWithOverride() throws Exception {
String almond = MAPPER.writeValueAsString(EnumFlavorE.ALMOND_BUTTER);
assertEquals(q("almond"), almond);

String peanut = MAPPER.writeValueAsString(EnumFlavorE.PEANUT_BUTTER);
assertEquals(q("peanutButter"), peanut);
}
}