search

odrotbohm / spring-restbucks

Implementation of the sample from REST in Practice based on Spring projects
1.21k stars 411 forks source link

Submitting Money input via HAL browser broken #63

Open otrosien opened 8 years ago

otrosien commented 8 years ago

Hi Oliver,

I have to bug you again for the REST API of money values. Polishing of my pull request (specifically https://github.com/olivergierke/spring-restbucks/blob/master/src/main/java/org/springsource/restbucks/JacksonCustomizations.java#L188) introduces LocaleContextHolder.getLocale() to obtain a money parser for a given locale. This does not fit anymore with the other part of the code (lines 123 ff) where you provide a regular expression pattern for the money value.

E.g. given locale "de-DE", I am expected to provide a money value like this: "EUR 123,45", which is rejected by the input validation inside the HAL browser (taking the regex from json schema iirc). So either stick to a fixed locale (I was using Locale.US in my PR, and that matches your regex), or remove the regular expression validation.

If you want, I can provide a test case that reproduces the bug.

odrotbohm commented 7 years ago

I wouldn't mind a fix even! :D I guess there's the fundamental question of how we'd create a proper regex depending on the Locale. The Money API unfortunately doesn't expose that yet so I filed a [ticket](MonetaryAmountFormat should expose regular expression that will match the format) for the JSR.

Not sure what a good workaround in the meantime could look like.

otrosien commented 7 years ago

Perfecfly valid choice. But I would question the idea to use locale-specific representations of money values in the API in the first place. There are other options. In my PR I used the root-locale so the format always looks like CurrencyCode + " " + NumberValue, quite easy/stable to parse and use from the client side. Another option could be to separate amount and currency into separate json properties. See zalando-money module: https://github.com/zalando/jackson-datatype-money

odrotbohm commented 7 years ago

There are, but I think if we go the length to return properly i18ned schema, the values should match the locale to. I stopped counting the number of times I was annoyed about software that has a German interface but exposes the rules of the US number formatting on me (dots VS. commas etc.).

It's perfectly fine not to do that, but I guess if one'd like to do that it should work. And even if you go with separated currency and amount, you're still left with the locale dependent formatting of the numbers.