From 8cde8aa38a0c97fe70a8048d00a7b2333241eb14 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 20 Oct 2020 17:09:12 -0700 Subject: [PATCH] Add release notes for #184, rework a bit --- .../jsr310/deser/DurationDeserializer.java | 130 +++++++++++------- .../jsr310/deser/JSR310DeserializerBase.java | 2 +- .../jsr310/deser/DurationDeserTest.java | 19 ++- .../deser/DurationUnitConverterTest.java | 57 ++++++++ .../deser/DurationUnitParserEmptyTest.java | 59 -------- .../jsr310/deser/DurationUnitParserTest.java | 53 ------- release-notes/CREDITS-2.x | 10 ++ release-notes/VERSION-2.x | 3 + 8 files changed, 164 insertions(+), 169 deletions(-) create mode 100644 datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitConverterTest.java delete mode 100644 datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserEmptyTest.java delete mode 100644 datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserTest.java diff --git a/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserializer.java b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserializer.java index 1f768ed8..02a36b7f 100644 --- a/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserializer.java +++ b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserializer.java @@ -16,38 +16,32 @@ package com.fasterxml.jackson.datatype.jsr310.deser; +import java.io.IOException; +import java.math.BigDecimal; +import java.time.DateTimeException; +import java.time.Duration; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalUnit; +import java.util.*; +import java.util.stream.Collectors; + import com.fasterxml.jackson.annotation.JsonFormat; + import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.core.JsonTokenId; import com.fasterxml.jackson.core.StreamReadCapability; import com.fasterxml.jackson.core.io.NumberInput; -import com.fasterxml.jackson.databind.BeanProperty; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.JsonMappingException; + +import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.deser.ContextualDeserializer; import com.fasterxml.jackson.datatype.jsr310.DecimalUtils; -import java.io.IOException; -import java.math.BigDecimal; -import java.time.DateTimeException; -import java.time.Duration; -import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalUnit; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Optional; -import java.util.Set; - - /** * Deserializer for Java 8 temporal {@link Duration}s. * * @author Nick Williams - * @since 2.2.0 + * @since 2.2 */ public class DurationDeserializer extends JSR310DeserializerBase implements ContextualDeserializer @@ -57,29 +51,38 @@ public class DurationDeserializer extends JSR310DeserializerBase public static final DurationDeserializer INSTANCE = new DurationDeserializer(); /** - * Since 2.12 - * When set, integer values will be deserialized using the specified unit. Using this parser will tipically - * override the value specified in {@link DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS} as it is - * considered that the unit set in {@link JsonFormat#pattern()} has precedence since is more specific. + * When defined (not {@code null}) integer values will be converted into duration + * unit configured for the converter. + * Using this converter will typically override the value specified in + * {@link DeserializationFeature#READ_DATE_TIMESTAMPS_AS_NANOSECONDS} as it is + * considered that the unit set in {@link JsonFormat#pattern()} has precedence + * since it is more specific. + *

