Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove address books sync authority and content provider #877

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class SyncerTest {

/** use our WebDAV provider as a mock provider because it's our own and we don't need any permissions for it */
private val mockAuthority = context.getString(R.string.webdav_authority)
private val mockProvider = context.contentResolver!!.acquireContentProviderClient(mockAuthority)!!

val account = Account(javaClass.canonicalName, context.getString(R.string.account_type))

Expand All @@ -47,7 +46,7 @@ class SyncerTest {
@Test
fun testOnPerformSync_runsSyncAndSetsClassLoader() {
val syncer = TestSyncer(context, db)
syncer.onPerformSync(account, arrayOf(), mockAuthority, mockProvider, SyncResult())
syncer.onPerformSync(account, arrayOf(), mockAuthority, SyncResult())

// check whether onPerformSync() actually calls sync()
assertEquals(1, syncer.syncCalled.get())
Expand Down
27 changes: 5 additions & 22 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
android:resource="@xml/account_authenticator"/>
</service>
<service
android:name=".sync.adapter.CalendarsSyncAdapterService"
android:name=".sync.CalendarsSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
Expand All @@ -188,7 +188,7 @@
android:resource="@xml/sync_calendars"/>
</service>
<service
android:name=".sync.adapter.JtxSyncAdapterService"
android:name=".sync.JtxSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
Expand All @@ -199,7 +199,7 @@
android:resource="@xml/sync_notes"/>
</service>
<service
android:name=".sync.adapter.OpenTasksSyncAdapterService"
android:name=".sync.OpenTasksSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
Expand All @@ -210,7 +210,7 @@
android:resource="@xml/sync_opentasks"/>
</service>
<service
android:name=".sync.adapter.TasksOrgSyncAdapterService"
android:name=".sync.TasksOrgSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
Expand Down Expand Up @@ -244,25 +244,8 @@
android:name="android.accounts.AccountAuthenticator"
android:resource="@xml/account_authenticator_address_book"/>
</service>
<provider
android:authorities="@string/address_books_authority"
android:exported="false"
android:label="@string/address_books_authority_title"
android:name=".sync.adapter.AddressBookProvider" />
<service
android:name=".sync.adapter.AddressBooksSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
<action android:name="android.content.SyncAdapter"/>
</intent-filter>

<meta-data
android:name="android.content.SyncAdapter"
android:resource="@xml/sync_address_books"/>
</service>
<service
android:name=".sync.adapter.ContactsSyncAdapterService"
android:name=".sync.ContactsSyncAdapterService"
android:exported="true"
tools:ignore="ExportedService">
<intent-filter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,26 +90,19 @@ class AccountRepository @Inject constructor(
// insert CardDAV service
val id = insertService(accountName, Service.TYPE_CARDDAV, config.cardDAV)

// initial CardDAV account settings
// initial CardDAV account settings and sync intervals
accountSettings.setGroupMethod(groupMethod)
accountSettings.setSyncInterval(addrBookAuthority, defaultSyncInterval)

// start CardDAV service detection (refresh collections)
RefreshCollectionsWorker.enqueue(context, id)

// set default sync interval and enable sync regardless of permissions
ContentResolver.setIsSyncable(account, addrBookAuthority, 1)
accountSettings.setSyncInterval(addrBookAuthority, defaultSyncInterval)
} else
ContentResolver.setIsSyncable(account, addrBookAuthority, 0)
}

// Configure CalDAV service
if (config.calDAV != null) {
// insert CalDAV service
val id = insertService(accountName, Service.TYPE_CALDAV, config.calDAV)

// start CalDAV service detection (refresh collections)
RefreshCollectionsWorker.enqueue(context, id)

// set default sync interval and enable sync regardless of permissions
ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 1)
accountSettings.setSyncInterval(CalendarContract.AUTHORITY, defaultSyncInterval)
Expand All @@ -123,6 +116,9 @@ class AccountRepository @Inject constructor(
Logger.log.info("Tasks provider ${taskProvider.authority} found. Tasks sync enabled.")
} else
Logger.log.info("No tasks provider found. Did not enable tasks sync.")

// start CalDAV service detection (refresh collections)
RefreshCollectionsWorker.enqueue(context, id)
} else
ContentResolver.setIsSyncable(account, CalendarContract.AUTHORITY, 0)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,13 @@ class AccountSettings(
* @return sync interval in seconds; *[SYNC_INTERVAL_MANUALLY]* if manual sync; *null* if not set
*/
fun getSyncInterval(authority: String): Long? {
if (ContentResolver.getIsSyncable(account, authority) <= 0)
val addrBookAuthority = context.getString(R.string.address_books_authority)

if (ContentResolver.getIsSyncable(account, authority) <= 0 && authority != addrBookAuthority)
return null

val key = when {
authority == context.getString(R.string.address_books_authority) ->
authority == addrBookAuthority ->
KEY_SYNC_INTERVAL_ADDRESSBOOKS
authority == CalendarContract.AUTHORITY ->
KEY_SYNC_INTERVAL_CALENDARS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
**************************************************************************************************/

package at.bitfire.davdroid.sync.adapter
package at.bitfire.davdroid.sync

import android.accounts.Account
import android.app.Service
Expand Down Expand Up @@ -134,7 +134,6 @@ abstract class SyncAdapterService: Service() {
}

// exported sync adapter services; we need a separate class for each authority
class AddressBooksSyncAdapterService: SyncAdapterService()
class CalendarsSyncAdapterService: SyncAdapterService()
class ContactsSyncAdapterService: SyncAdapterService()
class JtxSyncAdapterService: SyncAdapterService()
Expand Down
37 changes: 32 additions & 5 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import android.content.ContentProviderClient
import android.content.Context
import android.content.SyncResult
import android.os.DeadObjectException
import android.provider.ContactsContract
import at.bitfire.davdroid.InvalidAccountException
import at.bitfire.davdroid.R
import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.network.HttpClient
Expand Down Expand Up @@ -59,19 +61,44 @@ abstract class Syncer(
account: Account,
extras: Array<String>,
authority: String,
provider: ContentProviderClient,
syncResult: SyncResult
) {
Logger.log.log(Level.INFO, "$authority sync of $account initiated", extras.joinToString(", "))

// use contacts provider for address books
val contentAuthority =
if (authority == context.getString(R.string.address_books_authority))
ContactsContract.AUTHORITY
else
authority

val accountSettings by lazy { AccountSettings(context, account) }
val httpClient = lazy { HttpClient.Builder(context, accountSettings).build() }

// acquire ContentProviderClient of authority to be synced
val provider = try {
context.contentResolver.acquireContentProviderClient(contentAuthority)
} catch (e: SecurityException) {
Logger.log.log(Level.WARNING, "Missing permissions for authority $contentAuthority", e)
null
}

if (provider == null) {
/* Can happen if
- we're not allowed to access the content provider, or
- the content provider is not available at all, for instance because the respective
system app, like "contacts storage" is disabled */
Logger.log.warning("Couldn't connect to content provider of authority $contentAuthority")
syncResult.stats.numParseExceptions++ // hard sync error
return
}

// run sync
try {
val runSync = /* ose */ true

if (runSync)
sync(account, extras, authority, httpClient, provider, syncResult)
sync(account, extras, contentAuthority, httpClient, provider, syncResult)

} catch (e: DeadObjectException) {
/* May happen when the remote process dies or (since Android 14) when IPC (for instance with the calendar provider)
Expand All @@ -83,15 +110,15 @@ abstract class Syncer(
Logger.log.log(Level.WARNING, "Account was removed during synchronization", e)

} catch (e: Exception) {
Logger.log.log(Level.SEVERE, "Couldn't sync $authority", e)
syncResult.stats.numParseExceptions++
Logger.log.log(Level.SEVERE, "Couldn't sync $contentAuthority", e)
syncResult.stats.numParseExceptions++ // Hard sync error

} finally {
if (httpClient.isInitialized())
httpClient.value.close()
Logger.log.log(
Level.INFO,
"$authority sync of $account finished",
"$contentAuthority sync of $account finished",
extras.joinToString(", "))
}
}
Expand Down

This file was deleted.

125 changes: 56 additions & 69 deletions app/src/main/kotlin/at/bitfire/davdroid/sync/worker/BaseSyncWorker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -290,81 +290,68 @@ abstract class BaseSyncWorker(
// Comes in through SyncAdapterService and is used only by ContactsSyncManager for an Android 7 workaround.
extras.add(ContentResolver.SYNC_EXTRAS_UPLOAD)

// acquire ContentProviderClient of authority to be synced
try {
applicationContext.contentResolver.acquireContentProviderClient(authority)
} catch (e: SecurityException) {
Logger.log.log(Level.WARNING, "Missing permissions to acquire ContentProviderClient for $authority", e)
null
}.use { provider ->
if (provider == null) {
Logger.log.warning("Couldn't acquire ContentProviderClient for $authority")
return@withContext Result.failure()
}

val result = SyncResult()
// Start syncing. We still use the sync adapter framework's SyncResult to pass the sync results, but this
// is only for legacy reasons and can be replaced by an own result class in the future.
runInterruptible {
syncer.onPerformSync(account, extras.toTypedArray(), authority, provider, result)
}
val result = SyncResult()
// Start syncing. We still use the sync adapter framework's SyncResult to pass the sync results, but this
// is only for legacy reasons and can be replaced by an own result class in the future.
runInterruptible {
syncer.onPerformSync(account, extras.toTypedArray(), authority, result)
}

// Check for errors
if (result.hasError()) {
val syncResult = Data.Builder()
.putString("syncresult", result.toString())
.putString("syncResultStats", result.stats.toString())
.build()

val softErrorNotificationTag = account.type + "-" + account.name + "-" + authority

// On soft errors the sync is retried a few times before considered failed
if (result.hasSoftError()) {
Logger.log.warning("Soft error while syncing: result=$result, stats=${result.stats}")
if (runAttemptCount < MAX_RUN_ATTEMPTS) {
val blockDuration = result.delayUntil - System.currentTimeMillis() / 1000
Logger.log.warning("Waiting for $blockDuration seconds, before retrying ...")

// We block the SyncWorker here so that it won't be started by the sync framework immediately again.
// This should be replaced by proper work scheduling as soon as we don't depend on the sync framework anymore.
if (blockDuration > 0)
delay(blockDuration * 1000)

Logger.log.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
return@withContext Result.retry()
}

Logger.log.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")

notificationManager.notifyIfPossible(
softErrorNotificationTag,
NotificationUtils.NOTIFY_SYNC_ERROR,
NotificationUtils.newBuilder(applicationContext, NotificationUtils.CHANNEL_SYNC_IO_ERRORS)
.setSmallIcon(R.drawable.ic_sync_problem_notify)
.setContentTitle(account.name)
.setContentText(applicationContext.getString(R.string.sync_error_retry_limit_reached))
.setSubText(account.name)
.setOnlyAlertOnce(true)
.setPriority(NotificationCompat.PRIORITY_MIN)
.setCategory(NotificationCompat.CATEGORY_ERROR)
.build()
)

return@withContext Result.failure(syncResult)
// Check for errors
if (result.hasError()) {
val syncResult = Data.Builder()
.putString("syncresult", result.toString())
.putString("syncResultStats", result.stats.toString())
.build()

val softErrorNotificationTag = account.type + "-" + account.name + "-" + authority

// On soft errors the sync is retried a few times before considered failed
if (result.hasSoftError()) {
Logger.log.warning("Soft error while syncing: result=$result, stats=${result.stats}")
if (runAttemptCount < MAX_RUN_ATTEMPTS) {
val blockDuration = result.delayUntil - System.currentTimeMillis() / 1000
Logger.log.warning("Waiting for $blockDuration seconds, before retrying ...")

// We block the SyncWorker here so that it won't be started by the sync framework immediately again.
// This should be replaced by proper work scheduling as soon as we don't depend on the sync framework anymore.
if (blockDuration > 0)
delay(blockDuration * 1000)

Logger.log.warning("Retrying on soft error (attempt $runAttemptCount of $MAX_RUN_ATTEMPTS)")
return@withContext Result.retry()
}

// If no soft error found, dismiss sync error notification
notificationManager.cancel(
Logger.log.warning("Max retries on soft errors reached ($runAttemptCount of $MAX_RUN_ATTEMPTS). Treating as failed")

notificationManager.notifyIfPossible(
softErrorNotificationTag,
NotificationUtils.NOTIFY_SYNC_ERROR
NotificationUtils.NOTIFY_SYNC_ERROR,
NotificationUtils.newBuilder(applicationContext, NotificationUtils.CHANNEL_SYNC_IO_ERRORS)
.setSmallIcon(R.drawable.ic_sync_problem_notify)
.setContentTitle(account.name)
.setContentText(applicationContext.getString(R.string.sync_error_retry_limit_reached))
.setSubText(account.name)
.setOnlyAlertOnce(true)
.setPriority(NotificationCompat.PRIORITY_MIN)
.setCategory(NotificationCompat.CATEGORY_ERROR)
.build()
)

// On a hard error - fail with an error message
// Note: SyncManager should have notified the user
if (result.hasHardError()) {
Logger.log.warning("Hard error while syncing: result=$result, stats=${result.stats}")
return@withContext Result.failure(syncResult)
}
return@withContext Result.failure(syncResult)
}

// If no soft error found, dismiss sync error notification
notificationManager.cancel(
softErrorNotificationTag,
NotificationUtils.NOTIFY_SYNC_ERROR
)

// On a hard error - fail with an error message
// Note: SyncManager should have notified the user
if (result.hasHardError()) {
Logger.log.warning("Hard error while syncing: result=$result, stats=${result.stats}")
return@withContext Result.failure(syncResult)
}
}

Expand Down
5 changes: 0 additions & 5 deletions app/src/main/res/xml/sync_address_books.xml

This file was deleted.

Loading