Closed Sam-Kruglov closed 2 years ago
Actually same happens to some other classes like Instant
Yes, looks like DurationSerializer
is only looking at pattern
from @JsonFormat
and not shape
-- it definitely should.
One thing that would help would be to show a test case: description is helpful (and should be quite easy to create a test), but often the original use case is a very good test case on its own. I hope someone can look into this as it seems like a potentially easy fix (I will have very little time for coding for next 3 weeks unfortunately) and would get in 2.13.0 final (theoretically 2.12 backport is possible too but 2.13.0 likely released before 2.12.5).
This is my primary issue:
class MyDto {
@JsonFormat(pattern = "MINUTES")
@JsonProperty("durationInMins")
private final Duration duration;
public MyDto(Duration d) { duration = d; }
public Duration getDuration() { return duration; }
}
var mapper = new ObjectMapper().findAndRegisterModules();
Map<String, Object> map = mapper.convertValue(new MyDto(Duration.ofHours(2)), new TypeReference<>(){});
assertEqual(map.get("durationInMins"), 120);
This will fail since the serialized value is the number of nanoseconds, so the format is totally ignored, even though the duration converter under the hood is correct, it's just not used as I explained earlier. Fixing the shape not being used would allow me to pass the test case by adding , shape = NUMBER_INT
to the format, which is fine but it'd be better if that wouldn't be required.
P.S. wrote the test case by hand
I added a modified, failing test to reproduce the issue. Unfortunately I don't have time to pursue this further at this point, but I hope someone else can -- I will make time to review PRs and so on, just not actively write a patch myself.
@kupci Not sure what you think -- might be able to add in a 2.13.x patch although I am also worried that a change might be needed in construction/contextualization.
@Sam-Kruglov FINALLY found time to get back to this and hope I fixed it correctly -- trivially simple change but I'm slightly worried about regression. Still, as things are tests pass and it'll go in 2.14.0 unless something is found.
But I was wondering wrt Instant
you mentioned has similar issues: would it be possible to show similar unit test? If so, could you please file a separate new issue for type(s) and I should be able to see if I could do more fixes before 2.14.0 release (planning to start release candidates soon).
https://github.com/FasterXML/jackson-modules-java8/blob/3075bfe40669fd1853407c2f949a63d1f1f1fa8b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/ser/DurationSerializer.java#L84-L88
If I do this, I expect an integer number in JSON representing the number minutes:
But it serializes it as nanoseconds. After some debugging, I decided to add
, shape = NUMBER_INT
in order to forcecom.fasterxml.jackson.datatype.jsr310.ser.JSR310FormattedSerializerBase#useNanoseconds
respondfalse
, but it has no effect because shape is not used in the factory method I linked above.Found a workaround: adding
, without = JsonFormat.Feature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS
to@JsonFormat
. However, it only works if I add it to each property, adding it on class level has no effect.