Let instance view return instance DTSTART and DUE rather than the tas…

opentasks-contract/src/main/java/org/dmfs/tasks/contract/TaskContract.java

@@ -1036,8 +1036,7 @@ public interface InstanceColumns
         String INSTANCE_DURATION = "instance_duration";
 
         /**
-         * The start of the original instance as specified in the master task. For non-recurring task instances this equals the value of {@link
-         * #INSTANCE_START}, except that `null` values are represented as `0`.
+         * The start of the original instance as specified in the master task. For non-recurring task instances this is {@code null}.
          * <p>
          * For recurring tasks, these are the timestamps which have been derived from the recurrence rule or dates, except those specified as exdates.
          */
@@ -1056,16 +1055,61 @@ public interface InstanceColumns
 
 
     /**
-     * Instances of a task. At present this table is read only. Currently it contains exactly one entry per task (and task exception), so it's merely a copy of
-     * {@link Tasks}.
+     * A table containing one entry per task instance. This table is writable in order to allow modification of single instances of a task. Write operations to
+     * this table will be converted into operations on overrides and forwarded to the task table.
      * <p>
-     * TODO: Insert all instances of recurring the tasks.
-     * </p>
+     * Note: The {@link #DTSTART}, {@link #DUE} values of instances of recurring tasks represent the actual instance values, i.e. they are different for each
+     * instance ({@link #DURATION} is always {@code null}).
      * <p>
-     * TODO: In later releases it's planned to provide a convenient interface to add, change or delete task instances via this URI.
-     * </p>
+     * Also, none of the instances are recurring themselves, so {@link #RRULE}, {@link #RDATE} and {@link #EXDATE} are always {@code null}.
+     * <p>
+     * TODO: Insert all instances of recurring tasks.
+     * <p>
+     * The following operations are supported:
+     * <p>
+     * <h2>Insert</h2>
+     * <p>
+     * Note, the data of an insert must not contain the fields {@link #RRULE}, {@link #RDATE} or {@link #EXDATE}. If the new instance belongs to an existing
+     * task the data must contain the fields {@link #ORIGINAL_INSTANCE_ID} and {@link #ORIGINAL_INSTANCE_TIME}. Also note, this table supports writing {@link
+     * #DURATION} (if the instance has a {@link #DTSTART}), but reading it back will always return a {@code null} {@link #DURATION} and a non-{@code null}
+     * {@link #DUE} date. Reading the task in the tasks table will, however, return the original {@link #DURATION}.
+     * <p>
+     * If there already is an instance (with or without override) for the given {@link #ORIGINAL_INSTANCE_ID} and {@link #ORIGINAL_INSTANCE_TIME} an exception
+     * is thrown.
+     * <p>
+     * <table> <tr><th>ORIGINAL_INSTANCE_ID value</th><th>Result</th></tr> <tr><td>absent or empty</td><td>A new non-recurring task is created with the given
+     * values.</td></tr> <tr><td>a valid {@link Tasks} row {@code _ID}</td><td>An {@link #RDATE} for the given {@link #ORIGINAL_INSTANCE_TIME} time is added to
+     * the given master task, any {@link #EXDATE} for this time is removed. The task is inserted as an override to the given master. No fields are inherited
+     * though. {@link #ORIGINAL_INSTANCE_ALLDAY} will be set to {@link #IS_ALLDAY} of the master.
+     * <p>
+     * Note, if the given master is non-recurring, this operation will turn it into a recurring task. </td></tr> <tr><td>invalid {@link Tasks} row {@code
+     * _ID}</td><td>An exception is thrown.</td></tr></table>
+     * <p>
+     * <h2>Update</h2>
+     * <p>
+     * Note, the data of an update must not contain any fields related to recurrence ({@link #RRULE}, {@link #RDATE}, {@link #EXDATE}, {@link
+     * #ORIGINAL_INSTANCE_ID}, {@link #ORIGINAL_INSTANCE_TIME} and {@link #ORIGINAL_INSTANCE_ALLDAY}). Also note, this table supports writing {@link #DURATION}
+     * (if the instance has a {@link #DTSTART}), but reading it back will always return a {@code null} {@link #DURATION} and a non-{@code null} {@link #DUE}
+     * date. Reading the task in the tasks table will, however, return the original {@link #DURATION}.
+     * <p>
+     * <table> <tr><th>Target task type</th><th>Result</th></tr> <tr><td>Recurring master task</td><td>A new override is created with the given data.<p> Note,
+     * any fields which are not provided are inherited from the master, except for {@link #DTSTART} and {@link #DUE} which will be inherited from the instance
+     * and {@link #DURATION}, {@link #RRULE}, {@link #RDATE} and {@link #EXDATE} which are set to {@code null}. {@link #ORIGINAL_INSTANCE_ID}, {@link
+     * #ORIGINAL_INSTANCE_TIME} and {@link #ORIGINAL_INSTANCE_ALLDAY} will be set accordingly.</td></tr> <tr><td>Single instance task</td><td>The task is
+     * updated with the given values.</td></tr> <tr><td>Recurrence override with existing master</td><td>The task is updated with the given values.</td></tr>
+     * <tr><td>Recurrence override without existing master</td><td>The task is updated with the given values.</td></tr> </table>
+     * <p>
+     * <h2>Delete</h2>
+     * <p>
+     * <table> <tr><th>Target task type</th><th>Result</th></tr> <tr><td>Recurring master task</td><td>An {@link #EXDATE} for this instance is added, any {@link
+     * #RDATE} for this instance is removed. The instance row is removed.<p> TODO: mark the task deleted if the remaining recurrence set is empty </td></tr>
+     * <tr><td>Single instance task</td><td>The {@link Tasks#_DELETED} flag of the task is set.</td></tr> <tr><td>Recurrence override with existing
+     * master</td><td>The {@link Tasks#_DELETED} flag of the override is set, an {@link #EXDATE} for this instance is added to the master, any {@link #RDATE}
+     * for this instance is removed from the master. TODO: mark the master deleted if the remaining recurrence set of the master is empty </td></tr>
+     * <tr><td>Recurrence override without existing master</td><td>The {@link Tasks#_DELETED} flag of the task is set.</td></tr> </table>
      *
-     * @author Yannic Ahrens <yannic@dmfs.org>
+     * @author Yannic Ahrens
+     * @author Marten Gajda
      */
     public static final class Instances implements TaskColumns, InstanceColumns
     {

opentasks-provider/src/main/java/org/dmfs/provider/tasks/TaskDatabaseHelper.java

@@ -60,7 +60,7 @@ public interface OnDatabaseOperationListener
     /**
      * The database version.
      */
-    private static final int DATABASE_VERSION = 18;
+    private static final int DATABASE_VERSION = 19;
 
 
     /**
@@ -82,6 +82,8 @@ public interface Tables
 
         public static final String INSTANCE_VIEW = "Instance_View";
 
+        public static final String INSTANCE_CLIENT_VIEW = "Instance_Client_View";
+
         public static final String INSTANCE_PROPERTY_VIEW = "Instance_Property_View";
 
         public static final String INSTANCE_CATEGORY_VIEW = "Instance_Cagetory_View";
@@ -166,6 +168,36 @@ public interface CategoriesMapping
             + " JOIN " + Tables.LISTS + " ON (" + Tables.TASKS + "." + TaskContract.Tasks.LIST_ID + "=" + Tables.LISTS + "." + TaskContract.Tasks._ID + ")"
             + " JOIN " + Tables.INSTANCES + " ON (" + Tables.TASKS + "." + TaskContract.Tasks._ID + "=" + Tables.INSTANCES + "." + TaskContract.Instances.TASK_ID + ");";
 
+    /**
+     * SQL command to create a view that combines task instances with some data from the list they belong to. This replaces the task DTSTART, DUE and
+     * ORIGINAL_INSTANCE_TIME values with respective values of the instance.
+     * <p>
+     * This is the instances view as seen by the content provider clients.
+     */
+    private final static String SQL_CREATE_INSTANCE_CLIENT_VIEW = "CREATE VIEW " + Tables.INSTANCE_CLIENT_VIEW + " AS SELECT "
+            + Tables.INSTANCES + ".*, "
+            // override task due, start and original times with the instance values
+            + Tables.INSTANCES + "." + TaskContract.Instances.INSTANCE_START + " as " + Tasks.DTSTART + ", "
+            + Tables.INSTANCES + "." + TaskContract.Instances.INSTANCE_DUE + " as " + Tasks.DUE + ", "
+            + Tables.INSTANCES + "." + TaskContract.Instances.INSTANCE_ORIGINAL_TIME + " as " + Tasks.ORIGINAL_INSTANCE_TIME + ", "
+            // override task duration with null, we already have a due
+            + "null as " + Tasks.DURATION + ", "
+            // override recurrence values with null, instances themselves are not recurring
+            + "null as " + Tasks.RRULE + ", "
+            + "null as " + Tasks.RDATE + ", "
+            + "null as " + Tasks.EXDATE + ", "
+            + Tables.TASKS + ".*, "
+            + Tables.LISTS + "." + Tasks.ACCOUNT_NAME + ", "
+            + Tables.LISTS + "." + Tasks.ACCOUNT_TYPE + ", "
+            + Tables.LISTS + "." + Tasks.LIST_OWNER + ", "
+            + Tables.LISTS + "." + Tasks.LIST_NAME + ", "
+            + Tables.LISTS + "." + Tasks.LIST_ACCESS_LEVEL + ", "
+            + Tables.LISTS + "." + Tasks.LIST_COLOR + ", "
+            + Tables.LISTS + "." + Tasks.VISIBLE
+            + " FROM " + Tables.TASKS
+            + " JOIN " + Tables.LISTS + " ON (" + Tables.TASKS + "." + TaskContract.Tasks.LIST_ID + "=" + Tables.LISTS + "." + TaskContract.TaskLists._ID + ")"
+            + " JOIN " + Tables.INSTANCES + " ON (" + Tables.TASKS + "." + TaskContract.Tasks._ID + "=" + Tables.INSTANCES + "." + TaskContract.Instances.TASK_ID + ");";
+
     /**
      * SQL command to create a view that combines task instances view with the belonging properties.
      */
@@ -586,6 +618,7 @@ public void onCreate(SQLiteDatabase db)
         db.execSQL(SQL_CREATE_TASK_VIEW);
         db.execSQL(SQL_CREATE_TASK_PROPERTY_VIEW);
         db.execSQL(SQL_CREATE_INSTANCE_VIEW);
+        db.execSQL(SQL_CREATE_INSTANCE_CLIENT_VIEW);
         db.execSQL(SQL_CREATE_INSTANCE_PROPERTY_VIEW);
         db.execSQL(SQL_CREATE_INSTANCE_CATEGORY_VIEW);
 
@@ -773,6 +806,11 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion)
             db.execSQL("alter table " + Tables.INSTANCES + " add column " + TaskContract.Instances.DISTANCE_FROM_CURRENT + " integer default 0;");
         }
 
