Add fallback strategy to fetching tags

Pinboard has been acting up, and from time to time the tags endpoint will not respond with the expected model. When that happens, fallback to the tags inferred from all bookmarks. Also, finally split the original `TagsDataSource` into `TagsDataSourcePinboardApi` and `TagsDataSourceNoApi` (same as the `PostsRepository`)
fibelatti · Nov 26, 2024 · 35f667d · 35f667d
1 parent ac0d97f
commit 35f667d
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 38 deletions.
diff --git a/...rc/main/kotlin/com/fibelatti/pinboard/features/linkding/data/TagsDataSourceLinkdingApi.kt b/...rc/main/kotlin/com/fibelatti/pinboard/features/linkding/data/TagsDataSourceLinkdingApi.kt
@@ -1,10 +1,13 @@
 package com.fibelatti.pinboard.features.linkding.data
 
+import com.fibelatti.core.functional.Failure
 import com.fibelatti.core.functional.Result
 import com.fibelatti.core.functional.Success
 import com.fibelatti.core.functional.getOrNull
 import com.fibelatti.core.functional.mapCatching
 import com.fibelatti.core.functional.onFailure
+import com.fibelatti.core.functional.onFailureReturn
+import com.fibelatti.core.functional.onSuccess
 import com.fibelatti.pinboard.core.android.ConnectivityInfoProvider
 import com.fibelatti.pinboard.core.functional.resultFrom
 import com.fibelatti.pinboard.core.network.resultFromNetwork
@@ -26,20 +29,24 @@ internal class TagsDataSourceLinkdingApi @Inject constructor(
     private var localTags: List<Tag>? = null
 
     override fun getAllTags(): Flow<Result<List<Tag>>> = flow {
+        localTags?.let { value -> emit(Success(value)) }
         emit(getLocalTags())
         if (connectivityInfoProvider.isConnected()) {
             emitAll(getRemoteTags().map(::Success))
         }
-    }.onEach { result -> result.getOrNull()?.let { localTags = it } }
+    }.onEach { result ->
+        result.onSuccess { value -> localTags = value }
+    }
 
