Polish 'Add Period converter support'

Polish period converter support, primarily by changing
`PeriodStyle` to parse and print periods that include
more than one unit.

See gh-21136

spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc

@@ -1450,6 +1450,26 @@ Doing so gives a transparent upgrade path while supporting a much richer format.
 
 
 
+[[boot-features-external-config-conversion-period]]
+===== Converting periods
+In addition to durations, Spring Boot can also work with `java.time.Period` type.
+The following formats can be used in application properties:
+
+* An regular `int` representation (using days as the default unit unless a `@PeriodUnit` has been specified)
+* The standard ISO-8601 format {java-api}/java/time/Period.html#parse-java.lang.CharSequence-[used by `java.time.Period`]
+* A simpler format where the value and the unit pairs are coupled (e.g. `1y3d` means 1 year and 3 days)
+
+The following units are supported with the simple format:
+
+* `y` for years
+* `m` for months
+* `w` for weeks
+* `d` for days
+
+NOTE: The `java.time.Period` type never actually stores the number of weeks, it is simply a shortcut that means "`7 days`".
+
+
+
 [[boot-features-external-config-conversion-datasize]]
 ===== Converting Data Sizes
 Spring Framework has a `DataSize` value type that expresses a size in bytes.

spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/fieldvalues/javac/JavaCompilerFieldValuesParser.java

@@ -116,6 +116,19 @@ public class JavaCompilerFieldValuesParser implements FieldValuesParser {
 			DURATION_SUFFIX = Collections.unmodifiableMap(values);
 		}
 
+		private static final String PERIOD_OF = "Period.of";
+
+		private static final Map<String, String> PERIOD_SUFFIX;
+
+		static {
+			Map<String, String> values = new HashMap<>();
+			values.put("Days", "d");
+			values.put("Weeks", "w");
+			values.put("Months", "m");
+			values.put("Years", "y");
+			PERIOD_SUFFIX = Collections.unmodifiableMap(values);
+		}
+
 		private static final String DATA_SIZE_OF = "DataSize.of";
 
 		private static final Map<String, String> DATA_SIZE_SUFFIX;
@@ -130,19 +143,6 @@ public class JavaCompilerFieldValuesParser implements FieldValuesParser {
 			DATA_SIZE_SUFFIX = Collections.unmodifiableMap(values);
 		}
 
-		private static final String PERIOD_OF = "Period.of";
-
-		private static final Map<String, String> PERIOD_SUFFIX;
-
-		static {
-			Map<String, String> values = new HashMap<>();
-			values.put("Days", "d");
-			values.put("Weeks", "w");
-			values.put("Months", "m");
-			values.put("Years", "y");
-			PERIOD_SUFFIX = Collections.unmodifiableMap(values);
-		}
-
 		private final Map<String, Object> fieldValues = new HashMap<>();
 
 		private final Map<String, Object> staticFinals = new HashMap<>();

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/convert/ApplicationConversionService.java

