Global enum naming strategy to use when it's not set directly on a class level #4716
Conversation
| CacheProvider cacheProvider, JsonNodeFactory nodeFactory, | ||
| ConstructorDetector ctorDetector) | ||
| public BaseSettings(AnnotationIntrospector ai, PropertyNamingStrategy pns, | ||
| EnumNamingStrategy ens, AccessorNamingStrategy.Provider accNaming, |
There was a problem hiding this comment.
Please don't make source incompatible changes. Add a new constructor and deprecate the old one.
| * | ||
| * @param s Strategy instance to use | ||
| * | ||
| * @return Builder instance itself to allow chaining |
There was a problem hiding this comment.
add @since 2.18 to all new public methods (including the ones that you didn't add javadoc to)
| * @throws IllegalArgumentException if {@code namingDef} is not an instance of {@link java.lang.Class} or | ||
| * not a subclass of {@link EnumNamingStrategy}. | ||
| */ | ||
| public static EnumNamingStrategy createEnumNamingStrategyInstance(Object namingDef, boolean canOverrideAccessModifiers) { | ||
| public static EnumNamingStrategy createEnumNamingStrategyInstance(Object namingDef, boolean canOverrideAccessModifiers, EnumNamingStrategy defaultNamingStrategy) { |
There was a problem hiding this comment.
add back the old method - this is an incompatible change to the API
|
Makes sense overall (although +1 for concerns @pjfanning pointed out; cannot just change signatures of methods that may be in use). |
| @Test | ||
| void testUseEnumMappingStrategySetInMapper() { | ||
| ObjectMapper mapper = jsonMapperBuilder() | ||
| .enumNamingStrategy(EnumNamingStrategies.CamelCaseStrategy.INSTANCE) |
There was a problem hiding this comment.
This seems awkward.
How about .enumNamingStrategy(EnumNamingStrategies.CamelCaseStrategy.class) then handle the instance internally?
There was a problem hiding this comment.
No, I think strategy passed should be instance, not class. This is used for most configuration; mapper/builder not having to dynamically construct instances.
If we want instantiation, should instead pass factory; Class only to be used in cases where we must (from Annotation where you cannot indicate instance but only Class).
There was a problem hiding this comment.
We could just ask for new EnumNamingStrategies.CamelCaseStrategy(), but if there's already INSTANCE can use that.
|
Hey @pjfanning, @cowtowncoder, thanks for the review |
|
Needs to be-based against 2.19 now. |
Closes #4674