Fix some race conditions

This changes the API to require clients to pass in an initial list of
bookmarks. This should ensure that it's not possible for the reader to
switch to a page _before_ having been informed that there's already a
bookmark for a later page. This also removes some deprecated events,
and replaces the broken bookmark creation with a much simpler system.

Additionally, view events are now published from a
BehaviorSubject. This allows for activities to avoid mild race
conditions when resuming.

Affects: https://ebce-lyrasis.atlassian.net/browse/PP-1120
Affects: https://ebce-lyrasis.atlassian.net/browse/PP-1119

gradle.properties

@@ -11,7 +11,7 @@ POM_SCM_CONNECTION=scm:git:git://github.com/ThePalaceProject/android-r2
 POM_SCM_DEV_CONNECTION=scm:git:ssh://[email protected]/ThePalaceProject/android-r2
 POM_SCM_URL=http://github.com/ThePalaceProject/android-r2
 POM_URL=http://github.com/ThePalaceProject/android-r2
-VERSION_NAME=3.2.0-SNAPSHOT
+VERSION_NAME=4.0.0-SNAPSHOT
 VERSION_PREVIOUS=3.1.0
 
 android.useAndroidX=true

org.librarysimplified.r2.api/src/main/java/org/librarysimplified/r2/api/SR2Bookmark.kt