+        if (oldVersion < 19)
+        {
+            db.execSQL(SQL_CREATE_INSTANCE_CLIENT_VIEW);
+        }
+
         // upgrade FTS
         FTSDatabaseHelper.onUpgrade(db, oldVersion, newVersion);
 

opentasks-provider/src/main/java/org/dmfs/provider/tasks/TaskProvider.java

@@ -602,7 +602,7 @@ public Cursor query(Uri uri, String[] projection, String selection, String[] sel
                 }
                 else
                 {
-                    sqlBuilder.setTables(Tables.INSTANCE_VIEW);
+                    sqlBuilder.setTables(Tables.INSTANCE_CLIENT_VIEW);
                 }
                 if (!isSyncAdapter)
                 {
@@ -625,7 +625,7 @@ public Cursor query(Uri uri, String[] projection, String selection, String[] sel
                 }
                 else
                 {
-                    sqlBuilder.setTables(Tables.INSTANCE_VIEW);
+                    sqlBuilder.setTables(Tables.INSTANCE_CLIENT_VIEW);
                 }
                 selectId(sqlBuilder, Instances._ID, uri);
                 if (!isSyncAdapter)

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

@@ -24,6 +24,7 @@
 import org.dmfs.jems.iterable.decorators.Mapped;
 import org.dmfs.jems.pair.Pair;
 import org.dmfs.jems.single.Single;
+import org.dmfs.optional.NullSafe;
 import org.dmfs.optional.Optional;
 import org.dmfs.provider.tasks.TaskDatabaseHelper;
 import org.dmfs.provider.tasks.model.TaskAdapter;
@@ -185,7 +186,8 @@ private void updateInstances(SQLiteDatabase db, TaskAdapter taskAdapter, long id
                     (newInstanceValues, cursorRow) ->
                     {
                         existingInstances.moveToPosition(cursorRow);
-                        return (int) (existingInstances.getLong(startIdx) - newInstanceValues.getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME));
+                        return (int) (existingInstances.getLong(startIdx) -
+                                new NullSafe<>(newInstanceValues.getAsLong(TaskContract.Instances.INSTANCE_ORIGINAL_TIME)).value(0L));
                     });
 
             // sync the instances table with the new instances

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

