Add support for converting a String to an Instant

[JanecekPetr]

What I'm trying to achieve:

@ConfigurationProperties
record ApplicationProps(Instant instant) {}

with the following application.properties:

instant = 1680612880417

where the value is millis since the epoch, see Instant.ofEpochMilli().

Edit:
Currently this fails with a

...
Caused by: java.lang.IllegalArgumentException: Parse attempt failed for value [123456789]
	at org.springframework.format.support.FormattingConversionService$ParserConverter.convert(FormattingConversionService.java:224) ~[spring-context-6.0.6.jar:6.0.6]
	at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41) ~[spring-core-6.0.6.jar:6.0.6]
	... 117 common frames omitted
Caused by: java.time.format.DateTimeParseException: Text '123456789' could not be parsed at index 0

Perhaps this is a good candidate for a default binding.

Considerations:

• Why are we choosing Instant.ofEpochMilli() and not Instant.ofEpochSecond()? It just felt more natural. Is this uncertainty reason enough to refuse this request? That said, there already is a registered-by-default Converter<Long, Instant> which uses ofEpochMilli. Therefore, an additional Formatter would be consistent with the already-existing Converter.
• Do we also need to add more similar formatters for types like java.util.Date or java.sql.Timestamp? I do no think think so as neither of them has a simple ofEpochMilli()-like creator method or clear semantics. Other java.time also are not good targets for this unless OffsetDateTime/ZonedDateTime is considered okay given a default timezone.

[snicoll]

Why are we choosing Instant.ofEpochMilli() and not Instant.ofEpochSecond()?

Because the usual way of representing a "time" is by using the number of ms, not seconds. I understand that you'd prefer seconds but that's not what the default representation is at the moment: the seconds variant is merely a shortcut for those who don't need ms precision.

Both inputs are numbers so it's impossible for us to know which format you want to use automatically. I can't even suggest to configure a custom converter, unless you want to trump the default one for every fields.

Having said all of that, I am not sure I understand what you'd expect from us. Can you clarify?

[JanecekPetr]

I understand that you'd prefer seconds

Sorry, maybe I need to rephrase my question as that was not what I meant. I absolutely prefer millis and I agree. I was just adding considerations and alternatives that someone might ask for instead.

Can you clarify?

Sure! (Should I also edit the original question?)

Currently it is not possible to use a timestamp as a property of type Instant. I'd like Spring to parse that, ideally by default. My suggestion is for you to add this capability to the existing InstantFormatter.

If you try the code I suggested above, it fails with:

...
Caused by: org.springframework.beans.TypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'java.time.Instant'; Failed to convert from type [java.lang.String] to type [@org.springframework.beans.factory.annotation.Value java.time.Instant] for value '123456789'
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:79) ~[spring-beans-6.0.6.jar:6.0.6]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1353) ~[spring-beans-6.0.6.jar:6.0.6]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1325) ~[spring-beans-6.0.6.jar:6.0.6]
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:885) ~[spring-beans-6.0.6.jar:6.0.6]
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:789) ~[spring-beans-6.0.6.jar:6.0.6]
	... 110 common frames omitted
Caused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [@org.springframework.beans.factory.annotation.Value java.time.Instant] for value '123456789'
	at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:47) ~[spring-core-6.0.6.jar:6.0.6]
	at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:192) ~[spring-core-6.0.6.jar:6.0.6]
	at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:129) ~[spring-beans-6.0.6.jar:6.0.6]
	at org.springframework.beans.TypeConverterSupport.convertIfNecessary(TypeConverterSupport.java:73) ~[spring-beans-6.0.6.jar:6.0.6]
	... 114 common frames omitted
Caused by: java.lang.IllegalArgumentException: Parse attempt failed for value [123456789]
	at org.springframework.format.support.FormattingConversionService$ParserConverter.convert(FormattingConversionService.java:224) ~[spring-context-6.0.6.jar:6.0.6]
	at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41) ~[spring-core-6.0.6.jar:6.0.6]
	... 117 common frames omitted
