Merge pull request #17210 from wordpress-mobile/analysis/wordpress-coroutines-warnings

ParaskP7 · web-flow · commit 6092a722182d · 2022-10-04T13:31:30.000+03:00
[Compile Warnings As Errors] WordPress Module - Resolve Coroutines Warnings
diff --git a/WordPress/src/main/java/org/wordpress/android/models/usecases/LocalCommentCacheUpdateHandler.kt b/WordPress/src/main/java/org/wordpress/android/models/usecases/LocalCommentCacheUpdateHandler.kt
@@ -1,5 +1,6 @@
 package org.wordpress.android.models.usecases
 
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.merge
 import org.wordpress.android.models.usecases.LocalCommentCacheUpdateUseCase.PropagateCommentsUpdateAction.UpdatedComments
 import javax.inject.Inject
@@ -11,6 +12,7 @@ class LocalCommentCacheUpdateHandler @Inject constructor(
 ) {
     private val useCases = listOf(localCommentCacheUpdateUseCase)
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     fun subscribe() = useCases.map { it.subscribe() }.merge()
 
     suspend fun requestCommentsUpdate() = localCommentCacheUpdateUseCase.manageAction(
diff --git a/WordPress/src/main/java/org/wordpress/android/models/usecases/UnifiedCommentsListHandler.kt b/WordPress/src/main/java/org/wordpress/android/models/usecases/UnifiedCommentsListHandler.kt
@@ -1,5 +1,6 @@
 package org.wordpress.android.models.usecases
 
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.merge
 import org.wordpress.android.models.usecases.BatchModerateCommentsUseCase.ModerateCommentsAction.OnModerateComments
 import org.wordpress.android.models.usecases.BatchModerateCommentsUseCase.Parameters.ModerateCommentsParameters
@@ -21,6 +22,7 @@ class UnifiedCommentsListHandler @Inject constructor(
 ) {
     private val useCases = listOf(paginateCommentsUseCase, batchModerationUseCase, moderationWithUndoUseCase)
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     fun subscribe() = useCases.map { it.subscribe() }.merge()
 
     suspend fun requestPage(parameters: GetPageParameters) = paginateCommentsUseCase.manageAction(
diff --git a/WordPress/src/main/java/org/wordpress/android/modules/ThreadModule.kt b/WordPress/src/main/java/org/wordpress/android/modules/ThreadModule.kt
@@ -10,49 +10,34 @@ import kotlinx.coroutines.Dispatchers
 import org.wordpress.android.util.helpers.Debouncer
 import javax.inject.Named
 
-@Deprecated(message = "Implement CoroutineScope interface and cancel all child coroutines in onCleared/onDestroy/..")
-const val UI_SCOPE = "UI_SCOPE"
-@Deprecated(message = "Implement CoroutineScope interface and cancel all child coroutines in onCleared/onDestroy/..")
-const val DEFAULT_SCOPE = "DEFAULT_SCOPE"
 const val APPLICATION_SCOPE = "APPLICATION_SCOPE"
+
 const val UI_THREAD = "UI_THREAD"
 const val BG_THREAD = "BG_THREAD"
 const val IO_THREAD = "IO_THREAD"
 
 @InstallIn(SingletonComponent::class)
 @Module
 class ThreadModule {
+    /* SCOPE */
+
     @Provides
-    @Named(UI_SCOPE)
-    fun provideUiScope(): CoroutineScope {
-        return CoroutineScope(Dispatchers.Main)
+    @Named(APPLICATION_SCOPE)
+    fun provideApplicationScope(): CoroutineScope {
+        return CoroutineScope(Dispatchers.Default)
     }
 
+    /* DISPATCHER */
+
     @Provides
     @Named(UI_THREAD)
     fun provideUiDispatcher(): CoroutineDispatcher {
         return Dispatchers.Main
     }
 
-    @Provides
-    @Named(DEFAULT_SCOPE)
-    @Deprecated(
-            message = "CoroutineScope should be provided by an object which implements CoroutineScope",
-            replaceWith = ReplaceWith("Inject dispatcher and implement CoroutineScope interface")
-    )
-    fun provideBackgroundScope(): CoroutineScope {
-        return CoroutineScope(Dispatchers.Default)
-    }
-
-    @Provides
-    @Named(APPLICATION_SCOPE)
-    fun provideApplicationScope(): CoroutineScope {
-        return CoroutineScope(Dispatchers.Default)
-    }
-
     @Provides
     @Named(BG_THREAD)
-    fun provideBackgroundDispatcher(): CoroutineDispatcher {
+    fun provideBgDispatcher(): CoroutineDispatcher {
         return Dispatchers.Default
     }
 
@@ -62,6 +47,8 @@ class ThreadModule {
         return Dispatchers.IO
     }
 
+    /* OTHER */
+
     @Provides
     fun provideDebouncer(): Debouncer {
         return Debouncer()
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt
@@ -12,6 +12,7 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.async
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.sync.Mutex
 import org.wordpress.android.analytics.AnalyticsTracker.Stat
 import org.wordpress.android.fluxc.Dispatcher
 import org.wordpress.android.fluxc.generated.UploadActionBuilder
@@ -64,6 +65,13 @@ class UploadStarter @Inject constructor(
 ) : CoroutineScope {
     private val job = Job()
 
+    /**
+     * When the app comes to foreground both `queueUploadFromAllSites` and `queueUploadFromSite` are invoked.
+     * The problem is that they can run in parallel and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return
+     * out-of-date result and a same post is added twice.
+     */
+    private val mutex = Mutex()
+
     override val coroutineContext: CoroutineContext get() = job + bgDispatcher
 
     /**
@@ -134,44 +142,44 @@ class UploadStarter @Inject constructor(
 
     /**
      * This is meant to be used by [checkConnectionAndUpload] only.
-     *
-     * The method needs to be synchronized from the following reasons. When the app comes to foreground both
-     * `queueUploadFromAllSites` and `queueUploadFromSite` are invoked. The problem is that they can run in parallel
-     * and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return out-of-date result and a same post is added
-     * twice.
      */
-    @Synchronized
     private suspend fun upload(site: SiteModel) = coroutineScope {
-        val posts = async { postStore.getPostsWithLocalChanges(site) }
-        val pages = async { pageStore.getPagesWithLocalChanges(site) }
-        val list = posts.await() + pages.await()
+        try {
+            mutex.lock()
+
+            val posts = async { postStore.getPostsWithLocalChanges(site) }
+            val pages = async { pageStore.getPagesWithLocalChanges(site) }
+            val list = posts.await() + pages.await()
 
-        list.asSequence()
-                .map { post ->
-                    val action = uploadActionUseCase.getAutoUploadAction(post, site)
-                    Pair(post, action)
-                }
-                .filter { (_, action) ->
-                    action != DO_NOTHING
-                }
-                .toList()
-                .forEach { (post, action) ->
-                    trackAutoUploadAction(action, post.status, post.isPage)
-                    AppLog.d(
-                            AppLog.T.POSTS,
-                            "UploadStarter for post (isPage: ${post.isPage}) title: ${post.title}, action: $action"
-                    )
-                    dispatcher.dispatch(
-                            UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction(
-                                    post
-                            )
-                    )
-                    uploadServiceFacade.uploadPost(
-                            context = context,
-                            post = post,
-                            trackAnalytics = false
-                    )
-                }
+            list.asSequence()
+                    .map { post ->
+                        val action = uploadActionUseCase.getAutoUploadAction(post, site)
+                        Pair(post, action)
+                    }
+                    .filter { (_, action) ->
+                        action != DO_NOTHING
+                    }
+                    .toList()
+                    .forEach { (post, action) ->
+                        trackAutoUploadAction(action, post.status, post.isPage)
+                        AppLog.d(
+                                AppLog.T.POSTS,
+                                "UploadStarter for post (isPage: ${post.isPage}) title: ${post.title}, action: $action"
+                        )
+                        dispatcher.dispatch(
+                                UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction(
+                                        post
+                                )
+                        )
+                        uploadServiceFacade.uploadPost(
+                                context = context,
+                                post = post,
+                                trackAnalytics = false
+                        )
+                    }
+        } finally {
+            mutex.unlock()
+        }
     }
 
     private fun trackAutoUploadAction(
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/utils/ConcurrentContinuationWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/utils/ConcurrentContinuationWrapper.kt
@@ -1,6 +1,7 @@
 package org.wordpress.android.ui.utils
 
 import kotlinx.coroutines.CancellableContinuation
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.suspendCancellableCoroutine
 
 class ConcurrentContinuationWrapper<T> : ContinuationWrapper<T> {
@@ -18,6 +19,7 @@ class ConcurrentContinuationWrapper<T> : ContinuationWrapper<T> {
         }
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     override fun continueWith(t: T) {
         continuationList.removeFirstOrNull()?.let {
             if (it.isActive) {
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpack/backup/download/usecases/GetBackupDownloadStatusUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpack/backup/download/usecases/GetBackupDownloadStatusUseCaseTest.kt
@@ -2,7 +2,6 @@ package org.wordpress.android.ui.jetpack.backup.download.usecases
 
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.whenever
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import kotlinx.coroutines.flow.toList
 import org.assertj.core.api.Assertions.assertThat
@@ -30,7 +29,6 @@ import java.util.Calendar
 import java.util.Date
 
 @InternalCoroutinesApi
-@ExperimentalCoroutinesApi
 class GetBackupDownloadStatusUseCaseTest : BaseUnitTest() {
     private lateinit var useCase: GetBackupDownloadStatusUseCase
     @Mock lateinit var networkUtilsWrapper: NetworkUtilsWrapper
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpack/restore/usecases/GetRestoreStatusUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpack/restore/usecases/GetRestoreStatusUseCaseTest.kt
@@ -2,7 +2,6 @@ package org.wordpress.android.ui.jetpack.restore.usecases
 
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.whenever
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import kotlinx.coroutines.flow.toList
 import org.assertj.core.api.Assertions.assertThat
@@ -41,7 +40,6 @@ private const val CURRENT_ENTRY = "current entry"
 private val PUBLISHED = Date()
 
 @InternalCoroutinesApi
-@ExperimentalCoroutinesApi
 @RunWith(MockitoJUnitRunner::class)
 class GetRestoreStatusUseCaseTest {
     private lateinit var useCase: GetRestoreStatusUseCase
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ScanViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ScanViewModelTest.kt
@@ -64,9 +64,9 @@ private const val ON_ENTER_SERVER_CREDS_MESSAGE_CLICKED_PARAM_POSITION = 7
 private const val TEST_SITE_ID = 1L
 private const val SERVER_CREDS_LINK = "${Constants.URL_JETPACK_SETTINGS}/$TEST_SITE_ID}"
 
-@ExperimentalCoroutinesApi
-@InternalCoroutinesApi
 @Suppress("LargeClass")
+@InternalCoroutinesApi
+@ExperimentalCoroutinesApi
 class ScanViewModelTest : BaseUnitTest() {
     @Rule
     @JvmField val coroutineScope = MainCoroutineScopeRule()
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/usecases/FetchFixThreatsStatusUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/usecases/FetchFixThreatsStatusUseCaseTest.kt
@@ -30,8 +30,8 @@ import org.wordpress.android.ui.jetpack.scan.usecases.FetchFixThreatsStatusUseCa
 import org.wordpress.android.ui.jetpack.scan.usecases.FetchFixThreatsStatusUseCase.FetchFixThreatsState.NotStarted
 import org.wordpress.android.util.NetworkUtilsWrapper
 
-@ExperimentalCoroutinesApi
 @InternalCoroutinesApi
+@ExperimentalCoroutinesApi
 class FetchFixThreatsStatusUseCaseTest : BaseUnitTest() {
     @Rule
     @JvmField val coroutineScope = MainCoroutineScopeRule()
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/usecases/FetchScanStateUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/usecases/FetchScanStateUseCaseTest.kt
@@ -5,7 +5,6 @@ import com.nhaarman.mockitokotlin2.anyOrNull
 import com.nhaarman.mockitokotlin2.times
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import kotlinx.coroutines.flow.toList
 import org.assertj.core.api.Assertions.assertThat
@@ -27,7 +26,6 @@ import org.wordpress.android.ui.jetpack.scan.usecases.FetchScanStateUseCase.Fetc
 import org.wordpress.android.ui.jetpack.scan.usecases.FetchScanStateUseCase.FetchScanState.Success
 import org.wordpress.android.util.NetworkUtilsWrapper
 
-@ExperimentalCoroutinesApi
 @InternalCoroutinesApi
 class FetchScanStateUseCaseTest : BaseUnitTest() {
     private lateinit var useCase: FetchScanStateUseCase
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mediapicker/MediaPickerViewModelTest.kt
@@ -694,7 +694,7 @@ class MediaPickerViewModelTest : BaseUnitTest() {
                 hasStoragePermissions = false
         )
 
-        assertThat(actions.poll()).isNull()
+        assertThat(actions.tryReceive().getOrNull()).isNull()
     }
 
     @Test
@@ -705,7 +705,7 @@ class MediaPickerViewModelTest : BaseUnitTest() {
                 hasStoragePermissions = true
         )
 
-        assertThat(actions.poll()).isEqualTo(LoadAction.Start(null))
+        assertThat(actions.tryReceive().getOrNull()).isEqualTo(LoadAction.Start(null))
     }
 
     @Test
@@ -716,7 +716,7 @@ class MediaPickerViewModelTest : BaseUnitTest() {
                 hasStoragePermissions = false
         )
 
-        assertThat(actions.poll()).isEqualTo(LoadAction.Start(null))
+        assertThat(actions.tryReceive().getOrNull()).isEqualTo(LoadAction.Start(null))
     }
 
     private fun selectItem(position: Int) {
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteSourceManagerTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteSourceManagerTest.kt
@@ -4,7 +4,6 @@ import androidx.lifecycle.MediatorLiveData
 import com.nhaarman.mockitokotlin2.times
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
@@ -33,7 +32,6 @@ import org.wordpress.android.ui.quickstart.QuickStartTracker
 
 const val SITE_LOCAL_ID = 1
 
-@ExperimentalCoroutinesApi
 @RunWith(MockitoJUnitRunner::class)
 class MySiteSourceManagerTest : BaseUnitTest() {
     @Mock lateinit var quickStartTracker: QuickStartTracker
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/mysite/MySiteViewModelTest.kt
@@ -17,7 +17,6 @@ import com.nhaarman.mockitokotlin2.times
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
@@ -150,7 +149,6 @@ import java.util.Date
 
 private const val DYNAMIC_CARDS_BUILDER_MORE_CLICK_PARAM_POSITION = 3
 
-@ExperimentalCoroutinesApi
 @Suppress("LargeClass")
 @RunWith(MockitoJUnitRunner::class)
 class MySiteViewModelTest : BaseUnitTest() {
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/qrcodeauth/QRCodeAuthViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/qrcodeauth/QRCodeAuthViewModelTest.kt
@@ -62,9 +62,9 @@ const val SCANNED_VALUE =
 const val VALID_EXPIRED_MESSAGE = "qr code data expired"
 const val INVALID_EXPIRED_MESSAGE = "invalid qr code data expired"
 
+@Suppress("LargeClass")
 @InternalCoroutinesApi
 @ExperimentalCoroutinesApi
-@Suppress("LargeClass")
 class QRCodeAuthViewModelTest : BaseUnitTest() {
     private lateinit var viewModel: QRCodeAuthViewModel
     @Mock lateinit var store: QRCodeAuthStore
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProviderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProviderTest.kt
@@ -4,6 +4,7 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule
 import androidx.lifecycle.Observer
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.InternalCoroutinesApi
 import org.assertj.core.api.Assertions
 import org.junit.Before
@@ -38,6 +39,7 @@ import org.wordpress.android.util.EventBusWrapper
 private const val NUMBER_OF_ITEMS = 10L
 
 @InternalCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(MockitoJUnitRunner::class)
 class ReaderDiscoverDataProviderTest {
     @Rule
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/usecases/ReaderFetchRelatedPostsUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/usecases/ReaderFetchRelatedPostsUseCaseTest.kt
@@ -23,12 +23,12 @@ import org.wordpress.android.ui.reader.usecases.ReaderFetchRelatedPostsUseCase.F
 import org.wordpress.android.util.NetworkUtilsWrapper
 
 @InternalCoroutinesApi
+@ExperimentalCoroutinesApi
 @RunWith(MockitoJUnitRunner::class)
 class ReaderFetchRelatedPostsUseCaseTest {
     @Rule
     @JvmField val rule = InstantTaskExecutorRule()
 
-    @ExperimentalCoroutinesApi
     @Rule
     @JvmField val coroutineScope = MainCoroutineScopeRule()
 
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/stories/StoriesIntroViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/stories/StoriesIntroViewModelTest.kt
diff --git a/WordPress/src/test/java/org/wordpress/android/util/SnackbarSequencerConcurrentTest.kt b/WordPress/src/test/java/org/wordpress/android/util/SnackbarSequencerConcurrentTest.kt
diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/main/WPMainActivityViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/main/WPMainActivityViewModelTest.kt
diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt
diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/whatsnew/FeatureAnnouncementViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/whatsnew/FeatureAnnouncementViewModelTest.kt
diff --git a/build.gradle b/build.gradle

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`package org.wordpress.android.ui.utils`
`2`	`2`
`3`	`3`	`import kotlinx.coroutines.CancellableContinuation`
	`4`	`+import kotlinx.coroutines.ExperimentalCoroutinesApi`
`4`	`5`	`import kotlinx.coroutines.suspendCancellableCoroutine`
`5`	`6`
`6`	`7`	`class ConcurrentContinuationWrapper<T> : ContinuationWrapper<T> {`
`@@ -18,6 +19,7 @@ class ConcurrentContinuationWrapper<T> : ContinuationWrapper<T> {`
`18`	`19`	`}`
`19`	`20`	`}`
`20`	`21`
	`22`	`+ @OptIn(ExperimentalCoroutinesApi::class)`
`21`	`23`	`override fun continueWith(t: T) {`
`22`	`24`	`continuationList.removeFirstOrNull()?.let {`
`23`	`25`	`if (it.isActive) {`