Add provider operations data auto-correction to set timezone to null …

build.gradle

@@ -6,6 +6,7 @@ buildscript {
     }
     dependencies {
         classpath 'com.android.tools.build:gradle:3.0.1'
+        classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:' + KOTLIN_VERSION
     }
 }
 

gradle.properties

@@ -3,6 +3,7 @@ BUILD_TOOLS_VERSION=27.0.1
 MIN_SDK_VERSION=15
 TARGET_SDK_VERSION=25
 # dependency versions
+KOTLIN_VERSION=1.2.10
 SUPPORT_LIBRARY_VERSION=25.4.0
 # contentpal 9b087b2 -> 2017-12-12
 CONTENTPAL_VERSION=9b087b2

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

@@ -660,14 +660,15 @@ public interface TaskColumns
 
         /**
          * When this task starts in milliseconds since the epoch.
+         * {@link #IS_ALLDAY} and {@link #TZ} have to be considered together with this timestamp create the date-time.
          * <p>
          * Value: Long
-         * </p>
          */
         String DTSTART = "dtstart";
 
         /**
          * Boolean: flag that indicates that this is an all-day task.
+         * This has to be considered to get the date-time for the {@link #DTSTART} or {@link #DUE} timestamps.
          */
         String IS_ALLDAY = "is_allday";
 
