FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
399 stars 116 forks source link

Add `JavaTimeFeature.ALWAYS_ALLOW_STRINGIFIED_TIMESTAMPS` to allow parsing quoted numbers when using a custom DateTimeFormatter #263

Closed mpkorstanje closed 10 months ago

mpkorstanje commented 1 year ago

I have a scenario where I would like to de-serialize epoch milis, and serialize with a @JsonFormat annotation. In essence:

  final ObjectMapper mapper = new ObjectMapper()
      .disable(DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS)
      .disable(SerializationFeature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS)
      .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
      .setTimeZone(TimeZone.getTimeZone("UTC"))
      .findAndRegisterModules();
  final Instant instant = Instant.ofEpochMilli(123456);

  @Test
  public void test() throws JsonProcessingException {
    assertEquals("{\"time\":\"1970-01-01T00:02:03.456+0000\"}", mapper.writeValueAsString(new TimeHolderWithFormat(instant)));
    assertEquals(instant, mapper.readValue("{\"time\":123456}", TimeHolder.class).time);
    assertEquals(instant, mapper.readValue("{\"time\":\"123456\"}", TimeHolder.class).time);
    assertEquals(instant, mapper.readValue("{\"time\":123456}", TimeHolderWithFormat.class).time);
    assertEquals(instant, mapper.readValue("{\"time\":\"123456\"}", TimeHolderWithFormat.class).time);
  }

  static class TimeHolder {
    private Instant time;
    public void setTime(Instant time) {
      this.time = time;
    }
    // Getter ommited
  }

  static class TimeHolderWithFormat {
    private Instant time;
    // Constructors ommited
    @JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss.SSSZ")
    public void setTime(Instant time) {
      this.time = time;
    }
    // Getter ommited
  }

This will fail on the last assertion with:

com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `java.time.Instant` from String "123456": Failed to deserialize java.time.Instant: (java.time.format.DateTimeParseException) Text '123456' could not be parsed at index 0
 at [Source: (String)"{"time":"123456"}"; line: 1, column: 9] (through reference chain: com.example.app.AppTest$TimeHolderWithFormat["time"])

There is an apparent inconsistency in the way Jackson de-serializes numbers that are shaped as a string into an instant.

https://github.com/FasterXML/jackson-modules-java8/blob/8693cf9c51377ab99e044b45bdb220ca2fb61625/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java#L267-L277

There is however no way to construct a pattern that handles both ISO dates and epoch milis/nanos. Would it be possible to add a feature toggle here?

Details:

cowtowncoder commented 1 year ago

On adding feature: PRs welcome.

mpkorstanje commented 1 year ago

Cheers! I'll try and rustle up something.

cowtowncoder commented 1 year ago

Hmmh. But what is the need to use "quoted numbers" instead of plain numbers?

mpkorstanje commented 1 year ago

Hmmh. But what is the need to use "quoted numbers" instead of plain numbers?

Unfortunately the data I'm reading is using timestamps formatted as quoted numbers. The systems producing this data are rather old and peculiar. And I would agree that in principle they need not be this way. But this is the reality I'm working with.

Nevertheless, without a formatter (i.e. @JsonFormat annotation) it is possible to read these dates in the current implementation. It is currently possible to de-serialize "3.141592653" into a ZoneDateTime. However when adding the @JsonFormat annotation this is no-longer the case. This is the inconsistency I'm trying to address.

Unfortunately I can not access the issue that prompted the original addition of the "read qouted numbers as timestamps" feature (datatype-jsr310#16) but looking at https://github.com/FasterXML/jackson-datatype-jsr310/commit/f85c7085e5d1b77bafb58c4f40b78a6e8368a12c that resolved it appears to be a similar use-case.

So I am not looking to enable a novel use for quoted numbers, rather I'm looking to make an already supported use-case consistent with other features of the library.

cowtowncoder commented 1 year ago

Ok. That makes sense.

So, I am not against supporting such use cases; my main concern is with API. Unfortunately whereas there is now extensive configurability for "alternative Shapes" (accept String where Number is expected) is not quite what we have here, because Date/Time DOES accept String values (ISO-8610) but rather "String that is actually a Number. Because of that extra configurability is indeed needed.

But I am trying to limit use of DeserializationConfig for very specific/targeted uses: it should be for "bigger" features -- there are some legacy settings that are different, but for new things they should be general-purpose ones.

Given this, I think I would actually suggest adding configurability in JavaTimeModule; especially since this is not meant to affect JDK or Joda date/time types.

If this can be done, I'd be happy to get it merged.

I'll add similar note to jackson-databind issue.

mpkorstanje commented 1 year ago

Cheers. I'll have a look at how to accomplish this. Might take a little while.