Fix app stream & cluster state issues

app/src/main/java/com/aurora/store/view/ui/commons/StreamBrowseFragment.kt

@@ -58,9 +58,9 @@ class StreamBrowseFragment : BaseFragment<FragmentGenericWithToolbarBinding>() {
             }
         })
 
-        viewModel.initCluster(streamCluster)
+        viewModel.seedCluster(streamCluster)
         viewModel.liveData.observe(viewLifecycleOwner) {
-            updateController(streamCluster)
+            updateController(it)
         }
     }
 

app/src/main/java/com/aurora/store/viewmodel/browse/StreamBrowseViewModel.kt

@@ -28,7 +28,6 @@ import com.aurora.gplayapi.helpers.web.WebStreamHelper
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.supervisorScope
 import javax.inject.Inject
 
 @HiltViewModel
@@ -42,31 +41,27 @@ class StreamBrowseViewModel @Inject constructor(
 
     private lateinit var streamCluster: StreamCluster
 
-    fun initCluster(cluster: StreamCluster) {
+    fun seedCluster(cluster: StreamCluster) {
         streamCluster = cluster
         liveData.postValue(streamCluster)
     }
 
     fun nextCluster() {
         viewModelScope.launch(Dispatchers.IO) {
-            supervisorScope {
-                try {
-                    if (streamCluster.hasNext()) {
-                        val nextCluster = streamHelper.nextStreamCluster(
-                            streamCluster.clusterNextPageUrl
-                        )
+            try {
+                if (streamCluster.hasNext()) {
+                    val next = streamHelper.nextStreamCluster(streamCluster.clusterNextPageUrl)
 
-                        streamCluster = streamCluster.copy(
-                            clusterNextPageUrl = nextCluster.clusterNextPageUrl,
-                            clusterAppList = streamCluster.clusterAppList + nextCluster.clusterAppList
-                        )
+                    streamCluster = streamCluster.copy(
+                        clusterNextPageUrl = next.clusterNextPageUrl,
+                        clusterAppList = streamCluster.clusterAppList + next.clusterAppList
+                    )
 
-                        liveData.postValue(streamCluster)
-                    } else {
-                        Log.i(TAG, "End of Cluster")
-                    }
-                } catch (_: Exception) {
+                    liveData.postValue(streamCluster)
+                } else {
+                    Log.i(TAG, "End of Cluster")
                 }
+            } catch (_: Exception) {
             }
         }
     }

app/src/main/java/com/aurora/store/viewmodel/details/DetailsClusterViewModel.kt

@@ -107,9 +107,9 @@ class DetailsClusterViewModel @Inject constructor(
                 clusterNextPageUrl = newCluster.clusterNextPageUrl,
                 clusterAppList = oldCluster.clusterAppList + newCluster.clusterAppList
             )
-            val newStreamClusters = bundle.streamClusters.toMutableMap().also {
-                it.remove(clusterID)
-                it[clusterID] = mergedCluster
+
+            val newStreamClusters = bundle.streamClusters.toMutableMap().apply {
+                this[clusterID] = mergedCluster
             }
 
             stash.put(url, bundle.copy(streamClusters = newStreamClusters))

app/src/main/java/com/aurora/store/viewmodel/details/DevProfileViewModel.kt

@@ -90,9 +90,9 @@ class DevProfileViewModel @Inject constructor(
                 clusterNextPageUrl = newCluster.clusterNextPageUrl,
                 clusterAppList = oldCluster.clusterAppList + newCluster.clusterAppList
             )
-            val newStreamClusters = streamBundle.streamClusters.toMutableMap().also {
-                it.remove(newCluster.id)
-                it[newCluster.id] = mergedCluster
+
+            val newStreamClusters = streamBundle.streamClusters.toMutableMap().apply {
+                this[newCluster.id] = mergedCluster
             }
 
             streamBundle = streamBundle.copy(streamClusters = newStreamClusters)

app/src/main/java/com/aurora/store/viewmodel/homestream/StreamViewModel.kt

@@ -32,7 +32,8 @@ import com.aurora.store.data.model.ViewState
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.supervisorScope
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import javax.inject.Inject
 
 @HiltViewModel
@@ -44,11 +45,14 @@ class StreamViewModel @Inject constructor(
 
     val liveData: MutableLiveData<ViewState> = MutableLiveData()
 
-    private var stash: HomeStash = mutableMapOf()
+    private val stash: HomeStash = mutableMapOf()
 
     private val streamContract: StreamContract
         get() = webStreamHelper
 
+    // Mutex to protect stash access for thread safety
+    private val stashMutex = Mutex()
+
     fun getStreamBundle(category: StreamContract.Category, type: StreamContract.Type) {
         liveData.postValue(ViewState.Loading)
         observe(category, type)
@@ -56,16 +60,18 @@ class StreamViewModel @Inject constructor(
 
     fun observe(category: StreamContract.Category, type: StreamContract.Type) {
         viewModelScope.launch(Dispatchers.IO) {
-            supervisorScope {
-                val bundle = targetBundle(category)
-                if (bundle.hasCluster()) {
-                    liveData.postValue(ViewState.Success(stash))
-                }
+            try {
+                stashMutex.withLock {
+                    val bundle = targetBundle(category)
+
+                    // Post existing data if any clusters exist
+                    if (bundle.hasCluster()) {
+                        liveData.postValue(ViewState.Success(stash.toMap()))
+                    }
 
-                try {
                     if (!bundle.hasCluster() || bundle.hasNext()) {
 
-                        //Fetch new stream bundle
+                        // Fetch new stream bundle
                         val newBundle = if (bundle.hasCluster()) {
                             streamContract.nextStreamBundle(
                                 category,
@@ -75,41 +81,43 @@ class StreamViewModel @Inject constructor(
                             streamContract.fetch(type, category)
                         }
 
-                        //Update old bundle
+                        // Update old bundle
                         val mergedBundle = bundle.copy(
                             streamClusters = bundle.streamClusters + newBundle.streamClusters,
                             streamNextPageUrl = newBundle.streamNextPageUrl
                         )
                         stash[category] = mergedBundle
 
-                        //Post updated to UI
-                        liveData.postValue(ViewState.Success(stash))
+                        // Post updated to UI
+                        liveData.postValue(ViewState.Success(stash.toMap()))
                     } else {
                         Log.i(TAG, "End of Bundle")
                     }
-                } catch (e: Exception) {
-                    liveData.postValue(ViewState.Error(e.message))
                 }
+            } catch (e: Exception) {
+                liveData.postValue(ViewState.Error(e.message))
             }
         }
     }
 
     fun observeCluster(category: StreamContract.Category, streamCluster: StreamCluster) {
         viewModelScope.launch(Dispatchers.IO) {
-            supervisorScope {
-                try {
-                    if (streamCluster.hasNext()) {
-                        val newCluster = streamContract.nextStreamCluster(
-                            streamCluster.clusterNextPageUrl
-                        )
+            try {
+                if (streamCluster.hasNext()) {
+                    val newCluster = streamContract.nextStreamCluster(
+                        streamCluster.clusterNextPageUrl
+                    )
+
+                    stashMutex.withLock {
                         updateCluster(category, streamCluster.id, newCluster)
-                        liveData.postValue(ViewState.Success(stash))
-                    } else {
-                        Log.i(TAG, "End of cluster")
                     }
-                } catch (e: Exception) {
-                    liveData.postValue(ViewState.Error(e.message))
+
+                    liveData.postValue(ViewState.Success(stash.toMap()))
+                } else {
+                    Log.i(TAG, "End of cluster ${streamCluster.id}")
                 }
+            } catch (e: Exception) {
+                liveData.postValue(ViewState.Error(e.message))
             }
         }
     }
@@ -119,26 +127,22 @@ class StreamViewModel @Inject constructor(
         clusterID: Int,
         newCluster: StreamCluster
     ) {
-        val bundle = targetBundle(category)
-        bundle.streamClusters[clusterID]?.let { oldCluster ->
-            val mergedCluster = oldCluster.copy(
-                clusterNextPageUrl = newCluster.clusterNextPageUrl,
-                clusterAppList = oldCluster.clusterAppList + newCluster.clusterAppList
-            )
-            val newStreamClusters = bundle.streamClusters.toMutableMap().also {
-                it.remove(clusterID)
-                it[clusterID] = mergedCluster
-            }
+        val bundle = stash[category] ?: return
+        val oldCluster = bundle.streamClusters[clusterID] ?: return
 
-            stash.put(category, bundle.copy(streamClusters = newStreamClusters))
+        val mergedCluster = oldCluster.copy(
+            clusterNextPageUrl = newCluster.clusterNextPageUrl,
+            clusterAppList = oldCluster.clusterAppList + newCluster.clusterAppList
+        )
+
+        val updatedClusters = bundle.streamClusters.toMutableMap().apply {
+            this[clusterID] = mergedCluster
         }
+
+        stash[category] = bundle.copy(streamClusters = updatedClusters)
     }
 
     private fun targetBundle(category: StreamContract.Category): StreamBundle {
-        val streamBundle = stash.getOrPut(category) {
-            StreamBundle()
-        }
-
-        return streamBundle
+        return stash.getOrPut(category) { StreamBundle() }
     }
 }

app/src/main/java/com/aurora/store/viewmodel/review/ReviewViewModel.kt

@@ -19,6 +19,7 @@
 
 package com.aurora.store.viewmodel.review
 
+import android.util.Log
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
@@ -28,13 +29,13 @@ import com.aurora.gplayapi.helpers.ReviewsHelper
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.supervisorScope
 import javax.inject.Inject
 
 @HiltViewModel
 class ReviewViewModel @Inject constructor(
     private val reviewsHelper: ReviewsHelper
 ) : ViewModel() {
+    val TAG = javaClass.simpleName
 
     val liveData: MutableLiveData<ReviewCluster> = MutableLiveData()
 
@@ -42,29 +43,29 @@ class ReviewViewModel @Inject constructor(
 
     fun fetchReview(packageName: String, filter: Review.Filter) {
         viewModelScope.launch(Dispatchers.IO) {
-            supervisorScope {
-                try {
-                    reviewsCluster = reviewsHelper.getReviews(packageName, filter)
-                    liveData.postValue(reviewsCluster)
-                } catch (_: Exception) {
-                }
+            try {
+                reviewsCluster = reviewsHelper.getReviews(packageName, filter)
+                liveData.postValue(reviewsCluster)
+            } catch (e: Exception) {
+                Log.e(TAG, "Failed to fetch reviews", e)
             }
         }
     }
 
     fun next(nextReviewPageUrl: String) {
         viewModelScope.launch(Dispatchers.IO) {
-            supervisorScope {
-                try {
-                    val nextReviewCluster = reviewsHelper.next(nextReviewPageUrl)
-                    reviewsCluster = reviewsCluster.copy(
-                        nextPageUrl = nextReviewCluster.nextPageUrl,
-                        reviewList = nextReviewCluster.reviewList + nextReviewCluster.reviewList
-                    )
+            try {
+                val currentCluster = reviewsCluster
+                val nextReviewCluster = reviewsHelper.next(nextReviewPageUrl)
 
-                    liveData.postValue(reviewsCluster)
-                } catch (_: Exception) {
-                }
+                reviewsCluster = currentCluster.copy(
+                    nextPageUrl = nextReviewCluster.nextPageUrl,
+                    reviewList = currentCluster.reviewList + nextReviewCluster.reviewList
+                )
+
+                liveData.postValue(reviewsCluster)
+            } catch (e: Exception) {
+                Log.e(TAG, "Failed to fetch next reviews $nextReviewPageUrl", e)
             }
         }
     }

app/src/main/java/com/aurora/store/viewmodel/subcategory/CategoryStreamViewModel.kt

@@ -121,9 +121,9 @@ class CategoryStreamViewModel @Inject constructor(
                 clusterNextPageUrl = newCluster.clusterNextPageUrl,
                 clusterAppList = oldCluster.clusterAppList + newCluster.clusterAppList
             )
-            val newStreamClusters = bundle.streamClusters.toMutableMap().also {
-                it.remove(clusterID)
-                it[clusterID] = mergedCluster
+
+            val newStreamClusters = bundle.streamClusters.toMutableMap().apply {
+                this[clusterID] = mergedCluster
             }
 
             stash.put(browseUrl, bundle.copy(streamClusters = newStreamClusters))