Respect existing overrides, implements #952 (#974) (b3896516) · Commits · e / os / Tasks

opentasks-provider/src/androidTest/java/org/dmfs/provider/tasks/TaskProviderRecurrenceTest.java

+128 −0

Original line number	Diff line number	Diff line
		@@ -648,6 +648,134 @@ public class TaskProviderRecurrenceTest
		}


		/**
		* 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)

opentasks-provider/src/main/java/org/dmfs/provider/tasks/model/adapters/DateTimeFieldAdapter.java

+3 −6

Original line number	Diff line number	Diff line
		@@ -89,9 +89,8 @@ public final class DateTimeFieldAdapter<EntityType> extends SimpleFieldAdapter<D
		// if the time stamp is null we return null
		return null;
		}
		// create a new Time for the given time zone, falling back to UTC if none is given
		String timezone = mTzField == null ? null : values.getAsString(mTzField);
		DateTime value = new DateTime(timezone == null ? DateTime.UTC : TimeZone.getTimeZone(timezone), timestamp);
		DateTime value = new DateTime(timezone == null ? null : TimeZone.getTimeZone(timezone), timestamp);

		// cache mAlldayField locally
		String allDayField = mAllDayField;
		@@ -128,9 +127,8 @@ public final class DateTimeFieldAdapter<EntityType> extends SimpleFieldAdapter<D

		Long timestamp = cursor.getLong(tsIdx);

		// create a new Time for the given time zone, falling back to UTC if none is given
		String timezone = mTzField == null ? null : cursor.getString(tzIdx);
		DateTime value = new DateTime(timezone == null ? DateTime.UTC : TimeZone.getTimeZone(timezone), timestamp);
		DateTime value = new DateTime(timezone == null ? null : TimeZone.getTimeZone(timezone), timestamp);

		// set the allday flag appropriately
		Integer allDayInt = adIdx < 0 ? null : cursor.getInt(adIdx);
		@@ -208,8 +206,7 @@ public final class DateTimeFieldAdapter<EntityType> extends SimpleFieldAdapter<D
		}
		}

		// create a new Time for the given time zone, falling back to UTC if none is given
		DateTime value = new DateTime(timeZoneId == null ? DateTime.UTC : TimeZone.getTimeZone(timeZoneId), timestamp);
		DateTime value = new DateTime(timeZoneId == null ? null : TimeZone.getTimeZone(timeZoneId), timestamp);

		if (allDay != 0)
		{

opentasks-provider/src/main/java/org/dmfs/provider/tasks/processors/instances/Detaching.java

+2 −1

Original line number	Diff line number	Diff line
		@@ -26,6 +26,7 @@ import org.dmfs.jems.iterable.composite.Joined;
		import org.dmfs.jems.optional.adapters.FirstPresent;
		import org.dmfs.jems.optional.elementary.NullSafe;
		import org.dmfs.jems.predicate.composite.AnyOf;
		import org.dmfs.jems.predicate.composite.Not;
		import org.dmfs.provider.tasks.TaskDatabaseHelper;
		import org.dmfs.provider.tasks.model.CursorContentValuesInstanceAdapter;
		import org.dmfs.provider.tasks.model.CursorContentValuesTaskAdapter;
		@@ -195,7 +196,7 @@ public final class Detaching implements EntityProcessor<InstanceAdapter>
		{
		// remove the RRULE but keep a mask for the old start
		masterTask.set(TaskAdapter.EXDATE,
		new Joined<>(new SingletonIterable<>(oldStart), new Sieved<>(oldStart::equals, masterTask.valueOf(TaskAdapter.EXDATE))));
		new Joined<>(new SingletonIterable<>(oldStart), new Sieved<>(new Not<>(oldStart::equals), masterTask.valueOf(TaskAdapter.EXDATE))));
		masterTask.set(TaskAdapter.RRULE, null);
		}
		else

opentasks-provider/src/main/java/org/dmfs/provider/tasks/processors/tasks/Instantiating.java

+75 −21

Original line number	Diff line number	Diff line
		@@ -20,6 +20,7 @@ import android.content.ContentValues;
		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;
		@@ -36,7 +37,9 @@ import org.dmfs.provider.tasks.processors.EntityProcessor;
		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;
		@@ -49,6 +52,19 @@ import java.util.Locale;
		*/
		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
		@@ -154,7 +170,7 @@ public final class Instantiating implements EntityProcessor<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)
		{
		@@ -201,8 +217,6 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>

		/**
		* Updates the instances of an existing task
		* <p>
		* TODO: take instance overrides into account
		*
		* @param db
		* An {@link SQLiteDatabase}.
		@@ -218,11 +232,19 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		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),
		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);
		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.Instances.INSTANCE_ORIGINAL_TIME))
		TaskContract.Tasks.ORIGINAL_INSTANCE_TIME);)
		{

		/*
		@@ -240,14 +262,43 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		// 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;
		@@ -256,10 +307,17 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		{
		if (distance >= UPCOMING_INSTANCE_COUNT_LIMIT - 1)
		{
		// if we already expanded enough instances, we pretend no other instance exists
		// we already expanded enough instances
		if (!next.right().isPresent())
		{
		// actually no instance exists, no need to do anything
		// if no further instances exist, stop here
		Long original =next.left().value().getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);
		if (original != null && existingInstances.moveToLast() && existingInstances.getLong(startIdx)<original)
		{
		break;
		}

		// we may have to delete a few future instances
		continue;
		}
		next = new RightSidedPair<>(next.right());
		@@ -276,8 +334,7 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		{
		// 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;
		}
		@@ -289,12 +346,10 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		// 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;
		@@ -305,7 +360,7 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		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;
		@@ -316,10 +371,9 @@ public final class Instantiating implements EntityProcessor<TaskAdapter>
		db.update(TaskDatabaseHelper.Tables.INSTANCES, contentValues,
		String.format(Locale.ENGLISH, "%s = %d", TaskContract.Instances._ID, existingInstances.getLong(idIdx)), null);
		}
		}*/
		}
		}
		}
		}
		}

		}

opentasks-provider/src/main/java/org/dmfs/provider/tasks/processors/tasks/instancedata/Overridden.java

+10 −14

Original line number	Diff line number	Diff line
		@@ -18,21 +18,18 @@ package org.dmfs.provider.tasks.processors.tasks.instancedata;

		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.optional.elementary.Present;
		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
		*/
		@@ -42,6 +39,12 @@ public final class Overridden implements Single<ContentValues>
		private final Single<ContentValues> mDelegate;


		public Overridden(DateTime originalTime, ContentValues delegate)
		{
		this(new Present<>(originalTime), () -> delegate);
		}


		public Overridden(Optional<DateTime> originalTime, Single<ContentValues> delegate)
		{
		mOriginalTime = originalTime;
		@@ -53,14 +56,7 @@ public final class Overridden implements Single<ContentValues>
		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;
		}
		}