Merge pull request #20870 from wordpress-mobile/andy/sentry-issue-529…

…9933951 [Bug] Remove flow and use callbacks instead. I know, pain.
wordpress-mobile · May 29, 2024 · 04851b3 · 04851b3
2 parents 0254e9e + 3329d74
commit 04851b3
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 84 deletions.
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScanner.kt b/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScanner.kt
@@ -18,21 +18,20 @@ import androidx.compose.ui.platform.LocalLifecycleOwner
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.viewinterop.AndroidView
 import androidx.core.content.ContextCompat
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.flowOf
 import org.wordpress.android.ui.compose.theme.AppTheme
 import androidx.camera.core.Preview as CameraPreview
 
 @Composable
 fun BarcodeScanner(
     codeScanner: CodeScanner,
-    onScannedResult: (Flow<CodeScannerStatus>) -> Unit
+    onScannedResult: CodeScannerCallback
 ) {
     val context = LocalContext.current
     val lifecycleOwner = LocalLifecycleOwner.current
     val cameraProviderFuture = remember {
         ProcessCameraProvider.getInstance(context)
     }
+
     Column(
         modifier = Modifier.fillMaxSize()
     ) {
@@ -51,30 +50,27 @@ fun BarcodeScanner(
                     .setBackpressureStrategy(STRATEGY_KEEP_ONLY_LATEST)
                     .build()
                 imageAnalysis.setAnalyzer(ContextCompat.getMainExecutor(context)) { imageProxy ->
-                    onScannedResult(codeScanner.startScan(imageProxy))
+                    val callback = object : CodeScannerCallback {
+                        override fun run(status: CodeScannerStatus?) {
+                            status?.let { onScannedResult.run(it) }
+                        }
+                    }
+                    codeScanner.startScan(imageProxy, callback)
                 }
                 try {
                     cameraProviderFuture.get().bindToLifecycle(lifecycleOwner, selector, preview, imageAnalysis)
                 } catch (e: IllegalStateException) {
-                    onScannedResult(
-                        flowOf(
-                            CodeScannerStatus.Failure(
-                                e.message
-                                    ?: "Illegal state exception while binding camera provider to lifecycle",
-                                CodeScanningErrorType.Other(e)
-                            )
-                        )
-                    )
+                    onScannedResult.run(CodeScannerStatus.Failure(
+                        e.message
+                            ?: "Illegal state exception while binding camera provider to lifecycle",
+                        CodeScanningErrorType.Other(e)
+                    ))
                 } catch (e: IllegalArgumentException) {
-                    onScannedResult(
-                        flowOf(
-                            CodeScannerStatus.Failure(
-                                e.message
-                                    ?: "Illegal argument exception while binding camera provider to lifecycle",
-                                CodeScanningErrorType.Other(e)
-                            )
-                        )
-                    )
+                    onScannedResult.run(CodeScannerStatus.Failure(
+                        e.message
+                            ?: "Illegal argument exception while binding camera provider to lifecycle",
+                        CodeScanningErrorType.Other(e)
+                    ))
                 }
                 previewView
             },
@@ -84,8 +80,8 @@ fun BarcodeScanner(
 }
 
 class DummyCodeScanner : CodeScanner {
-    override fun startScan(imageProxy: ImageProxy): Flow<CodeScannerStatus> {
-        return flowOf(CodeScannerStatus.Success("", GoogleBarcodeFormatMapper.BarcodeFormat.FormatUPCA))
+    override fun startScan(imageProxy: ImageProxy, callback: CodeScannerCallback) {
+        callback.run(CodeScannerStatus.Success("", GoogleBarcodeFormatMapper.BarcodeFormat.FormatUPCA))
     }
 }
 
@@ -94,6 +90,10 @@ class DummyCodeScanner : CodeScanner {
 @Composable
 private fun BarcodeScannerScreenPreview() {
     AppTheme {
-        BarcodeScanner(codeScanner = DummyCodeScanner(), onScannedResult = {})
+        BarcodeScanner(codeScanner = DummyCodeScanner(), onScannedResult = object : CodeScannerCallback {
+            override fun run(status: CodeScannerStatus?) {
+                // no-ops
+            }
+        })
     }
 }
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScannerScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScannerScreen.kt
@@ -15,15 +15,14 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
 import org.wordpress.android.R
-import kotlinx.coroutines.flow.Flow
 import org.wordpress.android.ui.compose.theme.AppTheme
 
 @Composable
 fun BarcodeScannerScreen(
     codeScanner: CodeScanner,
     permissionState: BarcodeScanningViewModel.PermissionState,
     onResult: (Boolean) -> Unit,
-    onScannedResult: (Flow<CodeScannerStatus>) -> Unit,
+    onScannedResult: CodeScannerCallback,
 ) {
     val cameraPermissionLauncher = rememberLauncherForActivityResult(
         contract = ActivityResultContracts.RequestPermission(),

diff --git a/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScanningFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/BarcodeScanningFragment.kt
@@ -12,11 +12,7 @@ import androidx.core.os.bundleOf
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.setFragmentResult
 import androidx.fragment.app.viewModels
-import androidx.lifecycle.Lifecycle
-import androidx.lifecycle.lifecycleScope
-import androidx.lifecycle.repeatOnLifecycle
 import dagger.hilt.android.AndroidEntryPoint
-import kotlinx.coroutines.launch
 import org.wordpress.android.ui.compose.theme.AppTheme
 import org.wordpress.android.util.WPPermissionUtils
 import javax.inject.Inject
@@ -52,12 +48,10 @@ class BarcodeScanningFragment : Fragment() {
                                 shouldShowRequestPermissionRationale(KEY_CAMERA_PERMISSION)
                             )
                         },
-                        onScannedResult = { codeScannerStatus ->
-                            viewLifecycleOwner.lifecycleScope.launch {
-                                viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
-                                    codeScannerStatus.collect { status ->
-                                        setResultAndPopStack(status)
-                                    }
+                        onScannedResult = object : CodeScannerCallback {
+                            override fun run(status: CodeScannerStatus?) {
+                                if (status != null) {
+                                    setResultAndPopStack(status)
                                 }
                             }
                         },

diff --git a/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/CodeScanner.kt b/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/CodeScanner.kt
@@ -2,11 +2,14 @@ package org.wordpress.android.ui.barcodescanner
 
 import android.os.Parcelable
 import androidx.camera.core.ImageProxy
-import kotlinx.coroutines.flow.Flow
 import kotlinx.parcelize.Parcelize
 
 interface CodeScanner {
-    fun startScan(imageProxy: ImageProxy): Flow<CodeScannerStatus>
+    fun startScan(imageProxy: ImageProxy, callback: CodeScannerCallback)
+}
+
+interface CodeScannerCallback {
+    fun run(status: CodeScannerStatus?)
 }
 
 sealed class CodeScannerStatus : Parcelable {

diff --git a/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/GoogleMLKitCodeScanner.kt b/WordPress/src/main/java/org/wordpress/android/ui/barcodescanner/GoogleMLKitCodeScanner.kt
@@ -3,10 +3,6 @@ package org.wordpress.android.ui.barcodescanner
 import androidx.camera.core.ImageProxy
 import com.google.mlkit.vision.barcode.BarcodeScanner
 import com.google.mlkit.vision.barcode.common.Barcode
-import kotlinx.coroutines.channels.ProducerScope
-import kotlinx.coroutines.channels.awaitClose
-import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.callbackFlow
 import javax.inject.Inject
 
 class GoogleMLKitCodeScanner @Inject constructor(
@@ -17,53 +13,41 @@ class GoogleMLKitCodeScanner @Inject constructor(
 ) : CodeScanner {
     private var barcodeFound = false
     @androidx.camera.core.ExperimentalGetImage
-    override fun startScan(imageProxy: ImageProxy): Flow<CodeScannerStatus> {
-        return callbackFlow {
-            val barcodeTask = barcodeScanner.process(inputImageProvider.provideImage(imageProxy))
-            barcodeTask.addOnCompleteListener {
-                // We must call image.close() on received images when finished using them.
-                // Otherwise, new images may not be received or the camera may stall.
-                imageProxy.close()
-            }
-            barcodeTask.addOnSuccessListener { barcodeList ->
-                // The check for barcodeFound is done because the startScan method will be called
-                // continuously by the library as long as we are in the scanning screen.
-                // There will be a good chance that the same barcode gets identified multiple times and as a result
-                // success callback will be called multiple times.
-                if (!barcodeList.isNullOrEmpty() && !barcodeFound) {
-                    barcodeFound = true
-                    handleScanSuccess(barcodeList.firstOrNull())
-                    this@callbackFlow.close()
-                }
-            }
-            barcodeTask.addOnFailureListener { exception ->
-                this@callbackFlow.trySend(
-                    CodeScannerStatus.Failure(
-                        error = exception.message,
-                        type = errorMapper.mapGoogleMLKitScanningErrors(exception)
-                    )
-                )
-                this@callbackFlow.close()
+    override fun startScan(imageProxy: ImageProxy, callback: CodeScannerCallback) {
+        val barcodeTask = barcodeScanner.process(inputImageProvider.provideImage(imageProxy))
+        barcodeTask.addOnCompleteListener {
+            // We must call image.close() on received images when finished using them.
+            // Otherwise, new images may not be received or the camera may stall.
+            imageProxy.close()
+        }
+        barcodeTask.addOnSuccessListener { barcodeList ->
+            // The check for barcodeFound is done because the startScan method will be called
+            // continuously by the library as long as we are in the scanning screen.
+            // There will be a good chance that the same barcode gets identified multiple times and as a result
+            // success callback will be called multiple times.
+            if (!barcodeList.isNullOrEmpty() && !barcodeFound) {
+                barcodeFound = true
+                callback.run(handleScanSuccess(barcodeList.firstOrNull()))
             }
-
-            awaitClose()
+        }
+        barcodeTask.addOnFailureListener { exception ->
+            callback.run(CodeScannerStatus.Failure(
+                error = exception.message,
+                type = errorMapper.mapGoogleMLKitScanningErrors(exception)
+            ))
         }
     }
 
-    private fun ProducerScope<CodeScannerStatus>.handleScanSuccess(code: Barcode?) {
-        code?.rawValue?.let {
-            trySend(
-                CodeScannerStatus.Success(
-                    it,
-                    barcodeFormatMapper.mapBarcodeFormat(code.format)
-                )
+    private fun handleScanSuccess(code: Barcode?): CodeScannerStatus {
+        return code?.rawValue?.let {
+            CodeScannerStatus.Success(
+                it,
+                barcodeFormatMapper.mapBarcodeFormat(code.format)
             )
         } ?: run {
-            trySend(
-                CodeScannerStatus.Failure(
-                    error = "Failed to find a valid raw value!",
-                    type = CodeScanningErrorType.Other(Throwable("Empty raw value"))
-                )
+            CodeScannerStatus.Failure(
+                error = "Failed to find a valid raw value!",
+                type = CodeScanningErrorType.Other(Throwable("Empty raw value"))
             )
         }
     }