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

Support annotation override for WRITE_DATES_AS_TIMESTAMPS #4278

Open
nhmarujo opened this issue Dec 22, 2023 · 8 comments
Open

Support annotation override for WRITE_DATES_AS_TIMESTAMPS #4278

nhmarujo opened this issue Dec 22, 2023 · 8 comments
Labels
2.17 Issues planned at earliest for 2.17 to-evaluate Issue that has been received but not yet evaluated

Comments

@nhmarujo
Copy link

nhmarujo commented Dec 22, 2023

Is your feature request related to a problem? Please describe.

Not a problem, just lack of flexibility.

Describe the solution you'd like

It is quite normal for someone to create an ObjectMapper and configure it with reasonable defaults that apply to the majority of serializations/deserializations done within a project. For anything that deviates from these defaults, we can annotate and override the defaults. This mechanism is great, since it allows to follow the principle of "convention over configuration", but still allows flexibility when needed.

It seems however that there is no feature in JsonFormat.Feature allowing to override, for an individual case, the value chosen by SerializationFeature.WRITE_DATES_AS_TIMESTAMPS.

This would be great to have, as most of the times people have a particular preference, and that is what they use on their projects, but they need to deal with 3rd parties which JSON contract differs in approach. Would be great to be able to just annotate on those specific objects.

Usage example

Potential use, if that JsonFormat.Feature was added:

@JsonFormat(without = JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS)
private Instant timestamp;

Additional context

No response

@nhmarujo nhmarujo added the to-evaluate Issue that has been received but not yet evaluated label Dec 22, 2023
@JooHyukKim
Copy link
Member

The proposed use case seems to fit into ObjectWriter usage. Have you considered using writer()?

ObjectWriter w = GLOBAL_MAPPER
                .writer()
                .without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);

@nhmarujo
Copy link
Author

nhmarujo commented Dec 23, 2023

Hi @JooHyukKim.

Honestly I did not and I thank you for your suggestion, but that doesn't really give the flexibility I was looking for.

Most of the times I don't deal directly with ObjectMapper, only on very extreme cases. Most of the times the framework (in my case Spring) does it behind the scenes, by calling it for serialization just before sending a request or for deserialization just before calling a controller with an incoming request.

The ability to disable that feature on the @JsonFormat annotation would allow to just annotate the specific fields that deviate from the norm (that is set by the default configuration) on specific DTOs and be able to have both worlds coexisting, without a lot of custom code.

Let me know if I made it clear enough, please 🙂 Thanks

@yihtserns
Copy link
Contributor

Coincidentally, I did a quick study on @JsonFormat yesterday, which made me think your problem can be solved with this:

If the default is to serialize dates as timestamp (i.e. numbers), but you want to serialize it as yyyy-MM-ddTHH:mm:ss.SSSZ:

@JsonFormat(shape = JsonFormat.Shape.STRING)
private Instant timestamp;

If the default is to serialize dates as yyyy-MM-ddTHH:mm:ss.SSSZ, but you want to serialize it as timestamp (i.e. numbers):

@JsonFormat(shape = JsonFormat.Shape.NUMBER)
private Instant timestamp;

@nhmarujo
Copy link
Author

nhmarujo commented Dec 23, 2023

Hi @yihtserns . Thank you so much, that is really helpful.

I gave a try to your suggestion and it does seem to work:

@JsonFormat(shape = JsonFormat.Shape.NUMBER)
private Instant timestamp;

The only flaw I can see with this approach, is that you lose the flexibility to also use JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS, which would allow you to write the nanoseconds too (if you have enough precision for that).

Not saying your suggestion isn't great - honestly I didn't thought of it and probably will solve most of the cases. But I still think there is merit to implement my suggestion, as it will result in a clearer and also more flexible contract.

I should also add that it feels very counterintuitive to me that JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS shall exist and JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS doesn't).

Please let me know your thoughts 🙂

@yihtserns
Copy link
Contributor

yihtserns commented Dec 23, 2023

