Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Unverified Commit 620f24e5 authored by Marten Gajda's avatar Marten Gajda Committed by GitHub
Browse files

Fix duplicate task after moving a task to a new list (#833) 

Upon moving a task to a new list, it's not just moved, but a deleted copy is left behind in the old list, so sync adapters can take note of the removal in the old list.
This commit fixes the creation of the deleted instance. The code tried to create instance columns in the task table, which failed and caused the deleted instance to be missing.

This commit ensures that the TaskAdapter which is used to insert the deleted instance does not contain instance values.
parent 9cefce5e

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

+94 −3
Original line number Diff line number Diff line
@@ -16,13 +16,12 @@ 


package org.dmfs.provider.tasks;



import android.accounts.Account;

import android.content.ContentProviderClient;

import android.content.ContentResolver;

import android.content.ContentValues;

import android.content.Context;

import android.os.Build;

import androidx.test.InstrumentationRegistry;

import androidx.test.runner.AndroidJUnit4;



import org.dmfs.android.contentpal.Operation;

import org.dmfs.android.contentpal.OperationsQueue;
@@ -31,6 +30,7 @@ import org.dmfs.android.contentpal.Table; 
import org.dmfs.android.contentpal.operations.Assert;

import org.dmfs.android.contentpal.operations.BulkDelete;

import org.dmfs.android.contentpal.operations.BulkUpdate;

import org.dmfs.android.contentpal.operations.Counted;

import org.dmfs.android.contentpal.operations.Delete;

import org.dmfs.android.contentpal.operations.Put;

import org.dmfs.android.contentpal.predicates.ReferringTo;
@@ -38,7 +38,9 @@ import org.dmfs.android.contentpal.queues.BasicOperationsQueue; 
import org.dmfs.android.contentpal.rowdata.CharSequenceRowData;

import org.dmfs.android.contentpal.rowdata.Composite;

import org.dmfs.android.contentpal.rowdata.EmptyRowData;

import org.dmfs.android.contentpal.rowdata.Referring;

import org.dmfs.android.contentpal.rowsnapshots.VirtualRowSnapshot;

import org.dmfs.android.contentpal.tables.Synced;

import org.dmfs.android.contenttestpal.operations.AssertEmptyTable;

import org.dmfs.android.contenttestpal.operations.AssertRelated;

import org.dmfs.iterables.SingletonIterable;
@@ -69,6 +71,9 @@ import org.junit.runner.RunWith; 


import java.util.TimeZone;



import androidx.test.InstrumentationRegistry;

import androidx.test.runner.AndroidJUnit4;



import static org.dmfs.android.contenttestpal.ContentMatcher.resultsIn;

import static org.dmfs.optional.Absent.absent;

import static org.junit.Assert.assertThat;
@@ -88,6 +93,7 @@ public class TaskProviderTest 
    private String mAuthority;

    private Context mContext;

    private ContentProviderClient mClient;

    private final Account testAccount = new Account("foo", "bar");





    @Before
@@ -119,7 +125,9 @@ public class TaskProviderTest 


        // Clear the DB:

        BasicOperationsQueue queue = new BasicOperationsQueue(mClient);

        queue.enqueue(new SingletonIterable<Operation<?>>(new BulkDelete<>(new LocalTaskListsTable(mAuthority))));

        queue.enqueue(new Seq<Operation<?>>(

                new BulkDelete<>(new LocalTaskListsTable(mAuthority)),

                new BulkDelete<>(new Synced<>(testAccount, new TaskListsTable(mAuthority)))));

        queue.flush();



        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N)
