Use timezone ID instead of full VTIMEZONE in DB

[ArnyminerZ]

Purpose

Currently we are storing the full VTimeZone definition in the database, even though we only use the timezone ID.
This PR aims to fix this, and only store the identifier.

Short description

• Removed the column timezone from Collection
• Added a new column timezoneId to Collection
• Upgraded database version number to 15
• Added migrations for database from 14 to 15:
  • Create the new timezoneId column.
  • Extract the timezone id from timezone, and copy it to timezoneId (if not null)
  • Remove the timezone column.

Note

We are storing the timezone id as given by the server, which may not be a valid Android Timezone. This is obviously converted later on when storing into the calendar provider, but maybe it's not a bad idea to convert it beforehand.

More information

• calendar-timezone-id is already being used for MKCALENDAR since MKCALENDAR: send VTIMEZONE in calendar-timezone #1044
• dav4jvm got support for CalendarTimezoneId at bitfireAT/dav4jvm@fbd95a5
• calendar-timezone must contain a full CALENDAR as speciifed by Calendaring Extensions to WebDAV (CalDAV) Section 5.2.2
• calendar-timezone-id definition: CalDAV: Time Zones by Reference - Section 5.2

Checklist

• The PR has a proper title, description and label.
• I have self-reviewed the PR.
• I have added documentation to complex functions and functions that can be used by other modules.
• I have added reasonable tests or consciously decided to not add tests.

[rfc2822]

Is there something missing for this PR yet?

[ArnyminerZ]

Is there something missing for this PR yet?

It should be ready, I forgot to mark it as complete 😅

[sunkup]

Really nice PR! A pleasure to review ;)

Can you tell me how to test the migration? I can see someone created a "Calendar with timezone" in the nextcloud test instance, but it does not have a timezone in the collections table in the database? None of the calendars does in fact, they are all NULL.

I tried to figure out how to change the collection/calendar timezone in the nextcloud interface but neither the setting in the calendar view, nor the "locale" setting for the user seems to work ...

How did you do it?

[sunkup]

Getting an exception when installing app update.

Process: at.bitfire.davdroid, PID: 6791
                 android.database.sqlite.SQLiteException: near "DROP": syntax error (code 1): , while compiling: ALTER TABLE collection DROP COLUMN timezone
                 	at android.database.sqlite.SQLiteConnection.nativePrepareStatement(Native Method)
                 	at android.database.sqlite.SQLiteConnection.acquirePreparedStatement(SQLiteConnection.java:889)
                 	at android.database.sqlite.SQLiteConnection.prepare(SQLiteConnection.java:500)
                 	at android.database.sqlite.SQLiteSession.prepare(SQLiteSession.java:588)
                 	at android.database.sqlite.SQLiteProgram.<init>(SQLiteProgram.java:58)
                 	at android.database.sqlite.SQLiteStatement.<init>(SQLiteStatement.java:31)
                 	at android.database.sqlite.SQLiteDatabase.executeSql(SQLiteDatabase.java:1677)
                 	at android.database.sqlite.SQLiteDatabase.execSQL(SQLiteDatabase.java:1608)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteDatabase.execSQL(FrameworkSQLiteDatabase.kt:246)
                 	at at.bitfire.davdroid.db.AppDatabase$Companion$migrations$1.migrate(AppDatabase.kt:141)
                 	at androidx.room.RoomOpenHelper.onUpgrade(RoomOpenHelper.kt:90)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.onUpgrade(FrameworkSQLiteOpenHelper.kt:253)
                 	at android.database.sqlite.SQLiteOpenHelper.getDatabaseLocked(SQLiteOpenHelper.java:256)
                 	at android.database.sqlite.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:163)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getWritableOrReadableDatabase(FrameworkSQLiteOpenHelper.kt:232)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.innerGetDatabase(FrameworkSQLiteOpenHelper.kt:190)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper$OpenHelper.getSupportDatabase(FrameworkSQLiteOpenHelper.kt:151)
                 	at androidx.sqlite.db.framework.FrameworkSQLiteOpenHelper.getWritableDatabase(FrameworkSQLiteOpenHelper.kt:104)
                 	at androidx.room.RoomDatabase.inTransaction(RoomDatabase.kt:632)
                 	at androidx.room.RoomDatabase.assertNotSuspendingTransaction(RoomDatabase.kt:451)
                 	at at.bitfire.davdroid.db.ServiceDao_Impl.getByAccountAndType(ServiceDao_Impl.java:190)
                 	at at.bitfire.davdroid.sync.TasksAppManager.selectProvider(TasksAppManager.kt:108)
                 	at at.bitfire.davdroid.startup.TasksAppWatcher.onPackageChanged(TasksAppWatcher.kt:76)
                 	at at.bitfire.davdroid.startup.TasksAppWatcher.access$onPackageChanged(TasksAppWatcher.kt:26)
                 	at at.bitfire.davdroid.startup.TasksAppWatcher$onAppCreateAsync$2.emit(TasksAppWatcher.kt:49)
                 	at at.bitfire.davdroid.startup.TasksAppWatcher$onAppCreateAsync$2.emit(TasksAppWatcher.kt:48)
                 	at kotlinx.coroutines.flow.FlowKt__ChannelsKt.emitAllImpl$FlowKt__ChannelsKt(Channels.kt:33)
                 	at kotlinx.coroutines.flow.FlowKt__ChannelsKt.access$emitAllImpl$FlowKt__ChannelsKt(Channels.kt:1)
                 	at kotlinx.coroutines.flow.FlowKt__ChannelsKt$emitAllImpl$1.invokeSuspend(Channels.kt)
                 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
                 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:101)
                 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:589)
                 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:832)
                 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:720)
                 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:707)
                 	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@914fdb, Dispatchers.Default]

[rfc2822]

I have rebased but there's still a failing test

[ArnyminerZ]

The issue was that we were actually not removing the column, which was expected by Room's automated migration test. I've added the drop column, and it does work on SDK 35 and 34, but not on 33-24.

This is only for the test, the app runs correctly on versions lower than 34, but the tests simply don't pass, because they don't do what is expected.

[rfc2822]

Very nice, good that we now have a first DB migration with tests!

Some minor comments.

[rfc2822]

Now Context is injected. The MigrationTestHelper doesn't have to be a @Rule, except when you use some methods from it that we don't need (see docs).

I changed the manual migration to an auto-migration so that we don't need to change the schema manually (including DROP COLUMN). Whenever possible, migrations should be auto-migrations (with as little own code as possible).

Then I changed the migrate15To16 tests so that the result is in v16, regardless of what the current version is. So we can only test the code in question and are not affected by later migrations (which may for example drop timezone at all or whatever).

And I added a migrate15To16_WithTimeZone_Unparseable test. When there was an unparsable VTIMEZONE, the previous migration threw an exception, which is really bad. In case of migrations we really have to go every code line and think of every possibility. I do that by looking at every called method and in this case DateUtils.parseVTimeZone() says in its KDoc that it throws an exception on unparseable time zone definitions.

Please have another look at the details of code and whether you find something missing.