From b4ceef286d9c3c204463c4beafef55adce8024e9 Mon Sep 17 00:00:00 2001
From: perceptron8 <perceptron8@users.noreply.github.com>
Date: Sat, 10 Dec 2022 23:03:08 +0100
Subject: [PATCH] Rewrite `Interval.parse(CharSequence)`

---
 .../java/org/threeten/extra/Interval.java     | 152 ++++++++++--------
 .../java/org/threeten/extra/TestInterval.java |  40 ++++-
 2 files changed, 124 insertions(+), 68 deletions(-)

diff --git a/src/main/java/org/threeten/extra/Interval.java b/src/main/java/org/threeten/extra/Interval.java
index f7b2a8cf..ad45b4d3 100644
--- a/src/main/java/org/threeten/extra/Interval.java
+++ b/src/main/java/org/threeten/extra/Interval.java
@@ -40,7 +40,8 @@
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
-import java.time.temporal.TemporalAccessor;
+import java.time.temporal.Temporal;
+import java.time.temporal.TemporalAmount;
 import java.util.Objects;
 
 import org.joda.convert.FromString;
@@ -78,6 +79,11 @@ public final class Interval
      */
     private static final long serialVersionUID = 8375285238652L;
 
+    /**
+     * Leap year cycle length.
+     */
+    private static final Duration leapYearCycleLength = Duration.ofHours(3506328L);
+
     /**
      * The start instant (inclusive).
      */
