FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
399 stars 116 forks source link

use BigDecimalParser #237

Closed pjfanning closed 2 years ago

pjfanning commented 2 years ago

https://github.com/FasterXML/jackson-modules-java8/issues/236

pjfanning commented 2 years ago

@cowtowncoder I got this error in the CI build (java 11 only)

Error:    ZonedDateTimeSerTest.testSerializationAsTimestamp01NegativeSecondsWithDefaults:117 The value is not correct. expected:<1969-04-13T05:05:38.599Z[Etc/UTC]> but was:<1969-04-13T05:05:38.599Z[UTC]>

Seems unrelated to my change

kupci commented 2 years ago

It does seem unrelated, especially given Java 8 and Java 14 builds run successfully. Assuming it also fails without your changes, and if so, need to figure out if this was some change in Java 11, that then got restored in Java 14..

GedMarc commented 2 years ago

change the test suite from 14 to 17 and 18 please, the results of 11 are correct here -

14 is not the correct version to use on this due to

Accounting Currency Format Support

Currency format instances with accounting style, in which the amount is formatted in parentheses in some locales, can be obtained by calling NumberFormat.getCurrencyInstance(Locale)

and

core-libs/java.text
Plural Support in CompactNumberFormat

java.text.CompactNumberFormat is now capable of dealing with plural forms. For example, the number 2,000,000 is formatted to "2 Millionen" in LONG style, whereas 1,000,000 to "1 Million" in the German language.

And more, as an interim release a few options were tried to solve base issues with numbers (specifically negative) and the "compacting" and ISO- formula usage for faster calculations ;)

GedMarc commented 2 years ago

please ensure jdk 11 is post update 9 in the test case -

https://www.oracle.com/java/technologies/javase/11-0-9-bugfixes.html

pjfanning commented 2 years ago

will rebase this once https://github.com/FasterXML/jackson-modules-java8/pull/239 is merged - should tidy up the test issue

GedMarc commented 2 years ago

@cowtowncoder check comments on #239 - regarding the failures

Are you happy with a trampoline to the big decimal parser?
If so, I would like to centralize the java.time ser/der to OffsetTimeSerializer and trampoline through that as it is also my preferred mechanism, https://github.com/GedMarc/GuiceInjection/blob/master/src/main/java/com/guicedee/guicedinjection/json/LocalDateDeserializer.java#L46

cowtowncoder commented 2 years ago

@GedMarc what do you mean wrt "trampoline" here? (I recall it being used to refer to double indirection, just not sure what that'd be here)

Otherwise changes look good although instead of direct ref to BigDecimalParser, should go via NumberInput wrapper (just to encapsulate details).