Custom property naming in present property json type resolver #1
Conversation
| return constant.toString().toLowerCase(); | ||
| } | ||
|
|
||
| private List<Type> getAllInterfaces(Class<?> type) { |
There was a problem hiding this comment.
This method is copied from JsonTypeResolverFactory. Maybe it should extracted to some util class? Or maybe assume that the NamedPropertyTypeProvider have to be implemented directly on enum - then, instead of this method, calling constant.getClass().getInterfaces() would be enough. What do you think?
|
|
||
| public interface NamedPropertyTypeProvider extends TypeProvider { | ||
|
|
||
| String getJsonPropertyName(); |
There was a problem hiding this comment.
This kind of flexibility doesn't make sense to me, it seems equal to just implementing the JsonTypeResolver manually on the use case site.
There was a problem hiding this comment.
Maybe in simple case, when you have one set of types (like one TypeProvider). But if you have N sets of types, you add only N NamedPropertyTypeProvider - all handled by one PresentPropertyJsonTypeResolver. Implementing it on use case side would require either N custom resolvers (all very similar) or one generic - but here resolving a target type need to be added (as it is already done in JsonTypeResolverFactory).
| @@ -14,11 +14,33 @@ public PresentPropertyJsonTypeResolver(Class<E> type) { | |||
| @Override | |||
| public final Class<?> resolve(Map<String, Object> json) { | |||
| for (E constant : type.getEnumConstants()) { | |||
| if (json.containsKey(constant.toString().toLowerCase())) { | |||
| if (json.containsKey(resolvePropertyName(constant))) { | |||
There was a problem hiding this comment.
It's now evident that default use case of camelCase field to camelCase json property doesn't work (it works only for simple names with which there's no casing actually).
I suggest you add guava(29.0-jre) to this project and introduce CaseFormat field to this class which should be overridable by a specific constructor and for current one the default should be CaseFormat.LOWER_CAMEL.
Then instead of new resolvePropertyName(constant) and related new infrastructure CaseFormat.UPPER_UNDERSCORE.to(caseFormat, constant.toString()) on this line should be sufficient.
| if (json.containsKey(resolvePropertyName(constant))) { | |
| if (json.containsKey(CaseFormat.UPPER_UNDERSCORE.to(caseFormat, constant.toString()))) { |
There was a problem hiding this comment.
Ok, I'll try with this approach 👍
There was a problem hiding this comment.
Fixed as you suggested. Tests as well as readme tuned to reflect current changes.
README.md
Outdated
| @Value | ||
| static class RoadBike implements Bike { | ||
|
|
||
| private final String road_bike; |
There was a problem hiding this comment.
it's never a convention to use snakeCase in field names, you should use @JsonNaming instead
marek-sontag
deleted the
custom-property-naming-in-PresentPropertuJsonTypeResolver
branch
Although
PresentPropertyJsonTypeResolverworks well for lowercase json properties (e.g. in json: "foo", inTypeProvider: "FOO"), the problem shows up when json properties are in camel case or with any character not allowed in enum values names (e.g. json property is "userName").Here I introducedNamedPropertyTypeProviderwhich one can use in such cases.PresentPropertyJsonTypeResolvercan works with both kind of providers.