From 33bd3397e3100e2bfece658e8e54cdeeee892025 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 16 Oct 2024 16:30:05 +0530 Subject: [PATCH 01/16] Fixed: Move `canReadFile()` method to IO thread. * Moved the file readability check to the IO thread to prevent ANR. * Refactored the code to accommodate this change. --- .../destination/reader/KiwixReaderFragment.kt | 36 ++++++++++------- .../effects/OpenFileWithNavigation.kt | 26 ++++++++---- buildSrc/src/main/kotlin/Libs.kt | 3 ++ buildSrc/src/main/kotlin/Versions.kt | 2 + core/build.gradle.kts | 1 + core/detekt_baseline.xml | 2 +- .../kiwixmobile/core/dao/LibkiwixBookmarks.kt | 31 +++++++++----- .../kiwix/kiwixmobile/core/dao/NewBookDao.kt | 27 ++++++++----- .../kiwix/kiwixmobile/core/data/DataSource.kt | 6 ++- .../kiwix/kiwixmobile/core/data/Repository.kt | 40 +++++++++---------- .../kiwixmobile/core/error/ErrorActivity.kt | 12 ++++-- .../core/extensions/FileExtensions.kt | 6 +-- .../core/main/CoreReaderFragment.kt | 29 +++++++++----- .../core/main/MainRepositoryActions.kt | 4 +- .../core/reader/ZimReaderSource.kt | 4 +- .../core/settings/CorePrefsFragment.kt | 7 +++- .../core/utils/DonationDialogHandler.kt | 19 ++++++--- .../core/utils/dialog/RateDialogHandler.kt | 13 ++++-- 18 files changed, 169 insertions(+), 99 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index b1f2e655e9..15a7318374 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -31,7 +31,11 @@ import android.view.View.VISIBLE import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.drawerlayout.widget.DrawerLayout +import androidx.lifecycle.lifecycleScope import com.google.android.material.bottomnavigation.BottomNavigationView +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.R import org.kiwix.kiwixmobile.cachedComponent import org.kiwix.kiwixmobile.core.R.anim @@ -219,23 +223,27 @@ class KiwixReaderFragment : CoreReaderFragment() { ) { val settings = requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) - - if (zimReaderSource != null && zimReaderSource.canOpenInLibkiwix()) { - if (zimReaderContainer?.zimReaderSource == null) { - openZimFile(zimReaderSource) - Log.d( - TAG_KIWIX, - "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" - ) + lifecycleScope.launch { + val canOpenInLibkiwix = withContext(Dispatchers.IO) { + zimReaderSource?.canOpenInLibkiwix() + } + if (zimReaderSource != null && canOpenInLibkiwix == true) { + if (zimReaderContainer?.zimReaderSource == null) { + openZimFile(zimReaderSource) + Log.d( + TAG_KIWIX, + "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" + ) + } else { + zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + } } else { - zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + getCurrentWebView()?.snack(string.zim_not_opened) + exitBook() // hide the options for zim file to avoid unexpected UI behavior + return@launch // book not found so don't need to restore the tabs for this file } - } else { - getCurrentWebView()?.snack(string.zim_not_opened) - exitBook() // hide the options for zim file to avoid unexpected UI behavior - return // book not found so don't need to restore the tabs for this file + restoreTabs(zimArticles, zimPositions, currentTab) } - restoreTabs(zimArticles, zimPositions, currentTab) } @Throws(IllegalArgumentException::class) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt index 43eb6aed8f..5ecd169fc0 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt @@ -19,11 +19,16 @@ package org.kiwix.kiwixmobile.zimManager.fileselectView.effects import androidx.appcompat.app.AppCompatActivity +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.navigate import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem +import org.kiwix.kiwixmobile.main.KiwixMainActivity import org.kiwix.kiwixmobile.nav.destination.library.LocalLibraryFragmentDirections.actionNavigationLibraryToNavigationReader data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.BookOnDisk) : @@ -31,14 +36,19 @@ data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.Bo override fun invokeWith(activity: AppCompatActivity) { val zimReaderSource = bookOnDisk.zimReaderSource - if (!zimReaderSource.canOpenInLibkiwix()) { - activity.toast(R.string.error_file_not_found) - } else { - activity.navigate( - actionNavigationLibraryToNavigationReader().apply { - zimFileUri = zimReaderSource.toDatabase() - } - ) + (activity as KiwixMainActivity).lifecycleScope.launch { + val canOpenInLibkiwix = withContext(Dispatchers.IO) { + zimReaderSource.canOpenInLibkiwix() + } + if (!canOpenInLibkiwix) { + activity.toast(R.string.error_file_not_found) + } else { + activity.navigate( + actionNavigationLibraryToNavigationReader().apply { + zimFileUri = zimReaderSource.toDatabase() + } + ) + } } } } diff --git a/buildSrc/src/main/kotlin/Libs.kt b/buildSrc/src/main/kotlin/Libs.kt index 252ace60b0..5633c803b0 100644 --- a/buildSrc/src/main/kotlin/Libs.kt +++ b/buildSrc/src/main/kotlin/Libs.kt @@ -22,6 +22,9 @@ object Libs { "org.jetbrains.kotlinx:kotlinx-coroutines-android:" + Versions.org_jetbrains_kotlinx_kotlinx_coroutines + const val kotlinx_coroutines_rx3: String = + "org.jetbrains.kotlinx:kotlinx-coroutines-rx3:" + Versions.kotlinx_coroutines_rx3 + /** * /~https://github.com/Kotlin/kotlinx.coroutines */ diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index 63b40eeca2..aa59c85998 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -16,6 +16,8 @@ object Versions { const val org_jetbrains_kotlinx_kotlinx_coroutines: String = "1.8.1" + const val kotlinx_coroutines_rx3: String = "1.3.9" + const val androidx_test_espresso: String = "3.5.1" const val tracing: String = "1.1.0" diff --git a/core/build.gradle.kts b/core/build.gradle.kts index aeb3bec79a..20216805f3 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -61,5 +61,6 @@ dependencies { implementation(Libs.webkit) testImplementation(Libs.kotlinx_coroutines_test) implementation(Libs.kotlinx_coroutines_android) + implementation(Libs.kotlinx_coroutines_rx3) implementation(Libs.zxing) } diff --git a/core/detekt_baseline.xml b/core/detekt_baseline.xml index af8150f87b..552831001b 100644 --- a/core/detekt_baseline.xml +++ b/core/detekt_baseline.xml @@ -12,7 +12,7 @@ LongParameterList:MainMenu.kt$MainMenu$( private val activity: Activity, zimFileReader: ZimFileReader?, menu: Menu, webViews: MutableList<KiwixWebView>, urlIsValid: Boolean, disableReadAloud: Boolean = false, disableTabs: Boolean = false, private val menuClickListener: MenuClickListener ) LongParameterList:MainMenu.kt$MainMenu.Factory$( menu: Menu, webViews: MutableList<KiwixWebView>, urlIsValid: Boolean, menuClickListener: MenuClickListener, disableReadAloud: Boolean, disableTabs: Boolean ) LongParameterList:PageTestHelpers.kt$( bookmarkTitle: String = "bookmarkTitle", isSelected: Boolean = false, id: Long = 2, zimId: String = "zimId", zimName: String = "zimName", zimFilePath: String = "zimFilePath", bookmarkUrl: String = "bookmarkUrl", favicon: String = "favicon" ) - LongParameterList:Repository.kt$Repository$( @param:IO private val io: Scheduler, @param:MainThread private val mainThread: Scheduler, private val bookDao: NewBookDao, private val bookmarksDao: NewBookmarksDao, private val historyDao: HistoryDao, private val languageDao: NewLanguagesDao, private val recentSearchDao: NewRecentSearchDao, private val zimReaderContainer: ZimReaderContainer ) + LongParameterList:Repository.kt$Repository$( @param:IO private val ioThread: Scheduler, @param:MainThread private val mainThread: Scheduler, private val bookDao: NewBookDao, private val libkiwixBookmarks: LibkiwixBookmarks, private val historyRoomDao: HistoryRoomDao, private val notesRoomDao: NotesRoomDao, private val languageDao: NewLanguagesDao, private val recentSearchRoomDao: RecentSearchRoomDao, private val zimReaderContainer: ZimReaderContainer ) LongParameterList:ToolbarScrollingKiwixWebView.kt$ToolbarScrollingKiwixWebView$( context: Context, callback: WebViewCallback, attrs: AttributeSet, nonVideoView: ViewGroup, videoView: ViewGroup, webViewClient: CoreWebViewClient, private val toolbarView: View, private val bottomBarView: View, sharedPreferenceUtil: SharedPreferenceUtil, private val parentNavigationBar: View? = null ) MagicNumber:ArticleCount.kt$ArticleCount$3 MagicNumber:CompatFindActionModeCallback.kt$CompatFindActionModeCallback$100 diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt index bd909712ff..310f9ec298 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt @@ -29,10 +29,12 @@ import io.reactivex.subjects.BehaviorSubject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.rx3.rxSingle import org.kiwix.kiwixmobile.core.CoreApp import org.kiwix.kiwixmobile.core.DarkModeConfig import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isCustomApp +import org.kiwix.kiwixmobile.core.extensions.deleteFile import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.page.adapter.Page @@ -68,8 +70,13 @@ class LibkiwixBookmarks @Inject constructor( private var bookmarkList: List = arrayListOf() private var libraryBooksList: List = arrayListOf() + @Suppress("CheckResult") private val bookmarkListBehaviour: BehaviorSubject>? by lazy { - BehaviorSubject.createDefault(getBookmarksList()) + BehaviorSubject.create>().also { subject -> + rxSingle { getBookmarksList() } + .subscribeOn(io.reactivex.rxjava3.schedulers.Schedulers.io()) + .subscribe(subject::onNext, subject::onError) + } } private val bookmarksFolderPath: String by lazy { @@ -113,7 +120,7 @@ class LibkiwixBookmarks @Inject constructor( override fun deletePages(pagesToDelete: List) = deleteBookmarks(pagesToDelete as List) - fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List { + suspend fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List { return zimFileReader?.let { reader -> getBookmarksList() .filter { it.zimId == reader.id } @@ -134,7 +141,7 @@ class LibkiwixBookmarks @Inject constructor( * during data migration, where data is written to the file only once after all bookmarks * have been added to libkiwix to optimize the process. */ - fun saveBookmark( + suspend fun saveBookmark( libkiwixBookmarkItem: LibkiwixBookmarkItem, shouldWriteBookmarkToFile: Boolean = true ) { @@ -241,7 +248,7 @@ class LibkiwixBookmarks @Inject constructor( } @Suppress("ReturnCount") - private fun getBookmarksList(): List { + private suspend fun getBookmarksList(): List { if (!bookmarksChanged && bookmarkList.isNotEmpty()) { // No changes, return the cached data return bookmarkList.distinctBy(LibkiwixBookmarkItem::bookmarkUrl) @@ -290,7 +297,7 @@ class LibkiwixBookmarks @Inject constructor( } @Suppress("NestedBlockDepth") - private fun deleteDuplicateBookmarks() { + private suspend fun deleteDuplicateBookmarks() { bookmarkList.groupBy { it.bookmarkUrl to it.zimReaderSource } .filter { it.value.size > 1 } .forEach { (_, value) -> @@ -319,7 +326,7 @@ class LibkiwixBookmarks @Inject constructor( } } - private fun getZimFileReaderFromBookmark( + private suspend fun getZimFileReaderFromBookmark( bookmarkItem: LibkiwixBookmarkItem, coreApp: CoreApp ): ZimFileReader? { @@ -342,7 +349,7 @@ class LibkiwixBookmarks @Inject constructor( } } - private fun isBookMarkExist(libkiwixBookmarkItem: LibkiwixBookmarkItem): Boolean = + private suspend fun isBookMarkExist(libkiwixBookmarkItem: LibkiwixBookmarkItem): Boolean = getBookmarksList() .any { it.url == libkiwixBookmarkItem.bookmarkUrl && @@ -368,7 +375,11 @@ class LibkiwixBookmarks @Inject constructor( } private fun updateFlowableBookmarkList() { - bookmarkListBehaviour?.onNext(getBookmarksList()) + bookmarkListBehaviour?.let { subject -> + rxSingle { getBookmarksList() } + .subscribeOn(io.reactivex.rxjava3.schedulers.Schedulers.io()) + .subscribe(subject::onNext, subject::onError) + } } // Export the `bookmark.xml` file to the `Download/org.kiwix/` directory of internal storage. @@ -408,7 +419,7 @@ class LibkiwixBookmarks @Inject constructor( }.first { !it.isFileExist() } } - fun importBookmarks(bookmarkFile: File) { + suspend fun importBookmarks(bookmarkFile: File) { // Create a temporary library manager to import the bookmarks. val tempLibrary = Library() Manager(tempLibrary).apply { @@ -426,7 +437,7 @@ class LibkiwixBookmarks @Inject constructor( sharedPreferenceUtil.context.toast(R.string.bookmark_imported_message) if (bookmarkFile.exists()) { - bookmarkFile.delete() + bookmarkFile.deleteFile() } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt index 96fd199077..410848931e 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/NewBookDao.kt @@ -21,6 +21,7 @@ import io.objectbox.Box import io.objectbox.kotlin.inValues import io.objectbox.kotlin.query import io.objectbox.query.QueryBuilder +import kotlinx.coroutines.rx3.rxSingle import org.kiwix.kiwixmobile.core.dao.entities.BookOnDiskEntity import org.kiwix.kiwixmobile.core.dao.entities.BookOnDiskEntity_ import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity.Book @@ -31,23 +32,29 @@ import javax.inject.Inject class NewBookDao @Inject constructor(private val box: Box) { fun books() = box.asFlowable() - .map { books -> - books.map { bookOnDiskEntity -> - bookOnDiskEntity.file.let { file -> - // set zimReaderSource for previously saved books + .flatMap { books -> + io.reactivex.rxjava3.core.Flowable.fromIterable(books) + .flatMapSingle { bookOnDiskEntity -> + val file = bookOnDiskEntity.file val zimReaderSource = ZimReaderSource(file) - if (zimReaderSource.canOpenInLibkiwix()) { - bookOnDiskEntity.zimReaderSource = zimReaderSource - } + + rxSingle { zimReaderSource.canOpenInLibkiwix() } + .map { canOpen -> + if (canOpen) { + bookOnDiskEntity.zimReaderSource = zimReaderSource + } + bookOnDiskEntity + } + .onErrorReturn { bookOnDiskEntity } } - bookOnDiskEntity - } + .toList() + .toFlowable() } .doOnNext { removeBooksThatDoNotExist(it.toMutableList()) } .map { books -> books.filter { it.zimReaderSource.exists() } } .map { it.map(::BookOnDisk) } - fun getBooks() = box.all.map { bookOnDiskEntity -> + suspend fun getBooks() = box.all.map { bookOnDiskEntity -> bookOnDiskEntity.file.let { file -> // set zimReaderSource for previously saved books val zimReaderSource = ZimReaderSource(file) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt index 290d07efb4..0b897b0393 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt @@ -41,9 +41,11 @@ interface DataSource { fun deleteHistory(historyList: List): Completable fun clearHistory(): Completable fun getBookmarks(): Flowable> - fun getCurrentZimBookmarksUrl(): Single> + fun getCurrentZimBookmarksUrl(): io.reactivex.rxjava3.core.Single> + + fun saveBookmark(libkiwixBookmarkItem: LibkiwixBookmarkItem): + io.reactivex.rxjava3.core.Completable - fun saveBookmark(libkiwixBookmarkItem: LibkiwixBookmarkItem): Completable fun deleteBookmarks(bookmarks: List): Completable fun deleteBookmark(bookId: String, bookmarkUrl: String): Completable? fun booksOnDiskAsListItems(): Flowable> diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt index bca9ca6111..de4c7884d4 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt @@ -21,7 +21,8 @@ package org.kiwix.kiwixmobile.core.data import io.reactivex.Completable import io.reactivex.Flowable import io.reactivex.Scheduler -import io.reactivex.Single +import kotlinx.coroutines.rx3.rxCompletable +import kotlinx.coroutines.rx3.rxSingle import org.kiwix.kiwixmobile.core.dao.HistoryRoomDao import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks import org.kiwix.kiwixmobile.core.dao.NewBookDao @@ -47,10 +48,9 @@ import javax.inject.Singleton * A central repository of data which should provide the presenters with the required data. */ -@Suppress("LongParameterList") @Singleton class Repository @Inject internal constructor( - @param:IO private val io: Scheduler, + @param:IO private val ioThread: Scheduler, @param:MainThread private val mainThread: Scheduler, private val bookDao: NewBookDao, private val libkiwixBookmarks: LibkiwixBookmarks, @@ -64,7 +64,7 @@ class Repository @Inject internal constructor( override fun getLanguageCategorizedBooks() = booksOnDiskAsListItems() .first(emptyList()) - .subscribeOn(io) + .subscribeOn(ioThread) .observeOn(mainThread) override fun booksOnDiskAsListItems(): Flowable> = bookDao.books() @@ -91,60 +91,60 @@ class Repository @Inject internal constructor( override fun saveBooks(books: List) = Completable.fromAction { bookDao.insert(books) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun saveBook(book: BookOnDisk) = Completable.fromAction { bookDao.insert(listOf(book)) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun saveLanguages(languages: List) = Completable.fromAction { languageDao.insert(languages) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun saveHistory(history: HistoryItem) = Completable.fromAction { historyRoomDao.saveHistory(history) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun deleteHistory(historyList: List) = Completable.fromAction { historyRoomDao.deleteHistory(historyList.filterIsInstance(HistoryItem::class.java)) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun clearHistory() = Completable.fromAction { historyRoomDao.deleteAllHistory() recentSearchRoomDao.deleteSearchHistory() - }.subscribeOn(io) + }.subscribeOn(ioThread) override fun getBookmarks() = libkiwixBookmarks.bookmarks() as Flowable> override fun getCurrentZimBookmarksUrl() = - Single.just(libkiwixBookmarks.getCurrentZimBookmarksUrl(zimReaderContainer.zimFileReader)) - .subscribeOn(io) - .observeOn(mainThread) + rxSingle { + libkiwixBookmarks.getCurrentZimBookmarksUrl(zimReaderContainer.zimFileReader) + }.subscribeOn(io.reactivex.rxjava3.schedulers.Schedulers.io()) override fun saveBookmark(libkiwixBookmarkItem: LibkiwixBookmarkItem) = - Completable.fromAction { libkiwixBookmarks.saveBookmark(libkiwixBookmarkItem) } - .subscribeOn(io) + rxCompletable { libkiwixBookmarks.saveBookmark(libkiwixBookmarkItem) } + .subscribeOn(io.reactivex.rxjava3.schedulers.Schedulers.io()) override fun deleteBookmarks(bookmarks: List) = Completable.fromAction { libkiwixBookmarks.deleteBookmarks(bookmarks) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun deleteBookmark(bookId: String, bookmarkUrl: String): Completable? = Completable.fromAction { libkiwixBookmarks.deleteBookmark(bookId, bookmarkUrl) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun saveNote(noteListItem: NoteListItem): Completable = Completable.fromAction { notesRoomDao.saveNote(noteListItem) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun deleteNotes(noteList: List) = Completable.fromAction { notesRoomDao.deleteNotes(noteList) } - .subscribeOn(io) + .subscribeOn(ioThread) override fun deleteNote(noteTitle: String): Completable = Completable.fromAction { notesRoomDao.deleteNote(noteTitle) } - .subscribeOn(io) + .subscribeOn(ioThread) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/error/ErrorActivity.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/error/ErrorActivity.kt index f1002a074d..80c2404598 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/error/ErrorActivity.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/error/ErrorActivity.kt @@ -25,6 +25,8 @@ import android.os.Process import androidx.activity.result.contract.ActivityResultContracts import androidx.core.content.ContextCompat import androidx.core.content.FileProvider +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.CoreApp.Companion.coreComponent import org.kiwix.kiwixmobile.core.base.BaseActivity import org.kiwix.kiwixmobile.core.compat.CompatHelper.Companion.getPackageInformation @@ -87,7 +89,9 @@ open class ErrorActivity : BaseActivity() { private fun setupReportButton() { activityKiwixErrorBinding?.reportButton?.setOnClickListener { - sendEmailLauncher.launch(Intent.createChooser(emailIntent(), "Send email...")) + lifecycleScope.launch { + sendEmailLauncher.launch(Intent.createChooser(emailIntent(), "Send email...")) + } } } @@ -96,7 +100,7 @@ open class ErrorActivity : BaseActivity() { restartApp() } - private fun emailIntent(): Intent { + private suspend fun emailIntent(): Intent { val emailBody = buildBody() return Intent(Intent.ACTION_SEND).apply { type = "text/plain" @@ -122,7 +126,7 @@ open class ErrorActivity : BaseActivity() { } } - private fun buildBody(): String = """ + private suspend fun buildBody(): String = """ $initialBody ${if (activityKiwixErrorBinding?.allowCrash?.isChecked == true && exception != null) exceptionDetails() else ""} @@ -139,7 +143,7 @@ open class ErrorActivity : BaseActivity() { ${exception?.let(::toStackTraceString)} """.trimIndent() - private fun zimFiles(): String { + private suspend fun zimFiles(): String { val allZimFiles = bookDao.getBooks().joinToString { """ ${it.book.title}: diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/extensions/FileExtensions.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/extensions/FileExtensions.kt index a405c4a305..95c733ded9 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/extensions/FileExtensions.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/extensions/FileExtensions.kt @@ -41,10 +41,6 @@ fun File.totalSpace(): Long = runBlocking { } } -fun File.canReadFile(): Boolean = runBlocking { - withContext(Dispatchers.IO) { - canRead() - } -} +suspend fun File.canReadFile(): Boolean = withContext(Dispatchers.IO) { canRead() } suspend fun File.deleteFile(): Boolean = withContext(Dispatchers.IO) { delete() } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 6bf336600c..d9cf414def 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -80,6 +80,7 @@ import androidx.core.widget.ContentLoadingProgressBar import androidx.drawerlayout.widget.DrawerLayout import androidx.lifecycle.Lifecycle import androidx.lifecycle.Observer +import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView @@ -93,6 +94,9 @@ import io.reactivex.Flowable import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import io.reactivex.processors.BehaviorProcessor +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.json.JSONArray import org.json.JSONException import org.kiwix.kiwixmobile.core.BuildConfig @@ -1641,16 +1645,21 @@ abstract class CoreReaderFragment : fun openZimFile(zimReaderSource: ZimReaderSource, isCustomApp: Boolean = false) { if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE) || isCustomApp) { - if (zimReaderSource.canOpenInLibkiwix()) { - // Show content if there is `Open Library` button showing - // and we are opening the ZIM file - reopenBook() - openAndSetInContainer(zimReaderSource) - updateTitle() - } else { - exitBook() - Log.w(TAG_KIWIX, "ZIM file doesn't exist at " + zimReaderSource.toDatabase()) - requireActivity().toast(R.string.error_file_not_found, Toast.LENGTH_LONG) + lifecycleScope.launch { + val canOpenInLibkiwix = withContext(Dispatchers.IO) { + zimReaderSource.canOpenInLibkiwix() + } + if (canOpenInLibkiwix) { + // Show content if there is `Open Library` button showing + // and we are opening the ZIM file + reopenBook() + openAndSetInContainer(zimReaderSource) + updateTitle() + } else { + exitBook() + Log.w(TAG_KIWIX, "ZIM file doesn't exist at " + zimReaderSource.toDatabase()) + requireActivity().toast(R.string.error_file_not_found, Toast.LENGTH_LONG) + } } } else { this.zimReaderSource = zimReaderSource diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt index 0198c9fb51..5557c2b23c 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt @@ -17,13 +17,13 @@ */ package org.kiwix.kiwixmobile.core.main -import org.kiwix.kiwixmobile.core.utils.files.Log import io.reactivex.disposables.Disposable import org.kiwix.kiwixmobile.core.data.DataSource import org.kiwix.kiwixmobile.core.di.ActivityScope import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem import org.kiwix.kiwixmobile.core.page.history.adapter.HistoryListItem.HistoryItem import org.kiwix.kiwixmobile.core.page.notes.adapter.NoteListItem +import org.kiwix.kiwixmobile.core.utils.files.Log import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem.BookOnDisk import javax.inject.Inject @@ -32,7 +32,7 @@ private const val TAG = "MainPresenter" @ActivityScope class MainRepositoryActions @Inject constructor(private val dataSource: DataSource) { private var saveHistoryDisposable: Disposable? = null - private var saveBookmarkDisposable: Disposable? = null + private var saveBookmarkDisposable: io.reactivex.rxjava3.disposables.Disposable? = null private var saveNoteDisposable: Disposable? = null private var saveBookDisposable: Disposable? = null private var deleteNoteDisposable: Disposable? = null diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt index f52a80e9f3..dc0ff40ad8 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt @@ -62,7 +62,7 @@ class ZimReaderSource( } } - fun canOpenInLibkiwix(): Boolean { + suspend fun canOpenInLibkiwix(): Boolean { return when { file?.canReadFile() == true -> true assetFileDescriptorList?.get(0)?.parcelFileDescriptor?.fd @@ -72,7 +72,7 @@ class ZimReaderSource( } } - fun createArchive(): Archive? { + suspend fun createArchive(): Archive? { if (canOpenInLibkiwix()) { return when { file != null -> Archive(file.canonicalPath) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt index e1f16f5d3c..65638c765f 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt @@ -40,6 +40,9 @@ import androidx.preference.Preference import androidx.preference.PreferenceFragmentCompat import com.google.android.material.snackbar.Snackbar import eu.mhutti1.utils.storage.StorageDevice +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.CoreApp.Companion.coreComponent import org.kiwix.kiwixmobile.core.CoreApp.Companion.instance import org.kiwix.kiwixmobile.core.DarkModeConfig @@ -403,7 +406,9 @@ abstract class CorePrefsFragment : createTempFile(contentResolver.openInputStream(uri)).apply { if (isValidXmlFile(this)) { - libkiwixBookmarks?.importBookmarks(this) + CoroutineScope(Dispatchers.IO).launch { + libkiwixBookmarks?.importBookmarks(this@apply) + } } else { activity.toast( resources.getString(R.string.error_invalid_bookmark_file), diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt index 75e0dec317..8a03593f80 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt @@ -19,8 +19,11 @@ package org.kiwix.kiwixmobile.core.utils import android.app.Activity +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isCustomApp +import org.kiwix.kiwixmobile.core.main.CoreMainActivity import javax.inject.Inject const val THREE_DAYS_IN_MILLISECONDS = 3 * 24 * 60 * 60 * 1000L @@ -42,15 +45,19 @@ class DonationDialogHandler @Inject constructor( val currentMilliSeconds = System.currentTimeMillis() val lastPopupMillis = sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds val timeDifference = currentMilliSeconds - lastPopupMillis - if (shouldShowInitialPopup(lastPopupMillis) || timeDifference >= THREE_MONTHS_IN_MILLISECONDS) { - if (isZimFilesAvailableInLibrary() && isTimeToShowDonation(currentMilliSeconds)) { - showDonationDialogCallback?.showDonationDialog() - resetDonateLater() + (activity as CoreMainActivity).lifecycleScope.launch { + if (shouldShowInitialPopup(lastPopupMillis) || + timeDifference >= THREE_MONTHS_IN_MILLISECONDS + ) { + if (isZimFilesAvailableInLibrary() && isTimeToShowDonation(currentMilliSeconds)) { + showDonationDialogCallback?.showDonationDialog() + resetDonateLater() + } } } } - private fun shouldShowInitialPopup(lastPopupMillis: Long): Boolean = + private suspend fun shouldShowInitialPopup(lastPopupMillis: Long): Boolean = lastPopupMillis == 0L && isZimFilesAvailableInLibrary() private fun isTimeToShowDonation(currentMillis: Long): Boolean = @@ -63,7 +70,7 @@ class DonationDialogHandler @Inject constructor( return timeDifference >= THREE_DAYS_IN_MILLISECONDS } - fun isZimFilesAvailableInLibrary(): Boolean = + suspend fun isZimFilesAvailableInLibrary(): Boolean = if (activity.isCustomApp()) true else newBookDao.getBooks().isNotEmpty() fun updateLastDonationPopupShownTime() { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/RateDialogHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/RateDialogHandler.kt index 0f1cf3666f..70c68a342e 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/RateDialogHandler.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/dialog/RateDialogHandler.kt @@ -22,11 +22,14 @@ import android.content.ActivityNotFoundException import android.content.Intent import android.net.Uri import androidx.annotation.IdRes +import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.BuildConfig import org.kiwix.kiwixmobile.core.compat.CompatHelper.Companion.getPackageInformation import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.di.ActivityScope import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isCustomApp +import org.kiwix.kiwixmobile.core.main.CoreMainActivity import org.kiwix.kiwixmobile.core.utils.NetworkUtils import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import javax.inject.Inject @@ -67,18 +70,20 @@ class RateDialogHandler @Inject constructor( tempVisitCount = visitCounterPref?.count ?: 0 ++tempVisitCount visitCounterPref?.count = tempVisitCount - if (shouldShowRateDialog() && NetworkUtils.isNetworkAvailable(activity)) { - showRateDialog(iconResId) + (activity as CoreMainActivity).lifecycleScope.launch { + if (shouldShowRateDialog() && NetworkUtils.isNetworkAvailable(activity)) { + showRateDialog(iconResId) + } } } - private fun shouldShowRateDialog(): Boolean { + private suspend fun shouldShowRateDialog(): Boolean { return tempVisitCount >= VISITS_REQUIRED_TO_SHOW_RATE_DIALOG && visitCounterPref?.noThanksState == false && isTwoWeekPassed() && isZimFilesAvailableInLibrary() && !BuildConfig.DEBUG } - private fun isZimFilesAvailableInLibrary(): Boolean { + private suspend fun isZimFilesAvailableInLibrary(): Boolean { // If it is a custom app, return true since custom apps always have the ZIM file. if (activity.isCustomApp()) return true // For Kiwix app, check if there are ZIM files available in the library. From 13014db1a1ff215c639104a6702f1c6672bdc104 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 16 Oct 2024 17:59:17 +0530 Subject: [PATCH 02/16] Fixed EncodedUrlTest, LibkiwixBookmarkTest, MimeTypeTest, and ZimFileReaderWithSplittedZimFileTest. --- .../kiwixmobile/mimetype/MimeTypeTest.kt | 62 ++++---- .../page/bookmarks/LibkiwixBookmarkTest.kt | 9 +- .../kiwixmobile/reader/EncodedUrlTest.kt | 140 +++++++++--------- .../ZimFileReaderWithSplittedZimFileTest.kt | 24 +-- 4 files changed, 129 insertions(+), 106 deletions(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt index 44a0133d0e..8557776b2f 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/mimetype/MimeTypeTest.kt @@ -21,10 +21,12 @@ package org.kiwix.kiwixmobile.mimetype import androidx.core.content.ContextCompat import androidx.core.content.edit import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope import androidx.preference.PreferenceManager import androidx.test.core.app.ActivityScenario import androidx.test.platform.app.InstrumentationRegistry import androidx.test.uiautomator.UiDevice +import kotlinx.coroutines.launch import org.junit.After import org.junit.Assert import org.junit.Before @@ -87,37 +89,41 @@ class MimeTypeTest : BaseActivityTest() { } } val zimSource = ZimReaderSource(zimFile) - val archive = zimSource.createArchive() - val zimFileReader = ZimFileReader( - zimSource, - archive!!, - DarkModeConfig(SharedPreferenceUtil(context), context), - SuggestionSearcher(archive) - ) - zimFileReader.getRandomArticleUrl()?.let { - val mimeType = zimFileReader.getMimeTypeFromUrl(it) - if (mimeType?.contains("^([^ ]+).*$") == true || mimeType?.contains(";") == true) { - Assert.fail( - "Unable to get mime type from zim file. File = " + - " $zimFile and url of article = $it" + activityScenario.onActivity { + it.lifecycleScope.launch { + val archive = zimSource.createArchive() + val zimFileReader = ZimFileReader( + zimSource, + archive!!, + DarkModeConfig(SharedPreferenceUtil(context), context), + SuggestionSearcher(archive) + ) + zimFileReader.getRandomArticleUrl()?.let { randomArticle -> + val mimeType = zimFileReader.getMimeTypeFromUrl(randomArticle) + if (mimeType?.contains("^([^ ]+).*$") == true || mimeType?.contains(";") == true) { + Assert.fail( + "Unable to get mime type from zim file. File = " + + " $zimFile and url of article = $randomArticle" + ) + } + } ?: kotlin.run { + Assert.fail("Unable to get article from zim file $zimFile") + } + // test mimetypes for some actual url + Assert.assertEquals( + "text/html", + zimFileReader.getMimeTypeFromUrl("https://kiwix.app/A/index.html") ) + Assert.assertEquals( + "text/css", + zimFileReader.getMimeTypeFromUrl("https://kiwix.app/-/assets/style1.css") + ) + // test mimetype for invalid url + Assert.assertEquals(null, zimFileReader.getMimeTypeFromUrl("https://kiwix.app/A/test.html")) + // dispose the ZimFileReader + zimFileReader.dispose() } - } ?: kotlin.run { - Assert.fail("Unable to get article from zim file $zimFile") } - // test mimetypes for some actual url - Assert.assertEquals( - "text/html", - zimFileReader.getMimeTypeFromUrl("https://kiwix.app/A/index.html") - ) - Assert.assertEquals( - "text/css", - zimFileReader.getMimeTypeFromUrl("https://kiwix.app/-/assets/style1.css") - ) - // test mimetype for invalid url - Assert.assertEquals(null, zimFileReader.getMimeTypeFromUrl("https://kiwix.app/A/test.html")) - // dispose the ZimFileReader - zimFileReader.dispose() } @After diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt index 13e10d39fb..bb658f4e09 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt @@ -32,6 +32,9 @@ import androidx.test.uiautomator.UiDevice import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesCheck import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesViews import com.google.android.apps.common.testing.accessibility.framework.checks.TouchTargetSizeCheck +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import org.hamcrest.Matchers.allOf import org.junit.Before import org.junit.Rule @@ -186,8 +189,10 @@ class LibkiwixBookmarkTest : BaseActivityTest() { coreReaderFragment.zimReaderContainer?.zimFileReader?.favicon, coreReaderFragment.zimReaderContainer?.zimFileReader?.zimReaderSource ) - coreReaderFragment.libkiwixBookmarks?.saveBookmark(libkiwixItem).also { - bookmarkList.add(libkiwixItem) + CoroutineScope(Dispatchers.Main).launch { + coreReaderFragment.libkiwixBookmarks?.saveBookmark(libkiwixItem).also { + bookmarkList.add(libkiwixItem) + } } } bookmarks { diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt index ac249c6c5e..56caee240d 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/EncodedUrlTest.kt @@ -21,10 +21,12 @@ package org.kiwix.kiwixmobile.reader import androidx.core.content.ContextCompat import androidx.core.content.edit import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope import androidx.preference.PreferenceManager import androidx.test.core.app.ActivityScenario import androidx.test.platform.app.InstrumentationRegistry import androidx.test.uiautomator.UiDevice +import kotlinx.coroutines.launch import org.junit.After import org.junit.Assert import org.junit.Before @@ -94,74 +96,78 @@ class EncodedUrlTest : BaseActivityTest() { } } val zimReaderSource = ZimReaderSource(zimFile) - val archive = zimReaderSource.createArchive() - val zimFileReader = ZimFileReader( - zimReaderSource, - archive!!, - DarkModeConfig(SharedPreferenceUtil(context), context), - SuggestionSearcher(archive) - ) - val encodedUrls = arrayOf( - EncodedUrl( - "https://kiwix.app/foo/part%2520with%2520space/bar%253Fkey%253Dvalue", - "https://kiwix.app/foo/part%2520with%2520space/bar%253Fkey%253Dvalue" - ), - EncodedUrl( - "https://kiwix.app/foo/part%20with%20space/bar%3Fkey%3Dvalue", - "https://kiwix.app/foo/part%20with%20space/bar%3Fkey%3Dvalue" - ), - EncodedUrl( - "https://kiwix.app/foo%2Fpart%20with%20space%2Fbar%3Fkey%3Dvalue", - "https://kiwix.app/foo%2Fpart%20with%20space%2Fbar%3Fkey%3Dvalue" - ), - EncodedUrl( - "https://kiwix.app/foo/part%20with%20space/bar?key=value", - "https://kiwix.app/foo/part%20with%20space/bar?key=value" - ), - EncodedUrl( - "https://kiwix.app/foo/part/file%20with%20%3F%20and%20+?who=Chip%26Dale&quer=Is%20" + - "there%20any%20%2B%20here%3F", - "https://kiwix.app/foo/part/file%20with%20%3F%20and%20+?who=Chip%26Dale&quer" + - "=Is%20there%20any%20%2B%20here%3F" - ), - EncodedUrl( - "https://kiwix.app/foo/part/file%20with%20%253F%20and%20%2B%3Fwho%3DChip%2526Dale%26" + - "quer%3DIs%2520there%2520any%2520%252B%2520here%253F", - "https://kiwix.app/foo/part/file%20with%20%253F%20and%20%2B%3Fwho%3DChip" + - "%2526Dale%26quer%3DIs%2520there%2520any%2520%252B%2520here%253F" - ), - EncodedUrl( - "https://kiwix.app/foo/part/file%20with%20%3F%20and%20%2B%3Fwho%3DChip%26Dale%26" + - "question%3DIt%20there%20any%20%2B%20here%3F", - "https://kiwix.app/foo/part/file%20with%20%3F%20and%20%2B%3Fwho%3DChip%26" + - "Dale%26question%3DIt%20there%20any%20%2B%20here%3F" - ), - EncodedUrl( - "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2520there%2520any" + - "%2520%252B%2520here%253F", - "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2520there%2520" + - "any%2520%252B%2520here%253F" - ), - EncodedUrl( - "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2Bthere%2Bany%2B%252B%2Bhere%253F", - "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2Bthere%2Bany%2B%252B%2B" + - "here%253F" - ), - EncodedUrl( - "https://kiwix.app/%F0%9F%A5%B3%F0%9F%A5%B0%F0%9F%98%98%F0%9F%A4%A9%F0%9F%98%8D%F0%9F" + - "%A4%8D%F0%9F%8E%80%F0%9F%A7%B8%F0%9F%8C%B7%F0%9F%8D%AD", - "https://kiwix.app/%F0%9F%A5%B3%F0%9F%A5%B0%F0%9F%98%98%F0%9F%A4%A9%F0%9F%98%8D" + - "%F0%9F%A4%8D%F0%9F%8E%80%F0%9F%A7%B8%F0%9F%8C%B7%F0%9F%8D%AD" - ) - ) - encodedUrls.forEach { - Assert.assertEquals( - it.expectedUrl, - zimFileReader.getRedirect(it.url) - ) + activityScenario.onActivity { + it.lifecycleScope.launch { + val archive = zimReaderSource.createArchive() + val zimFileReader = ZimFileReader( + zimReaderSource, + archive!!, + DarkModeConfig(SharedPreferenceUtil(context), context), + SuggestionSearcher(archive) + ) + val encodedUrls = arrayOf( + EncodedUrl( + "https://kiwix.app/foo/part%2520with%2520space/bar%253Fkey%253Dvalue", + "https://kiwix.app/foo/part%2520with%2520space/bar%253Fkey%253Dvalue" + ), + EncodedUrl( + "https://kiwix.app/foo/part%20with%20space/bar%3Fkey%3Dvalue", + "https://kiwix.app/foo/part%20with%20space/bar%3Fkey%3Dvalue" + ), + EncodedUrl( + "https://kiwix.app/foo%2Fpart%20with%20space%2Fbar%3Fkey%3Dvalue", + "https://kiwix.app/foo%2Fpart%20with%20space%2Fbar%3Fkey%3Dvalue" + ), + EncodedUrl( + "https://kiwix.app/foo/part%20with%20space/bar?key=value", + "https://kiwix.app/foo/part%20with%20space/bar?key=value" + ), + EncodedUrl( + "https://kiwix.app/foo/part/file%20with%20%3F%20and%20+?who=Chip%26Dale&quer=Is%20" + + "there%20any%20%2B%20here%3F", + "https://kiwix.app/foo/part/file%20with%20%3F%20and%20+?who=Chip%26Dale&quer" + + "=Is%20there%20any%20%2B%20here%3F" + ), + EncodedUrl( + "https://kiwix.app/foo/part/file%20with%20%253F%20and%20%2B%3Fwho%3DChip%2526Dale%26" + + "quer%3DIs%2520there%2520any%2520%252B%2520here%253F", + "https://kiwix.app/foo/part/file%20with%20%253F%20and%20%2B%3Fwho%3DChip" + + "%2526Dale%26quer%3DIs%2520there%2520any%2520%252B%2520here%253F" + ), + EncodedUrl( + "https://kiwix.app/foo/part/file%20with%20%3F%20and%20%2B%3Fwho%3DChip%26Dale%26" + + "question%3DIt%20there%20any%20%2B%20here%3F", + "https://kiwix.app/foo/part/file%20with%20%3F%20and%20%2B%3Fwho%3DChip%26" + + "Dale%26question%3DIt%20there%20any%20%2B%20here%3F" + ), + EncodedUrl( + "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2520there%2520any" + + "%2520%252B%2520here%253F", + "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2520there%2520" + + "any%2520%252B%2520here%253F" + ), + EncodedUrl( + "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2Bthere%2Bany%2B%252B%2Bhere%253F", + "https://kiwix.app/foo/part/file%3Fquestion%3DIs%2Bthere%2Bany%2B%252B%2B" + + "here%253F" + ), + EncodedUrl( + "https://kiwix.app/%F0%9F%A5%B3%F0%9F%A5%B0%F0%9F%98%98%F0%9F%A4%A9%F0%9F%98%8D%F0%9F" + + "%A4%8D%F0%9F%8E%80%F0%9F%A7%B8%F0%9F%8C%B7%F0%9F%8D%AD", + "https://kiwix.app/%F0%9F%A5%B3%F0%9F%A5%B0%F0%9F%98%98%F0%9F%A4%A9%F0%9F%98%8D" + + "%F0%9F%A4%8D%F0%9F%8E%80%F0%9F%A7%B8%F0%9F%8C%B7%F0%9F%8D%AD" + ) + ) + encodedUrls.forEach { encodedUrl -> + Assert.assertEquals( + encodedUrl.expectedUrl, + zimFileReader.getRedirect(encodedUrl.url) + ) + } + // dispose the ZimFileReader + zimFileReader.dispose() + } } - // dispose the ZimFileReader - zimFileReader.dispose() } @After diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt index e1f6a80c00..06cca480d9 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/ZimFileReaderWithSplittedZimFileTest.kt @@ -22,6 +22,7 @@ import androidx.core.content.ContextCompat import androidx.core.content.edit import androidx.core.net.toUri import androidx.lifecycle.Lifecycle +import androidx.lifecycle.lifecycleScope import androidx.preference.PreferenceManager import androidx.test.core.app.ActivityScenario import androidx.test.espresso.accessibility.AccessibilityChecks @@ -32,6 +33,7 @@ import androidx.test.uiautomator.UiDevice import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesCheck import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesViews import com.google.android.apps.common.testing.accessibility.framework.checks.TouchTargetSizeCheck +import kotlinx.coroutines.launch import org.hamcrest.Matchers.allOf import org.junit.After import org.junit.Assert @@ -135,15 +137,19 @@ class ZimFileReaderWithSplittedZimFileTest : BaseActivityTest() { createAndGetSplitedZimFile(true)?.let { zimFile -> // test the articleCount and mediaCount of this zim file. val zimReaderSource = ZimReaderSource(zimFile) - val archive = zimReaderSource.createArchive() - val zimFileReader = ZimFileReader( - zimReaderSource, - archive!!, - DarkModeConfig(SharedPreferenceUtil(context), context), - SuggestionSearcher(archive) - ) - Assert.assertEquals(zimFileReader.mediaCount, 16) - Assert.assertEquals(zimFileReader.articleCount, 4) + activityScenario.onActivity { + it.lifecycleScope.launch { + val archive = zimReaderSource.createArchive() + val zimFileReader = ZimFileReader( + zimReaderSource, + archive!!, + DarkModeConfig(SharedPreferenceUtil(context), context), + SuggestionSearcher(archive) + ) + Assert.assertEquals(zimFileReader.mediaCount, 16) + Assert.assertEquals(zimFileReader.articleCount, 4) + } + } } ?: kotlin.run { // error in creating the zim file chunk fail("Couldn't create the zim file chunk") From 93a01e5c29676bae2b74aeda4bb9b25966a373b5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Wed, 16 Oct 2024 19:20:35 +0530 Subject: [PATCH 03/16] Fixed: NewBookDaoTest, and DonationDialogHandlerTest. * Improved the saving of bookmarks. --- .../kiwixmobile/core/dao/LibkiwixBookmarks.kt | 37 +++++------ .../bookmark/adapter/LibkiwixBookmarkItem.kt | 14 +++++ .../kiwixmobile/core/dao/NewBookDaoTest.kt | 11 +++- .../core/utils/DonationDialogHandlerTest.kt | 62 +++++++++++-------- 4 files changed, 74 insertions(+), 50 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt index 310f9ec298..4d864221fd 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt @@ -170,7 +170,7 @@ class LibkiwixBookmarks @Inject constructor( } } - fun addBookToLibrary(file: File? = null, archive: Archive? = null) { + suspend fun addBookToLibrary(file: File? = null, archive: Archive? = null) { try { bookmarksChanged = true val book = Book().apply { @@ -219,32 +219,31 @@ class LibkiwixBookmarks @Inject constructor( fun deleteBookmarks(bookmarks: List) { bookmarks.map { library.removeBookmark(it.zimId, it.bookmarkUrl) } .also { - writeBookMarksAndSaveLibraryToFile() - updateFlowableBookmarkList() + CoroutineScope(Dispatchers.IO).launch { + writeBookMarksAndSaveLibraryToFile() + updateFlowableBookmarkList() + } } } fun deleteBookmark(bookId: String, bookmarkUrl: String) { - library.removeBookmark(bookId, bookmarkUrl).also { - writeBookMarksAndSaveLibraryToFile() - updateFlowableBookmarkList() - } + deleteBookmarks(listOf(LibkiwixBookmarkItem(zimId = bookId, bookmarkUrl = bookmarkUrl))) } /** * Asynchronously writes the library and bookmarks data to their respective files in a background thread * to prevent potential data loss and ensures that the library holds the updated ZIM file paths and favicons. */ - private fun writeBookMarksAndSaveLibraryToFile() { - CoroutineScope(Dispatchers.IO).launch { - // Save the library, which contains ZIM file paths and favicons, to a file. - library.writeToFile(libraryFile.canonicalPath) + private suspend fun writeBookMarksAndSaveLibraryToFile() { + // Save the library, which contains ZIM file paths and favicons, to a file. + library.writeToFile(libraryFile.canonicalPath) - // Save the bookmarks data to a separate file. - library.writeBookmarksToFile(bookmarkFile.canonicalPath) - } + // Save the bookmarks data to a separate file. + library.writeBookmarksToFile(bookmarkFile.canonicalPath) // set the bookmark change to true so that libkiwix will return the new data. - bookmarksChanged = true + CoroutineScope(Dispatchers.Main).launch { + bookmarksChanged = true + } } @Suppress("ReturnCount") @@ -374,12 +373,8 @@ class LibkiwixBookmarks @Inject constructor( }, backpressureStrategy) } - private fun updateFlowableBookmarkList() { - bookmarkListBehaviour?.let { subject -> - rxSingle { getBookmarksList() } - .subscribeOn(io.reactivex.rxjava3.schedulers.Schedulers.io()) - .subscribe(subject::onNext, subject::onError) - } + private suspend fun updateFlowableBookmarkList() { + bookmarkListBehaviour?.onNext(getBookmarksList()) } // Export the `bookmark.xml` file to the `Download/org.kiwix/` directory of internal storage. diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt index efe2835fc6..e703908701 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt @@ -84,4 +84,18 @@ data class LibkiwixBookmarkItem( favicon = bookmarkEntity.favicon, libKiwixBook = libkiwixBook ) + + constructor( + zimId: String, + bookmarkUrl: String + ) : this( + zimId = zimId, + zimFilePath = null, + zimReaderSource = null, + zimName = "", + bookmarkUrl = bookmarkUrl, + title = "", + favicon = null, + libKiwixBook = null + ) } diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt index d2f4888bcb..eb729213b4 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt @@ -31,6 +31,7 @@ import io.objectbox.query.Query import io.objectbox.query.QueryBuilder import io.objectbox.rx.RxQuery import io.reactivex.Observable +import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested @@ -61,7 +62,10 @@ internal class NewBookDaoTest { @Test fun `books emits entities whose file exists`() { val (expectedEntity, _) = expectEmissionOfExistingAndNotExistingBook() - newBookDao.books().test().assertValues(listOf(BookOnDisk(expectedEntity))) + val books = newBookDao.books().test().also { + it.awaitTerminalEvent() + } + books.assertValues(listOf(BookOnDisk(expectedEntity))) } @SuppressLint("CheckResult") @@ -92,10 +96,11 @@ internal class NewBookDaoTest { } @Test - fun getBooks() { + fun getBooks() = runTest { val entity = bookOnDiskEntity() every { box.all } returns mutableListOf(entity) - assertThat(newBookDao.getBooks()).isEqualTo(listOf(BookOnDisk(entity))) + val result = newBookDao.getBooks() + assertThat(result).isEqualTo(listOf(BookOnDisk(entity))) } @Nested diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt index 6cfa469eeb..b4cebfd120 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt @@ -18,40 +18,48 @@ package org.kiwix.kiwixmobile.core.utils -import android.app.Activity import io.mockk.Runs +import io.mockk.coEvery import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions +import org.kiwix.kiwixmobile.core.main.CoreMainActivity class DonationDialogHandlerTest { - private lateinit var activity: Activity + private lateinit var coreMainActivity: CoreMainActivity private lateinit var sharedPreferenceUtil: SharedPreferenceUtil private lateinit var newBookDao: NewBookDao private lateinit var donationDialogHandler: DonationDialogHandler private lateinit var showDonationDialogCallback: DonationDialogHandler.ShowDonationDialogCallback + private val testDispatcher = StandardTestDispatcher() + private val testScope = TestScope(testDispatcher) @BeforeEach fun setup() { - activity = mockk(relaxed = true) + coreMainActivity = mockk(relaxed = true) + // every { coreMainActivity.lifecycle } returns testScope.coroutineContext sharedPreferenceUtil = mockk(relaxed = true) newBookDao = mockk(relaxed = true) showDonationDialogCallback = mockk(relaxed = true) - donationDialogHandler = DonationDialogHandler(activity, sharedPreferenceUtil, newBookDao) + donationDialogHandler = + DonationDialogHandler(coreMainActivity, sharedPreferenceUtil, newBookDao) donationDialogHandler.setDonationDialogCallBack(showDonationDialogCallback) } @Test fun `test should show initial popup`() { every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds } returns 0L - every { newBookDao.getBooks() } returns listOf(mockk()) + coEvery { newBookDao.getBooks() } returns listOf(mockk()) donationDialogHandler.attemptToShowDonationPopup() verify { showDonationDialogCallback.showDonationDialog() } } @@ -62,7 +70,7 @@ class DonationDialogHandlerTest { every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds } returns currentMilliSeconds - (THREE_MONTHS_IN_MILLISECONDS / 2) - every { newBookDao.getBooks() } returns listOf(mockk()) + coEvery { newBookDao.getBooks() } returns listOf(mockk()) donationDialogHandler.attemptToShowDonationPopup() verify(exactly = 0) { showDonationDialogCallback.showDonationDialog() } } @@ -73,7 +81,7 @@ class DonationDialogHandlerTest { every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds } returns currentMilliSeconds - (THREE_MONTHS_IN_MILLISECONDS + 1000) - every { newBookDao.getBooks() } returns listOf(mockk()) + coEvery { newBookDao.getBooks() } returns listOf(mockk()) donationDialogHandler.attemptToShowDonationPopup() verify { showDonationDialogCallback.showDonationDialog() } } @@ -94,34 +102,36 @@ class DonationDialogHandlerTest { } @Test - fun `test isZimFilesAvailableInLibrary returns true when isCustomApp is true`() { + fun `test isZimFilesAvailableInLibrary returns true when isCustomApp is true`() = runTest { with(mockk()) { - every { activity.packageName } returns "org.kiwix.kiwixcustom" - every { activity.isCustomApp() } returns true + every { coreMainActivity.packageName } returns "org.kiwix.kiwixcustom" + every { coreMainActivity.isCustomApp() } returns true val result = donationDialogHandler.isZimFilesAvailableInLibrary() assertTrue(result) } } @Test - fun `test isZimFilesAvailableInLibrary returns false when no books and isCustomApp is false`() { - with(mockk()) { - every { activity.packageName } returns "org.kiwix.kiwixmobile" - every { activity.isCustomApp() } returns false - every { newBookDao.getBooks() } returns emptyList() - val result = donationDialogHandler.isZimFilesAvailableInLibrary() - assertFalse(result) + fun `test isZimFilesAvailableInLibrary returns false when no books and isCustomApp is false`() = + runTest { + with(mockk()) { + every { coreMainActivity.packageName } returns "org.kiwix.kiwixmobile" + every { coreMainActivity.isCustomApp() } returns false + coEvery { newBookDao.getBooks() } returns emptyList() + val result = donationDialogHandler.isZimFilesAvailableInLibrary() + assertFalse(result) + } } - } @Test - fun `isZimFilesAvailableInLibrary returns true when books available and isCustomApp is false`() { - with(mockk()) { - every { activity.packageName } returns "org.kiwix.kiwixmobile" - every { activity.isCustomApp() } returns false - every { newBookDao.getBooks() } returns listOf(mockk()) - val result = donationDialogHandler.isZimFilesAvailableInLibrary() - assertTrue(result) + fun `isZimFilesAvailableInLibrary returns true when books available and isCustomApp is false`() = + runTest { + with(mockk()) { + every { coreMainActivity.packageName } returns "org.kiwix.kiwixmobile" + every { coreMainActivity.isCustomApp() } returns false + coEvery { newBookDao.getBooks() } returns listOf(mockk()) + val result = donationDialogHandler.isZimFilesAvailableInLibrary() + assertTrue(result) + } } - } } From 01320a563e47379f47f329922e1ae74f0b1fa1e2 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 11:40:58 +0530 Subject: [PATCH 04/16] Improved the refreshing of bookmarks after adding, deleting. * Fixed the test cases. --- .../kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt | 4 ++++ .../kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt | 6 ++---- .../org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt | 4 +--- .../java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt | 3 +-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt index 1be215d35f..97d0a23e5c 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/ObjectBoxToLibkiwixMigratorTest.kt @@ -99,6 +99,10 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() { putBoolean(SharedPreferenceUtil.PREF_IS_TEST, true) putBoolean(SharedPreferenceUtil.IS_PLAY_STORE_BUILD, true) putString(SharedPreferenceUtil.PREF_LANG, "en") + putLong( + SharedPreferenceUtil.PREF_LAST_DONATION_POPUP_SHOWN_IN_MILLISECONDS, + System.currentTimeMillis() + ) } activityScenario = ActivityScenario.launch(KiwixMainActivity::class.java).apply { moveToState(Lifecycle.State.RESUMED) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt index bb658f4e09..965f29b9c7 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt @@ -32,9 +32,7 @@ import androidx.test.uiautomator.UiDevice import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesCheck import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesViews import com.google.android.apps.common.testing.accessibility.framework.checks.TouchTargetSizeCheck -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking import org.hamcrest.Matchers.allOf import org.junit.Before import org.junit.Rule @@ -189,7 +187,7 @@ class LibkiwixBookmarkTest : BaseActivityTest() { coreReaderFragment.zimReaderContainer?.zimFileReader?.favicon, coreReaderFragment.zimReaderContainer?.zimFileReader?.zimReaderSource ) - CoroutineScope(Dispatchers.Main).launch { + runBlocking { coreReaderFragment.libkiwixBookmarks?.saveBookmark(libkiwixItem).also { bookmarkList.add(libkiwixItem) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt index 4d864221fd..826c906c66 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt @@ -241,9 +241,7 @@ class LibkiwixBookmarks @Inject constructor( // Save the bookmarks data to a separate file. library.writeBookmarksToFile(bookmarkFile.canonicalPath) // set the bookmark change to true so that libkiwix will return the new data. - CoroutineScope(Dispatchers.Main).launch { - bookmarksChanged = true - } + bookmarksChanged = true } @Suppress("ReturnCount") diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt index eb729213b4..26ef54240e 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt @@ -99,8 +99,7 @@ internal class NewBookDaoTest { fun getBooks() = runTest { val entity = bookOnDiskEntity() every { box.all } returns mutableListOf(entity) - val result = newBookDao.getBooks() - assertThat(result).isEqualTo(listOf(BookOnDisk(entity))) + assertThat(newBookDao.getBooks()).isEqualTo(listOf(BookOnDisk(entity))) } @Nested From 5c25b20ad7e11574bbf4016fb9992c2e3b69fd7a Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 12:57:13 +0530 Subject: [PATCH 05/16] Fixed: DonationDialogHandlerTest. --- .../kiwixmobile/core/main/CoreReaderFragment.kt | 2 +- .../core/utils/DonationDialogHandler.kt | 17 +++++------------ core/src/main/res/layout/fragment_reader.xml | 2 +- .../core/utils/DonationDialogHandlerTest.kt | 13 +++++-------- 4 files changed, 12 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index d9cf414def..5fa38a4c30 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -1875,7 +1875,7 @@ abstract class CoreReaderFragment : if (tts == null) { setUpTTS() } - donationDialogHandler?.attemptToShowDonationPopup() + lifecycleScope.launch { donationDialogHandler?.attemptToShowDonationPopup() } } @Suppress("InflateParams", "MagicNumber") diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt index 8a03593f80..1217f519e9 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandler.kt @@ -19,11 +19,8 @@ package org.kiwix.kiwixmobile.core.utils import android.app.Activity -import androidx.lifecycle.lifecycleScope -import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isCustomApp -import org.kiwix.kiwixmobile.core.main.CoreMainActivity import javax.inject.Inject const val THREE_DAYS_IN_MILLISECONDS = 3 * 24 * 60 * 60 * 1000L @@ -41,18 +38,14 @@ class DonationDialogHandler @Inject constructor( this.showDonationDialogCallback = showDonationDialogCallback } - fun attemptToShowDonationPopup() { + suspend fun attemptToShowDonationPopup() { val currentMilliSeconds = System.currentTimeMillis() val lastPopupMillis = sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds val timeDifference = currentMilliSeconds - lastPopupMillis - (activity as CoreMainActivity).lifecycleScope.launch { - if (shouldShowInitialPopup(lastPopupMillis) || - timeDifference >= THREE_MONTHS_IN_MILLISECONDS - ) { - if (isZimFilesAvailableInLibrary() && isTimeToShowDonation(currentMilliSeconds)) { - showDonationDialogCallback?.showDonationDialog() - resetDonateLater() - } + if (shouldShowInitialPopup(lastPopupMillis) || timeDifference >= THREE_MONTHS_IN_MILLISECONDS) { + if (isZimFilesAvailableInLibrary() && isTimeToShowDonation(currentMilliSeconds)) { + showDonationDialogCallback?.showDonationDialog() + resetDonateLater() } } } diff --git a/core/src/main/res/layout/fragment_reader.xml b/core/src/main/res/layout/fragment_reader.xml index aa3d60ce59..768822c347 100644 --- a/core/src/main/res/layout/fragment_reader.xml +++ b/core/src/main/res/layout/fragment_reader.xml @@ -62,7 +62,7 @@ android:id="@+id/donation_layout" android:layout_width="match_parent" android:layout_height="wrap_content" - android:visibility="visible" + android:visibility="gone" app:layout_constraintStart_toStartOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintBottom_toBottomOf="parent" /> diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt index b4cebfd120..574ff5f078 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt @@ -24,8 +24,7 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.verify -import kotlinx.coroutines.test.StandardTestDispatcher -import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue @@ -35,19 +34,17 @@ import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions import org.kiwix.kiwixmobile.core.main.CoreMainActivity +@ExperimentalCoroutinesApi class DonationDialogHandlerTest { private lateinit var coreMainActivity: CoreMainActivity private lateinit var sharedPreferenceUtil: SharedPreferenceUtil private lateinit var newBookDao: NewBookDao private lateinit var donationDialogHandler: DonationDialogHandler private lateinit var showDonationDialogCallback: DonationDialogHandler.ShowDonationDialogCallback - private val testDispatcher = StandardTestDispatcher() - private val testScope = TestScope(testDispatcher) @BeforeEach fun setup() { coreMainActivity = mockk(relaxed = true) - // every { coreMainActivity.lifecycle } returns testScope.coroutineContext sharedPreferenceUtil = mockk(relaxed = true) newBookDao = mockk(relaxed = true) showDonationDialogCallback = mockk(relaxed = true) @@ -57,7 +54,7 @@ class DonationDialogHandlerTest { } @Test - fun `test should show initial popup`() { + fun `test should show initial popup`() = runTest { every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds } returns 0L coEvery { newBookDao.getBooks() } returns listOf(mockk()) donationDialogHandler.attemptToShowDonationPopup() @@ -65,7 +62,7 @@ class DonationDialogHandlerTest { } @Test - fun `test should not show popup when time difference is less than 3 months`() { + fun `test should not show popup when time difference is less than 3 months`() = runTest { val currentMilliSeconds = System.currentTimeMillis() every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds @@ -76,7 +73,7 @@ class DonationDialogHandlerTest { } @Test - fun `test should show popup when time difference is more than 3 months`() { + fun `test should show popup when time difference is more than 3 months`() = runTest { val currentMilliSeconds = System.currentTimeMillis() every { sharedPreferenceUtil.lastDonationPopupShownInMilliSeconds From a7f662fe45e04434bb2213fd1f01368f03ea68a1 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 13:16:50 +0530 Subject: [PATCH 06/16] Fixed: The search fragment test was failing in both the custom and app modules due to a socket exception during the ZIM file download. --- .../reader/KiwixReaderFragmentTest.kt | 4 ++-- .../kiwixmobile/search/SearchFragmentTest.kt | 6 ++--- .../kiwix/kiwixmobile/testutils/TestUtils.kt | 20 +++++++++++++++++ .../search/SearchFragmentTestForCustomApp.kt | 22 +++++++++++++++++-- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt index d2d1ae3b3b..94fe11ba0f 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/reader/KiwixReaderFragmentTest.kt @@ -33,7 +33,6 @@ import com.google.android.apps.common.testing.accessibility.framework.Accessibil import com.google.android.apps.common.testing.accessibility.framework.AccessibilityCheckResultUtils.matchesViews import com.google.android.apps.common.testing.accessibility.framework.checks.TouchTargetSizeCheck import leakcanary.LeakAssertions -import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.ResponseBody import org.hamcrest.Matchers.allOf @@ -50,6 +49,7 @@ import org.kiwix.kiwixmobile.nav.destination.library.LocalLibraryFragmentDirecti import org.kiwix.kiwixmobile.testutils.RetryRule import org.kiwix.kiwixmobile.testutils.TestUtils import org.kiwix.kiwixmobile.testutils.TestUtils.closeSystemDialogs +import org.kiwix.kiwixmobile.testutils.TestUtils.getOkkHttpClientForTesting import org.kiwix.kiwixmobile.testutils.TestUtils.isSystemUINotRespondingDialogVisible import java.io.File import java.io.FileOutputStream @@ -147,7 +147,7 @@ class KiwixReaderFragmentTest : BaseActivityTest() { kiwixMainActivity.navigate(R.id.libraryFragment) } val downloadingZimFile = getDownloadingZimFile() - OkHttpClient().newCall(downloadRequest()).execute().use { response -> + getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> writeZimFileData(responseBody, downloadingZimFile) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt index 32f9d77b68..cb0c0c1d2b 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/search/SearchFragmentTest.kt @@ -36,7 +36,6 @@ import com.google.android.apps.common.testing.accessibility.framework.checks.Tou import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import leakcanary.LeakAssertions -import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.ResponseBody import org.hamcrest.Matchers.allOf @@ -57,6 +56,7 @@ import org.kiwix.kiwixmobile.nav.destination.library.LocalLibraryFragmentDirecti import org.kiwix.kiwixmobile.testutils.RetryRule import org.kiwix.kiwixmobile.testutils.TestUtils import org.kiwix.kiwixmobile.testutils.TestUtils.closeSystemDialogs +import org.kiwix.kiwixmobile.testutils.TestUtils.getOkkHttpClientForTesting import org.kiwix.kiwixmobile.testutils.TestUtils.isSystemUINotRespondingDialogVisible import java.io.File import java.io.FileOutputStream @@ -169,7 +169,7 @@ class SearchFragmentTest : BaseActivityTest() { UiThreadStatement.runOnUiThread { kiwixMainActivity.navigate(R.id.libraryFragment) } // test with a large ZIM file to properly test the scenario downloadingZimFile = getDownloadingZimFile() - OkHttpClient().newCall(downloadRequest()).execute().use { response -> + getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> writeZimFileData(responseBody, downloadingZimFile) @@ -242,7 +242,7 @@ class SearchFragmentTest : BaseActivityTest() { kiwixMainActivity.navigate(R.id.libraryFragment) } downloadingZimFile = getDownloadingZimFile() - OkHttpClient().newCall(downloadRequest()).execute().use { response -> + getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> writeZimFileData(responseBody, downloadingZimFile) diff --git a/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt b/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt index dfb27e45f2..3b7127692f 100644 --- a/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt +++ b/app/src/androidTest/java/org/kiwix/kiwixmobile/testutils/TestUtils.kt @@ -37,8 +37,14 @@ import androidx.test.uiautomator.UiDevice import androidx.test.uiautomator.UiObject import androidx.test.uiautomator.UiObjectNotFoundException import androidx.test.uiautomator.UiSelector +import okhttp3.OkHttpClient import org.hamcrest.Description import org.hamcrest.Matcher +import org.kiwix.kiwixmobile.core.data.remote.UserAgentInterceptor +import org.kiwix.kiwixmobile.core.di.modules.CALL_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.CONNECTION_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.READ_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.USER_AGENT import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity import org.kiwix.kiwixmobile.core.utils.files.Log import java.io.File @@ -47,6 +53,8 @@ import java.io.FileOutputStream import java.io.OutputStream import java.text.SimpleDateFormat import java.util.Date +import java.util.concurrent.TimeUnit +import javax.inject.Singleton /** * Created by mhutti1 on 07/04/17. @@ -234,4 +242,16 @@ object TestUtils { } } } + + @JvmStatic + @Singleton + fun getOkkHttpClientForTesting(): OkHttpClient = + OkHttpClient().newBuilder() + .followRedirects(true) + .followSslRedirects(true) + .connectTimeout(CONNECTION_TIMEOUT, TimeUnit.SECONDS) + .readTimeout(READ_TIMEOUT, TimeUnit.SECONDS) + .callTimeout(CALL_TIMEOUT, TimeUnit.SECONDS) + .addNetworkInterceptor(UserAgentInterceptor(USER_AGENT)) + .build() } diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt index ec50e92277..8802aab8e3 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt @@ -43,6 +43,11 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.kiwix.kiwixmobile.core.data.remote.UserAgentInterceptor +import org.kiwix.kiwixmobile.core.di.modules.CALL_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.CONNECTION_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.READ_TIMEOUT +import org.kiwix.kiwixmobile.core.di.modules.USER_AGENT import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.search.SearchFragment import org.kiwix.kiwixmobile.core.search.viewmodel.Action @@ -58,6 +63,8 @@ import java.io.File import java.io.FileOutputStream import java.io.IOException import java.net.URI +import java.util.concurrent.TimeUnit +import javax.inject.Singleton @RunWith(AndroidJUnit4::class) class SearchFragmentTestForCustomApp { @@ -123,7 +130,7 @@ class SearchFragmentTestForCustomApp { } // test with a large ZIM file to properly test the scenario downloadingZimFile = getDownloadingZimFile() - OkHttpClient().newCall(downloadRequest()).execute().use { response -> + getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> writeZimFileData(responseBody, downloadingZimFile) @@ -201,7 +208,7 @@ class SearchFragmentTestForCustomApp { } // test with a large ZIM file to properly test the scenario downloadingZimFile = getDownloadingZimFile() - OkHttpClient().newCall(downloadRequest()).execute().use { response -> + getOkkHttpClientForTesting().newCall(downloadRequest()).execute().use { response -> if (response.isSuccessful) { response.body?.let { responseBody -> writeZimFileData(responseBody, downloadingZimFile) @@ -321,6 +328,17 @@ class SearchFragmentTestForCustomApp { return zimFile } + @Singleton + private fun getOkkHttpClientForTesting(): OkHttpClient = + OkHttpClient().newBuilder() + .followRedirects(true) + .followSslRedirects(true) + .connectTimeout(CONNECTION_TIMEOUT, TimeUnit.SECONDS) + .readTimeout(READ_TIMEOUT, TimeUnit.SECONDS) + .callTimeout(CALL_TIMEOUT, TimeUnit.SECONDS) + .addNetworkInterceptor(UserAgentInterceptor(USER_AGENT)) + .build() + @After fun finish() { TestUtils.deleteTemporaryFilesOfTestCases(context) From 656a64df320e596c3037afc0566d7b4c5d49825e Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 14:53:32 +0530 Subject: [PATCH 07/16] Improved the search functionality of our application: * The article URL was already available in the searchState, but we were redundantly retrieving the article URL from the pageTitle, resulting in an unnecessary extra call to libkiwix. To optimize this, we now directly use the URL provided by libkiwix for the searched item, reducing the extra call. --- .../destination/reader/KiwixReaderFragment.kt | 35 +++++++------------ .../core/main/CoreReaderFragment.kt | 10 ++---- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index 15a7318374..12b0273291 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -31,11 +31,7 @@ import android.view.View.VISIBLE import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.drawerlayout.widget.DrawerLayout -import androidx.lifecycle.lifecycleScope import com.google.android.material.bottomnavigation.BottomNavigationView -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.R import org.kiwix.kiwixmobile.cachedComponent import org.kiwix.kiwixmobile.core.R.anim @@ -223,27 +219,22 @@ class KiwixReaderFragment : CoreReaderFragment() { ) { val settings = requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) - lifecycleScope.launch { - val canOpenInLibkiwix = withContext(Dispatchers.IO) { - zimReaderSource?.canOpenInLibkiwix() - } - if (zimReaderSource != null && canOpenInLibkiwix == true) { - if (zimReaderContainer?.zimReaderSource == null) { - openZimFile(zimReaderSource) - Log.d( - TAG_KIWIX, - "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" - ) - } else { - zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) - } + if (zimReaderSource != null) { + if (zimReaderContainer?.zimReaderSource == null) { + openZimFile(zimReaderSource) + Log.d( + TAG_KIWIX, + "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" + ) } else { - getCurrentWebView()?.snack(string.zim_not_opened) - exitBook() // hide the options for zim file to avoid unexpected UI behavior - return@launch // book not found so don't need to restore the tabs for this file + zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) } - restoreTabs(zimArticles, zimPositions, currentTab) + } else { + getCurrentWebView()?.snack(string.zim_not_opened) + exitBook() // hide the options for zim file to avoid unexpected UI behavior + return // book not found so don't need to restore the tabs for this file } + restoreTabs(zimArticles, zimPositions, currentTab) } @Throws(IllegalArgumentException::class) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 5fa38a4c30..12b8eca649 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -167,8 +167,6 @@ import org.kiwix.kiwixmobile.core.utils.dialog.UnsupportedMimeTypeHandler import org.kiwix.kiwixmobile.core.utils.files.FileUtils.deleteCachedFiles import org.kiwix.kiwixmobile.core.utils.files.FileUtils.readFile import org.kiwix.kiwixmobile.core.utils.files.Log -import org.kiwix.kiwixmobile.core.utils.titleToUrl -import org.kiwix.kiwixmobile.core.utils.urlSuffixToParsableUrl import org.kiwix.libkiwix.Book import java.io.IOException import java.text.SimpleDateFormat @@ -1986,12 +1984,10 @@ abstract class CoreReaderFragment : } private fun openSearchItem(item: SearchItemToOpen) { - zimReaderContainer?.titleToUrl(item.pageTitle)?.let { - if (item.shouldOpenInNewTab) { - createNewTab() - } - loadUrlWithCurrentWebview(zimReaderContainer?.urlSuffixToParsableUrl(it)) + if (item.shouldOpenInNewTab) { + createNewTab() } + loadUrlWithCurrentWebview(item.pageUrl) requireActivity().consumeObservable(TAG_FILE_SEARCHED) } From 31bb875d8707ab12b603ff7ea999336f5d44a828 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 16:24:50 +0530 Subject: [PATCH 08/16] Removed the unused `ZimReaderContainerUtils.kt` class. * Refactored some unnecessary code. --- .../core/utils/ZimReaderContainerUtils.kt | 35 ------------------- .../core/utils/DonationDialogHandlerTest.kt | 20 +++++------ 2 files changed, 10 insertions(+), 45 deletions(-) delete mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt deleted file mode 100644 index 398e58df90..0000000000 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Kiwix Android - * Copyright (c) 2020 Kiwix - * 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 . - * - */ - -package org.kiwix.kiwixmobile.core.utils - -import android.net.Uri -import org.kiwix.kiwixmobile.core.reader.ZimFileReader -import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer - -private fun ZimReaderContainer.redirectOrOriginal(contentUrl: String): String = - if (isRedirect(contentUrl)) getRedirect(contentUrl) else contentUrl - -private fun contentUrl(articleUrl: String): String = - Uri.parse(ZimFileReader.CONTENT_PREFIX + articleUrl).toString() - -fun ZimReaderContainer.urlSuffixToParsableUrl(suffixUrl: String): String = - redirectOrOriginal(contentUrl(suffixUrl)) - -fun ZimReaderContainer.titleToUrl(title: String): String? = - if (title.startsWith("A/")) title else getPageUrlFromTitle(title) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt index 574ff5f078..bfa9287093 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/utils/DonationDialogHandlerTest.kt @@ -18,6 +18,7 @@ package org.kiwix.kiwixmobile.core.utils +import android.app.Activity import io.mockk.Runs import io.mockk.coEvery import io.mockk.every @@ -32,11 +33,10 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.kiwix.kiwixmobile.core.dao.NewBookDao import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions -import org.kiwix.kiwixmobile.core.main.CoreMainActivity @ExperimentalCoroutinesApi class DonationDialogHandlerTest { - private lateinit var coreMainActivity: CoreMainActivity + private lateinit var activity: Activity private lateinit var sharedPreferenceUtil: SharedPreferenceUtil private lateinit var newBookDao: NewBookDao private lateinit var donationDialogHandler: DonationDialogHandler @@ -44,12 +44,12 @@ class DonationDialogHandlerTest { @BeforeEach fun setup() { - coreMainActivity = mockk(relaxed = true) + activity = mockk(relaxed = true) sharedPreferenceUtil = mockk(relaxed = true) newBookDao = mockk(relaxed = true) showDonationDialogCallback = mockk(relaxed = true) donationDialogHandler = - DonationDialogHandler(coreMainActivity, sharedPreferenceUtil, newBookDao) + DonationDialogHandler(activity, sharedPreferenceUtil, newBookDao) donationDialogHandler.setDonationDialogCallBack(showDonationDialogCallback) } @@ -101,8 +101,8 @@ class DonationDialogHandlerTest { @Test fun `test isZimFilesAvailableInLibrary returns true when isCustomApp is true`() = runTest { with(mockk()) { - every { coreMainActivity.packageName } returns "org.kiwix.kiwixcustom" - every { coreMainActivity.isCustomApp() } returns true + every { activity.packageName } returns "org.kiwix.kiwixcustom" + every { activity.isCustomApp() } returns true val result = donationDialogHandler.isZimFilesAvailableInLibrary() assertTrue(result) } @@ -112,8 +112,8 @@ class DonationDialogHandlerTest { fun `test isZimFilesAvailableInLibrary returns false when no books and isCustomApp is false`() = runTest { with(mockk()) { - every { coreMainActivity.packageName } returns "org.kiwix.kiwixmobile" - every { coreMainActivity.isCustomApp() } returns false + every { activity.packageName } returns "org.kiwix.kiwixmobile" + every { activity.isCustomApp() } returns false coEvery { newBookDao.getBooks() } returns emptyList() val result = donationDialogHandler.isZimFilesAvailableInLibrary() assertFalse(result) @@ -124,8 +124,8 @@ class DonationDialogHandlerTest { fun `isZimFilesAvailableInLibrary returns true when books available and isCustomApp is false`() = runTest { with(mockk()) { - every { coreMainActivity.packageName } returns "org.kiwix.kiwixmobile" - every { coreMainActivity.isCustomApp() } returns false + every { activity.packageName } returns "org.kiwix.kiwixmobile" + every { activity.isCustomApp() } returns false coEvery { newBookDao.getBooks() } returns listOf(mockk()) val result = donationDialogHandler.isZimFilesAvailableInLibrary() assertTrue(result) From 78e265b9420a06ddee3b8bdce15c63af06c42cc5 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 17:56:13 +0530 Subject: [PATCH 09/16] Fixed an issue where bookmarks would not open for a ZIM file if another ZIM file was already opened in the reader. Instead of opening the bookmarked page, it always opens the home page of the current ZIM file. --- .../destination/reader/KiwixReaderFragment.kt | 72 ++++++++++--------- .../core/main/CoreReaderFragment.kt | 33 ++++----- .../search/SearchFragmentTestForCustomApp.kt | 10 +-- .../custom/main/CustomReaderFragment.kt | 52 +++++++------- 4 files changed, 87 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index 12b0273291..ce8ce6daf7 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -31,7 +31,9 @@ import android.view.View.VISIBLE import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.drawerlayout.widget.DrawerLayout +import androidx.lifecycle.lifecycleScope import com.google.android.material.bottomnavigation.BottomNavigationView +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.R import org.kiwix.kiwixmobile.cachedComponent import org.kiwix.kiwixmobile.core.R.anim @@ -86,29 +88,30 @@ class KiwixReaderFragment : CoreReaderFragment() { private fun openPageInBookFromNavigationArguments() { val args = KiwixReaderFragmentArgs.fromBundle(requireArguments()) - - if (args.pageUrl.isNotEmpty()) { - if (args.zimFileUri.isNotEmpty()) { - tryOpeningZimFile(args.zimFileUri) + lifecycleScope.launch { + if (args.pageUrl.isNotEmpty()) { + if (args.zimFileUri.isNotEmpty()) { + tryOpeningZimFile(args.zimFileUri) + } else { + // Set up bookmarks for the current book when opening bookmarks from the Bookmark screen. + // This is necessary because we are not opening the ZIM file again; the bookmark is + // inside the currently opened book. Bookmarks are set up when opening the ZIM file. + // See /~https://github.com/kiwix/kiwix-android/issues/3541 + zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + } + loadUrlWithCurrentWebview(args.pageUrl) } else { - // Set up bookmarks for the current book when opening bookmarks from the Bookmark screen. - // This is necessary because we are not opening the ZIM file again; the bookmark is - // inside the currently opened book. Bookmarks are set up when opening the ZIM file. - // See /~https://github.com/kiwix/kiwix-android/issues/3541 - zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) - } - loadUrlWithCurrentWebview(args.pageUrl) - } else { - if (args.zimFileUri.isNotEmpty()) { - tryOpeningZimFile(args.zimFileUri) - } else { - manageExternalLaunchAndRestoringViewState() + if (args.zimFileUri.isNotEmpty()) { + tryOpeningZimFile(args.zimFileUri) + } else { + manageExternalLaunchAndRestoringViewState() + } } + requireArguments().clear() } - requireArguments().clear() } - private fun tryOpeningZimFile(zimFileUri: String) { + private suspend fun tryOpeningZimFile(zimFileUri: String) { val filePath = FileUtils.getLocalFilePathByUri( requireActivity().applicationContext, Uri.parse(zimFileUri) ) @@ -217,24 +220,27 @@ class KiwixReaderFragment : CoreReaderFragment() { zimPositions: String?, currentTab: Int ) { - val settings = requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) - val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) - if (zimReaderSource != null) { - if (zimReaderContainer?.zimReaderSource == null) { - openZimFile(zimReaderSource) - Log.d( - TAG_KIWIX, - "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" - ) + lifecycleScope.launch { + val settings = + requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) + val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) + if (zimReaderSource != null && zimReaderSource.canOpenInLibkiwix()) { + if (zimReaderContainer?.zimReaderSource == null) { + openZimFile(zimReaderSource) + Log.d( + TAG_KIWIX, + "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" + ) + } else { + zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + } } else { - zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + getCurrentWebView()?.snack(string.zim_not_opened) + exitBook() // hide the options for zim file to avoid unexpected UI behavior + return@launch // book not found so don't need to restore the tabs for this file } - } else { - getCurrentWebView()?.snack(string.zim_not_opened) - exitBook() // hide the options for zim file to avoid unexpected UI behavior - return // book not found so don't need to restore the tabs for this file + restoreTabs(zimArticles, zimPositions, currentTab) } - restoreTabs(zimArticles, zimPositions, currentTab) } @Throws(IllegalArgumentException::class) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 12b8eca649..9ed2179571 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -94,9 +94,7 @@ import io.reactivex.Flowable import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import io.reactivex.processors.BehaviorProcessor -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.json.JSONArray import org.json.JSONException import org.kiwix.kiwixmobile.core.BuildConfig @@ -1641,23 +1639,18 @@ abstract class CoreReaderFragment : unsupportedMimeTypeHandler?.showSaveOrOpenUnsupportedFilesDialog(url, documentType) } - fun openZimFile(zimReaderSource: ZimReaderSource, isCustomApp: Boolean = false) { + suspend fun openZimFile(zimReaderSource: ZimReaderSource, isCustomApp: Boolean = false) { if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE) || isCustomApp) { - lifecycleScope.launch { - val canOpenInLibkiwix = withContext(Dispatchers.IO) { - zimReaderSource.canOpenInLibkiwix() - } - if (canOpenInLibkiwix) { - // Show content if there is `Open Library` button showing - // and we are opening the ZIM file - reopenBook() - openAndSetInContainer(zimReaderSource) - updateTitle() - } else { - exitBook() - Log.w(TAG_KIWIX, "ZIM file doesn't exist at " + zimReaderSource.toDatabase()) - requireActivity().toast(R.string.error_file_not_found, Toast.LENGTH_LONG) - } + if (zimReaderSource.canOpenInLibkiwix()) { + // Show content if there is `Open Library` button showing + // and we are opening the ZIM file + reopenBook() + openAndSetInContainer(zimReaderSource) + updateTitle() + } else { + exitBook() + Log.w(TAG_KIWIX, "ZIM file doesn't exist at " + zimReaderSource.toDatabase()) + requireActivity().toast(R.string.error_file_not_found, Toast.LENGTH_LONG) } } else { this.zimReaderSource = zimReaderSource @@ -1738,7 +1731,9 @@ abstract class CoreReaderFragment : when (requestCode) { REQUEST_STORAGE_PERMISSION -> { if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE)) { - zimReaderSource?.let(::openZimFile) + lifecycleScope.launch { + zimReaderSource?.let { openZimFile(it) } + } } else { snackBarRoot?.let { snackBarRoot -> Snackbar.make(snackBarRoot, R.string.request_storage, Snackbar.LENGTH_LONG) diff --git a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt index 8802aab8e3..0dcc83daf0 100644 --- a/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt +++ b/custom/src/androidTest/java/org/kiwix/kiwixmobile/custom/search/SearchFragmentTestForCustomApp.kt @@ -279,10 +279,12 @@ class SearchFragmentTestForCustomApp { ) as NavHostFragment val customReaderFragment = navHostFragment.childFragmentManager.fragments[0] as CustomReaderFragment - customReaderFragment.openZimFile( - ZimReaderSource(null, null, listOf(assetFileDescriptor)), - true - ) + runBlocking { + customReaderFragment.openZimFile( + ZimReaderSource(null, null, listOf(assetFileDescriptor)), + true + ) + } } } diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt index 2ccc70b1f8..0e157113a5 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt @@ -30,7 +30,9 @@ import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.widget.Toolbar import androidx.core.net.toUri import androidx.drawerlayout.widget.DrawerLayout +import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.R.dimen import org.kiwix.kiwixmobile.core.base.BaseActivity import org.kiwix.kiwixmobile.core.extensions.browserIntent @@ -183,33 +185,35 @@ class CustomReaderFragment : CoreReaderFragment() { private fun openObbOrZim() { customFileValidator.validate( onFilesFound = { - when (it) { - is ValidationState.HasFile -> { - openZimFile( - ZimReaderSource( - file = it.file, - null, - it.assetFileDescriptorList - ), - true - ) - // Save book in the database to display it in `ZimHostFragment`. - zimReaderContainer?.zimFileReader?.let { zimFileReader -> - // Check if the file is not null. If the file is null, - // it means we have created zimFileReader with a fileDescriptor, - // so we create a demo file to save it in the database for display on the `ZimHostFragment`. - val file = it.file ?: createDemoFile() - val bookOnDisk = BookOnDisk(zimFileReader) - repositoryActions?.saveBook(bookOnDisk) + lifecycleScope.launch { + when (it) { + is ValidationState.HasFile -> { + openZimFile( + ZimReaderSource( + file = it.file, + null, + it.assetFileDescriptorList + ), + true + ) + // Save book in the database to display it in `ZimHostFragment`. + zimReaderContainer?.zimFileReader?.let { zimFileReader -> + // Check if the file is not null. If the file is null, + // it means we have created zimFileReader with a fileDescriptor, + // so we create a demo file to save it in the database for display on the `ZimHostFragment`. + val file = it.file ?: createDemoFile() + val bookOnDisk = BookOnDisk(zimFileReader) + repositoryActions?.saveBook(bookOnDisk) + } } - } - is ValidationState.HasBothFiles -> { - it.zimFile.delete() - openZimFile(ZimReaderSource(it.obbFile), true) - } + is ValidationState.HasBothFiles -> { + it.zimFile.delete() + openZimFile(ZimReaderSource(it.obbFile), true) + } - else -> {} + else -> {} + } } }, onNoFilesFound = { From 22daa3d86fa4c798b26f78861f3d65e96b93cd7e Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Thu, 17 Oct 2024 18:26:45 +0530 Subject: [PATCH 10/16] Reintroduced the fallback method for searched items. --- .../core/main/CoreReaderFragment.kt | 10 ++++-- .../core/utils/ZimReaderContainerUtils.kt | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 9ed2179571..c704b918ee 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -165,6 +165,8 @@ import org.kiwix.kiwixmobile.core.utils.dialog.UnsupportedMimeTypeHandler import org.kiwix.kiwixmobile.core.utils.files.FileUtils.deleteCachedFiles import org.kiwix.kiwixmobile.core.utils.files.FileUtils.readFile import org.kiwix.kiwixmobile.core.utils.files.Log +import org.kiwix.kiwixmobile.core.utils.titleToUrl +import org.kiwix.kiwixmobile.core.utils.urlSuffixToParsableUrl import org.kiwix.libkiwix.Book import java.io.IOException import java.text.SimpleDateFormat @@ -1676,7 +1678,7 @@ abstract class CoreReaderFragment : ) } - private fun openAndSetInContainer(zimReaderSource: ZimReaderSource) { + private suspend fun openAndSetInContainer(zimReaderSource: ZimReaderSource) { try { if (isNotPreviouslyOpenZim(zimReaderSource)) { webViewList.clear() @@ -1982,7 +1984,11 @@ abstract class CoreReaderFragment : if (item.shouldOpenInNewTab) { createNewTab() } - loadUrlWithCurrentWebview(item.pageUrl) + item.pageUrl?.let(::loadUrlWithCurrentWebview) ?: kotlin.run { + zimReaderContainer?.titleToUrl(item.pageTitle)?.apply { + loadUrlWithCurrentWebview(zimReaderContainer?.urlSuffixToParsableUrl(this)) + } + } requireActivity().consumeObservable(TAG_FILE_SEARCHED) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt new file mode 100644 index 0000000000..398e58df90 --- /dev/null +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/ZimReaderContainerUtils.kt @@ -0,0 +1,35 @@ +/* + * Kiwix Android + * Copyright (c) 2020 Kiwix + * 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 . + * + */ + +package org.kiwix.kiwixmobile.core.utils + +import android.net.Uri +import org.kiwix.kiwixmobile.core.reader.ZimFileReader +import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer + +private fun ZimReaderContainer.redirectOrOriginal(contentUrl: String): String = + if (isRedirect(contentUrl)) getRedirect(contentUrl) else contentUrl + +private fun contentUrl(articleUrl: String): String = + Uri.parse(ZimFileReader.CONTENT_PREFIX + articleUrl).toString() + +fun ZimReaderContainer.urlSuffixToParsableUrl(suffixUrl: String): String = + redirectOrOriginal(contentUrl(suffixUrl)) + +fun ZimReaderContainer.titleToUrl(title: String): String? = + if (title.startsWith("A/")) title else getPageUrlFromTitle(title) From c97cf0725da1dfdf057be2bd72eac972b107290d Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 18 Oct 2024 12:30:08 +0530 Subject: [PATCH 11/16] Fixed: `test search cancellation` sometimes failing in CI. --- .../kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt index c474cbc4fe..9aa2f45baa 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/SearchStateTest.kt @@ -22,6 +22,7 @@ import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat @@ -121,6 +122,7 @@ internal class SearchStateTest { var list: List? = emptyList() var list1: List? = emptyList() val job = launch(Dispatchers.IO) { + delay(1000) list = searchState.getVisibleResults(0) } From ecbf1b3813c701d73215cd25b8e35175ea485a34 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 18 Oct 2024 13:59:57 +0530 Subject: [PATCH 12/16] Fixed: `books deletes entities whose file does not exist` failing. --- .../java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt index 26ef54240e..5308977d4d 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/dao/NewBookDaoTest.kt @@ -72,7 +72,9 @@ internal class NewBookDaoTest { @Test fun `books deletes entities whose file does not exist`() { val (_, deletedEntity) = expectEmissionOfExistingAndNotExistingBook() - newBookDao.books().test() + newBookDao.books().test().also { + it.awaitTerminalEvent() + } verify { box.remove(listOf(deletedEntity)) } } From c7f8d99eb095b06e4969624a26335157d4dfbe2e Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 18 Oct 2024 16:32:48 +0530 Subject: [PATCH 13/16] Fixed: Search was not working properly, it always showing the current loaded page when we click on any searched item. --- .../destination/reader/KiwixReaderFragment.kt | 67 +++++++++++++------ .../core/main/CoreReaderFragment.kt | 17 ++++- .../viewmodel/effects/OpenSearchItem.kt | 7 +- .../custom/main/CustomReaderFragment.kt | 5 +- 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index ce8ce6daf7..31950a35ce 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -52,6 +52,9 @@ import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.main.CoreMainActivity import org.kiwix.kiwixmobile.core.main.CoreReaderFragment import org.kiwix.kiwixmobile.core.main.CoreWebViewClient +import org.kiwix.kiwixmobile.core.main.RestoreOrigin +import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromSearchScreen +import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromExternalLaunch import org.kiwix.kiwixmobile.core.main.ToolbarScrollingKiwixWebView import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.reader.ZimReaderSource.Companion.fromDatabaseValue @@ -104,7 +107,9 @@ class KiwixReaderFragment : CoreReaderFragment() { if (args.zimFileUri.isNotEmpty()) { tryOpeningZimFile(args.zimFileUri) } else { - manageExternalLaunchAndRestoringViewState() + val restoreOrigin = + if (args.searchItemTitle.isNotEmpty()) FromSearchScreen else FromExternalLaunch + manageExternalLaunchAndRestoringViewState(restoreOrigin) } } requireArguments().clear() @@ -215,31 +220,53 @@ class KiwixReaderFragment : CoreReaderFragment() { exitBook() } + /** + * Restores the view state based on the provided JSON data and restore origin. + * + * Depending on the `restoreOrigin`, this method either restores the last opened ZIM file + * (if the launch is external) or skips re-opening the ZIM file when coming from the search screen, + * as the ZIM file is already set in the reader. The method handles setting up the ZIM file and bookmarks, + * and restores the tabs and positions from the provided data. + * + * @param zimArticles JSON string representing the list of articles to be restored. + * @param zimPositions JSON string representing the positions of the restored articles. + * @param currentTab Index of the tab to be restored as the currently active one. + * @param restoreOrigin Indicates whether the restoration is triggered from an external launch or the search screen. + */ + override fun restoreViewStateOnValidJSON( zimArticles: String?, zimPositions: String?, - currentTab: Int + currentTab: Int, + restoreOrigin: RestoreOrigin ) { - lifecycleScope.launch { - val settings = - requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) - val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) - if (zimReaderSource != null && zimReaderSource.canOpenInLibkiwix()) { - if (zimReaderContainer?.zimReaderSource == null) { - openZimFile(zimReaderSource) - Log.d( - TAG_KIWIX, - "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" - ) - } else { - zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + when (restoreOrigin) { + FromExternalLaunch -> { + lifecycleScope.launch { + val settings = + requireActivity().getSharedPreferences(SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0) + val zimReaderSource = fromDatabaseValue(settings.getString(TAG_CURRENT_FILE, null)) + if (zimReaderSource?.canOpenInLibkiwix() == true) { + if (zimReaderContainer?.zimReaderSource == null) { + openZimFile(zimReaderSource) + Log.d( + TAG_KIWIX, + "Kiwix normal start, Opened last used zimFile: -> ${zimReaderSource.toDatabase()}" + ) + } else { + zimReaderContainer?.zimFileReader?.let(::setUpBookmarks) + } + restoreTabs(zimArticles, zimPositions, currentTab) + } else { + getCurrentWebView()?.snack(string.zim_not_opened) + exitBook() // hide the options for zim file to avoid unexpected UI behavior + } } - } else { - getCurrentWebView()?.snack(string.zim_not_opened) - exitBook() // hide the options for zim file to avoid unexpected UI behavior - return@launch // book not found so don't need to restore the tabs for this file } - restoreTabs(zimArticles, zimPositions, currentTab) + + FromSearchScreen -> { + restoreTabs(zimArticles, zimPositions, currentTab) + } } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index c704b918ee..556b7c65ea 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -174,6 +174,9 @@ import java.util.Date import javax.inject.Inject import kotlin.math.abs import kotlin.math.max +import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromExternalLaunch + +const val SEARCH_ITEM_TITLE_KEY = "searchItemTitle" @Suppress("LargeClass") abstract class CoreReaderFragment : @@ -2427,7 +2430,9 @@ abstract class CoreReaderFragment : private fun isInvalidJson(jsonString: String?): Boolean = jsonString == null || jsonString == "[]" - protected fun manageExternalLaunchAndRestoringViewState() { + protected fun manageExternalLaunchAndRestoringViewState( + restoreOrigin: RestoreOrigin = FromExternalLaunch + ) { val settings = requireActivity().getSharedPreferences( SharedPreferenceUtil.PREF_KIWIX_MOBILE, 0 @@ -2438,7 +2443,7 @@ abstract class CoreReaderFragment : if (isInvalidJson(zimArticles) || isInvalidJson(zimPositions)) { restoreViewStateOnInvalidJSON() } else { - restoreViewStateOnValidJSON(zimArticles, zimPositions, currentTab) + restoreViewStateOnValidJSON(zimArticles, zimPositions, currentTab, restoreOrigin) } } @@ -2554,7 +2559,8 @@ abstract class CoreReaderFragment : protected abstract fun restoreViewStateOnValidJSON( zimArticles: String?, zimPositions: String?, - currentTab: Int + currentTab: Int, + restoreOrigin: RestoreOrigin ) /** @@ -2567,3 +2573,8 @@ abstract class CoreReaderFragment : */ abstract fun restoreViewStateOnInvalidJSON() } + +enum class RestoreOrigin { + FromSearchScreen, + FromExternalLaunch +} diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItem.kt index b765368dca..ed25f97205 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItem.kt @@ -20,10 +20,12 @@ package org.kiwix.kiwixmobile.core.search.viewmodel.effects import android.os.Parcelable import androidx.appcompat.app.AppCompatActivity +import androidx.core.os.bundleOf import kotlinx.parcelize.Parcelize import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.setNavigationResultOnCurrent import org.kiwix.kiwixmobile.core.main.CoreMainActivity +import org.kiwix.kiwixmobile.core.main.SEARCH_ITEM_TITLE_KEY import org.kiwix.kiwixmobile.core.reader.addContentPrefix import org.kiwix.kiwixmobile.core.search.adapter.SearchListItem import org.kiwix.kiwixmobile.core.utils.TAG_FILE_SEARCHED @@ -34,7 +36,10 @@ data class OpenSearchItem( ) : SideEffect { override fun invokeWith(activity: AppCompatActivity) { val readerFragmentResId = (activity as CoreMainActivity).readerFragmentResId - activity.navigate(readerFragmentResId) + activity.navigate( + readerFragmentResId, + bundleOf(SEARCH_ITEM_TITLE_KEY to SEARCH_ITEM_TITLE_KEY) + ) activity.setNavigationResultOnCurrent( SearchItemToOpen( searchListItem.value, diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt index 0e157113a5..31f925f3a0 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/main/CustomReaderFragment.kt @@ -40,6 +40,7 @@ import org.kiwix.kiwixmobile.core.extensions.getResizedDrawable import org.kiwix.kiwixmobile.core.extensions.isFileExist import org.kiwix.kiwixmobile.core.main.CoreReaderFragment import org.kiwix.kiwixmobile.core.main.MainMenu +import org.kiwix.kiwixmobile.core.main.RestoreOrigin import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.utils.LanguageUtils import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower @@ -165,7 +166,9 @@ class CustomReaderFragment : CoreReaderFragment() { override fun restoreViewStateOnValidJSON( zimArticles: String?, zimPositions: String?, - currentTab: Int + currentTab: Int, + // Unused in custom apps as there is only one ZIM file that is already set. + restoreOrigin: RestoreOrigin ) { restoreTabs(zimArticles, zimPositions, currentTab) } From b834e62b6a5b4ecc93502e7fe9bb0c63378798e0 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Fri, 18 Oct 2024 17:45:24 +0530 Subject: [PATCH 14/16] Fixed: OpenSearchItemTest was failing after fixing the search functionality. --- .../core/search/viewmodel/effects/OpenSearchItemTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItemTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItemTest.kt index 1f5fe3f91b..cf26c018de 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItemTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/search/viewmodel/effects/OpenSearchItemTest.kt @@ -19,6 +19,7 @@ package org.kiwix.kiwixmobile.core.search.viewmodel.effects import android.content.Intent +import android.os.Bundle import io.mockk.every import io.mockk.mockk import io.mockk.mockkConstructor @@ -45,7 +46,7 @@ internal class OpenSearchItemTest { } returns intent OpenSearchItem(searchListItem, false).invokeWith(activity) verify { - activity.navigate(activity.readerFragmentResId) + activity.navigate(activity.readerFragmentResId, any()) activity.setNavigationResultOnCurrent( SearchItemToOpen(searchListItem.value, false, ZimFileReader.CONTENT_PREFIX), TAG_FILE_SEARCHED @@ -65,7 +66,7 @@ internal class OpenSearchItemTest { } returns intent OpenSearchItem(searchListItem, true).invokeWith(activity) verify { - activity.navigate(activity.readerFragmentResId) + activity.navigate(activity.readerFragmentResId, any()) activity.setNavigationResultOnCurrent( SearchItemToOpen(searchListItem.value, true, ZimFileReader.CONTENT_PREFIX), TAG_FILE_SEARCHED From d46a850c44934da5cae0f78a1d1450eacfd26173 Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 21 Oct 2024 12:19:39 +0530 Subject: [PATCH 15/16] Fixed: sometimes a native crash happens when opening new ZIM files in the reader. --- .../nav/destination/reader/KiwixReaderFragment.kt | 11 ++++++++++- .../kiwix/kiwixmobile/core/main/CoreReaderFragment.kt | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index 31950a35ce..7bc2eee4e8 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -53,8 +53,8 @@ import org.kiwix.kiwixmobile.core.main.CoreMainActivity import org.kiwix.kiwixmobile.core.main.CoreReaderFragment import org.kiwix.kiwixmobile.core.main.CoreWebViewClient import org.kiwix.kiwixmobile.core.main.RestoreOrigin -import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromSearchScreen import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromExternalLaunch +import org.kiwix.kiwixmobile.core.main.RestoreOrigin.FromSearchScreen import org.kiwix.kiwixmobile.core.main.ToolbarScrollingKiwixWebView import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.reader.ZimReaderSource.Companion.fromDatabaseValue @@ -117,6 +117,15 @@ class KiwixReaderFragment : CoreReaderFragment() { } private suspend fun tryOpeningZimFile(zimFileUri: String) { + // Close the previously opened book in the reader before opening a new ZIM file + // to avoid native crashes due to "null pointer dereference." These crashes can occur + // when setting a new ZIM file in the archive while the previous one is being disposed of. + // Since the WebView may still asynchronously request data from the disposed archive, + // we close the previous book before opening a new ZIM file in the archive. + closeZimBook() + // Update the reader screen title to prevent showing the previously set title + // when creating the new archive object. + updateTitle() val filePath = FileUtils.getLocalFilePathByUri( requireActivity().applicationContext, Uri.parse(zimFileUri) ) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 556b7c65ea..b857bcd5d5 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -1386,7 +1386,7 @@ abstract class CoreReaderFragment : closeZimBook() } - private fun closeZimBook() { + fun closeZimBook() { zimReaderContainer?.setZimReaderSource(null) } From 98611336620e9b5f8925858d848f63b41209a06d Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Mon, 21 Oct 2024 14:29:11 +0530 Subject: [PATCH 16/16] Clearing the webView list to avoid loading previous ZIM file URLs if we open a new ZIM file in the reader. --- .../destination/reader/KiwixReaderFragment.kt | 5 +++-- .../core/main/CoreReaderFragment.kt | 18 +++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt index 7bc2eee4e8..b21cd4c15c 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/reader/KiwixReaderFragment.kt @@ -129,7 +129,6 @@ class KiwixReaderFragment : CoreReaderFragment() { val filePath = FileUtils.getLocalFilePathByUri( requireActivity().applicationContext, Uri.parse(zimFileUri) ) - if (filePath == null || !File(filePath).isFileExist()) { // Close the previously opened book in the reader. Since this file is not found, // it will not be set in the zimFileReader. The previously opened ZIM file @@ -140,7 +139,9 @@ class KiwixReaderFragment : CoreReaderFragment() { activity.toast(string.error_file_not_found) return } - openZimFile(ZimReaderSource(File(filePath))) + val zimReaderSource = ZimReaderSource(File(filePath)) + clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource) + openZimFile(zimReaderSource) } override fun loadDrawerViews() { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index b857bcd5d5..5e7496e7a7 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -1682,13 +1682,7 @@ abstract class CoreReaderFragment : } private suspend fun openAndSetInContainer(zimReaderSource: ZimReaderSource) { - try { - if (isNotPreviouslyOpenZim(zimReaderSource)) { - webViewList.clear() - } - } catch (e: IOException) { - e.printStackTrace() - } + clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource) zimReaderContainer?.let { zimReaderContainer -> zimReaderContainer.setZimReaderSource(zimReaderSource) @@ -1704,6 +1698,16 @@ abstract class CoreReaderFragment : } } + fun clearWebViewListIfNotPreviouslyOpenZimFile(zimReaderSource: ZimReaderSource) { + try { + if (isNotPreviouslyOpenZim(zimReaderSource)) { + webViewList.clear() + } + } catch (e: IOException) { + e.printStackTrace() + } + } + protected fun setUpBookmarks(zimFileReader: ZimFileReader) { safeDispose() bookmarkingDisposable = Flowable.combineLatest(