diff --git a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/model/AyahBookmark.kt b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/model/AyahBookmark.kt index 10bfd4ae..6d757585 100644 --- a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/model/AyahBookmark.kt +++ b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/model/AyahBookmark.kt @@ -5,6 +5,7 @@ import com.quran.shared.persistence.util.PlatformDateTime data class AyahBookmark( val sura: Int, val ayah: Int, + val localId: String, val lastUpdated: PlatformDateTime, - val localId: String + val addedDate: PlatformDateTime = lastUpdated ) diff --git a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/extension/BookmarkQueriesExtensions.kt b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/extension/BookmarkQueriesExtensions.kt index 55f77402..3d4ba19a 100644 --- a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/extension/BookmarkQueriesExtensions.kt +++ b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/extension/BookmarkQueriesExtensions.kt @@ -14,8 +14,9 @@ internal fun DatabaseBookmark.toAyahBookmark(): AyahBookmark { return AyahBookmark( sura = requireNotNull(sura).toInt(), ayah = requireNotNull(ayah).toInt(), + localId = local_id.toString(), lastUpdated = Instant.fromEpochMilliseconds(modified_at).toPlatform(), - localId = local_id.toString() + addedDate = Instant.fromEpochMilliseconds(created_at).toPlatform() ) } diff --git a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepository.kt b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepository.kt index fe2b2d15..c7a9d702 100644 --- a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepository.kt +++ b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepository.kt @@ -37,39 +37,39 @@ interface BookmarksRepository { /** * Add a saved ayah bookmark and add it to the requested memberships. * - * Null or empty memberships normalize to the virtual default collection. A non-empty list is + * Empty memberships normalize to the virtual default collection. A non-empty list is * additive: requested memberships are added, while existing custom memberships not present in * the list are left unchanged. [com.quran.shared.persistence.model.DEFAULT_COLLECTION_ID] * represents default membership. */ @NativeCoroutines - suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List?): AyahBookmark + suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List): AyahBookmark @NativeCoroutines suspend fun addBookmark( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): AyahBookmark /** * Replaces the saved collection memberships for an existing ayah bookmark. * - * Null or empty memberships normalize to the virtual default collection. Use [deleteBookmark] + * Empty memberships normalize to the virtual default collection. Use [deleteBookmark] * when a saved bookmark should be removed from every collection. * * @return `true` when memberships changed, or `false` when the bookmark is missing, deleted, * or already has exactly the requested memberships. */ @NativeCoroutines - suspend fun replaceBookmarkCollections(localId: String, collectionLocalIds: List?): Boolean + suspend fun replaceBookmarkCollections(localId: String, collectionLocalIds: List): Boolean /** * Replaces the saved collection memberships for an existing ayah bookmark with an explicit * mutation timestamp. * - * Null or empty memberships normalize to the virtual default collection. Use [deleteBookmark] + * Empty memberships normalize to the virtual default collection. Use [deleteBookmark] * when a saved bookmark should be removed from every collection. * * @return `true` when memberships changed, or `false` when the bookmark is missing, deleted, @@ -78,33 +78,33 @@ interface BookmarksRepository { @NativeCoroutines suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): Boolean /** * Creates an ayah bookmark if needed, then replaces its saved collection memberships exactly. * - * Null or empty memberships normalize to the virtual default collection. + * Empty memberships normalize to the virtual default collection. */ @NativeCoroutines suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List? + collectionLocalIds: List ): BookmarkCollectionsReplacementResult /** * Creates an ayah bookmark if needed, then replaces its saved collection memberships exactly * with an explicit mutation timestamp. * - * Null or empty memberships normalize to the virtual default collection. + * Empty memberships normalize to the virtual default collection. */ @NativeCoroutines suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): BookmarkCollectionsReplacementResult diff --git a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepositoryImpl.kt b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepositoryImpl.kt index d2004a81..0d6f782c 100644 --- a/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepositoryImpl.kt +++ b/persistence/src/commonMain/kotlin/com/quran/shared/persistence/repository/bookmark/repository/BookmarksRepositoryImpl.kt @@ -63,14 +63,14 @@ class BookmarksRepositoryImpl( } override suspend fun addBookmark(sura: Int, ayah: Int): AyahBookmark { - return addBookmark(sura = sura, ayah = ayah, collectionLocalIds = null) + return addBookmark(sura = sura, ayah = ayah, collectionLocalIds = emptyList()) } override suspend fun addBookmark(sura: Int, ayah: Int, timestamp: PlatformDateTime): AyahBookmark { return addBookmark( sura = sura, ayah = ayah, - collectionLocalIds = null, + collectionLocalIds = emptyList(), timestamp = timestamp ) } @@ -78,7 +78,7 @@ class BookmarksRepositoryImpl( override suspend fun addBookmark( sura: Int, ayah: Int, - collectionLocalIds: List? + collectionLocalIds: List ): AyahBookmark { return addBookmarkWithTimestampMillis( sura = sura, @@ -91,7 +91,7 @@ class BookmarksRepositoryImpl( override suspend fun addBookmark( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): AyahBookmark { return addBookmarkWithTimestampMillis( @@ -105,7 +105,7 @@ class BookmarksRepositoryImpl( private suspend fun addBookmarkWithTimestampMillis( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestampMillis: Long? ): AyahBookmark { logger.i { "Adding ayah bookmark for $sura:$ayah" } @@ -163,7 +163,7 @@ class BookmarksRepositoryImpl( override suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List? + collectionLocalIds: List ): Boolean { return replaceBookmarkCollectionsWithTimestampMillis( localId = localId, @@ -174,7 +174,7 @@ class BookmarksRepositoryImpl( override suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): Boolean { return replaceBookmarkCollectionsWithTimestampMillis( @@ -186,7 +186,7 @@ class BookmarksRepositoryImpl( private suspend fun replaceBookmarkCollectionsWithTimestampMillis( localId: String, - collectionLocalIds: List?, + collectionLocalIds: List, timestampMillis: Long? ): Boolean { logger.i { "Replacing ayah bookmark collection memberships localId=$localId" } @@ -210,7 +210,7 @@ class BookmarksRepositoryImpl( override suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List? + collectionLocalIds: List ): BookmarkCollectionsReplacementResult { return replaceAyahBookmarkCollectionsWithTimestampMillis( sura = sura, @@ -223,7 +223,7 @@ class BookmarksRepositoryImpl( override suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): BookmarkCollectionsReplacementResult { return replaceAyahBookmarkCollectionsWithTimestampMillis( @@ -237,7 +237,7 @@ class BookmarksRepositoryImpl( private suspend fun replaceAyahBookmarkCollectionsWithTimestampMillis( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestampMillis: Long? ): BookmarkCollectionsReplacementResult { logger.i { "Replacing ayah bookmark collection memberships for $sura:$ayah" } @@ -326,7 +326,7 @@ class BookmarksRepositoryImpl( private fun replaceBookmarkCollectionsInTransaction( bookmark: DatabaseBookmark, - collectionLocalIds: List?, + collectionLocalIds: List, timestampMillis: Long? ): Boolean { require(bookmark.bookmark_type == "AYAH") { @@ -809,12 +809,11 @@ class BookmarksRepositoryImpl( } } - private fun normalizeCollectionIds(collectionLocalIds: List?): List { + private fun normalizeCollectionIds(collectionLocalIds: List): List { val nonBlankIds = collectionLocalIds - ?.map { it.trim() } - ?.filter { it.isNotEmpty() } - ?.distinct() - .orEmpty() + .map { it.trim() } + .filter { it.isNotEmpty() } + .distinct() return nonBlankIds.ifEmpty { listOf(DEFAULT_COLLECTION_ID) } } diff --git a/persistence/src/commonTest/kotlin/com/quran/shared/persistence/repository/BookmarkSyncArchitectureTest.kt b/persistence/src/commonTest/kotlin/com/quran/shared/persistence/repository/BookmarkSyncArchitectureTest.kt index 83334b1a..fe935090 100644 --- a/persistence/src/commonTest/kotlin/com/quran/shared/persistence/repository/BookmarkSyncArchitectureTest.kt +++ b/persistence/src/commonTest/kotlin/com/quran/shared/persistence/repository/BookmarkSyncArchitectureTest.kt @@ -336,6 +336,17 @@ class BookmarkSyncArchitectureTest { assertTrue(defaults.all { it.collectionLocalId == DEFAULT_COLLECTION_ID }) } + @Test + fun `saved bookmark exposes added date separately from last updated`() = runTest { + bookmarksRepository.addBookmark(2, 20, listOf(DEFAULT_COLLECTION_ID), at(100)) + bookmarksRepository.addBookmark(2, 20, listOf(DEFAULT_COLLECTION_ID), at(250)) + + val bookmark = bookmarksRepository.getAllBookmarks().single() + + assertEquals(100L, bookmark.addedDate.fromPlatform().toEpochMilliseconds()) + assertEquals(250L, bookmark.lastUpdated.fromPlatform().toEpochMilliseconds()) + } + @Test fun `addBookmark supports default and custom membership together`() = runTest { val collectionId = createCollection("Both", "remote-both") diff --git a/sync-pipelines/src/commonMain/kotlin/com/quran/shared/pipeline/QuranDataService.kt b/sync-pipelines/src/commonMain/kotlin/com/quran/shared/pipeline/QuranDataService.kt index c16400c1..67c96122 100644 --- a/sync-pipelines/src/commonMain/kotlin/com/quran/shared/pipeline/QuranDataService.kt +++ b/sync-pipelines/src/commonMain/kotlin/com/quran/shared/pipeline/QuranDataService.kt @@ -8,6 +8,8 @@ import com.quran.shared.auth.repository.RemoteLogoutFailure import com.quran.shared.auth.repository.RemoteLogoutOperation import com.quran.shared.auth.service.AuthService import com.quran.shared.di.AppScope +import com.quran.shared.persistence.input.PersistenceImportData +import com.quran.shared.persistence.input.PersistenceImportResult import com.quran.shared.persistence.model.AyahBookmark import com.quran.shared.persistence.model.AyahReadingBookmark import com.quran.shared.persistence.model.Collection @@ -18,18 +20,15 @@ import com.quran.shared.persistence.model.Note import com.quran.shared.persistence.model.PageReadingBookmark import com.quran.shared.persistence.model.ReadingBookmark import com.quran.shared.persistence.model.ReadingSession -import com.quran.shared.persistence.input.PersistenceImportData -import com.quran.shared.persistence.input.PersistenceImportResult -import com.quran.shared.persistence.util.PlatformDateTime -import com.quran.shared.persistence.util.toPlatform -import com.quran.shared.persistence.repository.bookmark.repository.BookmarksRepository import com.quran.shared.persistence.repository.PersistenceResetRepository +import com.quran.shared.persistence.repository.bookmark.repository.BookmarksRepository import com.quran.shared.persistence.repository.collection.repository.CollectionsRepository import com.quran.shared.persistence.repository.collectionbookmark.repository.CollectionBookmarksRepository import com.quran.shared.persistence.repository.importdata.PersistenceImportRepository import com.quran.shared.persistence.repository.note.repository.NotesRepository -import com.quran.shared.persistence.repository.readingbookmark.repository.ReadingBookmarksRepository import com.quran.shared.persistence.repository.readingsession.repository.ReadingSessionsRepository +import com.quran.shared.persistence.util.PlatformDateTime +import com.quran.shared.persistence.util.toPlatform import com.quran.shared.syncengine.AuthenticationDataFetcher import com.quran.shared.syncengine.LocalModificationDateFetcher import com.quran.shared.syncengine.SyncLifecycleGate @@ -39,10 +38,8 @@ import com.rickclephas.kmp.nativecoroutines.NativeCoroutines import com.rickclephas.kmp.nativecoroutines.NativeCoroutinesState import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn -import kotlin.native.HiddenFromObjC -import kotlin.time.Instant -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob @@ -53,6 +50,8 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlin.native.HiddenFromObjC +import kotlin.time.Instant /** * Creates the scheduler-backed synchronization client used by [QuranDataService]. @@ -399,7 +398,7 @@ class QuranDataService internal constructor( } @NativeCoroutines - suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List?): AyahBookmark { + suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List): AyahBookmark { return mutatingCall("Failed to add ayah bookmark with collection memberships") { bookmarksRepository.addBookmark(sura, ayah, collectionLocalIds) } @@ -409,7 +408,7 @@ class QuranDataService internal constructor( suspend fun addBookmark( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): AyahBookmark { return mutatingCall("Failed to add ayah bookmark with collection memberships") { @@ -418,7 +417,7 @@ class QuranDataService internal constructor( } @NativeCoroutines - suspend fun replaceBookmarkCollections(localId: String, collectionLocalIds: List?): Boolean { + suspend fun replaceBookmarkCollections(localId: String, collectionLocalIds: List): Boolean { return mutatingCall("Failed to replace bookmark collection memberships", triggerAfter = false) { val changed = bookmarksRepository.replaceBookmarkCollections(localId, collectionLocalIds) if (changed) { @@ -435,7 +434,7 @@ class QuranDataService internal constructor( @NativeCoroutines suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): Boolean { return mutatingCall("Failed to replace bookmark collection memberships", triggerAfter = false) { @@ -451,7 +450,7 @@ class QuranDataService internal constructor( suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List? + collectionLocalIds: List ): AyahBookmark { return mutatingCall("Failed to replace ayah bookmark collection memberships", triggerAfter = false) { val result = bookmarksRepository.replaceAyahBookmarkCollections(sura, ayah, collectionLocalIds) @@ -470,7 +469,7 @@ class QuranDataService internal constructor( suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: PlatformDateTime ): AyahBookmark { return mutatingCall("Failed to replace ayah bookmark collection memberships", triggerAfter = false) { diff --git a/sync-pipelines/src/commonTest/kotlin/com/quran/shared/pipeline/QuranDataServiceLifecycleTest.kt b/sync-pipelines/src/commonTest/kotlin/com/quran/shared/pipeline/QuranDataServiceLifecycleTest.kt index 752f0553..1ef3cf9c 100644 --- a/sync-pipelines/src/commonTest/kotlin/com/quran/shared/pipeline/QuranDataServiceLifecycleTest.kt +++ b/sync-pipelines/src/commonTest/kotlin/com/quran/shared/pipeline/QuranDataServiceLifecycleTest.kt @@ -875,7 +875,15 @@ class QuranDataServiceLifecycleTest { val result = fixture.service.replaceAyahBookmarkCollections(2, 255, listOf("collection-a")) - assertEquals(AyahBookmark(2, 255, testTimestamp(), "bookmark-replaced"), result) + assertEquals( + AyahBookmark( + sura = 2, + ayah = 255, + localId = "bookmark-replaced", + lastUpdated = testTimestamp() + ), + result + ) assertEquals( listOf(BookmarkAyahCollectionsReplaceCall(2, 255, listOf("collection-a"))), fixture.bookmarksRepository.replaceAyahCalls @@ -893,7 +901,15 @@ class QuranDataServiceLifecycleTest { val result = fixture.service.replaceAyahBookmarkCollections(2, 255, listOf("collection-a")) - assertEquals(AyahBookmark(2, 255, testTimestamp(), "bookmark-replaced"), result) + assertEquals( + AyahBookmark( + sura = 2, + ayah = 255, + localId = "bookmark-replaced", + lastUpdated = testTimestamp() + ), + result + ) assertEquals(0, fixture.syncClient.localDataUpdatedCount) fixture.clearAndJoin() } @@ -913,7 +929,15 @@ class QuranDataServiceLifecycleTest { timestamp = timestamp ) - assertEquals(AyahBookmark(2, 255, timestamp, "bookmark-replaced"), result) + assertEquals( + AyahBookmark( + sura = 2, + ayah = 255, + localId = "bookmark-replaced", + lastUpdated = timestamp + ), + result + ) assertEquals( listOf(BookmarkAyahCollectionsReplaceCall(2, 255, listOf("collection-a"), timestamp)), fixture.bookmarksRepository.replaceAyahCalls @@ -1284,24 +1308,29 @@ private class ServiceBookmarksRepository : BookmarksRepository, BookmarksSynchro override suspend fun getAllBookmarks(): List = bookmarks.value override fun getBookmarksFlow(): Flow> = bookmarks override suspend fun addBookmark(sura: Int, ayah: Int): AyahBookmark = - AyahBookmark(sura, ayah, testTimestamp(), "bookmark-${++addCount}") + AyahBookmark( + sura = sura, + ayah = ayah, + localId = "bookmark-${++addCount}", + lastUpdated = testTimestamp() + ) override suspend fun addBookmark(sura: Int, ayah: Int, timestamp: com.quran.shared.persistence.util.PlatformDateTime): AyahBookmark = addBookmark(sura, ayah) - override suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List?): AyahBookmark = + override suspend fun addBookmark(sura: Int, ayah: Int, collectionLocalIds: List): AyahBookmark = addBookmark(sura, ayah) override suspend fun addBookmark( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: com.quran.shared.persistence.util.PlatformDateTime ): AyahBookmark = addBookmark(sura, ayah) override suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List? + collectionLocalIds: List ): Boolean { replaceCalls += BookmarkCollectionsReplaceCall(localId, collectionLocalIds) return replaceResult @@ -1309,7 +1338,7 @@ private class ServiceBookmarksRepository : BookmarksRepository, BookmarksSynchro override suspend fun replaceBookmarkCollections( localId: String, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: com.quran.shared.persistence.util.PlatformDateTime ): Boolean { replaceCalls += BookmarkCollectionsReplaceCall(localId, collectionLocalIds, timestamp) @@ -1319,11 +1348,16 @@ private class ServiceBookmarksRepository : BookmarksRepository, BookmarksSynchro override suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List? + collectionLocalIds: List ): BookmarkCollectionsReplacementResult { replaceAyahCalls += BookmarkAyahCollectionsReplaceCall(sura, ayah, collectionLocalIds) return BookmarkCollectionsReplacementResult( - bookmark = AyahBookmark(sura, ayah, testTimestamp(), "bookmark-replaced"), + bookmark = AyahBookmark( + sura = sura, + ayah = ayah, + localId = "bookmark-replaced", + lastUpdated = testTimestamp() + ), changed = replaceAyahResultChanged ) } @@ -1331,12 +1365,17 @@ private class ServiceBookmarksRepository : BookmarksRepository, BookmarksSynchro override suspend fun replaceAyahBookmarkCollections( sura: Int, ayah: Int, - collectionLocalIds: List?, + collectionLocalIds: List, timestamp: com.quran.shared.persistence.util.PlatformDateTime ): BookmarkCollectionsReplacementResult { replaceAyahCalls += BookmarkAyahCollectionsReplaceCall(sura, ayah, collectionLocalIds, timestamp) return BookmarkCollectionsReplacementResult( - bookmark = AyahBookmark(sura, ayah, timestamp, "bookmark-replaced"), + bookmark = AyahBookmark( + sura = sura, + ayah = ayah, + localId = "bookmark-replaced", + lastUpdated = timestamp + ), changed = replaceAyahResultChanged ) } @@ -1368,14 +1407,14 @@ private class ServiceBookmarksRepository : BookmarksRepository, BookmarksSynchro private data class BookmarkCollectionsReplaceCall( val localId: String, - val collectionLocalIds: List?, + val collectionLocalIds: List, val timestamp: PlatformDateTime? = null ) private data class BookmarkAyahCollectionsReplaceCall( val sura: Int, val ayah: Int, - val collectionLocalIds: List?, + val collectionLocalIds: List, val timestamp: PlatformDateTime? = null )