FasterXML / jackson-modules-java8

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

NPE when serializing a `LocalDate` or `LocalDateTime` using `AsDeductionTypeSerializer` #296

Closed mike-reynolds-savient closed 8 months ago

mike-reynolds-savient commented 8 months ago

Search before asking

Describe the bug

In v2.14.2 AsDeductionTypeSerializer was introduced. Prior to this we could define an immutable interface as follows:

    @Nullable
    @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION)
    /* Complex/ nested classes should be added to this list for Jackson to check */
    @JsonSubTypes({
            @JsonSubTypes.Type(StoreField.class),
            @JsonSubTypes.Type(StringArray.class),
            @JsonSubTypes.Type(NumberArray.class),
            @JsonSubTypes.Type(QueryRequest.class) })
    T getValue();

and set a value of LocalDate, LocalDateTime, OffsetDateTime etc as the return of getValue()

This would serialize correctly with JsonMapper.builder().disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) set.

We now get a "Cannot read field 'valueShape'" exception because AsDeductionTypeSerializer returns null if the WritableTypeId.valueShape is not a structure start token. This results in an NPE within the LocalDateTimeSerializer class on line 89;

 if (typeIdDef.valueShape == JsonToken.START_ARRAY) {

Version Information

2.14.2

Reproduction


@Value.Immutable
@JsonSerialize(as = ImmutableQueryPredicate.class)
@JsonDeserialize(as = ImmutableQueryPredicate.class)
public interface QueryPredicate<T> extends Serializable {

    @Nullable
    @JsonTypeInfo(use = JsonTypeInfo.Id.DEDUCTION)
    /* Complex/ nested classes should be added to this list for Jackson to check */
    @JsonSubTypes({
            @JsonSubTypes.Type(StoreField.class),
            @JsonSubTypes.Type(StringArray.class),
            @JsonSubTypes.Type(NumberArray.class),
            @JsonSubTypes.Type(QueryRequest.class) })
    T getValue();

}
    @Test
    void testSerialiseLocalDatePredicate() {

        final QueryPredicate<LocalDateTime> dateTimePredicate = ImmutableQueryPredicate.<LocalDateTime>builder()
                .value(LocalDateTime.of(2022, 06, 24, 10, 23, 51))
                .build();

        final String asString = mapper.writeValueAsString(dateTimePredicate);
}

Expected behavior

If deduction cannot find the class it should return default property behaviour.

Additional context

https://github.com/FasterXML/jackson-databind/issues/3711#issuecomment-1371580012

pjfanning commented 8 months ago

Is this the serializer and have you registered the related module ?

https://github.com/FasterXML/jackson-modules-java8/blob/2.17/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/ser/LocalDateTimeSerializer.java

mike-reynolds-savient commented 8 months ago

Yep, that's the one. Note that this issue also effects com.fasterxml.jackson.datatype.jsr310.ser.LocalDateSerializer

pjfanning commented 8 months ago

@mike-reynolds-savient you repeated back the name of the class that I sent - did you mean to name a different class?

mike-reynolds-savient commented 8 months ago

Apologies - LocalDateTimeSerializer

cowtowncoder commented 8 months ago

Version 2.14.2 is not the latest so would be good repro with 2.16.0.

But also if this requires use of Java 8 Date/Time types, need to move to different repo; jackson-databind tests cannot depend on Java 8 date/time module (cyclic dependency). Will move the isse.

cowtowncoder commented 8 months ago

Ok, so, couple of things now that I re-read this again.

First: I can definitely avoid NPE for LocalDate[Time]Serializer, and that probably makes sense.

But second, I think that DEDUCTION based type deserializer can only work JSON Object represenations, not String-valued ones. So it can work to figure out type of POJOs, but not LocalDate or LocalDateTime. So ultimately I am not sure this can be made to work, if I understand use case correctly (I might misunderstand it tho).

cowtowncoder commented 8 months ago

Was able to reproduce (must force serialization as String), partial fix via #298, will try to cover all types for which NPE would be produced.

cowtowncoder commented 8 months ago

@mike-reynolds-savient I fixed the NPE part for 2.16(.2) -- although it will be a month or two until that release. If you can try 2.16.2-SNAPSHOT to see how things work that'd be good. I am not confident DEDUCTION will work in general, but at least it should not fail with NPE no matter what.

mike-reynolds-savient commented 7 months ago

Many thanks @cowtowncoder When 2.16.2 or 2.17 become available for download I'll give it a go again. (I can't get to the -SNAPSHOT)