From 803edb5a430931cc857740e1c60ba4c48ab5e8cd Mon Sep 17 00:00:00 2001 From: Marten Gajda Date: Mon, 15 Feb 2021 22:48:25 +0100 Subject: [PATCH] Fix iteration of exception that falls on last instance, fixes #93 (#94) This fix ensures an exception that happens to fall on the last instance of a recurrence set is not returned as an instance. --- .../recurrenceset/RecurrenceSetIterator.java | 14 +- .../RecurrenceSetIteratorTest.java | 210 +++++++++++------- 2 files changed, 140 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIterator.java b/src/main/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIterator.java index ce36593..9605394 100644 --- a/src/main/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIterator.java +++ b/src/main/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIterator.java @@ -53,15 +53,15 @@ public class RecurrenceSetIterator * Create a new recurrence iterator for specific lists of instances and exceptions. * * @param instances - * The instances, must not be null or empty. + * The instances, must not be null or empty. * @param exceptions - * The exceptions, may be null. + * The exceptions, may be null. */ RecurrenceSetIterator(List instances, List exceptions) { mInstances = instances.size() == 1 ? instances.get(0) : new CompositeIterator(instances); mExceptions = exceptions == null || exceptions.isEmpty() ? new EmptyIterator() : - exceptions.size() == 1 ? exceptions.get(0) : new CompositeIterator(exceptions); + exceptions.size() == 1 ? exceptions.get(0) : new CompositeIterator(exceptions); pullNext(); } @@ -71,7 +71,7 @@ public class RecurrenceSetIterator * be set before you start iterating, otherwise you may get wrong results. * * @param end - * The date at which to stop the iteration in milliseconds since the epoch. + * The date at which to stop the iteration in milliseconds since the epoch. */ RecurrenceSetIterator setEnd(long end) { @@ -97,7 +97,7 @@ public boolean hasNext() * @return The time in milliseconds since the epoch of the next instance. * * @throws ArrayIndexOutOfBoundsException - * if there are no more instances. + * if there are no more instances. */ public long next() { @@ -115,7 +115,7 @@ public long next() * Fast forward to the next instance at or after the given date. * * @param until - * The date to fast forward to in milliseconds since the epoch. + * The date to fast forward to in milliseconds since the epoch. */ public void fastForward(long until) { @@ -148,6 +148,8 @@ private void pullNext() { throw new RuntimeException(String.format(Locale.ENGLISH, "Skipped too many (%d) instances", MAX_SKIPPED_INSTANCES)); } + // we've skipped the next instance, this might have bene the last one + next = Long.MAX_VALUE; } mNextInstance = next; mNextException = nextException; diff --git a/src/test/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIteratorTest.java b/src/test/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIteratorTest.java index 3bd2278..7e0d89b 100644 --- a/src/test/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIteratorTest.java +++ b/src/test/java/org/dmfs/rfc5545/recurrenceset/RecurrenceSetIteratorTest.java @@ -17,17 +17,20 @@ package org.dmfs.rfc5545.recurrenceset; +import org.dmfs.iterators.AbstractBaseIterator; import org.dmfs.rfc5545.DateTime; import org.dmfs.rfc5545.Duration; import org.dmfs.rfc5545.recur.InvalidRecurrenceRuleException; import org.dmfs.rfc5545.recur.RecurrenceRule; import org.junit.Test; +import java.util.NoSuchElementException; import java.util.TimeZone; import java.util.concurrent.TimeUnit; import static java.util.Arrays.asList; import static org.dmfs.jems.hamcrest.matchers.GeneratableMatcher.startsWith; +import static org.dmfs.jems.hamcrest.matchers.iterator.IteratorMatcher.iteratorOf; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; @@ -48,8 +51,8 @@ public void testExceptionsAllDay() TimeZone testZone = TimeZone.getTimeZone("UTC"); DateTime start = DateTime.parse("20180101"); RecurrenceSetIterator recurrenceSetIterator = new RecurrenceSetIterator( - asList(new RecurrenceList("20180101,20180102,20180103,20180104", testZone).getIterator(testZone, start.getTimestamp())), - asList(new RecurrenceList("20180102,20180103", testZone).getIterator(testZone, start.getTimestamp()))); + asList(new RecurrenceList("20180101,20180102,20180103,20180104", testZone).getIterator(testZone, start.getTimestamp())), + asList(new RecurrenceList("20180102,20180103", testZone).getIterator(testZone, start.getTimestamp()))); // note we call hasNext twice to ensure it's idempotent assertThat(recurrenceSetIterator.hasNext(), is(true)); @@ -72,9 +75,9 @@ public void testMultipleExceptionsAllDay() TimeZone testZone = TimeZone.getTimeZone("UTC"); DateTime start = DateTime.parse("20180101"); RecurrenceSetIterator recurrenceSetIterator = new RecurrenceSetIterator( - asList(new RecurrenceList("20180101,20180102,20180103,20180104", testZone).getIterator(testZone, start.getTimestamp())), - asList(new RecurrenceList("20180103", testZone).getIterator(testZone, start.getTimestamp()), - new RecurrenceList("20180102", testZone).getIterator(testZone, start.getTimestamp()))); + asList(new RecurrenceList("20180101,20180102,20180103,20180104", testZone).getIterator(testZone, start.getTimestamp())), + asList(new RecurrenceList("20180103", testZone).getIterator(testZone, start.getTimestamp()), + new RecurrenceList("20180102", testZone).getIterator(testZone, start.getTimestamp()))); // note we call hasNext twice to ensure it's idempotent assertThat(recurrenceSetIterator.hasNext(), is(true)); @@ -97,9 +100,9 @@ public void testExceptions() TimeZone testZone = TimeZone.getTimeZone("UTC"); DateTime start = DateTime.parse("20180101T120000"); RecurrenceSetIterator recurrenceSetIterator = new RecurrenceSetIterator( - asList(new RecurrenceList("20180101T120000,20180102T120000,20180103T120000,20180104T120000", testZone).getIterator(testZone, - start.getTimestamp())), - asList(new RecurrenceList("20180102T120000,20180103T120000", testZone).getIterator(testZone, start.getTimestamp()))); + asList(new RecurrenceList("20180101T120000,20180102T120000,20180103T120000,20180104T120000", testZone).getIterator(testZone, + start.getTimestamp())), + asList(new RecurrenceList("20180102T120000,20180103T120000", testZone).getIterator(testZone, start.getTimestamp()))); // note we call hasNext twice to ensure it's idempotent assertThat(recurrenceSetIterator.hasNext(), is(true)); @@ -122,10 +125,10 @@ public void testMultipleExceptions() TimeZone testZone = TimeZone.getTimeZone("UTC"); DateTime start = DateTime.parse("20180101T120000"); RecurrenceSetIterator recurrenceSetIterator = new RecurrenceSetIterator( - asList(new RecurrenceList("20180101T120000,20180102T120000,20180103T120000,20180104T120000", testZone).getIterator(testZone, - start.getTimestamp())), - asList(new RecurrenceList("20180103T120000", testZone).getIterator(testZone, start.getTimestamp()), - new RecurrenceList("20180102T120000", testZone).getIterator(testZone, start.getTimestamp()))); + asList(new RecurrenceList("20180101T120000,20180102T120000,20180103T120000,20180104T120000", testZone).getIterator(testZone, + start.getTimestamp())), + asList(new RecurrenceList("20180103T120000", testZone).getIterator(testZone, start.getTimestamp()), + new RecurrenceList("20180102T120000", testZone).getIterator(testZone, start.getTimestamp()))); // note we call hasNext twice to ensure it's idempotent assertThat(recurrenceSetIterator.hasNext(), is(true)); @@ -156,19 +159,19 @@ public void testMultipleRules() throws InvalidRecurrenceRuleException RecurrenceSetIterator it = ruleSet.iterator(start.getTimeZone(), start.getTimestamp()); assertThat(() -> it::next, startsWith( - new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 1, 5, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 1, 10, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 1, 15, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 1, 20, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 1, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 6, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 11, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 16, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 21, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 2, 0, 0).getTimestamp() + new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 1, 5, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 1, 10, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 1, 15, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 1, 20, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 1, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 6, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 11, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 16, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 21, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 2, 0, 0).getTimestamp() )); } @@ -191,24 +194,45 @@ public void testMultipleRulesWithSameValues() throws InvalidRecurrenceRuleExcept RecurrenceSetIterator it = ruleSet.iterator(start.getTimeZone(), start.getTimestamp()); assertThat(() -> it::next, startsWith( - new DateTime(2019, 1, 2).getTimestamp(), // SA - new DateTime(2019, 1, 5).getTimestamp(), // TU - new DateTime(2019, 1, 6).getTimestamp(), // WE - new DateTime(2019, 1, 9).getTimestamp(), // SA - new DateTime(2019, 1, 12).getTimestamp(), // TU - new DateTime(2019, 1, 13).getTimestamp(), // WE - new DateTime(2019, 1, 16).getTimestamp(), // SA - new DateTime(2019, 1, 19).getTimestamp(), // TU - new DateTime(2019, 1, 20).getTimestamp(), // WE - new DateTime(2019, 1, 23).getTimestamp(), // SA - new DateTime(2019, 1, 26).getTimestamp(), // TU - new DateTime(2019, 1, 27).getTimestamp(), // WE - new DateTime(2019, 2, 2).getTimestamp(), // SA - new DateTime(2019, 2, 5).getTimestamp() // TU + new DateTime(2019, 1, 2).getTimestamp(), // SA + new DateTime(2019, 1, 5).getTimestamp(), // TU + new DateTime(2019, 1, 6).getTimestamp(), // WE + new DateTime(2019, 1, 9).getTimestamp(), // SA + new DateTime(2019, 1, 12).getTimestamp(), // TU + new DateTime(2019, 1, 13).getTimestamp(), // WE + new DateTime(2019, 1, 16).getTimestamp(), // SA + new DateTime(2019, 1, 19).getTimestamp(), // TU + new DateTime(2019, 1, 20).getTimestamp(), // WE + new DateTime(2019, 1, 23).getTimestamp(), // SA + new DateTime(2019, 1, 26).getTimestamp(), // TU + new DateTime(2019, 1, 27).getTimestamp(), // WE + new DateTime(2019, 2, 2).getTimestamp(), // SA + new DateTime(2019, 2, 5).getTimestamp() // TU )); } + /** + * See https://github.com/dmfs/lib-recur/issues/93 + */ + @Test + public void testGithubIssue93() throws InvalidRecurrenceRuleException + { + DateTime start = DateTime.parse("20200414T160000Z"); + + // Combine all Recurrence Rules into a RecurrenceSet + RecurrenceSet ruleSet = new RecurrenceSet(); + ruleSet.addInstances(new RecurrenceRuleAdapter(new RecurrenceRule("FREQ=WEEKLY;UNTIL=20200511T000000Z;BYDAY=TU"))); + ruleSet.addExceptions(new RecurrenceList("20200421T160000Z,20200505T160000Z", DateTime.UTC)); + + // Create an iterator using the RecurrenceSet + assertThat(() -> new RecurrenceAdapter(ruleSet.iterator(start.getTimeZone(), start.getTimestamp())), + iteratorOf( + DateTime.parse("20200414T160000Z").getTimestamp(), + DateTime.parse("20200428T160000Z").getTimestamp())); + } + + @Test public void testMultipleRulesWithSameValuesAndCount() throws InvalidRecurrenceRuleException { @@ -227,22 +251,22 @@ public void testMultipleRulesWithSameValuesAndCount() throws InvalidRecurrenceRu RecurrenceSetIterator it = ruleSet.iterator(start.getTimeZone(), start.getTimestamp()); assertThat(() -> it::next, startsWith( - new DateTime(2019, 1, 2).getTimestamp(), // SA - new DateTime(2019, 1, 5).getTimestamp(), // TU - new DateTime(2019, 1, 6).getTimestamp(), // WE - new DateTime(2019, 1, 9).getTimestamp(), // SA - new DateTime(2019, 1, 12).getTimestamp(), // TU - new DateTime(2019, 1, 13).getTimestamp(), // WE - //new DateTime(2019, 1, 16).getTimestamp(), // SA - new DateTime(2019, 1, 19).getTimestamp(), // TU - new DateTime(2019, 1, 20).getTimestamp(), // WE - //new DateTime(2019, 1, 23).getTimestamp(), // SA - new DateTime(2019, 1, 25).getTimestamp(), // MO - new DateTime(2019, 1, 26).getTimestamp(), // TU - new DateTime(2019, 1, 27).getTimestamp(), // WE - //new DateTime(2019, 2, 2).getTimestamp(), // SA - new DateTime(2019, 2, 4).getTimestamp(), // MO - new DateTime(2019, 2, 5).getTimestamp() // TU + new DateTime(2019, 1, 2).getTimestamp(), // SA + new DateTime(2019, 1, 5).getTimestamp(), // TU + new DateTime(2019, 1, 6).getTimestamp(), // WE + new DateTime(2019, 1, 9).getTimestamp(), // SA + new DateTime(2019, 1, 12).getTimestamp(), // TU + new DateTime(2019, 1, 13).getTimestamp(), // WE + //new DateTime(2019, 1, 16).getTimestamp(), // SA + new DateTime(2019, 1, 19).getTimestamp(), // TU + new DateTime(2019, 1, 20).getTimestamp(), // WE + //new DateTime(2019, 1, 23).getTimestamp(), // SA + new DateTime(2019, 1, 25).getTimestamp(), // MO + new DateTime(2019, 1, 26).getTimestamp(), // TU + new DateTime(2019, 1, 27).getTimestamp(), // WE + //new DateTime(2019, 2, 2).getTimestamp(), // SA + new DateTime(2019, 2, 4).getTimestamp(), // MO + new DateTime(2019, 2, 5).getTimestamp() // TU )); } @@ -267,14 +291,14 @@ public void testMultipleRulesWithFastForward() throws InvalidRecurrenceRuleExcep it.fastForward(new DateTime(DateTime.UTC, 2019, 1, 1, 22, 0, 0).getTimestamp()); assertThat(() -> it::next, startsWith( - new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 1, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 6, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 11, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 16, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 21, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 2, 0, 0).getTimestamp() + new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 1, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 6, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 11, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 16, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 21, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 2, 0, 0).getTimestamp() )); } @@ -299,11 +323,11 @@ public void testFastForwardToStart() throws InvalidRecurrenceRuleException it.fastForward(start.getTimestamp()); assertThat(() -> it::next, startsWith( - new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp() + new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp() )); } @@ -323,11 +347,11 @@ public void testFastForwardToPast() throws InvalidRecurrenceRuleException it.fastForward(start.getTimestamp() - TimeUnit.DAYS.toMillis(100)); assertThat(() -> it::next, startsWith( - new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp() + new DateTime(DateTime.UTC, 2019, 1, 1, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp() )); } @@ -347,12 +371,42 @@ public void testFastForwardToNext() throws InvalidRecurrenceRuleException it.fastForward(start.getTimestamp() + 1); assertThat(() -> it::next, startsWith( - new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp(), - new DateTime(DateTime.UTC, 2019, 1, 6, 0, 0, 0).getTimestamp() + new DateTime(DateTime.UTC, 2019, 1, 2, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 3, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 4, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 5, 0, 0, 0).getTimestamp(), + new DateTime(DateTime.UTC, 2019, 1, 6, 0, 0, 0).getTimestamp() )); } + + private final static class RecurrenceAdapter extends AbstractBaseIterator + { + + private final RecurrenceSetIterator mDelegate; + + + private RecurrenceAdapter(RecurrenceSetIterator delegate) + { + mDelegate = delegate; + } + + + @Override + public boolean hasNext() + { + return mDelegate.hasNext(); + } + + + @Override + public Long next() + { + if (!hasNext()) + { + throw new NoSuchElementException(); + } + return mDelegate.next(); + } + } } \ No newline at end of file