Revert "fix(library): notify when sync fails before library update"

This reverts commit 9ccf5ca291.

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateJob.kt

@@ -182,17 +182,6 @@ class LibraryUpdateJob(private val context: Context, workerParams: WorkerParamet
 
         return withIOContext {
             try {
-                LibraryUpdateSyncResult.consumeFailure()?.let { syncError ->
-                    notifier.showUpdateErrorNotification(
-                        failed = 1,
-                        errorMessage = context.stringResource(
-                            MR.strings.library_update_skipped_sync_error,
-                            syncError,
-                        ),
-                    )
-                    return@withIOContext Result.success()
-                }
-
                 when (target) {
                     Target.CHAPTERS -> updateChapterList()
                     // SY -->
@@ -855,7 +844,6 @@ class LibraryUpdateJob(private val context: Context, workerParams: WorkerParamet
                 // Define the SyncDataJob
                 val syncDataJob = OneTimeWorkRequestBuilder<SyncDataJob>()
                     .addTag(SyncDataJob.TAG_MANUAL)
-                    .addTag(SyncDataJob.TAG_BEFORE_LIBRARY_UPDATE)
                     .build()
 
                 // Chain SyncDataJob to run before LibraryUpdateJob

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateNotifier.kt

@@ -153,7 +153,7 @@ class LibraryUpdateNotifier(
      *
      * @param failed Number of entries that failed to update.
      */
-    fun showUpdateErrorNotification(failed: Int, errorMessage: String? = null) {
+    fun showUpdateErrorNotification(failed: Int) {
         if (failed == 0) {
             return
         }
@@ -163,7 +163,7 @@ class LibraryUpdateNotifier(
             Notifications.CHANNEL_LIBRARY_ERROR,
         ) {
             setContentTitle(context.stringResource(MR.strings.notification_update_error, failed))
-            setContentText(errorMessage ?: context.stringResource(MR.strings.action_show_errors))
+            setContentText(context.stringResource(MR.strings.action_show_errors))
             setSmallIcon(R.drawable.ic_komikku)
             setColor(ContextCompat.getColor(context, R.color.ic_launcher))
 

app/src/main/java/eu/kanade/tachiyomi/data/library/LibraryUpdateSyncResult.kt

@@ -1,20 +0,0 @@
-package eu.kanade.tachiyomi.data.library
-
-/**
- * Holds a sync failure message to be reported by [LibraryUpdateJob] when a library update
- * was chained to run after [eu.kanade.tachiyomi.data.sync.SyncDataJob].
- */
-object LibraryUpdateSyncResult {
-    @Volatile
-    private var pendingError: String? = null
-
-    fun setFailure(message: String) {
-        pendingError = message
-    }
-
-    fun consumeFailure(): String? {
-        val error = pendingError
-        pendingError = null
-        return error
-    }
-}

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncDataJob.kt

@@ -14,18 +14,14 @@ import androidx.work.WorkQuery
 import androidx.work.WorkerParameters
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.SyncStatus
-import eu.kanade.tachiyomi.data.library.LibraryUpdateSyncResult
 import eu.kanade.tachiyomi.data.notification.Notifications
 import eu.kanade.tachiyomi.util.system.isOnline
 import eu.kanade.tachiyomi.util.system.isRunning
 import eu.kanade.tachiyomi.util.system.setForegroundSafely
 import eu.kanade.tachiyomi.util.system.workManager
-import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.runBlocking
 import logcat.LogPriority
-import tachiyomi.core.common.i18n.stringResource
 import tachiyomi.core.common.util.system.logcat
-import tachiyomi.i18n.MR
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
 import java.util.concurrent.TimeUnit
@@ -57,22 +53,12 @@ class SyncDataJob(private val context: Context, workerParams: WorkerParameters)
         setForegroundSafely()
 
         return try {
-            when (val result = SyncManager(context).syncData()) {
+            SyncManager(context).syncData()
-                SyncDataResult.Success -> Result.success()
+            Result.success()
-                is SyncDataResult.Failure -> {
-                    handleSyncFailure(result.message, result.errorAlreadyNotified)
-                    Result.success()
-                }
-            }
-        } catch (e: CancellationException) {
-            throw e
         } catch (e: Exception) {
             logcat(LogPriority.ERROR, e)
-            handleSyncFailure(
+            notifier.showSyncError(e.message)
-                e.message ?: context.stringResource(MR.strings.unknown_error),
+            Result.success() // try again next time
-                errorAlreadyNotified = false,
-            )
-            Result.success()
         } finally {
             // KMK -->
             syncStatus.stop()
@@ -92,19 +78,10 @@ class SyncDataJob(private val context: Context, workerParams: WorkerParameters)
         )
     }
 
-    private fun handleSyncFailure(message: String, errorAlreadyNotified: Boolean) {
-        if (tags.contains(TAG_BEFORE_LIBRARY_UPDATE)) {
-            LibraryUpdateSyncResult.setFailure(message)
-        } else if (!errorAlreadyNotified) {
-            notifier.showSyncError(message)
-        }
-    }
-
     companion object {
         private const val TAG_JOB = "SyncDataJob"
         private const val TAG_AUTO = "$TAG_JOB:auto"
         const val TAG_MANUAL = "$TAG_JOB:manual"
-        const val TAG_BEFORE_LIBRARY_UPDATE = "$TAG_JOB:before-library-update"
 
         fun isRunning(context: Context): Boolean {
             return context.workManager.isRunning(TAG_JOB)

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncDataResult.kt

@@ -1,28 +0,0 @@
-package eu.kanade.tachiyomi.data.sync
-
-/**
- * Captures the most recent sync failure reported by a [eu.kanade.tachiyomi.data.sync.service.SyncService]
- * implementation so [SyncManager] can propagate the message without losing context.
- */
-internal object SyncFailureState {
-    private var failure: Pair<String, Boolean>? = null
-
-    fun report(message: String, errorAlreadyNotified: Boolean = true) {
-        failure = message to errorAlreadyNotified
-    }
-
-    fun consume(): Pair<String, Boolean>? = failure.also { failure = null }
-}
-
-sealed interface SyncDataResult {
-    data object Success : SyncDataResult
-
-    /**
-     * @param message Human-readable failure reason.
-     * @param errorAlreadyNotified `true` when a sync service already showed a sync error notification.
-     */
-    data class Failure(
-        val message: String,
-        val errorAlreadyNotified: Boolean = false,
-    ) : SyncDataResult
-}

app/src/main/java/eu/kanade/tachiyomi/data/sync/SyncManager.kt

@@ -19,9 +19,7 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.protobuf.ProtoBuf
 import logcat.LogPriority
 import logcat.logcat
-import tachiyomi.core.common.i18n.stringResource
 import tachiyomi.core.common.util.system.logcat
-import tachiyomi.i18n.MR
 import tachiyomi.data.Chapters
 import tachiyomi.data.DatabaseHandler
 import tachiyomi.data.manga.MangaMapper.mapManga
@@ -74,9 +72,7 @@ class SyncManager(
      * This function retrieves local data (favorites, manga, extensions, and categories)
      * from the database using the BackupManager, then synchronizes the data with a sync service.
      */
-    suspend fun syncData(): SyncDataResult {
+    suspend fun syncData() {
-        SyncFailureState.consume()
-
         // Reset isSyncing in case it was left over or failed syncing during restore.
         handler.await(inTransaction = true) {
             mangasQueries.resetIsSyncing()
@@ -157,22 +153,12 @@ class SyncManager(
             }
         }
 
-        if (syncService == null) {
+        val remoteBackup = syncService?.doSync(syncData)
-            return SyncDataResult.Failure(
-                message = context.stringResource(MR.strings.sync_failed_generic),
-            )
-        }
-
-        val remoteBackup = syncService.doSync(syncData)
 
         if (remoteBackup == null) {
-            logcat(LogPriority.DEBUG) { "Skip restore due to sync failure" }
+            logcat(LogPriority.DEBUG) { "Skip restore due to network issues" }
-            val (message, errorAlreadyNotified) = SyncFailureState.consume()
+            // should we call showSyncError?
-                ?: (context.stringResource(MR.strings.sync_failed_generic) to false)
+            return
-            return SyncDataResult.Failure(
-                message = message,
-                errorAlreadyNotified = errorAlreadyNotified,
-            )
         }
 
         if (remoteBackup === syncData.backup) {
@@ -180,14 +166,13 @@ class SyncManager(
             logcat(LogPriority.DEBUG) { "Skip restore due to remote was overwrite from local" }
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Sync completed successfully")
-            return SyncDataResult.Success
+            return
         }
 
         // Stop the sync early if the remote backup is null or empty
         if (remoteBackup.backupManga.isEmpty()) {
-            val message = "No data found on remote server."
+            notifier.showSyncError("No data found on remote server.")
-            notifier.showSyncError(message)
+            return
-            return SyncDataResult.Failure(message = message, errorAlreadyNotified = true)
         }
 
         // Check if it's first sync based on lastSyncTimestamp
@@ -195,7 +180,7 @@ class SyncManager(
             // It's first sync no need to restore data. (just update remote data)
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Updated remote data successfully")
-            return SyncDataResult.Success
+            return
         }
 
         val (filteredFavorites, nonFavorites) = filterFavoritesAndNonFavorites(remoteBackup)
@@ -223,7 +208,7 @@ class SyncManager(
             // update the sync timestamp
             syncPreferences.lastSyncTimestamp().set(Date().time)
             notifier.showSyncSuccess("Sync completed successfully")
-            return SyncDataResult.Success
+            return
         }
 
         val backupUri = writeSyncDataToCache(context, newSyncData)
@@ -243,12 +228,8 @@ class SyncManager(
 
             // update the sync timestamp
             syncPreferences.lastSyncTimestamp().set(Date().time)
-            return SyncDataResult.Success
         } else {
             logcat(LogPriority.ERROR) { "Failed to write sync data to file" }
-            return SyncDataResult.Failure(
-                message = context.stringResource(MR.strings.sync_failed_generic),
-            )
         }
     }
 

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/GoogleDriveSyncService.kt

@@ -18,8 +18,6 @@ import com.google.api.services.drive.DriveScopes
 import com.google.api.services.drive.model.File
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
-import eu.kanade.tachiyomi.data.sync.SyncFailureState
-import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import kotlinx.coroutines.launch
 import kotlinx.serialization.json.Json
 import kotlinx.serialization.protobuf.ProtoBuf
@@ -65,8 +63,6 @@ class GoogleDriveSyncService(context: Context, json: Json, syncPreferences: Sync
 
     private val googleDriveService = GoogleDriveService(context)
 
-    private val notifier = SyncNotifier(context)
-
     private val protoBuf: ProtoBuf = Injekt.get()
 
     override suspend fun doSync(syncData: SyncData): Backup? {
@@ -100,10 +96,7 @@ class GoogleDriveSyncService(context: Context, json: Json, syncPreferences: Sync
             pushSyncData(syncData)
             return syncData.backup
         } catch (e: Exception) {
-            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            logcat(LogPriority.ERROR, "SyncService") { "Error syncing: ${e.message}" }
-            logcat(LogPriority.ERROR, "SyncService") { "Error syncing: $message" }
-            notifier.showSyncError(message)
-            SyncFailureState.report(message)
             return null
         }
     }

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/SyncYomiSyncService.kt

@@ -3,7 +3,6 @@ package eu.kanade.tachiyomi.data.sync.service
 import android.content.Context
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
-import eu.kanade.tachiyomi.data.sync.SyncFailureState
 import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import eu.kanade.tachiyomi.network.GET
 import eu.kanade.tachiyomi.network.POST
@@ -88,11 +87,10 @@ class SyncYomiSyncService(
             if (success) {
                 reportSyncEvent(SyncEventStatus.SYNC_SUCCESS)
             } else {
-                val message = "Failed to push sync data"
+                reportSyncEvent(SyncEventStatus.SYNC_FAILED, "Failed to push sync data")
-                reportSyncEvent(SyncEventStatus.SYNC_FAILED, message)
+                // KMK -->
-                notifier.showSyncError(message)
-                SyncFailureState.report(message)
                 return null
+                // KMK <--
             }
 
             return finalSyncData.backup
@@ -101,11 +99,9 @@ class SyncYomiSyncService(
                 reportSyncEvent(SyncEventStatus.SYNC_CANCELLED, e.message)
                 throw e
             }
-            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            logcat(LogPriority.ERROR) { "Error syncing: ${e.message}" }
-            logcat(LogPriority.ERROR) { "Error syncing: $message" }
+            notifier.showSyncError(e.message)
-            notifier.showSyncError(message)
+            reportSyncEvent(SyncEventStatus.SYNC_ERROR, e.message)
-            SyncFailureState.report(message)
-            reportSyncEvent(SyncEventStatus.SYNC_ERROR, message)
             return null
         }
     }

app/src/main/java/eu/kanade/tachiyomi/data/sync/service/WebDavSyncService.kt

@@ -3,7 +3,6 @@ package eu.kanade.tachiyomi.data.sync.service
 import android.content.Context
 import eu.kanade.domain.sync.SyncPreferences
 import eu.kanade.tachiyomi.data.backup.models.Backup
-import eu.kanade.tachiyomi.data.sync.SyncFailureState
 import eu.kanade.tachiyomi.data.sync.SyncNotifier
 import eu.kanade.tachiyomi.network.GET
 import eu.kanade.tachiyomi.network.PUT
@@ -118,9 +117,7 @@ class WebDavSyncService(
             return finalSyncData.backup
         } catch (e: Exception) {
             xLogE("WebDAV sync error:", e)
-            val message = e.message ?: context.stringResource(MR.strings.unknown_error)
+            notifier.showSyncError(e.message)
-            notifier.showSyncError(message)
-            SyncFailureState.report(message)
             return null
         }
     }
@@ -185,7 +182,6 @@ class WebDavSyncService(
                     "Please retry syncing or check your WebDAV folder."
                 xLogW(message)
                 notifier.showSyncError(message)
-                SyncFailureState.report(message)
             }
             else -> {
                 val bodyStr = response.body.string()

i18n/src/commonMain/moko-resources/base/strings.xml

@@ -923,8 +923,6 @@
     <string name="notification_chapters_single_and_more">Chapter %1$s and %2$d more</string>
     <string name="notification_chapters_multiple">Chapters %1$s</string>
     <string name="notification_update_error">%1$d update(s) failed</string>
-    <string name="library_update_skipped_sync_error">Library update skipped: %1$s</string>
-    <string name="sync_failed_generic">Could not sync with the remote service</string>
     <string name="learn_more">Tap to learn more</string>
     <string name="notification_cover_update_failed">Failed to update cover</string>
     <string name="notification_first_add_to_library">Please add the entry to your library before doing this</string>