PageBookamrks – New Synced DB (#683)

* Create GRDBPAgeBookmarkPersistenceTests

* Fix typo in GRDBPAgeBookmarkPersistence's name

* Copy test logic from CoreData to GRDB for bookamrks

* Add a readPublisher function to DatabaseConnection

* Implement GRDBPageBookmarkPersistence

* Add some documentation to DatabaseConnection

* Test readPublisher of DatabaseConnection

* Remove GRDBPageBookmarkPersistence's conformance to PAgeBookPErsistence

* Move GRDBSyncedPageBookmarkPersistence into a separate package

* Create SyncedPageBookmarkPersistence as the return type of SyncedPageBookmarkPersistence package

* Wrap GRDBSyncedPageBookmarkPersistence behind a SyncedPageBookmarkPersistence protocol

* Test SyncedPageBookmarkPersistence

* Linting

* Linting SQLitePersistence

* Guard against removing with an empty remote ID

* Clear unneeded modifications in Package.swift

* Edit SyncedPageBookmarkPersistence's interface for consistency

* Rename another part of the SyncedPageBookmarkPersistence's name

* Add a unique constraint on page column for the synced table

* Make SyncedPageBookmarkPersistenceModel's types public

* Add a function to fetch bookmark for a given page

* Increase timeouts for publisher tests in DatabaseConnectionTests

* Linting

Data/SQLitePersistence/Sources/DatabaseConnection.swift

@@ -5,6 +5,7 @@
 //  Created by Mohamed Afifi on 2023-05-25.
 //
 
+import Combine
 import Foundation
 import GRDB
 import Utilities
@@ -109,6 +110,20 @@ public final class DatabaseConnection: Sendable {
         }
     }
 
