This commit introduces a notion of different styles for the formatting of Duration. The DurationFormat annotation is added to ease selection of a style, which are represented as DurationFormat.Style enum, as well as a supported time unit represented as DurationFormat.Unit enum.

DurationFormatter has been retroffited to take such a Style, optionally, at construction. The default is still the JDK style aka ISO-8601.

This introduces the new SIMPLE style which uses a single number + a short human-readable suffix. For instance -3ms or 2h.

This has the same semantics as the DurationStyle in Spring Boot and is intended as a replacement for that feature, providing access to the feature to projects that only depend on Spring Framework.

Finally, the @Scheduled annotation is improved by adding detection of the style and parsing for the String versions of initial delay, fixed delay and fixed rate.

See gh-22013 See gh-22474

Comment From: simonbasle

Note: the first commit was closer to the arrangement found in Boot, but there was too much logic in the annotation package for my taste, and it was exposing Unit as a wrapper over ChronoUnit. This difference in package structure was also increasing the risk of package tangle, so I extracted the parsing and printing logic into DurationFormatterUtils.

Comment From: simonbasle

third iteration (missing a bit of documentation) is reintroducing DurationFormat.Unit, bearing part of the logic in dealing with suffixes and giving a better sense of exactly which units are actually supported (unlike with ChronoUnit where eg. MILLENIA would compile but fail at runtime)

Comment From: philwebb

I might be a bit biased, but I personally quite liked the logic being in the DurationStyle enum rather than another *Utils class. Especially as DurationFormatterUtils now depends on an enum that's inside the org.springframework.format.annotation.DurationFormat annotation.

I also think it's a bit easier to use. E.g. DurationFormatterUtils.parse("10s", DurationFormat.Style.SIMPLE) vs DurationStyle.SIMPLE.parse("10s").

Perhaps the enum can be extracted from the annotation and moved to the org.springframework.format.datetime.standard package?

These tests in Spring Boot show how that API might look when being used.

Comment From: snicoll

Spring Framework has Utils classes in various places so I can understand why Simon went down that route. And having all that code as part of the annotation package felt wrong.

I like the idea of moving the unit class outside of the annotation package. It looks like it would improve the usability of the code and address the concern of having too much code in the annotation package.

Comment From: simonbasle

I went that route for consistency with Framework current package arrangement, with some extra effort in avoiding too much logic in the annotation. Unit-related methods felt simple enough that they could be implemented in the enum, as a middle ground.

The trouble with moving the Unit (or Style for that matter) outside of the annotation package is that the @DurationFormat annotation depends on the Style and Unit as part of its API. We can have package org.springframework.format.datetime.standard depend on org.springframework.format.annotation (there's precedent here), but not the other way around.

edit: to be clear I'm not against making further changes here, as ensuring good compatibility/migrability with what Boot has is definitely a priority, but this felt like the best arrangement to me so far.

Comment From: philwebb

I missed the existing org.springframework.format.datetime.standard -> org.springframework.format.annotation dependency. That does indeed complicate things!

I wonder what DurationFormatterUtils would look like if it didn't use enums from org.springframework.format.annotation? Perhaps it could offer public parseIso8601, parseSimple and parseDetected methods and leave something else to work out which of those methods to call if the annotation is present. I guess the DurationFormat.Unit somewhat messes with that, but ChronoUnit or TimeUnit could be an option (at the expense of runtime failures if a user is picks ChronoUnit.MILLENIA.

I think as it stands DurationFormatterUtils is quite coupled to @DurationFormat. That shows up in ScheduledAnnotationBeanPostProcessor which is pulling in DurationFormat.Unit so that it can call fromChronoUnit and then DurationFormatterUtils.detectAndParse. In fact, I think ScheduledAnnotationBeanPostProcessor is the only reason that DurationFormatterUtils is public and I think detectAndParse is the only method it needs.

AFAICT we don't have any other public utils classes under org.springframework.format. I wonder if using DurationFormatter directly in ScheduledAnnotationBeanPostProcessor might help us keep the utils package-private?

E.g., in ScheduledAnnotationBeanPostProcessor:

private static Duration toDuration(String value, TimeUnit timeUnit) {
    new DurationFormatter(timeUnit).parse(value);
}

Comment From: bclozel

I've just reviewed this PR and rebased against the main branch. There were some changes on the @Scheduled annotation which create a conflict. I've resolved them locally with the following (see the 3rd item in the bullet point list):

/**
 * Execute the annotated method with a fixed period between the end of the
 * last invocation and the start of the next.
 * <p>The duration String can be in several formats:
 * <ul>
 *     <li>a plain integer &mdash; which is interpreted to represent a duration in
 *     milliseconds by default unless overridden via {@link #timeUnit()} (prefer
 *     using {@link #fixedDelay()} in that case)</li>
 *     <li>any of the known {@link org.springframework.format.annotation.DurationFormat.Style
 *     DurationFormat.Style}: the {@link org.springframework.format.annotation.DurationFormat.Style#ISO8601 ISO8601}
 *     style or the {@link org.springframework.format.annotation.DurationFormat.Style#SIMPLE SIMPLE} style
 *     &mdash; using the {@link #timeUnit()} as fallback if the string doesn't contain an explicit unit</li>
 *     <li>one of the above, with Spring-style "${...}" placeholders as well as SpEL expressions</li>
 * </ul>
 * @return the delay as a String value &mdash; for example a placeholder,
 * or a {@link org.springframework.format.annotation.DurationFormat.Style#ISO8601 java.time.Duration} compliant value
 * or a {@link org.springframework.format.annotation.DurationFormat.Style#SIMPLE simple format} compliant value
 * @since 3.2.2
 * @see #fixedDelay()
 */

AFAICT we don't have any other public utils classes under org.springframework.format. I wonder if using DurationFormatter directly in ScheduledAnnotationBeanPostProcessor might help us keep the utils package-private?

I've tried that approach, and this involves moving the detection code to the DurationFormat.Style enum and the usage pattern doesn't seem ideal:

private static Duration toDuration(String value, TimeUnit timeUnit) {
    DurationFormat.Unit unit = DurationFormat.Unit.fromChronoUnit(timeUnit.toChronoUnit());
    DurationFormat.Style style = DurationFormat.Style.detect(value);
    try {
        return new DurationFormatter(style, unit).parse(value, null);
    } catch (ParseException exc) {
        throw new IllegalArgumentException(exc);
    }
}

As Simon said:

ensuring good compatibility/migrability with what Boot has is definitely a priority, but this felt like the best arrangement to me so far.

I think we don't have enough information at this point to decide whether this arrangement is good enough for Spring Boot. I'm rescheduling this to 6.1.x as we need to find a proper time to experiment with both Spring Framework and Spring Boot SNAPSHOTs.

Comment From: philwebb

There's a comment on https://github.com/spring-projects/spring-boot/issues/37562#issuecomment-2036949439 which might be relevant to this feature. It would be nice to support Kotlin 1.5s style values.