Caused by: java.time.format.DateTimeParseException: Text '123456789' could not be parsed at index 0
	at java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2052) ~[na:na]
	at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1954) ~[na:na]
	at java.base/java.time.Instant.parse(Instant.java:397) ~[na:na]
	at org.springframework.format.datetime.standard.InstantFormatter.parse(InstantFormatter.java:50) ~[spring-context-6.0.6.jar:6.0.6]
	at org.springframework.format.datetime.standard.InstantFormatter.parse(InstantFormatter.java:40) ~[spring-context-6.0.6.jar:6.0.6]
	at org.springframework.format.support.FormattingConversionService$ParserConverter.convert(FormattingConversionService.java:218) ~[spring-context-6.0.6.jar:6.0.6]
	... 118 common frames omitted

[snicoll]

Oh. I think the considerations have confused me. Yes, I agree supporting instant would be nice and I don't think we should bother about the seconds variant.

[wilkinsona]

@snicoll Given the involvement of org.springframework.format.datetime.standard.InstantFormatter, should this be transferred to Framework?

[snicoll]

Yes. We do have issues to move some of the converters that we have in Spring Boot already so that makes sense.

[RemusRD]

Hey! Can I take this one? :)

[simonbasle]

@RemusRD I think it is worth a shot yes. From my perspective it should be possible to retrofit this parsing into existing InstantFormatter. I have the following test to suggest in DateTimeFormattingTests.java, but please include other more direct unit tests as well:

	@Test
	void testBindInstantAsLongEpochMillis() {
		MutablePropertyValues propertyValues = new MutablePropertyValues();
		propertyValues.add("instant", 1234L);
		binder.bind(propertyValues);
		assertThat(binder.getBindingResult().getErrorCount()).isZero();
		assertThat(binder.getBindingResult().getRawFieldValue("instant"))
				.isInstanceOf(Instant.class)
				.isEqualTo(Instant.ofEpochMilli(1234L));
		assertThat(binder.getBindingResult().getFieldValue("instant"))
				.hasToString("1970-01-01T00:00:01.234Z");
	}

[RemusRD]

@RemusRD I think it is worth a shot yes. From my perspective it should be possible to retrofit this parsing into existing InstantFormatter. I have the following test to suggest in DateTimeFormattingTests.java, but please include other more direct unit tests as well:

	@Test
	void testBindInstantAsLongEpochMillis() {
		MutablePropertyValues propertyValues = new MutablePropertyValues();
		propertyValues.add("instant", 1234L);
		binder.bind(propertyValues);
		assertThat(binder.getBindingResult().getErrorCount()).isZero();
		assertThat(binder.getBindingResult().getRawFieldValue("instant"))
				.isInstanceOf(Instant.class)
				.isEqualTo(Instant.ofEpochMilli(1234L));
		assertThat(binder.getBindingResult().getFieldValue("instant"))
				.hasToString("1970-01-01T00:00:01.234Z");
	}

I already started working on this! I added your test which seems to be already passing, should I keep it?

Also, would be great if you could share with me any other merge requests that are already implemented similar to this one so I can take inspiration :) from.

[RemusRD]

I have added more tests and the functionality to the InstantFormatter.java however, I have found if I use RandomInstantProvider.java in my tests, the values provided will overflow the Long.class making my tests fail, any suggestion on how to approach this? I could create a new provider, or limit the current one to be from Long.MIN to Long.Max

@ParameterizedTest
@ArgumentsSource(RandomInstantProvider.class)
void should_parse_into_an_Instant_from_epoch_mili(Instant input) throws ParseException {
    String inputS = String.valueOf(input.toEpochMilli()); // toEpochMilli() will overflow and make the test fail
    Instant expected = Instant.ofEpochMilli(Long.parseLong(inputS));

    Instant actual = instantFormatter.parse(inputS, null);

    assertThat(actual).isEqualTo(expected);
}

[simonbasle]

yeah RandomInstantProvider is not a good fit because it will create Instants that fall off the limit of epochMillis (due to using seconds and not millis). You could take inspiration from it to create a similar provider which uses ofEpochMillis(Long.MIN_VALUE)/ofEpochMilli(Long.MAX_VALUE). I would also advise to truncateTo(ChronoUnit.MILLIS) each generated Instant.

[simonbasle]

any progress @RemusRD ?

[RemusRD]

any progress @RemusRD ?

Submitting the PR shortly, I added tests into the InstantFormatter, wondering if it would make sense to do a more end to end test? Like checking it works from parsing a yaml or something like that.

[RemusRD]

@simonbasle I have opened the PR, but not sure how I can link it here? #30546

[simonbasle]

• superseded by Support parsing long dates in InstantFormatter #30546