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

Add support for ISO8601 to java.time.Instant serialization/deserialization #1417

Merged
merged 26 commits into from
Jul 7, 2020

Conversation

gmckerrell
Copy link
Contributor

Resolves #1416

Description

See issue

How Has This Been Tested?

Unittest added for new serde

License

I confirm that this contribution is made under an Apache 2.0 license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@gmckerrell
Copy link
Contributor Author

gmckerrell commented Jul 6, 2020

Hi @aklish
Sorry to bother you, but I'm very confused as to why this PR is failing to build on travis. As far as I can see the class in question exists in the source tree but ElideSettingsBuilder fails to import the class.

I've see some reports of earlier versions of Travis (2015) having such issues because of JDK versions, but not sure this is related or not.

I'm hoping this may be a quirk that you've come across... or that I'm missing something blindingly obvious but am too close to the changes to see it...

Yours slightly confused
Graeme

@gmckerrell
Copy link
Contributor Author

I'm going to close this PR, as on reflection this is not a good fit with with8601Dates() which takes a format string (which would be ignored for these temporal types).

@gmckerrell gmckerrell closed this Jul 6, 2020
@gmckerrell gmckerrell reopened this Jul 6, 2020
@gmckerrell
Copy link
Contributor Author

just thought better support could be via withJavaTime() method

@gmckerrell
Copy link
Contributor Author

Hi @aklish
Sorry to bother you, but I'm very confused as to why this PR is failing to build on travis. As far as I can see the class in question exists in the source tree but ElideSettingsBuilder fails to import the class.

I've see some reports of earlier versions of Travis (2015) having such issues because of JDK versions, but not sure this is related or not.

I'm hoping this may be a quirk that you've come across... or that I'm missing something blindingly obvious but am too close to the changes to see it...

Yours slightly confused
Graeme

hmm just realised I forgot to make the source code a .java file!!!

@gmckerrell
Copy link
Contributor Author

There is no need to touch the Builder class. The serde can be auto registered using the ElideTypeConverter annotation

@gmckerrell gmckerrell changed the title Add support for ISO8601 to Instant/OffsetDateTime serialization/deserialization Add support for ISO8601 to java.time.Instant serialization/deserialization Jul 6, 2020
@gmckerrell
Copy link
Contributor Author

build currently failing due to CVE in tomcat version. Looks like dependabot has already got a PR open to bump tomcat version.

@aklish
Copy link
Member

aklish commented Jul 7, 2020

Every artifact in the mono repo runs a CVE security scan to see if any package in the dependency list has an outstanding security CVE with a severity greater than 7.

The build is failing because of a spring dependency on tomcat:

[ERROR] One or more dependencies were identified with vulnerabilities that have a CVSS score greater than or equal to '7.0': 
[ERROR] 
[ERROR] tomcat-embed-core-9.0.35.jar: CVE-2020-11996

I'll patch master so the build works again. That should unblock this PR.

@aklish
Copy link
Member

aklish commented Jul 7, 2020

I also think Elide should have out of box support for all the java.time classes. Changes look good to me.

The only downside I see is that Elide has no mechanism to override a Serde registered by annotation (ElideTypeConverter). OffsetDateTimeSerde has the same issue.

We may want to add (probably in a separate ticket) some mechanism to shade the serdes that come bundled from com.yahoo.elide in case the application wants to override the behavior.

@coveralls
Copy link
Collaborator

Coverage Status

Coverage increased (+0.008%) to 81.84% when pulling 16487cd on gmckerrell:master into 7681ec1 on yahoo:master.

