Fix for the bug? that READ_ENUMS_USING_TO_STRING doesn't support null…

[andersonbd1]

… values.

[cowtowncoder]

I think it'd be best to file an issue first explaining what exactly is the problem, since that's bit unclear to me. That would make sure I understand problem being fixed (I know there's a unit test, which is good, but it shows what happens more than why), and then that the fix works for use case as well as overall design.

[andersonbd1]

Thanks for the quick response, cowtowncoder. I created an issue here:
#2309
I didn't add a lot more detail, though, as I'm not sure how else to explain it. The example in the test (with a linked real world example from facebook) demonstrates the problem - an enum with a null toString value causes exceptions, even if the value being deserialized isn't the null value.

[cowtowncoder]

Ok. My point is this: if you indicate that the value of toString() should be used as the JSON (String) representation, how should null be mapped to Strings: or are you suggesting that JSON null should then map to that Enum value? Or that it would be sort of alias for empty String? Or something else?

However... I am not sure I accept the premise that null return value is legal, however: although Object#toString() does not explicitly forbid that, intent seems clear to me -- null is not a valid "textual representation" of anything. So I'd need to be convinced as to why such value should be supported.

I could easily add an explicit exception to throw in this case, however, to indicate that it is invalid usage, if that would help.

[andersonbd1]

or maybe it should continue if e is null and not add anything to the map

[cowtowncoder]

I think I like "skip nulls" approach: will use that.

[andersonbd1]

w/out the supplied "fix", this throws an exception even through I'm not mapping the NULL enum.

[andersonbd1]

I think my expectation would probably be that null should map to the enum value EnumCustomerFileSource.NULL. However, given the weirdness of a null returned from toString (especially from an enum), I wouldn't be set on any expected behavior here. In fact, my code submission doesn't map null to the enum NULL - it only maps the string "null" to NULL, which isn't really correct, but at least lets me use jackson with this facebook api. It's a shallow solution to a problem that probably doesn't warrant a deeper solution. My main goal here was to allow READ_ENUMS_USING_TO_STRING to work in cases like this for the non-null values. As it is, simply the fact that one of the enums returns null means that an exception is thrown every time you attempt to map any enum value (including non-nulls) using READ_ENUMS_USING_TO_STRING. This facebook code is in github, so I'll comment over there as well.

[cowtowncoder]

Thank you for contributing this -- I finally found time to get back to this, and I think I agree with "skip the nulls" approach, as such entries seem wrong to me, and I think my preferences would be in order of

1. Skip nulls
2. Throw more descriptive exception when constructing deserializer, indicating broken type
3. Map null to ""

and (1) seems best just because one does not always have control over types, so bit of defensiveness seems appropriate.

I will actually make changes to 2.10, mostly because 2.9 branch is almost closed and I want to minimize changes there. I will use test included, and do simple patch -- thank you again for suggesting it.

[andersonbd1]

Thanks, @cowtowncoder !