-    private suspend fun getLocalTags(): Result<List<Tag>> = localTags?.let(::Success)
-        ?: resultFrom { bookmarksDao.getAllBookmarkTags() }.mapCatching { concatenatedTags ->
+    private suspend fun getLocalTags(): Result<List<Tag>> = resultFrom { bookmarksDao.getAllBookmarkTags() }
+        .mapCatching { concatenatedTags ->
             concatenatedTags
                 .flatMap { it.split(" ") }
                 .groupBy { it }
                 .map { (tag, postList) -> Tag(tag, postList.size) }
                 .sortedBy { it.name }
         }
+        .onFailureReturn(localTags.orEmpty())
 
     private fun getRemoteTags(): Flow<List<Tag>> = flow {
         val localTags = getLocalTags().getOrNull()?.associateBy { it.name }.orEmpty()
@@ -61,7 +68,6 @@ internal class TagsDataSourceLinkdingApi @Inject constructor(
     }
 
     override suspend fun renameTag(oldName: String, newName: String): Result<List<Tag>> {
-        // Linkding doesn't support renaming tags
-        return Success(emptyList())
+        return Failure(UnsupportedOperationException())
     }
 }
diff --git a/app/src/main/kotlin/com/fibelatti/pinboard/features/tags/data/TagsDataSourceNoApi.kt b/app/src/main/kotlin/com/fibelatti/pinboard/features/tags/data/TagsDataSourceNoApi.kt
@@ -0,0 +1,45 @@
+package com.fibelatti.pinboard.features.tags.data
+
+import com.fibelatti.core.functional.Failure
+import com.fibelatti.core.functional.Result
+import com.fibelatti.core.functional.Success
+import com.fibelatti.core.functional.getOrNull
+import com.fibelatti.core.functional.mapCatching
+import com.fibelatti.core.functional.onFailureReturn
+import com.fibelatti.core.functional.onSuccess
+import com.fibelatti.pinboard.core.functional.resultFrom
+import com.fibelatti.pinboard.features.posts.data.PostsDao
+import com.fibelatti.pinboard.features.tags.domain.TagsRepository
+import com.fibelatti.pinboard.features.tags.domain.model.Tag
+import javax.inject.Inject
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.flow.onEach
+
+internal class TagsDataSourceNoApi @Inject constructor(
+    private val postsDao: PostsDao,
+) : TagsRepository {
+
+    private var localTags: List<Tag>? = null
+
+    override fun getAllTags(): Flow<Result<List<Tag>>> = flow {
+        localTags?.let { value -> emit(Success(value)) }
+        emit(getLocalTags())
+    }.onEach { result ->
+        result.onSuccess { value -> localTags = value }
+    }
+
+    private suspend fun getLocalTags(): Result<List<Tag>> = resultFrom { postsDao.getAllPostTags() }
+        .mapCatching { concatenatedTags ->
+            concatenatedTags
+                .flatMap { it.split(" ") }
+                .groupBy { it }
+                .map { (tag, postList) -> Tag(tag, postList.size) }
+                .sortedBy { it.name }
+        }
+        .onFailureReturn(localTags.orEmpty())
+
+    override suspend fun renameTag(oldName: String, newName: String): Result<List<Tag>> {
+        return Failure(UnsupportedOperationException())
+    }
+}
diff --git a/...oard/features/tags/data/TagsDataSource.kt → ...es/tags/data/TagsDataSourcePinboardApi.kt b/...oard/features/tags/data/TagsDataSource.kt → ...es/tags/data/TagsDataSourcePinboardApi.kt
@@ -3,11 +3,10 @@ package com.fibelatti.pinboard.features.tags.data
 import com.fibelatti.core.functional.Failure
 import com.fibelatti.core.functional.Result
 import com.fibelatti.core.functional.Success
-import com.fibelatti.core.functional.getOrNull
 import com.fibelatti.core.functional.map
 import com.fibelatti.core.functional.mapCatching
-import com.fibelatti.pinboard.core.AppMode
-import com.fibelatti.pinboard.core.AppModeProvider
+import com.fibelatti.core.functional.onFailureReturn
+import com.fibelatti.core.functional.onSuccess
 import com.fibelatti.pinboard.core.android.ConnectivityInfoProvider
 import com.fibelatti.pinboard.core.functional.resultFrom
 import com.fibelatti.pinboard.core.network.ApiException
@@ -21,38 +20,41 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.onEach
 
-internal class TagsDataSource @Inject constructor(
+internal class TagsDataSourcePinboardApi @Inject constructor(
     private val tagsApi: TagsApi,
     private val postsDao: PostsDao,
     private val connectivityInfoProvider: ConnectivityInfoProvider,
-    private val appModeProvider: AppModeProvider,
 ) : TagsRepository {
 
     private var localTags: List<Tag>? = null
 
     override fun getAllTags(): Flow<Result<List<Tag>>> = flow {
+        localTags?.let { value -> emit(Success(value)) }
         emit(getLocalTags())
-        if (AppMode.PINBOARD == appModeProvider.appMode.value && connectivityInfoProvider.isConnected()) {
+        if (connectivityInfoProvider.isConnected()) {
             emit(getRemoteTags())
         }
-    }.onEach { result -> result.getOrNull()?.let { localTags = it } }
+    }.onEach { result ->
+        result.onSuccess { value -> localTags = value }
+    }
 
-    private suspend fun getLocalTags(): Result<List<Tag>> = localTags?.let(::Success)
-        ?: resultFrom { postsDao.getAllPostTags() }.mapCatching { concatenatedTags ->
+    private suspend fun getLocalTags(): Result<List<Tag>> = resultFrom { postsDao.getAllPostTags() }
+        .mapCatching { concatenatedTags ->
             concatenatedTags
                 .flatMap { it.split(" ") }
                 .groupBy { it }
                 .map { (tag, postList) -> Tag(tag, postList.size) }
                 .sortedBy { it.name }
         }
+        .onFailureReturn(localTags.orEmpty())
 
-    private suspend fun getRemoteTags(): Result<List<Tag>> = resultFromNetwork {
-        tagsApi.getTags()
-    }.mapCatching { tagsAndPostCount ->
-        tagsAndPostCount
-            .map { (tag, postCount) -> Tag(tag, postCount) }
-            .sortedBy { it.name }
-    }
+    private suspend fun getRemoteTags(): Result<List<Tag>> = resultFromNetwork { tagsApi.getTags() }
+        .mapCatching { tagsAndPostCount ->
+            tagsAndPostCount
+                .map { (tag, postCount) -> Tag(tag, postCount) }
+                .sortedBy { it.name }
+        }
+        .onFailureReturn(localTags.orEmpty())
 
     override suspend fun renameTag(
         oldName: String,

diff --git a/app/src/main/kotlin/com/fibelatti/pinboard/features/tags/data/TagsDataSourceProxy.kt b/app/src/main/kotlin/com/fibelatti/pinboard/features/tags/data/TagsDataSourceProxy.kt
@@ -10,26 +10,30 @@ import javax.inject.Inject
 import javax.inject.Provider
 import javax.inject.Singleton
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
 
 @Singleton
 internal class TagsDataSourceProxy @Inject constructor(
-    private val tagsDataSource: Provider<TagsDataSource>,
+    private val tagsDataSourcePinboardApi: Provider<TagsDataSourcePinboardApi>,
     private val tagsDataSourceLinkdingApi: Provider<TagsDataSourceLinkdingApi>,
+    private val tagsDataSourceNoApi: Provider<TagsDataSourceNoApi>,
     private val appModeProvider: AppModeProvider,
 ) : TagsRepository {
 
     private var currentAppMode: AppMode? = null
     private var currentRepository: TagsRepository? = null
 
     private val repository: TagsRepository
-        get() {
-            val appMode = appModeProvider.appMode.value
+        get() = runBlocking {
+            val appMode = appModeProvider.appMode.first { AppMode.UNSET != it }
 
-            return currentRepository?.takeIf { currentAppMode == appMode }
-                ?: if (AppMode.LINKDING == appModeProvider.appMode.value) {
-                    tagsDataSourceLinkdingApi.get()
-                } else {
-                    tagsDataSource.get()
+            currentRepository?.takeIf { currentAppMode == appMode }
+                ?: when (appMode) {
+                    AppMode.NO_API -> tagsDataSourceNoApi.get()
+                    AppMode.PINBOARD -> tagsDataSourcePinboardApi.get()
+                    AppMode.LINKDING -> tagsDataSourceLinkdingApi.get()
+                    AppMode.UNSET -> throw IllegalStateException()
                 }.also {
                     currentAppMode = appMode
                     currentRepository = it

diff --git a/.../features/tags/data/TagsDataSourceTest.kt → ...ags/data/TagsDataSourcePinboardApiTest.kt b/.../features/tags/data/TagsDataSourceTest.kt → ...ags/data/TagsDataSourcePinboardApiTest.kt
@@ -2,7 +2,6 @@ package com.fibelatti.pinboard.features.tags.data
 
 import com.fibelatti.core.functional.exceptionOrNull
 import com.fibelatti.core.functional.getOrNull
-import com.fibelatti.pinboard.core.AppMode
 import com.fibelatti.pinboard.core.android.ConnectivityInfoProvider
 import com.fibelatti.pinboard.core.network.ApiException
 import com.fibelatti.pinboard.features.posts.data.PostsDao
@@ -11,14 +10,13 @@ import com.google.common.truth.Truth.assertThat
 import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk
-import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.toList
 import kotlinx.coroutines.test.runTest
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 
-class TagsDataSourceTest {
+class TagsDataSourcePinboardApiTest {
 
     private val mockApi = mockk<TagsApi>()
     private val mockPostsDao = mockk<PostsDao> {
@@ -28,13 +26,10 @@ class TagsDataSourceTest {
         every { isConnected() } returns true
     }
 
-    private val dataSource = TagsDataSource(
+    private val dataSource = TagsDataSourcePinboardApi(
         tagsApi = mockApi,
         postsDao = mockPostsDao,
         connectivityInfoProvider = mockConnectivityInfoProvider,
-        appModeProvider = mockk {
-            every { appMode } returns MutableStateFlow(AppMode.PINBOARD)
-        },
     )
 
     @Nested
@@ -51,7 +46,7 @@ class TagsDataSourceTest {
             // THEN
             assertThat(result).hasSize(2)
             assertThat(result[0].getOrNull()).isEmpty()
-            assertThat(result[1].exceptionOrNull()).isInstanceOf(Exception::class.java)
+            assertThat(result[1].getOrNull()).isEmpty()
         }
 
         @Test
@@ -101,7 +96,7 @@ class TagsDataSourceTest {
 
             // THEN
             assertThat(result).hasSize(1)
-            assertThat(result.first().exceptionOrNull()).isInstanceOf(Exception::class.java)
+            assertThat(result.first().getOrNull()).isEmpty()
         }
 
         @Test