@@ -156,12 +162,14 @@ public static Interval endingAt(Instant endExclusive) {
      * Obtains an instance of {@code Interval} from a text string such as
      * {@code 2007-12-03T10:15:30Z/2007-12-04T10:15:30Z}, where the end instant is exclusive.
      * <p>
-     * The string must consist of one of the following four formats:
+     * The string must consist of one of the following five formats:
      * <ul>
      * <li>a representations of an {@link OffsetDateTime}, followed by a forward slash,
      *  followed by a representation of a {@link OffsetDateTime}
      * <li>a representations of an {@link OffsetDateTime}, followed by a forward slash,
      *  followed by a representation of a {@link LocalDateTime}, where the end offset is implied.
+     * <li>a representations of an {@link LocalDateTime}, followed by a forward slash,
+     *  followed by a representation of a {@link OffsetDateTime}, where the start offset is implied.
      * <li>a representation of an {@link OffsetDateTime}, followed by a forward slash,
      *  followed by a representation of a {@link PeriodDuration}
      * <li>a representation of a {@link PeriodDuration}, followed by a forward slash,
@@ -180,80 +188,92 @@ public static Interval parse(CharSequence text) {
         Objects.requireNonNull(text, "text");
         for (int i = 0; i < text.length(); i++) {
             if (text.charAt(i) == '/') {
-                return parseSplit(text.subSequence(0, i), text.subSequence(i + 1, text.length()));
+                CharSequence startStr = text.subSequence(0, i);
+                CharSequence endStr = text.subSequence(i + 1, text.length());
+                if (startStr.charAt(0) == 'P' || startStr.charAt(0) == 'p') {
+                    // duration followed by temporal
+                    TemporalAmount duration = parseDuration(startStr);
+                    Temporal temporal = parseTemporal(endStr);
+                    return Interval.of(Instant.from(minus(temporal, duration)), Instant.from(temporal));
+                } else if (endStr.charAt(0) == 'P' || endStr.charAt(0) == 'p') {
+                    // temporal followed by duration
+                    Temporal temporal = parseTemporal(startStr);
+                    TemporalAmount duration = parseDuration(endStr);
+                    return Interval.of(Instant.from(temporal), Instant.from(plus(temporal, duration)));
+                } else {
+                    // temporal followed by temporal
+                    Temporal start = parseTemporal(startStr);
+                    Temporal end = parseTemporal(endStr);
+                    if (start instanceof LocalDateTime) {
+                        return Interval.of(((LocalDateTime) start).atOffset(ZoneOffset.from(end)).toInstant(), Instant.from(end));
+                    }
+                    if (end instanceof LocalDateTime) {
+                        return Interval.of(Instant.from(start), ((LocalDateTime) end).atOffset(ZoneOffset.from(start)).toInstant());
+                    }
+                    return Interval.of(Instant.from(start), Instant.from(end));
+                }
             }
         }
         throw new DateTimeParseException("Interval cannot be parsed, no forward slash found", text, 0);
     }
 
-    private static Interval parseSplit(CharSequence startStr, CharSequence endStr) {
-        char firstChar = startStr.charAt(0);
-        if (firstChar == 'P' || firstChar == 'p') {
-            // duration followed by instant
-            PeriodDuration amount = PeriodDuration.parse(startStr);
-            try {
-                OffsetDateTime end = OffsetDateTime.parse(endStr);
-                return Interval.of(end.minus(amount).toInstant(), end.toInstant());
-            } catch (DateTimeParseException ex) {
-                // handle case where Instant is outside the bounds of OffsetDateTime
-                Instant end = Instant.parse(endStr);
-                // addition of PeriodDuration only supported by OffsetDateTime,
-                // but to make that work need to move point being subtracted from closer to EPOCH
-                long move = end.isBefore(Instant.EPOCH) ? 1000 * 86400 : -1000 * 86400;
-                Instant start = end.plusSeconds(move).atOffset(ZoneOffset.UTC).minus(amount).toInstant().minusSeconds(move);
-                return Interval.of(start, end);
-            }
-        }
-        // instant followed by instant or duration
-        OffsetDateTime start;
+    /**
+     * Obtains an instance of {@code Temporal} from a text string.
+     *
+     * @param text the text to parse, validated not null
+     * @return the parsed temporal, not null
+     */
+    private static Temporal parseTemporal(CharSequence text) {
         try {
-            start = OffsetDateTime.parse(startStr);
-        } catch (DateTimeParseException ex) {
-            return parseStartExtended(startStr, endStr);
-        }
-        if (endStr.length() > 0) {
-            char c = endStr.charAt(0);
-            if (c == 'P' || c == 'p') {
-                PeriodDuration amount = PeriodDuration.parse(endStr);
-                return Interval.of(start.toInstant(), start.plus(amount).toInstant());
-            }
+            // temporal within date-time bounds
+            return (Temporal) DateTimeFormatter.ISO_DATE_TIME.parseBest(text, OffsetDateTime::from, LocalDateTime::from);
+        } catch (DateTimeParseException exception) {
+            // temporal outside date-time bounds
+            return Instant.parse(text);
         }
-        return parseEndDateTime(start.toInstant(), start.getOffset(), endStr);
-    }
-
-    // handle case where Instant is outside the bounds of OffsetDateTime
-    private static Interval parseStartExtended(CharSequence startStr, CharSequence endStr) {
-        Instant start = Instant.parse(startStr);
-        if (endStr.length() > 0) {
-            char c = endStr.charAt(0);
-            if (c == 'P' || c == 'p') {
-                PeriodDuration amount = PeriodDuration.parse(endStr);
-                // addition of PeriodDuration only supported by OffsetDateTime,
-                // but to make that work need to move point being added to closer to EPOCH
-                long move = start.isBefore(Instant.EPOCH) ? 1000 * 86400 : -1000 * 86400;
-                Instant end = start.plusSeconds(move).atOffset(ZoneOffset.UTC).plus(amount).toInstant().minusSeconds(move);
-                return Interval.of(start, end);
-            }
+    }
+
+    /**
+     * Obtains an instance of {@code TemporalAmount} from a text string.
+     *
+     * @param text the text to parse, validated not null
+     * @return the parsed temporal amount, not null
+     */
+    private static TemporalAmount parseDuration(CharSequence text) {
+        return PeriodDuration.parse(text);
+    }
+
+    /**
+     * Returns a copy of given temporal with the specified amount added.
+     *
+     * @param temporal  the temporal, validated not null
+     * @param amount  the amount to add, validated not null
+     * @return a {@code Temporal} based on given temporal with the addition made, not null
+     */
+    private static Temporal plus(Temporal temporal, TemporalAmount amount) {
+        if (temporal instanceof Instant) {
+            Instant instant = (Instant) temporal;
+            TemporalAmount shift = instant.isBefore(Instant.EPOCH) ? leapYearCycleLength : leapYearCycleLength.negated();
+            return instant.plus(shift).atOffset(ZoneOffset.UTC).plus(amount).toInstant().minus(shift);
+        } else {
+            return temporal.plus(amount);
         }
-        // infer offset from start if not specified by end
-        return parseEndDateTime(start, ZoneOffset.UTC, endStr);
     }
 
-    // parse when there are two date-times
-    private static Interval parseEndDateTime(Instant start, ZoneOffset offset, CharSequence endStr) {
-        try {
-            TemporalAccessor temporal = DateTimeFormatter.ISO_DATE_TIME.parseBest(endStr, OffsetDateTime::from, LocalDateTime::from);
-            if (temporal instanceof OffsetDateTime) {
-                OffsetDateTime odt = (OffsetDateTime) temporal;
-                return Interval.of(start, odt.toInstant());
-            } else {
-                // infer offset from start if not specified by end
-                LocalDateTime ldt = (LocalDateTime) temporal;
-                return Interval.of(start, ldt.toInstant(offset));
-            }
-        } catch (DateTimeParseException ex) {
-            Instant end = Instant.parse(endStr);
-            return Interval.of(start, end);
+    /**
+     * Returns a copy of given temporal with the specified amount subtracted.
+     *
+     * @param temporal  the temporal, validated not null
+     * @param amount  the amount to subtract, validated not null
+     * @return a {@code Temporal} based on given temporal with the subtraction made, not null
+     */
+    private static Temporal minus(Temporal temporal, TemporalAmount amount) {
+        if (temporal instanceof Instant) {
+            Instant instant = (Instant) temporal;
+            TemporalAmount shift = instant.isBefore(Instant.EPOCH) ? leapYearCycleLength : leapYearCycleLength.negated();
+            return instant.plus(shift).atOffset(ZoneOffset.UTC).minus(amount).toInstant().minus(shift);
+        } else {
+            return temporal.minus(amount);
         }
     }
 
diff --git a/src/test/java/org/threeten/extra/TestInterval.java b/src/test/java/org/threeten/extra/TestInterval.java
index d7b698cd..13ec014a 100644
--- a/src/test/java/org/threeten/extra/TestInterval.java
+++ b/src/test/java/org/threeten/extra/TestInterval.java
@@ -51,8 +51,10 @@
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeParseException;
 
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 
 /**
@@ -230,8 +232,8 @@ public void test_endingAt_null() {
     }
 
     /* Lower and upper bound for Intervals */
-    private static final Instant MIN_OFFSET_DATE_TIME = OffsetDateTime.MIN.plusDays(1L).toInstant();
-    private static final Instant MAX_OFFSET_DATE_TIME = OffsetDateTime.MAX.minusDays(1L).toInstant();
+    private static final Instant MIN_OFFSET_DATE_TIME = OffsetDateTime.MIN.toInstant();
+    private static final Instant MAX_OFFSET_DATE_TIME = OffsetDateTime.MAX.toInstant();
 
     //-----------------------------------------------------------------------
     public static Object[][] data_parseValid() {
@@ -252,6 +254,7 @@ public static Object[][] data_parseValid() {
             {NOW1.atOffset(ZoneOffset.ofHours(2)) + "/" + NOW2.atOffset(ZoneOffset.ofHours(2)), NOW1, NOW2},
             {NOW1.atOffset(ZoneOffset.ofHours(2)) + "/" + NOW2.atOffset(ZoneOffset.ofHours(3)), NOW1, NOW2},
             {NOW1.atOffset(ZoneOffset.ofHours(2)) + "/" + NOW2.atOffset(ZoneOffset.ofHours(2)).toLocalDateTime(), NOW1, NOW2},
+            {NOW1.atOffset(ZoneOffset.ofHours(2)).toLocalDateTime() + "/" + NOW2.atOffset(ZoneOffset.ofHours(2)), NOW1, NOW2},
             {MIN_OFFSET_DATE_TIME.toString() + "/" + MAX_OFFSET_DATE_TIME, MIN_OFFSET_DATE_TIME, MAX_OFFSET_DATE_TIME},
             {NOW1 + "/" + Instant.MAX, NOW1, Instant.MAX},
             {Instant.MIN.toString() + "/" + NOW2, Instant.MIN, NOW2},
@@ -267,6 +270,39 @@ public void test_parse_CharSequence(String input, Instant start, Instant end) {
         assertEquals(end, test.getEnd());
     }
 
+    @ParameterizedTest
+    @CsvSource({
+        "-1000000000-01-31T00:00:00Z/P1M, -1000000000-01-31T00:00:00Z, -1000000000-02-29T00:00:00Z",
+        "-1000000000-01-31T00:00:00Z/P2M, -1000000000-01-31T00:00:00Z, -1000000000-03-31T00:00:00Z",
+        "P5M/+1000000000-12-31T23:59:59.999999999Z, +1000000000-07-31T23:59:59.999999999Z, +1000000000-12-31T23:59:59.999999999Z",
+        "P10M/+1000000000-12-31T23:59:59.999999999Z, +1000000000-02-29T23:59:59.999999999Z, +1000000000-12-31T23:59:59.999999999Z",
+    })
+    public void data_parse_outside_bounds(String interval, String start, String end) {
+        assertEquals(Interval.of(Instant.parse(start), Instant.parse(end)), Interval.parse(interval));
+    }
+
+    @Disabled("not implemented yet")
+    @ParameterizedTest
+    @CsvSource({
+        "P1Y/-999999999-01-01T00:00:00+00:00, -1000000000-01-01T00:00:00Z, +999999999-01-01T00:00:00Z",
+        "-999999999-01-01T00:00:00+00:00/P-1Y, -1000000000-01-01T00:00:00Z, +999999999-01-01T00:00:00Z",
+        "+999999999-01-01T00:00:00+00:00/P1Y, +999999999-01-01T00:00:00Z, +1000000000-01-01T00:00:00Z",
+        "P-1Y/+999999999-01-01T00:00:00+00:00, +999999999-01-01T00:00:00Z, +1000000000-01-01T00:00:00Z",
+    })
+    public void data_parse_crossing_bounds(String interval, String start, String end) {
+        assertEquals(Interval.of(Instant.parse(start), Instant.parse(end)), Interval.parse(interval));
+    }
+
+    @Disabled("not implemented yet")
+    @ParameterizedTest
+    @CsvSource({
+        "-1000000000-01-01T00:00:00Z/P2000000000Y11M30DT23H59M59.999999999S, -1000000000-01-01T00:00:00Z, +1000000000-12-31T23:59:59.999999999Z",
+        "P2000000000Y11M30DT23H59M59.999999999S/+1000000000-12-31T23:59:59.999999999Z, -1000000000-01-01T00:00:00Z, +1000000000-12-31T23:59:59.999999999Z",
+    })
+    public void data_parse_crossing_bounds_twice(String interval, String start, String end) {
+        assertEquals(Interval.of(Instant.parse(start), Instant.parse(end)), Interval.parse(interval));
+    }
+
     @Test
     public void test_parse_CharSequence_badOrder() {
         assertThrows(DateTimeException.class, () -> Interval.parse(NOW2 + "/" + NOW1));