I have to use DateTimeFormatter.ISO_OFFSET_DATE_TIME
in Spring MVC controllers with OffsetDateTime
parameters.
It would be useful if the new spring.mvc.format.date-time
property could accept a value iso-offset
as a shortcut for this.
Comment From: philwebb
This is has been marked as a first-timers-only
issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.
If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!
If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution
issues. Thanks!
Problem
The spring.mvc.format.date-time
property has a convenient shortcut that can be used for ISO formatted dates. In addition to supporting the "iso" shortcut, we'd like to support "iso-offset" which would map to the ISO_OFFSET
constants in java.time.format.DateTimeFormatter
Solution
The actual logic for this shortcut is in the org.springframework.boot.autoconfigure.web.format.DateTimeFormatters
class. The method of that class call isIso
to determine if the ISO format should be used and then return the appropriate formatter.
This logic should be updated so that iso-offset
or isooffset
can be used as a value.
Steps to Fix
- [x] Claim this issue with a comment below and ask any clarifying questions you need
- [x] Set up a repository locally following the Contributing Guidelines
- [x] Try to fix the issue following the steps above
- [x] Commit your changes and start a pull request.
Comment From: philwebb
@gaurav-91 has been asking if he can help with issues, so I'd like to give him the opportunity to claim this one if they'd like to.
Comment From: gaurav-91
Thank you @philwebb , I would love to start working on it. Apologies for commenting on multiple requests as this is first time I am starting up on open source.
Comment From: gaurav-91
@philwebb Just make sure my understanding is correct we need to edit additional-spring-configuration-metadata.json with extra property for "spring.mvc.format.date-time" alone and then corresponding changes into public DateTimeFormatters dateTimeFormat(String pattern) method.
Or it is required to be added for dateFormmatter & timeFormatter as well. Thanks
Comment From: philwebb
I believe that three methods (dateFormat
, timeFormat
and dateTimeFormat
) in DateTimeFormatters
will all need to be updated and likewise all three of the properties in configuration-metadata.json
.
Feel free to submit a pull-request with FIXMEs
if you like and we can iterate on the code together next week.
Comment From: gaurav-91
I have made the changes but was trying write sample test case as well for it. Below is what is tried but doesn't seem to be correct to me.
WebConversionService conversionService = new WebConversionService(new DateTimeFormatters().dateFormat("isooffset"));
//LocalDate date = LocalDate.parse("2011-12-03", DateTimeFormatter.ISO_OFFSET_DATE);
OffsetDateTime offsetdate=new Date().toInstant().atOffset(ZoneOffset.ofHoursMinutes(1, 30));
System.out.println(conversionService.convert(offsetdate, String.class));
assertThat(conversionService.convert(offsetdate, String.class)) .isEqualTo(DateTimeFormatter.ISO_OFFSET_DATE.format(offsetdate));
Could you please guide me little bit here.
Comment From: mbhave
@gaurav-91 Could you create a Pull Request with your changes? That would make it easier for us to look at the code and iterate on it. It's fine even if your test case doesn't pass at the moment.
Comment From: philwebb
Looks like a PR has been started in #21630
Comment From: gaurav-91
Yes I have created PR , could you please review it. Please let me know if we need to iterate over it together. Thanks
Comment From: dvrajitha
@philwebb any new first-timers-only issues?
Comment From: wilkinsona
@dvrajitha Not at the moment, I'm afraid. You can use this link to see all the issues labelled as first-timers-only.
Comment From: dvrajitha
@wilkinsona Thank you.. There is only one and I think @gaurav-91 has already started it?
Comment From: wilkinsona
Yes, that’s right, hence my “not at the moment”. You can use the link to keep an eye out for first-timer issues in the future.
Comment From: dvrajitha
@wilkinsona Thank you.. Yes will do!
Comment From: mbhave
Closing in favor of PR #21630.