Refine duration converter for optional suffix

Update `StringToDurationConverter` so that the suffix is optional and
values such as `100`, `+100`, `-100` are assumed to be milliseconds.

Also add support for `@DurationUnit` to allow the unit to be changed
on a per-field basis (allowing for better back-compatibility).

Closes gh-11078

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/convert/StringToDurationConverter.java → spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/convert/DurationConverter.java

@@ -20,12 +20,17 @@ import java.time.Duration;
 import java.time.temporal.ChronoUnit;
 import java.util.Collections;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.Converter;
+import org.springframework.core.convert.converter.GenericConverter;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * {@link Converter} for {@link String} to {@link Duration}. Support
@@ -33,11 +38,19 @@ import org.springframework.util.Assert;
  *
  * @author Phillip Webb
  */
-class StringToDurationConverter implements Converter<String, Duration> {
+class DurationConverter implements GenericConverter {
+
+	private static final Set<ConvertiblePair> TYPES;
+
+	static {
+		Set<ConvertiblePair> types = new LinkedHashSet<>();
+		types.add(new ConvertiblePair(String.class, Duration.class));
+		TYPES = Collections.unmodifiableSet(types);
+	}
 
 	private static Pattern ISO8601 = Pattern.compile("^[\\+\\-]?P.*$");
 
-	private static Pattern SIMPLE = Pattern.compile("^([\\+\\-]?\\d+)([a-zA-Z]{1,2})$");
+	private static Pattern SIMPLE = Pattern.compile("^([\\+\\-]?\\d+)([a-zA-Z]{0,2})$");
 
 	private static final Map<String, ChronoUnit> UNITS;
 
@@ -53,15 +66,32 @@ class StringToDurationConverter implements Converter<String, Duration> {
 	}
 
 	@Override
-	public Duration convert(String source) {
+	public Set<ConvertiblePair> getConvertibleTypes() {
+		return TYPES;
+	}
+
+	@Override
+	public Object convert(Object source, TypeDescriptor sourceType,
+			TypeDescriptor targetType) {
+		if (source == null) {
+			return null;
+		}
+		return toDuration(source.toString(),
+				targetType.getAnnotation(DurationUnit.class));
+	}
+
+	private Duration toDuration(String source, DurationUnit defaultUnit) {
 		try {
+			if (!StringUtils.hasLength(source)) {
+				return null;
+			}
 			if (ISO8601.matcher(source).matches()) {
 				return Duration.parse(source);
 			}
 			Matcher matcher = SIMPLE.matcher(source);
 			Assert.state(matcher.matches(), "'" + source + "' is not a valid duration");
 			long amount = Long.parseLong(matcher.group(1));
-			ChronoUnit unit = getUnit(matcher.group(2));
+			ChronoUnit unit = getUnit(matcher.group(2), defaultUnit);
 			return Duration.of(amount, unit);
 		}
 		catch (Exception ex) {
@@ -70,7 +100,10 @@ class StringToDurationConverter implements Converter<String, Duration> {
 		}
 	}
 
-	private ChronoUnit getUnit(String value) {
+	private ChronoUnit getUnit(String value, DurationUnit defaultUnit) {
+		if (StringUtils.isEmpty(value)) {
+			return (defaultUnit != null ? defaultUnit.value() : ChronoUnit.MILLIS);
+		}
 		ChronoUnit unit = UNITS.get(value.toLowerCase());
 		Assert.state(unit != null, "Unknown unit '" + value + "'");
 		return unit;

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/convert/DurationUnit.java

+/*
+ * Copyright 2012-2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.context.properties.bind.convert;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Annotation that can be used to change the default unit used when converting a
+ * {@link Duration}.
+ *
+ * @author Phillip Webb
+ * @since 2.0.0
+ */
+@Target(ElementType.FIELD)
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface DurationUnit {
+
+	/**
+	 * The duration unit to use if one is not specified.
+	 * @return the duration unit
+	 */
+	ChronoUnit value();
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/convert/StringToDurationConverterTests.java → spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/convert/DurationConverterTests.java

@@ -17,24 +17,31 @@
 package org.springframework.boot.context.properties.bind.convert;
 
 import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Collections;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.core.convert.TypeDescriptor;
+
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
 
 /**
- * Tests for {@link StringToDurationConverter}.
+ * Tests for {@link DurationConverter}.
  *
  * @author Phillip Webb
  */
-public class StringToDurationConverterTests {
+public class DurationConverterTests {
 
 	@Rule
 	public ExpectedException thrown = ExpectedException.none();
 
-	private StringToDurationConverter converter = new StringToDurationConverter();
+	private DurationConverter converter = new DurationConverter();
 
 	@Test
 	public void convertWhenIso8601ShouldReturnDuration() throws Exception {
@@ -97,6 +104,21 @@ public class StringToDurationConverterTests {
 		assertThat(convert("-10d")).isEqualTo(Duration.ofDays(-10));
 	}
 
+	@Test
+	public void convertWhenSimpleWithoutSuffixShouldReturnDuration() throws Exception {
+		assertThat(convert("10")).isEqualTo(Duration.ofMillis(10));
+		assertThat(convert("+10")).isEqualTo(Duration.ofMillis(10));
+		assertThat(convert("-10")).isEqualTo(Duration.ofMillis(-10));
+	}
+
+	@Test
+	public void convertWhenSimpleWithoutSuffixButWithAnnotationShouldReturnDuration()
+			throws Exception {
+		assertThat(convert("10", ChronoUnit.SECONDS)).isEqualTo(Duration.ofSeconds(10));
+		assertThat(convert("+10", ChronoUnit.SECONDS)).isEqualTo(Duration.ofSeconds(10));
+		assertThat(convert("-10", ChronoUnit.SECONDS)).isEqualTo(Duration.ofSeconds(-10));
+	}
+
 	@Test
 	public void convertWhenBadFormatShouldThrowException() throws Exception {
 		this.thrown.expect(IllegalStateException.class);
@@ -104,8 +126,23 @@ public class StringToDurationConverterTests {
 		convert("10foo");
 	}
 
+	@Test
+	public void convertWhenEmptyShouldReturnNull() throws Exception {
+		assertThat(convert("")).isNull();
+	}
+
 	private Duration convert(String source) {
-		return this.converter.convert(source);
+		return (Duration) this.converter.convert(source, TypeDescriptor.forObject(source),
+				TypeDescriptor.valueOf(Duration.class));
+	}
+
+	private Duration convert(String source, ChronoUnit defaultUnit) {
+		TypeDescriptor targetType = mock(TypeDescriptor.class);
+		DurationUnit annotation = AnnotationUtils.synthesizeAnnotation(
+				Collections.singletonMap("value", defaultUnit), DurationUnit.class, null);
+		given(targetType.getAnnotation(DurationUnit.class)).willReturn(annotation);
+		return (Duration) this.converter.convert(source, TypeDescriptor.forObject(source),
+				targetType);
 	}
 
 }