Avoid concurrency issues when reordering categories

Maybe fixes #8372

app/src/main/java/eu/kanade/domain/category/interactor/ReorderCategory.kt

@@ -5,25 +5,42 @@ import eu.kanade.domain.category.model.CategoryUpdate
 import eu.kanade.domain.category.repository.CategoryRepository
 import eu.kanade.tachiyomi.util.lang.withNonCancellableContext
 import eu.kanade.tachiyomi.util.system.logcat
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import logcat.LogPriority
+import java.util.Collections
 
 class ReorderCategory(
     private val categoryRepository: CategoryRepository,
 ) {
 
-    suspend fun await(categoryId: Long, newPosition: Int) = withNonCancellableContext {
+    private val mutex = Mutex()
-        val categories = categoryRepository.getAll().filterNot(Category::isSystemCategory)
 
-        val currentIndex = categories.indexOfFirst { it.id == categoryId }
+    suspend fun moveUp(category: Category): Result =
+        await(category, MoveTo.UP)
+
+    suspend fun moveDown(category: Category): Result =
+        await(category, MoveTo.DOWN)
+
+    private suspend fun await(category: Category, moveTo: MoveTo) = withNonCancellableContext {
+        mutex.withLock {
+            val categories = categoryRepository.getAll()
+                .filterNot(Category::isSystemCategory)
+                .toMutableList()
+
+            val newPosition = when (moveTo) {
+                MoveTo.UP -> category.order - 1
+                MoveTo.DOWN -> category.order + 1
+            }.toInt()
+
+            val currentIndex = categories.indexOfFirst { it.id == category.id }
             if (currentIndex == newPosition) {
                 return@withNonCancellableContext Result.Unchanged
             }
 
-        val reorderedCategories = categories.toMutableList()
+            Collections.swap(categories, currentIndex, newPosition)
-        val reorderedCategory = reorderedCategories.removeAt(currentIndex)
-        reorderedCategories.add(newPosition, reorderedCategory)
 
-        val updates = reorderedCategories.mapIndexed { index, category ->
+            val updates = categories.mapIndexed { index, category ->
                 CategoryUpdate(
                     id = category.id,
                     order = index.toLong(),
@@ -38,13 +55,16 @@ class ReorderCategory(
                 Result.InternalError(e)
             }
         }
-
+    }
-    suspend fun await(category: Category, newPosition: Long): Result =
-        await(category.id, newPosition.toInt())
 
     sealed class Result {
         object Success : Result()
         object Unchanged : Result()
         data class InternalError(val error: Throwable) : Result()
     }
+
+    private enum class MoveTo {
+        UP,
+        DOWN,
+    }
 }

app/src/main/java/eu/kanade/presentation/components/ChangeCategoryDialog.kt

@@ -96,8 +96,7 @@ fun ChangeCategoryDialog(
                         val index = selection.indexOf(it)
                         if (index != -1) {
                             val mutableList = selection.toMutableList()
-                            mutableList.removeAt(index)
+                            mutableList[index] = it.next()
-                            mutableList.add(index, it.next())
                             selection = mutableList.toList()
                         }
                     }

app/src/main/java/eu/kanade/presentation/components/DeleteLibraryMangaDialog.kt

@@ -64,8 +64,7 @@ fun DeleteLibraryMangaDialog(
                     val onCheck = {
                         val index = list.indexOf(state)
                         val mutableList = list.toMutableList()
-                        mutableList.removeAt(index)
+                        mutableList[index] = state.next() as CheckboxState.State<Int>
-                        mutableList.add(index, state.next() as CheckboxState.State<Int>)
                         list = mutableList.toList()
                     }
 

app/src/main/java/eu/kanade/tachiyomi/ui/category/CategoryScreenModel.kt

@@ -48,7 +48,7 @@ class CategoryScreenModel(
             when (createCategoryWithName.await(name)) {
                 is CreateCategoryWithName.Result.InternalError -> _events.send(CategoryEvent.InternalError)
                 CreateCategoryWithName.Result.NameAlreadyExistsError -> _events.send(CategoryEvent.CategoryWithNameAlreadyExists)
-                CreateCategoryWithName.Result.Success -> {}
+                else -> {}
             }
         }
     }
@@ -57,27 +57,25 @@ class CategoryScreenModel(
         coroutineScope.launch {
             when (deleteCategory.await(categoryId = categoryId)) {
                 is DeleteCategory.Result.InternalError -> _events.send(CategoryEvent.InternalError)
-                DeleteCategory.Result.Success -> {}
+                else -> {}
             }
         }
     }
 
     fun moveUp(category: Category) {
         coroutineScope.launch {
-            when (reorderCategory.await(category, category.order - 1)) {
+            when (reorderCategory.moveUp(category)) {
                 is ReorderCategory.Result.InternalError -> _events.send(CategoryEvent.InternalError)
-                ReorderCategory.Result.Success -> {}
+                else -> {}
-                ReorderCategory.Result.Unchanged -> {}
             }
         }
     }
 
     fun moveDown(category: Category) {
         coroutineScope.launch {
-            when (reorderCategory.await(category, category.order + 1)) {
+            when (reorderCategory.moveDown(category)) {
                 is ReorderCategory.Result.InternalError -> _events.send(CategoryEvent.InternalError)
-                ReorderCategory.Result.Success -> {}
+                else -> {}
-                ReorderCategory.Result.Unchanged -> {}
             }
         }
     }
@@ -87,7 +85,7 @@ class CategoryScreenModel(
             when (renameCategory.await(category, name)) {
                 is RenameCategory.Result.InternalError -> _events.send(CategoryEvent.InternalError)
                 RenameCategory.Result.NameAlreadyExistsError -> _events.send(CategoryEvent.CategoryWithNameAlreadyExists)
-                RenameCategory.Result.Success -> {}
+                else -> {}
             }
         }
     }