RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets

[Stephen Colebourne]

Thanks for the update.

The test case does not cover the situation of MAX/MIN instant and an
offset other than zero, where the offset makes the instant invalid.
eg. a negative offset at MAX or a positive offset at MIN.

The doc of appendInstant() in DateTimeFormatterBuilder should be
clarified to cover the fact that any OffsetId is parsed.

thanks
Stephen


On Thu, 20 Sep 2018 at 13:54, Pallavi Sonal <pallavi.sonal at oracle.com> wrote:
>
> Thanks Roger , Naoto and Stephen for the review and valuable inputs.
> Here is the updated webrev for review :
> http://cr.openjdk.java.net/~rpatil/8166138/webrev.01/
>
> Thanks,
> Pallavi Sonal
>
> -----Original Message-----
> From: Stephen Colebourne <scolebourne at joda.org>
> Sent: Thursday, September 20, 2018 2:50 AM
> To: core-libs-dev <core-libs-dev at openjdk.java.net>
> Subject: Re: RFR: JDK-8166138 - DateTimeFormatter.ISO_INSTANT should handle offsets
>
> Thanks for looking at this.
>
> The proposed fix does not tackle the bug fully. The bug is that the spec says
>
> "The format consists of: The ISO_OFFSET_DATE_TIME where ..."
>
> As such, the format must parse *any* offset, not just "Z" / "+00:00" etc.
>
> In addition, the fix doesn't work properly. Parsers work off a CharSequence which may be much longer than the instant. For example, the instant might be followed by a literal space and then a ZoneId.
> Using (length - 3) is simply not a valid approach - the parsing code cannot use the length like that.
>
> Furthermore, although there are numerous valid ISO-8601 ways of saying zero, this format uses dashes and colons in the date/time part, so
> ISO-8601 restricts the offset to only those formats that include colons.
>
> I think it simply needs the appendLiteral(Z) changing to appendOffsetId() And line 3495 changes to use the offset from the newContext.
>
> thanks
> Stephen
>
>
> On Wed, 19 Sep 2018 at 18:16, Pallavi Sonal <pallavi.sonal at oracle.com> wrote:
> >
> > Hi,
> >
> > Please review the changes to the following issue:
> >
> > Bug : https://bugs.openjdk.java.net/browse/JDK-8166138
> >
> >
> >
> > The proposed fix is located at:
> >
> > Webrev : http://cr.openjdk.java.net/~rpatil/8166138/webrev.00/
> >
> >
> >
> > As per ISO 8601 standards, an offset of zero, in addition to having the special representation "Z", can also be stated numerically as "+00:00", "+0000", or "+00" [1].  With this fix, Instant.parse() can parse a String containing the zero offsets in any of these three forms. Any other offset apart from "Z", "+00:00", "+0000", or "+00" will not be accepted in the input string to be parsed.
> >
> >
> >
> > [1] https://en.wikipedia.org/wiki/ISO_8601
> >
> >
> >
> > Thanks,
> >
> > Pallavi Sonal
> >
> >
> >
> >