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

Unverified Commit 606960ee authored by Arnau Mora's avatar Arnau Mora Committed by GitHub
Browse files

Fix migrations (#133)



* Added calendar id to subscription

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Dao: use specific update methods

* Fixed calendar id removal

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Fixed calendar matching

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Added url to calendar properties

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Fixed calendar id find

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Fixed test

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Fixed subscription id fetching

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>

* Move LocalCalendar, LocalEvent to separate package

* Introduce MANAGED_BY_DB column to control migration

* Fix migration test

* Use NULL/NOT NULL for COLUMN_MANAGED_BY_DB

* Handle v2.1 migrations (don't double-migrate)

* Test migration from 2.0.3 and 2.1

* More logging during migration

* Improve migration tests

Also closes #135

---------

Signed-off-by: default avatarArnau Mora <arnyminer.z@gmail.com>
Co-authored-by: default avatarRicki Hirner <hirner@bitfire.at>
parent 34ab68d0
Loading
Loading
Loading
Loading
+138 −0
Original line number Diff line number Diff line
{
  "formatVersion": 1,
  "database": {
    "version": 2,
    "identityHash": "d61bf6fb08b622a180a1933b983faae2",
    "entities": [
      {
        "tableName": "subscriptions",
        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `calendarId` INTEGER, `url` TEXT NOT NULL, `eTag` TEXT, `displayName` TEXT NOT NULL, `lastModified` INTEGER, `lastSync` INTEGER, `errorMessage` TEXT, `ignoreEmbeddedAlerts` INTEGER NOT NULL, `defaultAlarmMinutes` INTEGER, `color` INTEGER)",
        "fields": [
          {
            "fieldPath": "id",
            "columnName": "id",
            "affinity": "INTEGER",
            "notNull": true
          },
          {
            "fieldPath": "calendarId",
            "columnName": "calendarId",
            "affinity": "INTEGER",
            "notNull": false
          },
          {
            "fieldPath": "url",
            "columnName": "url",
            "affinity": "TEXT",
            "notNull": true
          },
          {
            "fieldPath": "eTag",
            "columnName": "eTag",
            "affinity": "TEXT",
            "notNull": false
          },
          {
            "fieldPath": "displayName",
            "columnName": "displayName",
            "affinity": "TEXT",
            "notNull": true
          },
          {
            "fieldPath": "lastModified",
            "columnName": "lastModified",
            "affinity": "INTEGER",
            "notNull": false
          },
          {
            "fieldPath": "lastSync",
            "columnName": "lastSync",
            "affinity": "INTEGER",
            "notNull": false
          },
          {
            "fieldPath": "errorMessage",
            "columnName": "errorMessage",
            "affinity": "TEXT",
            "notNull": false
          },
          {
            "fieldPath": "ignoreEmbeddedAlerts",
            "columnName": "ignoreEmbeddedAlerts",
            "affinity": "INTEGER",
            "notNull": true
          },
          {
            "fieldPath": "defaultAlarmMinutes",
            "columnName": "defaultAlarmMinutes",
            "affinity": "INTEGER",
            "notNull": false
          },
          {
            "fieldPath": "color",
            "columnName": "color",
            "affinity": "INTEGER",
            "notNull": false
          }
        ],
        "primaryKey": {
          "autoGenerate": true,
          "columnNames": [
            "id"
          ]
        },
        "indices": [],
        "foreignKeys": []
      },
      {
        "tableName": "credentials",
        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`subscriptionId` INTEGER NOT NULL, `username` TEXT NOT NULL, `password` TEXT NOT NULL, PRIMARY KEY(`subscriptionId`), FOREIGN KEY(`subscriptionId`) REFERENCES `subscriptions`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )",
        "fields": [
          {
            "fieldPath": "subscriptionId",
            "columnName": "subscriptionId",
            "affinity": "INTEGER",
            "notNull": true
          },
          {
            "fieldPath": "username",
            "columnName": "username",
            "affinity": "TEXT",
            "notNull": true
          },
          {
            "fieldPath": "password",
            "columnName": "password",
            "affinity": "TEXT",
            "notNull": true
          }
        ],
        "primaryKey": {
          "autoGenerate": false,
          "columnNames": [
            "subscriptionId"
          ]
        },
        "indices": [],
        "foreignKeys": [
          {
            "table": "subscriptions",
            "onDelete": "CASCADE",
            "onUpdate": "NO ACTION",
            "columns": [
              "subscriptionId"
            ],
            "referencedColumns": [
              "id"
            ]
          }
        ]
      }
    ],
    "views": [],
    "setupQueries": [
      "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'd61bf6fb08b622a180a1933b983faae2')"
    ]
  }
}
 No newline at end of file
+60 −15
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import androidx.test.rule.GrantPermissionRule
import androidx.work.Configuration
import androidx.work.Data
import androidx.work.ListenableWorker.Result
import androidx.work.testing.SynchronousExecutor
import androidx.work.testing.TestListenableWorkerBuilder
@@ -23,16 +24,17 @@ import androidx.work.testing.WorkManagerTestInitHelper
import at.bitfire.ical4android.AndroidCalendar
import at.bitfire.ical4android.util.MiscUtils.ContentProviderClientHelper.closeCompat
import at.bitfire.icsdroid.AppAccount
import at.bitfire.icsdroid.Constants.TAG
import at.bitfire.icsdroid.SyncWorker
import at.bitfire.icsdroid.calendar.LocalCalendar
import at.bitfire.icsdroid.db.AppDatabase
import at.bitfire.icsdroid.db.CalendarCredentials
import at.bitfire.icsdroid.db.LocalCalendar
import at.bitfire.icsdroid.db.dao.CredentialsDao
import at.bitfire.icsdroid.db.dao.SubscriptionsDao
import at.bitfire.icsdroid.db.entity.Subscription
import kotlinx.coroutines.runBlocking
import org.junit.*
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.*

class CalendarToRoomMigrationTest {

@@ -104,12 +106,14 @@ class CalendarToRoomMigrationTest {
                Calendars.NAME to CALENDAR_URL
            )
        )
        val calendarId = ContentUris.parseId(uri)
        Log.i(TAG, "Created test calendar $calendarId")

        val calendar = AndroidCalendar.findByID(
            account,
            provider,
            LocalCalendar.Factory,
            ContentUris.parseId(uri)
            calendarId
        )

        // associate credentials, too
