Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: Online library cannot be retrieved in the Play Store variant. #3994

Merged
merged 16 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fe2f4a3
Fixed: Online library cannot be retrieved in the Play Store variant.
MohitMaliDeveloper Sep 12, 2024
0278fa8
Added test cases to the CI for testing the minified version of the ap…
MohitMaliDeveloper Sep 13, 2024
8770efd
Improved the DownloadTest to handle cases where pausing the download …
MohitMaliDeveloper Sep 13, 2024
3ab0dcd
Fixed: `AndroidXTracer: java.lang.NoSuchMethodError: No static method…
MohitMaliDeveloper Sep 13, 2024
222e868
Added UI to show the progress on fetching online content.
MohitMaliDeveloper Sep 16, 2024
90f711b
Updated the detekt file to suppress the lint on ZimManageViewModel.
MohitMaliDeveloper Sep 16, 2024
c5d0d8a
Improved the design of showing the downloading progress of online lib…
MohitMaliDeveloper Sep 17, 2024
8ebeb4e
Improved the fetching and downloading the online library on low bandw…
MohitMaliDeveloper Sep 17, 2024
10837ae
Refactored our code to properly show the downloading online library p…
MohitMaliDeveloper Sep 18, 2024
318ae98
Getting the content-length on the background thread to free the UI tr…
MohitMaliDeveloper Sep 18, 2024
476733b
Fixed the ZimManageViewModelTest.
MohitMaliDeveloper Sep 18, 2024
93fad4d
Showing the progress below the online library downloading status text.
MohitMaliDeveloper Sep 18, 2024
098deb1
Increase the call timeout from 1 minute to 3 minutes. Since on slow b…
MohitMaliDeveloper Sep 18, 2024
61661a5
Fixed: Showing the progress update when no wifi is available.
MohitMaliDeveloper Sep 18, 2024
386a936
Hiding the refresh layout when no internet connection is available.
MohitMaliDeveloper Sep 19, 2024
b3cff85
Refactored the UI test cases according to this new change.
MohitMaliDeveloper Sep 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,25 @@ jobs:
emulator-boot-timeout: 1200
script: bash contrib/instrumentation.sh

- name: create instrumentation coverage for playStore variant
uses: reactivecircus/android-emulator-runner@v2
env:
GRADLE_OPTS: "-Dorg.gradle.internal.http.connectionTimeout=60000 -Dorg.gradle.internal.http.socketTimeout=60000 -Dorg.gradle.internal.network.retry.max.attempts=6 -Dorg.gradle.internal.network.retry.initial.backOff=2000"
with:
api-level: ${{ matrix.api-level }}
target: default
arch: x86_64
profile: pixel_2
ram-size: 3072M
cores: 4
emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
sdcard-path-or-size: 1024M
disable-animations: true
force-avd-creation: false
heap-size: 512M
emulator-boot-timeout: 1200
script: bash contrib/instrumentation-release.sh

- name: Test custom app
uses: reactivecircus/android-emulator-runner@v2
env:
Expand Down
12 changes: 12 additions & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import com.slack.keeper.optInToKeeper
import org.w3c.dom.Element
import plugin.KiwixConfigurationPlugin
import java.io.StringWriter
Expand All @@ -11,6 +12,9 @@ plugins {
android
id("com.github.triplet.play") version Versions.com_github_triplet_play_gradle_plugin
}
if (hasProperty("testingMinimizedBuild")) {
apply(plugin = "com.slack.keeper")
}
plugins.apply(KiwixConfigurationPlugin::class)

apply(from = rootProject.file("jacoco.gradle"))
Expand Down Expand Up @@ -93,6 +97,14 @@ play {
resolutionStrategy.set(com.github.triplet.gradle.androidpublisher.ResolutionStrategy.FAIL)
}

androidComponents {
beforeVariants { variantBuilder ->
if (variantBuilder.name == "debug" && hasProperty("testingMinimizedBuild")) {
variantBuilder.optInToKeeper()
}
}
}

dependencies {
androidTestImplementation(Libs.leakcanary_android_instrumentation)
testImplementation(Libs.kotlinx_coroutines_test)
Expand Down
2 changes: 1 addition & 1 deletion app/detekt_baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<ID>EmptyFunctionBlock:None.kt$None${ }</ID>
<ID>EmptyFunctionBlock:SimplePageChangeListener.kt$SimplePageChangeListener${ }</ID>
<ID>LongParameterList:ZimManageViewModel.kt$ZimManageViewModel$( booksOnFileSystem: List&lt;BookOnDisk>, activeDownloads: List&lt;DownloadModel>, allLanguages: List&lt;Language>, libraryNetworkEntity: LibraryNetworkEntity, filter: String, fileSystemState: FileSystemState )</ID>
<ID>LongParameterList:ZimManageViewModel.kt$ZimManageViewModel$( private val downloadDao: DownloadRoomDao, private val bookDao: NewBookDao, private val languageDao: NewLanguagesDao, private val storageObserver: StorageObserver, private val kiwixService: KiwixService, private val context: Application, private val connectivityBroadcastReceiver: ConnectivityBroadcastReceiver, private val bookUtils: BookUtils, private val fat32Checker: Fat32Checker, private val defaultLanguageProvider: DefaultLanguageProvider, private val dataSource: DataSource, private val connectivityManager: ConnectivityManager, private val sharedPreferenceUtil: SharedPreferenceUtil )</ID>
<ID>LongParameterList:ZimManageViewModel.kt$ZimManageViewModel$( private val downloadDao: DownloadRoomDao, private val bookDao: NewBookDao, private val languageDao: NewLanguagesDao, private val storageObserver: StorageObserver, private var kiwixService: KiwixService, val context: Application, private val connectivityBroadcastReceiver: ConnectivityBroadcastReceiver, private val bookUtils: BookUtils, private val fat32Checker: Fat32Checker, private val defaultLanguageProvider: DefaultLanguageProvider, private val dataSource: DataSource, private val connectivityManager: ConnectivityManager, private val sharedPreferenceUtil: SharedPreferenceUtil )</ID>
<ID>MagicNumber:LibraryListItem.kt$LibraryListItem.LibraryDownloadItem$1000L</ID>
<ID>MagicNumber:PeerGroupHandshake.kt$PeerGroupHandshake$15000</ID>
<ID>MagicNumber:ShareFiles.kt$ShareFiles$24</ID>
Expand Down
11 changes: 11 additions & 0 deletions app/proguard-rules.pro
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,20 @@

## keep everything in MetaLinkNetworkEntity.kt

-keep class org.kiwix.kiwixmobile.core.entity.MetaLinkNetworkEntity { *; }
-keep class org.kiwix.kiwixmobile.core.entity.MetaLinkNetworkEntity$* { *; }
-keepnames class org.kiwix.kiwixmobile.core.entity.MetaLinkNetworkEntity$*
-keepclassmembers class org.kiwix.kiwixmobile.core.entity.MetaLinkNetworkEntity$* {
<init>(...);
}

## keep everything in LibraryNetworkEntity.kt
-keep class org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity { *; }
-keep class org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity$* { *; }
-keepclassmembers class org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity$* {
<init>(...);
}

-keep class javax.xml.stream.** { *; }
-dontwarn javax.xml.stream.Location
-dontwarn javax.xml.stream.XMLEventReader
Expand Down Expand Up @@ -82,3 +91,5 @@
-dontwarn org.openjsse.javax.net.ssl.SSLParameters
-dontwarn org.openjsse.javax.net.ssl.SSLSocket
-dontwarn org.openjsse.net.ssl.OpenJSSE

-keep class io.reactivex.** { *; }
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withSubstring
import androidx.test.espresso.matcher.ViewMatchers.withText
import applyWithViewHierarchyPrinting
import com.adevinta.android.barista.interaction.BaristaSleepInteractions
Expand Down Expand Up @@ -76,7 +77,13 @@ class DownloadRobot : BaseRobot() {
onView(withText(string.swipe_down_for_library)).check(matches(isDisplayed()))
refreshOnlineList()
} catch (e: RuntimeException) {
// do nothing as the view is not visible
try {
// do nothing as currently downloading the online library.
onView(withId(R.id.onlineLibraryProgressLayout)).check(matches(isDisplayed()))
} catch (e: RuntimeException) {
// if not visible try to get the online library.
refreshOnlineList()
}
}
}

Expand Down Expand Up @@ -131,11 +138,7 @@ class DownloadRobot : BaseRobot() {
fun assertDownloadPaused() {
testFlakyView({
pauseForBetterTestPerformance()
onView(
withText(
org.kiwix.kiwixmobile.core.R.string.paused_state
)
).check(matches(isDisplayed()))
onView(withSubstring(context.getString(string.paused_state))).check(matches(isDisplayed()))
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,13 @@ class InitialDownloadRobot : BaseRobot() {
onView(withText(string.swipe_down_for_library)).check(matches(isDisplayed()))
refreshOnlineList()
} catch (e: RuntimeException) {
// do nothing as the view is not visible
try {
// do nothing as currently downloading the online library.
onView(withId(R.id.onlineLibraryProgressLayout)).check(matches(isDisplayed()))
} catch (e: RuntimeException) {
// if not visible try to get the online library.
refreshOnlineList()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,13 @@ class LanguageRobot : BaseRobot() {
onView(ViewMatchers.withText(string.swipe_down_for_library)).check(matches(isDisplayed()))
refreshOnlineList()
} catch (e: RuntimeException) {
// do nothing as the view is not visible
try {
// do nothing as currently downloading the online library.
onView(withId(R.id.onlineLibraryProgressLayout)).check(matches(isDisplayed()))
} catch (e: RuntimeException) {
// if not visible try to get the online library.
refreshOnlineList()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import androidx.core.content.ContextCompat
import androidx.core.view.MenuHost
import androidx.core.view.MenuProvider
import androidx.core.view.isVisible
import androidx.fragment.app.FragmentActivity
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.Observer
Expand Down Expand Up @@ -104,7 +105,7 @@
@Inject lateinit var availableSpaceCalculator: AvailableSpaceCalculator
@Inject lateinit var alertDialogShower: AlertDialogShower
private var fragmentDestinationDownloadBinding: FragmentDestinationDownloadBinding? = null

private val lock = Any()
private var downloadBookItem: LibraryListItem.BookItem? = null
private val zimManageViewModel by lazy {
requireActivity().viewModel<ZimManageViewModel>(viewModelFactory)
Expand Down Expand Up @@ -193,8 +194,10 @@
) {
if (it && !NetworkUtils.isWiFi(requireContext())) {
showInternetAccessViaMobileNetworkDialog()
hideProgressBarOfFetchingOnlineLibrary()

Check warning on line 197 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt#L197

Added line #L197 was not covered by tests
}
}
zimManageViewModel.downloadProgress.observe(viewLifecycleOwner, ::onLibraryStatusChanged)
setupMenu()

// hides keyboard when scrolled
Expand Down Expand Up @@ -247,13 +250,11 @@
dialogShower.show(
WifiOnly,
{
onRefreshStateChange(true)
showRecyclerviewAndHideSwipeDownForLibraryErrorText()
sharedPreferenceUtil.putPrefWifiOnly(false)
zimManageViewModel.shouldShowWifiOnlyDialog.value = false
},
{
onRefreshStateChange(false)
context.toast(
resources.getString(string.denied_internet_permission_message),
Toast.LENGTH_SHORT
Expand All @@ -268,6 +269,7 @@
libraryErrorText.visibility = View.GONE
libraryList.visibility = View.VISIBLE
}
showProgressBarOfFetchingOnlineLibrary()
}

private fun hideRecyclerviewAndShowSwipeDownForLibraryErrorText() {
Expand All @@ -278,6 +280,34 @@
libraryErrorText.visibility = View.VISIBLE
libraryList.visibility = View.GONE
}
hideProgressBarOfFetchingOnlineLibrary()
}

Check warning on line 284 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt#L283-L284

Added lines #L283 - L284 were not covered by tests

private fun showProgressBarOfFetchingOnlineLibrary() {
onRefreshStateChange(false)
fragmentDestinationDownloadBinding?.apply {
libraryErrorText.visibility = View.GONE
librarySwipeRefresh.isEnabled = false
onlineLibraryProgressLayout.visibility = View.VISIBLE
onlineLibraryProgressStatusText.setText(string.reaching_remote_library)
}
}

private fun hideProgressBarOfFetchingOnlineLibrary() {
onRefreshStateChange(false)
fragmentDestinationDownloadBinding?.apply {
librarySwipeRefresh.isEnabled = true
onlineLibraryProgressLayout.visibility = View.GONE
onlineLibraryProgressStatusText.setText(string.reaching_remote_library)
}
}

private fun onLibraryStatusChanged(libraryStatus: String) {
synchronized(lock) {
fragmentDestinationDownloadBinding?.apply {
onlineLibraryProgressStatusText.text = libraryStatus
}
}
}

override fun onDestroyView() {
Expand All @@ -293,18 +323,27 @@
}

private fun onRefreshStateChange(isRefreshing: Boolean?) {
fragmentDestinationDownloadBinding?.librarySwipeRefresh?.isRefreshing = isRefreshing == true
var refreshing = isRefreshing == true
// do not show the refreshing when the online library is downloading
if (fragmentDestinationDownloadBinding?.onlineLibraryProgressLayout?.isVisible == true ||
fragmentDestinationDownloadBinding?.libraryErrorText?.isVisible == true
) {
refreshing = false
}
fragmentDestinationDownloadBinding?.librarySwipeRefresh?.isRefreshing = refreshing
}

private fun onNetworkStateChange(networkState: NetworkState?) {
when (networkState) {
NetworkState.CONNECTED -> {
if (NetworkUtils.isWiFi(requireContext())) {
onRefreshStateChange(true)
refreshFragment()
} else if (noWifiWithWifiOnlyPreferenceSet) {
onRefreshStateChange(false)
hideRecyclerviewAndShowSwipeDownForLibraryErrorText()
} else if (!noWifiWithWifiOnlyPreferenceSet) {
if (libraryAdapter.items.isEmpty()) {
showProgressBarOfFetchingOnlineLibrary()

Check warning on line 345 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt#L345

Added line #L345 was not covered by tests
}
}
}

Expand All @@ -325,7 +364,7 @@
)
fragmentDestinationDownloadBinding?.libraryErrorText?.visibility = View.VISIBLE
}
fragmentDestinationDownloadBinding?.librarySwipeRefresh?.isRefreshing = false
hideProgressBarOfFetchingOnlineLibrary()

Check warning on line 367 in app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt#L367

Added line #L367 was not covered by tests
}

private fun noInternetSnackbar() {
Expand All @@ -345,6 +384,7 @@
if (it != null) {
libraryAdapter.items = it
}
hideProgressBarOfFetchingOnlineLibrary()
if (it?.isEmpty() == true) {
fragmentDestinationDownloadBinding?.libraryErrorText?.setText(
if (isNotConnected) string.no_network_connection
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Kiwix Android
* Copyright (c) 2024 Kiwix <android.kiwix.org>
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package org.kiwix.kiwixmobile.zimManager

import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.data.remote.OnlineLibraryProgressListener
import org.kiwix.kiwixmobile.core.downloader.downloadManager.DEFAULT_INT_VALUE
import org.kiwix.kiwixmobile.core.downloader.downloadManager.HUNDERED
import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO

class AppProgressListenerProvider(
private val zimManageViewModel: ZimManageViewModel
) : OnlineLibraryProgressListener {
@Suppress("MagicNumber")
override fun onProgress(bytesRead: Long, contentLength: Long) {
val progress =
if (contentLength == DEFAULT_INT_VALUE.toLong()) {
ZERO

Check warning on line 34 in app/src/main/java/org/kiwix/kiwixmobile/zimManager/AppProgressListenerProvider.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/zimManager/AppProgressListenerProvider.kt#L34

Added line #L34 was not covered by tests
} else {
(bytesRead * HUNDERED / contentLength).toInt() * 3
}
zimManageViewModel.downloadProgress.postValue(
zimManageViewModel.context.getString(
R.string.downloading_library,
zimManageViewModel.context.getString(R.string.percentage, progress)
)
)
}
}
Loading
Loading