Skip to content

Commit

Permalink
Tolerate floating time mismatch, fixes #850
Browse files Browse the repository at this point in the history 
This commit (temporarily) allows tasks with floating start or due and absolute until part in the RRULE and vice versa.
Apparently such tasks exist and we didn't validate this condition. So for now we tolerate this issue until we can be sure
the database won't contain such tasks.
  • Loading branch information
@dmfs
dmfs committed Sep 22, 2019
1 parent d0fd159 commit 79dd7f3
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 0 deletions.
126 changes: 126 additions & 0 deletions ...sks-provider/src/androidTest/java/org/dmfs/provider/tasks/TaskProviderRecurrenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,69 @@ public void testRRule() throws InvalidRecurrenceRuleException
}


/**
* Test if instances of a task with a timed DTSTART, DUE and a floating RRULE UNTIL.
* <p>
* Note, this combination should not be accepted by the provider. For the time being, however, it should be tolerated instead of causing a crash.
*/
@Test
public void testRRuleWithFloatingMismatch() throws InvalidRecurrenceRuleException
{
RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
Table<Instances> instancesTable = new InstanceTable(mAuthority);
RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));

Duration hour = new Duration(1, 0, 3600 /* 1 hour */);
DateTime start = DateTime.parse("20180104T123456Z");
DateTime due = start.addDuration(hour);
DateTime localStart = start.shiftTimeZone(TimeZone.getDefault());

Duration day = new Duration(1, 1, 0);

DateTime second = localStart.addDuration(day);
DateTime third = second.addDuration(day);
DateTime fourth = third.addDuration(day);
DateTime fifth = fourth.addDuration(day);

DateTime localDue = due.shiftTimeZone(TimeZone.getDefault());

assertThat(new Seq<>(
new Put<>(taskList, new EmptyRowData<>()),
new Put<>(task,
new Composite<>(
new TimeData<>(start, due),
new RRuleTaskData(new RecurrenceRule("FREQ=DAILY;UNTIL=20180106", RecurrenceRule.RfcMode.RFC2445_LAX))))

), resultsIn(mClient,
new Assert<>(task,
new Composite<>(
new TimeData<>(start, due),
new CharSequenceRowData<>(Tasks.RRULE, "FREQ=DAILY;UNTIL=20180106"))),
// new Counted<>(5, new AssertRelated<>(instancesTable, Instances.TASK_ID, task)),
new Counted<>(1, new AssertRelated<>(instancesTable, Instances.TASK_ID, task)),
// 1st instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(localStart, localDue, new Present<>(start), 0),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, start.getTimestamp()))/*,
// 2nd instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(second, second.addDuration(hour), new Present<>(second), 1),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, second.getTimestamp())),
// 3rd instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(third, third.addDuration(hour), new Present<>(third), 2),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, third.getTimestamp())),
// 4th instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(fourth, fourth.addDuration(hour), new Present<>(fourth), 3),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, fourth.getTimestamp())),
// 5th instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(fifth, fifth.addDuration(hour), new Present<>(fifth), 4),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, fifth.getTimestamp())) */)
);
}


/**
* Test if instances of a task with an all-day DTSTART, DUE and an RRULE.
Expand Down Expand Up @@ -263,6 +326,69 @@ public void testAllDayRRule() throws InvalidRecurrenceRuleException
}


/**
* Test if instances of a task with an all-day DTSTART, DUE and an RRULE with an absolute UNTIL
* <p>
* Note, this combination should not be accepted by the provider. For the time being, however, it should be tolerated instead of causing a crash.
*/
@Test
public void testAllDayRRuleFloatingMismatch() throws InvalidRecurrenceRuleException
{
RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
Table<Instances> instancesTable = new InstanceTable(mAuthority);
RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));

Duration days = new Duration(1, 2, 0);
DateTime start = DateTime.parse("20180104");
DateTime due = start.addDuration(days);
DateTime localStart = start;

Duration day = new Duration(1, 1, 0);

DateTime second = localStart.addDuration(day);
DateTime third = second.addDuration(day);
DateTime fourth = third.addDuration(day);
DateTime fifth = fourth.addDuration(day);

DateTime localDue = due;

assertThat(new Seq<>(
new Put<>(taskList, new EmptyRowData<>()),
new Put<>(task,
new Composite<>(
new TimeData<>(start, due),
new RRuleTaskData(new RecurrenceRule("FREQ=DAILY;UNTIL=20180106T120000Z", RecurrenceRule.RfcMode.RFC2445_LAX))))

), resultsIn(mClient,
new Assert<>(task,
new Composite<>(
new TimeData<>(start, due),
new CharSequenceRowData<>(Tasks.RRULE, "FREQ=DAILY;UNTIL=20180106T120000Z"))),
// new Counted<>(5, new AssertRelated<>(instancesTable, Instances.TASK_ID, task)),
new Counted<>(1, new AssertRelated<>(instancesTable, Instances.TASK_ID, task)),
// 1st instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(localStart, localDue, new Present<>(start), 0),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, start.getTimestamp()))/*,
// 2nd instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(second, second.addDuration(hour), new Present<>(second), 1),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, second.getTimestamp())),
// 3rd instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(third, third.addDuration(hour), new Present<>(third), 2),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, third.getTimestamp())),
// 4th instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(fourth, fourth.addDuration(hour), new Present<>(fourth), 3),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, fourth.getTimestamp())),
// 5th instance:
new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(fifth, fifth.addDuration(hour), new Present<>(fifth), 4),
new EqArg(Instances.INSTANCE_ORIGINAL_TIME, fifth.getTimestamp())) */)
);
}


/**
* Test if instances of a task with a DUE and an RRULE but no DTSTART.
Expand Down
16 changes: 16 additions & 0 deletions opentasks-provider/src/main/java/org/dmfs/provider/tasks/utils/TaskInstanceIterable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.dmfs.rfc5545.recurrenceset.RecurrenceSetIterator;

import java.util.Iterator;
import java.util.TimeZone;


/**
Expand Down Expand Up @@ -54,6 +55,21 @@ public Iterator<DateTime> iterator()
RecurrenceRule rule = mTaskAdapter.valueOf(TaskAdapter.RRULE);
if (rule != null)
{
if (rule.getUntil() != null && dtstart.isFloating() != rule.getUntil().isFloating())
{
// rule UNTIL date mismatches start. This is merely a workaround for existing users. In future we should make sure
// such tasks don't exist
if (dtstart.isFloating())
{
// make until floating too by making it floating in the current time zone
rule.setUntil(rule.getUntil().shiftTimeZone(TimeZone.getDefault()).swapTimeZone(null));
}
else
{
// anchor UNTIL in the current time zone
rule.setUntil(new DateTime(null, rule.getUntil().getTimestamp()).swapTimeZone(TimeZone.getDefault()));
}
}
set.addInstances(new RecurrenceRuleAdapter(rule));
}

Expand Down

0 comments on commit 79dd7f3

Please sign in to comment.