(javax-money) Report bad CurrencyUnit input as databind exception rather than UnknownCurrencyException/NPE - #95
Open
pjfanning wants to merge 2 commits into
Conversation
…not `UnknownCurrencyException`/NPE
`CurrencyUnitDeserializer` did no token check and no error handling:
final String currencyCode = parser.getValueAsString();
return Monetary.getCurrency(currencyCode);
so provider exceptions escaped to the caller instead of a `JacksonException`:
"XYZQ" -> javax.money.UnknownCurrencyException
{"a":1} -> java.lang.NullPointerException: Currency Code may not be null
(`getValueAsString()` returns `null` for structured values, which `Monetary`
then rejects with an NPE.)
Handle both cases the way joda-money's `CurrencyUnitDeserializer` already does:
`handleWeirdStringValue()` for an unusable code and `handleUnexpectedToken()`
for a non-String token. This also lets a `DeserializationProblemHandler`
substitute a value.
NOTE: behaviour change. `UnknownCurrencyException` is now reported as
`InvalidFormatException`; the two existing tests asserting the old type are
updated, in both `javax-money` and `moneta`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CurrencyUnitDeserializerdoes no token check and no error handling:so provider exceptions escape to the caller instead of a
JacksonException:CurrencyUnit"XYZQ"javax.money.UnknownCurrencyException: Unknown currency code: XYZQ{"a":1}java.lang.NullPointerException: Currency Code may not be null12UnknownCurrencyException: Unknown currency code: 12(silently coerced)getValueAsString()returnsnullfor structured values, whichMonetarythen rejects with an NPE — so a caller catchingJacksonExceptionsees neither.The sibling joda-money module's
CurrencyUnitDeserializeralready does this correctly, so the two modules in this repo disagree on error handling for the same conceptual type.Fix
Mirror joda-money:
handleWeirdStringValue()for an unusable currency code,handleUnexpectedToken()for a non-String token. As a bonus this routes throughDeserializationProblemHandler, so applications can substitute a value.UnknownCurrencyExceptionis now reported asInvalidFormatException(aMismatchedInputException, soJacksonException). The existingshouldNotDeserializeInvalidCurrencytest asserted the old type, in bothjavax-moneyandmoneta; both are updated here.Flagging explicitly since it is user-visible — happy to drop the
handleWeirdStringValuehalf and keep only the non-String-token fix if you would rather preserveUnknownCurrencyExceptionfor3.x.Tests
Updated
shouldNotDeserializeInvalidCurrency(×2 modules) plus new cases for number/object/array input.javax-money138 tests andmoneta140 tests all green.