From f5be12e67fcf600c4072a710582a8ad3453d109a Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Tue, 13 Jan 2026 02:30:09 +0400 Subject: [PATCH] Run fewer queries for collection bookmarks --- .../CollectionBookmarksRepositoryImpl.kt | 119 ++++++++++++------ .../persistence/bookmark_collections.sq | 79 ++++++++++++ 2 files changed, 157 insertions(+), 41 deletions(-) diff --git a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/collectionbookmark/repository/CollectionBookmarksRepositoryImpl.kt b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/collectionbookmark/repository/CollectionBookmarksRepositoryImpl.kt index 6334fba..f23bb4f 100644 --- a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/collectionbookmark/repository/CollectionBookmarksRepositoryImpl.kt +++ b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/collectionbookmark/repository/CollectionBookmarksRepositoryImpl.kt @@ -9,7 +9,6 @@ import com.quran.shared.persistence.input.RemoteCollectionBookmark import com.quran.shared.persistence.model.Bookmark import com.quran.shared.persistence.model.CollectionBookmark import com.quran.shared.persistence.model.DatabaseBookmarkCollection -import com.quran.shared.persistence.repository.bookmark.extension.toBookmark import com.quran.shared.persistence.util.fromPlatform import com.quran.shared.persistence.util.toPlatform import kotlinx.coroutines.Dispatchers @@ -30,21 +29,21 @@ class CollectionBookmarksRepositoryImpl( override suspend fun getBookmarksForCollection(collectionLocalId: String): List { return withContext(Dispatchers.IO) { bookmarkCollectionQueries.value - .getCollectionBookmarksForCollection(collectionLocalId.toLong()) + .getCollectionBookmarksForCollectionWithDetails(collection_local_id = collectionLocalId.toLong()) .executeAsList() .mapNotNull { record -> - val collection = collectionQueries.value - .getCollectionByLocalId(record.collection_local_id) - .executeAsOneOrNull() - val bookmark = resolveBookmark(record) - if (collection == null || bookmark == null) { - null - } else { - record.toCollectionBookmark( - collectionRemoteId = collection.remote_id, - bookmark = bookmark - ) - } + toCollectionBookmark( + bookmarkType = record.bookmark_type, + bookmarkLocalId = record.bookmark_local_id, + page = record.page, + sura = record.sura, + ayah = record.ayah, + collectionLocalId = record.collection_local_id, + collectionRemoteId = record.collection_remote_id, + modifiedAt = record.modified_at, + localId = record.local_id, + logMissingBookmark = false + ) } } } @@ -85,29 +84,30 @@ class CollectionBookmarksRepositoryImpl( override suspend fun fetchMutatedCollectionBookmarks(): List> { return withContext(Dispatchers.IO) { - bookmarkCollectionQueries.value.getUnsyncedCollectionBookmarks() + bookmarkCollectionQueries.value.getUnsyncedCollectionBookmarksWithDetails() .executeAsList() .mapNotNull { record -> - val collection = collectionQueries.value - .getCollectionByLocalId(record.collection_local_id) - .executeAsOneOrNull() - val collectionRemoteId = collection?.remote_id + val collectionRemoteId = record.collection_remote_id if (collectionRemoteId.isNullOrEmpty()) { logger.w { "Skipping collection bookmark without remote collection ID: localId=${record.local_id}" } return@mapNotNull null } - val bookmark = resolveBookmark(record) - if (bookmark == null) { - logger.w { "Skipping collection bookmark without local bookmark: localId=${record.local_id}" } - return@mapNotNull null - } val mutation = if (record.deleted == 1L) Mutation.DELETED else Mutation.CREATED + val collectionBookmark = toCollectionBookmark( + bookmarkType = record.bookmark_type, + bookmarkLocalId = record.bookmark_local_id, + page = record.page, + sura = record.sura, + ayah = record.ayah, + collectionLocalId = record.collection_local_id, + collectionRemoteId = collectionRemoteId, + modifiedAt = record.modified_at, + localId = record.local_id, + logMissingBookmark = true + ) ?: return@mapNotNull null LocalModelMutation( mutation = mutation, - model = record.toCollectionBookmark( - collectionRemoteId = collectionRemoteId, - bookmark = bookmark - ), + model = collectionBookmark, remoteID = record.remote_id, localID = record.local_id.toString() ) @@ -214,24 +214,61 @@ class CollectionBookmarksRepositoryImpl( } } - private fun resolveBookmark(record: DatabaseBookmarkCollection): Bookmark? { - val bookmarkLocalId = record.bookmark_local_id.toLongOrNull() - if (bookmarkLocalId == null) { - logger.w { "Skipping collection bookmark with non-numeric bookmark id: ${record.bookmark_local_id}" } + private fun toCollectionBookmark( + bookmarkType: String, + bookmarkLocalId: String, + page: Long?, + sura: Long?, + ayah: Long?, + collectionLocalId: Long, + collectionRemoteId: String?, + modifiedAt: Long, + localId: Long, + logMissingBookmark: Boolean + ): CollectionBookmark? { + if (bookmarkLocalId.toLongOrNull() == null) { + logger.w { "Skipping collection bookmark with non-numeric bookmark id: $bookmarkLocalId" } return null } - return when (record.bookmark_type.uppercase()) { + val updatedAt = Instant.fromEpochMilliseconds(modifiedAt).toPlatform() + return when (bookmarkType.uppercase()) { "PAGE" -> { - val recordBookmark = pageBookmarkQueries.value - .getBookmarkByLocalId(bookmarkLocalId) - .executeAsOneOrNull() - recordBookmark?.toBookmark() + val pageValue = page?.toInt() + if (pageValue == null) { + if (logMissingBookmark) { + logger.w { "Skipping collection bookmark without local bookmark: localId=$localId" } + } + null + } else { + CollectionBookmark.PageBookmark( + collectionLocalId = collectionLocalId.toString(), + collectionRemoteId = collectionRemoteId, + bookmarkLocalId = bookmarkLocalId, + page = pageValue, + lastUpdated = updatedAt, + localId = localId.toString() + ) + } } "AYAH" -> { - val recordBookmark = ayahBookmarkQueries.value - .getBookmarkByLocalId(bookmarkLocalId) - .executeAsOneOrNull() - recordBookmark?.toBookmark() + val suraValue = sura?.toInt() + val ayahValue = ayah?.toInt() + if (suraValue == null || ayahValue == null) { + if (logMissingBookmark) { + logger.w { "Skipping collection bookmark without local bookmark: localId=$localId" } + } + null + } else { + CollectionBookmark.AyahBookmark( + collectionLocalId = collectionLocalId.toString(), + collectionRemoteId = collectionRemoteId, + bookmarkLocalId = bookmarkLocalId, + sura = suraValue, + ayah = ayahValue, + lastUpdated = updatedAt, + localId = localId.toString() + ) + } } else -> null } diff --git a/persistence/src/commonMain/sqldelight/com/quran/shared/persistence/bookmark_collections.sq b/persistence/src/commonMain/sqldelight/com/quran/shared/persistence/bookmark_collections.sq index 477ce53..b4ec344 100644 --- a/persistence/src/commonMain/sqldelight/com/quran/shared/persistence/bookmark_collections.sq +++ b/persistence/src/commonMain/sqldelight/com/quran/shared/persistence/bookmark_collections.sq @@ -23,6 +23,85 @@ getCollectionBookmarksForCollection: WHERE collection_local_id = ? AND deleted = 0 ORDER BY created_at DESC; +getCollectionBookmarksWithDetails: + SELECT + bc.local_id, + bc.remote_id, + bc.bookmark_local_id, + bc.bookmark_type, + bc.collection_local_id, + bc.created_at, + bc.modified_at, + bc.deleted, + c.remote_id AS collection_remote_id, + c.name AS collection_name, + pb.page AS page, + ab.sura AS sura, + ab.ayah AS ayah + FROM bookmark_collection bc + JOIN collection c ON c.local_id = bc.collection_local_id + LEFT JOIN page_bookmark pb + ON pb.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'PAGE' + LEFT JOIN ayah_bookmark ab + ON ab.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'AYAH' + WHERE bc.deleted = 0 + ORDER BY bc.created_at DESC; + +getCollectionBookmarksForCollectionWithDetails: + SELECT + bc.local_id, + bc.remote_id, + bc.bookmark_local_id, + bc.bookmark_type, + bc.collection_local_id, + bc.created_at, + bc.modified_at, + bc.deleted, + c.remote_id AS collection_remote_id, + c.name AS collection_name, + pb.page AS page, + ab.sura AS sura, + ab.ayah AS ayah + FROM bookmark_collection bc + JOIN collection c ON c.local_id = bc.collection_local_id + LEFT JOIN page_bookmark pb + ON pb.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'PAGE' + LEFT JOIN ayah_bookmark ab + ON ab.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'AYAH' + WHERE bc.collection_local_id = :collection_local_id + AND bc.deleted = 0 + ORDER BY bc.created_at DESC; + +getUnsyncedCollectionBookmarksWithDetails: + SELECT + bc.local_id, + bc.remote_id, + bc.bookmark_local_id, + bc.bookmark_type, + bc.collection_local_id, + bc.created_at, + bc.modified_at, + bc.deleted, + c.remote_id AS collection_remote_id, + c.name AS collection_name, + pb.page AS page, + ab.sura AS sura, + ab.ayah AS ayah + FROM bookmark_collection bc + JOIN collection c ON c.local_id = bc.collection_local_id + LEFT JOIN page_bookmark pb + ON pb.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'PAGE' + LEFT JOIN ayah_bookmark ab + ON ab.local_id = CAST(bc.bookmark_local_id AS INTEGER) + AND bc.bookmark_type = 'AYAH' + WHERE bc.remote_id IS NULL OR bc.deleted = 1 + ORDER BY bc.created_at DESC; + getCollectionBookmarkFor: SELECT * FROM bookmark_collection WHERE bookmark_local_id = ? AND collection_local_id = ? LIMIT 1;