@@ -52,11 +52,13 @@ public Overridden(Optional<DateTime> originalTime, Single<ContentValues> delegat
     public ContentValues value()
     {
         ContentValues values = mDelegate.value();
-        values.put(TaskContract.Instances.INSTANCE_ORIGINAL_TIME, 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))))
-                .value(null));
+        values.put(TaskContract.Instances.INSTANCE_ORIGINAL_TIME,
+                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))))
+                        .value(null));
         return values;
     }
 }

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

@@ -24,8 +24,7 @@
 
 /**
  * A {@link Single} of instance data {@link ContentValues}. It initializes most columns with {@code null} values, except for {@link
- * TaskContract.Instances#TASK_ID} which is left out, {@link TaskContract.Instances#INSTANCE_ORIGINAL_TIME} which is initialized with {@code 0} and {@link
- * TaskContract.Instances#DISTANCE_FROM_CURRENT} which is initialized with {@code 0} as well.
+ * TaskContract.Instances#TASK_ID} which is left out and {@link TaskContract.Instances#DISTANCE_FROM_CURRENT} which is initialized with {@code 0} as well.
  *
  * @author Marten Gajda
  */
@@ -41,7 +40,7 @@ public ContentValues value()
         values.putNull(TaskContract.Instances.INSTANCE_DUE_SORTING);
         values.putNull(TaskContract.Instances.INSTANCE_DURATION);
         values.put(TaskContract.Instances.DISTANCE_FROM_CURRENT, 0);