@@ -110,12 +110,12 @@ public class ApplicationConversionService extends FormattingConversionService {
 	public static void addApplicationConverters(ConverterRegistry registry) {
 		addDelimitedStringConverters(registry);
 		registry.addConverter(new StringToDurationConverter());
-		registry.addConverter(new StringToPeriodConverter());
 		registry.addConverter(new DurationToStringConverter());
-		registry.addConverter(new PeriodToStringConverter());
 		registry.addConverter(new NumberToDurationConverter());
-		registry.addConverter(new NumberToPeriodConverter());
 		registry.addConverter(new DurationToNumberConverter());
+		registry.addConverter(new StringToPeriodConverter());
+		registry.addConverter(new PeriodToStringConverter());
+		registry.addConverter(new NumberToPeriodConverter());
 		registry.addConverter(new StringToDataSizeConverter());
 		registry.addConverter(new NumberToDataSizeConverter());
 		registry.addConverter(new StringToFileConverter());

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/convert/PeriodStyle.java

@@ -23,7 +23,6 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.springframework.util.Assert;
-import org.springframework.util.StringUtils;
 
 /**
  * A standard set of {@link Period} units.
@@ -38,25 +37,64 @@ public enum PeriodStyle {
 	/**
 	 * Simple formatting, for example '1d'.
 	 */
-	SIMPLE("^([\\+\\-]?\\d+)([a-zA-Z]{0,2})$") {
+	SIMPLE("^" + "(?:([-+]?[0-9]+)Y)?" + "(?:([-+]?[0-9]+)M)?" + "(?:([-+]?[0-9]+)W)?" + "(?:([-+]?[0-9]+)D)?" + "$",
+			Pattern.CASE_INSENSITIVE) {
 
 		@Override
 		public Period parse(String value, ChronoUnit unit) {
 			try {
+				if (NUMERIC.matcher(value).matches()) {
+					return Unit.fromChronoUnit(unit).parse(value);
+				}
 				Matcher matcher = matcher(value);
 				Assert.state(matcher.matches(), "Does not match simple period pattern");
-				String suffix = matcher.group(2);
+				Assert.isTrue(hasAtLeastOneGroupValue(matcher), "'" + value + "' is not a valid simple period");
-				return (StringUtils.hasLength(suffix) ? Unit.fromSuffix(suffix) : Unit.fromChronoUnit(unit))
+				int years = parseInt(matcher, 1);
-						.parse(matcher.group(1));
+				int months = parseInt(matcher, 2);
+				int weeks = parseInt(matcher, 3);
+				int days = parseInt(matcher, 4);
+				return Period.of(years, months, Math.addExact(Math.multiplyExact(weeks, 7), days));
 			}
 			catch (Exception ex) {
 				throw new IllegalArgumentException("'" + value + "' is not a valid simple period", ex);
 			}
 		}
 
+		boolean hasAtLeastOneGroupValue(Matcher matcher) {
+			for (int i = 0; i < matcher.groupCount(); i++) {
+				if (matcher.group(i + 1) != null) {
+					return true;
+				}
+			}
+			return false;
+		}
+
+		private int parseInt(Matcher matcher, int group) {
+			String value = matcher.group(group);
+			return (value != null) ? Integer.parseInt(value) : 0;
+		}
+
+		@Override
+		protected boolean matches(String value) {
+			return NUMERIC.matcher(value).matches() || matcher(value).matches();
+		}
+
 		@Override
 		public String print(Period value, ChronoUnit unit) {
-			return Unit.fromChronoUnit(unit).print(value);
+			if (value.isZero()) {
+				return Unit.fromChronoUnit(unit).print(value);
+			}
+			StringBuilder result = new StringBuilder();
+			append(result, value, Unit.YEARS);
+			append(result, value, Unit.MONTHS);
+			append(result, value, Unit.DAYS);
+			return result.toString();
+		}
+
+		private void append(StringBuilder result, Period value, Unit unit) {
+			if (!unit.isZero(value)) {
+				result.append(unit.print(value));
+			}
 		}
 
 	},
@@ -64,7 +102,7 @@ public enum PeriodStyle {
 	/**
 	 * ISO-8601 formatting.
 	 */
-	ISO8601("^[\\+\\-]?P.*$") {
+	ISO8601("^[\\+\\-]?P.*$", 0) {
 
 		@Override
 		public Period parse(String value, ChronoUnit unit) {
@@ -83,13 +121,15 @@ public enum PeriodStyle {
 
 	};
 
+	private static final Pattern NUMERIC = Pattern.compile("^[-+]?[0-9]+$");
+
 	private final Pattern pattern;
 
-	PeriodStyle(String pattern) {
+	PeriodStyle(String pattern, int flags) {
-		this.pattern = Pattern.compile(pattern);
+		this.pattern = Pattern.compile(pattern, flags);
 	}
 
-	protected final boolean matches(String value) {
+	protected boolean matches(String value) {
 		return this.pattern.matcher(value).matches();
 	}
 
@@ -175,17 +215,17 @@ public enum PeriodStyle {
 		/**
 		 * Days, represented by suffix {@code d}.
 		 */
-		DAYS(ChronoUnit.DAYS, "d", Period::getDays),
+		DAYS(ChronoUnit.DAYS, "d", Period::getDays, Period::ofDays),
 
 		/**
 		 * Months, represented by suffix {@code m}.
 		 */
-		MONTHS(ChronoUnit.MONTHS, "m", Period::getMonths),
+		MONTHS(ChronoUnit.MONTHS, "m", Period::getMonths, Period::ofMonths),
 
 		/**
 		 * Years, represented by suffix {@code y}.
 		 */
-		YEARS(ChronoUnit.YEARS, "y", Period::getYears);
+		YEARS(ChronoUnit.YEARS, "y", Period::getYears, Period::ofYears);
 
 		private final ChronoUnit chronoUnit;
 
@@ -193,51 +233,29 @@ public enum PeriodStyle {
 
 		private final Function<Period, Integer> intValue;
 
-		Unit(ChronoUnit chronoUnit, String suffix, Function<Period, Integer> intValue) {
+		private final Function<Integer, Period> factory;
+
+		Unit(ChronoUnit chronoUnit, String suffix, Function<Period, Integer> intValue,
+				Function<Integer, Period> factory) {
 			this.chronoUnit = chronoUnit;
 			this.suffix = suffix;
 			this.intValue = intValue;
+			this.factory = factory;
 		}
 
-		/**
+		private Period parse(String value) {
-		 * Return the {@link Unit} matching the specified {@code suffix}.
+			return this.factory.apply(Integer.parseInt(value));
-		 * @param suffix one of the standard suffixes
-		 * @return the {@link Unit} matching the specified {@code suffix}
-		 * @throws IllegalArgumentException if the suffix does not match the suffix of any
-		 * of this enum's constants
-		 */
-		public static Unit fromSuffix(String suffix) {
-			for (Unit candidate : values()) {
-				if (candidate.suffix.equalsIgnoreCase(suffix)) {
-					return candidate;
-				}
-			}
-			throw new IllegalArgumentException("Unknown unit suffix '" + suffix + "'");
 		}
 
-		public Period parse(String value) {
+		private String print(Period value) {
-			int intValue = Integer.parseInt(value);
+			return intValue(value) + this.suffix;
-
-			if (ChronoUnit.DAYS == this.chronoUnit) {
-				return Period.ofDays(intValue);
-			}
-			else if (ChronoUnit.WEEKS == this.chronoUnit) {
-				return Period.ofWeeks(intValue);
-			}
-			else if (ChronoUnit.MONTHS == this.chronoUnit) {
-				return Period.ofMonths(intValue);
-			}
-			else if (ChronoUnit.YEARS == this.chronoUnit) {
-				return Period.ofYears(intValue);
-			}
-			throw new IllegalArgumentException("Unknow unit '" + this.chronoUnit + "'");
 		}
 
-		public String print(Period value) {
+		public boolean isZero(Period value) {
-			return longValue(value) + this.suffix;
+			return intValue(value) == 0;
 		}
 
-		public long longValue(Period value) {
+		public int intValue(Period value) {
 			return this.intValue.apply(value);
 		}
 
@@ -250,7 +268,7 @@ public enum PeriodStyle {
 					return candidate;
 				}
 			}
-			throw new IllegalArgumentException("Unknown unit " + chronoUnit);
+			throw new IllegalArgumentException("Unsupported unit " + chronoUnit);
 		}
 
 	}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/PeriodStyleTests.java

@@ -56,6 +56,14 @@ class PeriodStyleTests {
 		assertThat(PeriodStyle.detectAndParse("-10D")).isEqualTo(Period.ofDays(-10));
 	}
 
+	@Test
+	void detectAndParseWhenSimpleWeeksShouldReturnPeriod() {
+		assertThat(PeriodStyle.detectAndParse("10w")).isEqualTo(Period.ofWeeks(10));
+		assertThat(PeriodStyle.detectAndParse("10W")).isEqualTo(Period.ofWeeks(10));
+		assertThat(PeriodStyle.detectAndParse("+10w")).isEqualTo(Period.ofWeeks(10));
+		assertThat(PeriodStyle.detectAndParse("-10W")).isEqualTo(Period.ofWeeks(-10));
+	}
+
 	@Test
 	void detectAndParseWhenSimpleMonthsShouldReturnPeriod() {
 		assertThat(PeriodStyle.detectAndParse("10m")).isEqualTo(Period.ofMonths(10));
@@ -86,6 +94,16 @@ class PeriodStyleTests {
 		assertThat(PeriodStyle.detectAndParse("-10", ChronoUnit.MONTHS)).isEqualTo(Period.ofMonths(-10));
 	}
 
+	@Test
+	void detectAndParseWhenComplexShouldReturnPeriod() {
+		assertThat(PeriodStyle.detectAndParse("1y2m")).isEqualTo(Period.of(1, 2, 0));
+		assertThat(PeriodStyle.detectAndParse("1y2m3d")).isEqualTo(Period.of(1, 2, 3));
+		assertThat(PeriodStyle.detectAndParse("2m3d")).isEqualTo(Period.of(0, 2, 3));
+		assertThat(PeriodStyle.detectAndParse("1y3d")).isEqualTo(Period.of(1, 0, 3));
+		assertThat(PeriodStyle.detectAndParse("-1y3d")).isEqualTo(Period.of(-1, 0, 3));
+		assertThat(PeriodStyle.detectAndParse("-1y-3d")).isEqualTo(Period.of(-1, 0, -3));
+	}
+
 	@Test
 	void detectAndParseWhenBadFormatShouldThrowException() {
 		assertThatIllegalArgumentException().isThrownBy(() -> PeriodStyle.detectAndParse("10foo"))
@@ -161,8 +179,8 @@ class PeriodStyleTests {
 
 	@Test
 	void parseSimpleWhenUnknownUnitShouldThrowException() {
-		assertThatIllegalArgumentException().isThrownBy(() -> PeriodStyle.SIMPLE.parse("10mb"))
+		assertThatIllegalArgumentException().isThrownBy(() -> PeriodStyle.SIMPLE.parse("10x")).satisfies(
-				.satisfies((ex) -> assertThat(ex.getCause().getMessage()).isEqualTo("Unknown unit suffix 'mb'"));
+				(ex) -> assertThat(ex.getCause().getMessage()).isEqualTo("Does not match simple period pattern"));
 	}
 
 	@Test
@@ -184,15 +202,21 @@ class PeriodStyleTests {
 	}
 
 	@Test
-	void printSimpleWithoutUnitShouldPrintInDays() {
+	void printSimpleWhenZeroWithoutUnitShouldPrintInDays() {
-		Period period = Period.ofMonths(1);
+		Period period = Period.ofMonths(0);
 		assertThat(PeriodStyle.SIMPLE.print(period)).isEqualTo("0d");
 	}
 
 	@Test
-	void printSimpleWithUnitShouldPrintInUnit() {
+	void printSimpleWhenZeroWithUnitShouldPrintInUnit() {
-		Period period = Period.ofYears(1000);
+		Period period = Period.ofYears(0);
-		assertThat(PeriodStyle.SIMPLE.print(period, ChronoUnit.YEARS)).isEqualTo("1000y");
+		assertThat(PeriodStyle.SIMPLE.print(period, ChronoUnit.YEARS)).isEqualTo("0y");
+	}
+
+	@Test
+	void printSimpleWhenNonZeroShouldIgnoreUnit() {
+		Period period = Period.of(1, 2, 3);
+		assertThat(PeriodStyle.SIMPLE.print(period, ChronoUnit.YEARS)).isEqualTo("1y2m3d");
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/PeriodToStringConverterTests.java

@@ -42,18 +42,33 @@ class PeriodToStringConverterTests {
 	}
 
 	@ConversionServiceTest
-	void convertWithFormatShouldUseFormatAndDays(ConversionService conversionService) {
+	void convertWithFormatWhenZeroShouldUseFormatAndDays(ConversionService conversionService) {
-		String converted = (String) conversionService.convert(Period.ofMonths(1),
+		String converted = (String) conversionService.convert(Period.ofMonths(0),
 				MockPeriodTypeDescriptor.get(null, PeriodStyle.SIMPLE), TypeDescriptor.valueOf(String.class));
 		assertThat(converted).isEqualTo("0d");
 	}
 
 	@ConversionServiceTest
-	void convertWithFormatAndUnitShouldUseFormatAndUnit(ConversionService conversionService) {
+	void convertWithFormatShouldUseFormat(ConversionService conversionService) {
-		String converted = (String) conversionService.convert(Period.ofYears(1),
+		String converted = (String) conversionService.convert(Period.of(1, 2, 3),
+				MockPeriodTypeDescriptor.get(null, PeriodStyle.SIMPLE), TypeDescriptor.valueOf(String.class));
+		assertThat(converted).isEqualTo("1y2m3d");
+	}
+
+	@ConversionServiceTest
+	void convertWithFormatAndUnitWhenZeroShouldUseFormatAndUnit(ConversionService conversionService) {
+		String converted = (String) conversionService.convert(Period.ofYears(0),
+				MockPeriodTypeDescriptor.get(ChronoUnit.YEARS, PeriodStyle.SIMPLE),
+				TypeDescriptor.valueOf(String.class));
+		assertThat(converted).isEqualTo("0y");
+	}
+
+	@ConversionServiceTest
+	void convertWithFormatAndUnitWhenNonZeroShouldUseFormatAndIgnoreUnit(ConversionService conversionService) {
+		String converted = (String) conversionService.convert(Period.of(1, 0, 3),
 				MockPeriodTypeDescriptor.get(ChronoUnit.YEARS, PeriodStyle.SIMPLE),
 				TypeDescriptor.valueOf(String.class));
-		assertThat(converted).isEqualTo("1y");
+		assertThat(converted).isEqualTo("1y3d");
 	}
 
 	static Stream<? extends Arguments> conversionServices() throws Exception {