@aklish aklish merged commit 3d22939 into yahoo:master Jul 7, 2020
gmckerrell added a commit to gmckerrell/elide that referenced this pull request Jul 9, 2020
* Add support for ISO8601 to java.time.Instant serialization/deserialization (yahoo#1417)

* add queryParams variant for post(), patch() and delete()

* remove trailing spaces

* add query request parameters to POST, PATCH and DELETE controllers

* Update JsonApiEndpoint.java

* shorten line

* Create ISO8601InstantSerdes.java (#3)

* Update ElideSettingsBuilder.java

* Update ISO8601InstantSerdeTest.java

* Update ISO8601InstantSerde

* Update ISO8601InstantSerdeTest.java

* remove trailing whitespace

* remove redundant import

* try changing class name to avoid travis failure...

* Update InstantSerdeTest.java

* Update ElideSettingsBuilder.java

* Update ElideSettingsBuilder.java

* change to jupiter test framework

* Rename InstantSerde to InstantSerde.java

* auto-register InstantSerde (#5)

* switch to using ISO_OFFSET_DATE_TIME to avoid JDK-8 bug

* fix typo

* fix serialization

Co-authored-by: Aaron Klish <[email protected]>

* [README] - Fix the standalone README Java sample (yahoo#1415)

The old code isn't valid Java. It has a stray `)` and isn't formatted correctly. Thus fix it.

Co-authored-by: Aaron Klish <[email protected]>

* Bump classgraph from 4.8.86 to 4.8.87 (yahoo#1419)

Bumps [classgraph](https://github.com/classgraph/classgraph) from 4.8.86 to 4.8.87.
- [Release notes](https://github.com/classgraph/classgraph/releases)
- [Commits](classgraph/classgraph@classgraph-4.8.86...classgraph-4.8.87)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Aaron Klish <[email protected]>

* Bump spring.boot.version from 2.3.0.RELEASE to 2.3.1.RELEASE (yahoo#1395)

Bumps `spring.boot.version` from 2.3.0.RELEASE to 2.3.1.RELEASE.

Updates `spring-boot-starter-web` from 2.3.0.RELEASE to 2.3.1.RELEASE
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.3.0.RELEASE...v2.3.1.RELEASE)

Updates `spring-boot-starter-data-jpa` from 2.3.0.RELEASE to 2.3.1.RELEASE
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.3.0.RELEASE...v2.3.1.RELEASE)

Updates `spring-boot-starter-test` from 2.3.0.RELEASE to 2.3.1.RELEASE
- [Release notes](https://github.com/spring-projects/spring-boot/releases)
- [Commits](spring-projects/spring-boot@v2.3.0.RELEASE...v2.3.1.RELEASE)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Aaron Klish <[email protected]>

* Bump postgresql from 42.2.12 to 42.2.14 (yahoo#1392)

Bumps [postgresql](https://github.com/pgjdbc/pgjdbc) from 42.2.12 to 42.2.14.
- [Release notes](https://github.com/pgjdbc/pgjdbc/releases)
- [Changelog](https://github.com/pgjdbc/pgjdbc/blob/master/CHANGELOG.md)
- [Commits](pgjdbc/pgjdbc@REL42.2.12...REL42.2.14)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

* Bump version.jetty from 9.4.29.v20200521 to 9.4.30.v20200611 (yahoo#1393)

Bumps `version.jetty` from 9.4.29.v20200521 to 9.4.30.v20200611.

Updates `jetty-server` from 9.4.29.v20200521 to 9.4.30.v20200611
- [Release notes](https://github.com/eclipse/jetty.project/releases)
- [Commits](jetty/jetty.project@jetty-9.4.29.v20200521...jetty-9.4.30.v20200611)

Updates `jetty-servlet` from 9.4.29.v20200521 to 9.4.30.v20200611
- [Release notes](https://github.com/eclipse/jetty.project/releases)
- [Commits](jetty/jetty.project@jetty-9.4.29.v20200521...jetty-9.4.30.v20200611)

Updates `jetty-continuation` from 9.4.29.v20200521 to 9.4.30.v20200611

Updates `jetty-webapp` from 9.4.29.v20200521 to 9.4.30.v20200611
- [Release notes](https://github.com/eclipse/jetty.project/releases)
- [Commits](jetty/jetty.project@jetty-9.4.29.v20200521...jetty-9.4.30.v20200611)

Updates `jetty-servlets` from 9.4.29.v20200521 to 9.4.30.v20200611
- [Release notes](https://github.com/eclipse/jetty.project/releases)
- [Commits](jetty/jetty.project@jetty-9.4.29.v20200521...jetty-9.4.30.v20200611)

Updates `websocket-server` from 9.4.29.v20200521 to 9.4.30.v20200611

Updates `javax-websocket-server-impl` from 9.4.29.v20200521 to 9.4.30.v20200611

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Aaron Klish <[email protected]>

* Bump build-helper-maven-plugin from 3.1.0 to 3.2.0 (yahoo#1391)

Bumps [build-helper-maven-plugin](https://github.com/mojohaus/build-helper-maven-plugin) from 3.1.0 to 3.2.0.
- [Release notes](https://github.com/mojohaus/build-helper-maven-plugin/releases)
- [Commits](mojohaus/build-helper-maven-plugin@build-helper-maven-plugin-3.1.0...build-helper-maven-plugin-3.2.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Aaron Klish <[email protected]>

* Issue 683 (yahoo#1384)

* Improved error handling

* Tests

* update test

* Added logic to test collections against collections

* Added operator in test that traverses to-many relationship

* Converted text operators

* Added support for comparator operators

* Build passes

* Added back failing Filter IT test

* Updated dialect tests

* Updateed FilterIT tests

Co-authored-by: wcekan <[email protected]>
Co-authored-by: Aaron Klish <[email protected]>
Co-authored-by: Aaron Klish <[email protected]>

* Update changelog.md

Co-authored-by: Aaron Klish <[email protected]>
Co-authored-by: David R Soller <[email protected]>
Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: William Cekan <[email protected]>
Co-authored-by: wcekan <[email protected]>
Co-authored-by: Aaron Klish <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

java.time.Instant is not (de)serialized "out of the box"
3 participants