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

[TCK Challenge]: ee.jakarta.tck.json.bind.customizedmapping.numberformat.NumberFormatCustomizationTest #360

Open
struberg opened this issue Oct 20, 2024 · 1 comment
Labels
challenge A challenge to the TCK

Comments

@struberg
Copy link

Per the current TCK Process documentation, please find bellow a challenge related to JSON-B TCK. Can someone please add the challenge label which is required by the TCK Process but not available to everyone to add.

The relevant specification version and section number(s)

https://jakarta.ee/specifications/jsonb/3.0/jakarta-jsonb-spec-3.0.html#custom-number-format

Only following test in the class are challenged

  • testNumberFormatAccessors:135
  • testNumberFormatPackageTypeOverride:159
  • testNumberFormatTypeFieldOverride:183

The exact TCK version

JSON-B TCK version 3.0.0

The implementation being tested, including name and company

Apache Johnzon 2.0.0-SNAPSHOT from the Apache Software Foundation
https://github.com/apache/johnzon/

A full description of why the test is invalid and what the correct behavior is believed to be

The tests check for the output of a custom @JsonbNumberFormat(value = "###,###.##") in various places.
Common for all of them is that the tests do not specify a Locale.
And since the JavaDoc of the JsonNumberFormat defines that DecimalFormat is to be used the result is obviously dependent on the Locale of the environment in which you run the TCK.

You can try this out yourself: If you run new DecimalFormat("###,####.##").format(123123123.3434d) with Locale.USyou'll end up with "1,2312,3123.34".
But when running it over here in Austria with the Locale.ROOT (##default) I get the String "1 2312 3123,34".

I didn't find any requirement in the TCK documents to specify a certain Locale when running the JSONB TCK tests.
Thus I assume they should run fine in any Locale.

The TCK tests in question can be fixed by using JsonbConfig#withLocale(Locale.US) or anything else which pinpoints it to a country.
Or by specifying a certain Locale in the JsonbNumberFormat annotations in question.

Acutally I think both ways should be covered by the TCK.
But that would maybe rather be an enhancement for future TCK versions and not a Challenge, right?

And this should also be pointed to in the Javadoc of JsonbNumberFormat because usually you want your serialised JSON not to depend on the Locale the servers or clients runs in, right? I'll submit a clarification PR for the API.

Btw, this might also explain the troubles you have with other tests in this area which you tried fixing with a COMPAT setting?

struberg added a commit to struberg/jsonb-api that referenced this issue Oct 20, 2024
A @JsonbNumberFormat(value = "###,###.##") without any specified locale
results in a JSON serialisation which is depending on the Locale of the
box you run the TCK on. This is not reproducible.

There are 2 ways to define the Locale:
* with the JsonbNumberFormat#locale attribute
* with JsonbConfig#withLocale

This test at least pins down the Locale to guarantee reproducible builds.
@jungm
Copy link

jungm commented Oct 20, 2024

Btw, this might also explain the troubles you have with other tests in this area which you tried fixing with a COMPAT setting?

Actually not, the problematic test there (#272) is explicitly using the french locale:

    @JsonbNumberFormat(locale = "fr")
    private Double instance;

Which is formatted differently now because OpenJDK switched from their legacy locale data to CLDR in Java 13, so the issue there isn't an ambiguous test but an OpenJDK change breaking it

asfgit pushed a commit to apache/johnzon that referenced this issue Oct 20, 2024
The TCK test for JsonbNumberFormat does not specify a Locale and thus
creates random results depending on the Locale the TCK box is set up with.
See jakartaee/jsonb-api#360
@njr-11 njr-11 added the challenge A challenge to the TCK label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
challenge A challenge to the TCK
Projects
None yet
Development

No branches or pull requests

3 participants