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

Jackson integration tests fail if TZ is west of GMT #37662

Closed
dmlloyd opened this issue Dec 11, 2023 · 4 comments · Fixed by #37668
Closed

Jackson integration tests fail if TZ is west of GMT #37662

dmlloyd opened this issue Dec 11, 2023 · 4 comments · Fixed by #37668
Labels
area/jackson Issues related to Jackson (JSON library) kind/bug Something isn't working
Milestone

Comments

@dmlloyd
Copy link
Member

dmlloyd commented Dec 11, 2023

Describe the bug

Test fails if time zone is west of GMT:

[ERROR] io.quarkus.it.jackson.DateDeserializerPojoResourceTest.testSqlDate -- Time elapsed: 0.022 s <<< FAILURE!
java.lang.AssertionError: 
1 expectation failed.
JSON path date doesn't match.
Expected: a string starting with "1970-"
  Actual: 1969-12-30

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at org.codehaus.groovy.reflection.CachedConstructor.invoke(CachedConstructor.java:73)
	at org.codehaus.groovy.runtime.callsite.ConstructorSite$ConstructorSiteNoUnwrapNoCoerce.callConstructor(ConstructorSite.java:108)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallConstructor(CallSiteArray.java:57)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:263)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callConstructor(AbstractCallSite.java:277)
	at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure.validate(ResponseSpecificationImpl.groovy:512)
	at io.restassured.internal.ResponseSpecificationImpl$HamcrestAssertionClosure$validate$1.call(Unknown Source)
	at io.restassured.internal.ResponseSpecificationImpl.validateResponseIfRequired(ResponseSpecificationImpl.groovy:696)
	at io.restassured.internal.ResponseSpecificationImpl.this$2$validateResponseIfRequired(ResponseSpecificationImpl.groovy)
	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 org.codehaus.groovy.runtime.callsite.PlainObjectMetaMethodSite.doInvoke(PlainObjectMetaMethodSite.java:43)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite$PogoCachedMethodSiteNoUnwrapNoCoerce.invoke(PogoMetaMethodSite.java:198)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:62)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:185)
	at io.restassured.internal.ResponseSpecificationImpl.body(ResponseSpecificationImpl.groovy:270)
	at io.restassured.specification.ResponseSpecification$body$1.callCurrent(Unknown Source)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:49)
	at io.restassured.specification.ResponseSpecification$body$1.callCurrent(Unknown Source)
	at io.restassured.internal.ResponseSpecificationImpl.body(ResponseSpecificationImpl.groovy:117)
	at io.restassured.internal.ValidatableResponseOptionsImpl.body(ValidatableResponseOptionsImpl.java:244)
	at io.quarkus.it.jackson.DateDeserializerPojoResourceTest.testSqlDate(DateDeserializerPojoResourceTest.java:42)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1013)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:827)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Expected behavior

Test should pass in all local time zones.

Actual behavior

The test only passes in certain time zones with a positive offset.

How to Reproduce?

Run TZ=America/Chicago mvn install -rf :quarkus-integration-test-jackson after a clean build e.g. mvn clean && mvn install -DskipTests.

Output of uname -a or ver

Darwin xxxxx 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:28:45 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6020 arm64

Output of java -version

openjdk version "17.0.9" 2023-10-17 OpenJDK Runtime Environment Temurin-17.0.9+9 (build 17.0.9+9) OpenJDK 64-Bit Server VM Temurin-17.0.9+9 (build 17.0.9+9, mixed mode)

Quarkus version or git rev

0d90b7c

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.5 (57804ffe001d7215b5e7bcb531cf83df38f93546) Maven home: /Users/david/local/apache-maven Java version: 17.0.9, vendor: Eclipse Adoptium, runtime: /Library/Java/JavaVirtualMachines/temurin-17.jdk/Contents/Home Default locale: en_US, platform encoding: UTF-8 OS name: "mac os x", version: "14.1.2", arch: "aarch64", family: "mac"

Additional information

No response

@dmlloyd dmlloyd added the kind/bug Something isn't working label Dec 11, 2023
@quarkus-bot quarkus-bot bot added the area/jackson Issues related to Jackson (JSON library) label Dec 11, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 11, 2023

/cc @geoand (jackson), @gsmet (jackson)

@dmlloyd
Copy link
Member Author

dmlloyd commented Dec 11, 2023

Rather than testing an exact time stamp string, the test checks that the date starts with 1970-, presumably exactly because it expects an unknown time zone offset (however it wrongly assumes that the unknown time zone offset will always be positive, i.e. the test will not work in the Americas). I'm looking into what the actual cause for the difference between Timestamp and Date tests are (since the former passes and the latter does not).

@dmlloyd
Copy link
Member Author

dmlloyd commented Dec 11, 2023

It looks like this problem is much worse than expected. Within Jackson we have this ominous note (see com.fasterxml.jackson.databind.ser.std.SqlDateSerializer#serialize):

        // Alas, can't just call `_serializeAsString()`....
        if (_customFormat == null) {
            // 11-Oct-2016, tatu: For backwards-compatibility purposes, we shall just use
            //    the awful standard JDK serialization via `sqlDate.toString()`... this
            //    is problematic in multiple ways (including using arbitrary timezone...)
            g.writeString(value.toString());
            return;
        }
        _serializeAsString(value, g, provider);

So, it is known that this result will reflect Jackson's basically incorrect behavior.

This gives us a few possible options:

Option 1: Test the incorrect behavior to make sure it stays exactly as incorrect as it is now, but having the test pass in all time zones (i.e. "fix the test", and do regression testing only).

Option 2: Drop the test, and say java.sql.Date is unsupported and must not be used.

Option 2a: Just drop the test. Users are on their own.

Option 3: Provide a "correct" serializer which converts the java.sql.Date into a local date by creating an Instant using the getTime epoch millis of the Date, and then gets the LocalDate of the Instant using the UTC time zone, serializing that value to string format. This will make the behavior "correct" (or as close to "correct" as can exist with java.sql.Date, which is a fundamentally broken type). It would require us to provide this custom conversion by default (I'm not sure how possible that is).

@dmlloyd
Copy link
Member Author

dmlloyd commented Dec 11, 2023

In this case the problem appears doubled. The date is wrongly serialized in the request, which subtracts a day, and then wrongly serialized on the response as well, which subtracts another day.

dmlloyd added a commit to dmlloyd/quarkus that referenced this issue Dec 11, 2023
Fixes quarkusio#37662. Emulate the problematic transformation perpetrated by Jackson to ensure the result is "expected". Tested in multiple timezones in multiple continents, including unusual time zones and time zones near the date line.
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jackson Issues related to Jackson (JSON library) kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant