Skip to content

Commit

Permalink
Respect existing overrides, implements #952
Browse files Browse the repository at this point in the history
  • Loading branch information
@dmfs
dmfs committed Feb 6, 2021
1 parent c2cf2f2 commit d834fc4
Show file tree
Hide file tree
Showing 9 changed files with 345 additions and 47 deletions.
128 changes: 128 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 @@ -648,6 +648,134 @@ public void testRRuleWithOverride() throws InvalidRecurrenceRuleException
}


/**
* Test RRULE with overridden instance (inserted into the tasks table) and a completed 1st instance.
*/
@Test
public void testRRuleWith2ndOverrideAndCompleted1st() 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)));
RowSnapshot<Tasks> taskOverride = 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);

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

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

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

assertThat(new Seq<>(
new Put<>(taskList, new EmptyRowData<>()),
new Put<>(task,
new Composite<>(
new TimeData<>(start, due),
new TitleData("original"),
new RRuleTaskData(new RecurrenceRule("FREQ=DAILY;COUNT=5", RecurrenceRule.RfcMode.RFC2445_LAX)))),
// the override moves the instance by an hour
new Put<>(taskOverride, new Composite<>(
new TimeData<>(second.addDuration(hour), second.addDuration(hour).addDuration(hour)),
new TitleData("override"),
new OriginalInstanceData(task, second))),
new Put<>(task, new StatusData<>(Tasks.STATUS_COMPLETED))),
resultsIn(mClient,
new Assert<>(task,
new Composite<>(
new TimeData<>(start, due),
new CharSequenceRowData<>(Tasks.TITLE, "original"),
new CharSequenceRowData<>(Tasks.RRULE, "FREQ=DAILY;COUNT=5"),
new StatusData<>(Tasks.STATUS_COMPLETED))),
new Assert<>(taskOverride,
new Composite<>(
new TimeData<>(second.addDuration(hour), second.addDuration(hour).addDuration(hour)),
new CharSequenceRowData<>(Tasks.TITLE, "override"),
new OriginalInstanceData(task, second))),
// 1st (completed) instance:
new Counted<>(1, new AssertRelated<>(instancesTable, Instances.TASK_ID, task,
new InstanceTestData(localStart, localDue, new Present<>(start), -1),
new EqArg<>(Instances.INSTANCE_ORIGINAL_TIME, start.getTimestamp()))),
// 2nd instance (now the current one):
new Counted<>(1, new AssertRelated<>(instancesTable, Instances.TASK_ID, taskOverride,
new InstanceTestData(
second.addDuration(hour),
second.addDuration(hour).addDuration(hour),
new Present<>(second), 0)))));
}


/**
* Test RRULE with overridden instance (inserted into the tasks table) and a deleted 1st instance.
*/
@Test
public void testRRuleWith2ndOverrideAndDeleted1st() 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)));
RowSnapshot<Tasks> taskOverride = 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);

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

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

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

assertThat(new Seq<>(
new Put<>(taskList, new EmptyRowData<>()),
new Put<>(task,
new Composite<>(
new TimeData<>(start, due),
new TitleData("original"),
new RRuleTaskData(new RecurrenceRule("FREQ=DAILY;COUNT=5", RecurrenceRule.RfcMode.RFC2445_LAX)))),
// the override moves the instance by an hour
new Put<>(taskOverride, new Composite<>(
new TimeData<>(second.addDuration(hour), second.addDuration(hour).addDuration(hour)),
new TitleData("override"),
new OriginalInstanceData(task, second))),
// delete 1st instance
new BulkDelete<>(instancesTable, new AllOf<>(
new ReferringTo<>(Instances.TASK_ID, task),
new EqArg<>(Instances.DISTANCE_FROM_CURRENT, "0")))),
resultsIn(mClient,
new Assert<>(task,
new Composite<>(
new TimeData<>(start, due),
new CharSequenceRowData<>(Tasks.TITLE, "original"),
new CharSequenceRowData<>(Tasks.RRULE, "FREQ=DAILY;COUNT=5"),
new CharSequenceRowData<>(Tasks.EXDATE, start.toString()),
new StatusData<>(Tasks.STATUS_DEFAULT))),
new Assert<>(taskOverride,
new Composite<>(
new TimeData<>(second.addDuration(hour), second.addDuration(hour).addDuration(hour)),
new CharSequenceRowData<>(Tasks.TITLE, "override"),
new OriginalInstanceData(task, second))),
// no instances point to the original task
new Counted<>(0, new AssertRelated<>(instancesTable, Instances.TASK_ID, task)),
// 2nd instance (now the current one):
new Counted<>(1, new AssertRelated<>(instancesTable, Instances.TASK_ID, taskOverride,
new InstanceTestData(
second.addDuration(hour),
second.addDuration(hour).addDuration(hour),
new Present<>(second), 0)))));
}


/**
* Test RRULE with overridden instance (via update on the instances table). This time we don't override the date time fields and expect the instance to
* inherit the original instance start and due (instead of the master start and due)
Expand Down
83 changes: 66 additions & 17 deletions opentasks-provider/src/main/java/org/dmfs/provider/tasks/processors/tasks/Instantiating.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;

import org.dmfs.jems.function.elementary.DiffMap;
import org.dmfs.jems.iterable.composite.Diff;
import org.dmfs.jems.iterable.decorators.Mapped;
import org.dmfs.jems.optional.Optional;
Expand All @@ -36,7 +37,9 @@
import org.dmfs.provider.tasks.processors.tasks.instancedata.TaskRelated;
import org.dmfs.provider.tasks.utils.InstanceValuesIterable;
import org.dmfs.provider.tasks.utils.Limited;
import org.dmfs.provider.tasks.utils.OverrideValuesFunction;
import org.dmfs.provider.tasks.utils.Range;
import org.dmfs.provider.tasks.utils.RowIterator;
import org.dmfs.tasks.contract.TaskContract;

import java.util.Locale;
Expand All @@ -49,6 +52,19 @@
*/
public final class Instantiating implements EntityProcessor<TaskAdapter>
{
/**
* Projection we use to read the overrides of a task
*/
private final static String[] OVERRIDE_PROJECTION = {
TaskContract.Tasks._ID,
TaskContract.Tasks.DTSTART,
TaskContract.Tasks.DUE,
TaskContract.Tasks.DURATION,
TaskContract.Tasks.TZ,
TaskContract.Tasks.IS_ALLDAY,
TaskContract.Tasks.IS_CLOSED,
TaskContract.Tasks.ORIGINAL_INSTANCE_TIME,
TaskContract.Tasks.ORIGINAL_INSTANCE_ALLDAY };

/**
* This is a field adapter for a pseudo column to indicate that the instances may need an update, even if no relevant value has changed. This is useful to
Expand Down Expand Up @@ -154,7 +170,7 @@ private void updateOverrideInstance(SQLiteDatabase db, TaskAdapter taskAdapter,
{
long origId = taskAdapter.valueOf(TaskAdapter.ORIGINAL_INSTANCE_ID);
int count = 0;
for (Single<ContentValues> values : new InstanceValuesIterable(taskAdapter))
for (Single<ContentValues> values : new InstanceValuesIterable(id, taskAdapter))
{
if (count++ > 1)
{
Expand Down Expand Up @@ -218,11 +234,19 @@ private void updateMasterInstances(SQLiteDatabase db, TaskAdapter taskAdapter, l
new String[] {
TaskContract.Instances._ID, TaskContract.Instances.INSTANCE_ORIGINAL_TIME, TaskContract.Instances.TASK_ID,
TaskContract.Instances.IS_CLOSED, TaskContract.Instances.DISTANCE_FROM_CURRENT },
String.format(Locale.ENGLISH, "%s = %d or %s = %d", TaskContract.Instances.TASK_ID, id, TaskContract.Instances.ORIGINAL_INSTANCE_ID, id),
null,
String.format(Locale.ENGLISH, "%s = ? or %s = ?", TaskContract.Instances.TASK_ID, TaskContract.Instances.ORIGINAL_INSTANCE_ID),
new String[] { Long.toString(id), Long.toString(id) },
null,
null,
TaskContract.Instances.INSTANCE_ORIGINAL_TIME))
TaskContract.Instances.INSTANCE_ORIGINAL_TIME);
Cursor overrides = db.query(
TaskDatabaseHelper.Tables.TASKS,
OVERRIDE_PROJECTION,
String.format("%s = ? AND %s != 1", TaskContract.Tasks.ORIGINAL_INSTANCE_ID, TaskContract.Tasks._DELETED),
new String[] { Long.toString(id) },
null,
null,
TaskContract.Tasks.ORIGINAL_INSTANCE_TIME);)
{

/*
Expand All @@ -240,14 +264,43 @@ private void updateMasterInstances(SQLiteDatabase db, TaskAdapter taskAdapter, l
// for very long or even infinite series we need to stop iterating at some point.

Iterable<Pair<Optional<ContentValues>, Optional<Integer>>> diff = new Diff<>(
new Mapped<>(Single::value,
new Limited<>(10000 /* hard limit for infinite rules*/, new InstanceValuesIterable(taskAdapter))),
new Mapped<>(Single::value, new Limited<>(10000 /* hard limit for infinite rules*/,
new Mapped<>(
new DiffMap<>(
(original, override) -> override, // we have both, a regular instance and an override -> take the override
original -> original,
override -> override // we only have an override :-o, not really valid but tolerated
),
new Diff<>(
new InstanceValuesIterable(id, taskAdapter),
new Mapped<>(
cursor ->
new OverrideValuesFunction()
.value(new CursorContentValuesTaskAdapter(cursor, new ContentValues())),
() -> new RowIterator(overrides)),
(left, right) -> {
Long leftLong = left.value().getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);
Long rightLong = right.value().getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);
// null is always smaller
if (leftLong == null)
{
return rightLong == null ? 0 : -1;
}
if (rightLong == null)
{
return 1;
}

long ldiff = leftLong - rightLong;
return ldiff < 0 ? -1 : (ldiff > 0 ? 1 : 0);
})))),
new Range(existingInstances.getCount()),
(newInstanceValues, cursorRow) ->
{
existingInstances.moveToPosition(cursorRow);
return (int) (new Backed<>(new NullSafe<>(newInstanceValues.getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME)), 0L).value()
- existingInstances.getLong(startIdx));
long ldiff = new Backed<>(new NullSafe<>(newInstanceValues.getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME)), 0L).value()
- existingInstances.getLong(startIdx);
return ldiff < 0 ? -1 : (ldiff > 0 ? 1 : 0);
});

int distance = -1;
Expand Down Expand Up @@ -276,8 +329,7 @@ else if (!next.right().isPresent())
{
// there is no old instance for this new one, add it
ContentValues values = next.left().value();
values.put(TaskContract.Instances.TASK_ID, taskAdapter.id());
if (distance >= 0 || !taskAdapter.valueOf(TaskAdapter.IS_CLOSED))
if (distance >= 0 || values.getAsLong(TaskContract.Instances.DISTANCE_FROM_CURRENT) >= 0)
{
distance += 1;
}
Expand All @@ -289,12 +341,10 @@ else if (!next.right().isPresent())
// update this instance
existingInstances.moveToPosition(next.right().value());
// only update if the instance belongs to this task
if (existingInstances.getLong(taskIdIdx) == id)
// if (existingInstances.getLong(taskIdIdx) == id)
{
ContentValues values = next.left().value();
if (distance >= 0 ||
taskAdapter.isUpdated(TaskAdapter.IS_CLOSED) && !taskAdapter.valueOf(TaskAdapter.IS_CLOSED) ||
!taskAdapter.isUpdated(TaskAdapter.IS_CLOSED) && existingInstances.getInt(isClosedIdx) == 0)
if (distance >= 0 ||values.getAsLong(TaskContract.Instances.DISTANCE_FROM_CURRENT) >= 0)
{
// the distance needs to be updated
distance += 1;
Expand All @@ -305,7 +355,7 @@ else if (!next.right().isPresent())
db.update(TaskDatabaseHelper.Tables.INSTANCES, values,
String.format(Locale.ENGLISH, "%s = %d", TaskContract.Instances._ID, existingInstances.getLong(idIdx)), null);
}
else if (distance >= 0 || existingInstances.getInt(isClosedIdx) == 0)
/* else if (distance >= 0 || existingInstances.getInt(isClosedIdx) == 0)
{
// this is an override and we need to check the distance value
distance += 1;
Expand All @@ -316,10 +366,9 @@ else if (distance >= 0 || existingInstances.getInt(isClosedIdx) == 0)
db.update(TaskDatabaseHelper.Tables.INSTANCES, contentValues,
String.format(Locale.ENGLISH, "%s = %d", TaskContract.Instances._ID, existingInstances.getLong(idIdx)), null);
}
}
}*/
}
}
}
}

}
17 changes: 3 additions & 14 deletions ...vider/src/main/java/org/dmfs/provider/tasks/processors/tasks/instancedata/Overridden.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,17 @@

import android.content.ContentValues;

import org.dmfs.iterables.elementary.Seq;
import org.dmfs.jems.optional.Optional;
import org.dmfs.jems.optional.adapters.FirstPresent;
import org.dmfs.jems.optional.decorators.Mapped;
import org.dmfs.jems.optional.elementary.NullSafe;
import org.dmfs.jems.procedure.composite.ForEach;
import org.dmfs.jems.single.Single;
import org.dmfs.jems.single.combined.Backed;
import org.dmfs.rfc5545.DateTime;
import org.dmfs.tasks.contract.TaskContract;


/**
* A decorator for {@link Single}s of Instance {@link ContentValues} which populates the {@link TaskContract.Instances#INSTANCE_ORIGINAL_TIME} field based on
* the given {@link Optional} original start and the already populated {@link TaskContract.Instances#INSTANCE_START} and {@link
* TaskContract.Instances#INSTANCE_DUE} fields.
* the given {@link Optional} original start.
*
* @author Marten Gajda
*/
Expand All @@ -53,14 +49,7 @@ public Overridden(Optional<DateTime> originalTime, Single<ContentValues> delegat
public ContentValues value()
{
ContentValues values = mDelegate.value();
values.put(TaskContract.Instances.INSTANCE_ORIGINAL_TIME,
new Backed<Long>(
new FirstPresent<>(
new Seq<>(
new Mapped<>(DateTime::getTimestamp, mOriginalTime),
new NullSafe<>(values.getAsLong(TaskContract.Instances.INSTANCE_START)),
new NullSafe<>(values.getAsLong(TaskContract.Instances.INSTANCE_DUE)))),
() -> null).value());
new ForEach<>(new Mapped<>(DateTime::getTimestamp, mOriginalTime)).process(time -> values.put(TaskContract.Instances.INSTANCE_ORIGINAL_TIME, time));
return values;
}
}
7 changes: 7 additions & 0 deletions ...ider/src/main/java/org/dmfs/provider/tasks/processors/tasks/instancedata/TaskRelated.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import android.content.ContentValues;

import org.dmfs.jems.single.Single;
import org.dmfs.provider.tasks.model.TaskAdapter;
import org.dmfs.tasks.contract.TaskContract;


Expand All @@ -33,6 +34,12 @@ public final class TaskRelated implements Single<ContentValues>
private final Single<ContentValues> mDelegate;


public TaskRelated(TaskAdapter taskAdapter, Single<ContentValues> delegate)
{
this(taskAdapter.id(), delegate);
}


public TaskRelated(long taskId, Single<ContentValues> delegate)
{
mTaskId = taskId;
Expand Down
Loading

0 comments on commit d834fc4

Please sign in to comment.