Skip to content

Commit c52ccd8

Browse files
Fixed: Input dispatching timed out error on Manager.readFile.
* Moved the initialization on `LibkiwixBookOnDisk` on IO thread instead of main thread. Now, the manager will read the file on IO thread before any method runs that depends on it. It will run only once. * Improved the `localBooksFlow`. * Refactored `LibkiwixBookOnDisk` similarly, since it also used the same initialization approach. * Since we now use `tempLibrary` for parsing the online library content, we no longer need the library and manager objects provided via Dagger. * Fixed: The bookmark icon was not showing when adding a bookmark for a new book, deleting it, and then adding it again.
1 parent 987aebf commit c52ccd8

File tree

5 files changed

+84
-99
lines changed

5 files changed

+84
-99
lines changed

app/src/main/java/org/kiwix/kiwixmobile/di/components/KiwixComponent.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ package org.kiwix.kiwixmobile.di.components
2121
import dagger.Component
2222
import org.kiwix.kiwixmobile.core.data.ObjectBoxDataMigrationHandler
2323
import org.kiwix.kiwixmobile.core.di.components.CoreComponent
24-
import org.kiwix.kiwixmobile.migration.di.module.MigrationModule
2524
import org.kiwix.kiwixmobile.di.KiwixScope
2625
import org.kiwix.kiwixmobile.di.components.ServiceComponent.Builder
27-
import org.kiwix.kiwixmobile.di.modules.JNIModule
2826
import org.kiwix.kiwixmobile.di.modules.KiwixModule
2927
import org.kiwix.kiwixmobile.di.modules.KiwixViewModelModule
3028
import org.kiwix.kiwixmobile.migration.di.module.DatabaseModule
29+
import org.kiwix.kiwixmobile.migration.di.module.MigrationModule
3130
import org.kiwix.kiwixmobile.storage.StorageSelectDialog
3231
import org.kiwix.kiwixmobile.zimManager.OnlineLibraryManager
3332

@@ -37,7 +36,6 @@ import org.kiwix.kiwixmobile.zimManager.OnlineLibraryManager
3736
modules = [
3837
KiwixViewModelModule::class,
3938
KiwixModule::class,
40-
JNIModule::class,
4139
MigrationModule::class,
4240
DatabaseModule::class
4341
]

app/src/main/java/org/kiwix/kiwixmobile/di/modules/JNIModule.kt

Lines changed: 0 additions & 52 deletions
This file was deleted.

app/src/main/java/org/kiwix/kiwixmobile/zimManager/OnlineLibraryManager.kt

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,14 @@ import kotlinx.coroutines.withContext
2424
import org.kiwix.kiwixmobile.core.data.remote.KiwixService.Companion.OPDS_LIBRARY_ENDPOINT
2525
import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO
2626
import org.kiwix.kiwixmobile.core.entity.LibkiwixBook
27-
import org.kiwix.kiwixmobile.di.modules.ONLINE_BOOKS_LIBRARY
28-
import org.kiwix.kiwixmobile.di.modules.ONLINE_BOOKS_MANAGER
2927
import org.kiwix.libkiwix.Library
3028
import org.kiwix.libkiwix.Manager
3129
import org.xmlpull.v1.XmlPullParser
3230
import org.xmlpull.v1.XmlPullParserFactory
3331
import java.io.StringReader
3432
import javax.inject.Inject
35-
import javax.inject.Named
3633

