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

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

Don't perform noop updates, implements #836 (#838)

Updates TaskProvider to skip updates which don't change any value. In such case no database operation is necessary and no notifications will be triggered.
parent 620f24e5
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
@@ -43,4 +43,8 @@ dependencies {
    androidTestImplementation deps.support_annotations
    androidTestImplementation deps.support_test_runner
    androidTestImplementation deps.support_test_rules
    androidTestImplementation deps.mockito
    androidTestImplementation deps.jems_testing
    androidTestImplementation deps.hamcrest
    androidTestImplementation deps.contentpal_testing
}
+239 −0
Original line number Diff line number Diff line
/*
 * Copyright 2019 dmfs GmbH
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package org.dmfs.provider.tasks;

import android.accounts.Account;
import android.content.ContentProviderClient;
import android.content.Context;
import android.content.OperationApplicationException;
import android.os.Build;
import android.os.RemoteException;

import org.dmfs.android.contentpal.Operation;
import org.dmfs.android.contentpal.OperationsQueue;
import org.dmfs.android.contentpal.RowSnapshot;
import org.dmfs.android.contentpal.operations.BulkDelete;
import org.dmfs.android.contentpal.operations.Put;
import org.dmfs.android.contentpal.queues.BasicOperationsQueue;
import org.dmfs.android.contentpal.rowsnapshots.VirtualRowSnapshot;
import org.dmfs.android.contentpal.tables.Synced;
import org.dmfs.android.contenttestpal.operations.AssertEmptyTable;
import org.dmfs.iterables.elementary.Seq;
import org.dmfs.opentaskspal.tables.InstanceTable;
import org.dmfs.opentaskspal.tables.LocalTaskListsTable;
import org.dmfs.opentaskspal.tables.TaskListScoped;
import org.dmfs.opentaskspal.tables.TaskListsTable;
import org.dmfs.opentaskspal.tables.TasksTable;
import org.dmfs.opentaskspal.tasklists.NameData;
import org.dmfs.opentaskspal.tasks.TitleData;
import org.dmfs.tasks.contract.TaskContract;
import org.dmfs.tasks.contract.TaskContract.TaskLists;
import org.dmfs.tasks.contract.TaskContract.Tasks;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;

import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;

import static org.dmfs.android.contentpal.testing.android.uri.UriMatcher.hasParam;
import static org.dmfs.provider.tasks.matchers.NotifiesMatcher.notifies;
import static org.dmfs.provider.tasks.matchers.UriMatcher.authority;
import static org.dmfs.provider.tasks.matchers.UriMatcher.path;
import static org.dmfs.provider.tasks.matchers.UriMatcher.scheme;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.emptyIterable;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;


/**
 * Tests for {@link TaskProvider}.
 *
 * @author Marten Gajda
 */
@RunWith(AndroidJUnit4.class)
public class TaskProviderObserverTest
{
    private String mAuthority;
    private Context mContext;
    private ContentProviderClient mClient;
    private final Account testAccount = new Account("foo", "bar");


    @Before
    public void setUp() throws Exception
    {
        mContext = InstrumentationRegistry.getTargetContext();
        mAuthority = AuthorityUtil.taskAuthority(mContext);
        mClient = mContext.getContentResolver().acquireContentProviderClient(mAuthority);

        // Assert that tables are empty:
        OperationsQueue queue = new BasicOperationsQueue(mClient);
        queue.enqueue(new Seq<Operation<?>>(
                new AssertEmptyTable<>(new TasksTable(mAuthority)),
                new AssertEmptyTable<>(new TaskListsTable(mAuthority)),
                new AssertEmptyTable<>(new InstanceTable(mAuthority))));
        queue.flush();
    }


