Allow default enums with @JsonCreator
#4979
Conversation
src/main/java/com/fasterxml/jackson/databind/deser/std/FactoryBasedEnumDeserializer.java
Show resolved
Hide resolved
src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumDeserializationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumDeserializationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumDeserializationTest.java
Outdated
Show resolved
Hide resolved
|
Looks like there's one unit test regression with NPE. |
This follows the pattern for READ_UNKNOWN_ENUM_VALUES_AS_NULL from here: https://github.com/FasterXML/jackson-databind/pull/1642/files but for READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE It's more tricky because it wasn't immediately apparent how to get the configured default value on the Creator impl side. I decided to try using the EnumResolver, which works, but am not sure what the repercussions of that are fully though. It works the same way, so only IllegalArgumentExceptions will trigger default behavior, so if you have some other custom creator exception logic that will be unaffected.
dropofwill
force-pushed
the
unknown-with-creator-default-value
branch
from
db9fb7f to
143b66f
Compare
|
|
dropofwill
changed the title
cowtowncoder
added
the
cla-needed
|
Aside from test fail(s), one thing before I can merge this PR (after review) is CLA from here: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf (unless already sent). Needs to be sent just once, good for all future contributions. Looking forward to getting this merged, thank you! |
|
We have compile error now |
|
Yeah, made a typo when fixing something. I’ll fix it and sign the CLA next week. |
src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java
Show resolved
Hide resolved
| _defaultValue = Objects.nonNull(enumResolver)? enumResolver.getDefaultValue() : null; | ||
| } | ||
|
|
||
| public FactoryBasedEnumDeserializer(Class<?> cls, AnnotatedMethod f, JavaType paramType, |
There was a problem hiding this comment.
Needs to be marked @deprecated (will add)
|
Ok, I think we'd be ready, once CLA is received! |
|
Sorry for the delay, thanks for making those updates. I've sent over the CLA. |
cowtowncoder
added
cla-received
|
CLA received: can proceed today with final review and merging. Thank you, @dropofwill ! |
| @@ -202,6 +224,10 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx | |||
| if (ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL)) { | |||
| return null; | |||
| } | |||
|
|
|||
| if (ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE)) { | |||
There was a problem hiding this comment.
I'll move this before as null... plus, should only return non-null _defaultValue, I think.
cowtowncoder
changed the title
@JsonCreator
This follows the pattern for READ_UNKNOWN_ENUM_VALUES_AS_NULL from here:
https://github.com/FasterXML/jackson-databind/pull/1642/files
but for READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE
It's more tricky because it wasn't immediately apparent how to get the configured default value on the Creator impl side. I decided to try using the EnumResolver, which works, but am not sure what the repercussions of that are fully though.
It works the same way, so only IllegalArgumentExceptions will trigger default behavior, so if you have some other custom creator exception logic that will be unaffected.
There may be a better way to do this, just wanted to open this POC to see what you think. This would be useful in a couple scenarios for apps I work on, but I can work around it if this is not desired/too risky.