Skip to content

Commit

Permalink
Rebased and resolved conflicts.
Browse files Browse the repository at this point in the history
* Improved the test cases for saving/migrating bookmarks in libkiwix.
* Improved libkiwix bookmark saving/retrieving according to one reader.
  • Loading branch information
MohitMaliDeveloper committed Feb 8, 2024
1 parent 45c5c63 commit 3986c0a
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity
import org.kiwix.kiwixmobile.core.data.remote.ObjectBoxToLibkiwixMigrator
import org.kiwix.kiwixmobile.core.di.modules.DatabaseModule
import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
import org.kiwix.kiwixmobile.core.reader.ZimReaderSource
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.testutils.RetryRule
Expand Down Expand Up @@ -76,6 +77,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
expectedZimId,
expectedZimName,
expectedZimFilePath,
ZimReaderSource(File(expectedZimFilePath)),
expectedBookmarkUrl,
expectedTitle,
expectedFavicon
Expand Down Expand Up @@ -153,6 +155,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
expectedZimId,
expectedZimName,
expectedZimFilePath,
ZimReaderSource(File(expectedZimFilePath)),
expectedBookmarkUrl,
expectedTitle,
expectedFavicon
Expand All @@ -169,7 +172,10 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
.subscribe(
{ actualDataAfterMigration ->
assertEquals(1, actualDataAfterMigration.size)
assertEquals(actualDataAfterMigration[0].zimFilePath, expectedZimFilePath)
assertEquals(
actualDataAfterMigration[0].zimReaderSource?.toDatabase(),
expectedZimFilePath
)
assertEquals(actualDataAfterMigration[0].zimId, expectedZimId)
assertEquals(actualDataAfterMigration[0].title, expectedTitle)
assertEquals(actualDataAfterMigration[0].url, expectedBookmarkUrl)
Expand Down Expand Up @@ -213,6 +219,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
expectedZimId,
expectedZimName,
expectedZimFilePath,
ZimReaderSource(File(expectedZimFilePath)),
existingBookmarkUrl,
existingTitle,
expectedFavicon
Expand Down Expand Up @@ -269,6 +276,7 @@ class ObjectBoxToLibkiwixMigratorTest : BaseActivityTest() {
expectedZimId,
expectedZimName,
expectedZimFilePath,
ZimReaderSource(File(expectedZimFilePath)),
"https://alpine_linux/search_$it",
"title_$it",
expectedFavicon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ data class OpenFileWithNavigation(private val bookOnDisk: BooksOnDiskListItem.Bo

override fun invokeWith(activity: AppCompatActivity) {
val zimReaderSource = bookOnDisk.zimReaderSource

Check warning on line 33 in app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/org/kiwix/kiwixmobile/zimManager/fileselectView/effects/OpenFileWithNavigation.kt#L33

Added line #L33 was not covered by tests
if (!zimReaderSource.exists()) {
if (!zimReaderSource.canOpenInLibkiwix()) {
activity.toast(R.string.error_file_not_found)
} else {
activity.navigate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class LibkiwixBookmarks @Inject constructor(
getBookmarksList()
.any {
it.url == libkiwixBookmarkItem.bookmarkUrl &&
it.zimFilePath == libkiwixBookmarkItem.zimFilePath
it.zimReaderSource?.toDatabase() == libkiwixBookmarkItem.zimReaderSource?.toDatabase()
}

private fun flowableBookmarkList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class ObjectBoxToLibkiwixMigrator {
Log.e(
"MIGRATING_BOOKMARKS",
"there is an error while migrating the bookmark for\n" +
" ZIM file = ${bookmarkEntity.zimReaderSource.toDatabase()} \n" +
" ZIM file = ${bookmarkEntity.zimFilePath} \n" +
"Bookmark Title = ${bookmarkEntity.bookmarkTitle} \n" +
"Original exception is = $ignore"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ import org.kiwix.kiwixmobile.core.dao.entities.BookmarkEntity
import org.kiwix.kiwixmobile.core.page.adapter.Page
import org.kiwix.kiwixmobile.core.reader.ZimFileReader
import org.kiwix.kiwixmobile.core.reader.ZimReaderSource
import org.kiwix.kiwixmobile.core.reader.ZimReaderSource.Companion.fromDatabaseValue
import org.kiwix.libkiwix.Book
import org.kiwix.libkiwix.Bookmark

data class LibkiwixBookmarkItem(
val databaseId: Long = 0L,
override val zimId: String,
val zimName: String,
val zimFilePath: String?,
override val zimReaderSource: ZimReaderSource?,
val bookmarkUrl: String,
override val title: String,
Expand All @@ -46,8 +46,7 @@ data class LibkiwixBookmarkItem(
) : this(
zimId = libkiwixBookmark.bookId,
zimName = libkiwixBookmark.bookTitle,
zimFilePath = zimFilePath,
zimReaderSource = null,
zimReaderSource = fromDatabaseValue(zimFilePath),
bookmarkUrl = libkiwixBookmark.url,
title = libkiwixBookmark.title,
favicon = favicon,
Expand All @@ -60,8 +59,7 @@ data class LibkiwixBookmarkItem(
zimFileReader: ZimFileReader,
libKiwixBook: Book
) : this(
zimFilePath = zimFileReader.zimReaderSource.toDatabase(),
zimReaderSource = null,
zimReaderSource = zimFileReader.zimReaderSource,
zimId = libKiwixBook.id,
zimName = libKiwixBook.name,
bookmarkUrl = articleUrl,
Expand All @@ -75,7 +73,6 @@ data class LibkiwixBookmarkItem(
libkiwixBook: Book
) : this(
zimId = bookmarkEntity.zimId,
zimFilePath = bookmarkEntity.zimReaderSource.toDatabase(),
zimReaderSource = bookmarkEntity.zimReaderSource,
zimName = bookmarkEntity.zimName,
bookmarkUrl = bookmarkEntity.bookmarkUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class ZimReaderContainer @Inject constructor(private val zimFileReaderFactory: F
return
}
zimFileReader =
if (zimReaderSource?.exists() == true && zimReaderSource.canOpenInLibkiwix())
if (zimReaderSource?.exists() == true)
zimFileReaderFactory.create(zimReaderSource)
else null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,19 @@ class ZimReaderSource(
}

fun createArchive(): Archive? {
return file?.let {
Archive(it.canonicalPath)
} ?: assetFileDescriptor?.let {
Archive(
it.parcelFileDescriptor.dup().fileDescriptor,
it.startOffset,
it.length
)
if (canOpenInLibkiwix()) {
return file?.let {
Archive(it.canonicalPath)
} ?: assetFileDescriptor?.let {
Archive(
it.parcelFileDescriptor.dup().fileDescriptor,
it.startOffset,
it.length

Check warning on line 82 in core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt#L79-L82

Added lines #L79 - L82 were not covered by tests
)
}
}

return null

Check warning on line 87 in core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimReaderSource.kt#L87

Added line #L87 was not covered by tests
}

fun toDatabase(): String = file?.canonicalPath ?: uri.toString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.kiwix.kiwixmobile.core.page

import org.kiwix.kiwixmobile.core.page.adapter.Page
import org.kiwix.kiwixmobile.core.page.bookmark.adapter.BookmarkItem
import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.BookmarkState
import org.kiwix.kiwixmobile.core.page.history.adapter.HistoryListItem
Expand Down Expand Up @@ -83,13 +84,34 @@ fun bookmark(
zimReaderSource: ZimReaderSource,
bookmarkUrl: String = "bookmarkUrl",
favicon: String = "favicon"
): BookmarkItem {
return BookmarkItem(
id = id,
zimId = zimId,
zimName = zimName,
zimReaderSource = zimReaderSource,
bookmarkUrl = bookmarkUrl,
title = bookmarkTitle,
isSelected = isSelected,
favicon = favicon
)
}

fun libkiwixBookmarkItem(
bookmarkTitle: String = "bookmarkTitle",
isSelected: Boolean = false,
id: Long = 2,
zimId: String = "zimId",
zimName: String = "zimName",
zimReaderSource: ZimReaderSource,
bookmarkUrl: String = "bookmarkUrl",
favicon: String = "favicon"
): LibkiwixBookmarkItem {
return LibkiwixBookmarkItem(
id = id,
zimId = zimId,
zimName = zimName,
zimFilePath = zimReaderSource.toDatabase(),
zimReaderSource = null,
zimReaderSource = zimReaderSource,
bookmarkUrl = bookmarkUrl,
title = bookmarkTitle,
isSelected = isSelected,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ package org.kiwix.kiwixmobile.core.page.bookmark.viewmodel
import io.mockk.mockk
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.kiwix.kiwixmobile.core.page.bookmark
import org.kiwix.kiwixmobile.core.page.bookmarkState
import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
import org.kiwix.kiwixmobile.core.reader.ZimReaderSource

internal class BookmarkStateTest {
Expand All @@ -31,10 +31,10 @@ internal class BookmarkStateTest {
val zimReaderSource: ZimReaderSource = mockk()
assertThat(
bookmarkState(emptyList()).copy(
listOf(bookmark(zimReaderSource = zimReaderSource))
listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
).pageItems
).isEqualTo(
listOf(bookmark(zimReaderSource = zimReaderSource))
listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks
import org.kiwix.kiwixmobile.core.page.adapter.Page
import org.kiwix.kiwixmobile.core.page.bookmark
import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.effects.ShowDeleteBookmarksDialog
import org.kiwix.kiwixmobile.core.page.bookmark.viewmodel.effects.UpdateAllBookmarksPreference
import org.kiwix.kiwixmobile.core.page.bookmarkState
import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
import org.kiwix.kiwixmobile.core.page.viewmodel.Action
import org.kiwix.kiwixmobile.core.page.viewmodel.Action.Filter
import org.kiwix.kiwixmobile.core.page.viewmodel.Action.UpdatePages
Expand Down Expand Up @@ -90,10 +90,10 @@ internal class BookmarkViewModelTest {
assertThat(
viewModel.updatePages(
bookmarkState(),
UpdatePages(listOf(bookmark(zimReaderSource = zimReaderSource)))
UpdatePages(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
)
).isEqualTo(
bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
)
}

Expand Down Expand Up @@ -122,10 +122,10 @@ internal class BookmarkViewModelTest {
assertThat(
viewModel.updatePages(
bookmarkState(),
UpdatePages(listOf(bookmark(zimReaderSource = zimReaderSource)))
UpdatePages(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
)
).isEqualTo(
bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
)
}

Expand All @@ -136,7 +136,7 @@ internal class BookmarkViewModelTest {
viewModel.deselectAllPages(
bookmarkState(
bookmarks = listOf(
bookmark(
libkiwixBookmarkItem(
isSelected = true,
zimReaderSource = zimReaderSource
)
Expand All @@ -146,7 +146,7 @@ internal class BookmarkViewModelTest {
).isEqualTo(
bookmarkState(
bookmarks = listOf(
bookmark(
libkiwixBookmarkItem(
isSelected = false,
zimReaderSource = zimReaderSource
)
Expand All @@ -170,10 +170,10 @@ internal class BookmarkViewModelTest {
assertThat(
viewModel.copyWithNewItems(
bookmarkState(),
listOf(bookmark(zimReaderSource = zimReaderSource))
listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource))
)
).isEqualTo(
bookmarkState(listOf(bookmark(zimReaderSource = zimReaderSource)))
bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = zimReaderSource)))
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import org.junit.jupiter.api.Test
import org.kiwix.kiwixmobile.core.base.SideEffect
import org.kiwix.kiwixmobile.core.dao.NewBookmarksDao
import org.kiwix.kiwixmobile.core.main.CoreMainActivity
import org.kiwix.kiwixmobile.core.page.bookmark
import org.kiwix.kiwixmobile.core.page.bookmarkState
import org.kiwix.kiwixmobile.core.page.libkiwixBookmarkItem
import org.kiwix.kiwixmobile.core.page.viewmodel.effects.DeletePageItems
import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower
import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.DeleteAllBookmarks
Expand Down Expand Up @@ -64,7 +64,7 @@ internal class ShowDeleteBookmarksDialogTest {
val showDeleteBookmarksDialog =
ShowDeleteBookmarksDialog(
effects,
bookmarkState(listOf(bookmark(isSelected = true, zimReaderSource = mockk()))),
bookmarkState(listOf(libkiwixBookmarkItem(isSelected = true, zimReaderSource = mockk()))),
newBookmarksDao
)
mockkActivityInjection(showDeleteBookmarksDialog)
Expand All @@ -77,7 +77,7 @@ internal class ShowDeleteBookmarksDialogTest {
val showDeleteBookmarksDialog =
ShowDeleteBookmarksDialog(
effects,
bookmarkState(listOf(bookmark(zimReaderSource = mockk()))),
bookmarkState(listOf(libkiwixBookmarkItem(zimReaderSource = mockk()))),
newBookmarksDao
)
mockkActivityInjection(showDeleteBookmarksDialog)
Expand Down

0 comments on commit 3986c0a

Please sign in to comment.