-        values.put(TaskContract.Instances.INSTANCE_ORIGINAL_TIME, 0);
+        values.putNull(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);
         return values;
     }
 }

opentasks-provider/src/test/java/org/dmfs/provider/tasks/processors/tasks/instancedata/VanillaInstanceDataTest.java

@@ -46,7 +46,7 @@ public void testValue() throws Exception
         assertThat(values.get(TaskContract.Instances.INSTANCE_DUE_SORTING), nullValue());
         assertThat(values.get(TaskContract.Instances.INSTANCE_DURATION), nullValue());
         assertThat(values.get(TaskContract.Instances.DISTANCE_FROM_CURRENT), is(0));
-        assertThat(values.get(TaskContract.Instances.INSTANCE_ORIGINAL_TIME), is(0));
+        assertThat(values.get(TaskContract.Instances.INSTANCE_ORIGINAL_TIME), nullValue());
         assertThat(values.size(), is(7));
     }
 

opentaskspal/src/main/java/org/dmfs/opentaskstestpal/InstanceTestData.java

@@ -32,7 +32,8 @@
 
 
 /**
- * {@link RowData} of the instance table. This sets all values except for {@link TaskContract.Instances#TASK_ID}.
+ * {@link RowData} of the instance view. This sets all instance values except for {@link TaskContract.Instances#TASK_ID} as well as some instance specific task
+ * values.
  * <p>
  * Note: this is meant for use with an assert operation during tests as the instances table is read only and doesn't allow inserts nor updates.
  *
@@ -78,8 +79,16 @@ public ContentProviderOperation.Builder updatedBuilder(@NonNull TransactionConte
                 .withValue(TaskContract.Instances.INSTANCE_DUE_SORTING, new Mapped<>(DateTime::getInstance, mInstanceDue).value(null))
                 .withValue(TaskContract.Instances.INSTANCE_DURATION,
                         new Zipped<>(mInstanceStart, mInstanceDue, (start, due) -> (due.getTimestamp() - start.getTimestamp())).value(null))
-                .withValue(TaskContract.Instances.INSTANCE_ORIGINAL_TIME, mOriginalTime.value(new DateTime(0)).getTimestamp())
-                .withValue(TaskContract.Instances.DISTANCE_FROM_CURRENT, mDistanceFromCurrent);
+                .withValue(TaskContract.Instances.INSTANCE_ORIGINAL_TIME, new Mapped<>(DateTime::getTimestamp, mOriginalTime).value(null))
+                .withValue(TaskContract.Instances.DISTANCE_FROM_CURRENT, mDistanceFromCurrent)
+                // the instances view overrides some of the task values. Since they are closely tied to the instance data we test them here as well.
+                .withValue(TaskContract.Instances.DTSTART, new Mapped<>(DateTime::getTimestamp, mInstanceStart).value(null))
+                .withValue(TaskContract.Instances.DUE, new Mapped<>(DateTime::getTimestamp, mInstanceDue).value(null))
+                .withValue(TaskContract.Instances.ORIGINAL_INSTANCE_TIME, new Mapped<>(DateTime::getTimestamp, mOriginalTime).value(null))
+                .withValue(TaskContract.Instances.DURATION, null)
+                .withValue(TaskContract.Instances.RRULE, null)
+                .withValue(TaskContract.Instances.RDATE, null)
+                .withValue(TaskContract.Instances.EXDATE, null);
     }
 
 }