@@ -779,4 +787,87 @@ public class TaskProviderTest 
                        ))

        ));

    }





    /**

     * Move a non-recurring task to another list.

     */

    @Test

    public void testMoveTaskInstance() throws Exception

    {

        RowSnapshot<TaskLists> taskListOld = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));

        RowSnapshot<TaskLists> taskListNew = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));

        RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskListOld, new TasksTable(mAuthority)));

        OperationsQueue queue = new BasicOperationsQueue(mClient);



        // create two lists and a single task in the first list

        queue.enqueue(new Seq<>(

                new Put<>(taskListOld, new NameData("list1")),

                new Put<>(taskListNew, new NameData("list2")),

                new Put<>(task, new TitleData("title"))

        ));

        queue.flush();



        assertThat(new SingletonIterable<>(

                // update the sole task instance to the new list

                new BulkUpdate<>(new InstanceTable(mAuthority), new Referring<>(Tasks.LIST_ID, taskListNew), new ReferringTo<>(Tasks.LIST_ID, taskListOld))

        ), resultsIn(queue,

                // assert the old list is empty

                new Counted<>(0, new AssertRelated<>(new InstanceTable(mAuthority), Tasks.LIST_ID, taskListOld)),

                new Counted<>(0, new AssertRelated<>(new TasksTable(mAuthority), Tasks.LIST_ID, taskListOld)),

                // assert the new list contains a single entry

                new Counted<>(1, new AssertRelated<>(new InstanceTable(mAuthority), Tasks.LIST_ID, taskListNew)),

                new Counted<>(1, new AssertRelated<>(new TasksTable(mAuthority), Tasks.LIST_ID, taskListNew, new TitleData("title")))

        ));

    }





    /**

     * Move a non-recurring task to another list.

     */

    @Test

    public void testMoveTaskInstanceAsSyncAdapter() throws Exception

    {

        Table<TaskLists> taskListsTable = new Synced<>(testAccount, new TaskListsTable(mAuthority));

        Table<Instances> instancesTable = new Synced<>(testAccount, new InstanceTable(mAuthority));

        Table<Tasks> tasksTable = new Synced<>(testAccount, new TasksTable(mAuthority));



        RowSnapshot<TaskLists> taskListOld = new VirtualRowSnapshot<>(taskListsTable);

        RowSnapshot<TaskLists> taskListNew = new VirtualRowSnapshot<>(taskListsTable);

        RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskListOld, tasksTable));

        OperationsQueue queue = new BasicOperationsQueue(mClient);



        // create two lists and a single task in the first list

        queue.enqueue(new Seq<>(

                new Put<>(taskListOld, new NameData("list1")),

                new Put<>(taskListNew, new NameData("list2")),

                new Put<>(task, new Composite<>(

                        new SyncIdData("syncid"), // give it a sync id, so it counts as synced

                        new TitleData("title")))));

        queue.flush();



        assertThat(new SingletonIterable<>(

                // update the sole task instance to the new list

                new BulkUpdate<>(new InstanceTable(mAuthority), new Referring<>(Tasks.LIST_ID, taskListNew), new ReferringTo<>(Tasks.LIST_ID, taskListOld))

        ), resultsIn(queue,

                // assert the old list contains a deleted entry for the task

                new Counted<>(0,

                        new AssertRelated<>(

                                instancesTable,

                                Tasks.LIST_ID,

                                taskListOld)),

                new Counted<>(1,

                        new AssertRelated<>(

                                tasksTable,

                                Tasks.LIST_ID,

                                taskListOld,

                                new Composite<>(

                                        new TitleData("title"),

                                        new CharSequenceRowData<>(Tasks._DELETED, "1")))),

                // assert the new list contains a single entry

                new Counted<>(1, new AssertRelated<>(instancesTable, Tasks.LIST_ID, taskListNew)),

                new Counted<>(1, new AssertRelated<>(tasksTable, Tasks.LIST_ID, taskListNew, new TitleData("title")))

        ));

    }



}

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

+8 −12
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package org.dmfs.provider.tasks.model; 
import android.content.ContentValues;

import android.database.sqlite.SQLiteDatabase;



import org.dmfs.jems.single.elementary.Reduced;

import org.dmfs.provider.tasks.TaskDatabaseHelper;

import org.dmfs.provider.tasks.model.adapters.FieldAdapter;

import org.dmfs.tasks.contract.TaskContract;
@@ -149,17 +150,12 @@ public class ContentValuesInstanceAdapter extends AbstractInstanceAdapter 
    public TaskAdapter taskAdapter()

    {

        // make sure we remove any instance fields

        ContentValues values = new ContentValues(mValues);

        values.remove(TaskContract.Instances.INSTANCE_START);

        values.remove(TaskContract.Instances.INSTANCE_START_SORTING);

        values.remove(TaskContract.Instances.INSTANCE_DUE);

        values.remove(TaskContract.Instances.INSTANCE_DUE_SORTING);

        values.remove(TaskContract.Instances.INSTANCE_DURATION);

        values.remove(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);

        values.remove(TaskContract.Instances.TASK_ID);

        values.remove(TaskContract.Instances.DISTANCE_FROM_CURRENT);

        values.remove("_id:1");



        return new ContentValuesTaskAdapter(values);

        return new ContentValuesTaskAdapter(new Reduced<String, ContentValues>(

                () -> new ContentValues(mValues),

                (contentValues, column) -> {

                    contentValues.remove(column);

                    return contentValues;

                },

                INSTANCE_COLUMN_NAMES).value());

    }

}

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

+30 −12
Original line number Diff line number Diff line
@@ -18,12 +18,20 @@ package org.dmfs.provider.tasks.model; 


import android.content.ContentValues;