37-
@Suppress("UnusedPrivateProperty")
38-
class OnlineLibraryManager @Inject constructor(
39-
@Named(ONLINE_BOOKS_LIBRARY) private val library: Library,
40-
@Named(ONLINE_BOOKS_MANAGER) private val manager: Manager,
41-
) {
34+
class OnlineLibraryManager @Inject constructor() {
4235
var totalResult = 0
4336
suspend fun parseOPDSStreamAndGetBooks(
4437
content: String?,

core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookOnDisk.kt

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@ import kotlinx.coroutines.CoroutineScope
2424
import kotlinx.coroutines.Dispatchers
2525
import kotlinx.coroutines.ExperimentalCoroutinesApi
2626
import kotlinx.coroutines.flow.MutableStateFlow
27+
import kotlinx.coroutines.flow.filterNotNull
2728
import kotlinx.coroutines.flow.flowOn
2829
import kotlinx.coroutines.flow.map
2930
import kotlinx.coroutines.flow.mapLatest
3031
import kotlinx.coroutines.launch
31-
import kotlinx.coroutines.runBlocking
32+
import kotlinx.coroutines.sync.Mutex
33+
import kotlinx.coroutines.sync.withLock
3234
import kotlinx.coroutines.withContext
3335
import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks.Companion.TAG
3436
import org.kiwix.kiwixmobile.core.di.modules.LOCAL_BOOKS_LIBRARY
@@ -50,6 +52,8 @@ class LibkiwixBookOnDisk @Inject constructor(
5052
@Named(LOCAL_BOOKS_MANAGER) private val manager: Manager,
5153
private val sharedPreferenceUtil: SharedPreferenceUtil
5254
) {
55+
private val initMutex = Mutex()
56+
private var isManagerInitialized = false
5357
private var libraryBooksList: List<String> = arrayListOf()
5458
private var localBooksList: List<LibkiwixBook> = arrayListOf()
5559

@@ -73,18 +77,29 @@ class LibkiwixBookOnDisk @Inject constructor(
7377
File("$localBookFolderPath/library.xml")
7478
}
7579

76-
init {
77-
// Check if ZIM files folder exist if not then create the folder first.
78-
if (runBlocking { !File(localBookFolderPath).isFileExist() }) File(localBookFolderPath).mkdir()
79-
// Check if library file exist if not then create the file to save the library with book information.
80-
if (runBlocking { !libraryFile.isFileExist() }) libraryFile.createNewFile()
81-
// set up manager to read the library from this file
82-
manager.readFile(libraryFile.canonicalPath)
80+
private suspend fun ensureInitialized(dispatcher: CoroutineDispatcher = Dispatchers.IO) {
81+
if (isManagerInitialized) return
82+
initMutex.withLock {
83+
if (isManagerInitialized) return@withLock true
84+
withContext(dispatcher) {
85+
// Check if ZIM files folder exist if not then create the folder first.
86+
if (!File(localBookFolderPath).isFileExist()) {
87+
File(localBookFolderPath).mkdirs()
88+
}
89+
// Check if library file exist if not then create the file to save the library with book information.
90+
if (!libraryFile.isFileExist()) {
91+
libraryFile.createNewFile()
92+
}
93+
// set up manager to read the library from this file
94+
manager.readFile(libraryFile.canonicalPath)
95+
isManagerInitialized = true
96+
}
97+
}
8398
}
8499

85100
@Suppress("InjectDispatcher")
86-
private val localBooksFlow: MutableStateFlow<List<LibkiwixBook>> by lazy {
87-
MutableStateFlow<List<LibkiwixBook>>(emptyList()).also { flow ->
101+
private val localBooksFlow: MutableStateFlow<List<LibkiwixBook>?> by lazy {
102+
MutableStateFlow<List<LibkiwixBook>?>(null).also { flow ->
88103
CoroutineScope(Dispatchers.IO).launch {
89104
runCatching {
90105
flow.emit(getBooksList())
@@ -95,6 +110,8 @@ class LibkiwixBookOnDisk @Inject constructor(
95110

96111
private suspend fun getBooksList(dispatcher: CoroutineDispatcher = Dispatchers.IO): List<LibkiwixBook> =
97112
withContext(dispatcher) {
113+
// if reading library failed, return empty list
114+
ensureInitialized()
98115
if (!booksChanged && localBooksList.isNotEmpty()) {
99116
// No changes, return the cached data
100117
return@withContext localBooksList.distinctBy(LibkiwixBook::path)
@@ -118,6 +135,7 @@ class LibkiwixBookOnDisk @Inject constructor(
118135
@OptIn(ExperimentalCoroutinesApi::class)
119136
fun books(dispatcher: CoroutineDispatcher = Dispatchers.IO) =
120137
localBooksFlow
138+
.filterNotNull()
121139
.mapLatest { booksList ->
122140
removeBooksThatAreInTrashFolder(booksList)
123141
removeBooksThatDoNotExist(booksList.toMutableList())
@@ -143,6 +161,7 @@ class LibkiwixBookOnDisk @Inject constructor(
143161
@Suppress("InjectDispatcher")
144162
suspend fun insert(libkiwixBooks: List<Book>) {
145163
withContext(Dispatchers.IO) {
164+
ensureInitialized()
146165
val existingBookIds = library.booksIds.toSet()
147166
val existingBookPaths = existingBookIds
148167
.mapNotNull { id -> library.getBookById(id)?.path }
@@ -207,6 +226,7 @@ class LibkiwixBookOnDisk @Inject constructor(
207226

208227
suspend fun delete(books: List<LibkiwixBook>) {
209228
runCatching {
229+
ensureInitialized()
210230
books.forEach {
211231
library.removeBookById(it.id)
212232
}
@@ -217,6 +237,7 @@ class LibkiwixBookOnDisk @Inject constructor(
217237

218238
suspend fun delete(bookId: String) {
219239
runCatching {
240+
ensureInitialized()
220241
library.removeBookById(bookId)
221242
writeBookMarksAndSaveLibraryToFile()
222243
updateLocalBooksFlow()
@@ -236,6 +257,7 @@ class LibkiwixBookOnDisk @Inject constructor(
236257
* to prevent potential data loss and ensures that the library holds the updated ZIM file data.
237258
*/
238259
private suspend fun writeBookMarksAndSaveLibraryToFile() {
260+
ensureInitialized()
239261
// Save the library, which contains ZIM file data.
240262
library.writeToFile(libraryFile.canonicalPath)
241263
// set the bookmark change to true so that libkiwix will return the new data.

core/src/main/java/org/kiwix/kiwixmobile/core/dao/LibkiwixBookmarks.kt

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import kotlinx.coroutines.flow.MutableStateFlow
2828
import kotlinx.coroutines.flow.flowOn
2929
import kotlinx.coroutines.flow.map
3030
import kotlinx.coroutines.launch
31-
import kotlinx.coroutines.runBlocking
31+
import kotlinx.coroutines.sync.Mutex
32+
import kotlinx.coroutines.sync.withLock
3233
import kotlinx.coroutines.withContext
3334
import org.kiwix.kiwixmobile.core.CoreApp
3435
import org.kiwix.kiwixmobile.core.R
@@ -70,6 +71,8 @@ class LibkiwixBookmarks @Inject constructor(
7071
private var bookmarksChanged: Boolean = false
7172
private var bookmarkList: List<LibkiwixBookmarkItem> = arrayListOf()
7273
private var libraryBooksList: List<String> = arrayListOf()
74+
private val initMutex = Mutex()
75+
private var initialized: Boolean = false
7376

7477
@Suppress("InjectDispatcher")
7578
private val bookmarkListFlow: MutableStateFlow<List<LibkiwixBookmarkItem>> by lazy {
@@ -101,17 +104,29 @@ class LibkiwixBookmarks @Inject constructor(
101104
File("$bookmarksFolderPath/library.xml")
102105
}
103106

104-
init {
105-
// Check if bookmark folder exist if not then create the folder first.
106-
if (runBlocking { !File(bookmarksFolderPath).isFileExist() }) File(bookmarksFolderPath).mkdir()
107-
// Check if library file exist if not then create the file to save the library with book information.
108-
if (runBlocking { !libraryFile.isFileExist() }) libraryFile.createNewFile()
109-
// set up manager to read the library from this file
110-
manager.readFile(libraryFile.canonicalPath)
111-
// Check if bookmark file exist if not then create the file to save the bookmarks.
112-
if (runBlocking { !bookmarkFile.isFileExist() }) bookmarkFile.createNewFile()
113-
// set up manager to read the bookmarks from this file
114-
manager.readBookmarkFile(bookmarkFile.canonicalPath)
107+
/**
108+
* Ensure initialization runs once. This method performs all file I/O and manager setup
109+
* on Dispatchers.IO so it won't block the main thread. It is safe to call multiple times.
110+
*/
111+
private suspend fun ensureInitialized(dispatcher: CoroutineDispatcher = Dispatchers.IO) {
112+
if (initialized) return
113+
114+
initMutex.withLock {
115+
if (initialized) return
116+
withContext(dispatcher) {
117+
// Check if bookmark folder exist if not then create the folder first.
118+
if (!File(bookmarksFolderPath).isFileExist()) File(bookmarksFolderPath).mkdir()
119+
// Check if library file exist if not then create the file to save the library with book information.
120+
if (!libraryFile.isFileExist()) libraryFile.createNewFile()
121+
// set up manager to read the library from this file
122+
manager.readFile(libraryFile.canonicalPath)
123+
// Check if bookmark file exist if not then create the file to save the bookmarks.
124+
if (!bookmarkFile.isFileExist()) bookmarkFile.createNewFile()
125+
// set up manager to read the bookmarks from this file
126+
manager.readBookmarkFile(bookmarkFile.canonicalPath)
127+
initialized = true
128+
}
129+
}
115130
}
116131

117132
fun bookmarks(): Flow<List<Page>> =
@@ -124,14 +139,16 @@ class LibkiwixBookmarks @Inject constructor(
124139
deleteBookmarks(pagesToDelete as List<LibkiwixBookmarkItem>)
125140

126141
@Suppress("InjectDispatcher")
127-
suspend fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List<String> =
128-
withContext(Dispatchers.IO) {
129-
return@withContext zimFileReader?.let { reader ->
142+
suspend fun getCurrentZimBookmarksUrl(zimFileReader: ZimFileReader?): List<String> {
143+
ensureInitialized()
144+
return withContext(Dispatchers.IO) {
145+
zimFileReader?.let { reader ->
130146
getBookmarksList()
131147
.filter { it.zimId == reader.id }
132148
.map(LibkiwixBookmarkItem::bookmarkUrl)
133149
}.orEmpty()
134150
}
151+
}
135152

136153
@Suppress("InjectDispatcher")
137154
fun bookmarkUrlsForCurrentBook(zimId: String): Flow<List<String>> =
@@ -150,6 +167,7 @@ class LibkiwixBookmarks @Inject constructor(
150167
libkiwixBookmarkItem: LibkiwixBookmarkItem,
151168
shouldWriteBookmarkToFile: Boolean = true
152169
) {
170+
ensureInitialized()
153171
if (!isBookMarkExist(libkiwixBookmarkItem)) {
154172
addBookToLibraryIfNotExist(libkiwixBookmarkItem.libKiwixBook)
155173
val bookmark =
@@ -178,7 +196,8 @@ class LibkiwixBookmarks @Inject constructor(
178196
}
179197

180198
suspend fun addBookToLibrary(file: File? = null, archive: Archive? = null) {
181-
try {
199+
ensureInitialized()
200+
runCatching {
182201
bookmarksChanged = true
183202
val book =
184203
Book().apply {
@@ -190,10 +209,10 @@ class LibkiwixBookmarks @Inject constructor(
190209
}
191210
addBookToLibraryIfNotExist(book)
192211
updateFlowBookmarkList()
193-
} catch (ignore: Exception) {
212+
}.onFailure {
194213
Log.e(
195214
TAG,
196-
"Error: Couldn't add the book to library.\nOriginal exception = $ignore"
215+
"Error: Couldn't add the book to library.\nOriginal exception = $it"
197216
)
198217
}
199218
}
@@ -228,14 +247,13 @@ class LibkiwixBookmarks @Inject constructor(
228247
bookmarks: List<LibkiwixBookmarkItem>,
229248
dispatcher: CoroutineDispatcher = Dispatchers.IO
230249
) {
231-
bookmarks.map { library.removeBookmark(it.zimId, it.bookmarkUrl) }
232-
.also {
233-
CoroutineScope(dispatcher).launch {
234-
writeBookMarksAndSaveLibraryToFile()
235-
updateFlowBookmarkList()
236-
removeBookFromLibraryIfNoRelatedBookmarksAreStored(dispatcher, bookmarks)
237-
}
238-
}
250+
CoroutineScope(dispatcher).launch {
251+
ensureInitialized()
252+
bookmarks.map { library.removeBookmark(it.zimId, it.bookmarkUrl) }
253+
writeBookMarksAndSaveLibraryToFile()
254+
updateFlowBookmarkList()
255+
removeBookFromLibraryIfNoRelatedBookmarksAreStored(dispatcher, bookmarks)
256+
}
239257
}
240258

241259
fun deleteBookmark(bookId: String, bookmarkUrl: String) {
@@ -255,6 +273,7 @@ class LibkiwixBookmarks @Inject constructor(
255273
dispatcher: CoroutineDispatcher,
256274
deletedBookmarks: List<LibkiwixBookmarkItem>
257275
) {
276+
ensureInitialized()
258277
withContext(dispatcher) {
259278
val currentBookmarks = getBookmarksList()
260279
val deletedZimIds = deletedBookmarks.map { it.zimId }.distinct()
@@ -263,6 +282,7 @@ class LibkiwixBookmarks @Inject constructor(
263282
val stillExists = currentBookmarks.any { it.zimId == zimId }
264283
if (!stillExists) {
265284
library.removeBookById(zimId)
285+
libraryBooksList = library.booksIds.toList()
266286
Log.d(TAG, "Removed book from library since no bookmarks exist for: $zimId")
267287
}
268288
}
@@ -275,6 +295,7 @@ class LibkiwixBookmarks @Inject constructor(
275295
* to prevent potential data loss and ensures that the library holds the updated ZIM file paths and favicons.
276296
*/
277297
private suspend fun writeBookMarksAndSaveLibraryToFile() {
298+
ensureInitialized()
278299
// Save the library, which contains ZIM file paths and favicons, to a file.
279300
library.writeToFile(libraryFile.canonicalPath)
280301

@@ -286,6 +307,7 @@ class LibkiwixBookmarks @Inject constructor(
286307

287308
@Suppress("ReturnCount")
288309
private suspend fun getBookmarksList(): List<LibkiwixBookmarkItem> {
310+
ensureInitialized()
289311
if (!bookmarksChanged && bookmarkList.isNotEmpty()) {
290312
// No changes, return the cached data
291313
return bookmarkList.distinctBy(LibkiwixBookmarkItem::bookmarkUrl)
@@ -400,6 +422,7 @@ class LibkiwixBookmarks @Inject constructor(
400422

401423
// Export the `bookmark.xml` file to the `Download/org.kiwix/` directory of internal storage.
402424
suspend fun exportBookmark() {
425+
ensureInitialized()
403426
try {
404427
val bookmarkDestinationFile = exportedFile("bookmark.xml")
405428
bookmarkFile.inputStream().use { inputStream ->
@@ -438,6 +461,7 @@ class LibkiwixBookmarks @Inject constructor(
438461
}
439462

440463
suspend fun importBookmarks(bookmarkFile: File) {
464+
ensureInitialized()
441465
// Create a temporary library manager to import the bookmarks.
442466
val tempLibrary = Library()
443467
Manager(tempLibrary).apply {

0 commit comments

Comments
 (0)