+    /// Creates a publisher that tracks changes in the results of database requests,
+    /// and notifies fresh values whenever the database changes
+    ///
+    /// The first value is notified when the publisher is created. Subsequent changes *may* get coalesced in notifications.
+    public func readPublisher<T>(_ block: @Sendable @escaping (Database) throws -> T) throws -> AnyPublisher<T, Error> {
+        // - ValueObservation may coalesce subsequent changes into a single notification
+        // - ValueObservation fetches a fresh value immediately after a change *is committed in the database*.
+        // - By default, delivers on the main thread.
+        ValueObservation
+            .tracking(block)
+            .publisher(in: try getDatabase())
+            .eraseToAnyPublisher()
+    }
+
     public func write<T>(_ block: @Sendable @escaping (Database) throws -> T) async throws -> T {
         let database = try getDatabase()
         do {

Data/SQLitePersistence/Tests/DatabaseConnectionTests.swift

@@ -6,6 +6,7 @@
 //
 
 import AsyncUtilitiesForTesting
+import Combine
 import GRDB
 import XCTest
 @testable import SQLitePersistence
@@ -41,6 +42,52 @@ class DatabaseConnectionTests: XCTestCase {
         XCTAssertEqual(names, ["Alice"])
     }
 
+    func test_readPublisher() async throws {
+        // Since we can't guarantee a defined sequence of intermediate states, the test
+        // here attempts to perform a series of changes in a way that would eventually
+        // deliver a specifc set of values.
+        // Adding some latency is key, as it allows SQLite to commit changes to the desk.
+        let connection = DatabaseConnection(url: testURL, readonly: false)
+        let publisher = try connection.namesPublisher()
+            .catch { _ in Empty<[String], Never>() }
+            .eraseToAnyPublisher()
+
+        var assertExpectation: XCTestExpectation?
+        var expectedNames: [String]?
+        let cancellable = publisher.sink { names in
+            guard let expected = expectedNames else { return }
+
+            if Set(expected) == Set(names) {
+                assertExpectation?.fulfill()
+                assertExpectation = nil
+                expectedNames = nil
+            }
+        }
+
+        expectedNames = ["Alice"]
+        let expectation1 = expectation(description: "Expected to deliver the first batch of inserted names")
+        assertExpectation = expectation1
+        try await connection.insertNames()
+        await fulfillment(of: [expectation1], timeout: 2)
+
+        // Add more
+        expectedNames = ["Alice", "Bob", "Derek"]
+        let expectation2 = expectation(description: "Expected to deliver the second batch of names")
+        assertExpectation = expectation2
+        try await connection.insert(name: "Bob")
+        try await connection.insert(name: "Derek")
+        await fulfillment(of: [expectation2], timeout: 2)
+
+        // Remove one
+        expectedNames = ["Alice", "Derek"]
+        let expectation3 = expectation(description: "Expected to deliver the third batch of names")
+        assertExpectation = expectation3
+        try await connection.remove(name: "Bob")
+        await fulfillment(of: [expectation3], timeout: 2)
+
+        cancellable.cancel()
+    }
+
     func test_sharing() async throws {
         let connection1 = DatabaseConnection(url: testURL, readonly: false)
         let connection2 = DatabaseConnection(url: testURL)
@@ -85,6 +132,24 @@ private extension DatabaseConnection {
         }
     }
 
+    func insert(name: String) async throws {
+        try await write { db in
+            try db.execute(sql: "INSERT INTO test (name) VALUES (?)", arguments: StatementArguments([name]))
+        }
+    }
+
+    func remove(name: String) async throws {
+        try await write { db in
+            try db.execute(sql: "DELETE FROM test WHERE name = ?", arguments: [name])
+        }
+    }
+
+    func namesPublisher() throws -> AnyPublisher<[String], Error> {
+        try readPublisher { db in
+            try String.fetchAll(db, sql: "SELECT name FROM test")
+        }
+    }
+
     func readNames() async throws -> [String] {
         try await read { db in
             try String.fetchAll(db, sql: "SELECT name FROM test")

Data/SyncedPageBookmarkPersistence/Sources/GRDBSyncedPageBookmarkPersistence.swift

@@ -0,0 +1,123 @@
+//
+//  GRDBSyncedPageBookmarkPersistence.swift
+//  QuranEngine
+//
+//  Created by Mohannad Hassan on 31/01/2025.
+//
+
+import Combine
+import Foundation
+import GRDB
+import SQLitePersistence
+import VLogging
+
+public struct GRDBSyncedPageBookmarkPersistence: SyncedPageBookmarkPersistence {
+    // MARK: Lifecycle
+
+    init(db: DatabaseConnection) {
+        self.db = db
+        do {
+            try migrator.migrate(db)
+        } catch {
+            logger.error("Failed to to do Page Bookmarks migration: \(error)")
+        }
+    }
+
+    public init(directory: URL) {
+        let fileURL = directory.appendingPathComponent("pagebookmarks.db", isDirectory: false)
+        self.init(db: DatabaseConnection(url: fileURL, readonly: false))
+    }
+
+    // MARK: Public
+
+    public func pageBookmarksPublisher() throws -> AnyPublisher<[SyncedPageBookmarkPersistenceModel], Never> {
+        do {
+            return try db.readPublisher { db in
+                try GRDBSyncedPageBookmark.fetchAll(db).map { $0.toPersistenceModel() }
+            }
+            .catch { error in
+                logger.error("Error in page bookmarks publisher: \(error)")
+                return Empty<[SyncedPageBookmarkPersistenceModel], Never>()
+            }
+            .eraseToAnyPublisher()
+        } catch {
+            logger.error("Failed to create a publisher for page bookmarks: \(error)")
+            return Empty<[SyncedPageBookmarkPersistenceModel], Never>().eraseToAnyPublisher()
+        }
+    }
+
+    public func bookmark(page: Int) async throws -> SyncedPageBookmarkPersistenceModel? {
+        try await db.read { db in
+            try GRDBSyncedPageBookmark.fetchOne(
+                db.makeStatement(sql: "SELECT * FROM \(GRDBSyncedPageBookmark.databaseTableName) WHERE page = ?"),
+                arguments: [page]
+            )
+            .map { $0.toPersistenceModel() }
+        }
+    }
+
+    public func insertBookmark(_ bookmark: SyncedPageBookmarkPersistenceModel) async throws {
+        try await db.write { db in
+            var bookmark = GRDBSyncedPageBookmark(bookmark)
+            try bookmark.insert(db)
+        }
+    }
+
+    public func removeBookmark(withRemoteID remoteID: String) async throws {
+        guard !remoteID.isEmpty else {
+            logger.critical("[SyncedPageBookmarkPersistence] Attempted to remove a bookmark with an empty remote ID.")
+            fatalError()
+        }
+        try await db.write { db in
+            try db.execute(sql: "DELETE FROM \(GRDBSyncedPageBookmark.databaseTableName) WHERE remote_id = ?", arguments: [remoteID])
+        }
+    }
+
+    // MARK: Private
+
+    private let db: DatabaseConnection
+
+    private var migrator: DatabaseMigrator {
+        var migrator = DatabaseMigrator()
+        migrator.registerMigration("createPageBookmarks") { db in
+            try db.create(table: GRDBSyncedPageBookmark.databaseTableName, options: .ifNotExists) { table in
+                table.column("page", .integer).notNull().unique()
+                table.column("remote_id", .text).primaryKey()
+                table.column("creation_date", .datetime).notNull()
+            }
+        }
+        return migrator
+    }
+}
+
+private struct GRDBSyncedPageBookmark: Identifiable, Codable, FetchableRecord, MutablePersistableRecord {
+    enum CodingKeys: String, CodingKey {
+        case page
+        case creationDate = "creation_date"
+        case remoteID = "remote_id"
+    }
+
+    static var databaseTableName: String {
+        "synced_page_bookmarks"
+    }
+
+    var page: Int
+    var creationDate: Date
+    var remoteID: String
+
+    var id: Int {
+        page
+    }
+}
+
+extension GRDBSyncedPageBookmark {
+    init(_ bookmark: SyncedPageBookmarkPersistenceModel) {
+        page = bookmark.page
+        creationDate = bookmark.creationDate
+        remoteID = bookmark.remoteID
+    }
+
+    func toPersistenceModel() -> SyncedPageBookmarkPersistenceModel {
+        .init(page: page, remoteID: remoteID, creationDate: creationDate)
+    }
+}

Data/SyncedPageBookmarkPersistence/Sources/SyncedPageBookmarkPersistence.swift

@@ -0,0 +1,16 @@
+//
+//  SyncedPageBookmarkPersistence.swift
+//  QuranEngine
+//
+//  Created by Mohannad Hassan on 09/02/2025.
+//
+
+import Combine
+import Foundation
+
+public protocol SyncedPageBookmarkPersistence {
+    func pageBookmarksPublisher() throws -> AnyPublisher<[SyncedPageBookmarkPersistenceModel], Never>
+    func bookmark(page: Int) async throws -> SyncedPageBookmarkPersistenceModel?
+    func insertBookmark(_ bookmark: SyncedPageBookmarkPersistenceModel) async throws
+    func removeBookmark(withRemoteID remoteID: String) async throws
+}

Data/SyncedPageBookmarkPersistence/Sources/SyncedPageBookmarkPersistenceModel.swift

@@ -0,0 +1,20 @@
+//
+//  SyncedPageBookmarkPersistenceModel.swift
+//  QuranEngine
+//
+//  Created by Mohannad Hassan on 09/02/2025.
+//
+
+import Foundation
+
+public struct SyncedPageBookmarkPersistenceModel {
+    public let page: Int
+    public let remoteID: String
+    public let creationDate: Date
+
+    public init(page: Int, remoteID: String, creationDate: Date) {
+        self.page = page
+        self.remoteID = remoteID
+        self.creationDate = creationDate
+    }
+}

Data/SyncedPageBookmarkPersistence/Tests/GRDBSyncedPageBookmarkPersistenceTests.swift

@@ -0,0 +1,82 @@
+//
+//  GRDBSyncedPageBookmarkPersistenceTests.swift
+//  QuranEngine
+//
+//  Created by Mohannad Hassan on 01/02/2025.
+//
+
+import AsyncUtilitiesForTesting
+import SQLitePersistence
+import XCTest
+@testable import SyncedPageBookmarkPersistence
+
+final class GRDBSyncedPageBookmarkPersistenceTests: XCTestCase {
+    private var testURL: URL!
+    private var db: DatabaseConnection!
+    private var persistence: GRDBSyncedPageBookmarkPersistence!
+
+    override func setUp() {
+        super.setUp()
+
+        testURL = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
+        db = DatabaseConnection(url: testURL, readonly: false)
+        persistence = GRDBSyncedPageBookmarkPersistence(db: db)
+    }
+
+    override func tearDown() {
+        try? FileManager.default.removeItem(at: testURL)
+        super.tearDown()
+    }
+
+    func testInsertion() async throws {
+        let pages = [1, 2, 300]
+
+        let expectedBookmarkedPages = [1, 2, 300]
+
+        let exp = expectation(description: "Expected to send the expected bookmarks")
+        let cancellable = try persistence.pageBookmarksPublisher()
+            .sink { bookmarks in
+                let pages = bookmarks.map(\.page)
+                guard Set(pages) == Set(expectedBookmarkedPages) else {
+                    return
+                }
+                exp.fulfill()
+            }
+
+        for page in pages {
+            try await persistence.insertBookmark(SyncedPageBookmarkPersistenceModel(page: page))
+        }
+        await fulfillment(of: [exp], timeout: 1)
+        cancellable.cancel()
+    }
+
+    func testDeletion() async throws {
+        let pageNos = [1, 2, 300]
+        let pages = pageNos.map(SyncedPageBookmarkPersistenceModel.init(page:))
+        for page in pages {
+            try await persistence.insertBookmark(page)
+        }
+
+        let expectedPageNumbers = [1, 300]
+        let exp = expectation(description: "Expected to send the expected bookmarks without the deleted one")
+        let cancellable = try persistence.pageBookmarksPublisher()
+            .sink { bookmarks in
+                let pages = bookmarks.map(\.page)
+                guard Set(pages) == Set(expectedPageNumbers) else {
+                    return
+                }
+                exp.fulfill()
+            }
+
+        try await persistence.removeBookmark(withRemoteID: pages[1].remoteID)
+
+        await fulfillment(of: [exp], timeout: 1)
+        cancellable.cancel()
+    }
+}
+
+private extension SyncedPageBookmarkPersistenceModel {
+    init(page: Int) {
+        self.init(page: page, remoteID: UUID().uuidString, creationDate: .distantPast)
+    }
+}

Package.swift

@@ -196,20 +196,29 @@ private func uiTargets() -> [[Target]] {
 private func dataTargets() -> [[Target]] {
     let type = TargetType.data
     return [
-        // MARK: - Core Data
+        // MARK: - Page Bookmarks
 
-        target(type, name: "LastPagePersistence", dependencies: [
+        target(type, name: "PageBookmarkPersistence", dependencies: [
             "CoreDataModel",
             "CoreDataPersistence",
+            "QuranKit",
         ], testDependencies: [
             "AsyncUtilitiesForTesting",
             "CoreDataPersistenceTestSupport",
         ]),
 
-        target(type, name: "PageBookmarkPersistence", dependencies: [
+        target(type, name: "SyncedPageBookmarkPersistence", dependencies: [
+            "SQLitePersistence",
+            .product(name: "GRDB", package: "GRDB.swift"),
+        ], testDependencies: [
+            "AsyncUtilitiesForTesting",
+        ]),
+
+        // MARK: - Core Data
+
+        target(type, name: "LastPagePersistence", dependencies: [
             "CoreDataModel",
             "CoreDataPersistence",
-            "QuranKit",
         ], testDependencies: [
             "AsyncUtilitiesForTesting",
             "CoreDataPersistenceTestSupport",