import android.database.Cursor;

import android.database.MatrixCursor;

import android.database.sqlite.SQLiteDatabase;



import org.dmfs.iterables.decorators.Sieved;

import org.dmfs.iterables.elementary.Seq;

import org.dmfs.jems.iterable.decorators.Mapped;

import org.dmfs.jems.single.elementary.Collected;

import org.dmfs.jems.single.elementary.Reduced;

import org.dmfs.provider.tasks.TaskDatabaseHelper;

import org.dmfs.provider.tasks.model.adapters.FieldAdapter;

import org.dmfs.tasks.contract.TaskContract;



import java.util.ArrayList;





/**

 * An {@link InstanceAdapter} that adapts a {@link Cursor} and a {@link ContentValues} instance. All changes are written to the {@link ContentValues} and can be
@@ -170,17 +178,27 @@ public class CursorContentValuesInstanceAdapter extends AbstractInstanceAdapter 
    public TaskAdapter taskAdapter()

    {

        // make sure we remove any instance fields

        ContentValues values = new ContentValues(mValues);

        values.remove(TaskContract.Instances.INSTANCE_START);

        values.remove(TaskContract.Instances.INSTANCE_START_SORTING);

        values.remove(TaskContract.Instances.INSTANCE_DUE);

        values.remove(TaskContract.Instances.INSTANCE_DUE_SORTING);

        values.remove(TaskContract.Instances.INSTANCE_DURATION);

        values.remove(TaskContract.Instances.INSTANCE_ORIGINAL_TIME);

        values.remove(TaskContract.Instances.TASK_ID);

        values.remove(TaskContract.Instances.DISTANCE_FROM_CURRENT);

        values.remove("_id:1");



        return new CursorContentValuesTaskAdapter(valueOf(InstanceAdapter.TASK_ID), mCursor, values);

        ContentValues values = new Reduced<String, ContentValues>(

                () -> new ContentValues(mValues),

                (contentValues, column) -> {

                    contentValues.remove(column);

                    return contentValues;

                },

                INSTANCE_COLUMN_NAMES).value();



        // create a new cursor which doesn't contain the instance columns

        String[] cursorColumns = new Collected<>(

                ArrayList::new,

                new Sieved<>(col -> !INSTANCE_COLUMN_NAMES.contains(col), new Seq<>(mCursor.getColumnNames())))

                .value().toArray(new String[0]);

        MatrixCursor cursor = new MatrixCursor(cursorColumns);

        cursor.addRow(

                new Mapped<>(

                        column -> mCursor.getType(column) == Cursor.FIELD_TYPE_BLOB ? mCursor.getBlob(column) : mCursor.getString(column),

                        new Mapped<>(

                                mCursor::getColumnIndex,

                                new Seq<>(cursorColumns))));

        cursor.moveToFirst();

        return new CursorContentValuesTaskAdapter(valueOf(InstanceAdapter.TASK_ID), cursor, values);

    }

}

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

+18 −0
Original line number Diff line number Diff line
@@ -23,9 +23,15 @@ import org.dmfs.provider.tasks.model.adapters.DateTimeFieldAdapter; 
import org.dmfs.provider.tasks.model.adapters.IntegerFieldAdapter;

import org.dmfs.provider.tasks.model.adapters.LongFieldAdapter;

import org.dmfs.provider.tasks.model.adapters.StringFieldAdapter;

import org.dmfs.tasks.contract.TaskContract;

import org.dmfs.tasks.contract.TaskContract.Instances;

import org.dmfs.tasks.contract.TaskContract.Tasks;



import java.util.Collection;

import java.util.HashSet;



import static java.util.Arrays.asList;





/**

 * Adapter to read instance values from primitive data sets like {@link Cursor}s or {@link ContentValues}s.
@@ -34,6 +40,18 @@ import org.dmfs.tasks.contract.TaskContract.Tasks; 
 */

public interface InstanceAdapter extends EntityAdapter<InstanceAdapter>

{



    Collection<String> INSTANCE_COLUMN_NAMES = new HashSet<>(asList(

            TaskContract.Instances.INSTANCE_START,

            TaskContract.Instances.INSTANCE_START_SORTING,

            TaskContract.Instances.INSTANCE_DUE,

            TaskContract.Instances.INSTANCE_DUE_SORTING,

            TaskContract.Instances.INSTANCE_DURATION,

            TaskContract.Instances.INSTANCE_ORIGINAL_TIME,

            TaskContract.Instances.TASK_ID,

            TaskContract.Instances.DISTANCE_FROM_CURRENT,

            "_id:1"));



    /**

     * Adapter for the row id of a task instance.

     */