@@ -119,20 +123,27 @@ class CalendarToRoomMigrationTest {
    }

    @Test
    fun testSubscriptionCreated() {
        val worker = TestListenableWorkerBuilder<SyncWorker>(
            context = appContext
        ).build()

    fun testMigrateFromV2_0_3() {
        // prepare: create local calendar without subscription
        val calendar = createCalendar()
        assertFalse(calendar.isManagedByDB())

        try {
            runBlocking {
                val result = worker.doWork()
                assertEquals(result, Result.success())

                val subscription = subscriptionsDao.getAll().first()
                // check that the calendar has been added to the subscriptions list
                assertEquals(calendar.id, subscription.id)
                // run worker
                val result = TestListenableWorkerBuilder<SyncWorker>(appContext)
                    .setInputData(Data.Builder()
                        .putBoolean(SyncWorker.ONLY_MIGRATE, true)
                        .build())
                    .build().doWork()
                assertEquals(Result.success(), result)

                // check that calendar is marked as "managed by DB" so that it won't be migrated again
                assertTrue(calendar.isManagedByDB())

                // check that the subscription has been added
                val subscription = subscriptionsDao.getByCalendarId(calendar.id)!!
                assertEquals(calendar.id, subscription.calendarId)
                assertEquals(CALENDAR_DISPLAY_NAME, subscription.displayName)
                assertEquals(Uri.parse(CALENDAR_URL), subscription.url)

@@ -146,4 +157,38 @@ class CalendarToRoomMigrationTest {
        }
    }

    @Test
    fun testMigrateFromV2_1() {
        // prepare: create local calendar plus subscription with subscription.id = LocalCalendar.id,
        // but with calendarId=null and COLUMN_MANAGED_BY_DB=null
        val calendar = createCalendar()
        assertFalse(calendar.isManagedByDB())

        val oldSubscriptionId = subscriptionsDao.add(Subscription.fromLegacyCalendar(calendar).copy(id = calendar.id, calendarId = null))

        try {
            runBlocking {
                // run worker
                val result = TestListenableWorkerBuilder<SyncWorker>(appContext)
                    .setInputData(Data.Builder()
                        .putBoolean(SyncWorker.ONLY_MIGRATE, true)
                        .build())
                    .build().doWork()
                assertEquals(Result.success(), result)

                // check that calendar is marked as "managed by DB" so that it won't be migrated again
                assertTrue(calendar.isManagedByDB())

                // check that the subscription has been added
                val subscription = subscriptionsDao.getByCalendarId(calendar.id)!!
                assertEquals(oldSubscriptionId, subscription.id)
                assertEquals(calendar.id, subscription.calendarId)
                assertEquals(CALENDAR_DISPLAY_NAME, subscription.displayName)
                assertEquals(Uri.parse(CALENDAR_URL), subscription.url)
            }
        } finally {
            calendar.delete()
        }
    }

}
 No newline at end of file
+2 −2
Original line number Diff line number Diff line
@@ -12,8 +12,8 @@ import android.util.Log
import androidx.core.app.NotificationCompat
import at.bitfire.ical4android.Event
import at.bitfire.icsdroid.db.AppDatabase
import at.bitfire.icsdroid.db.LocalCalendar
import at.bitfire.icsdroid.db.LocalEvent
import at.bitfire.icsdroid.calendar.LocalCalendar
import at.bitfire.icsdroid.calendar.LocalEvent
import at.bitfire.icsdroid.db.entity.Subscription
import at.bitfire.icsdroid.ui.EditCalendarActivity
import at.bitfire.icsdroid.ui.NotificationUtils
+52 −24
Original line number Diff line number Diff line
@@ -5,15 +5,16 @@
package at.bitfire.icsdroid

import android.content.ContentProviderClient
import android.content.ContentUris
import android.content.Context
import android.util.Log
import androidx.work.*
import at.bitfire.ical4android.AndroidCalendar
import at.bitfire.ical4android.util.MiscUtils.ContentProviderClientHelper.closeCompat
import at.bitfire.icsdroid.Constants.TAG
import at.bitfire.icsdroid.calendar.LocalCalendar
import at.bitfire.icsdroid.db.AppDatabase
import at.bitfire.icsdroid.db.CalendarCredentials
import at.bitfire.icsdroid.db.LocalCalendar
import at.bitfire.icsdroid.db.entity.Credential
import at.bitfire.icsdroid.db.entity.Subscription
import at.bitfire.icsdroid.ui.NotificationUtils
@@ -29,16 +30,16 @@ class SyncWorker(
        const val NAME = "SyncWorker"

        /**
         * An input data for the Worker that tells whether the synchronization should be performed
         * An input data (Boolean) for the Worker that tells whether the synchronization should be performed
         * without taking into account the current network condition.
         */
        private const val FORCE_RESYNC = "forceResync"
        const val FORCE_RESYNC = "forceResync"

        /**
         * An input data for the Worker that tells if only migration should be performed, without
         * An input data (Boolean) for the Worker that tells if only migration should be performed, without
         * fetching data.
         */
        private const val ONLY_MIGRATE = "onlyMigration"
        const val ONLY_MIGRATE = "onlyMigration"

        /**
         * Enqueues a sync job for immediate execution. If the sync is forced,
@@ -114,7 +115,9 @@ class SyncWorker(

            // sync local calendars
            for (subscription in subscriptionsDao.getAll()) {
                val calendar = LocalCalendar.findById(account, provider, subscription.id)
                // Make sure the subscription has a matching calendar
                subscription.calendarId ?: continue
                val calendar = LocalCalendar.findById(account, provider, subscription.calendarId)
                ProcessEventsTask(applicationContext, subscription, calendar, forceReSync).sync()
            }
        } catch (e: SecurityException) {
@@ -137,26 +140,36 @@ class SyncWorker(
     * 2. Checks that those calendars have a matching [Subscription] in the database.
     * 3. If there's no matching [Subscription], create it.
     */
    @Suppress("DEPRECATION")
    private fun migrateLegacyCalendars() {
        val legacyCredentials = CalendarCredentials(applicationContext)
        @Suppress("DEPRECATION")
        val legacyCredentials by lazy { CalendarCredentials(applicationContext) }

        // if there's a provider available, get all the calendars available in the system
        for (calendar in LocalCalendar.findAll(account, provider)) {
            val match = subscriptionsDao.getById(calendar.id)
            if (match == null) {
                // still no subscription for this calendar ID, create one (= migration)
        for (calendar in LocalCalendar.findUnmanaged(account, provider)) {
            Log.i(TAG, "Found unmanaged (<= v2.1.1) calendar ${calendar.id}, migrating")
            val url = calendar.url ?: continue

            // Special case v2.1: it created subscriptions, but did not set the COLUMN_MANAGED_BY_DB flag.
            val subscription = subscriptionsDao.getByUrl(url)
            if (subscription != null) {
                // So we already have a subscription and only net to set its calendar_id.
                Log.i(TAG, "Migrating from v2.1: updating subscription ${subscription.id} with calendar ID")
                subscriptionsDao.updateCalendarId(subscription.id, calendar.id)

            } else {
                // before v2.1: if there's no subscription with the same URL
                val newSubscription = Subscription.fromLegacyCalendar(calendar)
                subscriptionsDao.add(newSubscription)
                Log.i(TAG, "The calendar #${calendar.id} didn't have a matching subscription. Just created it.")
                Log.i(TAG, "Migrating from < v2.1: creating subscription $newSubscription")
                val subscriptionId = subscriptionsDao.add(newSubscription)

                // migrate credentials, too (if available)
                val (legacyUsername, legacyPassword) = legacyCredentials.get(calendar)
                if (legacyUsername != null && legacyPassword != null)
                    credentialsDao.create(Credential(
                        newSubscription.id, legacyUsername, legacyPassword
                    ))
                    credentialsDao.create(Credential(subscriptionId, legacyUsername, legacyPassword))
            }

            // set MANAGED_BY_DB=1 so that the calendar won't be migrated anymore
            calendar.setManagedByDB()
        }
    }

@@ -168,17 +181,32 @@ class SyncWorker(
     * - deleted if there's no [Subscription] for this calendar.
     */
    private fun updateLocalCalendars() {
        // subscriptions from DB
        val subscriptions = subscriptionsDao.getAll()
        val calendars = LocalCalendar.findAll(account, provider).associateBy { it.id }.toMutableMap()

        // local calendars from provider as Map: <Calendar ID, LocalCalendar>
        val calendars = LocalCalendar.findManaged(account, provider).associateBy { it.id }.toMutableMap()

        // synchronize them
        for (subscription in subscriptions) {
            val calendar = calendars.remove(subscription.id)
            if (calendar != null) {
                Log.d(TAG, "Updating local calendar #${calendar.id} from subscription")
                calendar.update(subscription.toCalendarProperties())
            } else {
            val calendarId = subscription.calendarId
            val calendar = calendars.remove(calendarId)
            // note that calendar might still be null even if calendarId is not null,
            // for instance when the calendar has been removed from the system

            if (calendar == null) {
                // no local calendar yet, create it
                Log.d(TAG, "Creating local calendar from subscription #${subscription.id}")
                AndroidCalendar.create(account, provider, subscription.toCalendarProperties())
                // create local calendar
                val uri = AndroidCalendar.create(account, provider, subscription.toCalendarProperties())
                // update calendar ID in DB
                val newCalendarId = ContentUris.parseId(uri)
                subscriptionsDao.updateCalendarId(subscription.id, newCalendarId)

            } else {
                // local calendar already existing, update accordingly
                Log.d(TAG, "Updating local calendar #$calendarId from subscription")
                calendar.update(subscription.toCalendarProperties())
            }
        }

+46 −3
Original line number Diff line number Diff line
@@ -2,7 +2,7 @@
 * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
 **************************************************************************************************/

package at.bitfire.icsdroid.db
package at.bitfire.icsdroid.calendar

import android.accounts.Account
import android.content.ContentProviderClient
@@ -28,21 +28,34 @@ class LocalCalendar private constructor(

        const val DEFAULT_COLOR = 0xFF2F80C7.toInt()

        @Deprecated("Use Subscription table")
        const val COLUMN_ETAG = Calendars.CAL_SYNC1
        @Deprecated("Use Subscription table")
        const val COLUMN_LAST_MODIFIED = Calendars.CAL_SYNC4
        @Deprecated("Use Subscription table")
        const val COLUMN_LAST_SYNC = Calendars.CAL_SYNC5
        @Deprecated("Use Subscription table")
        const val COLUMN_ERROR_MESSAGE = Calendars.CAL_SYNC6

        /**
         * Stores if the calendar's embedded alerts should be ignored.
         */
        @Deprecated("Use Subscription table")
        const val COLUMN_IGNORE_EMBEDDED = Calendars.CAL_SYNC8

        /**
         * Stores the default alarm to set to all events in the given calendar.
         */
        @Deprecated("Use Subscription table")
        const val COLUMN_DEFAULT_ALARM = Calendars.CAL_SYNC7

        /**
         * Whether this calendar is managed by the [at.bitfire.icsdroid.db.entity.Subscription] table.
         * All calendars should be set to `1` except legacy calendars from the time before we had a database.
         * A `null` value should be considered as _this calendar has not been migrated to the database yet_.
         */
        const val COLUMN_MANAGED_BY_DB = Calendars.CAL_SYNC9

        /**
         * Gets the calendar provider for a given context.
         * The caller (you) is responsible for closing the client!
@@ -60,27 +73,37 @@ class LocalCalendar private constructor(
        fun findById(account: Account, provider: ContentProviderClient, id: Long) =
            findByID(account, provider, Factory, id)

        fun findAll(account: Account, provider: ContentProviderClient) =
            find(account, provider, Factory, null, null)
        fun findManaged(account: Account, provider: ContentProviderClient) =
            find(account, provider, Factory, "$COLUMN_MANAGED_BY_DB IS NOT NULL", null)

        fun findUnmanaged(account: Account, provider: ContentProviderClient) =
            find(account, provider, Factory, "$COLUMN_MANAGED_BY_DB IS NULL", null)

    }

    /** URL of iCalendar file */
    @Deprecated("Use Subscription table")
    var url: String? = null
    /** iCalendar ETag at last successful sync */
    @Deprecated("Use Subscription table")
    var eTag: String? = null

    /** iCalendar Last-Modified at last successful sync (or 0 for none) */
    @Deprecated("Use Subscription table")
    var lastModified = 0L
    /** time of last sync (0 if none) */
    @Deprecated("Use Subscription table")
    var lastSync = 0L
    /** error message (HTTP status or exception name) of last sync (or null) */
    @Deprecated("Use Subscription table")
    var errorMessage: String? = null

    /** Setting: whether to ignore alarms embedded in the Webcal */
    @Deprecated("Use Subscription table")
    var ignoreEmbeddedAlerts: Boolean? = null
    /** Setting: Shall a default alarm be added to every event in the calendar? If yes, this
     *  field contains the minutes before the event. If no, it is *null*. */
    @Deprecated("Use Subscription table")
    var defaultAlarmMinutes: Long? = null


@@ -127,6 +150,26 @@ class LocalCalendar private constructor(
        }
    }

    fun isManagedByDB(): Boolean {
        provider.query(calendarSyncURI(), arrayOf(COLUMN_MANAGED_BY_DB), null, null, null)?.use { cursor ->
            if (cursor.moveToNext())
                return !cursor.isNull(0)
        }

        // row doesn't exist, assume default value
        return true
    }

    /**
     * Updates the entry in the provider to set [COLUMN_MANAGED_BY_DB] to 1.
     * The calendar is then marked as _managed by the database_ and won't be migrated anymore, for instance.
     */
    fun setManagedByDB() {
        val values = ContentValues(1)
        values.put(COLUMN_MANAGED_BY_DB, 1)
        provider.update(calendarSyncURI(), values, null, null)
    }


    object Factory : AndroidCalendarFactory<LocalCalendar> {

Loading