@@ -688,21 +689,25 @@ public interface TaskColumns
         String LAST_MODIFIED = "last_modified";
 
         /**
-         * String: An Olson Id of the time zone of this task. If this value is <code>null</code>, it's automatically replaced by the local time zone.
+         * An Olson Id of the time zone of this task.
+         * This has to be considered together with {@link #IS_ALLDAY} to get the date-time for the {@link #DTSTART} or {@link #DUE} timestamps.
+         * It is {@code null} when {@link #IS_ALLDAY} is true, or when the date-time is a floating time (local time without timezone).
+         * <p>
+         * Value: String
          */
         String TZ = "tz";
 
         /**
-         * When this task is due in milliseconds since the epoch. Only one of {@link #DUE} or {@link #DURATION} must be supplied (or none of both if the task
+         * When this task is due in milliseconds since the epoch. Only one of {@link #DUE} or {@link #DURATION} must be supplied (or neither if the task
          * has no due date).
+         * {@link #IS_ALLDAY} and {@link #TZ} have to be considered together with this timestamp to create the date-time.
          * <p>
          * Value: Long
-         * </p>
          */
         String DUE = "due";
 
         /**
-         * The duration of this task. Only one of {@link #DUE} or {@link #DURATION} must be supplied (or none of both if the task has no due date). Setting a
+         * The duration of this task. Only one of {@link #DUE} or {@link #DURATION} must be supplied (or neither if the task has no due date). Setting a
          * {@link #DURATION} is not allowed when {@link #DTSTART} is <code>null</code>. The Value must be a duration string as in <a
          * href="http://tools.ietf.org/html/rfc5545#section-3.3.6">RFC 5545 Section 3.3.6</a>.
          * <p>

opentasks-provider/build.gradle

@@ -1,4 +1,5 @@
 apply plugin: 'com.android.library'
+apply plugin: 'kotlin-android'
 
 android {
     compileSdkVersion COMPILE_SDK_VERSION.toInteger()
@@ -41,6 +42,8 @@ dependencies {
     androidTestImplementation 'com.android.support:support-annotations:' + SUPPORT_LIBRARY_VERSION
     androidTestImplementation 'com.android.support.test:runner:0.5'
     androidTestImplementation 'com.android.support.test:rules:0.5'
+
+    testImplementation 'org.jetbrains.anko:anko-sqlite:0.10.4'
     testImplementation 'org.robolectric:robolectric:' + ROBOLECTRIC_VERSION
     testImplementation 'junit:junit:4.12'
     testImplementation 'org.mockito:mockito-core:2.10.0'

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

@@ -0,0 +1,147 @@
+/*
+ * Copyright 2018 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.content.ContentProviderClient;
+import android.content.ContentResolver;
+import android.content.Context;
+import android.os.Build;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.runner.AndroidJUnit4;
+
+import org.dmfs.android.contentpal.Operation;
+import org.dmfs.android.contentpal.OperationsQueue;
+import org.dmfs.android.contentpal.RowSnapshot;
+import org.dmfs.android.contentpal.operations.Assert;
+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.rowdata.CharSequenceRowData;
+import org.dmfs.android.contentpal.rowsnapshots.VirtualRowSnapshot;
+import org.dmfs.android.contenttestpal.operations.AssertEmptyTable;
+import org.dmfs.iterables.SingletonIterable;
+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.provider.tasks.processors.tasks.AutoCompleting;
+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 static org.dmfs.android.contenttestpal.ContentMatcher.resultsIn;
+import static org.junit.Assert.assertThat;
+
+
+/**
+ * Test cases for {@link TaskProvider} that check data auto-corrections/auto-completions done by provider for task insert/update.
+ * <p>
+ * Implementation is in {@link AutoCompleting}.
+ *
+ * @author Gabor Keszthelyi
+ */
+@RunWith(AndroidJUnit4.class)
+public class TaskProviderAutoCompletingTest
+{
+    private ContentResolver mResolver;
+    private String mAuthority;
+    private Context mContext;
+    private ContentProviderClient mClient;
+
+
+    @Before
+    public void setUp() throws Exception
+    {
+        mContext = InstrumentationRegistry.getTargetContext();
+        mResolver = mContext.getContentResolver();
+        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 SingletonIterable<Operation<?>>(new BulkDelete<>(new LocalTaskListsTable(mAuthority))));
+        queue.flush();
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N)
+        {
+            mClient.close();
+        }
+        else
+        {
+            mClient.release();
+        }
+    }
+
+
+    /**
+     * Test that timezone for all-day tasks are set to null.
+     */
+    @Test
+    public void testInsertTaskWithAllDayAndTimeZone()
+    {
+        RowSnapshot<TaskLists> taskList = new VirtualRowSnapshot<>(new LocalTaskListsTable(mAuthority));
+        RowSnapshot<Tasks> allDayTask = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));
+        RowSnapshot<Tasks> notAllDayTask = new VirtualRowSnapshot<>(new TaskListScoped(taskList, new TasksTable(mAuthority)));
+
+        long start = System.currentTimeMillis();
+
+        assertThat(new Seq<>(
+                new Put<>(taskList, new NameData("list1")),
+                new Put<>(allDayTask, (transactionContext, builder) -> builder
+                        .withValue(Tasks.DTSTART, start)
+                        .withValue(Tasks.TZ, "Europe/Berlin")
+                        .withValue(Tasks.IS_ALLDAY, 1)
+                ),
+                new Put<>(notAllDayTask, (transactionContext, builder) -> builder
+                        .withValue(Tasks.DTSTART, start)
+                        .withValue(Tasks.TZ, "Europe/Paris")
+                        .withValue(Tasks.IS_ALLDAY, 0)
+                )
+
+        ), resultsIn(mClient,
+                new Assert<>(taskList, new NameData("list1")),
+                new Assert<>(allDayTask, new CharSequenceRowData<>(Tasks.TZ, null)),
+                new Assert<>(notAllDayTask, new CharSequenceRowData<>(Tasks.TZ, "Europe/Paris"))
+        ));
+    }
+
+}

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 = 19;
+    private static final int DATABASE_VERSION = 20;
 
 
     /**
@@ -811,6 +811,11 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion)
             db.execSQL(SQL_CREATE_INSTANCE_CLIENT_VIEW);
         }
 
+        if (oldVersion < 20)
+        {
+            db.execSQL("UPDATE " + Tables.TASKS + " SET " + Tasks.TZ + " = NULL WHERE " + Tasks.IS_ALLDAY + " = 1");
+        }
+
         // upgrade FTS
         FTSDatabaseHelper.onUpgrade(db, oldVersion, newVersion);
 

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

@@ -222,5 +222,11 @@ else if (!isSyncAdapter)
                 task.set(TaskAdapter.COMPLETED, null);
             }
         }
+
+        // Set timezone to null for all-day date-times
+        if (task.isUpdated(TaskAdapter.IS_ALLDAY) && task.valueOf(TaskAdapter.IS_ALLDAY))
+        {
+            task.set(TaskAdapter.TIMEZONE_RAW, null);
+        }
     }
 }

opentasks-provider/src/test/java/org/dmfs/provider/tasks/TaskDataBaseUpgradeTest.kt

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2017 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.database.sqlite.SQLiteOpenHelper
+import org.dmfs.provider.tasks.TaskDatabaseHelper.Tables.TASKS
+import org.dmfs.tasks.contract.TaskContract.Tasks.*
+import org.hamcrest.Matchers.equalTo
+import org.hamcrest.Matchers.not
+import org.jetbrains.anko.db.MapRowParser
+import org.jetbrains.anko.db.insert
+import org.jetbrains.anko.db.select
+import org.junit.Assert.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import org.robolectric.annotation.Config
+
+/**
+ * Test for checking the SQLite database upgrades.
+ *
+ * @author Gabor Keszthelyi
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(manifest = Config.NONE)
+class TaskDataBaseUpgradeTest {
+
+
+    /**
+     * On upgrading to version 20 the timezones of tasks with [IS_ALLDAY] true have to be replaced with `null`.
+     */
+    @Test
+    fun test_20_allDayTimeZonesToNull() {
+
+        val dbHelper: SQLiteOpenHelper = TaskDatabaseHelper(RuntimeEnvironment.application, null)
+
+        val expectedTimeZones: Map<Long, String?> = dbHelper.writableDatabase.use {
+
+            val lId = LIST_ID to 42 // LIST_ID is a required column in the table
+
+            mapOf(
+                    it.insert(TASKS, IS_ALLDAY to "1", TZ to "UTC", lId) to null,
+
+                    it.insert(TASKS, IS_ALLDAY to "1", TZ to "Europe/Berlin", lId) to null,
+
+                    it.insert(TASKS, IS_ALLDAY to "1", TZ to null, lId) to null,
+
+                    it.insert(TASKS, IS_ALLDAY to "0", TZ to "Europe/Paris", lId) to "Europe/Paris",
+
+                    it.insert(TASKS, IS_ALLDAY to null, TZ to "America/Chicago", lId) to "America/Chicago",
+
+                    it.insert(TASKS, IS_ALLDAY to null, TZ to null, lId) to null
+            )
+        }
+
+        dbHelper.writableDatabase.use {
+            dbHelper.onUpgrade(it, 19, 20)
+        }
+
+        dbHelper.readableDatabase.use {
+            val size = it.select(TASKS)
+                    .columns(_ID, TZ)
+                    .parseList(object : MapRowParser<String> {
+
+                        override fun parseRow(columns: Map<String, Any?>): String {
+
+                            // Assert that the actual and expected time zones are the same:
+                            assertThat(expectedTimeZones[columns[_ID]], equalTo(columns[TZ]))
+
+                            return "ignored";
+                        }
+                    }).size
+
+            assertThat(size, not(0)) // Just to make sure that there were rows actually
+        }
+    }
+}

