Reduce the chance of providing an unexpected duration for every() (#5)

tginsberg · web-flow · commit ad782121b92f · 2017-11-26T11:24:46.000-06:00
+ All amounts are taken as the absolute value.
+ If amount is zero, throw an exception or we will end up with
  infinite streams of the same instant.
diff --git a/src/main/java/com/ginsberg/timestream/LocalDateStream.java b/src/main/java/com/ginsberg/timestream/LocalDateStream.java
@@ -144,6 +144,9 @@ public LocalDateStream every(int amount,
         this.amount = Math.abs(amount);
         this.unit = unit;
         LocalDate.now().plus(0, unit); // Fail fast test
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Amount must be non-zero");
+        }
         return this;
     }
 
@@ -156,11 +159,14 @@ public LocalDateStream every(int amount,
      * @throws java.time.temporal.UnsupportedTemporalTypeException if the period is not supported.
      * @see ChronoUnit
      */
-    public LocalDateStream every(Period period) {
+    public LocalDateStream every(final Period period) {
         Objects.requireNonNull(period);
         this.unit = ChronoUnit.DAYS;
-        this.amount = period.get(this.unit);
+        this.amount = Math.abs(period.get(this.unit));
         LocalDate.now().plus(0, unit); // Fail fast test
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Effective amount must be non-zero (Period resolves to zero duration)");
+        }
         return this;
     }
 
diff --git a/src/main/java/com/ginsberg/timestream/LocalDateTimeStream.java b/src/main/java/com/ginsberg/timestream/LocalDateTimeStream.java
@@ -143,6 +143,9 @@ public LocalDateTimeStream every(int amount,
         Objects.requireNonNull(unit);
         this.amount = Math.abs(amount);
         this.unit = unit;
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Amount must be non-zero");
+        }
         return this;
     }
 
@@ -155,10 +158,13 @@ public LocalDateTimeStream every(int amount,
      * @throws java.time.temporal.UnsupportedTemporalTypeException if the unit is not supported.
      * @see ChronoUnit
      */
-    public LocalDateTimeStream every(Duration duration) {
+    public LocalDateTimeStream every(final Duration duration) {
         Objects.requireNonNull(unit);
         this.unit = ChronoUnit.SECONDS;
-        this.amount = duration.get(this.unit);
+        this.amount = Math.abs(duration.get(this.unit));
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Effective amount must be non-zero (Duration resolves to zero duration)");
+        }
         return this;
     }
 
diff --git a/src/main/java/com/ginsberg/timestream/YearMonthStream.java b/src/main/java/com/ginsberg/timestream/YearMonthStream.java
@@ -144,6 +144,9 @@ public YearMonthStream every(int amount,
         this.amount = Math.abs(amount);
         this.unit = unit;
         YearMonth.now().plus(0, unit); // Fail fast test
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Amount must be non-zero");
+        }
         return this;
     }
 
@@ -155,11 +158,14 @@ public YearMonthStream every(int amount,
      * @throws java.time.temporal.UnsupportedTemporalTypeException if the unit is not supported.
      * @see ChronoUnit
      */
-    public YearMonthStream every(Period period) {
+    public YearMonthStream every(final Period period) {
         Objects.requireNonNull(unit);
         this.unit = ChronoUnit.MONTHS;
-        this.amount = period.get(this.unit);
+        this.amount = Math.abs(period.get(this.unit));
         YearMonth.now().plus(0, unit); // Fail fast test
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Effective amount must be non-zero (Period resolves to zero duration)");
+        }
         return this;
     }
 
diff --git a/src/main/java/com/ginsberg/timestream/ZonedDateTimeStream.java b/src/main/java/com/ginsberg/timestream/ZonedDateTimeStream.java
@@ -143,22 +143,27 @@ public ZonedDateTimeStream every(int amount,
         Objects.requireNonNull(unit);
         this.amount = Math.abs(amount);
         this.unit = unit;
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Amount must be non-zero");
+        }
         return this;
     }
 
     /**
      * Set the duration between successive elements produced by the stream. The default
      * for this builder is 1 Second.
      *
-
      * @return A non-null ZonedDateTimeStream.
      * @throws java.time.temporal.UnsupportedTemporalTypeException if the unit is not supported.
      * @see ChronoUnit
      */
