Fix for "Crash/stall when downloading PDF." Closes #111

Achieved by:
Changing the way the state for downloading attachment's items is being calculated and stored to decrease the odds of concurrent modification exception.
Optimizing the way AllItemsProcessor sends Ui updates to the screen so as to eliminate possibility of changing itemModels collection while the screen is being updated as this caused crashes.

Also Increasing maximum number of parallel network calls to 30 to make sure there is enough room for both sync and other operations at the same time.

Upping versionCode to 45

app/src/main/java/org/zotero/android/api/module/BaseApiModule.kt

@@ -4,6 +4,8 @@ import com.google.gson.Gson
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.migration.DisableInstallInCheck
+import okhttp3.ConnectionPool
+import okhttp3.Dispatcher
 import okhttp3.OkHttpClient
 import org.zotero.android.BuildConfig
 import org.zotero.android.api.ClientInfoNetworkInterceptor
@@ -13,8 +15,10 @@ import org.zotero.android.ktx.setNetworkTimeout
 import retrofit2.Retrofit
 import retrofit2.converter.gson.GsonConverterFactory
 import retrofit2.converter.scalars.ScalarsConverterFactory
+import java.util.concurrent.TimeUnit
 import javax.inject.Singleton
 
+
 @Module
 @DisableInstallInCheck
 object BaseApiModule {
@@ -26,7 +30,17 @@ object BaseApiModule {
         clientInfoNetworkInterceptor: ClientInfoNetworkInterceptor,
         configuration: NetworkConfiguration
     ): OkHttpClient {
+        val connectionPool = ConnectionPool(
+            maxIdleConnections = 10,
+            keepAliveDuration = 5,
+            timeUnit = TimeUnit.MINUTES
+        )
+        val dispatcher = Dispatcher()
+        dispatcher.maxRequests = 30
+        dispatcher.maxRequestsPerHost = 30
         return OkHttpClient.Builder()
+            .dispatcher(dispatcher)
+            .connectionPool(connectionPool)
             .setNetworkTimeout(configuration.networkTimeout)
             .addInterceptor(clientInfoNetworkInterceptor)
             .build()

app/src/main/java/org/zotero/android/screens/allitems/AllItemsScreen.kt

@@ -164,6 +164,7 @@ internal fun AllItemsScreen(
                             itemCellModels = viewState.itemCellModels,
                             isEditing = viewState.isEditing,
                             isItemSelected = viewState::isSelected,
+                            getItemAccessory = viewState::getAccessoryForItem,
                             onItemTapped = viewModel::onItemTapped,
                             onAccessoryTapped = viewModel::onAccessoryTapped,
                             onItemLongTapped = viewModel::onItemLongTapped

app/src/main/java/org/zotero/android/screens/allitems/AllItemsTable.kt

@@ -43,6 +43,7 @@ internal fun AllItemsTable(
     layoutType: CustomLayoutSize.LayoutType,
     itemCellModels: SnapshotStateList<ItemCellModel>,
     isItemSelected: (key: String) -> Boolean,
+    getItemAccessory: (itemKey: String) -> ItemCellModel.Accessory?,
     isEditing: Boolean,
     onItemTapped: (item: ItemCellModel) -> Unit,
     onItemLongTapped: (key: String) -> Unit,
@@ -57,6 +58,7 @@ internal fun AllItemsTable(
             Box(modifier = Modifier.animateItemPlacement()) {
                 ItemRow(
                     cellModel = item,
+                    itemAccessory = getItemAccessory(item.key),
                     layoutType = layoutType,
                     showBottomDivider = index != itemCellModels.size - 1,
                     isEditing = isEditing,
@@ -73,6 +75,7 @@ internal fun AllItemsTable(
 @Composable
 private fun ItemRow(
     cellModel: ItemCellModel,
+    itemAccessory: ItemCellModel.Accessory?,
     isItemSelected: (key: String) -> Boolean,
     layoutType: CustomLayoutSize.LayoutType,
     showBottomDivider: Boolean = false,
@@ -106,6 +109,7 @@ private fun ItemRow(
             ItemRowCentralPart(
                 model = cellModel,
                 isEditing = isEditing,
+                itemAccessory = itemAccessory,
                 onAccessoryTapped = onAccessoryTapped,
             )
         }
@@ -146,6 +150,7 @@ private fun ItemRowLeftPart(
 @Composable
 private fun ItemRowCentralPart(
     model: ItemCellModel,
+    itemAccessory: ItemCellModel.Accessory?,
     isEditing: Boolean,
     onAccessoryTapped: (key: String) -> Unit,
 ) {
@@ -194,6 +199,7 @@ private fun ItemRowCentralPart(
             Spacer(modifier = Modifier.width(8.dp))
             ItemRowRightPart(
                 model = model,
+                itemAccessory = itemAccessory,
                 isEditing = isEditing,
                 onAccessoryTapped = onAccessoryTapped,
             )
@@ -204,11 +210,12 @@ private fun ItemRowCentralPart(
 @Composable
 private fun RowScope.ItemRowRightPart(
     model: ItemCellModel,
+    itemAccessory: ItemCellModel.Accessory?,
     isEditing: Boolean,
     onAccessoryTapped: (key: String) -> Unit,
 ) {
     SetAccessory(
-        accessory = model.accessory,
+        accessory = itemAccessory,
     )
     AnimatedContent(
         modifier = Modifier.align(Alignment.CenterVertically),

app/src/main/java/org/zotero/android/screens/allitems/AllItemsViewModel.kt

@@ -3,7 +3,9 @@ package org.zotero.android.screens.allitems
 import android.net.Uri
 import android.webkit.MimeTypeMap
 import androidx.compose.runtime.mutableStateListOf
+import androidx.compose.runtime.mutableStateMapOf
 import androidx.compose.runtime.snapshots.SnapshotStateList
+import androidx.compose.runtime.snapshots.SnapshotStateMap
 import androidx.core.net.toFile
 import androidx.core.net.toUri
 import androidx.lifecycle.viewModelScope
@@ -323,12 +325,20 @@ internal class AllItemsViewModel @Inject constructor(
         }
     }
 
-    override fun updateItemCellModels(itemCellModels: SnapshotStateList<ItemCellModel>) {
+    override fun sendChangesToUi(
+        updatedItemCellModels: SnapshotStateList<ItemCellModel>?,
+        updatedDownloadingAccessories: SnapshotStateMap<String, ItemCellModel.Accessory?>?
+    ) {
         viewModelScope.launch {
             updateState {
-                copy(itemCellModels = itemCellModels)
+                copy(
+                    itemCellModels = updatedItemCellModels ?: viewState.itemCellModels,
+                    accessoryBeingDownloaded = updatedDownloadingAccessories
+                        ?: viewState.accessoryBeingDownloaded
+                )
             }
         }
+
     }
 
     override fun updateLCE(lce: LCE2) {
@@ -1025,6 +1035,7 @@ internal data class AllItemsViewState(
     val lce: LCE2 = LCE2.Content,
     val snackbarMessage: SnackbarMessage? = null,
     val itemCellModels: SnapshotStateList<ItemCellModel> = mutableStateListOf(),
+    val accessoryBeingDownloaded: SnapshotStateMap<String, ItemCellModel.Accessory?> = mutableStateMapOf(),
     val selectedKeys: PersistentSet<String>? = null,
     val error: ItemsError? = null,
     val shouldShowAddBottomSheet: Boolean = false,
@@ -1060,6 +1071,14 @@ internal data class AllItemsViewState(
         val size = itemCellModels.size
         return (selectedKeys?.size ?: 0) == size
     }
+
+    fun getAccessoryForItem(itemKey: String): ItemCellModel.Accessory? {
+        val beingDownloadedAccessory = accessoryBeingDownloaded[itemKey]
+        if (beingDownloadedAccessory != null) {
+            return beingDownloadedAccessory
+        }
+        return itemCellModels.firstOrNull { it.key == itemKey }?.accessory
+    }
 }
 
 internal sealed class AllItemsViewEffect : ViewEffect {

app/src/main/java/org/zotero/android/screens/allitems/processor/AllItemsProcessor.kt

@@ -1,6 +1,6 @@
 package org.zotero.android.screens.allitems.processor
 
-import androidx.compose.runtime.mutableStateListOf
+import androidx.compose.runtime.mutableStateMapOf
 import androidx.compose.runtime.toMutableStateList
 import io.realm.OrderedCollectionChangeSet
 import io.realm.OrderedRealmCollectionChangeListener
@@ -60,7 +60,7 @@ class AllItemsProcessor @Inject constructor(
 ) {
     private lateinit var processorInterface: AllItemsProcessorInterface
 
-    private var listenersCoroutineScope: CoroutineScope = CoroutineScope(dispatchers.default)
+    private var listenersCoroutineScope: CoroutineScope = CoroutineScope(dispatchers.io)
 
     private val limitedParallelismDispatcher =
         kotlinx.coroutines.Dispatchers.IO.limitedParallelism(1)
@@ -74,6 +74,7 @@ class AllItemsProcessor @Inject constructor(
     private var attachmentToOpen: String? = null
     private var sortType: ItemsSortType = ItemsSortType.default
     private var mutableItemCellModels: MutableList<ItemCellModel> = mutableListOf()
+    private var mutableDownloadingAcessories: MutableMap<String, ItemCellModel.Accessory?> = mutableMapOf()
 
     private val library: Library get() {
         return processorInterface.currentLibrary()
@@ -273,12 +274,24 @@ class AllItemsProcessor @Inject constructor(
                     this.downloadBatchData = batchData
                 }
 
+
+            }
+        }
+        when (update.kind) {
+            is AttachmentDownloader.Update.Kind.progress -> {
+                mutableDownloadingAcessories[updateKey] = cellAccessory(itemAccessories[updateKey])
+                sendChangedToUi(includeAccessories = true)
+            }
+            else -> {
+                mutableDownloadingAcessories.remove(updateKey)
+                updateCellAccessoryForItem(updateKey)
+                sendChangedToUi(includeItemCellModels = true, includeAccessories = true)
             }
         }
-        updateCellAccessoryForItem(updateKey)
-        sendItemCellModelsToUi()
     }
 
+
+
     private fun updateCellAccessoryForItem(updateKey: String) {
         val cellAccessory = cellAccessory(itemAccessories[updateKey])
         val index = mutableItemCellModels.indexOfFirst { it.key == updateKey }
@@ -366,8 +379,8 @@ class AllItemsProcessor @Inject constructor(
         removeAllListenersFromResultsList()
         this.results = results
 
-        mutableItemCellModels = mutableStateListOf()
-        sendItemCellModelsToUi()
+        mutableItemCellModels = mutableListOf()
+        sendChangedToUi(includeItemCellModels = true)
 
         startObservingResults()
         processorInterface.updateTagFilter()
@@ -377,10 +390,29 @@ class AllItemsProcessor @Inject constructor(
         this.results?.removeAllChangeListeners()
     }
 
-    private fun sendItemCellModelsToUi() {
-        processorInterface.updateItemCellModels(mutableItemCellModels.toMutableStateList())
-    }
+    private fun sendChangedToUi(includeItemCellModels: Boolean = false, includeAccessories: Boolean = false) {
+        val updatedItemCellModels =
+            if (includeItemCellModels) {
+                val copyOfItemCellModels = mutableItemCellModels.toList()
+                copyOfItemCellModels.toMutableStateList()
+            } else {
+                null
+            }
 
+        val updatedDownloadingAccessories = if (includeAccessories) {
+            val copyOfDownloadingAccessories = mutableDownloadingAcessories.toMap()
+           mutableStateMapOf<String, ItemCellModel.Accessory?>().apply {
+               putAll(copyOfDownloadingAccessories)
+           }
+        } else {
+            null
+        }
+
+        processorInterface.sendChangesToUi(
+            updatedItemCellModels = updatedItemCellModels,
+            updatedDownloadingAccessories = updatedDownloadingAccessories
+        )
+    }
     internal fun filter(
         searchTerm: String?,
         filters: List<ItemsFilter>,
@@ -451,7 +483,7 @@ class AllItemsProcessor @Inject constructor(
                 }
                 currentProcessingCount++
                 if (currentProcessingCount % updateThreshold == 0) {
-                    sendItemCellModelsToUi()
+                    sendChangedToUi(includeItemCellModels = true)
                 }
             }
             modifications.forEach { idx ->
@@ -472,14 +504,14 @@ class AllItemsProcessor @Inject constructor(
                 }
                 currentProcessingCount++
                 if (currentProcessingCount % updateThreshold == 0) {
-                    sendItemCellModelsToUi()
+                    sendChangedToUi(includeItemCellModels = true)
                 }
             }
             if (!isActive) {
                 return@launch
             }
 //            mutableItemCellModels = itemCellModelsToUpdate
-            sendItemCellModelsToUi()
+            sendChangedToUi(includeItemCellModels = true)
         }
 
     }
@@ -610,7 +642,7 @@ class AllItemsProcessor @Inject constructor(
             this@AllItemsProcessor.itemAccessories.put(key, itemAccessory)
         }
         updateCellAccessoryForItem(key)
-        sendItemCellModelsToUi()
+        sendChangedToUi(includeItemCellModels = true)
     }
 
     fun clear() {

app/src/main/java/org/zotero/android/screens/allitems/processor/AllItemsProcessorInterface.kt

@@ -1,6 +1,7 @@
 package org.zotero.android.screens.allitems.processor
 
 import androidx.compose.runtime.snapshots.SnapshotStateList
+import androidx.compose.runtime.snapshots.SnapshotStateMap
 import org.zotero.android.architecture.LCE2
 import org.zotero.android.database.objects.Attachment
 import org.zotero.android.screens.allitems.data.ItemCellModel
@@ -21,8 +22,11 @@ interface AllItemsProcessorInterface {
     fun updateTagFilter()
     fun isEditing(): Boolean
     fun showItemDetailWithDelay(creation: DetailType.creation)
-    fun updateItemCellModels(itemCellModels: SnapshotStateList<ItemCellModel>)
 
     fun updateLCE(lce: LCE2)
     fun showError(error: ItemsError)
+    fun sendChangesToUi(
+        updatedItemCellModels: SnapshotStateList<ItemCellModel>?,
+        updatedDownloadingAccessories: SnapshotStateMap<String, ItemCellModel.Accessory?>?
+    )
 }

buildSrc/src/main/kotlin/BuildConfig.kt

@@ -4,7 +4,7 @@ object BuildConfig {
     const val compileSdkVersion = 34
     const val targetSdk = 33
 
-    val versionCode = 44 // Must be updated on every build
+    val versionCode = 45 // Must be updated on every build
     val version = Version(
         major = 1,
         minor = 0,