opentaskspal/src/main/java/org/dmfs/opentaskspal/tasks/TimeData.java

@@ -93,7 +93,7 @@ private static ContentProviderOperation.Builder doUpdateBuilder(DateTime start,
     {
         return builder
                 .withValue(TaskContract.Tasks.DTSTART, start.getTimestamp())
-                .withValue(TaskContract.Tasks.TZ, start.isAllDay() ? "UTC" : start.getTimeZone().getID())
+                .withValue(TaskContract.Tasks.TZ, start.isAllDay() ? null : start.getTimeZone().getID())
                 .withValue(TaskContract.Tasks.IS_ALLDAY, start.isAllDay() ? 1 : 0)
 
                 .withValue(TaskContract.Tasks.DUE, due.isPresent() ? due.value().getTimestamp() : null)

opentaskspal/src/test/java/org/dmfs/opentaskspal/tasks/TimeDataTest.java

@@ -160,16 +160,16 @@ public void test_whenStartHasDifferentTimeZoneFromDue_shiftsStartsToDue()
 
 
     @Test
-    public void test_whenStartHasAllDayFlag_correspondingValueIsOne()
+    public void test_whenStartHasAllDayFlag_correspondingValueIsOne_timeZoneIsNull()
     {
-        DateTime start = DateTime.now().toAllDay();
+        DateTime start = DateTime.nowAndHere().toAllDay();
         DateTime due = start.addDuration(new Duration(1, 3, 0));
 
         assertThat(new TimeData(start, due),
                 builds(
                         withValuesOnly(
                                 containing(Tasks.DTSTART, start.getTimestamp()),
-                                containing(Tasks.TZ, "UTC"),
+                                withNullValue(Tasks.TZ),
                                 containing(Tasks.IS_ALLDAY, 1),
 
                                 containing(Tasks.DUE, due.getTimestamp()),