Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixed: Input dispatching timed out in CoreReaderFragment.toggleBookmark. #4129

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package org.kiwix.kiwixmobile.webserver

import android.content.Context
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import org.kiwix.kiwixmobile.core.utils.files.Log
import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer
import org.kiwix.kiwixmobile.core.utils.files.FileUtils.getDemoFilePathForCustomApp
Expand All @@ -43,41 +45,42 @@
private val zimReaderContainer: ZimReaderContainer
) {
@Suppress("NestedBlockDepth")
fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer {
val kiwixLibrary = Library()
selectedBooksPath.forEach { path ->
try {
val book = Book().apply {
// Determine whether to create an Archive from an asset or a file path
val archive = if (path == getDemoFilePathForCustomApp(context)) {
// For custom apps using a demo file, create an Archive with FileDescriptor
val assetFileDescriptor =
zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0)
val startOffset = assetFileDescriptor?.startOffset ?: 0L
val size = assetFileDescriptor?.length ?: 0L
Archive(
assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor,
startOffset,
size
)
} else {
// For regular files, create an Archive from the file path
Archive(path)
suspend fun createKiwixServer(selectedBooksPath: ArrayList<String>): KiwixServer =
withContext(Dispatchers.IO) {
val kiwixLibrary = Library()
selectedBooksPath.forEach { path ->
try {
val book = Book().apply {
// Determine whether to create an Archive from an asset or a file path
val archive = if (path == getDemoFilePathForCustomApp(context)) {
// For custom apps using a demo file, create an Archive with FileDescriptor
val assetFileDescriptor =

Check warning on line 57 in app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt#L57

Added line #L57 was not covered by tests
zimReaderContainer.zimReaderSource?.assetFileDescriptorList?.get(0)
val startOffset = assetFileDescriptor?.startOffset ?: 0L
val size = assetFileDescriptor?.length ?: 0L
Archive(

Check warning on line 61 in app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt#L61

Added line #L61 was not covered by tests
assetFileDescriptor?.parcelFileDescriptor?.fileDescriptor,
startOffset,
size

Check warning on line 64 in app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt#L63-L64

Added lines #L63 - L64 were not covered by tests
)
} else {
// For regular files, create an Archive from the file path
Archive(path)
}
update(archive)
}
update(archive)
kiwixLibrary.addBook(book)
} catch (ignore: Exception) {

Check warning on line 73 in app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt#L73

Added line #L73 was not covered by tests
// Catch the other exceptions as well. e.g. while hosting the split zim files.
// we have an issue with split zim files, see #3827
Log.v(
TAG,
"Couldn't add book with path:{ $path }.\n Original Exception = $ignore"

Check warning on line 78 in app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/webserver/KiwixServer.kt#L76-L78

Added lines #L76 - L78 were not covered by tests
)
}
kiwixLibrary.addBook(book)
} catch (ignore: Exception) {
// Catch the other exceptions as well. e.g. while hosting the split zim files.
// we have an issue with split zim files, see #3827
Log.v(
TAG,
"Couldn't add book with path:{ $path }.\n Original Exception = $ignore"
)
}
return@withContext KiwixServer(kiwixLibrary, Server(kiwixLibrary))
}
return KiwixServer(kiwixLibrary, Server(kiwixLibrary))
}
}

fun startServer(port: Int): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class WebServerHelper @Inject constructor(
private var isServerStarted = false
private var validIpAddressDisposable: Disposable? = null

fun startServerHelper(
suspend fun startServerHelper(
selectedBooksPath: ArrayList<String>,
restartServer: Boolean
): ServerStatus? {
Expand All @@ -64,7 +64,7 @@ class WebServerHelper @Inject constructor(
}
}

private fun startAndroidWebServer(
private suspend fun startAndroidWebServer(
selectedBooksPath: ArrayList<String>,
restartServer: Boolean
): ServerStatus? {
Expand All @@ -78,7 +78,7 @@ class WebServerHelper @Inject constructor(
return serverStatus
}

private fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus {
private suspend fun startKiwixServer(selectedBooksPath: ArrayList<String>): ServerStatus {
var errorMessage: Int? = null
ServerUtils.port = DEFAULT_PORT
kiwixServer = kiwixServerFactory.createKiwixServer(selectedBooksPath).also {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import androidx.appcompat.widget.Toolbar
import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat
import androidx.fragment.app.FragmentActivity
import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.R.layout
import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.base.BaseActivity
import org.kiwix.kiwixmobile.core.base.BaseFragment
import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.hasNotificationPermission
Expand Down Expand Up @@ -443,6 +443,7 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
Intent(requireActivity(), HotspotService::class.java).setAction(action)

override fun onServerStarted(ip: String) {
dialog?.dismiss() // Dismiss dialog when server started.
this.ip = ip
layoutServerStarted()
}
Expand All @@ -452,6 +453,7 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
}

override fun onServerFailedToStart(errorMessage: Int?) {
dialog?.dismiss() // Dismiss dialog if there is some error in starting the server.
errorMessage?.let {
toast(errorMessage)
}
Expand Down Expand Up @@ -514,7 +516,6 @@ class ZimHostFragment : BaseFragment(), ZimHostCallbacks, ZimHostContract.View {
}

override fun onIpAddressValid() {
dialog?.dismiss()
startWifiHotspot(false)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import android.content.Intent
import android.os.Binder
import android.os.IBinder
import android.widget.Toast
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.kiwix.kiwixmobile.KiwixApp
import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.extensions.registerReceiver
Expand Down Expand Up @@ -75,18 +79,22 @@ class HotspotService :
ACTION_START_SERVER -> {
val restartServer = intent.getBooleanExtra(RESTART_SERVER, false)
intent.getStringArrayListExtra(ZimHostFragment.SELECTED_ZIM_PATHS_KEY)?.let {
val serverStatus = webServerHelper?.startServerHelper(it, restartServer)
if (serverStatus?.isServerStarted == true) {
zimHostCallbacks?.onServerStarted(getSocketAddress())
startForegroundNotificationHelper()
if (!restartServer) {
Toast.makeText(
this, R.string.server_started_successfully_toast_message,
Toast.LENGTH_SHORT
).show()
CoroutineScope(Dispatchers.Main).launch {
val serverStatus = withContext(Dispatchers.IO) {
webServerHelper?.startServerHelper(it, restartServer)
}
if (serverStatus?.isServerStarted == true) {
zimHostCallbacks?.onServerStarted(getSocketAddress())
startForegroundNotificationHelper()
if (!restartServer) {
Toast.makeText(
this@HotspotService, R.string.server_started_successfully_toast_message,
Toast.LENGTH_SHORT
).show()
}
} else {
onServerFailedToStart(serverStatus?.errorMessage)
}
} else {
onServerFailedToStart(serverStatus?.errorMessage)
}
} ?: kotlin.run { onServerFailedToStart(R.string.no_books_selected_toast_message) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,7 @@
override fun onDestroyView() {
super.onDestroyView()
try {
coreReaderLifeCycleScope?.cancel()
readerLifeCycleScope?.cancel()
readerLifeCycleScope = null
} catch (ignore: Exception) {
Expand Down Expand Up @@ -1803,7 +1804,7 @@
when (requestCode) {
REQUEST_STORAGE_PERMISSION -> {
if (hasPermission(Manifest.permission.READ_EXTERNAL_STORAGE)) {
lifecycleScope.launch {
coreReaderLifeCycleScope?.launch {
zimReaderSource?.let { openZimFile(it) }
}
} else {
Expand Down Expand Up @@ -1899,32 +1900,34 @@
@Suppress("NestedBlockDepth")
private fun toggleBookmark() {
try {
getCurrentWebView()?.url?.let { articleUrl ->
zimReaderContainer?.zimFileReader?.let { zimFileReader ->
val libKiwixBook = Book().apply {
update(zimFileReader.jniKiwixReader)
}
if (isBookmarked) {
repositoryActions?.deleteBookmark(libKiwixBook.id, articleUrl)
snackBarRoot?.snack(R.string.bookmark_removed)
} else {
getCurrentWebView()?.title?.let {
repositoryActions?.saveBookmark(
LibkiwixBookmarkItem(it, articleUrl, zimFileReader, libKiwixBook)
)
snackBarRoot?.snack(
stringId = R.string.bookmark_added,
actionStringId = R.string.open,
actionClick = {
goToBookmarks()
Unit
}
)
lifecycleScope.launch {
getCurrentWebView()?.url?.let { articleUrl ->
zimReaderContainer?.zimFileReader?.let { zimFileReader ->
val libKiwixBook = Book().apply {
update(zimFileReader.jniKiwixReader)
}
if (isBookmarked) {
repositoryActions?.deleteBookmark(libKiwixBook.id, articleUrl)
snackBarRoot?.snack(R.string.bookmark_removed)
} else {
getCurrentWebView()?.title?.let {
repositoryActions?.saveBookmark(
LibkiwixBookmarkItem(it, articleUrl, zimFileReader, libKiwixBook)
)
snackBarRoot?.snack(
stringId = R.string.bookmark_added,
actionStringId = R.string.open,
actionClick = {
goToBookmarks()
Unit
}
)
}
}
}
} ?: kotlin.run {
requireActivity().toast(R.string.unable_to_add_to_bookmarks, Toast.LENGTH_SHORT)

Check warning on line 1929 in core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt#L1928-L1929

Added lines #L1928 - L1929 were not covered by tests
}
} ?: kotlin.run {
requireActivity().toast(R.string.unable_to_add_to_bookmarks, Toast.LENGTH_SHORT)
}
} catch (ignore: Exception) {
// Catch the exception while saving the bookmarks for splitted zim files.
Expand Down