The only flaw I can see with this approach, is that you lose the flexibility to also use JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS, which would allow you to write the nanoseconds too (if you have enough precision for that).

On JDK 11, this:

@JsonFormat(shape = JsonFormat.Shape.STRING)
private Instant instantAsString;
@JsonFormat(shape = JsonFormat.Shape.NUMBER)
private Instant instantAsNumber;
@JsonFormat(shape = JsonFormat.Shape.NUMBER, without = JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS)
private Instant instantAsNumberWithoutNanos;
@JsonFormat(shape = JsonFormat.Shape.NUMBER, with = JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS)
private Instant instantAsNumberWithNanos;

...resulted in:

{
  "instantAsString" : "2023-12-23T11:28:20.493370400Z",
  "instantAsNumber" : 1703330900.493370400,
  "instantAsNumberWithoutNanos" : 1703330900493,
  "instantAsNumberWithNanos" : 1703330900.493370400
}

I should also add that it feels very counterintuitive to me that JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS shall exist and JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS doesn't).

I'm guessing having JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS would've duplicated JsonFormat.Shape.NUMBER. JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS can be seen as an instruction/config for JsonFormat.Shape.NUMBER? 🤷

@nhmarujo
Copy link
Author

nhmarujo commented Dec 23, 2023

@yihtserns you are absolutely right. I just tried it again and it behaved exactly as you stated. What mislead me was that I was testing it on a unit test with a mocked clock and the value being passed didn't have enough precision for the decimal part to kick in 😅

What you described seems like a working solution, but not a obvious one for most people. Maybe it should be clearer on documentation (at least I didn't find it) or, perhaps even better, have a JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS, that yes is kinda redundant, but much more explicit on intent.

I would like to hear more thoughts on this 🙂

Again, thank you so much for pointing a working solution!

@yihtserns
Copy link
Contributor

yihtserns commented Dec 23, 2023

What you described seems like a working solution, but not a obvious one for most people. Maybe it should be clearer on documentation (at least I didn't find it) or, perhaps even better, have a JsonFormat.Feature.WRITE_DATES_AS_TIMESTAMPS...

I think @JsonFormat is suffering from (lack of) "marketing" problem. As a user, I was also more familiar with configurations like SerializationFeature.WRITE_DATES_AS_TIMESTAMPS & spring.jackson.serialization.write-dates-as-timestamps, because those are the ones heavily documented by the top results when googling for Jackson+DateTime serialization formatting.

And even though I knew about the existence of @JsonFormat.shape, it only became obvious on how to use it to solve this issue after I studied the codebase in order to contribute to #1114 (comment). 🤷

@cowtowncoder cowtowncoder added the 2.17 Issues planned at earliest for 2.17 label Jan 2, 2024
@cowtowncoder
Copy link
Member

Sorry for late response but as the background, I have been trying to move away from datatype-specific DeserializationFeature / SerializationFeature settings since we have limitations on maximum number allowed, and also since granularity is bit too coarse.
@JsonFormat annotation and matching config override mechanism is meant to allow replacement to some degree; and here, shape is indeed preferred.

But at the same time, there is indeed the problem of "subtype" beyond shape: whereas for Shape.STRING we have format for further specialization, there isn't anything for Shape.NUMBER to indicate time unit to use. Perhaps there could/should be?

There is also yet another possibility I had been developing: DatatypeFeature extension. There are currently (as of 2.16) 2 of these:

  • EnumFeature (added in 2.14)
  • JsonNodeFeature (added in 2.14)

and originally I thought we would also want DateTimeFeature similarly.
This is still a possibility. However, I now realize that it might be challenging wrt combining all different settings, as this setting would be global and without mechanism to override per-property.
This is less of an issue with JsonNode (since it's sort of global type) and even for enums, but with wide variety of Date/Time types, per-property overriding is bit of must-have.

I am not sure what the best way forward is, but I would ideally avoid addition of DeserializationFeature/SerializationFeatures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 Issues planned at earliest for 2.17 to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

4 participants