+ * See [jackson-modules-java8#184] for more info. * - * @see [jackson-modules-java8#184] for more info + * @since 2.12 */ - private DurationUnitParser _durationUnitParser; + protected final DurationUnitConverter _durationUnitConverter; - private DurationDeserializer() { + public DurationDeserializer() { super(Duration.class); + _durationUnitConverter = null; } /** - * Since 2.11 + * @since 2.11 */ protected DurationDeserializer(DurationDeserializer base, Boolean leniency) { super(base, leniency); + _durationUnitConverter = base._durationUnitConverter; } - protected DurationDeserializer(DurationDeserializer base, DurationUnitParser durationUnitParser) { + /** + * @since 2.12 + */ + protected DurationDeserializer(DurationDeserializer base, DurationUnitConverter converter) { super(base, base._isLenient); - _durationUnitParser = durationUnitParser; + _durationUnitConverter = converter; } @Override @@ -87,9 +90,13 @@ protected DurationDeserializer withLeniency(Boolean leniency) { return new DurationDeserializer(this, leniency); } + protected DurationDeserializer withConverter(DurationUnitConverter pattern) { + return new DurationDeserializer(this, pattern); + } + @Override public JsonDeserializer createContextual(DeserializationContext ctxt, - BeanProperty property) throws JsonMappingException + BeanProperty property) throws JsonMappingException { JsonFormat.Value format = findFormatOverrides(ctxt, property, handledType()); DurationDeserializer deser = this; @@ -101,18 +108,20 @@ public JsonDeserializer createContextual(DeserializationContext ctxt, } } if (format.hasPattern()) { - deser = DurationUnitParser.from(format.getPattern()) - .map(deser::withPattern) - .orElse(deser); + final String pattern = format.getPattern(); + DurationUnitConverter p = DurationUnitConverter.from(pattern); + if (p == null) { + ctxt.reportBadDefinition(getValueType(ctxt), + String.format( +"Bad 'pattern' definition (\"%s\") for `Duration`: expected one of [%s]", +pattern, DurationUnitConverter.descForAllowed())); + } + deser = deser.withConverter(p); } } return deser; } - private DurationDeserializer withPattern(DurationUnitParser pattern) { - return new DurationDeserializer(this, pattern); - } - @Override public Duration deserialize(JsonParser parser, DeserializationContext context) throws IOException { @@ -122,11 +131,7 @@ public Duration deserialize(JsonParser parser, DeserializationContext context) t BigDecimal value = parser.getDecimalValue(); return DecimalUtils.extractSecondsAndNanos(value, Duration::ofSeconds); case JsonTokenId.ID_NUMBER_INT: - long intValue = parser.getLongValue(); - if (_durationUnitParser != null) { - return _durationUnitParser.parse(intValue); - } - return _fromTimestamp(context, intValue); + return _fromTimestamp(context, parser.getLongValue()); case JsonTokenId.ID_STRING: return _fromString(parser, context, parser.getText()); // 30-Sep-2020, tatu: New! "Scalar from Object" (mostly for XML) @@ -170,14 +175,22 @@ && _isValidTimestampString(value)) { } protected Duration _fromTimestamp(DeserializationContext ctxt, long ts) { + if (_durationUnitConverter != null) { + return _durationUnitConverter.convert(ts); + } + // 20-Oct-2020, tatu: This makes absolutely no sense but... somehow + // became the default handling. if (ctxt.isEnabled(DeserializationFeature.READ_DATE_TIMESTAMPS_AS_NANOSECONDS)) { return Duration.ofSeconds(ts); } return Duration.ofMillis(ts); } - protected static class DurationUnitParser { - final static Set PARSEABLE_UNITS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList( + protected static class DurationUnitConverter { + private final static Map PARSEABLE_UNITS; + static { + Map units = new LinkedHashMap<>(); + for (ChronoUnit unit : new ChronoUnit[] { ChronoUnit.NANOS, ChronoUnit.MICROS, ChronoUnit.MILLIS, @@ -186,22 +199,35 @@ protected static class DurationUnitParser { ChronoUnit.HOURS, ChronoUnit.HALF_DAYS, ChronoUnit.DAYS - ))); + }) { + units.put(unit.name(), unit); + } + PARSEABLE_UNITS = units; + } + final TemporalUnit unit; - DurationUnitParser(TemporalUnit unit) { + DurationUnitConverter(TemporalUnit unit) { this.unit = unit; } - Duration parse(long value) { + public Duration convert(long value) { return Duration.of(value, unit); } - static Optional from(String unit) { - return PARSEABLE_UNITS.stream() - .filter(u -> u.name().equals(unit)) - .map(DurationUnitParser::new) - .findFirst(); + /** + * @return Description of all allowed valued as a sequence of + * double-quoted values separated by comma + */ + public static String descForAllowed() { + return "\"" + PARSEABLE_UNITS.keySet().stream() + .collect(Collectors.joining("\", \"")) + +"\""; + } + + static DurationUnitConverter from(String unit) { + ChronoUnit chr = PARSEABLE_UNITS.get(unit); + return (chr == null) ? null : new DurationUnitConverter(chr); } } } diff --git a/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/JSR310DeserializerBase.java b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/JSR310DeserializerBase.java index 3c3f59c8..c509e74e 100644 --- a/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/JSR310DeserializerBase.java +++ b/datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/JSR310DeserializerBase.java @@ -183,7 +183,7 @@ protected R _handleUnexpectedToken(DeserializationContext context, @SuppressWarnings("unchecked") protected T _failForNotLenient(JsonParser p, DeserializationContext ctxt, - JsonToken expToken) throws IOException + JsonToken expToken) throws IOException { return (T) ctxt.handleUnexpectedToken(handledType(), expToken, p, "Cannot deserialize instance of %s out of %s token: not allowed because 'strict' mode set for property or type (enable 'lenient' handling to allow)", diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserTest.java index ec2ffe41..d7d2228d 100644 --- a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserTest.java +++ b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationDeserTest.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.exc.MismatchedInputException; import com.fasterxml.jackson.datatype.jsr310.MockObjectConfiguration; import com.fasterxml.jackson.datatype.jsr310.ModuleTestBase; @@ -429,6 +430,12 @@ public void testStrictDeserializeFromEmptyString() throws Exception { objectReader.readValue(valueFromEmptyStr); } + /* + /********************************************************** + /* Tests for custom patterns (modules-java8#184) + /********************************************************** + */ + @Test public void shouldDeserializeInNanos_whenNanosUnitAsPattern_andValueIsInteger() throws Exception { ObjectMapper mapper = newMapper(); @@ -550,15 +557,19 @@ public void shouldIgnoreUnitPattern_whenValueIsString() throws Exception { } @Test - public void shouldIgnoreUnitPattern_whenUnitPatternDoesNotMatchExactly() throws Exception { + public void shouldFailForInvalidPattern() throws Exception { ObjectMapper mapper = newMapper(); mapper.configOverride(Duration.class) .setFormat(JsonFormat.Value.forPattern("Nanos")); ObjectReader reader = mapper.readerFor(MAP_TYPE_REF); - Wrapper wrapper = reader.readValue(wrapperPayload(25), Wrapper.class); - - assertEquals(Duration.ofSeconds(25), wrapper.value); + try { + /*Wrapper wrapper =*/ reader.readValue(wrapperPayload(25), Wrapper.class); + fail("Should not allow invalid 'pattern'"); + } catch (InvalidDefinitionException e) { + verifyException(e, "Bad 'pattern' definition (\"Nanos\")"); + verifyException(e, "expected one of ["); + } } private String wrapperPayload(Number number) { diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitConverterTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitConverterTest.java new file mode 100644 index 00000000..9e385814 --- /dev/null +++ b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitConverterTest.java @@ -0,0 +1,57 @@ +package com.fasterxml.jackson.datatype.jsr310.deser; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import java.time.temporal.ChronoUnit; + +import org.junit.Test; + +import com.fasterxml.jackson.datatype.jsr310.ModuleTestBase; +import com.fasterxml.jackson.datatype.jsr310.deser.DurationDeserializer.DurationUnitConverter; + +public class DurationUnitConverterTest + extends ModuleTestBase +{ + @Test + public void shouldMapToTemporalUnit() { + for (ChronoUnit inputUnit : new ChronoUnit[] { + ChronoUnit.NANOS, + ChronoUnit.MICROS, + ChronoUnit.MILLIS, + ChronoUnit.SECONDS, + ChronoUnit.MINUTES, + ChronoUnit.HOURS, + ChronoUnit.HALF_DAYS, + ChronoUnit.DAYS, + }) { + DurationUnitConverter conv = DurationUnitConverter.from(inputUnit.name()); + assertNotNull(conv); + assertEquals(inputUnit, conv.unit); + // is case-sensitive: + assertNull(DurationUnitConverter.from(inputUnit.name().toLowerCase())); + } + } + + @Test + public void shouldNotMapToTemporalUnit() { + for (String invalid : new String[] { + // Inaccurate units not (yet?) supported + "WEEKS", + "MONTHS", + "YEARS", + "DECADES", + "CENTURIES", + "MILLENNIA", + "ERAS", + "FOREVER", + + // Not matching at all + "DOESNOTMATCH", "", " " + }) { + assertNull("Should not map pattern '"+invalid+"'", + DurationUnitConverter.from(invalid)); + } + } +} diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserEmptyTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserEmptyTest.java deleted file mode 100644 index b476eba1..00000000 --- a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserEmptyTest.java +++ /dev/null @@ -1,59 +0,0 @@ -package com.fasterxml.jackson.datatype.jsr310.deser; - -import com.fasterxml.jackson.datatype.jsr310.deser.DurationDeserializer.DurationUnitParser; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; - -import java.util.Collection; -import java.util.Optional; - -import static java.util.Arrays.asList; -import static java.util.Optional.empty; -import static org.junit.Assert.assertEquals; - -@RunWith(Parameterized.class) -public class DurationUnitParserEmptyTest { - - private final String stringPattern; - - public DurationUnitParserEmptyTest(String stringPattern) { - this.stringPattern = stringPattern; - } - - @Test - public void shouldNotMapToTemporalUnit() { - Optional durationPattern = DurationUnitParser.from(stringPattern); - - assertEquals(empty(), durationPattern); - } - - @Parameters - public static Collection testCases() { - return asList( - // Estimated units - asArray("WEEKS"), - asArray("MONTHS"), - asArray("YEARS"), - asArray("DECADES"), - asArray("CENTURIES"), - asArray("MILLENNIA"), - asArray("ERAS"), - asArray("FOREVER"), - // Is case sensitive - asArray("Nanos"), - asArray("nanos"), - // Not matching at all - asArray("DOESNOTMATCH"), - // Nilables - asArray(null), - asArray(""), - asArray(" ") - ); - } - - private static Object[] asArray(Object... values) { - return values; - } -} \ No newline at end of file diff --git a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserTest.java b/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserTest.java deleted file mode 100644 index 3c065631..00000000 --- a/datetime/src/test/java/com/fasterxml/jackson/datatype/jsr310/deser/DurationUnitParserTest.java +++ /dev/null @@ -1,53 +0,0 @@ -package com.fasterxml.jackson.datatype.jsr310.deser; - -import com.fasterxml.jackson.datatype.jsr310.deser.DurationDeserializer.DurationUnitParser; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; - -import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalUnit; -import java.util.Collection; -import java.util.Optional; - -import static java.util.Arrays.asList; -import static java.util.Optional.of; -import static org.junit.Assert.assertEquals; - -@RunWith(Parameterized.class) -public class DurationUnitParserTest { - - private final String stringPattern; - private final TemporalUnit temporalUnit; - - public DurationUnitParserTest(String stringPattern, TemporalUnit temporalUnit) { - this.stringPattern = stringPattern; - this.temporalUnit = temporalUnit; - } - - @Test - public void shouldMapToTemporalUnit() { - Optional durationPattern = DurationUnitParser.from(stringPattern); - - assertEquals(of(temporalUnit), durationPattern.map(dp -> dp.unit)); - } - - @Parameters - public static Collection testCases() { - return asList( - asArray("NANOS", ChronoUnit.NANOS), - asArray("MICROS", ChronoUnit.MICROS), - asArray("MILLIS", ChronoUnit.MILLIS), - asArray("SECONDS", ChronoUnit.SECONDS), - asArray("MINUTES", ChronoUnit.MINUTES), - asArray("HOURS", ChronoUnit.HOURS), - asArray("HALF_DAYS", ChronoUnit.HALF_DAYS), - asArray("DAYS", ChronoUnit.DAYS) - ); - } - - private static Object[] asArray(Object... values) { - return values; - } -} diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 55e57b49..dffa4aba 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -118,3 +118,13 @@ Ferenc Csaky (ferenc-csaky@github) * Contributed fix to #175: ObjectMapper#setTimeZone ignored by jsr-310/datetime types during serialization when using `@JsonFormat` annotation (2.12.0) + +Philipp Dargel (chisui@github) + * Requested #184: `DurationDeserializer` should use `@JsonFormat.pattern` (and + config override) to support configurable `ChronoUnit` + (2.12.0) + +Oriol Barcelona (obarcelonap@github) + * Contributed fix for #184: `DurationDeserializer` should use `@JsonFormat.pattern` + (and config override) to support configurable `ChronoUnit` + (2.12.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 346ac7e6..1a537d17 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -16,6 +16,9 @@ Modules: #175: ObjectMapper#setTimeZone ignored by jsr-310/datetime types during serialization when using `@JsonFormat` annotation (reported by Erwan L; fix contributed by Ferenc C) +#184: `DurationDeserializer` should use `@JsonFormat.pattern` (and config override) + to support configurable `ChronoUnit` + (requested by Philipp D, fix contributed by Oriol B) - Add Gradle Module Metadata (https://blog.gradle.org/alignment-with-gradle-module-metadata) 2.11.3 (02-Oct-2020)