@@ -44,12 +44,6 @@ data class SR2Bookmark(
    */
 
   val uri: URI?,
-
-  /**
-   * A flag that indicates if the bookmark is being deleted or not.
-   */
-
-  var isBeingDeleted: Boolean = false,
 ) {
 
   init {

org.librarysimplified.r2.api/src/main/java/org/librarysimplified/r2/api/SR2Command.kt

@@ -99,16 +99,6 @@ sealed class SR2Command {
 
   data object CancelSearch : SR2Command()
 
-  /**
-   * Load a set of bookmarks into the controller. This merely has the effect of making
-   * the bookmarks available to the table of contents; it does not trigger any changes
-   * in navigation.
-   */
-
-  data class BookmarksLoad(
-    val bookmarks: List<SR2Bookmark>,
-  ) : SR2Command()
-
   /**
    * Create a new (explicit) bookmark at the current reading position.
    */

org.librarysimplified.r2.api/src/main/java/org/librarysimplified/r2/api/SR2ControllerConfiguration.kt

@@ -58,4 +58,10 @@ data class SR2ControllerConfiguration(
    */
 
   val pageNumberingMode: SR2PageNumberingMode,
+
+  /**
+   * The initial set of bookmarks.
+   */
+
+  val initialBookmarks: List<SR2Bookmark>,
 )

org.librarysimplified.r2.api/src/main/java/org/librarysimplified/r2/api/SR2ControllerType.kt

@@ -51,7 +51,7 @@ interface SR2ControllerType : Closeable, SR2ControllerCommandQueueType {
 
   /**
    * The list of bookmarks currently loaded into the controller. This list is an immutable
-   * snapshots, and subsequent updates to bookmarks will not be reflected in the returned list.
+   * snapshot, and subsequent updates to bookmarks will not be reflected in the returned list.
    */
 
   fun bookmarksNow(): List<SR2Bookmark>

org.librarysimplified.r2.api/src/main/java/org/librarysimplified/r2/api/SR2Event.kt

@@ -1,6 +1,5 @@
 package org.librarysimplified.r2.api
 
-import org.readium.r2.shared.Search
 import org.readium.r2.shared.publication.Href
 import org.readium.r2.shared.publication.services.search.SearchIterator
 
@@ -83,54 +82,20 @@ sealed class SR2Event {
   sealed class SR2BookmarkEvent : SR2Event() {
 
     /**
-     * Create a bookmark.
+     * A bookmark was created in the reader. The observer of this event should save the bookmark
+     * into persistent storage.
      */
-    data class SR2BookmarkCreate(
-      val bookmark: SR2Bookmark,
-      val onBookmarkCreationCompleted: (SR2Bookmark?) -> Unit,
-    ) : SR2BookmarkEvent()
-
-    /**
-     * A bookmark was created.
-     */
-
-    @Deprecated("This event will stop being published soon.")
     data class SR2BookmarkCreated(
       val bookmark: SR2Bookmark,
     ) : SR2BookmarkEvent()
 
     /**
-     * A bookmark was deleted.
+     * A bookmark was deleted in the reader. The observer of this event should delete the bookmark
+     * from persistent storage.
      */
-
-    @Deprecated("This event will stop being published soon.")
     data class SR2BookmarkDeleted(
       val bookmark: SR2Bookmark,
     ) : SR2BookmarkEvent()
-
-    /**
-     * A bookmark failed to be deleted.
-     */
-
-    object SR2BookmarkFailedToBeDeleted : SR2BookmarkEvent()
-
-    /**
-     * Try to delete a bookmark.
-     */
-
-    data class SR2BookmarkTryToDelete(
-      val bookmark: SR2Bookmark,
-      val onDeleteOperationFinished: (Boolean) -> Unit,
-    ) : SR2BookmarkEvent()
-
-    /**
-     * Bookmarks were loaded into the controller.
-     */
-
-    object SR2BookmarksLoaded : SR2BookmarkEvent() {
-      override fun toString(): String =
-        "[SR2BookmarksLoaded]"
-    }
   }
 
   /**
@@ -182,7 +147,6 @@ sealed class SR2Event {
      * The searching command is finished and returns a search iterator containing the results
      */
 
-    @OptIn(Search::class)
     data class SR2CommandSearchResults constructor(
       override val command: SR2Command,
       val searchIterator: SearchIterator?,

org.librarysimplified.r2.demo/src/main/java/org/librarysimplified/r2/demo/DemoActivity.kt

@@ -12,14 +12,9 @@ import androidx.appcompat.widget.Toolbar
 import androidx.fragment.app.Fragment
 import io.reactivex.disposables.CompositeDisposable
 import kotlinx.coroutines.runBlocking
-import org.librarysimplified.r2.api.SR2Command
 import org.librarysimplified.r2.api.SR2Event
-import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarkCreate
 import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarkCreated
 import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarkDeleted
-import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarkFailedToBeDeleted
-import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarkTryToDelete
-import org.librarysimplified.r2.api.SR2Event.SR2BookmarkEvent.SR2BookmarksLoaded
 import org.librarysimplified.r2.api.SR2Event.SR2CommandEvent.SR2CommandEventCompleted.SR2CommandExecutionFailed
 import org.librarysimplified.r2.api.SR2Event.SR2CommandEvent.SR2CommandEventCompleted.SR2CommandExecutionSucceeded
 import org.librarysimplified.r2.api.SR2Event.SR2CommandEvent.SR2CommandExecutionRunningLong
@@ -131,20 +126,6 @@ class DemoActivity : AppCompatActivity(R.layout.demo_activity_host) {
   @UiThread
   private fun onControllerBecameAvailable(reference: SR2ControllerReference) {
     this.switchFragment(SR2ReaderFragment())
-
-    if (reference.isFirstStartup) {
-      // Navigate to the first chapter or saved reading position.
-      val bookId = reference.controller.bookMetadata.id
-      reference.controller.submitCommand(
-        SR2Command.BookmarksLoad(DemoModel.database.bookmarksFor(bookId)),
-      )
-      val lastRead = DemoModel.database.bookmarkFindLastReadLocation(bookId)
-      val startLocator = lastRead?.locator ?: reference.controller.bookMetadata.start
-      reference.controller.submitCommand(SR2Command.OpenChapter(startLocator))
-    } else {
-      // Refresh whatever the controller was looking at previously.
-      reference.controller.submitCommand(SR2Command.Refresh)
-    }
   }
 
   /**
@@ -178,6 +159,7 @@ class DemoActivity : AppCompatActivity(R.layout.demo_activity_host) {
       theme = DemoModel.database.theme(),
       context = DemoApplication.application,
       controllers = SR2Controllers(),
+      bookmarks = DemoModel.database.bookmarksFor(id),
     )
   }
 
@@ -275,12 +257,11 @@ class DemoActivity : AppCompatActivity(R.layout.demo_activity_host) {
 
   private fun onControllerEvent(event: SR2Event) {
     when (event) {
-      is SR2BookmarkCreate -> {
+      is SR2BookmarkCreated -> {
         DemoModel.database.bookmarkSave(
           SR2ReaderModel.controller().bookMetadata.id,
           event.bookmark,
         )
-        event.onBookmarkCreationCompleted(event.bookmark)
       }
 
       is SR2BookmarkDeleted -> {
@@ -294,12 +275,8 @@ class DemoActivity : AppCompatActivity(R.layout.demo_activity_host) {
         DemoModel.database.themeSet(event.theme)
       }
 
-      is SR2BookmarkCreated,
       is SR2OnCenterTapped,
       is SR2ReadingPositionChanged,
-      SR2BookmarksLoaded,
-      SR2BookmarkFailedToBeDeleted,
-      is SR2BookmarkTryToDelete,
       is SR2ChapterNonexistent,
       is SR2WebViewInaccessible,
       is SR2ExternalLinkSelected,

org.librarysimplified.r2.tests/src/test/java/org/librarysimplified/r2/tests/SR2NavigationGraphsTest.kt

@@ -85,7 +85,7 @@ class SR2NavigationGraphsTest {
     assertEquals(4, graph.resources.size)
     assertEquals("Something", graph.readingOrder[0].navigationPoint.title)
 
-    var target: SR2NavigationTarget? = null
+    var target: SR2NavigationTarget?
     target = graph.findNavigationNode(SR2LocatorPercent.start(Href("/epub/text/p1.xhtml")!!))
     assertEquals("Something Nested", target!!.node.navigationPoint.title)
     assertEquals(null, target.extraFragment)
@@ -142,7 +142,7 @@ class SR2NavigationGraphsTest {
     node = graph.findNextNode(node)
     assertEquals(null, node)
 
-    var target: SR2NavigationTarget? = null
+    var target: SR2NavigationTarget?
     target = graph.findNavigationNode(SR2LocatorPercent.start(Href("/epub/text/p0.xhtml")!!))
     assertEquals("Chapter 0", target!!.node.navigationPoint.title)
     assertEquals(null, target.extraFragment)

org.librarysimplified.r2.tests/src/test/java/org/librarysimplified/r2/tests/TestDirectories.kt

@@ -26,7 +26,7 @@ object TestDirectories {
 
   @Throws(IOException::class)
   fun temporaryBaseDirectory(): File {
-    val tmpBase = File(System.getProperty("java.io.tmpdir"))
+    val tmpBase = File(System.getProperty("java.io.tmpdir")!!)
     val path1 = File(tmpBase, "org.nypl.simplified")
     path1.mkdirs()
     return path1