Merge pull request #3893 from kiwix/Fixes#3889

Fixed: Sometimes not all Bookmarks are showing.

app/src/androidTest/java/org/kiwix/kiwixmobile/download/DownloadRobot.kt

@@ -29,9 +29,9 @@ import applyWithViewHierarchyPrinting
 import com.adevinta.android.barista.interaction.BaristaSleepInteractions
 import com.adevinta.android.barista.interaction.BaristaSwipeRefreshInteractions.refresh
 import junit.framework.AssertionFailedError
+import org.junit.Assert
 import org.kiwix.kiwixmobile.BaseRobot
 import org.kiwix.kiwixmobile.Findable.StringId.TextId
-import org.kiwix.kiwixmobile.Findable.Text
 import org.kiwix.kiwixmobile.Findable.ViewId
 import org.kiwix.kiwixmobile.R
 import org.kiwix.kiwixmobile.core.utils.files.Log
@@ -47,7 +47,6 @@ class DownloadRobot : BaseRobot() {
 
   private var retryCountForDataToLoad = 10
   private var retryCountForCheckDownloadStart = 10
-  private val zimFileTitle = "Off the Grid"
 
   fun clickLibraryOnBottomNav() {
     clickOn(ViewId(R.id.libraryFragment))
@@ -72,7 +71,17 @@ class DownloadRobot : BaseRobot() {
   }
 
   fun checkIfZimFileDownloaded() {
-    isVisible(Text(zimFileTitle))
+    pauseForBetterTestPerformance()
+    try {
+      testFlakyView({
+        onView(withId(R.id.file_management_no_files)).check(matches(isDisplayed()))
+      })
+      // if the "No files here" text found that means it failed to download the ZIM file.
+      Assert.fail("Couldn't download the zim file. The [No files here] text is visible on screen")
+    } catch (e: AssertionFailedError) {
+      // check if "No files here" text is not visible on
+      // screen that means zim file is downloaded successfully.
+    }
   }
 
   fun refreshOnlineList() {
@@ -163,8 +172,7 @@ class DownloadRobot : BaseRobot() {
     } catch (e: Exception) {
       Log.i(
         "DOWNLOAD_TEST",
-        "Failed to stop download with title [" + zimFileTitle + "]... " +
-          "Probably because it doesn't download the zim file"
+        "Failed to stop downloading. Probably because it is not downloading the zim file"
       )
     }
   }

app/src/androidTest/java/org/kiwix/kiwixmobile/download/DownloadTest.kt

@@ -140,7 +140,7 @@ class DownloadTest : BaseActivityTest() {
       }
     } catch (e: Exception) {
       Assert.fail(
-        "Couldn't find downloaded file ' Off the Grid ' Original Exception: ${e.message}"
+        "Couldn't find downloaded file\n Original Exception: ${e.message}"
       )
     }
     LeakAssertions.assertNoLeaks()

app/src/androidTest/java/org/kiwix/kiwixmobile/initial/download/InitialDownloadRobot.kt

@@ -30,18 +30,19 @@ import com.adevinta.android.barista.interaction.BaristaSleepInteractions
 import com.adevinta.android.barista.interaction.BaristaSwipeRefreshInteractions.refresh
 import junit.framework.AssertionFailedError
 import org.kiwix.kiwixmobile.BaseRobot
+import org.kiwix.kiwixmobile.Findable.StringId.TextId
 import org.kiwix.kiwixmobile.Findable.Text
 import org.kiwix.kiwixmobile.Findable.ViewId
 import org.kiwix.kiwixmobile.R
 import org.kiwix.kiwixmobile.core.utils.files.Log
 import org.kiwix.kiwixmobile.testutils.TestUtils
 import org.kiwix.kiwixmobile.testutils.TestUtils.testFlakyView
+import org.kiwix.kiwixmobile.utils.RecyclerViewMatcher
 
 fun initialDownload(func: InitialDownloadRobot.() -> Unit) =
   InitialDownloadRobot().applyWithViewHierarchyPrinting(func)
 
 class InitialDownloadRobot : BaseRobot() {
-  private val zimFileTitle = "Off the Grid"
 
   fun clickDownloadOnBottomNav() {
     clickOn(ViewId(R.id.downloadsFragment))
@@ -64,12 +65,28 @@ class InitialDownloadRobot : BaseRobot() {
     }
   }
 
-  fun waitForDataToLoad() {
-    testFlakyView({ isVisible(Text(zimFileTitle)) }, 10)
+  fun waitForDataToLoad(retryCountForDataToLoad: Int = 10) {
+    try {
+      isVisible(TextId(R.string.your_languages))
+    } catch (e: RuntimeException) {
+      if (retryCountForDataToLoad > 0) {
+        waitForDataToLoad(retryCountForDataToLoad - 1)
+        return
+      }
+      // throw the exception when there is no more retry left.
+      throw RuntimeException("Couldn't load the online library list.\n Original exception = $e")
+    }
   }
 
   fun downloadZimFile() {
-    testFlakyView({ onView(withText(zimFileTitle)).perform(click()) })
+    pauseForBetterTestPerformance()
+    testFlakyView({
+      onView(
+        RecyclerViewMatcher(R.id.libraryList).atPosition(
+          1
+        )
+      ).perform(click())
+    })
   }
 
   fun assertStorageConfigureDialogDisplayed() {
@@ -116,8 +133,7 @@ class InitialDownloadRobot : BaseRobot() {
     } catch (e: Exception) {
       Log.i(
         "INITIAL_DOWNLOAD_TEST",
-        "Failed to stop download with title [" + zimFileTitle + "]... " +
-          "Probably because it doesn't download the zim file"
+        "Failed to stop downloading. Probably because it is not downloading the zim file"
       )
     }
   }

app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/BookmarksRobot.kt

@@ -18,10 +18,14 @@
 
 package org.kiwix.kiwixmobile.page.bookmarks
 
+import androidx.recyclerview.widget.RecyclerView
 import androidx.test.espresso.Espresso.onView
 import androidx.test.espresso.action.ViewActions.click
 import androidx.test.espresso.action.ViewActions.longClick
 import androidx.test.espresso.assertion.ViewAssertions
+import androidx.test.espresso.assertion.ViewAssertions.matches
+import androidx.test.espresso.contrib.RecyclerViewActions
+import androidx.test.espresso.matcher.ViewMatchers.hasDescendant
 import androidx.test.espresso.matcher.ViewMatchers.withId
 import androidx.test.espresso.matcher.ViewMatchers.withText
 import applyWithViewHierarchyPrinting
@@ -33,8 +37,10 @@ import org.kiwix.kiwixmobile.Findable.StringId.TextId
 import org.kiwix.kiwixmobile.Findable.Text
 import org.kiwix.kiwixmobile.Findable.ViewId
 import org.kiwix.kiwixmobile.R
+import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
 import org.kiwix.kiwixmobile.testutils.TestUtils
 import org.kiwix.kiwixmobile.testutils.TestUtils.testFlakyView
+import org.kiwix.kiwixmobile.utils.StandardActions.openDrawer
 import java.util.concurrent.TimeUnit
 
 fun bookmarks(func: BookmarksRobot.() -> Unit) =
@@ -105,4 +111,21 @@ class BookmarksRobot : BaseRobot() {
   private fun pauseForBetterTestPerformance() {
     BaristaSleepInteractions.sleep(TestUtils.TEST_PAUSE_MS_FOR_SEARCH_TEST.toLong())
   }
+
+  fun openBookmarkScreen() {
+    testFlakyView({
+      openDrawer()
+      onView(withText(R.string.bookmarks)).perform(click())
+    })
+  }
+
+  fun testAllBookmarkShowing(bookmarkList: ArrayList<LibkiwixBookmarkItem>) {
+    bookmarkList.forEachIndexed { index, libkiwixBookmarkItem ->
+      testFlakyView({
+        onView(withId(R.id.recycler_view))
+          .perform(RecyclerViewActions.scrollToPosition<RecyclerView.ViewHolder>(index))
+          .check(matches(hasDescendant(withText(libkiwixBookmarkItem.title))))
+      })
+    }
+  }
 }

app/src/androidTest/java/org/kiwix/kiwixmobile/page/bookmarks/LibkiwixBookmarkTest.kt

@@ -22,24 +22,28 @@ import androidx.core.content.ContextCompat
 import androidx.core.content.edit
 import androidx.core.net.toUri
 import androidx.lifecycle.Lifecycle
+import androidx.navigation.fragment.NavHostFragment
 import androidx.preference.PreferenceManager
 import androidx.test.core.app.ActivityScenario
 import androidx.test.espresso.accessibility.AccessibilityChecks
-import androidx.test.internal.runner.junit4.statement.UiThreadStatement
 import androidx.test.platform.app.InstrumentationRegistry
 import androidx.test.uiautomator.UiDevice
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.kiwix.kiwixmobile.BaseActivityTest
 import org.kiwix.kiwixmobile.R
+import org.kiwix.kiwixmobile.core.main.CoreReaderFragment
+import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem
 import org.kiwix.kiwixmobile.core.utils.LanguageUtils.Companion.handleLocaleChange
 import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
 import org.kiwix.kiwixmobile.main.KiwixMainActivity
 import org.kiwix.kiwixmobile.main.topLevel
 import org.kiwix.kiwixmobile.nav.destination.library.LocalLibraryFragmentDirections
 import org.kiwix.kiwixmobile.testutils.RetryRule
 import org.kiwix.kiwixmobile.testutils.TestUtils
+import org.kiwix.libkiwix.Book
+import org.kiwix.libkiwix.Bookmark
 import java.io.File
 import java.io.FileOutputStream
 import java.io.OutputStream
@@ -88,30 +92,9 @@ class LibkiwixBookmarkTest : BaseActivityTest() {
     activityScenario.onActivity {
       kiwixMainActivity = it
       kiwixMainActivity.navigate(R.id.libraryFragment)
-    }
-    val loadFileStream =
-      LibkiwixBookmarkTest::class.java.classLoader.getResourceAsStream("testzim.zim")
-    val zimFile = File(
-      ContextCompat.getExternalFilesDirs(context, null)[0],
-      "testzim.zim"
-    )
-    if (zimFile.exists()) zimFile.delete()
-    zimFile.createNewFile()
-    loadFileStream.use { inputStream ->
-      val outputStream: OutputStream = FileOutputStream(zimFile)
-      outputStream.use { it ->
-        val buffer = ByteArray(inputStream.available())
-        var length: Int
-        while (inputStream.read(buffer).also { length = it } > 0) {
-          it.write(buffer, 0, length)
-        }
-      }
-    }
-
-    UiThreadStatement.runOnUiThread {
       kiwixMainActivity.navigate(
         LocalLibraryFragmentDirections.actionNavigationLibraryToNavigationReader()
-          .apply { zimFileUri = zimFile.toUri().toString() }
+          .apply { zimFileUri = getZimFile().toUri().toString() }
       )
     }
     bookmarks {
@@ -143,4 +126,78 @@ class LibkiwixBookmarkTest : BaseActivityTest() {
       clickBookmarksOnNavDrawer(BookmarksRobot::assertBookmarkSaved)
     }
   }
+
+  @Test
+  fun testSavedBookmarksShowingOnBookmarkScreen() {
+    val zimFile = getZimFile()
+    activityScenario.onActivity {
+      kiwixMainActivity = it
+      kiwixMainActivity.navigate(R.id.libraryFragment)
+      kiwixMainActivity.navigate(
+        LocalLibraryFragmentDirections.actionNavigationLibraryToNavigationReader()
+          .apply { zimFileUri = zimFile.toUri().toString() }
+      )
+    }
+    bookmarks {
+      // delete any bookmark if already saved to properly perform this test case.
+      longClickOnSaveBookmarkImage()
+      clickOnTrashIcon()
+      assertDeleteBookmarksDialogDisplayed()
+      clickOnDeleteButton()
+      assertNoBookMarkTextDisplayed()
+      pressBack()
+    }
+    val navHostFragment: NavHostFragment =
+      kiwixMainActivity.supportFragmentManager
+        .findFragmentById(R.id.nav_host_fragment) as NavHostFragment
+    val coreReaderFragment = navHostFragment.childFragmentManager.fragments[0] as CoreReaderFragment
+    val libKiwixBook = Book().apply {
+      update(coreReaderFragment.zimReaderContainer?.zimFileReader?.jniKiwixReader)
+    }
+    val bookmarkList = arrayListOf<LibkiwixBookmarkItem>()
+    for (i in 1..500) {
+      val bookmark = Bookmark().apply {
+        bookId = coreReaderFragment.zimReaderContainer?.zimFileReader?.id
+        title = "bookmark$i"
+        url = "http://kiwix.org/demoBookmark$i"
+        bookTitle = libKiwixBook.title
+      }
+      val libkiwixItem =
+        LibkiwixBookmarkItem(
+          bookmark,
+          coreReaderFragment.zimReaderContainer?.zimFileReader?.favicon,
+          coreReaderFragment.zimReaderContainer?.zimFileReader?.zimFile?.canonicalPath
+        )
+      coreReaderFragment.libkiwixBookmarks?.saveBookmark(libkiwixItem).also {
+        bookmarkList.add(libkiwixItem)
+      }
+    }
+    bookmarks {
+      // test all the saved bookmarks are showing on the bookmarks screen
+      openBookmarkScreen()
+      testAllBookmarkShowing(bookmarkList)
+    }
+  }
+
+  private fun getZimFile(): File {
+    val loadFileStream =
+      LibkiwixBookmarkTest::class.java.classLoader.getResourceAsStream("testzim.zim")
+    val zimFile = File(
+      ContextCompat.getExternalFilesDirs(context, null)[0],
+      "testzim.zim"
+    )
+    if (zimFile.exists()) zimFile.delete()
+    zimFile.createNewFile()
+    loadFileStream.use { inputStream ->
+      val outputStream: OutputStream = FileOutputStream(zimFile)
+      outputStream.use { it ->
+        val buffer = ByteArray(inputStream.available())
+        var length: Int
+        while (inputStream.read(buffer).also { length = it } > 0) {
+          it.write(buffer, 0, length)
+        }
+      }
+    }
+    return zimFile
+  }
 }

core/src/main/java/org/kiwix/kiwixmobile/core/page/bookmark/adapter/LibkiwixBookmarkItem.kt

@@ -23,9 +23,10 @@ import org.kiwix.kiwixmobile.core.page.adapter.Page
 import org.kiwix.kiwixmobile.core.reader.ZimFileReader
 import org.kiwix.libkiwix.Book
 import org.kiwix.libkiwix.Bookmark
+import java.util.UUID
 
 data class LibkiwixBookmarkItem(
-  val databaseId: Long = 0L,
+  val databaseId: Long = UUID.randomUUID().mostSignificantBits and Long.MAX_VALUE,
   override val zimId: String,
   val zimName: String,
   override val zimFilePath: String?,

core/src/test/java/org/kiwix/kiwixmobile/core/page/PageTestHelpers.kt

@@ -71,6 +71,7 @@ fun historyState(
   )
 
 fun bookmark(
+  databaseId: Long,
   bookmarkTitle: String = "bookmarkTitle",
   isSelected: Boolean = false,
   id: Long = 2,
@@ -81,6 +82,7 @@ fun bookmark(
   favicon: String = "favicon"
 ): LibkiwixBookmarkItem {
   return LibkiwixBookmarkItem(
+    databaseId = databaseId,
     id = id,
     zimId = zimId,
     zimName = zimName,

core/src/test/java/org/kiwix/kiwixmobile/core/page/bookmark/viewmodel/BookmarkStateTest.kt

@@ -22,12 +22,14 @@ 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 java.util.UUID
 
 internal class BookmarkStateTest {
   @Test
   internal fun `copyNewItems should set new items to pageItems`() {
-    assertThat(bookmarkState(emptyList()).copy(listOf(bookmark())).pageItems).isEqualTo(
-      listOf(bookmark())
+    val databaseId = UUID.randomUUID().mostSignificantBits and Long.MAX_VALUE
+    assertThat(bookmarkState(emptyList()).copy(listOf(bookmark(databaseId))).pageItems).isEqualTo(
+      listOf(bookmark(databaseId))
     )
   }
 }