Bumped Kotlin Coroutine to 1.3.7 and removed workaround when downlading translations / SN

Author: xizzhu

app/src/main/kotlin/me/xizzhu/android/joshua/core/repository/StrongNumberRepository.kt

@@ -67,8 +67,6 @@ class StrongNumberRepository(private val localStrongNumberStorage: LocalStrongNu
         Log.i(TAG, "Strong number saved to database")
         val installFinished = elapsedRealtime()
 
-        offer(101)
-
         Analytics.track(Analytics.EVENT_DOWNLOAD_STRONG_NUMBER, mapOf(
                 Pair(Analytics.PARAM_DOWNLOAD_TIME, downloadFinished - start),
                 Pair(Analytics.PARAM_INSTALL_TIME, installFinished - downloadFinished)

app/src/main/kotlin/me/xizzhu/android/joshua/core/repository/TranslationRepository.kt

@@ -168,8 +168,6 @@ class TranslationRepository(private val localTranslationStorage: LocalTranslatio
             return@synchronized Pair(available, downloaded)
         }
         notifyTranslationsUpdated(available, downloaded)
-
-        offer(101)
     }
 
     @VisibleForTesting

app/src/main/kotlin/me/xizzhu/android/joshua/reading/ReadingViewModel.kt

@@ -276,14 +276,7 @@ class ReadingViewModel(
 
     suspend fun readStrongNumber(verseIndex: VerseIndex): List<StrongNumber> = strongNumberManager.readStrongNumber(verseIndex)
 
-    fun downloadStrongNumber(): Flow<Int> =
-            strongNumberManager.download()
-                    .map { progress ->
-                        // Ideally, we should use onCompletion() to handle this. However, it doesn't
-                        // distinguish between a successful completion and a cancellation.
-                        // See https://github.com/Kotlin/kotlinx.coroutines/issues/1693
-                        if (progress <= 100) progress else -1
-                    }
+    fun downloadStrongNumber(): Flow<Int> = strongNumberManager.download()
 
     fun startTracking() {
         readingProgressManager.startTracking()

app/src/main/kotlin/me/xizzhu/android/joshua/reading/detail/VerseDetailPresenter.kt

@@ -119,13 +119,6 @@ class VerseDetailPresenter(
         downloadStrongNumberJob = viewModel.downloadStrongNumber()
                 .onEach { progress ->
                     when (progress) {
-                        -1 -> {
-                            activity.toast(R.string.toast_downloaded)
-                            verseDetail?.let {
-                                verseDetail = it.copy(strongNumberItems = viewModel.readStrongNumber(it.verseIndex).toStrongNumberItems())
-                                viewHolder.verseDetailViewLayout.setVerseDetail(verseDetail!!)
-                            }
-                        }
                         in 0 until 100 -> {
                             downloadStrongNumberDialog?.setProgress(progress)
                         }
@@ -140,10 +133,18 @@ class VerseDetailPresenter(
                     Log.e(tag, "Failed to download Strong's numberrs", e)
                     activity.dialog(true, R.string.dialog_download_error,
                             DialogInterface.OnClickListener { _, _ -> downloadStrongNumber() })
-                }.onCompletion {
+                }.onCompletion { e ->
                     downloadStrongNumberDialog?.dismiss()
                     downloadStrongNumberDialog = null
                     downloadStrongNumberJob = null
+
+                    if (e == null) {
+                        activity.toast(R.string.toast_downloaded)
+                        verseDetail?.let {
+                            verseDetail = it.copy(strongNumberItems = viewModel.readStrongNumber(it.verseIndex).toStrongNumberItems())
+                            viewHolder.verseDetailViewLayout.setVerseDetail(verseDetail!!)
+                        }
+                    }
                 }.launchIn(coroutineScope)
     }
 

app/src/main/kotlin/me/xizzhu/android/joshua/translations/TranslationListPresenter.kt

@@ -162,10 +162,6 @@ class TranslationListPresenter(
         downloadingJob = viewModel.downloadTranslation(translationToDownload)
                 .onEach { progress ->
                     when (progress) {
-                        -1 -> {
-                            activity.toast(R.string.toast_downloaded)
-                            loadTranslationList(false)
-                        }
                         in 0 until 100 -> {
                             downloadTranslationDialog?.setProgress(progress)
                         }
@@ -180,10 +176,15 @@ class TranslationListPresenter(
                     Log.e(tag, "Failed to download translation", e)
                     activity.dialog(true, R.string.dialog_download_error,
                             DialogInterface.OnClickListener { _, _ -> downloadTranslation(translationToDownload) })
-                }.onCompletion {
+                }.onCompletion { e ->
                     downloadTranslationDialog?.dismiss()
                     downloadTranslationDialog = null
                     downloadingJob = null
+
+                    if (e == null) {
+                        activity.toast(R.string.toast_downloaded)
+                        loadTranslationList(false)
+                    }
                 }.launchIn(coroutineScope)
     }
 

app/src/main/kotlin/me/xizzhu/android/joshua/translations/TranslationsViewModel.kt

@@ -45,12 +45,6 @@ class TranslationsViewModel(private val bibleReadingManager: BibleReadingManager
 
     fun downloadTranslation(translationToDownload: TranslationInfo): Flow<Int> =
             translationManager.downloadTranslation(translationToDownload)
-                    .map { progress ->
-                        // Ideally, we should use onCompletion() to handle this. However, it doesn't
-                        // distinguish between a successful completion and a cancellation.
-                        // See https://github.com/Kotlin/kotlinx.coroutines/issues/1693
-                        if (progress <= 100) progress else -1
-                    }
 
     fun removeTranslation(translationToRemove: TranslationInfo): Flow<Unit> = flow {
         emit(translationManager.removeTranslation(translationToRemove))

app/src/test/kotlin/me/xizzhu/android/joshua/core/repository/StrongNumberRepositoryTest.kt

@@ -56,8 +56,8 @@ class StrongNumberRepositoryTest : BaseUnitTest() {
         var called = false
         strongNumberRepository.download(versesDownloadProgress, wordsDownloadProgress)
                 .collect {
-                    assertTrue(it in (0..101))
-                    if (it == 101) called = true
+                    assertTrue(it in (0..100))
+                    if (it == 100) called = true
                 }
         assertTrue(called)
         assertTrue(versesDownloadProgress.isClosedForSend && versesDownloadProgress.isClosedForReceive)

app/src/test/kotlin/me/xizzhu/android/joshua/core/repository/TranslationRepositoryTest.kt

@@ -19,6 +19,7 @@ package me.xizzhu.android.joshua.core.repository
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.channels.consumeEach
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.toList
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.runBlockingTest
 import me.xizzhu.android.joshua.core.TranslationInfo
@@ -234,7 +235,7 @@ class TranslationRepositoryTest : BaseUnitTest() {
         assertTrue(translationRepository.downloadedTranslations().first().isEmpty())
         assertEquals(setOf(MockContents.cuvTranslationInfo, MockContents.kjvTranslationInfo), translationRepository.availableTranslations().first().toSet())
 
-        assertEquals(101, translationRepository.downloadTranslation(MockContents.kjvTranslationInfo).first { it == 101 })
+        assertTrue(translationRepository.downloadTranslation(MockContents.kjvTranslationInfo).toList().isEmpty())
         assertEquals(listOf(MockContents.cuvTranslationInfo), translationRepository.availableTranslations().first())
         assertEquals(listOf(MockContents.kjvDownloadedTranslationInfo), translationRepository.downloadedTranslations().first())
     }

app/src/test/kotlin/me/xizzhu/android/joshua/translations/TranslationsViewModelTest.kt

@@ -32,8 +32,10 @@ import kotlin.test.assertEquals
 class TranslationsViewModelTest : BaseUnitTest() {
     @Mock
     private lateinit var bibleReadingManager: BibleReadingManager
+
     @Mock
     private lateinit var translationManager: TranslationManager
+
     @Mock
     private lateinit var settingsManager: SettingsManager
 
@@ -104,11 +106,10 @@ class TranslationsViewModelTest : BaseUnitTest() {
     fun testDownloadTranslation() = testDispatcher.runBlockingTest {
         `when`(bibleReadingManager.currentTranslation()).thenReturn(flowOf(""))
         val translationToDownload = MockContents.kjvTranslationInfo
-        val progress = 89
-        `when`(translationManager.downloadTranslation(translationToDownload)).thenReturn(flowOf(progress, 101))
+        val progress = listOf(64, 89, 100)
+        `when`(translationManager.downloadTranslation(translationToDownload)).thenReturn(progress.asFlow())
 
-        assertEquals(listOf(progress, -1),
-                translationsViewModel.downloadTranslation(translationToDownload).toList())
+        assertEquals(progress, translationsViewModel.downloadTranslation(translationToDownload).toList())
         verify(translationManager, times(1)).downloadTranslation(translationToDownload)
     }
 

buildSrc/src/main/kotlin/Configurations.kt

@@ -49,7 +49,7 @@ object Versions {
     object Kotlin {
         const val jvmTarget = "1.8"
         const val core = "1.3.72"
-        const val coroutines = "1.3.5"
+        const val coroutines = "1.3.7"
     }
 
     object AndroidX {

docs/CHANGELOG.md

@@ -15,7 +15,7 @@ CHANGELOG
   - No longer needs ViewData
 - Updated dependencies:
   - Kotlin JVM target to 1.8
-  - Kotlin to 1.3.72, Coroutine to 1.3.5
+  - Kotlin to 1.3.72, Coroutine to 1.3.7
   - Gradle to 6.4.1
   - Android Gradle plugin to 3.6.3
   - Android build tools to 29.0.3