-    public ZonedDateTimeStream every(Duration duration) {
+    public ZonedDateTimeStream every(final Duration duration) {
         Objects.requireNonNull(unit);
         this.unit = ChronoUnit.SECONDS;
-        this.amount = duration.get(this.unit);
+        this.amount = Math.abs(duration.get(this.unit));
+        if (this.amount == 0) {
+            throw new IllegalArgumentException("Effective amount must be non-zero (Duration resolves to zero duration)");
+        }
         return this;
     }
 
diff --git a/src/test/java/com/ginsberg/timestream/LocalDateStreamTest.java b/src/test/java/com/ginsberg/timestream/LocalDateStreamTest.java
@@ -100,6 +100,54 @@ public void stopsBeforeToWhenEveryIsAfterEndDate() {
                 .containsExactly(now, now.plusDays(2));
     }
 
+    @Test
+    public void negativeEveryUnitStillGoesForward() {
+        final Stream<LocalDate> stream = LocalDateStream
+                .from(now)
+                .to(3, ChronoUnit.DAYS)
+                .every(-2, ChronoUnit.DAYS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusDays(2));
+    }
+
+    @Test
+    public void negativeEveryPeriodStillGoesForward() {
+        final Stream<LocalDate> stream = LocalDateStream
+                .from(now)
+                .to(3, ChronoUnit.DAYS)
+                .every(Period.parse("-P2D"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusDays(2));
+    }
+
+    @Test
+    public void positiveEveryUnitStillGoesBackward() {
+        final Stream<LocalDate> stream = LocalDateStream
+                .from(now)
+                .to(-3, ChronoUnit.DAYS)
+                .every(2, ChronoUnit.DAYS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusDays(2));
+    }
+
+    @Test
+    public void positiveEveryPeriodStillGoesBackward() {
+        final Stream<LocalDate> stream = LocalDateStream
+                .from(now)
+                .to(-3, ChronoUnit.DAYS)
+                .every(Period.parse("P2D"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusDays(2));
+    }
+
     @Test
     public void identicalFromAndToCreateOnePointStream() {
         final Stream<LocalDate> stream = LocalDateStream
@@ -164,14 +212,29 @@ public void untilByUnitsMustHaveUnit() {
         LocalDateStream.fromNow().until(1, null);
     }
 
+    @Test(expected = NullPointerException.class)
+    public void everyMustHavePeriod() {
+        LocalDateStream.fromNow().every(null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmount() {
+        LocalDateStream.fromNow().every(0, ChronoUnit.DAYS);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmountFromPeriod() {
+        LocalDateStream.fromNow().every(Period.parse("P0D"));
+    }
+
     @Test
     public void everyWithInvalidChronoUnitFailsFast() {
-        expectingChronoUnitException(u -> LocalDateStream.fromNow().every(0, u), validChronoUnits);
+        expectingChronoUnitException(u -> LocalDateStream.fromNow().every(1, u), validChronoUnits);
     }
 
     @Test
     public void everyWithValidChronoUnit() {
-        notExpectingChronoUnitException(u -> LocalDateStream.fromNow().every(0, u), validChronoUnits);
+        notExpectingChronoUnitException(u -> LocalDateStream.fromNow().every(1, u), validChronoUnits);
     }
 
     @Test
diff --git a/src/test/java/com/ginsberg/timestream/LocalDateTimeStreamTest.java b/src/test/java/com/ginsberg/timestream/LocalDateTimeStreamTest.java
@@ -142,6 +142,54 @@ public void stopsBeforeToWhenEveryDurationIsAfterEndDate() {
                 .containsExactly(now, now.plusSeconds(2));
     }
 
+    @Test
+    public void positiveEveryUnitStillGoesBackward() {
+        final Stream<LocalDateTime> stream = LocalDateTimeStream
+                .from(now)
+                .to(-3, ChronoUnit.SECONDS)
+                .every(2, ChronoUnit.SECONDS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusSeconds(2));
+    }
+
+    @Test
+    public void positiveEveryDurationStillGoesBackward() {
+        final Stream<LocalDateTime> stream = LocalDateTimeStream
+                .from(now)
+                .to(-3, ChronoUnit.SECONDS)
+                .every(Duration.parse("PT2S"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusSeconds(2));
+    }
+
+    @Test
+    public void negativeEveryUnitStillGoesForward() {
+        final Stream<LocalDateTime> stream = LocalDateTimeStream
+                .from(now)
+                .to(3, ChronoUnit.SECONDS)
+                .every(-2, ChronoUnit.SECONDS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusSeconds(2));
+    }
+
+    @Test
+    public void negativeEveryDurationStillGoesForward() {
+        final Stream<LocalDateTime> stream = LocalDateTimeStream
+                .from(now)
+                .to(3, ChronoUnit.SECONDS)
+                .every(Duration.parse("-PT2S"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusSeconds(2));
+    }
+
     @Test(expected = NullPointerException.class)
     public void mustHaveFromDate() {
         LocalDateTimeStream.from(null);
@@ -156,4 +204,19 @@ public void toByUnitsMustHaveUnit() {
     public void untilByUnitsMustHaveUnit() {
         LocalDateTimeStream.fromNow().until(1, null);
     }
+
+    @Test(expected = NullPointerException.class)
+    public void everyMustHaveDuration() {
+        LocalDateTimeStream.fromNow().every(null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmount() {
+        LocalDateTimeStream.fromNow().every(0, ChronoUnit.SECONDS);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmountFromPeriod() {
+        LocalDateTimeStream.fromNow().every(Duration.parse("PT0S"));
+    }
 }
diff --git a/src/test/java/com/ginsberg/timestream/YearMonthStreamTest.java b/src/test/java/com/ginsberg/timestream/YearMonthStreamTest.java
@@ -99,6 +99,55 @@ public void stopsBeforeToWhenEveryIsAfterEndDate() {
                 .containsExactly(now, now.plusMonths(2));
     }
 
+    @Test
+    public void negativeEveryUnitStillGoesForward() {
+        final Stream<YearMonth> stream = YearMonthStream
+                .from(now)
+                .to(3, ChronoUnit.MONTHS)
+                .every(-2, ChronoUnit.MONTHS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusMonths(2));
+    }
+
+
+    @Test
+    public void negativeEveryPeriodStillGoesForward() {
+        final Stream<YearMonth> stream = YearMonthStream
+                .from(now)
+                .to(3, ChronoUnit.MONTHS)
+                .every(Period.parse("-P2M"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.plusMonths(2));
+    }
+
+    @Test
+    public void positiveEveryUnitStillGoesBackward() {
+        final Stream<YearMonth> stream = YearMonthStream
+                .from(now)
+                .to(-3, ChronoUnit.MONTHS)
+                .every(2, ChronoUnit.MONTHS)
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusMonths(2));
+    }
+
+    @Test
+    public void positiveEveryPeriodStillGoesBackward() {
+        final Stream<YearMonth> stream = YearMonthStream
+                .from(now)
+                .to(-3, ChronoUnit.MONTHS)
+                .every(Period.parse("P2M"))
+                .stream();
+        assertThat(stream)
+                .isNotNull()
+                .containsExactly(now, now.minusMonths(2));
+    }
+
     @Test
     public void identicalFromAndToCreateOnePointStream() {
         final Stream<YearMonth> stream = YearMonthStream
@@ -163,14 +212,29 @@ public void untilByUnitsMustHaveUnit() {
         YearMonthStream.fromNow().until(1, null);
     }
 
+    @Test(expected = NullPointerException.class)
+    public void everyMustHavePeriod() {
+        YearMonthStream.fromNow().every(null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmount() {
+        YearMonthStream.fromNow().every(0, ChronoUnit.MONTHS);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void everyMustHaveNonZeroAmountFromPeriod() {
+        YearMonthStream.fromNow().every(Period.parse("P1W"));
+    }
+
     @Test
     public void everyWithInvalidChronoUnitFailsFast() {
-        expectingChronoUnitException(u -> YearMonthStream.fromNow().every(0, u), validChronoUnits);
+        expectingChronoUnitException(u -> YearMonthStream.fromNow().every(1, u), validChronoUnits);
     }
 
     @Test
     public void everyWithValidChronoUnit() {
-        notExpectingChronoUnitException(u -> YearMonthStream.fromNow().every(0, u), validChronoUnits);
+        notExpectingChronoUnitException(u -> YearMonthStream.fromNow().every(1, u), validChronoUnits);
     }
 
     @Test
diff --git a/src/test/java/com/ginsberg/timestream/ZonedDateTimeStreamTest.java b/src/test/java/com/ginsberg/timestream/ZonedDateTimeStreamTest.java