    @After
    public void tearDown() throws Exception
    {
        /*
        TODO When Test Orchestration is available, there will be no need for clean up here and check in setUp(), every test method will run in separate instrumentation
        https://android-developers.googleblog.com/2017/07/android-testing-support-library-10-is.html
        https://developer.android.com/training/testing/junit-runner.html#using-android-test-orchestrator
        */

        // Clear the DB:
        BasicOperationsQueue queue = new BasicOperationsQueue(mClient);
        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)
        {
            mClient.close();
        }
        else
        {
            mClient.release();
        }
    }


    /**
     * Test notifications for creating one task list and task.
     */
    @Test
    public void testSingleInsert() throws RemoteException, OperationApplicationException
    {
        RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
        RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));
        OperationsQueue queue = new BasicOperationsQueue(mClient);

        assertThat(new Seq<>(
                        new Put<>(taskList, new NameData("list1")),
                        new Put<>(task, new TitleData("task1"))),
                notifies(
                        TaskContract.getContentUri(mAuthority),
                        queue,
                        containsInAnyOrder(
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(is("/tasks"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(startsWith("/tasks/"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(startsWith("/instances"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(startsWith("/tasklists/"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(is("/tasklists")),
                                        hasParam(TaskContract.CALLER_IS_SYNCADAPTER, "true"),
                                        hasParam(TaskContract.ACCOUNT_NAME, TaskContract.LOCAL_ACCOUNT_NAME),
                                        hasParam(TaskContract.ACCOUNT_TYPE, TaskContract.LOCAL_ACCOUNT_TYPE)
                                ))));
    }


    /**
     * Update a task and check the notifications.
     */
    @Test
    public void testSingleUpdate() throws RemoteException, OperationApplicationException
    {
        RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
        RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));
        OperationsQueue queue = new BasicOperationsQueue(mClient);

        queue.enqueue(
                new Seq<>(
                        new Put<>(taskList, new NameData("list1")),
                        new Put<>(task, new TitleData("task1"))));
        queue.flush();

        assertThat(new Seq<>(
                        new Put<>(task, new TitleData("task1b"))),
                notifies(
                        TaskContract.getContentUri(mAuthority),
                        queue,
                        // taskprovider should notity the tasks URI iself, the task diretory and the instances directory
                        containsInAnyOrder(
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(is("/tasks"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(startsWith("/tasks/"))
                                ),
                                allOf(
                                        scheme("content"),
                                        authority(mAuthority),
                                        path(is("/instances"))
                                ))));
    }


    /**
     * Test that an update that doesn't change anything doesn't trigger a notification.
     */
    @Test
    public void testNoOpUpdate() throws RemoteException, OperationApplicationException
    {
        RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
        RowSnapshot<Tasks> task = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));
        OperationsQueue queue = new BasicOperationsQueue(mClient);

        queue.enqueue(
                new Seq<>(
                        new Put<>(taskList, new NameData("list1")),
                        new Put<>(task, new TitleData("task1"))));
        queue.flush();

        assertThat(new Seq<>(
                        new Put<>(task, new TitleData("task1"))),
                notifies(
                        TaskContract.getContentUri(mAuthority),
                        queue,
                        // there should no notification
                        emptyIterable()));
    }

}
+122 −0
Original line number Diff line number Diff line
/*
 * Copyright 2019 dmfs GmbH
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package org.dmfs.provider.tasks.matchers;

import android.content.Context;
import android.database.ContentObserver;
import android.net.Uri;
import android.os.Handler;
import android.os.HandlerThread;

import org.dmfs.android.contentpal.Operation;
import org.dmfs.android.contentpal.OperationsQueue;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeDiagnosingMatcher;

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;

import androidx.annotation.NonNull;
import androidx.test.platform.app.InstrumentationRegistry;


/**
 * @author Marten Gajda
 */
public final class NotifiesMatcher extends TypeSafeDiagnosingMatcher<Iterable<? extends Operation<?>>>
{
    private final Uri mUri;
    private final OperationsQueue mOperationsQueue;
    private final Matcher<Iterable<? extends Uri>> mDelegate;


    public static Matcher<Iterable<? extends Operation<?>>> notifies(@NonNull Uri uri, @NonNull OperationsQueue operationsQueue, @NonNull Matcher<Iterable<? extends Uri>> delegate)
    {
        return new NotifiesMatcher(uri, operationsQueue, delegate);
    }


    public NotifiesMatcher(Uri uri, @NonNull OperationsQueue operationsQueue, @NonNull Matcher<Iterable<? extends Uri>> delegate)
    {
        mUri = uri;
        mOperationsQueue = operationsQueue;
        mDelegate = delegate;
    }


    @Override
    protected boolean matchesSafely(Iterable<? extends Operation<?>> item, Description mismatchDescription)
    {
        Collection<Uri> notifications = Collections.synchronizedCollection(new HashSet<>());
        HandlerThread handlerThread = new HandlerThread("ObserverHandlerThread");
        handlerThread.start();

        ContentObserver observer = new ContentObserver(new Handler(handlerThread.getLooper()))
        {
            @Override
            public void onChange(boolean selfChange, Uri uri)
            {
                super.onChange(selfChange, uri);
                System.out.println("Notifcation: " + uri);
                notifications.add(uri);
            }
        };

        Context context = InstrumentationRegistry.getInstrumentation().getContext();
        context.getContentResolver().registerContentObserver(mUri, true, observer);
        try
        {
            try
            {
                mOperationsQueue.enqueue(item);
                mOperationsQueue.flush();
            }
            catch (Exception e)
            {
                throw new RuntimeException("Exception during executing the target OperationBatch", e);
            }

            Thread.sleep(100);
            if (!mDelegate.matches(notifications))
            {
                mismatchDescription.appendText("Wrong notifications ");
                mDelegate.describeMismatch(notifications, mismatchDescription);
                return false;
            }
            return true;
        }
        catch (InterruptedException e)
        {
            e.printStackTrace();
            return false;
        }
        finally
        {
            context.getContentResolver().unregisterContentObserver(observer);
            handlerThread.quit();
        }
    }


    @Override
    public void describeTo(Description description)
    {
        description.appendText("Notifies ").appendDescriptionOf(mDelegate);
    }
}
+49 −0
Original line number Diff line number Diff line
/*
 * Copyright 2019 dmfs GmbH
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package org.dmfs.provider.tasks.matchers;

import android.net.Uri;

import org.hamcrest.Matcher;

import static org.dmfs.jems.hamcrest.matchers.LambdaMatcher.having;
import static org.hamcrest.Matchers.is;


/**
 * @author Marten Gajda
 */
public final class UriMatcher
{
    public static Matcher<Uri> scheme(String scheme)
    {
        return having(Uri::getScheme, is(scheme));
    }


    public static Matcher<Uri> authority(String authority)
    {
        return having(Uri::getEncodedAuthority, is(authority));
    }


    public static Matcher<Uri> path(Matcher<String> patchMatcher)
    {
        return having(Uri::getEncodedPath, patchMatcher);
    }

}
+19 −11
Original line number Diff line number Diff line
@@ -80,6 +80,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;


@@ -154,9 +155,9 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
    /**
     * Boolean to track if there are changes within a transaction.
     * <p>
     * This can be shared by multiple threads, hence the {@link AtomicReference}.
     * This can be shared by multiple threads, hence the {@link AtomicBoolean}.
     */
    private AtomicReference<Boolean> mChanged = new AtomicReference<>(false);
    private AtomicBoolean mChanged = new AtomicBoolean(false);

    /**
     * This is a per transaction/thread flag which indicates whether new lists with an unknown account have been added.
@@ -1020,7 +1021,7 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
                                   final boolean isSyncAdapter)
    {
        int count = 0;
        boolean dataChanged = !TASK_LIST_SYNC_COLUMNS.containsAll(values.keySet());
        boolean dataChanged = false;
        switch (mUriMatcher.match(uri))
        {
            case SYNCSTATE_ID:
@@ -1076,8 +1077,12 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
                        // we need this, because the processors may change the values
                        final ListAdapter list = new CursorContentValuesListAdapter(listId, cursor, cursor.getCount() > 1 ? new ContentValues(values) : values);

                        if (list.hasUpdates())
                        {
                            mListProcessorChain.update(db, list, isSyncAdapter);
                        mChanged.set(true);
                            dataChanged |= !TASK_LIST_SYNC_COLUMNS.containsAll(values.keySet());
                        }
                        // note we still count the row even if no update was necessary
                        count++;
                    }
                }
@@ -1104,11 +1109,12 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
                        // we need this, because the processors may change the values
                        final TaskAdapter task = new CursorContentValuesTaskAdapter(cursor, cursor.getCount() > 1 ? new ContentValues(values) : values);

                        mTaskProcessorChain.update(db, task, isSyncAdapter);
                        if (dataChanged && task.hasUpdates())
                        if (task.hasUpdates())
                        {
                            mChanged.set(true);
                            mTaskProcessorChain.update(db, task, isSyncAdapter);
                            dataChanged |= !TASK_LIST_SYNC_COLUMNS.containsAll(values.keySet());
                        }
                        // note we still count the row even if no update was necessary
                        count++;
                    }
                }
@@ -1142,11 +1148,12 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
                        final InstanceAdapter instance = new CursorContentValuesInstanceAdapter(cursor,
                                cursor.getCount() > 1 ? new ContentValues(values) : values);

                        mInstanceProcessorChain.update(db, instance, isSyncAdapter);
                        if (dataChanged && instance.hasUpdates())
                        if (instance.hasUpdates())
                        {
                            mChanged.set(true);
                            mInstanceProcessorChain.update(db, instance, isSyncAdapter);
                            dataChanged = true;
                        }
                        // note we still count the row even if no update was necessary
                        count++;
                    }
                }
@@ -1229,6 +1236,7 @@ public final class TaskProvider extends SQLiteContentProvider implements OnAccou
        {
            // send notifications, because non-sync columns have been updated
            postNotifyUri(uri);
            mChanged.set(true);
        }

        return count;
Loading