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

DefaultDateTypeAdapterTest fails on JDK 8 and 17 #2199

Closed
eamonnmcmanus opened this issue Sep 23, 2022 · 5 comments
Closed

DefaultDateTypeAdapterTest fails on JDK 8 and 17 #2199

eamonnmcmanus opened this issue Sep 23, 2022 · 5 comments
Labels

Comments

@eamonnmcmanus
Copy link
Member

[ERROR] com.google.gson.internal.bind.DefaultDateTypeAdapterTest.testParsingDatesFormattedWithSystemLocale  Time elapsed: 0.015 s  <<< ERROR!
com.google.gson.JsonSyntaxException: Failed parsing '1 janv. 1970 à 00:00:00' as Date; at path $
        at com.google.gson.internal.bind.DefaultDateTypeAdapter.deserializeToDate(DefaultDateTypeAdapter.java:164)
        at com.google.gson.internal.bind.DefaultDateTypeAdapter.read(DefaultDateTypeAdapter.java:147)
        at com.google.gson.internal.bind.DefaultDateTypeAdapter.read(DefaultDateTypeAdapter.java:47)
        at com.google.gson.TypeAdapter.fromJson(TypeAdapter.java:263)
        at com.google.gson.TypeAdapter.fromJson(TypeAdapter.java:279)
        at com.google.gson.internal.bind.DefaultDateTypeAdapterTest.assertParsed(DefaultDateTypeAdapterTest.java:210)
        at com.google.gson.internal.bind.DefaultDateTypeAdapterTest.testParsingDatesFormattedWithSystemLocale(DefaultDateTypeAdapterTest.java:85)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at junit.framework.TestCase.runTest(TestCase.java:177)
        at junit.framework.TestCase.runBare(TestCase.java:142)
        at junit.framework.TestResult$1.protect(TestResult.java:122)
        at junit.framework.TestResult.runProtected(TestResult.java:142)
        at junit.framework.TestResult.run(TestResult.java:125)
        at junit.framework.TestCase.run(TestCase.java:130)
        at junit.framework.TestSuite.runTest(TestSuite.java:241)
        at junit.framework.TestSuite.run(TestSuite.java:236)
        at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
        at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:377)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:284)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:248)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:167)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
        at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
        at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581)
Caused by: java.text.ParseException: Failed to parse date ["1 janv. 1970 à 00:00:00"]: Invalid number: 1 ja
        at com.google.gson.internal.bind.util.ISO8601Utils.parse(ISO8601Utils.java:279)
        at com.google.gson.internal.bind.DefaultDateTypeAdapter.deserializeToDate(DefaultDateTypeAdapter.java:162)
        ... 27 more
Caused by: java.lang.NumberFormatException: Invalid number: 1 ja
        at com.google.gson.internal.bind.util.ISO8601Utils.parseInt(ISO8601Utils.java:323)
        at com.google.gson.internal.bind.util.ISO8601Utils.parse(ISO8601Utils.java:133)
        ... 28 more
@MaicolAntali
Copy link
Contributor

MaicolAntali commented Dec 13, 2022

I have debug this test and I have figure out that the problem is the date format.
The date format has changed from: 1 janv. 1970%s00:00:00 to: 1 janv. 1970, 00:00:00%s

Is it ok if i fix this with a ternary operator?
For example, somethings like JavaVersion.isJava9OrLater() ? "newDateFormat" : "oldDateFormat"

@eamonnmcmanus
Copy link
Member Author

I think that if we change the test so that it passes on all three tested platforms (JDK 8, 11, and 17) that would definitely be an improvement. It's still concerning that the same date string doesn't parse on all platforms, but I'm not sure how or whether we can fix that. At least making the per-platform strings explicit would be better than simply not running the test on platforms other than 11. So yes, if you want to investigate that change that would be a great contribution. Thanks in advance!

@MaicolAntali
Copy link
Contributor

Theoretical part

From Java 9 the default format for the date is the Unicode Common Local Date Repository (CLDR).

As written here to enable the behavior compatiple with JDK 8, you have to set the system property to: java.locale.providers=COMPAT,CLDR (The default system property value is: java.locale.providers=CLDR,COMPAT,SPI).

But it’s not that simple because the CLDR is available in versions, and not the same version is included with the different Java versions. That means potentially every Java version can format the date in different way.

Here an example:
In Java 8 (with the CLRD system property set) a date is formatted in this way: Tue, 16 Jul 2019 14:35:02 AEST
In Java 9 (CLRD is set by default) a date is formatted in this different way: Tue., 16 Jul. 2019 14:35:52 AEST

Between the two dates there are only small differences but is enough to make the test fail because if the date pattern change also the date format should change.

In fact, to pass the test (as i write above) it was enough to change the date format to pass the test ( from: 1 janv. 1970%s00:00:00 to: 1 janv. 1970, 00:00:00%s).

Practical part

Probably the first thing that we can do is say "goodbye" to the old date-time API and use the new one: java.time. Maybe just the switch to the new API can solve some problems.

Or we can keep use if/ternary operator to identify the java version and pass the correct date format but this is not a solution for the problem so i hope we don't go for it.

@eamonnmcmanus
Copy link
Member Author

If these long-format dates are parsed differently on the different platforms then the easiest thing is probably not to try to parse them. I think the short-format dates work everywhere and the ones with month names in them suffice to prove that the default locale is being used. Or alternatively, rewrite the test so that it obtains the date string to be parsed using the same kind of DateFormat that will be used for parsing, rather than having hard-coded strings.

@eamonnmcmanus
Copy link
Member Author

Thanks for the very detailed analysis, by the way! It's very helpful.

MaicolAntali added a commit to MaicolAntali/gson that referenced this issue Dec 16, 2022
tibor-universe pushed a commit to getuniverse/gson that referenced this issue Jan 20, 2023
…#2199 (google#2287)

* Rewrite the `testParsingDatesFormattedWithSystemLocale()`, Fix google#2199

* Format the test

* Format the code following Google Java Style Guide

* Revert "Format the code following Google Java Style Guide"

This reverts commit f5e2e16.

(cherry picked from commit 6c12ded)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants