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

Issue/#116 - Removes the fraction part based on the SerializationFeature WRITE_TIMESTAMP_WITHOUT_FRACTION #164

Closed
wants to merge 24 commits into from

Conversation

sashikanthr
Copy link

This fixes #116

We have added a new feature to in SerializationFeature in the databind package to avoid the fraction part. This has a separate pull request.

* test: add initial test case

* fix: fix problems in test case

for issue #1

* feat: add test cases for timestamps part and nanoseconds part

related to issue #1

* create a false test cases

#1

* Use flag for diabling fraction

* Use flag for diabling fraction

* Added the flag for DurationSerializer classes

* Removed test cases that are not relevant to the added Serialization Feature

* Only keep requirement specific tests

Some tests were set up to get an understanding of the code. They should
not be kept and included in the pull request

Co-authored-by: Nagisa <[email protected]>
Co-authored-by: Jacob Adlers <[email protected]>
@sashikanthr
Copy link
Author

Since this is updating the databind module, the build will fail. We couldnt find information on how to update changes in multiple modules.

@sashikanthr sashikanthr changed the title Issue/#116 (#4) Issue/#116 - Removes the fraction part based on the SerializationFeature WRITE_TIMESTAMP_WITHOUT_FRACTION Feb 27, 2020
@sashikanthr
Copy link
Author

@kupci May I know if we can address your comments and update the PR? Will it be still relevant?

@kupci
Copy link
Member

kupci commented Jun 6, 2020

@sashikanthr I think the issue is still relevant, but it might be easier to create a new PR. I think your original PR highlighted the need for a better overall solution for handling issues like these.

One thought (refreshing myself on the issue) was to use the Shape.* but the difficulty there was both milliseconds and epoch seconds would be Shape.NUMBER_INT.

However, on that subject @cowtowncoder describes some initial thoughts / Idea-lets here:

would it make sense to add unit (or timeUnit) property for @jsonformat?

On one hand, it is bit datatype-specific, but there is one big benefit: ability to set format defaults, and overrides (both programmatically and via annotations) would make implementation relatively easy

https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-5

One question I have is whether we can simply add more some sub-types, i.e. we already have Shape.NUMBER, Shape.NUMBER_FLOAT and Shape.NUMBER_INT, so could we have the new types following the Java convention, say Shape.NUMBER_INT_MILLISECONDS? And do we need to distinguish between epoch seconds, or would Shape.NUMBER_INT_SECONDS work, following the Java convention here:
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/TimeUnit.html

updated: See the newly added CoercionConfig - wondering if that can be used as DateTimeConfig, or does this use case not fit?

@cowtowncoder - not sure how to update the JSTEP wiki. On the jackson-databind/wiki there is an 'Edit' button, and a 'New Page', but I don't see that for the corresponding wiki page on 'Future Ideas', specifically the JSTEP-5 page: https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-5]

@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 10, 2020

So, CoercionConfig determines reading-side only (deserialization), on whether to accept alternate, secondary input shapes: like whether boolean property can be read from (JSON) integral number value.
It probably will not be quite as useful here because of this, unfortunately.

As to possible Shape.NUMBER_INT_MILLISECONDS, I don't think that works either unfortunately -- shapes, as a concept, refer to type in input format, and there is no qualifier in JSON Number (or most other dataformats) to really make such distinction.

Finally, on wiki: @kupci should now have access via "wiki" team (you may need to accept it or something, requested your addition) so hopefully can now edit that wiki page.

@kupci
Copy link
Member

kupci commented Jun 12, 2020

Thanks @cowtowncoder , that worked: I can get see the "Edit" button on the new-ideas wiki.

@kupci
Copy link
Member

kupci commented Jun 16, 2020

Closing this one as per comments above, the direction for changes like this is to implement via a more long-term solution. Details here: https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP-5

@kupci kupci closed this Jun 16, 2020
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.

Allow Instant to be serialized as epochSecond without the fraction part
4 participants