Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: BigDecimal serialized with comma as decimal divider #319

Open
Simulant87 opened this issue Aug 29, 2019 · 3 comments · May be fixed by #320
Open

Bug: BigDecimal serialized with comma as decimal divider #319

Simulant87 opened this issue Aug 29, 2019 · 3 comments · May be fixed by #320
Labels
bug Something isn't working right

Comments

@Simulant87
Copy link
Contributor

Simulant87 commented Aug 29, 2019

when I execute the Test org.eclipse.yasson.documented.DocumentationExampleTest#testDateNumberFormats1 on my local system, it fails because the BigDecimal gets serialized to 123,46 instead of 123.46 containing a comma instead of a dot as decimal divider.

This might be caused by the default localization settings (german) of my local system in combination with the used test class adding a Numberformat to the field @JsonbNumberFormat("#0.00"), but I would expect the test still not to fail depending on my local settings.

I think it is a bug as the JSON specification only allows the dot . as decimal dot and not the comma , referencing to the number section https://tools.ietf.org/html/rfc7159#section-6
So the BigDecimal 123.46 should never, even not with my german local settings get serialized to 123,46 as this results in invalid JSON.

Full JUnit output of the test org.eclipse.yasson.documented.DocumentationExampleTest#testDateNumberFormats1

org.junit.ComparisonFailure: 
Expected :{"birthDate":"07.08.1999","name":"Jason Bourne","salary":"123.46"}
Actual   :{"birthDate":"07.08.1999","name":"Jason Bourne","salary":"123,46"}
@aguibert
Copy link
Member

thanks for raising this issue @Simulant87, I've also raised jakartaee/jsonb-api#188 on the JSON-B spec so we can clarify this at the spec level in the future

@Simulant87
Copy link
Contributor Author

Thank you for taking this onto the spec level, after I saw existing tests failing, I already had the feeling that this should be clarified on a higher level.

@Simulant87
Copy link
Contributor Author

Simulant87 commented Aug 31, 2019

@aguibert I had again a look at the tests and now I am even thinking about rejecting my own merge request. As the input and the output of the BigDecimal is stored as a String (in quotes) in JSON the localized formatting is totally valid also in JSON as it is not a Number ( without quotes and only allowing .) but a String, that on the other side might be parsed with the same localized settings and could fail if this is not applied as expected.

I was just confused by the default picking up the system default locale to format the BigDecimal String representation and as a result the tests are not system (locale) independent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants