Skip to content

Commit

Permalink
Fix coincidence naming of LocalCollection members (#957)
Browse files Browse the repository at this point in the history
* Fix overlapping method name and use interface everywhere

* Fix overlapping property name

* Update logger usage

---------

Co-authored-by: Arnau Mora Gras <[email protected]>
  • Loading branch information
sunkup and ArnyminerZ authored Aug 8, 2024
1 parent 6df0925 commit eae6d0c
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class InitCalendarProviderRule private constructor(): ExternalResource() {
localRecurringEvent.add()
LocalEvent.numInstances(provider, account, localRecurringEvent.id!!)
} finally {
calendar.delete()
calendar.deleteCollection()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import android.content.ContentValues
import android.provider.CalendarContract
import android.provider.CalendarContract.ACCOUNT_TYPE_LOCAL
import android.provider.CalendarContract.Events
import android.util.Log
import androidx.test.platform.app.InstrumentationRegistry
import at.bitfire.davdroid.InitCalendarProviderRule
import at.bitfire.ical4android.AndroidCalendar
Expand Down Expand Up @@ -67,7 +66,7 @@ class LocalCalendarTest {

@After
fun tearDown() {
calendar.delete()
calendar.deleteCollection()
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class LocalEventTest {

@After
fun removeCalendar() {
calendar.delete()
calendar.deleteCollection()
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import at.bitfire.davdroid.resource.LocalCollection
class LocalTestCollection: LocalCollection<LocalTestResource> {

override val tag = "LocalTestCollection"
override val url: String
override val collectionUrl: String
get() = "https://example.com"
override val title = "Local Test Collection"

Expand All @@ -21,7 +21,7 @@ class LocalTestCollection: LocalCollection<LocalTestResource> {
override val readOnly: Boolean
get() = throw NotImplementedError()

override fun delete(): Boolean = true
override fun deleteCollection(): Boolean = true

override fun findDeleted() = entries.filter { it.deleted }
override fun findDirty() = entries.filter { it.dirty }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ open class LocalAddressBook @Inject constructor(
fun deleteByAccount(context: Context, accountName: String) {
val mainAccount = Account(accountName, context.getString(R.string.account_type))
findAll(context, null, mainAccount).forEach {
it.delete()
it.deleteCollection()
}
}

Expand Down Expand Up @@ -218,7 +218,7 @@ open class LocalAddressBook @Inject constructor(
fun requireMainAccount(): Account =
mainAccount ?: throw IllegalArgumentException("No main account assigned to address book $account")

override var url: String
override var collectionUrl: String
get() = AccountManager.get(context).getUserData(account, USER_DATA_URL)
?: throw IllegalStateException("Address book has no URL")
set(url) = AccountManager.get(context).setAndVerifyUserData(account, USER_DATA_URL, url)
Expand Down Expand Up @@ -279,7 +279,7 @@ open class LocalAddressBook @Inject constructor(

val nowReadOnly = forceReadOnly || !info.privWriteContent || info.forceReadOnly
if (nowReadOnly != readOnly) {
Constants.log.info("Address book now read-only = $nowReadOnly, updating contacts")
logger.info("Address book now read-only = $nowReadOnly, updating contacts")

// update address book itself
readOnly = nowReadOnly
Expand All @@ -304,7 +304,7 @@ open class LocalAddressBook @Inject constructor(
updateSyncFrameworkSettings()
}

override fun delete(): Boolean {
override fun deleteCollection(): Boolean {
val accountManager = AccountManager.get(context)
return accountManager.removeAccountExplicitly(account)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class LocalCalendar private constructor(

}

override val url: String?
override val collectionUrl: String?
get() = name

override val tag: String
Expand All @@ -107,6 +107,8 @@ class LocalCalendar private constructor(
override val readOnly
get() = accessLevel <= Calendars.CAL_ACCESS_READ

override fun deleteCollection(): Boolean = delete()

override var lastSyncState: SyncState?
get() = provider.query(calendarSyncURI(), arrayOf(COLUMN_SYNC_STATE), null, null, null)?.use { cursor ->
if (cursor.moveToNext())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface LocalCollection<out T: LocalResource<*>> {

/** Address of the remote collection */
@Deprecated("Local collection should be identified by ID, not by URL")
val url: String?
val collectionUrl: String?

/** collection title (used for user notifications etc.) **/
val title: String
Expand All @@ -32,7 +32,7 @@ interface LocalCollection<out T: LocalResource<*>> {
*
* @return true if the collection was deleted, false otherwise
*/
fun delete(): Boolean
fun deleteCollection(): Boolean

/**
* Finds local resources of this collection which have been marked as *deleted* by the user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,12 @@ class LocalJtxCollection(account: Account, client: ContentProviderClient, id: Lo
override val readOnly: Boolean
get() = throw NotImplementedError()

override fun deleteCollection(): Boolean = delete()

override val tag: String
get() = "jtx-${account.name}-$id"
override val collectionUrl: String?
get() = url
override val title: String
get() = displayname ?: id.toString()
override var lastSyncState: SyncState?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ class LocalTaskList private constructor(
accessLevel != TaskListColumns.ACCESS_LEVEL_UNDEFINED &&
accessLevel <= TaskListColumns.ACCESS_LEVEL_READ

override val url: String?
override fun deleteCollection(): Boolean = delete()

override val collectionUrl: String?
get() = syncId

override val tag: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class AddressBookSyncer @AssistedInject constructor(
}
accountSettings.accountManager.setAndVerifyUserData(account, PREVIOUS_GROUP_METHOD, groupMethod)

logger.info("Synchronizing address book: ${addressBook.url}")
logger.info("Synchronizing address book: ${addressBook.collectionUrl}")
logger.info("Taking settings from: ${addressBook.mainAccount}")

val syncManager = contactsSyncManagerFactory.contactsSyncManager(account, accountSettings, httpClient.value, extras, authority, syncResult, provider, addressBook, collection)
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ abstract class Syncer<CollectionType: LocalCollection<*>>(
val localSyncCollections = localSyncCollections(provider)
val newDbCollections = HashMap(dbCollections) // create a copy
for (localCollection in localSyncCollections) {
val dbCollection = dbCollections[localCollection.url?.toHttpUrlOrNull()]
val dbCollection = dbCollections[localCollection.collectionUrl?.toHttpUrlOrNull()]
if (dbCollection == null)
// Collection not available in db = on server (anymore), delete obsolete local collection
localCollection.delete()
localCollection.deleteCollection()
else {
// Collection exists locally, update local collection and remove it from "to be created" map
update(localCollection, dbCollection)
Expand All @@ -120,7 +120,7 @@ abstract class Syncer<CollectionType: LocalCollection<*>>(

// 4. sync local collection contents (events, contacts, tasks)
for (localCollection in localSyncCollections)
dbCollections[localCollection.url?.toHttpUrl()]?.let { dbCollection ->
dbCollections[localCollection.collectionUrl?.toHttpUrl()]?.let { dbCollection ->
syncCollection(provider, localCollection, dbCollection)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class AccountsCleanupWorker @AssistedInject constructor(
val mainAccount = addressBook.mainAccount
if (mainAccount == null || !mainAccountNames.contains(mainAccount.name))
// the main account for this address book doesn't exist anymore
addressBook.delete()
addressBook.deleteCollection()
} catch(e: Exception) {
logger.log(Level.SEVERE, "Couldn't delete address book account", e)
}
Expand Down

0 comments on commit eae6d0c

Please sign in to comment.