From 32a3199962f973aa6a98bf4725aa4f65afbb3efd Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Tue, 28 Jan 2025 13:40:43 +0100 Subject: [PATCH 1/9] Update ical4android --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 09c268af7..c70c1bab6 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -20,7 +20,7 @@ androidx-test-junit = "1.2.1" androidx-work = "2.10.0" bitfire-cert4android = "7cb60c5f6f" bitfire-dav4jvm = "a4f4068d25" -bitfire-ical4android = "12df9bfddb" +bitfire-ical4android = "883954c7ca" bitfire-vcard4android = "ae5d609f92" compose-accompanist = "0.37.0" compose-bom = "2025.01.00" From 552d1a28580291de651e8871ecd5b3fa782ae5e7 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Tue, 28 Jan 2025 14:30:44 +0100 Subject: [PATCH 2/9] Match DB collections with content provider collections via ID --- .../resource/LocalAddressBookStoreTest.kt | 6 +-- .../AccountSettingsMigration17Test.kt | 5 ++- .../davdroid/sync/LocalTestCollection.kt | 2 +- .../at/bitfire/davdroid/sync/SyncerTest.kt | 42 ++++++++++--------- .../davdroid/resource/LocalAddressBook.kt | 21 +++------- .../resource/LocalAddressBookStore.kt | 9 ++-- .../davdroid/resource/LocalCalendar.kt | 6 +-- .../davdroid/resource/LocalCalendarStore.kt | 2 +- .../davdroid/resource/LocalCollection.kt | 5 +-- .../davdroid/resource/LocalJtxCollection.kt | 9 +++- .../resource/LocalJtxCollectionStore.kt | 1 + .../davdroid/resource/LocalTaskList.kt | 8 ++-- .../davdroid/resource/LocalTaskListStore.kt | 2 +- .../migration/AccountSettingsMigration17.kt | 11 ++++- .../bitfire/davdroid/sync/CalendarSyncer.kt | 2 +- .../kotlin/at/bitfire/davdroid/sync/Syncer.kt | 19 ++++----- .../at/bitfire/davdroid/sync/TaskSyncer.kt | 2 +- .../bitfire/davdroid/ui/DebugInfoGenerator.kt | 1 - 18 files changed, 78 insertions(+), 75 deletions(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt index 4ac98ce97..29f45ac94 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt @@ -147,7 +147,7 @@ class LocalAddressBookStoreTest { every { id } returns 1 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() } - every { localAddressBookStore.createAddressBookAccount(any(), any(), any(), any()) } returns null + every { localAddressBookStore.createAddressBookAccount(any(), any(), any()) } returns null assertEquals(null, localAddressBookStore.create(provider, collection)) } @@ -158,7 +158,7 @@ class LocalAddressBookStoreTest { every { id } returns 1 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() } - every { localAddressBookStore.createAddressBookAccount(any(), any(), any(), any()) } returns addressBookAccount + every { localAddressBookStore.createAddressBookAccount(any(), any(), any()) } returns addressBookAccount every { addressBook.readOnly } returns true val addrBook = localAddressBookStore.create(provider, collection)!! @@ -177,7 +177,7 @@ class LocalAddressBookStoreTest { mockkObject(SystemAccountUtils) every { SystemAccountUtils.createAccount(any(), any(), any()) } returns true val result: Account = localAddressBookStore.createAddressBookAccount( - account, "MrRobert@example.com", 42, "https://example.com/addressbook/funnyfriends" + account, "MrRobert@example.com", 42 )!! verify(exactly = 1) { SystemAccountUtils.createAccount(any(), any(), any()) } assertEquals("MrRobert@example.com", result.name) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt index b45de7891..50fbdfb11 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17Test.kt @@ -53,6 +53,7 @@ class AccountSettingsMigration17Test { @Test fun testMigrate_OldAddressBook_CollectionInDB() { + val localAddressBookUserDataUrl = "url" TestAccount.provide(version = 16) { account -> val accountManager = AccountManager.get(context) val addressBookAccountType = context.getString(R.string.account_type_address_book) @@ -63,7 +64,7 @@ class AccountSettingsMigration17Test { // address book has account + URL val url = "https://example.com/address-book" accountManager.setAndVerifyUserData(addressBookAccount, "real_account_name", account.name) - accountManager.setAndVerifyUserData(addressBookAccount, LocalAddressBook.USER_DATA_URL, url) + accountManager.setAndVerifyUserData(addressBookAccount, localAddressBookUserDataUrl, url) // and is known in database db.serviceDao().insertOrReplace( @@ -86,7 +87,7 @@ class AccountSettingsMigration17Test { // migration renames address book, update account addressBookAccount = accountManager.getAccountsByType(addressBookAccountType).filter { - accountManager.getUserData(it, LocalAddressBook.USER_DATA_URL) == url + accountManager.getUserData(it, localAddressBookUserDataUrl) == url }.first() assertEquals("Some Address Book (${account.name}) #100", addressBookAccount.name) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt index 84b804c1d..830f19ca6 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt @@ -8,7 +8,7 @@ import at.bitfire.davdroid.db.SyncState import at.bitfire.davdroid.resource.LocalCollection class LocalTestCollection( - override val collectionUrl: String = "http://example.com/test/" + override val databaseId: Long = 0L ): LocalCollection { override val tag = "LocalTestCollection" diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt index 57b442343..74215b35e 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt @@ -17,7 +17,6 @@ import io.mockk.just import io.mockk.mockk import io.mockk.runs import io.mockk.verify -import okhttp3.HttpUrl.Companion.toHttpUrl import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -66,9 +65,10 @@ class SyncerTest { @Test fun testUpdateCollections_deletesCollection() { - val localCollection = mockk() - every { localCollection.collectionUrl } returns "http://delete.the/collection" - every { localCollection.title } returns "Collection to be deleted locally" + val localCollection = mockk { + every { databaseId } returns 0L + every { title } returns "Collection to be deleted locally" + } // Should delete the localCollection if dbCollection (remote) does not exist val localCollections = mutableListOf(localCollection) @@ -81,12 +81,14 @@ class SyncerTest { @Test fun testUpdateCollections_updatesCollection() { - val localCollection = mockk() - val dbCollection = mockk() - val dbCollections = mapOf("http://update.the/collection".toHttpUrl() to dbCollection) - every { dbCollection.url } returns "http://update.the/collection".toHttpUrl() - every { localCollection.collectionUrl } returns "http://update.the/collection" - every { localCollection.title } returns "The Local Collection" + val localCollection = mockk { + every { databaseId } returns 0L + every { title } returns "The Local Collection" + } + val dbCollection = mockk { + every { id } returns 0L + } + val dbCollections = mapOf(0L to dbCollection) // Should update the localCollection if it exists val result = syncer.updateCollections(provider, listOf(localCollection), dbCollections) @@ -99,13 +101,13 @@ class SyncerTest { @Test fun testUpdateCollections_findsNewCollection() { val dbCollection = mockk { - every { url } returns "http://newly.found/collection".toHttpUrl() + every { id } returns 0L } val localCollections = listOf(mockk { - every { collectionUrl } returns "http://newly.found/collection" + every { databaseId } returns 0L }) val dbCollections = listOf(dbCollection) - val dbCollectionsMap = mapOf(dbCollection.url to dbCollection) + val dbCollectionsMap = mapOf(dbCollection.id to dbCollection) every { syncer.createLocalCollections(provider, dbCollections) } returns localCollections // Should return the new collection, because it was not updated @@ -113,7 +115,7 @@ class SyncerTest { // Updated local collection list contain new entry assertEquals(1, result.size) - assertEquals(dbCollection.url.toString(), result[0].collectionUrl) + assertEquals(dbCollection.id, result[0].databaseId) } @@ -134,14 +136,14 @@ class SyncerTest { val dbCollection1 = mockk() val dbCollection2 = mockk() val dbCollections = mapOf( - "http://newly.found/collection1".toHttpUrl() to dbCollection1, - "http://newly.found/collection2".toHttpUrl() to dbCollection2 + 0L to dbCollection1, + 1L to dbCollection2 ) - val localCollection1 = mockk() - val localCollection2 = mockk() + val localCollection1 = mockk { every { databaseId } returns 0L } + val localCollection2 = mockk { every { databaseId } returns 1L } val localCollections = listOf(localCollection1, localCollection2) - every { localCollection1.collectionUrl } returns "http://newly.found/collection1" - every { localCollection2.collectionUrl } returns "http://newly.found/collection2" + every { localCollection1.databaseId } returns 0L + every { localCollection2.databaseId } returns 1L every { syncer.syncCollection(provider, any(), any()) } just runs // Should call the collection content sync on both collections diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt index ba93542e8..63606cb54 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt @@ -40,11 +40,11 @@ import java.util.logging.Level import java.util.logging.Logger /** - * A local address book. Requires an own Android account, because Android manages contacts per + * A local address book. Requires its own Android account, because Android manages contacts per * account and there is no such thing as "address books". So, DAVx5 creates a "DAVx5 * address book" account for every CardDAV address book. * - * @param account TODO + * @param account DAVx5 account which "owns" this address book * @param _addressBookAccount Address book account (not: DAVx5 account) storing the actual Android * contacts. This is the initial value of [addressBookAccount]. However when the address book is renamed, * the new name will only be available in [addressBookAccount], so usually that one should be used. @@ -103,11 +103,10 @@ open class LocalAddressBook @AssistedInject constructor( val includeGroups get() = groupMethod == GroupMethod.GROUP_VCARDS - @Deprecated("Local collection should be identified by ID, not by URL") - override var collectionUrl: String - get() = AccountManager.get(context).getUserData(addressBookAccount, USER_DATA_URL) - ?: throw IllegalStateException("Address book has no URL") - set(url) = AccountManager.get(context).setAndVerifyUserData(addressBookAccount, USER_DATA_URL, url) + override var databaseId: Long? + get() = AccountManager.get(context).getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull() + ?: throw IllegalStateException("Address book has no collection ID") + set(id) = AccountManager.get(context).setAndVerifyUserData(addressBookAccount, USER_DATA_COLLECTION_ID, id.toString()) /** * Read-only flag for the address book itself. @@ -336,14 +335,6 @@ open class LocalAddressBook @AssistedInject constructor( const val USER_DATA_ACCOUNT_NAME = "account_name" const val USER_DATA_ACCOUNT_TYPE = "account_type" - /** - * URL of the corresponding CardDAV address book. - * - * User data of the address book account (String). - */ - @Deprecated("Use the URL of the DB collection instead") - const val USER_DATA_URL = "url" - /** * ID of the corresponding database [at.bitfire.davdroid.db.Collection]. * diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt index e4ceaf085..42e81a861 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStore.kt @@ -76,8 +76,7 @@ class LocalAddressBookStore @Inject constructor( val addressBookAccount = createAddressBookAccount( account = account, name = name, - id = fromCollection.id, - url = fromCollection.url.toString() + id = fromCollection.id ) ?: return null val addressBook = localAddressBookFactory.create(account, addressBookAccount, provider) @@ -91,14 +90,13 @@ class LocalAddressBookStore @Inject constructor( } @OpenForTesting - internal fun createAddressBookAccount(account: Account, name: String, id: Long, url: String): Account? { + internal fun createAddressBookAccount(account: Account, name: String, id: Long): Account? { // create address book account with reference to account, collection ID and URL val addressBookAccount = Account(name, context.getString(R.string.account_type_address_book)) val userData = bundleOf( LocalAddressBook.USER_DATA_ACCOUNT_NAME to account.name, LocalAddressBook.USER_DATA_ACCOUNT_TYPE to account.type, - LocalAddressBook.USER_DATA_COLLECTION_ID to id.toString(), - LocalAddressBook.USER_DATA_URL to url + LocalAddressBook.USER_DATA_COLLECTION_ID to id.toString() ) if (!SystemAccountUtils.createAccount(context, addressBookAccount, userData)) { logger.warning("Couldn't create address book account: $addressBookAccount") @@ -139,7 +137,6 @@ class LocalAddressBookStore @Inject constructor( accountManager.setAndVerifyUserData(currentAccount, LocalAddressBook.USER_DATA_ACCOUNT_NAME, localCollection.account.name) accountManager.setAndVerifyUserData(currentAccount, LocalAddressBook.USER_DATA_ACCOUNT_TYPE, localCollection.account.type) accountManager.setAndVerifyUserData(currentAccount, LocalAddressBook.USER_DATA_COLLECTION_ID, fromCollection.id.toString()) - accountManager.setAndVerifyUserData(currentAccount, LocalAddressBook.USER_DATA_URL, fromCollection.url.toString()) // Set contacts provider settings localCollection.settings = contactsProviderSettings diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt index 75657a357..669326700 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt @@ -23,7 +23,7 @@ import java.util.logging.Logger /** * Application-specific subclass of [AndroidCalendar] for local calendars. * - * [Calendars.NAME] is used to store the calendar URL. + * [Calendars._SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. */ class LocalCalendar private constructor( account: Account, @@ -40,8 +40,8 @@ class LocalCalendar private constructor( } - override val collectionUrl: String? - get() = name + override val databaseId: Long? + get() = syncId?.toLongOrNull() override val tag: String get() = "events-${account.name}-$id" diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendarStore.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendarStore.kt index ec06c4731..ac1edaff3 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendarStore.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendarStore.kt @@ -82,7 +82,7 @@ class LocalCalendarStore @Inject constructor( private fun valuesFromCollectionInfo(info: Collection, withColor: Boolean): ContentValues { val values = ContentValues() - values.put(Calendars.NAME, info.url.toString()) + values.put(Calendars._SYNC_ID, info.id) values.put(Calendars.CALENDAR_DISPLAY_NAME, if (info.displayName.isNullOrBlank()) info.url.lastSegment else info.displayName) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt index 2028866e7..4592b45b8 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt @@ -11,9 +11,8 @@ interface LocalCollection> { /** A tag that uniquely identifies the collection (DAVx5-wide) */ val tag: String - /** Address of the remote collection */ - @Deprecated("Local collection should be identified by ID, not by URL") - val collectionUrl: String? + /** Database ID of the collection ([at.bitfire.davdroid.db.Collection.id]) */ + val databaseId: Long? /** collection title (used for user notifications etc.) **/ val title: String diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt index 289aedfed..c3cc2835f 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt @@ -11,6 +11,11 @@ import at.bitfire.ical4android.JtxCollection import at.bitfire.ical4android.JtxCollectionFactory import at.bitfire.ical4android.JtxICalObject +/** + * Application-specific implementation for jtx collections. + * + * [JtxContract.JtxCollection.SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. + */ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Long): JtxCollection(account, client, LocalJtxICalObject.Factory, id), LocalCollection{ @@ -20,8 +25,8 @@ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Lo override val tag: String get() = "jtx-${account.name}-$id" - override val collectionUrl: String? - get() = url + override val databaseId: Long? + get() = syncId override val title: String get() = displayname ?: id.toString() override var lastSyncState: SyncState? diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollectionStore.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollectionStore.kt index 194fc1122..47956859d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollectionStore.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollectionStore.kt @@ -59,6 +59,7 @@ class LocalJtxCollectionStore @Inject constructor( val owner = info.ownerId?.let { principalRepository.get(it) } return ContentValues().apply { + put(JtxContract.JtxCollection.SYNC_ID, info.id) put(JtxContract.JtxCollection.URL, info.url.toString()) put( JtxContract.JtxCollection.DISPLAYNAME, diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt index a149a4a0c..91876b13c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt @@ -7,6 +7,8 @@ package at.bitfire.davdroid.resource import android.accounts.Account import android.content.ContentProviderClient import android.content.ContentValues +import android.provider.CalendarContract.SyncColumns +import android.provider.CalendarContract.SyncColumns._SYNC_ID import androidx.core.content.contentValuesOf import at.bitfire.davdroid.db.SyncState import at.bitfire.ical4android.DmfsTaskList @@ -21,7 +23,7 @@ import java.util.logging.Logger /** * App-specific implementation of a task list. * - * [TaskLists._SYNC_ID] is used to store the task list URL. + * [TaskLists._SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. */ class LocalTaskList private constructor( account: Account, @@ -38,8 +40,8 @@ class LocalTaskList private constructor( accessLevel != TaskListColumns.ACCESS_LEVEL_UNDEFINED && accessLevel <= TaskListColumns.ACCESS_LEVEL_READ - override val collectionUrl: String? - get() = syncId + override val databaseId: Long? + get() = syncId?.toLongOrNull() override val tag: String get() = "tasks-${account.name}-$id" diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskListStore.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskListStore.kt index 72a67bc3d..2460dce86 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskListStore.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskListStore.kt @@ -74,7 +74,7 @@ class LocalTaskListStore @AssistedInject constructor( private fun valuesFromCollectionInfo(info: Collection, withColor: Boolean): ContentValues { val values = ContentValues(3) - values.put(TaskLists._SYNC_ID, info.url.toString()) + values.put(TaskLists._SYNC_ID, info.id.toString()) values.put(TaskLists.LIST_NAME, if (info.displayName.isNullOrBlank()) info.url.lastSegment else info.displayName) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt index 04e091e6f..69d70bb3b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt @@ -14,6 +14,7 @@ import at.bitfire.davdroid.repository.DavCollectionRepository import at.bitfire.davdroid.repository.DavServiceRepository import at.bitfire.davdroid.resource.LocalAddressBook import at.bitfire.davdroid.resource.LocalAddressBookStore +import at.bitfire.davdroid.sync.account.setAndVerifyUserData import dagger.Binds import dagger.Module import dagger.hilt.InstallIn @@ -40,6 +41,7 @@ class AccountSettingsMigration17 @Inject constructor( ): AccountSettingsMigration { override fun migrate(account: Account) { + val localAddressBookAccountUserDataUrl = "url" val addressBookAccountType = context.getString(R.string.account_type_address_book) try { context.contentResolver.acquireContentProviderClient(ContactsContract.AUTHORITY) @@ -62,10 +64,17 @@ class AccountSettingsMigration17 @Inject constructor( // Old address books only have a URL, so use it to determine the collection ID logger.info("Migrating address book ${oldAddressBookAccount.name}") val oldAddressBook = localAddressBookFactory.create(account, oldAddressBookAccount, provider) - val url = accountManager.getUserData(oldAddressBookAccount, LocalAddressBook.USER_DATA_URL) + val url = accountManager.getUserData(oldAddressBookAccount, localAddressBookAccountUserDataUrl) collectionRepository.getByServiceAndUrl(service.id, url)?.let { collection -> // Set collection ID and rename the account localAddressBookStore.update(provider, oldAddressBook, collection) + // The user-data-url is not being set in localAddressBookStore.update() anymore, + // but we need to keep it for the migration + accountManager.setAndVerifyUserData( + oldAddressBook.addressBookAccount, + localAddressBookAccountUserDataUrl, + collection.url.toString() + ) } } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt index 78008108a..14e260d54 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt @@ -56,7 +56,7 @@ class CalendarSyncer @AssistedInject constructor( collectionRepository.getSyncCalendars(serviceId) override fun syncCollection(provider: ContentProviderClient, localCollection: LocalCalendar, remoteCollection: Collection) { - logger.info("Synchronizing calendar #${localCollection.id}, URL: ${localCollection.name}") + logger.info("Synchronizing calendar #${localCollection.id}, DB Collection ID: ${localCollection.databaseId}, URL: ${localCollection.name}") val syncManager = calendarSyncManagerFactory.calendarSyncManager( account, diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt index ccb55fce1..6f4760340 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt @@ -19,9 +19,6 @@ import at.bitfire.davdroid.resource.LocalCollection import at.bitfire.davdroid.resource.LocalDataStore import at.bitfire.davdroid.ui.NotificationRegistry import dagger.hilt.android.qualifiers.ApplicationContext -import okhttp3.HttpUrl -import okhttp3.HttpUrl.Companion.toHttpUrl -import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import java.util.logging.Level import java.util.logging.Logger import javax.inject.Inject @@ -119,11 +116,11 @@ abstract class Syncer, CollectionType: * @return The sync enabled collections as hash map identified by their URL */ @VisibleForTesting - internal fun getSyncEnabledCollections(): Map { - val dbCollections = mutableMapOf() + internal fun getSyncEnabledCollections(): Map { + val dbCollections = mutableMapOf() serviceRepository.getByAccountAndType(account.name, serviceType)?.let { service -> for (dbCollection in getDbSyncCollections(service.id)) - dbCollections[dbCollection.url] = dbCollection + dbCollections[dbCollection.id] = dbCollection } return dbCollections } @@ -145,14 +142,14 @@ abstract class Syncer, CollectionType: internal fun updateCollections( provider: ContentProviderClient, localCollections: List, - dbCollections: Map + dbCollections: Map ): List { // create mutable copies of input val updatedLocalCollections = localCollections.toMutableList() val newDbCollections = dbCollections.toMutableMap() for (localCollection in localCollections) { - val dbCollection = dbCollections[localCollection.collectionUrl?.toHttpUrlOrNull()] + val dbCollection = dbCollections[localCollection.databaseId] if (dbCollection == null) { // Collection not available in db = on server (anymore), delete and remove from the updated list logger.info("Deleting local collection ${localCollection.title} without matching remote collection") @@ -162,7 +159,7 @@ abstract class Syncer, CollectionType: // Collection exists locally, update local collection and remove it from "to be created" map logger.fine("Updating local collection ${localCollection.title} with $dbCollection") dataStore.update(provider, localCollection, dbCollection) - newDbCollections -= dbCollection.url + newDbCollections -= dbCollection.id } } @@ -207,9 +204,9 @@ abstract class Syncer, CollectionType: internal fun syncCollectionContents( provider: ContentProviderClient, localCollections: List, - dbCollections: Map + dbCollections: Map ) = localCollections.forEach { localCollection -> - dbCollections[localCollection.collectionUrl?.toHttpUrl()]?.let { dbCollection -> + dbCollections[localCollection.databaseId]?.let { dbCollection -> syncCollection(provider, localCollection, dbCollection) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt index 3bf516b6a..193dec6fa 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt @@ -69,7 +69,7 @@ class TaskSyncer @AssistedInject constructor( collectionRepository.getSyncTaskLists(serviceId) override fun syncCollection(provider: ContentProviderClient, localCollection: LocalTaskList, remoteCollection: Collection) { - logger.info("Synchronizing task list #${localCollection.id} [${localCollection.syncId}]") + logger.info("Synchronizing task list ${localCollection.id} with database collection ID: ${localCollection.databaseId}") val syncManager = tasksSyncManagerFactory.tasksSyncManager( account, diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt index bfcac727c..90f97cab2 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt @@ -376,7 +376,6 @@ class DebugInfoGenerator @Inject constructor( val table = dumpAccount(account, null, AccountDumpInfo.addressBookAccount(account)) writer.append(TextTable.indent(table, 4)) .append("Collection ID: ${accountManager.getUserData(account, LocalAddressBook.USER_DATA_COLLECTION_ID)}\n") - .append(" URL: ${accountManager.getUserData(account, LocalAddressBook.USER_DATA_URL)}\n") .append(" Read-only: ${accountManager.getUserData(account, LocalAddressBook.USER_DATA_READ_ONLY) ?: 0}\n\n") } From 8af1ed0884590c7246124fb2a68b9e04a0536ad9 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Tue, 28 Jan 2025 16:33:53 +0100 Subject: [PATCH 3/9] Minor renaming and KDoc --- .../davdroid/sync/LocalTestCollection.kt | 2 +- .../at/bitfire/davdroid/sync/SyncerTest.kt | 16 ++++++++-------- .../davdroid/resource/LocalAddressBook.kt | 19 ++++++++++--------- .../davdroid/resource/LocalCalendar.kt | 4 ++-- .../davdroid/resource/LocalCollection.kt | 6 +++--- .../davdroid/resource/LocalJtxCollection.kt | 7 +++++-- .../davdroid/resource/LocalTaskList.kt | 14 ++++++-------- .../bitfire/davdroid/sync/CalendarSyncer.kt | 2 +- .../kotlin/at/bitfire/davdroid/sync/Syncer.kt | 4 ++-- .../at/bitfire/davdroid/sync/TaskSyncer.kt | 2 +- 10 files changed, 39 insertions(+), 37 deletions(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt index 830f19ca6..1c9908597 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/LocalTestCollection.kt @@ -8,7 +8,7 @@ import at.bitfire.davdroid.db.SyncState import at.bitfire.davdroid.resource.LocalCollection class LocalTestCollection( - override val databaseId: Long = 0L + override val dbCollectionId: Long = 0L ): LocalCollection { override val tag = "LocalTestCollection" diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt index 74215b35e..a41e8aa34 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/SyncerTest.kt @@ -66,7 +66,7 @@ class SyncerTest { @Test fun testUpdateCollections_deletesCollection() { val localCollection = mockk { - every { databaseId } returns 0L + every { dbCollectionId } returns 0L every { title } returns "Collection to be deleted locally" } @@ -82,7 +82,7 @@ class SyncerTest { @Test fun testUpdateCollections_updatesCollection() { val localCollection = mockk { - every { databaseId } returns 0L + every { dbCollectionId } returns 0L every { title } returns "The Local Collection" } val dbCollection = mockk { @@ -104,7 +104,7 @@ class SyncerTest { every { id } returns 0L } val localCollections = listOf(mockk { - every { databaseId } returns 0L + every { dbCollectionId } returns 0L }) val dbCollections = listOf(dbCollection) val dbCollectionsMap = mapOf(dbCollection.id to dbCollection) @@ -115,7 +115,7 @@ class SyncerTest { // Updated local collection list contain new entry assertEquals(1, result.size) - assertEquals(dbCollection.id, result[0].databaseId) + assertEquals(dbCollection.id, result[0].dbCollectionId) } @@ -139,11 +139,11 @@ class SyncerTest { 0L to dbCollection1, 1L to dbCollection2 ) - val localCollection1 = mockk { every { databaseId } returns 0L } - val localCollection2 = mockk { every { databaseId } returns 1L } + val localCollection1 = mockk { every { dbCollectionId } returns 0L } + val localCollection2 = mockk { every { dbCollectionId } returns 1L } val localCollections = listOf(localCollection1, localCollection2) - every { localCollection1.databaseId } returns 0L - every { localCollection2.databaseId } returns 1L + every { localCollection1.dbCollectionId } returns 0L + every { localCollection2.dbCollectionId } returns 1L every { syncer.syncCollection(provider, any(), any()) } just runs // Should call the collection content sync on both collections diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt index 63606cb54..bd721775e 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt @@ -79,6 +79,8 @@ open class LocalAddressBook @AssistedInject constructor( override val title get() = addressBookAccount.name + private val accountManager by lazy { AccountManager.get(context) } + /** * Whether contact groups ([LocalGroup]) are included in query results * and are affected by updates/deletes on generic members. @@ -87,8 +89,7 @@ open class LocalAddressBook @AssistedInject constructor( * but if it is enabled, [findDirty] will find dirty [LocalContact]s and [LocalGroup]s. */ open val groupMethod: GroupMethod by lazy { - val manager = AccountManager.get(context) - val account = manager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull()?.let { collectionId -> + val account = accountManager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull()?.let { collectionId -> collectionRepository.get(collectionId)?.let { collection -> serviceRepository.get(collection.serviceId)?.let { service -> Account(service.accountName, context.getString(R.string.account_type)) @@ -103,10 +104,11 @@ open class LocalAddressBook @AssistedInject constructor( val includeGroups get() = groupMethod == GroupMethod.GROUP_VCARDS - override var databaseId: Long? - get() = AccountManager.get(context).getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull() - ?: throw IllegalStateException("Address book has no collection ID") - set(id) = AccountManager.get(context).setAndVerifyUserData(addressBookAccount, USER_DATA_COLLECTION_ID, id.toString()) + override var dbCollectionId: Long? + get() = accountManager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull() ?: throw IllegalStateException("Address book has no collection ID") + set(id) { + accountManager.setAndVerifyUserData(addressBookAccount, USER_DATA_COLLECTION_ID, id.toString()) + } /** * Read-only flag for the address book itself. @@ -121,10 +123,10 @@ open class LocalAddressBook @AssistedInject constructor( * Reading this flag returns the stored value from [USER_DATA_READ_ONLY]. */ override var readOnly: Boolean - get() = AccountManager.get(context).getUserData(addressBookAccount, USER_DATA_READ_ONLY) != null + get() = accountManager.getUserData(addressBookAccount, USER_DATA_READ_ONLY) != null set(readOnly) { // set read-only flag for address book itself - AccountManager.get(context).setAndVerifyUserData(addressBookAccount, USER_DATA_READ_ONLY, if (readOnly) "1" else null) + accountManager.setAndVerifyUserData(addressBookAccount, USER_DATA_READ_ONLY, if (readOnly) "1" else null) // update raw contacts val rawContactValues = contentValuesOf(RawContacts.RAW_CONTACT_IS_READ_ONLY to if (readOnly) 1 else 0) @@ -212,7 +214,6 @@ open class LocalAddressBook @AssistedInject constructor( addressBookAccount = newAccount // delete old account - val accountManager = AccountManager.get(context) accountManager.removeAccountExplicitly(oldAccount) return true diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt index 669326700..4e3b0de19 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCalendar.kt @@ -23,7 +23,7 @@ import java.util.logging.Logger /** * Application-specific subclass of [AndroidCalendar] for local calendars. * - * [Calendars._SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. + * [Calendars._SYNC_ID] corresponds to the database collection ID ([at.bitfire.davdroid.db.Collection.id]). */ class LocalCalendar private constructor( account: Account, @@ -40,7 +40,7 @@ class LocalCalendar private constructor( } - override val databaseId: Long? + override val dbCollectionId: Long? get() = syncId?.toLongOrNull() override val tag: String diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt index 4592b45b8..38afb3894 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalCollection.kt @@ -8,11 +8,11 @@ import at.bitfire.davdroid.db.SyncState interface LocalCollection> { - /** A tag that uniquely identifies the collection (DAVx5-wide) */ + /** a tag that uniquely identifies the collection (DAVx5-wide) */ val tag: String - /** Database ID of the collection ([at.bitfire.davdroid.db.Collection.id]) */ - val databaseId: Long? + /** ID of the collection in the database (corresponds to [at.bitfire.davdroid.db.Collection.id]) */ + val dbCollectionId: Long? /** collection title (used for user notifications etc.) **/ val title: String diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt index c3cc2835f..2b1dd65b3 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalJtxCollection.kt @@ -14,7 +14,7 @@ import at.bitfire.ical4android.JtxICalObject /** * Application-specific implementation for jtx collections. * - * [JtxContract.JtxCollection.SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. + * [at.techbee.jtx.JtxContract.JtxCollection.SYNC_ID] corresponds to the database collection ID ([at.bitfire.davdroid.db.Collection.id]). */ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Long): JtxCollection(account, client, LocalJtxICalObject.Factory, id), @@ -25,10 +25,13 @@ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Lo override val tag: String get() = "jtx-${account.name}-$id" - override val databaseId: Long? + + override val dbCollectionId: Long? get() = syncId + override val title: String get() = displayname ?: id.toString() + override var lastSyncState: SyncState? get() = SyncState.fromString(syncstate) set(value) { syncstate = value.toString() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt index 91876b13c..c05ff32ec 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalTaskList.kt @@ -7,8 +7,6 @@ package at.bitfire.davdroid.resource import android.accounts.Account import android.content.ContentProviderClient import android.content.ContentValues -import android.provider.CalendarContract.SyncColumns -import android.provider.CalendarContract.SyncColumns._SYNC_ID import androidx.core.content.contentValuesOf import at.bitfire.davdroid.db.SyncState import at.bitfire.ical4android.DmfsTaskList @@ -23,13 +21,13 @@ import java.util.logging.Logger /** * App-specific implementation of a task list. * - * [TaskLists._SYNC_ID] is used for the DB-Collection ID [at.bitfire.davdroid.db.Collection]. + * [TaskLists._SYNC_ID] corresponds to the database collection ID ([at.bitfire.davdroid.db.Collection.id]). */ class LocalTaskList private constructor( - account: Account, - provider: ContentProviderClient, - providerName: TaskProvider.ProviderName, - id: Long + account: Account, + provider: ContentProviderClient, + providerName: TaskProvider.ProviderName, + id: Long ): DmfsTaskList(account, provider, providerName, LocalTask.Factory, id), LocalCollection { private val logger = Logger.getGlobal() @@ -40,7 +38,7 @@ class LocalTaskList private constructor( accessLevel != TaskListColumns.ACCESS_LEVEL_UNDEFINED && accessLevel <= TaskListColumns.ACCESS_LEVEL_READ - override val databaseId: Long? + override val dbCollectionId: Long? get() = syncId?.toLongOrNull() override val tag: String diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt index 14e260d54..d74f2a566 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncer.kt @@ -56,7 +56,7 @@ class CalendarSyncer @AssistedInject constructor( collectionRepository.getSyncCalendars(serviceId) override fun syncCollection(provider: ContentProviderClient, localCollection: LocalCalendar, remoteCollection: Collection) { - logger.info("Synchronizing calendar #${localCollection.id}, DB Collection ID: ${localCollection.databaseId}, URL: ${localCollection.name}") + logger.info("Synchronizing calendar #${localCollection.id}, DB Collection ID: ${localCollection.dbCollectionId}, URL: ${localCollection.name}") val syncManager = calendarSyncManagerFactory.calendarSyncManager( account, diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt index 6f4760340..9aa8b6b43 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt @@ -149,7 +149,7 @@ abstract class Syncer, CollectionType: val newDbCollections = dbCollections.toMutableMap() for (localCollection in localCollections) { - val dbCollection = dbCollections[localCollection.databaseId] + val dbCollection = dbCollections[localCollection.dbCollectionId] if (dbCollection == null) { // Collection not available in db = on server (anymore), delete and remove from the updated list logger.info("Deleting local collection ${localCollection.title} without matching remote collection") @@ -206,7 +206,7 @@ abstract class Syncer, CollectionType: localCollections: List, dbCollections: Map ) = localCollections.forEach { localCollection -> - dbCollections[localCollection.databaseId]?.let { dbCollection -> + dbCollections[localCollection.dbCollectionId]?.let { dbCollection -> syncCollection(provider, localCollection, dbCollection) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt index 193dec6fa..c56ade01d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/TaskSyncer.kt @@ -69,7 +69,7 @@ class TaskSyncer @AssistedInject constructor( collectionRepository.getSyncTaskLists(serviceId) override fun syncCollection(provider: ContentProviderClient, localCollection: LocalTaskList, remoteCollection: Collection) { - logger.info("Synchronizing task list ${localCollection.id} with database collection ID: ${localCollection.databaseId}") + logger.info("Synchronizing task list ${localCollection.id} with database collection ID: ${localCollection.dbCollectionId}") val syncManager = tasksSyncManagerFactory.tasksSyncManager( account, From 1b4f098dbe653d919d84e212541b4356457e3f69 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 10:15:13 +0100 Subject: [PATCH 4/9] Move string constant to companion object --- .../settings/migration/AccountSettingsMigration17.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt index 69d70bb3b..ee58b5035 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/settings/migration/AccountSettingsMigration17.kt @@ -41,7 +41,6 @@ class AccountSettingsMigration17 @Inject constructor( ): AccountSettingsMigration { override fun migrate(account: Account) { - val localAddressBookAccountUserDataUrl = "url" val addressBookAccountType = context.getString(R.string.account_type_address_book) try { context.contentResolver.acquireContentProviderClient(ContactsContract.AUTHORITY) @@ -64,7 +63,7 @@ class AccountSettingsMigration17 @Inject constructor( // Old address books only have a URL, so use it to determine the collection ID logger.info("Migrating address book ${oldAddressBookAccount.name}") val oldAddressBook = localAddressBookFactory.create(account, oldAddressBookAccount, provider) - val url = accountManager.getUserData(oldAddressBookAccount, localAddressBookAccountUserDataUrl) + val url = accountManager.getUserData(oldAddressBookAccount, LOCAL_ADDRESS_BOOK_ACCOUNT_USER_DATA_URL) collectionRepository.getByServiceAndUrl(service.id, url)?.let { collection -> // Set collection ID and rename the account localAddressBookStore.update(provider, oldAddressBook, collection) @@ -72,7 +71,7 @@ class AccountSettingsMigration17 @Inject constructor( // but we need to keep it for the migration accountManager.setAndVerifyUserData( oldAddressBook.addressBookAccount, - localAddressBookAccountUserDataUrl, + LOCAL_ADDRESS_BOOK_ACCOUNT_USER_DATA_URL, collection.url.toString() ) } @@ -80,6 +79,9 @@ class AccountSettingsMigration17 @Inject constructor( } } + companion object { + private const val LOCAL_ADDRESS_BOOK_ACCOUNT_USER_DATA_URL = "url" + } @Module @InstallIn(SingletonComponent::class) From 0799d1b4de219e28db155f7ebf007c4bf55c6eb0 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 10:16:28 +0100 Subject: [PATCH 5/9] Update KDoc --- app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt index 9aa8b6b43..92916001b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt @@ -113,7 +113,7 @@ abstract class Syncer, CollectionType: * Finds sync enabled collections in database. They contain collection info which might have * been updated by collection refresh [at.bitfire.davdroid.servicedetection.DavResourceFinder]. * - * @return The sync enabled collections as hash map identified by their URL + * @return The sync enabled database collections as hash map identified by their ID */ @VisibleForTesting internal fun getSyncEnabledCollections(): Map { From 9e9ebf3c090f0b4b156b2dd453fffbe9fe9ed13d Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 10:39:03 +0100 Subject: [PATCH 6/9] Use getOrDefault to be more explicit --- app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt index 92916001b..fe5f84056 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt @@ -149,7 +149,7 @@ abstract class Syncer, CollectionType: val newDbCollections = dbCollections.toMutableMap() for (localCollection in localCollections) { - val dbCollection = dbCollections[localCollection.dbCollectionId] + val dbCollection = dbCollections.getOrDefault(localCollection.dbCollectionId, null) if (dbCollection == null) { // Collection not available in db = on server (anymore), delete and remove from the updated list logger.info("Deleting local collection ${localCollection.title} without matching remote collection") From bb06506926cd48f99dcff6bd892583725a5d3c69 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 13:16:19 +0100 Subject: [PATCH 7/9] Remove exception throw on missing collection ID --- .../kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt index bd721775e..c54e82fd4 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/resource/LocalAddressBook.kt @@ -105,7 +105,7 @@ open class LocalAddressBook @AssistedInject constructor( get() = groupMethod == GroupMethod.GROUP_VCARDS override var dbCollectionId: Long? - get() = accountManager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull() ?: throw IllegalStateException("Address book has no collection ID") + get() = accountManager.getUserData(addressBookAccount, USER_DATA_COLLECTION_ID)?.toLongOrNull() set(id) { accountManager.setAndVerifyUserData(addressBookAccount, USER_DATA_COLLECTION_ID, id.toString()) } From 0d7618f1ae221c690231c304f69e465711aa770a Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 16:12:12 +0100 Subject: [PATCH 8/9] Rewrite LocalAddressBookStoreTest --- .../resource/LocalAddressBookStoreTest.kt | 149 ++++++++---------- 1 file changed, 68 insertions(+), 81 deletions(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt index 29f45ac94..2fdb42530 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/resource/LocalAddressBookStoreTest.kt @@ -9,26 +9,18 @@ import android.accounts.AccountManager import android.content.ContentProviderClient import android.content.Context import at.bitfire.davdroid.R +import at.bitfire.davdroid.db.AppDatabase import at.bitfire.davdroid.db.Collection import at.bitfire.davdroid.db.Service -import at.bitfire.davdroid.repository.DavCollectionRepository -import at.bitfire.davdroid.repository.DavServiceRepository -import at.bitfire.davdroid.settings.SettingsManager -import at.bitfire.davdroid.sync.account.SystemAccountUtils +import at.bitfire.davdroid.sync.account.TestAccount import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltAndroidTest -import io.mockk.MockKAnnotations import io.mockk.every -import io.mockk.impl.annotations.InjectMockKs import io.mockk.impl.annotations.RelaxedMockK -import io.mockk.impl.annotations.SpyK -import io.mockk.just +import io.mockk.junit4.MockKRule import io.mockk.mockk import io.mockk.mockkObject -import io.mockk.runs -import io.mockk.unmockkAll -import io.mockk.verify import okhttp3.HttpUrl.Companion.toHttpUrl import org.junit.After import org.junit.Assert.assertEquals @@ -37,71 +29,59 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test -import java.util.logging.Logger import javax.inject.Inject @HiltAndroidTest class LocalAddressBookStoreTest { - @get:Rule - val hiltRule = HiltAndroidRule(this) - - @Inject - @ApplicationContext - @SpyK + @Inject @ApplicationContext lateinit var context: Context - val account by lazy { Account("Test Account", context.getString(R.string.account_type)) } - val addressBookAccount by lazy { Account("MrRobert@example.com", context.getString(R.string.account_type_address_book)) } - - val provider = mockk(relaxed = true) - val addressBook: LocalAddressBook = mockk(relaxed = true) { - every { account } answers { this@LocalAddressBookStoreTest.account } - every { updateSyncFrameworkSettings() } just runs - every { addressBookAccount } answers { this@LocalAddressBookStoreTest.addressBookAccount } - every { settings } returns LocalAddressBookStore.contactsProviderSettings - } - - @Suppress("unused") // used by @InjectMockKs LocalAddressBookStore - @RelaxedMockK - lateinit var collectionRepository: DavCollectionRepository - - @Suppress("unused") // used by @InjectMockKs LocalAddressBookStore - val localAddressBookFactory = mockk { - every { create(any(), any(), provider) } returns addressBook - } + @Inject + lateinit var db: AppDatabase @Inject - @SpyK - lateinit var logger: Logger + lateinit var localAddressBookStore: LocalAddressBookStore @RelaxedMockK - lateinit var settingsManager: SettingsManager + lateinit var provider: ContentProviderClient - @Suppress("unused") // used by @InjectMockKs LocalAddressBookStore - val serviceRepository = mockk(relaxed = true) { - every { get(any()) } returns null - every { get(200) } returns mockk { - every { accountName } returns "MrRobert@example.com" - } - } + @get:Rule + val hiltRule = HiltAndroidRule(this) - @InjectMockKs - @SpyK - lateinit var localAddressBookStore: LocalAddressBookStore + @get:Rule + val mockkRule = MockKRule(this) + + lateinit var addressBookAccountType: String + lateinit var addressBookAccount: Account + lateinit var account: Account + lateinit var service: Service @Before fun setUp() { hiltRule.inject() - // initialize global mocks - MockKAnnotations.init(this) + addressBookAccountType = context.getString(R.string.account_type_address_book) + removeAddressBooks() + + account = TestAccount.create() + service = Service( + id = 200, + accountName = account.name, + type = Service.Companion.TYPE_CARDDAV, + principal = null + ) + db.serviceDao().insertOrReplace(service) + addressBookAccount = Account( + "MrRobert@example.com", + addressBookAccountType + ) } @After fun tearDown() { - unmockkAll() + TestAccount.remove(account) } @@ -111,7 +91,7 @@ class LocalAddressBookStoreTest { every { id } returns 42 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() every { displayName } returns null - every { serviceId } returns 404 + every { serviceId } returns 404 // missing service } assertEquals("funnyfriends #42", localAddressBookStore.accountName(collection)) } @@ -122,19 +102,19 @@ class LocalAddressBookStoreTest { every { id } returns 42 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() every { displayName } returns null - every { serviceId } returns 200 + every { serviceId } returns service.id } val accountName = localAddressBookStore.accountName(collection) - assertEquals("funnyfriends (MrRobert@example.com) #42", accountName) + assertEquals("funnyfriends (${account.name}) #42", accountName) } @Test fun test_accountName_missingDisplayNameAndService() { - val collection = mockk(relaxed = true) { + val collection = mockk { every { id } returns 1 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() every { displayName } returns null - every { serviceId } returns 404 // missing service + every { serviceId } returns 404 // missing service } assertEquals("funnyfriends #1", localAddressBookStore.accountName(collection)) } @@ -143,45 +123,42 @@ class LocalAddressBookStoreTest { @Test fun test_create_createAccountReturnsNull() { val collection = mockk(relaxed = true) { - every { serviceId } returns 200 + every { serviceId } returns service.id every { id } returns 1 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() } + + mockkObject(localAddressBookStore) every { localAddressBookStore.createAddressBookAccount(any(), any(), any()) } returns null + assertEquals(null, localAddressBookStore.create(provider, collection)) } @Test - fun test_create_createAccountReturnsAccount() { + fun test_create_ReadOnly() { val collection = mockk(relaxed = true) { - every { serviceId } returns 200 + every { serviceId } returns service.id every { id } returns 1 every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() + every { readOnly() } returns true } - every { localAddressBookStore.createAddressBookAccount(any(), any(), any()) } returns addressBookAccount - every { addressBook.readOnly } returns true val addrBook = localAddressBookStore.create(provider, collection)!! - - verify(exactly = 1) { addressBook.updateSyncFrameworkSettings() } - assertEquals(addressBookAccount, addrBook.addressBookAccount) - assertEquals(LocalAddressBookStore.contactsProviderSettings, addrBook.settings) - assertEquals(true, addrBook.readOnly) - - every { addressBook.readOnly } returns false - val addrBook2 = localAddressBookStore.create(provider, collection)!! - assertEquals(false, addrBook2.readOnly) + assertEquals(Account("funnyfriends (Test Account) #1", addressBookAccountType), addrBook.addressBookAccount) + assertTrue(addrBook.readOnly) } @Test - fun test_createAccount_succeeds() { - mockkObject(SystemAccountUtils) - every { SystemAccountUtils.createAccount(any(), any(), any()) } returns true - val result: Account = localAddressBookStore.createAddressBookAccount( - account, "MrRobert@example.com", 42 - )!! - verify(exactly = 1) { SystemAccountUtils.createAccount(any(), any(), any()) } - assertEquals("MrRobert@example.com", result.name) - assertEquals(context.getString(R.string.account_type_address_book), result.type) + fun test_create_ReadWrite() { + val collection = mockk(relaxed = true) { + every { serviceId } returns service.id + every { id } returns 1 + every { url } returns "https://example.com/addressbook/funnyfriends".toHttpUrl() + every { readOnly() } returns false + } + + val addrBook = localAddressBookStore.create(provider, collection)!! + assertEquals(Account("funnyfriends (Test Account) #1", addressBookAccountType), addrBook.addressBookAccount) + assertFalse(addrBook.readOnly) } @@ -223,4 +200,14 @@ class LocalAddressBookStoreTest { assertTrue(LocalAddressBookStore.shouldBeReadOnly(collectionNotReadOnly, true)) } + + // helpers + + private fun removeAddressBooks() { + val accountManager = AccountManager.get(context) + accountManager.getAccountsByType(addressBookAccountType).forEach { + accountManager.removeAccount(it, null, null) + } + } + } \ No newline at end of file From 3c8cb287352bd6c33b674fbda7cd866aa072ddc3 Mon Sep 17 00:00:00 2001 From: Sunik Kupfer Date: Wed, 29 Jan 2025 16:13:49 +0100 Subject: [PATCH 9/9] Minor changes - remove unused param - make companion methods internal --- .../at/bitfire/davdroid/ui/DebugInfoGenerator.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt index 90f97cab2..bfe3eed69 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/DebugInfoGenerator.kt @@ -329,7 +329,7 @@ class DebugInfoGenerator @Inject constructor( writer.append("\n\n - Account: ${account.name}\n") val accountSettings = accountSettingsFactory.create(account) - writer.append(dumpAccount(account, accountSettings, AccountDumpInfo.caldavAccount(account))) + writer.append(dumpAndroidAccount(account, AccountDumpInfo.caldavAccount(account))) try { val credentials = accountSettings.credentials() val authStr = mutableListOf() @@ -373,7 +373,7 @@ class DebugInfoGenerator @Inject constructor( */ private fun dumpAddressBookAccount(account: Account, accountManager: AccountManager, writer: Writer) { writer.append(" * Address book: ${account.name}\n") - val table = dumpAccount(account, null, AccountDumpInfo.addressBookAccount(account)) + val table = dumpAndroidAccount(account, AccountDumpInfo.addressBookAccount(account)) writer.append(TextTable.indent(table, 4)) .append("Collection ID: ${accountManager.getUserData(account, LocalAddressBook.USER_DATA_COLLECTION_ID)}\n") .append(" Read-only: ${accountManager.getUserData(account, LocalAddressBook.USER_DATA_READ_ONLY) ?: 0}\n\n") @@ -382,7 +382,7 @@ class DebugInfoGenerator @Inject constructor( /** * Retrieves specified information from an android account. */ - private fun dumpAccount(account: Account, accountSettings: AccountSettings?, infos: Iterable): String { + private fun dumpAndroidAccount(account: Account, infos: Iterable): String { val table = TextTable("Authority", "isSyncable", "syncsOnContentChange", "Entries") for (info in infos) { var nrEntries = "—" @@ -528,7 +528,7 @@ class DebugInfoGenerator @Inject constructor( companion object { - fun caldavAccount(account: Account) = listOf( + internal fun caldavAccount(account: Account) = listOf( AccountDumpInfo(account, CalendarContract.AUTHORITY, CalendarContract.Events.CONTENT_URI.asCalendarSyncAdapter(account), "event(s)"), AccountDumpInfo(account, TaskProvider.ProviderName.JtxBoard.authority, JtxContract.JtxICalObject.CONTENT_URI.asJtxSyncAdapter(account), "jtx Board ICalObject(s)"), AccountDumpInfo(account, TaskProvider.ProviderName.OpenTasks.authority, TaskContract.Tasks.getContentUri( @@ -538,7 +538,7 @@ class DebugInfoGenerator @Inject constructor( AccountDumpInfo(account, ContactsContract.AUTHORITY, ContactsContract.RawContacts.CONTENT_URI.asContactsSyncAdapter(account), "wrongly assigned raw contact(s)") ) - fun addressBookAccount(account: Account) = listOf( + internal fun addressBookAccount(account: Account) = listOf( AccountDumpInfo(account, ContactsContract.AUTHORITY, ContactsContract.RawContacts.CONTENT_URI.asContactsSyncAdapter(account), "raw contact(s)") )