From 83ea02013684c30a4f6ab68aa1b24100651697fa Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 10:06:26 +0300 Subject: [PATCH 01/10] MS-1537 Add reactive per-scope-type event scope count --- .../simprints/infra/events/EventRepository.kt | 8 ++ .../infra/events/EventRepositoryImpl.kt | 2 + .../event/local/EventLocalDataSource.kt | 12 +++ .../events/event/local/SessionScopeRoomDao.kt | 5 + .../event/local/models/DbScopeTypeCount.kt | 10 ++ .../event/local/EventLocalDataSourceTest.kt | 39 ++++++++ .../event/local/SessionScopeRoomDaoTest.kt | 92 +++++++++++++++++-- 7 files changed, 162 insertions(+), 6 deletions(-) create mode 100644 infra/events/src/main/java/com/simprints/infra/events/event/local/models/DbScopeTypeCount.kt diff --git a/infra/events/src/main/java/com/simprints/infra/events/EventRepository.kt b/infra/events/src/main/java/com/simprints/infra/events/EventRepository.kt index 308b99b757..f1044bd8c8 100644 --- a/infra/events/src/main/java/com/simprints/infra/events/EventRepository.kt +++ b/infra/events/src/main/java/com/simprints/infra/events/EventRepository.kt @@ -45,6 +45,14 @@ interface EventRepository { suspend fun getClosedEventScopesCount(type: EventScopeType): Int + /** + * Reactive count of *closed* event scopes per type, zero-filled for every [EventScopeType]. + * + * An open scope - the session currently being recorded, or an in-flight sync scope - is not + * counted, matching [getClosedEventScopesCount]. + */ + fun observeClosedEventScopeCounts(): Flow> + suspend fun deleteEventScope(scopeId: String) suspend fun deleteEventScopes(scopeIds: List) diff --git a/infra/events/src/main/java/com/simprints/infra/events/EventRepositoryImpl.kt b/infra/events/src/main/java/com/simprints/infra/events/EventRepositoryImpl.kt index 8a35abf9a4..b498ef68ea 100644 --- a/infra/events/src/main/java/com/simprints/infra/events/EventRepositoryImpl.kt +++ b/infra/events/src/main/java/com/simprints/infra/events/EventRepositoryImpl.kt @@ -147,6 +147,8 @@ internal open class EventRepositoryImpl @Inject constructor( override suspend fun getClosedEventScopesCount(type: EventScopeType): Int = eventLocalDataSource.countClosedEventScopes(type) + override fun observeClosedEventScopeCounts(): Flow> = eventLocalDataSource.observeClosedEventScopeCounts() + override suspend fun deleteEventScope(scopeId: String) = reportException { eventLocalDataSource.deleteEventScope(scopeId = scopeId) eventLocalDataSource.deleteEventsInScope(scopeId = scopeId) diff --git a/infra/events/src/main/java/com/simprints/infra/events/event/local/EventLocalDataSource.kt b/infra/events/src/main/java/com/simprints/infra/events/event/local/EventLocalDataSource.kt index 1a1acba4ee..dd2a4e15f3 100644 --- a/infra/events/src/main/java/com/simprints/infra/events/event/local/EventLocalDataSource.kt +++ b/infra/events/src/main/java/com/simprints/infra/events/event/local/EventLocalDataSource.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -106,6 +107,17 @@ internal open class EventLocalDataSource @Inject constructor( scopeDao.countClosed(type) } + /** + * Emits a count of *closed* scopes for every [EventScopeType], zero-filled, so that consumers + * never have to distinguish "no rows" from "zero". Open scopes are excluded, as in [countClosedEventScopes]. + */ + fun observeClosedEventScopeCounts(): Flow> = useRoomFlow(readingDispatcher) { + scopeDao.observeClosedCountsByType().map { rows -> + val counts = rows.associate { it.type to it.count } + EventScopeType.entries.associateWith { counts[it] ?: 0 } + } + } + suspend fun loadAllScopes(): List = useRoom(readingDispatcher) { scopeDao.loadAll().map { it.fromDbToDomain() } } diff --git a/infra/events/src/main/java/com/simprints/infra/events/event/local/SessionScopeRoomDao.kt b/infra/events/src/main/java/com/simprints/infra/events/event/local/SessionScopeRoomDao.kt index 66312d86ec..b3cf841cee 100644 --- a/infra/events/src/main/java/com/simprints/infra/events/event/local/SessionScopeRoomDao.kt +++ b/infra/events/src/main/java/com/simprints/infra/events/event/local/SessionScopeRoomDao.kt @@ -6,6 +6,8 @@ import androidx.room.OnConflictStrategy import androidx.room.Query import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.events.event.local.models.DbEventScope +import com.simprints.infra.events.event.local.models.DbScopeTypeCount +import kotlinx.coroutines.flow.Flow @Dao internal interface SessionScopeRoomDao { @@ -30,6 +32,9 @@ internal interface SessionScopeRoomDao { @Query("select count(*) from DbEventScope where type = :type AND end_unixMs IS NOT NULL") suspend fun countClosed(type: EventScopeType): Int + @Query("select type, count(*) as count from DbEventScope where end_unixMs IS NOT NULL group by type") + fun observeClosedCountsByType(): Flow> + @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun insertOrUpdate(dbEvent: DbEventScope) diff --git a/infra/events/src/main/java/com/simprints/infra/events/event/local/models/DbScopeTypeCount.kt b/infra/events/src/main/java/com/simprints/infra/events/event/local/models/DbScopeTypeCount.kt new file mode 100644 index 0000000000..0809ddc867 --- /dev/null +++ b/infra/events/src/main/java/com/simprints/infra/events/event/local/models/DbScopeTypeCount.kt @@ -0,0 +1,10 @@ +package com.simprints.infra.events.event.local.models + +import androidx.annotation.Keep +import com.simprints.infra.events.event.domain.models.scope.EventScopeType + +@Keep +internal data class DbScopeTypeCount( + val type: EventScopeType, + val count: Int, +) diff --git a/infra/events/src/test/java/com/simprints/infra/events/event/local/EventLocalDataSourceTest.kt b/infra/events/src/test/java/com/simprints/infra/events/event/local/EventLocalDataSourceTest.kt index ac11d4ad90..4f69ed747b 100644 --- a/infra/events/src/test/java/com/simprints/infra/events/event/local/EventLocalDataSourceTest.kt +++ b/infra/events/src/test/java/com/simprints/infra/events/event/local/EventLocalDataSourceTest.kt @@ -12,6 +12,7 @@ import com.simprints.infra.events.event.domain.models.EventType.CALLBACK_ENROLME import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.events.event.local.models.DbEvent import com.simprints.infra.events.event.local.models.DbEventScope +import com.simprints.infra.events.event.local.models.DbScopeTypeCount import com.simprints.infra.events.event.local.models.fromDbToDomain import com.simprints.infra.events.event.local.models.fromDomainToDb import com.simprints.infra.events.sampledata.SampleDefaults.GUID1 @@ -22,6 +23,7 @@ import io.mockk.* import io.mockk.impl.annotations.* import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.toList import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -456,6 +458,42 @@ internal class EventLocalDataSourceTest { } } + @Test + fun `observeClosedEventScopeCounts zero-fills every scope type`() = runTest { + every { scopeDao.observeClosedCountsByType() } returns flowOf( + listOf(DbScopeTypeCount(EventScopeType.SESSION, 3)), + ) + + val counts = eventLocalDataSource.observeClosedEventScopeCounts().first() + + assertThat(counts).containsExactlyEntriesIn( + EventScopeType.entries.associateWith { if (it == EventScopeType.SESSION) 3 else 0 }, + ) + } + + @Test + fun `observeClosedEventScopeCounts zero-fills when there are no rows at all`() = runTest { + every { scopeDao.observeClosedCountsByType() } returns flowOf(emptyList()) + + val counts = eventLocalDataSource.observeClosedEventScopeCounts().first() + + assertThat(counts.keys).containsExactlyElementsIn(EventScopeType.entries) + assertThat(counts.values.toSet()).containsExactly(0) + } + + @Test + fun `observeClosedEventScopeCounts recovers from database corruption`() = runTest { + every { scopeDao.observeClosedCountsByType() } + .throws(SQLiteDatabaseCorruptException()) + .andThen(flowOf(listOf(DbScopeTypeCount(EventScopeType.SESSION, 1)))) + + val counts = eventLocalDataSource.observeClosedEventScopeCounts().toList() + + verify { eventDatabaseFactory.recreateDatabase() } + verify(exactly = 2) { scopeDao.observeClosedCountsByType() } + assertThat(counts.single()[EventScopeType.SESSION]).isEqualTo(1) + } + private fun mockDaoLoadToMakeNothing() { db = mockk(relaxed = true) eventDao = mockk(relaxed = true) @@ -466,6 +504,7 @@ internal class EventLocalDataSourceTest { coEvery { scopeDao.loadOpen(any()) } returns emptyList() coEvery { scopeDao.loadClosed(any(), any()) } returns emptyList() coEvery { scopeDao.count(any()) } returns 0 + every { scopeDao.observeClosedCountsByType() } returns flowOf(emptyList()) every { db.eventDao } returns eventDao every { db.scopeDao } returns scopeDao every { eventDatabaseFactory.get() } returns db diff --git a/infra/events/src/test/java/com/simprints/infra/events/event/local/SessionScopeRoomDaoTest.kt b/infra/events/src/test/java/com/simprints/infra/events/event/local/SessionScopeRoomDaoTest.kt index 49d9ce3e4d..30cee8bf51 100644 --- a/infra/events/src/test/java/com/simprints/infra/events/event/local/SessionScopeRoomDaoTest.kt +++ b/infra/events/src/test/java/com/simprints/infra/events/event/local/SessionScopeRoomDaoTest.kt @@ -4,6 +4,7 @@ import android.content.Context import androidx.room.Room import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.event.domain.models.scope.EventScopeType @@ -13,6 +14,7 @@ import com.simprints.infra.events.sampledata.SampleDefaults.GUID1 import com.simprints.infra.events.sampledata.SampleDefaults.GUID2 import com.simprints.infra.events.sampledata.SampleDefaults.GUID3 import com.simprints.infra.events.sampledata.createSessionScope +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Before @@ -43,9 +45,24 @@ internal class SessionScopeRoomDaoTest { @Test fun `loadClosed returns scopes in FIFO order`() = runTest { - val oldestScope = createSessionScope(id = GUID1, createdAt = Timestamp(1000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() - val middleScope = createSessionScope(id = GUID2, createdAt = Timestamp(2000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() - val newestScope = createSessionScope(id = GUID3, createdAt = Timestamp(3000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() + val oldestScope = createSessionScope( + id = GUID1, + createdAt = Timestamp(1000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() + val middleScope = createSessionScope( + id = GUID2, + createdAt = Timestamp(2000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() + val newestScope = createSessionScope( + id = GUID3, + createdAt = Timestamp(3000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() // Insert in reverse order to ensure ordering isn't insertion-dependent scopeDao.insertOrUpdate(newestScope) scopeDao.insertOrUpdate(middleScope) @@ -58,9 +75,24 @@ internal class SessionScopeRoomDaoTest { @Test fun `loadClosed respects the limit parameter`() = runTest { - val oldestScope = createSessionScope(id = GUID1, createdAt = Timestamp(1000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() - val middleScope = createSessionScope(id = GUID2, createdAt = Timestamp(2000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() - val newestScope = createSessionScope(id = GUID3, createdAt = Timestamp(3000L), projectId = DEFAULT_PROJECT_ID, isClosed = true).fromDomainToDb() + val oldestScope = createSessionScope( + id = GUID1, + createdAt = Timestamp(1000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() + val middleScope = createSessionScope( + id = GUID2, + createdAt = Timestamp(2000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() + val newestScope = createSessionScope( + id = GUID3, + createdAt = Timestamp(3000L), + projectId = DEFAULT_PROJECT_ID, + isClosed = true, + ).fromDomainToDb() scopeDao.insertOrUpdate(oldestScope) scopeDao.insertOrUpdate(middleScope) scopeDao.insertOrUpdate(newestScope) @@ -70,4 +102,52 @@ internal class SessionScopeRoomDaoTest { // With FIFO ordering, the oldest 2 are returned assertThat(result.map { it.id }).isEqualTo(listOf(GUID1, GUID2)) } + + @Test + fun `observeClosedCountsByType returns no rows when there are no scopes`() = runTest { + val result = scopeDao.observeClosedCountsByType().first() + + assertThat(result).isEmpty() + } + + @Test + fun `observeClosedCountsByType counts closed scopes of every type`() = runTest { + scopeDao.insertOrUpdate(scope(GUID1, EventScopeType.SESSION, isClosed = false)) + scopeDao.insertOrUpdate(scope(GUID2, EventScopeType.SESSION, isClosed = true)) + scopeDao.insertOrUpdate(scope(GUID3, EventScopeType.UP_SYNC, isClosed = true)) + + val result = scopeDao.observeClosedCountsByType().first() + + // Only types that are actually present appear here - the data source zero-fills the rest. + assertThat(result.associate { it.type to it.count }).containsExactlyEntriesIn( + mapOf( + EventScopeType.SESSION to 1, + EventScopeType.UP_SYNC to 1, + ), + ) + } + + @Test + fun `observeClosedCountsByType re-emits after an insert and after a delete`() = runTest { + scopeDao.observeClosedCountsByType().test { + assertThat(awaitItem()).isEmpty() + + scopeDao.insertOrUpdate(scope(GUID1, EventScopeType.SESSION, isClosed = true)) + assertThat(awaitItem().associate { it.type to it.count }) + .isEqualTo(mapOf(EventScopeType.SESSION to 1)) + + scopeDao.delete(listOf(GUID1)) + assertThat(awaitItem()).isEmpty() + + cancelAndIgnoreRemainingEvents() + } + } + + private fun scope( + id: String, + type: EventScopeType, + isClosed: Boolean = false, + ) = createSessionScope(id = id, projectId = DEFAULT_PROJECT_ID, isClosed = isClosed) + .copy(type = type) + .fromDomainToDb() } From f0e19f529f4c3c7a4542646b07aaacf71f9efef3 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 10:24:36 +0300 Subject: [PATCH 02/10] MS-1537 Add DeviceStateDataTracker --- .../com/simprints/infra/sync/SyncModule.kt | 5 + .../infra/sync/devicestate/DeviceDataState.kt | 20 + .../devicestate/DeviceStateDataTracker.kt | 36 ++ .../devicestate/DeviceStateDataTrackerImpl.kt | 176 +++++++ .../DeviceStateDataTrackerImplTest.kt | 453 ++++++++++++++++++ 5 files changed, 690 insertions(+) create mode 100644 infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt create mode 100644 infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt create mode 100644 infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt create mode 100644 infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/SyncModule.kt b/infra/sync/src/main/java/com/simprints/infra/sync/SyncModule.kt index 48cf24d717..f4e85e2b7a 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/SyncModule.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/SyncModule.kt @@ -1,5 +1,7 @@ package com.simprints.infra.sync +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker +import com.simprints.infra.sync.devicestate.DeviceStateDataTrackerImpl import dagger.Binds import dagger.Module import dagger.hilt.InstallIn @@ -10,4 +12,7 @@ import dagger.hilt.components.SingletonComponent abstract class SyncModule { @Binds internal abstract fun provideSyncOrchestrator(syncOrchestratorImpl: SyncOrchestratorImpl): SyncOrchestrator + + @Binds + internal abstract fun provideDeviceStateDataTracker(impl: DeviceStateDataTrackerImpl): DeviceStateDataTracker } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt new file mode 100644 index 0000000000..d625aac856 --- /dev/null +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt @@ -0,0 +1,20 @@ +package com.simprints.infra.sync.devicestate + +import com.simprints.infra.events.event.domain.models.scope.EventScopeType + +/** + * How much data is currently sitting on this device, and when each upload channel last succeeded. + * + * `null` means the underlying source could not be read, `0` means there is genuinely nothing. + * Consumers must not collapse the two as they have very different consequences for a device that may be at risk. + */ +data class DeviceDataState( + val projectId: String?, + val recordCount: Int?, + val pendingScopes: Map?, + val pendingEvents: Int?, + val pendingEnrolments: Int?, + val pendingSamples: Int?, + val lastEventSyncAt: Long?, // Null when never synced on this install, or when the store could not be read. + val lastSampleSyncAt: Long?, // Null when never synced on this install, or when the store could not be read. +) diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt new file mode 100644 index 0000000000..bbd17b1bd8 --- /dev/null +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt @@ -0,0 +1,36 @@ +package com.simprints.infra.sync.devicestate + +import kotlinx.coroutines.flow.Flow + +/** + * Single entry point for "how much data is on this device". + * + * For "what is the sync machinery currently doing" use [com.simprints.infra.sync.SyncOrchestrator.observeSyncState] instead - + * this tracker deliberately knows nothing about WorkManager. + */ +interface DeviceStateDataTracker { + /** + * A freshly evaluated snapshot - never a replayed cache. + * + * Never throws: a source that fails is logged and its fields are left `null`. + * More expensive than [observeDeviceDataState] because it walks the sample directory. + */ + suspend fun getCurrentDeviceDataState(): DeviceDataState + + /** Shared, conflated stream for UI. */ + fun observeDeviceDataState(): Flow + + /** + * Number of enrolment records stored on the device, across every project. + * + * Deliberately unscoped, unlike [DeviceDataState.recordCount]: this feeds the session scope + * payload, which must keep counting records left behind by an earlier project. + * Propagates failures rather than degrading, matching the direct repository call it replaces. + */ + suspend fun getRecordCount(): Int + + /** + * Whether any events are still stored on the device. + */ + suspend fun hasPendingEvents(): Boolean +} diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt new file mode 100644 index 0000000000..271fa08e74 --- /dev/null +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt @@ -0,0 +1,176 @@ +package com.simprints.infra.sync.devicestate + +import com.simprints.core.AppScope +import com.simprints.infra.config.store.ConfigRepository +import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.EventType +import com.simprints.infra.eventsync.sync.common.EventSyncCache +import com.simprints.infra.images.ImageRepository +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC +import com.simprints.infra.logging.Simber +import com.simprints.infra.sync.ImageSyncTimestampProvider +import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase +import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.retryWhen +import kotlinx.coroutines.flow.shareIn +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.time.Duration.Companion.milliseconds + +@Singleton +internal class DeviceStateDataTrackerImpl @Inject constructor( + private val configRepository: ConfigRepository, + private val enrolmentRecordRepository: EnrolmentRecordRepository, + private val eventRepository: EventRepository, + private val imageRepository: ImageRepository, + private val eventSyncCache: EventSyncCache, + private val imageSyncTimestampProvider: ImageSyncTimestampProvider, + private val observeEnrolmentRecordsCount: ObserveEnrolmentRecordsCountUseCase, + private val observeSamplesToUploadCount: ObserveSamplesToUploadCountUseCase, + @param:AppScope private val appScope: CoroutineScope, +) : DeviceStateDataTracker { + private val sharedState: SharedFlow by lazy { + combine( + observeProjectId().nullOnFailure(PROJECT_ID), + observeEnrolmentRecordsCount().nullOnFailure(RECORD_COUNT), + observeSamplesToUploadCount().nullOnFailure(PENDING_SAMPLES), + eventRepository.observeClosedEventScopeCounts().nullOnFailure(PENDING_SCOPES), + observePendingEventCounts().nullOnFailure(PENDING_EVENTS), + ) { projectId, records, samples, scopes, events -> + DeviceDataState( + projectId = projectId, + recordCount = records, + pendingScopes = scopes, + pendingEvents = events?.first, + pendingEnrolments = events?.second, + pendingSamples = samples, + // Encrypted preferences have no change notification, so they are re-read per emission. + lastEventSyncAt = readLastEventSyncAt(), + lastSampleSyncAt = readLastSampleSyncAt(), + ) + }.shareIn( + appScope, + SharingStarted.WhileSubscribed(), + replay = 1, + ) + } + + override fun observeDeviceDataState(): Flow = sharedState + + /** + * Evaluates every source directly. Deliberately NOT [observeDeviceDataState] `.first()`, which + * on a `shareIn(replay = 1)` flow would hand back the last cached emission. + */ + override suspend fun getCurrentDeviceDataState(): DeviceDataState { + val config = readOrNull(PROJECT_ID) { configRepository.getProjectConfiguration() } + val projectId = config?.projectId?.takeIf { it.isNotBlank() } + + return DeviceDataState( + projectId = projectId, + recordCount = when { + config == null -> null // could not read + projectId == null -> 0 // signed out / nothing yet + else -> readOrNull(RECORD_COUNT) { countRecords() } + }, + pendingScopes = readOrNull(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts().first() }, + pendingEvents = readOrNull(PENDING_EVENTS) { countEvents() }, + pendingEnrolments = readOrNull(PENDING_ENROLMENTS) { countEnrolmentEvents() }, + pendingSamples = when { + config == null -> null // could not read + projectId == null -> 0 // signed out / nothing yet + else -> readOrNull(PENDING_SAMPLES) { imageRepository.getNumberOfImagesToUpload(projectId) } + }, + lastEventSyncAt = readLastEventSyncAt(), + lastSampleSyncAt = readLastSampleSyncAt(), + ) + } + + override suspend fun getRecordCount(): Int = countRecords() + + override suspend fun hasPendingEvents(): Boolean = countEvents() > 0 + + private suspend fun currentProjectId(): String? = configRepository + .getProjectConfiguration() + .projectId + .takeIf { it.isNotBlank() } + + private fun observeProjectId(): Flow = configRepository + .observeProjectConfiguration() + .map { it.projectId.takeIf { id -> id.isNotBlank() } } + .distinctUntilChanged() + + private fun observePendingEventCounts(): Flow> = combine( + eventRepository.observeEventCount(type = null), + eventRepository.observeEventCount(EventType.ENROLMENT_V2), + eventRepository.observeEventCount(EventType.ENROLMENT_V4), + ) { events, enrolmentsV2, enrolmentsV4 -> events to (enrolmentsV2 + enrolmentsV4) } + + private suspend fun countRecords(): Int = enrolmentRecordRepository.count() + + private suspend fun countEvents(): Int = eventRepository.observeEventCount(type = null).first() + + private suspend fun countEnrolmentEvents(): Int = eventRepository.observeEventCount(EventType.ENROLMENT_V2).first() + + eventRepository.observeEventCount(EventType.ENROLMENT_V4).first() + + private suspend fun readLastEventSyncAt(): Long? = readOrNull(LAST_EVENT_SYNC) { + eventSyncCache.readLastSuccessfulSyncTime()?.ms + } + + private fun readLastSampleSyncAt(): Long? = readOrNull(LAST_SAMPLE_SYNC) { + imageSyncTimestampProvider.getLastImageSyncTimestamp() + } + + /** + * Records a source failure as `null` rather than letting it abort the whole snapshot, or + * degrade to a zero that would read as "this device holds nothing". + */ + private inline fun readOrNull( + source: String, + block: () -> T?, + ): T? = try { + block() + } catch (cancellation: CancellationException) { + throw cancellation + } catch (t: Throwable) { + Simber.i("Could not read $source for device data state", t, tag = SYNC) + null + } + + private fun Flow.nullOnFailure(source: String): Flow { + // Flow is covariant, so widening before retryWhen{} is what lets the failure be emitted as null. + val nullable: Flow = this + return nullable.retryWhen { cause, _ -> + if (cause is CancellationException) return@retryWhen false + Simber.i("Could not observe $source for device data state", cause, tag = SYNC) + // Report the gap as null, then re-subscribe: catch{} would end the flow, leaving this + // source dark for as long as anything keeps collecting the shared state. + emit(null) + delay(SOURCE_RETRY_DELAY_MS.milliseconds) + true + } + } + + companion object { + private const val PROJECT_ID = "project id" + private const val RECORD_COUNT = "record count" + private const val PENDING_SCOPES = "pending scopes" + private const val PENDING_EVENTS = "pending events" + private const val PENDING_ENROLMENTS = "pending enrolments" + private const val PENDING_SAMPLES = "pending samples" + private const val LAST_EVENT_SYNC = "last event sync time" + private const val LAST_SAMPLE_SYNC = "last sample sync time" + + private const val SOURCE_RETRY_DELAY_MS = 2_000L + } +} diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt new file mode 100644 index 0000000000..9494f934e1 --- /dev/null +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt @@ -0,0 +1,453 @@ +package com.simprints.infra.sync.devicestate + +import app.cash.turbine.test +import com.google.common.truth.Truth.* +import com.simprints.core.tools.time.Timestamp +import com.simprints.infra.config.store.ConfigRepository +import com.simprints.infra.config.store.models.ProjectConfiguration +import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository +import com.simprints.infra.enrolment.records.repository.domain.models.EnrolmentRecordQuery +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.EventType +import com.simprints.infra.events.event.domain.models.scope.EventScopeType +import com.simprints.infra.eventsync.sync.common.EventSyncCache +import com.simprints.infra.images.ImageRepository +import com.simprints.infra.sync.ImageSyncTimestampProvider +import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase +import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase +import io.mockk.* +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class DeviceStateDataTrackerImplTest { + @MockK + private lateinit var configRepository: ConfigRepository + + @MockK + private lateinit var enrolmentRecordRepository: EnrolmentRecordRepository + + @MockK + private lateinit var eventRepository: EventRepository + + @MockK + private lateinit var imageRepository: ImageRepository + + @MockK + private lateinit var eventSyncCache: EventSyncCache + + @MockK + private lateinit var imageSyncTimestampProvider: ImageSyncTimestampProvider + + @MockK + private lateinit var observeEnrolmentRecordsCount: ObserveEnrolmentRecordsCountUseCase + + @MockK + private lateinit var observeSamplesToUploadCount: ObserveSamplesToUploadCountUseCase + + @MockK + private lateinit var projectConfiguration: ProjectConfiguration + + @Before + fun setup() { + MockKAnnotations.init(this, relaxed = true) + + every { projectConfiguration.projectId } returns PROJECT_ID + coEvery { configRepository.getProjectConfiguration() } returns projectConfiguration + every { configRepository.observeProjectConfiguration() } returns flowOf(projectConfiguration) + + coEvery { enrolmentRecordRepository.count(any(), any()) } returns RECORDS + every { observeEnrolmentRecordsCount() } returns flowOf(RECORDS) + + every { eventRepository.observeClosedEventScopeCounts() } returns flowOf(SCOPE_COUNTS) + every { eventRepository.observeEventCount(null) } returns flowOf(EVENTS) + every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns flowOf(ENROLMENTS_V2) + every { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } returns flowOf(ENROLMENTS_V4) + + coEvery { imageRepository.getNumberOfImagesToUpload(any()) } returns SAMPLES + every { observeSamplesToUploadCount() } returns flowOf(SAMPLES) + + coEvery { eventSyncCache.readLastSuccessfulSyncTime() } returns Timestamp(EVENT_SYNC_AT) + every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns SAMPLE_SYNC_AT + } + + // --- aggregation --- + + @Test + fun `snapshot reads every source into the right field`() = runTest { + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state).isEqualTo( + DeviceDataState( + projectId = PROJECT_ID, + recordCount = RECORDS, + pendingScopes = SCOPE_COUNTS, + pendingEvents = EVENTS, + pendingEnrolments = ENROLMENTS_V2 + ENROLMENTS_V4, + pendingSamples = SAMPLES, + lastEventSyncAt = EVENT_SYNC_AT, + lastSampleSyncAt = SAMPLE_SYNC_AT, + ), + ) + } + + @Test + fun `observed state reads every source into the right field`() = runTest { + tracker(backgroundScope).observeDeviceDataState().test { + assertThat(awaitItem()).isEqualTo( + DeviceDataState( + projectId = PROJECT_ID, + recordCount = RECORDS, + pendingScopes = SCOPE_COUNTS, + pendingEvents = EVENTS, + pendingEnrolments = ENROLMENTS_V2 + ENROLMENTS_V4, + pendingSamples = SAMPLES, + lastEventSyncAt = EVENT_SYNC_AT, + lastSampleSyncAt = SAMPLE_SYNC_AT, + ), + ) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `snapshot scopes the record count to the signed-in project`() = runTest { + tracker(backgroundScope).getCurrentDeviceDataState() + + // Matches the observed state, which scopes via ObserveEnrolmentRecordsCountUseCase. + coVerify { enrolmentRecordRepository.count(EnrolmentRecordQuery(projectId = PROJECT_ID), any()) } + } + + @Test + fun `getRecordCount counts every project on the device`() = runTest { + tracker(backgroundScope).getRecordCount() + + // The session scope payload must keep counting records left behind by an earlier project. + coVerify { enrolmentRecordRepository.count(EnrolmentRecordQuery(), any()) } + } + + // --- the never-throw contract, one case per source --- + + @Test + fun `snapshot reports a failing record count as null and keeps the other fields`() = runTest { + coEvery { enrolmentRecordRepository.count(any(), any()) } throws RuntimeException() + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.recordCount).isNull() + assertThat(state.pendingEvents).isEqualTo(EVENTS) + assertThat(state.pendingSamples).isEqualTo(SAMPLES) + } + + @Test + fun `snapshot reports a failing scope count as null and keeps the other fields`() = runTest { + every { eventRepository.observeClosedEventScopeCounts() } returns flow { throw RuntimeException() } + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.pendingScopes).isNull() + assertThat(state.recordCount).isEqualTo(RECORDS) + assertThat(state.pendingSamples).isEqualTo(SAMPLES) + } + + @Test + fun `snapshot reports failing event counts as null and keeps the other fields`() = runTest { + every { eventRepository.observeEventCount(null) } returns flow { throw RuntimeException() } + every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns flow { throw RuntimeException() } + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.pendingEvents).isNull() + assertThat(state.pendingEnrolments).isNull() + assertThat(state.recordCount).isEqualTo(RECORDS) + } + + @Test + fun `snapshot reports a failing sample count as null and keeps the other fields`() = runTest { + coEvery { imageRepository.getNumberOfImagesToUpload(any()) } throws RuntimeException() + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.pendingSamples).isNull() + assertThat(state.recordCount).isEqualTo(RECORDS) + assertThat(state.pendingEvents).isEqualTo(EVENTS) + } + + @Test + fun `snapshot reports failing sync timestamps as null and keeps the other fields`() = runTest { + coEvery { eventSyncCache.readLastSuccessfulSyncTime() } throws RuntimeException() + every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } throws RuntimeException() + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.lastEventSyncAt).isNull() + assertThat(state.lastSampleSyncAt).isNull() + assertThat(state.recordCount).isEqualTo(RECORDS) + } + + @Test + fun `snapshot reports a failing project id as null and does not throw`() = runTest { + coEvery { configRepository.getProjectConfiguration() } throws RuntimeException() + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.projectId).isNull() + assertThat(state.pendingEvents).isEqualTo(EVENTS) + } + + @Test + fun `observed state reports a failing source as null rather than failing the stream`() = runTest { + every { observeSamplesToUploadCount() } returns flow { throw RuntimeException() } + + tracker(backgroundScope).observeDeviceDataState().test { + val state = awaitItem() + assertThat(state.pendingSamples).isNull() + assertThat(state.recordCount).isEqualTo(RECORDS) + cancelAndIgnoreRemainingEvents() + } + } + + // --- freshness --- + + @Test + fun `snapshot is re-evaluated rather than replayed from the shared flow`() = runTest { + val records = MutableStateFlow(RECORDS) + every { observeEnrolmentRecordsCount() } returns records + val tracker = tracker(backgroundScope) + + // Warm the shared flow up with the old value, as a live UI subscriber would. + tracker.observeDeviceDataState().test { + assertThat(awaitItem().recordCount).isEqualTo(RECORDS) + cancelAndIgnoreRemainingEvents() + } + runCurrent() + coEvery { enrolmentRecordRepository.count(any(), any()) } returns RECORDS + 7 + + assertThat(tracker.getCurrentDeviceDataState().recordCount).isEqualTo(RECORDS + 7) + } + + @Test + fun `successive snapshots pick up a source change`() = runTest { + val tracker = tracker(backgroundScope) + assertThat(tracker.getCurrentDeviceDataState().pendingSamples).isEqualTo(SAMPLES) + + coEvery { imageRepository.getNumberOfImagesToUpload(any()) } returns SAMPLES + 1 + + assertThat(tracker.getCurrentDeviceDataState().pendingSamples).isEqualTo(SAMPLES + 1) + } + + @Test + fun `observed state re-emits when a source changes`() = runTest { + val records = MutableStateFlow(RECORDS) + every { observeEnrolmentRecordsCount() } returns records + + tracker(backgroundScope).observeDeviceDataState().test { + assertThat(awaitItem().recordCount).isEqualTo(RECORDS) + + records.value = RECORDS + 3 + + assertThat(awaitItem().recordCount).isEqualTo(RECORDS + 3) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `observed state does not re-walk the sample directory per emission`() = runTest { + val records = MutableStateFlow(RECORDS) + every { observeEnrolmentRecordsCount() } returns records + + tracker(backgroundScope).observeDeviceDataState().test { + awaitItem() + records.value = RECORDS + 1 + awaitItem() + records.value = RECORDS + 2 + awaitItem() + cancelAndIgnoreRemainingEvents() + } + + coVerify(exactly = 0) { imageRepository.getNumberOfImagesToUpload(any()) } + } + + @Test + fun `observeDeviceDataState returns the same shared flow across invocations`() = runTest { + val tracker = tracker(backgroundScope) + + val first = tracker.observeDeviceDataState() + val collectJob = launch { first.collect { } } + runCurrent() + val second = tracker.observeDeviceDataState() + + assertThat(first).isSameInstanceAs(second) + verify(exactly = 1) { observeEnrolmentRecordsCount() } + verify(exactly = 1) { observeSamplesToUploadCount() } + verify(exactly = 1) { eventRepository.observeClosedEventScopeCounts() } + collectJob.cancel() + } + + @Test + fun `sources are subscribed only while the state is collected`() = runTest { + val records = MutableSharedFlow(replay = 1).apply { tryEmit(RECORDS) } + every { observeEnrolmentRecordsCount() } returns records + + val state = tracker(backgroundScope).observeDeviceDataState() + runCurrent() + assertThat(records.subscriptionCount.value).isEqualTo(0) + + val collectJob = launch { state.collect { } } + runCurrent() + assertThat(records.subscriptionCount.value).isEqualTo(1) + + collectJob.cancel() + runCurrent() + assertThat(records.subscriptionCount.value).isEqualTo(0) + } + + @Test + fun `holding the tracker touches no repository until something collects`() = runTest { + val tracker = tracker(backgroundScope) + + tracker.observeDeviceDataState() + runCurrent() + + verify(exactly = 0) { observeEnrolmentRecordsCount() } + verify(exactly = 0) { observeSamplesToUploadCount() } + verify(exactly = 0) { eventRepository.observeClosedEventScopeCounts() } + verify(exactly = 0) { eventRepository.observeEventCount(any()) } + } + + // --- convenience members --- + + @Test + fun `getRecordCount does not walk the sample directory`() = runTest { + val count = tracker(backgroundScope).getRecordCount() + + assertThat(count).isEqualTo(RECORDS) + coVerify(exactly = 0) { imageRepository.getNumberOfImagesToUpload(any()) } + } + + @Test + fun `hasPendingEvents does not walk the sample directory`() = runTest { + val hasPending = tracker(backgroundScope).hasPendingEvents() + + assertThat(hasPending).isTrue() + coVerify(exactly = 0) { imageRepository.getNumberOfImagesToUpload(any()) } + } + + @Test + fun `hasPendingEvents ignores pending samples`() = runTest { + every { eventRepository.observeEventCount(null) } returns flowOf(0) + coEvery { imageRepository.getNumberOfImagesToUpload(any()) } returns 42 + + assertThat(tracker(backgroundScope).hasPendingEvents()).isFalse() + } + + @Test + fun `getRecordCount propagates a source failure instead of degrading`() = runTest { + coEvery { enrolmentRecordRepository.count(any(), any()) } throws RuntimeException() + + var thrown = false + try { + tracker(backgroundScope).getRecordCount() + } catch (e: RuntimeException) { + thrown = true + } + + assertThat(thrown).isTrue() + } + + @Test + fun `hasPendingEvents propagates a source failure instead of degrading`() = runTest { + every { eventRepository.observeEventCount(null) } returns flow { throw RuntimeException() } + + var thrown = false + try { + tracker(backgroundScope).hasPendingEvents() + } catch (e: RuntimeException) { + thrown = true + } + + assertThat(thrown).isTrue() + } + + // --- edge states --- + + @Test + fun `failing to fetch config snapshot has no project id and null project-scoped counts`() = runTest { + coEvery { configRepository.getProjectConfiguration() } throws Exception("stub") + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.projectId).isNull() + assertThat(state.recordCount).isNull() + assertThat(state.pendingSamples).isNull() + } + + @Test + fun `signed out snapshot has no project id and zero project-scoped counts`() = runTest { + every { projectConfiguration.projectId } returns "" + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.projectId).isNull() + assertThat(state.recordCount).isEqualTo(0) + assertThat(state.pendingSamples).isEqualTo(0) + assertThat(state.pendingEvents).isEqualTo(EVENTS) + coVerify(exactly = 0) { enrolmentRecordRepository.count(any(), any()) } + coVerify(exactly = 0) { imageRepository.getNumberOfImagesToUpload(any()) } + } + + @Test + fun `never-synced timestamps stay null rather than becoming zero`() = runTest { + coEvery { eventSyncCache.readLastSuccessfulSyncTime() } returns null + every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns null + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.lastEventSyncAt).isNull() + assertThat(state.lastSampleSyncAt).isNull() + } + + @Test + fun `timestamps are re-read on every snapshot`() = runTest { + val tracker = tracker(backgroundScope) + tracker.getCurrentDeviceDataState() + tracker.getCurrentDeviceDataState() + + coVerify(exactly = 2) { eventSyncCache.readLastSuccessfulSyncTime() } + verify(exactly = 2) { imageSyncTimestampProvider.getLastImageSyncTimestamp() } + } + + private fun tracker(scope: CoroutineScope) = DeviceStateDataTrackerImpl( + configRepository = configRepository, + enrolmentRecordRepository = enrolmentRecordRepository, + eventRepository = eventRepository, + imageRepository = imageRepository, + eventSyncCache = eventSyncCache, + imageSyncTimestampProvider = imageSyncTimestampProvider, + observeEnrolmentRecordsCount = observeEnrolmentRecordsCount, + observeSamplesToUploadCount = observeSamplesToUploadCount, + appScope = scope, + ) + + companion object { + private const val PROJECT_ID = "projectId" + private const val RECORDS = 5 + private const val EVENTS = 11 + private const val ENROLMENTS_V2 = 2 + private const val ENROLMENTS_V4 = 3 + private const val SAMPLES = 4 + private const val EVENT_SYNC_AT = 1000L + private const val SAMPLE_SYNC_AT = 2000L + private val SCOPE_COUNTS = EventScopeType.entries.associateWith { 1 } + } +} From e0e14d22d21be681accd7948f78f7f6eda10a6f9 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 10:36:36 +0300 Subject: [PATCH 03/10] MS-1537 Take session scope record count from DeviceStateDataTracker --- .../UpdateSessionScopePayloadUseCase.kt | 6 +++--- .../UpdateSessionScopePayloadUseCaseTest.kt | 20 +++++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/feature/login-check/src/main/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCase.kt b/feature/login-check/src/main/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCase.kt index 28258c083a..7b7e15e8ae 100644 --- a/feature/login-check/src/main/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCase.kt +++ b/feature/login-check/src/main/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCase.kt @@ -1,18 +1,18 @@ package com.simprints.feature.logincheck.usecases import com.simprints.infra.config.store.ConfigRepository -import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository import com.simprints.infra.events.session.SessionEventRepository +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import javax.inject.Inject internal class UpdateSessionScopePayloadUseCase @Inject constructor( private val eventRepository: SessionEventRepository, - private val enrolmentRecordRepository: EnrolmentRecordRepository, + private val deviceStateDataTracker: DeviceStateDataTracker, private val configRepository: ConfigRepository, ) { suspend operator fun invoke() { val configUpdatedAt = configRepository.getProjectConfiguration().updatedAt - val recordCount = enrolmentRecordRepository.count() + val recordCount = deviceStateDataTracker.getRecordCount() val sessionScope = eventRepository.getCurrentSessionScope() val updatedScope = sessionScope.copy( diff --git a/feature/login-check/src/test/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCaseTest.kt b/feature/login-check/src/test/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCaseTest.kt index daecbc7a00..889618bd80 100644 --- a/feature/login-check/src/test/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCaseTest.kt +++ b/feature/login-check/src/test/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCaseTest.kt @@ -3,13 +3,14 @@ package com.simprints.feature.logincheck.usecases import com.google.common.truth.Truth.assertThat import com.simprints.core.tools.time.Timestamp import com.simprints.infra.config.store.ConfigRepository -import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository import com.simprints.infra.events.event.domain.models.scope.DatabaseInfo import com.simprints.infra.events.event.domain.models.scope.Device import com.simprints.infra.events.event.domain.models.scope.EventScope import com.simprints.infra.events.event.domain.models.scope.EventScopePayload import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.events.session.SessionEventRepository +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker +import com.simprints.testtools.common.syntax.assertThrows import io.mockk.MockKAnnotations import io.mockk.coEvery import io.mockk.coVerify @@ -23,7 +24,7 @@ internal class UpdateSessionScopePayloadUseCaseTest { lateinit var eventRepository: SessionEventRepository @MockK - lateinit var enrolmentRecordRepository: EnrolmentRecordRepository + lateinit var deviceStateDataTracker: DeviceStateDataTracker @MockK lateinit var configRepository: ConfigRepository @@ -36,14 +37,14 @@ internal class UpdateSessionScopePayloadUseCaseTest { useCase = UpdateSessionScopePayloadUseCase( eventRepository, - enrolmentRecordRepository, + deviceStateDataTracker, configRepository, ) } @Test fun `Updates current scope with data from enrolments`() = runTest { - coEvery { enrolmentRecordRepository.count() } returns 42 + coEvery { deviceStateDataTracker.getRecordCount() } returns 42 coEvery { configRepository.getProjectConfiguration().updatedAt } returns "configUpdatedAt" coEvery { eventRepository.getCurrentSessionScope() } returns createBlankSessionScope() @@ -60,6 +61,17 @@ internal class UpdateSessionScopePayloadUseCaseTest { } } + @Test + fun `Propagates a record count failure rather than writing a wrong count`() = runTest { + coEvery { deviceStateDataTracker.getRecordCount() } throws RuntimeException() + coEvery { configRepository.getProjectConfiguration().updatedAt } returns "configUpdatedAt" + coEvery { eventRepository.getCurrentSessionScope() } returns createBlankSessionScope() + + assertThrows { useCase() } + + coVerify(exactly = 0) { eventRepository.saveSessionScope(any()) } + } + private fun createBlankSessionScope() = EventScope( id = "eventId", projectId = "projectId", From 8167ba26e00b4aeadb7a843f40cebb4ad93aec3d Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 10:40:59 +0300 Subject: [PATCH 04/10] MS-1537 Fix stale event count on the project-ending sign-out gate --- .../usecase/HandleProjectStateUseCase.kt | 8 +- .../usecase/HandleProjectStateUseCaseTest.kt | 74 ++++++++----------- 2 files changed, 32 insertions(+), 50 deletions(-) diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCase.kt b/infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCase.kt index f80ca22eff..d74d75f55d 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCase.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCase.kt @@ -3,12 +3,11 @@ package com.simprints.infra.sync.config.usecase import com.simprints.infra.config.store.models.ProjectState import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase -import kotlinx.coroutines.flow.first +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import javax.inject.Inject internal class HandleProjectStateUseCase @Inject constructor( - private val observeSyncableCounts: ObserveSyncableCountsUseCase, + private val deviceStateDataTracker: DeviceStateDataTracker, private val syncOrchestrator: SyncOrchestrator, ) { suspend operator fun invoke(state: ProjectState) { @@ -20,8 +19,7 @@ internal class HandleProjectStateUseCase @Inject constructor( private suspend fun shouldSignOut(projectState: ProjectState): Boolean { val isProjectEnded = projectState == ProjectState.PROJECT_ENDED val isProjectEnding = projectState == ProjectState.PROJECT_ENDING - val hasNoEventsToUpload = observeSyncableCounts().first().eventsToUpload == 0 - return isProjectEnded || (isProjectEnding && hasNoEventsToUpload) + return isProjectEnded || (isProjectEnding && !deviceStateDataTracker.hasPendingEvents()) } } diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCaseTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCaseTest.kt index 0c088037a1..c6fba3e32e 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCaseTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCaseTest.kt @@ -3,18 +3,17 @@ package com.simprints.infra.sync.config.usecase import com.simprints.infra.config.store.models.ProjectState import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator -import com.simprints.infra.sync.SyncableCounts -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker +import com.simprints.testtools.common.syntax.assertThrows import io.mockk.* import io.mockk.impl.annotations.MockK -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test internal class HandleProjectStateUseCaseTest { @MockK - private lateinit var observeSyncableCounts: ObserveSyncableCountsUseCase + private lateinit var deviceStateDataTracker: DeviceStateDataTracker @MockK private lateinit var syncOrchestrator: SyncOrchestrator @@ -26,23 +25,14 @@ internal class HandleProjectStateUseCaseTest { MockKAnnotations.init(this, relaxed = true) useCase = HandleProjectStateUseCase( - observeSyncableCounts = observeSyncableCounts, + deviceStateDataTracker = deviceStateDataTracker, syncOrchestrator = syncOrchestrator, ) } @Test fun `Fully logs out when project has ended`() = runTest { - every { observeSyncableCounts.invoke() } returns flowOf( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + coEvery { deviceStateDataTracker.hasPendingEvents() } returns false useCase(ProjectState.PROJECT_ENDED) @@ -51,16 +41,7 @@ internal class HandleProjectStateUseCaseTest { @Test fun `Logs out when project has ending and no items to upload`() = runTest { - every { observeSyncableCounts.invoke() } returns flowOf( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + coEvery { deviceStateDataTracker.hasPendingEvents() } returns false useCase(ProjectState.PROJECT_ENDING) @@ -69,16 +50,7 @@ internal class HandleProjectStateUseCaseTest { @Test fun `Does not logs out when project has ending and has items to upload`() = runTest { - every { observeSyncableCounts.invoke() } returns flowOf( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 5, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + coEvery { deviceStateDataTracker.hasPendingEvents() } returns true useCase(ProjectState.PROJECT_ENDING) @@ -87,19 +59,31 @@ internal class HandleProjectStateUseCaseTest { @Test fun `Does not logs out when project is running`() = runTest { - every { observeSyncableCounts.invoke() } returns flowOf( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + coEvery { deviceStateDataTracker.hasPendingEvents() } returns false useCase(ProjectState.RUNNING) coVerify(exactly = 0) { syncOrchestrator.execute(eq(OneTime.LogoutCommand(true))) } } + + @Test + fun `Re-reads pending events on every invocation rather than reusing an earlier value`() = runTest { + // The previous implementation read a shareIn(replay = 1) flow, so events arriving after the + // first read were invisible - and this sign-out deletes them. + coEvery { deviceStateDataTracker.hasPendingEvents() } returnsMany listOf(false, true) + + useCase(ProjectState.PROJECT_ENDING) + useCase(ProjectState.PROJECT_ENDING) + + coVerify(exactly = 1) { syncOrchestrator.execute(eq(OneTime.LogoutCommand(true))) } + } + + @Test + fun `Does not log out when the pending check fails`() = runTest { + coEvery { deviceStateDataTracker.hasPendingEvents() } throws RuntimeException() + + assertThrows { useCase(ProjectState.PROJECT_ENDING) } + + coVerify(exactly = 0) { syncOrchestrator.execute(eq(OneTime.LogoutCommand(true))) } + } } From 2967d9acc0362992653e20e14216abaaf43a611e Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 11:08:04 +0300 Subject: [PATCH 05/10] MS-1537 Migrate dashboard sync info to DeviceStateDataTracker --- .../settings/about/AboutViewModel.kt | 14 ++- .../usecase/ObserveSyncInfoUseCase.kt | 22 ++-- .../GetSyncInfoSectionImagesUseCase.kt | 6 +- .../GetSyncInfoSectionRecordsUseCase.kt | 38 +++---- .../settings/about/AboutViewModelTest.kt | 39 ++++--- .../usecase/ObserveSyncInfoUseCaseTest.kt | 65 +++++------ .../GetSyncInfoSectionImagesUseCaseTest.kt | 56 ++++++---- .../GetSyncInfoSectionRecordsUseCaseTest.kt | 101 +++++++++++------- .../devicestate/DeviceStateDataTrackerImpl.kt | 22 +++- 9 files changed, 218 insertions(+), 145 deletions(-) diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt index 99d40e98da..e6cfd9df16 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt @@ -12,20 +12,21 @@ import com.simprints.feature.troubleshooting.AutoResettingClickCounter import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.config.store.models.SettingsPasswordConfig import com.simprints.infra.config.store.models.canSyncDataToSimprints +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC +import com.simprints.infra.logging.Simber import com.simprints.infra.recent.user.activity.RecentUserActivityManager import com.simprints.infra.recent.user.activity.domain.RecentUserActivity import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel internal class AboutViewModel @Inject constructor( private val configRepository: ConfigRepository, - private val observeSyncableCounts: ObserveSyncableCountsUseCase, + private val deviceStateDataTracker: DeviceStateDataTracker, private val syncOrchestrator: SyncOrchestrator, private val recentUserActivityManager: RecentUserActivityManager, ) : ViewModel() { @@ -79,7 +80,12 @@ internal class AboutViewModel @Inject constructor( } } - private suspend fun hasEventsToUpload(): Boolean = observeSyncableCounts().first().eventsToUpload > 0 + private suspend fun hasEventsToUpload(): Boolean = try { + deviceStateDataTracker.hasPendingEvents() + } catch (t: Throwable) { + Simber.i("Could not check pending events before logout", t, tag = SYNC) + true + } private suspend fun canSyncDataToSimprints(): Boolean = configRepository.getProjectConfiguration().canSyncDataToSimprints() diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCase.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCase.kt index 275d105d04..e7f63dabe5 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCase.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCase.kt @@ -12,9 +12,12 @@ import com.simprints.feature.dashboard.settings.syncinfo.usecase.internal.GetSyn import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.store.models.isModuleSelectionAvailable import com.simprints.infra.config.store.models.isSampleUploadEnabledInProject +import com.simprints.infra.eventsync.status.models.DownSyncCounts +import com.simprints.infra.eventsync.sync.down.EventDownSyncPeriodicCountUseCase import com.simprints.infra.network.ConnectivityTracker import com.simprints.infra.sync.SyncOrchestrator -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase +import com.simprints.infra.sync.devicestate.DeviceDataState +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -32,7 +35,8 @@ internal class ObserveSyncInfoUseCase @Inject constructor( private val getSyncInfoSectionImages: GetSyncInfoSectionImagesUseCase, private val getSyncInfoSectionRecords: GetSyncInfoSectionRecordsUseCase, private val observeConfigurationFlow: ObserveConfigurationChangesUseCase, - private val observeSyncableCounts: ObserveSyncableCountsUseCase, + private val deviceStateDataTracker: DeviceStateDataTracker, + private val eventDownSyncCount: EventDownSyncPeriodicCountUseCase, private val syncOrchestrator: SyncOrchestrator, @param:DispatcherBG private val dispatcher: CoroutineDispatcher, ) { @@ -43,17 +47,22 @@ internal class ObserveSyncInfoUseCase @Inject constructor( ticker.observeTicks(1.minutes), ) { isOnline, _, _ -> isOnline } + private fun combinedDataState(): Flow> = combine( + deviceStateDataTracker.observeDeviceDataState(), + eventDownSyncCount(), + ) { deviceDataState, downSyncCounts -> deviceDataState to downSyncCounts } + operator fun invoke(isPreLogoutUpSync: Boolean = false): Flow = combine( combinedRefreshSignals(), authStore.observeSignedInProjectId(), syncOrchestrator.observeSyncState(), - observeSyncableCounts(), + combinedDataState(), observeConfigurationFlow(), ) { isOnline, projectId, (eventSyncState, imageSyncStatus), - syncableCounts, + (deviceDataState, downSyncCounts), (isRefreshing, isProjectRunning, moduleCounts, projectConfig), -> val isReLoginRequired = eventSyncState.isSyncFailedBecauseReloginRequired() @@ -67,7 +76,8 @@ internal class ObserveSyncInfoUseCase @Inject constructor( projectId, eventSyncState, imageSyncStatus, - syncableCounts, + deviceDataState, + downSyncCounts, isProjectRunning, moduleCounts, projectConfig, @@ -76,7 +86,7 @@ internal class ObserveSyncInfoUseCase @Inject constructor( isOnline, eventSyncState, imageSyncStatus, - syncableCounts, + deviceDataState, ) return@combine SyncInfo( isLoggedIn = projectId.isNotEmpty(), diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCase.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCase.kt index 10cbd8dd8f..226422a682 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCase.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCase.kt @@ -7,7 +7,7 @@ import com.simprints.feature.dashboard.settings.syncinfo.SyncProgressInfo import com.simprints.feature.dashboard.settings.syncinfo.SyncProgressInfoPart import com.simprints.infra.eventsync.status.models.EventSyncState import com.simprints.infra.sync.ImageSyncStatus -import com.simprints.infra.sync.SyncableCounts +import com.simprints.infra.sync.devicestate.DeviceDataState import javax.inject.Inject import kotlin.math.roundToInt @@ -18,12 +18,12 @@ internal class GetSyncInfoSectionImagesUseCase @Inject constructor( isOnline: Boolean, eventSyncState: EventSyncState, imageSyncStatus: ImageSyncStatus, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, ): SyncInfoSectionImages { val imagesToUploadOrNull = if (imageSyncStatus.isSyncing) { null } else { - syncableCounts.samplesToUpload // internal term is sample, user-facing (within sync info) term is image + deviceDataState.pendingSamples } val progress = getImageSyncProgress(imageSyncStatus) val imageLastSyncTimestamp = Timestamp(imageSyncStatus.lastUpdateTimeMillis ?: -1) diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCase.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCase.kt index 57d2164eb8..2024ffe668 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCase.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCase.kt @@ -17,7 +17,7 @@ import com.simprints.infra.eventsync.permission.CommCarePermissionChecker import com.simprints.infra.eventsync.status.models.DownSyncCounts import com.simprints.infra.eventsync.status.models.EventSyncState import com.simprints.infra.sync.ImageSyncStatus -import com.simprints.infra.sync.SyncableCounts +import com.simprints.infra.sync.devicestate.DeviceDataState import javax.inject.Inject import kotlin.math.roundToInt @@ -31,7 +31,8 @@ internal class GetSyncInfoSectionRecordsUseCase @Inject constructor( projectId: String, eventSyncState: EventSyncState, imageSyncStatus: ImageSyncStatus, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, + downSyncCounts: DownSyncCounts, isProjectRunning: Boolean, moduleCounts: List, projectConfig: ProjectConfiguration, @@ -39,12 +40,12 @@ internal class GetSyncInfoSectionRecordsUseCase @Inject constructor( val isSyncInProgress = eventSyncState.isSyncInProgress() || (isPreLogoutUpSync && imageSyncStatus.isSyncing) // also in progress if combined with image sync - val counterTotalRecords = getRecordsTotal(isSyncInProgress, projectId, syncableCounts) - val counterRecordsToDownload = getRecordsToDownload(isSyncInProgress, isPreLogoutUpSync, projectConfig, syncableCounts) - val counterRecordsToUpload = getRecordsToUpload(isSyncInProgress, syncableCounts) - val counterImagesToUpload = getImagesToUpload(imageSyncStatus, syncableCounts) + val counterTotalRecords = getRecordsTotal(isSyncInProgress, projectId, deviceDataState) + val counterRecordsToDownload = getRecordsToDownload(isSyncInProgress, isPreLogoutUpSync, projectConfig, downSyncCounts) + val counterRecordsToUpload = getRecordsToUpload(isSyncInProgress, deviceDataState) + val counterImagesToUpload = getImagesToUpload(imageSyncStatus, deviceDataState) - val progress = getSyncProgressInfo(isSyncInProgress, isPreLogoutUpSync, eventSyncState, imageSyncStatus, syncableCounts) + val progress = getSyncProgressInfo(isSyncInProgress, isPreLogoutUpSync, eventSyncState, imageSyncStatus, deviceDataState) val recordSyncVisibleState = getSyncVisibleState(isSyncInProgress, isOnline, isPreLogoutUpSync, projectConfig, moduleCounts, eventSyncState) @@ -82,41 +83,39 @@ internal class GetSyncInfoSectionRecordsUseCase @Inject constructor( isSyncInProgress: Boolean, isPreLogoutUpSync: Boolean, projectConfig: ProjectConfiguration, - syncableCounts: SyncableCounts, + downSyncCounts: DownSyncCounts, ): String = when { isSyncInProgress || isPreLogoutUpSync -> null - projectConfig.isSimprintsEventDownSyncAllowed() -> with(syncableCounts) { - DownSyncCounts(recordEventsToDownload, isRecordEventsToDownloadLowerBound) - } + projectConfig.isSimprintsEventDownSyncAllowed() -> downSyncCounts else -> DownSyncCounts(0, isLowerBound = false) }?.let { "${it.count}${if (it.isLowerBound) "+" else ""}" }.orEmpty() private fun getRecordsTotal( isSyncInProgress: Boolean, projectId: String, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, ): String = if (isSyncInProgress || projectId.isBlank()) { "" } else { - syncableCounts.totalRecords.toString() + deviceDataState.recordCount?.toString().orEmpty() } private fun getRecordsToUpload( isSyncInProgress: Boolean, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, ): String = if (isSyncInProgress) { "" } else { - syncableCounts.enrolmentsToUpload.toString() + deviceDataState.pendingEnrolments?.toString().orEmpty() } private fun getImagesToUpload( imageSyncStatus: ImageSyncStatus, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, ): String = if (imageSyncStatus.isSyncing) { "" } else { - syncableCounts.samplesToUpload.toString() + deviceDataState.pendingSamples?.toString().orEmpty() } private fun getSyncProgressInfo( @@ -124,7 +123,7 @@ internal class GetSyncInfoSectionRecordsUseCase @Inject constructor( isPreLogoutUpSync: Boolean, eventSyncState: EventSyncState, imageSyncStatus: ImageSyncStatus, - syncableCounts: SyncableCounts, + deviceDataState: DeviceDataState, ): SyncProgressInfo { if (!isSyncInProgress) { return SyncProgressInfo() @@ -142,7 +141,8 @@ internal class GetSyncInfoSectionRecordsUseCase @Inject constructor( ) val imageSyncProgressPart = SyncProgressInfoPart( isPending = eventSyncState.isSyncInProgress() && !imageSyncStatus.isSyncing, - isDone = !eventSyncState.isSyncInProgress() && !imageSyncStatus.isSyncing && syncableCounts.samplesToUpload == 0, + // An unreadable sample count (null) is deliberately not "done". + isDone = !eventSyncState.isSyncInProgress() && !imageSyncStatus.isSyncing && deviceDataState.pendingSamples == 0, areNumbersVisible = imageSyncStatus.isSyncing && totalImages > 0, currentNumber = currentImages, totalNumber = totalImages, diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/about/AboutViewModelTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/about/AboutViewModelTest.kt index 5587c758bf..da612b83b8 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/about/AboutViewModelTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/about/AboutViewModelTest.kt @@ -15,14 +15,12 @@ import com.simprints.infra.recent.user.activity.RecentUserActivityManager import com.simprints.infra.recent.user.activity.domain.RecentUserActivity import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator -import com.simprints.infra.sync.SyncableCounts -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import com.simprints.testtools.common.coroutines.TestCoroutineRule import com.simprints.testtools.common.livedata.getOrAwaitValue import io.mockk.* import io.mockk.impl.annotations.MockK import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -56,7 +54,7 @@ class AboutViewModelTest { ) @MockK - lateinit var observeSyncableCounts: ObserveSyncableCountsUseCase + lateinit var deviceStateDataTracker: DeviceStateDataTracker @MockK lateinit var configRepository: ConfigRepository @@ -79,7 +77,7 @@ class AboutViewModelTest { fun `should initialize the live data correctly`() = runTest(testDispatcher) { val viewModel = AboutViewModel( configRepository = configRepository, - observeSyncableCounts = observeSyncableCounts, + deviceStateDataTracker = deviceStateDataTracker, recentUserActivityManager = recentUserActivityManager, syncOrchestrator = syncOrchestrator, ) @@ -196,6 +194,20 @@ class AboutViewModelTest { } } + @Test + fun `should read pending events freshly rather than reusing an earlier value`() = runTest(testDispatcher) { + val viewModel = buildAboutViewModel(canSyncDataToSimprints = true, hasEventsToUpload = false) + viewModel.processLogoutRequest() + assertThat(viewModel.logoutDestinationEvent.getOrAwaitValue().peekContent()) + .isEqualTo(LogoutDestination.LoginScreen) + + coEvery { deviceStateDataTracker.hasPendingEvents() } returns true + viewModel.processLogoutRequest() + + assertThat(viewModel.logoutDestinationEvent.getOrAwaitValue().peekContent()) + .isEqualTo(LogoutDestination.LogoutDataSyncScreen) + } + private fun buildAboutViewModel( canSyncDataToSimprints: Boolean, hasEventsToUpload: Boolean, @@ -204,26 +216,13 @@ class AboutViewModelTest { true -> UpSynchronizationConfiguration.UpSynchronizationKind.ALL false -> UpSynchronizationConfiguration.UpSynchronizationKind.NONE } - val countEventsToUpload = when (hasEventsToUpload) { - true -> 1 - false -> 0 - } - every { observeSyncableCounts.invoke() } returns flowOf( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = countEventsToUpload, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + coEvery { deviceStateDataTracker.hasPendingEvents() } returns hasEventsToUpload coEvery { configRepository.getProjectConfiguration() } returns buildProjectConfigurationMock( upSyncKind, ) return AboutViewModel( configRepository = configRepository, - observeSyncableCounts = observeSyncableCounts, + deviceStateDataTracker = deviceStateDataTracker, recentUserActivityManager = recentUserActivityManager, syncOrchestrator = syncOrchestrator, ) diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt index e2cb5a6664..e630a2e718 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt @@ -24,13 +24,15 @@ import com.simprints.infra.config.store.models.isModuleSelectionAvailable import com.simprints.infra.config.store.models.isSampleUploadEnabledInProject import com.simprints.infra.config.store.models.isSimprintsEventDownSyncAllowed import com.simprints.infra.eventsync.permission.CommCarePermissionChecker +import com.simprints.infra.eventsync.status.models.DownSyncCounts import com.simprints.infra.eventsync.status.models.EventSyncState +import com.simprints.infra.eventsync.sync.down.EventDownSyncPeriodicCountUseCase import com.simprints.infra.network.ConnectivityTracker import com.simprints.infra.sync.ImageSyncStatus import com.simprints.infra.sync.SyncOrchestrator import com.simprints.infra.sync.SyncStatus -import com.simprints.infra.sync.SyncableCounts -import com.simprints.infra.sync.usecase.ObserveSyncableCountsUseCase +import com.simprints.infra.sync.devicestate.DeviceDataState +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -56,7 +58,8 @@ internal class ObserveSyncInfoUseCaseTest { private val connectivityTracker = mockk() private val authStore = mockk() - private val observeSyncableCounts = mockk() + private val deviceStateDataTracker = mockk() + private val eventDownSyncCount = mockk() private val syncOrchestrator = mockk() private val ticker = mockk() private val appForegroundStateTracker = mockk() @@ -67,20 +70,23 @@ internal class ObserveSyncInfoUseCaseTest { private val syncStatusFlow = MutableStateFlow( SyncStatus(eventSyncState = mockk(relaxed = true), imageSyncStatus = mockk(relaxed = true)), ) - private val syncableCountsFlow = MutableStateFlow( - SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ), - ) + private val deviceDataStateFlow = MutableStateFlow(createDeviceDataState()) + private val downSyncCountsFlow = MutableStateFlow(DownSyncCounts(count = 0, isLowerBound = false)) private lateinit var useCase: ObserveSyncInfoUseCase private companion object { + fun createDeviceDataState(): DeviceDataState = DeviceDataState( + projectId = TEST_PROJECT_ID, + recordCount = 0, + pendingScopes = emptyMap(), + pendingEvents = 0, + pendingEnrolments = 0, + pendingSamples = 0, + lastEventSyncAt = null, + lastSampleSyncAt = null, + ) + const val TEST_PROJECT_ID = "test_project_id" const val TEST_MODULE_NAME = "test_module" val TEST_TIMESTAMP = Timestamp(1000L) @@ -143,15 +149,10 @@ internal class ObserveSyncInfoUseCaseTest { every { syncOrchestrator.observeSyncState() } returns syncStatusFlow every { mockEventSyncState.lastSyncTime } returns TEST_TIMESTAMP - syncableCountsFlow.value = SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ) - every { observeSyncableCounts.invoke() } returns syncableCountsFlow + deviceDataStateFlow.value = createDeviceDataState() + downSyncCountsFlow.value = DownSyncCounts(count = 0, isLowerBound = false) + every { deviceStateDataTracker.observeDeviceDataState() } returns deviceDataStateFlow + every { eventDownSyncCount.invoke() } returns downSyncCountsFlow every { ticker.observeTicks(any()) } returns MutableStateFlow(Unit) @@ -165,7 +166,7 @@ internal class ObserveSyncInfoUseCaseTest { every { any().isSampleUploadEnabledInProject() } returns true every { - getSyncInfoSectionRecords(any(), any(), any(), any(), any(), any(), any(), any(), any()) + getSyncInfoSectionRecords(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()) } returns SyncInfoSectionRecords() every { @@ -182,7 +183,8 @@ internal class ObserveSyncInfoUseCaseTest { getSyncInfoSectionImages = getSyncInfoSectionImages, getSyncInfoSectionRecords = getSyncInfoSectionRecords, observeConfigurationFlow = observeConfigurationFlow, - observeSyncableCounts = observeSyncableCounts, + deviceStateDataTracker = deviceStateDataTracker, + eventDownSyncCount = eventDownSyncCount, syncOrchestrator = syncOrchestrator, dispatcher = testCoroutineRule.testCoroutineDispatcher, ) @@ -342,14 +344,14 @@ internal class ObserveSyncInfoUseCaseTest { useCase().first() - verify { getSyncInfoSectionRecords(any(), isOnline = false, any(), any(), any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), isOnline = false, any(), any(), any(), any(), any(), any(), any(), any()) } verify { getSyncInfoSectionImages(isOnline = false, any(), any(), any()) } connectivityFlow.value = true useCase().first() - verify { getSyncInfoSectionRecords(any(), isOnline = true, any(), any(), any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), isOnline = true, any(), any(), any(), any(), any(), any(), any(), any()) } verify { getSyncInfoSectionImages(isOnline = true, any(), any(), any()) } } @@ -363,14 +365,14 @@ internal class ObserveSyncInfoUseCaseTest { useCase().first() - verify { getSyncInfoSectionRecords(any(), isOnline = false, any(), any(), any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), isOnline = false, any(), any(), any(), any(), any(), any(), any(), any()) } verify { getSyncInfoSectionImages(isOnline = false, any(), any(), any()) } connectivityFlow.value = true // changed to online useCase().first() - verify { getSyncInfoSectionRecords(any(), isOnline = true, any(), any(), any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), isOnline = true, any(), any(), any(), any(), any(), any(), any(), any()) } verify { getSyncInfoSectionImages(isOnline = true, any(), any(), any()) } } @@ -418,7 +420,7 @@ internal class ObserveSyncInfoUseCaseTest { useCase().first() - verify { getSyncInfoSectionRecords(any(), any(), any(), mockIdleState, any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), any(), any(), mockIdleState, any(), any(), any(), any(), any(), any()) } val mockSyncingState = mockk(relaxed = true) { every { isSyncInProgress() } returns true @@ -429,7 +431,7 @@ internal class ObserveSyncInfoUseCaseTest { useCase().first() - verify { getSyncInfoSectionRecords(any(), any(), any(), mockSyncingState, any(), any(), any(), any(), any()) } + verify { getSyncInfoSectionRecords(any(), any(), any(), mockSyncingState, any(), any(), any(), any(), any(), any()) } } @Test @@ -553,7 +555,8 @@ internal class ObserveSyncInfoUseCaseTest { getSyncInfoSectionImages = getSyncInfoSectionImagesUseCase, getSyncInfoSectionRecords = getSyncInfoSectionRecordsUseCase, observeConfigurationFlow = observeConfigurationFlow, - observeSyncableCounts = observeSyncableCounts, + deviceStateDataTracker = deviceStateDataTracker, + eventDownSyncCount = eventDownSyncCount, syncOrchestrator = syncOrchestrator, dispatcher = testCoroutineRule.testCoroutineDispatcher, ) diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt index 2fa9209bf5..1699343609 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt @@ -5,7 +5,7 @@ import com.simprints.core.tools.time.TimeHelper import com.simprints.core.tools.time.Timestamp import com.simprints.infra.eventsync.status.models.EventSyncState import com.simprints.infra.sync.ImageSyncStatus -import com.simprints.infra.sync.SyncableCounts +import com.simprints.infra.sync.devicestate.DeviceDataState import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.mockk @@ -24,14 +24,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { every { progress } returns null every { lastUpdateTimeMillis } returns null } - private val mockSyncableCounts = SyncableCounts( - totalRecords = 0, - recordEventsToDownload = 0, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ) + private val mockDeviceDataState = createDeviceDataState() private lateinit var useCase: GetSyncInfoSectionImagesUseCase @@ -54,7 +47,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = false, eventSyncState = mockEventSyncState, imageSyncStatus = mockNotSyncingImageStatus, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isInstructionOfflineVisible).isTrue() @@ -71,7 +64,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockNormalEventSyncState, imageSyncStatus = mockImageSyncStatus, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isSyncButtonEnabled).isTrue() @@ -90,7 +83,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockImageStatusWithLastSync, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isFooterLastSyncTimeVisible).isTrue() @@ -109,7 +102,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = imageSyncStatus, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isProgressVisible).isTrue() @@ -127,7 +120,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockNotSyncingImageStatus, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isProgressVisible).isFalse() @@ -140,13 +133,13 @@ internal class GetSyncInfoSectionImagesUseCaseTest { every { isSyncing } returns false every { progress } returns null } - val syncableCounts = mockSyncableCounts.copy(samplesToUpload = 15) + val deviceDataState = createDeviceDataState(pendingSamples = 15) val result = useCase( isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockNotSyncingImageStatus, - syncableCounts = syncableCounts, + deviceDataState = deviceDataState, ) assertThat(result.counterImagesToUpload).isEqualTo("15") @@ -164,7 +157,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = imageSyncStatus, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.counterImagesToUpload).isEmpty() @@ -183,7 +176,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockImageStatusWithLastSync, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isFooterLastSyncTimeVisible).isTrue() @@ -202,7 +195,7 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockImageStatusWithoutLastSync, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isFooterLastSyncTimeVisible).isFalse() @@ -220,9 +213,32 @@ internal class GetSyncInfoSectionImagesUseCaseTest { isOnline = true, eventSyncState = mockEventSyncState, imageSyncStatus = mockImageStatusWithNegativeTimestamp, - syncableCounts = mockSyncableCounts, + deviceDataState = mockDeviceDataState, ) assertThat(result.isFooterLastSyncTimeVisible).isFalse() } + + private fun createDeviceDataState(pendingSamples: Int? = 0): DeviceDataState = DeviceDataState( + projectId = "projectId", + recordCount = 0, + pendingScopes = emptyMap(), + pendingEvents = 0, + pendingEnrolments = 0, + pendingSamples = pendingSamples, + lastEventSyncAt = null, + lastSampleSyncAt = null, + ) + + @Test + fun `should render an unreadable sample count as unknown rather than zero`() = runTest { + val result = useCase( + isOnline = true, + eventSyncState = mockEventSyncState, + imageSyncStatus = mockImageSyncStatus, + deviceDataState = createDeviceDataState(pendingSamples = null), + ) + + assertThat(result.counterImagesToUpload).isEqualTo("") + } } diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt index 96b737d631..d43f7ecfdb 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt @@ -11,9 +11,10 @@ import com.simprints.infra.config.store.models.isCommCareEventDownSyncAllowed import com.simprints.infra.config.store.models.isModuleSelectionAvailable import com.simprints.infra.config.store.models.isSimprintsEventDownSyncAllowed import com.simprints.infra.eventsync.permission.CommCarePermissionChecker +import com.simprints.infra.eventsync.status.models.DownSyncCounts import com.simprints.infra.eventsync.status.models.EventSyncState import com.simprints.infra.sync.ImageSyncStatus -import com.simprints.infra.sync.SyncableCounts +import com.simprints.infra.sync.devicestate.DeviceDataState import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.mockk @@ -224,18 +225,15 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { syncInProgress = false, syncRunning = false, ) - val syncableCounts = createSyncableCounts( - totalRecords = 25, - recordEventsToDownload = 8, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 5, - samplesToUpload = 0, + val deviceDataState = createDeviceDataState( + recordCount = 25, + pendingEnrolments = 5, ) val result = invokeUseCase( eventSyncState = mockIdleEventSyncState, - syncableCounts = syncableCounts, + deviceDataState = deviceDataState, + downSyncCounts = DownSyncCounts(count = 8, isLowerBound = false), ) assertThat(result.counterTotalRecords).isEqualTo("25") @@ -279,13 +277,13 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { isSyncing = false, progress = null, ) - val syncableCounts = createSyncableCounts( - samplesToUpload = 15, + val deviceDataState = createDeviceDataState( + pendingSamples = 15, ) val result = invokeUseCase( imageSyncStatus = mockNotSyncingImageStatus, - syncableCounts = syncableCounts, + deviceDataState = deviceDataState, ) assertThat(result.counterImagesToUpload).isEqualTo("15") @@ -311,21 +309,13 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { val mockIdleEventSyncState = createEventSyncState( syncInProgress = false, ) - val syncableCounts = createSyncableCounts( - totalRecords = 0, - recordEventsToDownload = 42, - isRecordEventsToDownloadLowerBound = false, - eventsToUpload = 0, - enrolmentsToUpload = 0, - samplesToUpload = 0, - ) every { mockProjectConfigWithDownSync.isSimprintsEventDownSyncAllowed() } returns true every { mockProjectConfigWithDownSync.isModuleSelectionAvailable() } returns false val result = invokeUseCase( projectConfig = mockProjectConfigWithDownSync, eventSyncState = mockIdleEventSyncState, - syncableCounts = syncableCounts, + downSyncCounts = DownSyncCounts(count = 42, isLowerBound = false), ) assertThat(result.isCounterRecordsToDownloadVisible).isTrue() @@ -659,13 +649,50 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { assertThat(result.recordSyncVisibleState).isEqualTo(RecordSyncVisibleState.ON_STANDBY) } + @Test + fun `should render unreadable counters as unknown rather than zero`() = runTest { + val mockIdleEventSyncState = createEventSyncState(syncInProgress = false) + val deviceDataState = createDeviceDataState( + recordCount = null, + pendingEnrolments = null, + pendingSamples = null, + ) + + val result = invokeUseCase( + eventSyncState = mockIdleEventSyncState, + deviceDataState = deviceDataState, + ) + + assertThat(result.counterTotalRecords).isEqualTo("") + assertThat(result.counterRecordsToUpload).isEqualTo("") + assertThat(result.counterImagesToUpload).isEqualTo("") + } + + @Test + fun `should not report image sync done when the sample count is unreadable`() = runTest { + val deviceDataState = createDeviceDataState(pendingSamples = null) + + val result = invokeUseCase( + isPreLogoutUpSync = true, + eventSyncState = createEventSyncState(syncInProgress = true), + deviceDataState = deviceDataState, + ) + + assertThat( + result.progress.progressParts + .last() + .isDone, + ).isFalse() + } + private fun invokeUseCase( isPreLogoutUpSync: Boolean = false, isOnline: Boolean = true, projectId: String = TEST_PROJECT_ID, eventSyncState: EventSyncState = createEventSyncState(), imageSyncStatus: ImageSyncStatus = createImageSyncStatus(), - syncableCounts: SyncableCounts = createSyncableCounts(), + deviceDataState: DeviceDataState = createDeviceDataState(), + downSyncCounts: DownSyncCounts = DownSyncCounts(count = 0, isLowerBound = false), isProjectRunning: Boolean = true, moduleCounts: List = emptyList(), projectConfig: ProjectConfiguration = mockProjectConfiguration, @@ -675,7 +702,8 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { projectId = projectId, eventSyncState = eventSyncState, imageSyncStatus = imageSyncStatus, - syncableCounts = syncableCounts, + deviceDataState = deviceDataState, + downSyncCounts = downSyncCounts, isProjectRunning = isProjectRunning, moduleCounts = moduleCounts, projectConfig = projectConfig, @@ -732,20 +760,19 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { lastUpdateTimeMillis = lastUpdateTimeMillis, ) - private fun createSyncableCounts( - totalRecords: Int = 0, - recordEventsToDownload: Int = 0, - isRecordEventsToDownloadLowerBound: Boolean = false, - eventsToUpload: Int = 0, - enrolmentsToUpload: Int = 0, - samplesToUpload: Int = 0, - ): SyncableCounts = SyncableCounts( - totalRecords = totalRecords, - recordEventsToDownload = recordEventsToDownload, - isRecordEventsToDownloadLowerBound = isRecordEventsToDownloadLowerBound, - eventsToUpload = eventsToUpload, - enrolmentsToUpload = enrolmentsToUpload, - samplesToUpload = samplesToUpload, + private fun createDeviceDataState( + recordCount: Int? = 0, + pendingEnrolments: Int? = 0, + pendingSamples: Int? = 0, + ): DeviceDataState = DeviceDataState( + projectId = TEST_PROJECT_ID, + recordCount = recordCount, + pendingScopes = emptyMap(), + pendingEvents = 0, + pendingEnrolments = pendingEnrolments, + pendingSamples = pendingSamples, + lastEventSyncAt = null, + lastSampleSyncAt = null, ) private companion object { diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt index 271fa08e74..a9c234bbb9 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt @@ -20,7 +20,9 @@ import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.retryWhen import kotlinx.coroutines.flow.shareIn @@ -42,11 +44,11 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( ) : DeviceStateDataTracker { private val sharedState: SharedFlow by lazy { combine( - observeProjectId().nullOnFailure(PROJECT_ID), - observeEnrolmentRecordsCount().nullOnFailure(RECORD_COUNT), - observeSamplesToUploadCount().nullOnFailure(PENDING_SAMPLES), - eventRepository.observeClosedEventScopeCounts().nullOnFailure(PENDING_SCOPES), - observePendingEventCounts().nullOnFailure(PENDING_EVENTS), + deferred(PROJECT_ID) { observeProjectId() }, + deferred(RECORD_COUNT) { observeEnrolmentRecordsCount() }, + deferred(PENDING_SAMPLES) { observeSamplesToUploadCount() }, + deferred(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts() }, + deferred(PENDING_EVENTS) { observePendingEventCounts() }, ) { projectId, records, samples, scopes, events -> DeviceDataState( projectId = projectId, @@ -131,6 +133,16 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( imageSyncTimestampProvider.getLastImageSyncTimestamp() } + /** + * Builds the source flow only once something collects, so that merely holding the tracker + * touches no repository, and reports a failure - in construction or in the stream - as `null` + * rather than killing the whole state flow. + */ + private fun deferred( + source: String, + block: () -> Flow, + ): Flow = flow { emitAll(block()) }.nullOnFailure(source) + /** * Records a source failure as `null` rather than letting it abort the whole snapshot, or * degrade to a zero that would read as "this device holds nothing". From 97749b932bb1d363db1f469f081a6efc6ceacfad Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 11:13:44 +0300 Subject: [PATCH 06/10] MS-1537 Remove SyncableCounts and move usecases to device state package --- .../simprints/infra/sync/SyncableCounts.kt | 13 - .../devicestate/DeviceStateDataTrackerImpl.kt | 4 +- .../ObserveEnrolmentRecordsCountUseCase.kt | 2 +- .../ObserveSamplesToUploadCountUseCase.kt | 2 +- .../usecase/ObserveSyncableCountsUseCase.kt | 70 ------ .../DeviceStateDataTrackerImplTest.kt | 8 +- ...ObserveEnrolmentRecordsCountUseCaseTest.kt | 2 +- .../ObserveSamplesToUploadCountUseCaseTest.kt | 2 +- .../ObserveSyncableCountsUseCaseTest.kt | 235 ------------------ 9 files changed, 12 insertions(+), 326 deletions(-) delete mode 100644 infra/sync/src/main/java/com/simprints/infra/sync/SyncableCounts.kt rename infra/sync/src/main/java/com/simprints/infra/sync/{usecase => devicestate}/internal/ObserveEnrolmentRecordsCountUseCase.kt (94%) rename infra/sync/src/main/java/com/simprints/infra/sync/{usecase => devicestate}/internal/ObserveSamplesToUploadCountUseCase.kt (94%) delete mode 100644 infra/sync/src/main/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCase.kt rename infra/sync/src/test/java/com/simprints/infra/sync/{usecase => devicestate}/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt (98%) rename infra/sync/src/test/java/com/simprints/infra/sync/{usecase => devicestate}/internal/ObserveSamplesToUploadCountUseCaseTest.kt (98%) delete mode 100644 infra/sync/src/test/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCaseTest.kt diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/SyncableCounts.kt b/infra/sync/src/main/java/com/simprints/infra/sync/SyncableCounts.kt deleted file mode 100644 index a10e3eff54..0000000000 --- a/infra/sync/src/main/java/com/simprints/infra/sync/SyncableCounts.kt +++ /dev/null @@ -1,13 +0,0 @@ -package com.simprints.infra.sync - -/** - * Combined counters for syncable entities (events & samples) in the current project - */ -data class SyncableCounts( - val totalRecords: Int, - val recordEventsToDownload: Int, - val isRecordEventsToDownloadLowerBound: Boolean, - val eventsToUpload: Int, - val enrolmentsToUpload: Int, - val samplesToUpload: Int, -) diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt index a9c234bbb9..916f265238 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt @@ -10,8 +10,8 @@ import com.simprints.infra.images.ImageRepository import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import com.simprints.infra.sync.ImageSyncTimestampProvider -import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase -import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase +import com.simprints.infra.sync.devicestate.internal.ObserveEnrolmentRecordsCountUseCase +import com.simprints.infra.sync.devicestate.internal.ObserveSamplesToUploadCountUseCase import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCase.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCase.kt similarity index 94% rename from infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCase.kt rename to infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCase.kt index c62a62d340..3ef70a08ab 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCase.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCase.kt @@ -1,4 +1,4 @@ -package com.simprints.infra.sync.usecase.internal +package com.simprints.infra.sync.devicestate.internal import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCase.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCase.kt similarity index 94% rename from infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCase.kt rename to infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCase.kt index 6e7465ad4d..01e9119707 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCase.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCase.kt @@ -1,4 +1,4 @@ -package com.simprints.infra.sync.usecase.internal +package com.simprints.infra.sync.devicestate.internal import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.images.ImageRepository diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCase.kt b/infra/sync/src/main/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCase.kt deleted file mode 100644 index 7c8a6c03cb..0000000000 --- a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCase.kt +++ /dev/null @@ -1,70 +0,0 @@ -package com.simprints.infra.sync.usecase - -import com.simprints.core.AppScope -import com.simprints.infra.events.EventRepository -import com.simprints.infra.events.event.domain.models.EventType -import com.simprints.infra.eventsync.sync.down.EventDownSyncPeriodicCountUseCase -import com.simprints.infra.sync.SyncableCounts -import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase -import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.SharedFlow -import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.emitAll -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.shareIn -import javax.inject.Inject -import javax.inject.Singleton - -/** - * Combines relevant syncable entity counts in the current project (for events & images) - * together in a reactive way. - */ -@Singleton -class ObserveSyncableCountsUseCase @Inject internal constructor( - private val observeEnrolmentRecordsCount: ObserveEnrolmentRecordsCountUseCase, - private val observeSamplesToUploadCount: ObserveSamplesToUploadCountUseCase, - private val eventDownSyncCount: EventDownSyncPeriodicCountUseCase, - private val eventRepository: EventRepository, - @param:AppScope private val appScope: CoroutineScope, -) { - private val sharedSyncableCounts: SharedFlow by lazy { - combine( - observeEnrolmentRecordsCount(), - eventDownSyncCount(), - flow { // recordEventsToDownload - emitAll(eventRepository.observeEventCount(type = null)) - }, - flow { // eventsToUpload - emitAll( - combine( - eventRepository.observeEventCount(EventType.ENROLMENT_V2), - eventRepository.observeEventCount(EventType.ENROLMENT_V4), - ) { countV2, countV4 -> - countV2 + countV4 - } - ) - }, - observeSamplesToUploadCount(), - ) { totalRecords, recordEventsToDownload, eventsToUpload, enrolmentsToUpload, samplesToUpload -> - val (recordEventsToDownloadCount, isRecordEventsToDownloadLowerBound) = recordEventsToDownload - SyncableCounts( - totalRecords, - recordEventsToDownload = recordEventsToDownloadCount, - isRecordEventsToDownloadLowerBound, - eventsToUpload, - enrolmentsToUpload, - samplesToUpload, - ) - }.shareIn( - appScope, - SharingStarted.WhileSubscribed(), - replay = 1, - ) - } - - operator fun invoke(): Flow = sharedSyncableCounts - -} diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt index 9494f934e1..aeb362980d 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt @@ -13,9 +13,13 @@ import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.eventsync.sync.common.EventSyncCache import com.simprints.infra.images.ImageRepository import com.simprints.infra.sync.ImageSyncTimestampProvider -import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase -import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase +import com.simprints.infra.sync.devicestate.internal.ObserveEnrolmentRecordsCountUseCase +import com.simprints.infra.sync.devicestate.internal.ObserveSamplesToUploadCountUseCase import io.mockk.* +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every import io.mockk.impl.annotations.MockK import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt similarity index 98% rename from infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt rename to infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt index 133303b942..84a55c74ca 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt @@ -1,4 +1,4 @@ -package com.simprints.infra.sync.usecase.internal +package com.simprints.infra.sync.devicestate.internal import com.google.common.truth.Truth.assertThat import com.simprints.infra.config.store.ConfigRepository diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCaseTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCaseTest.kt similarity index 98% rename from infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCaseTest.kt rename to infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCaseTest.kt index 7a0dea490f..706d1fbd4f 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveSamplesToUploadCountUseCaseTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCaseTest.kt @@ -1,4 +1,4 @@ -package com.simprints.infra.sync.usecase.internal +package com.simprints.infra.sync.devicestate.internal import com.google.common.truth.Truth.assertThat import com.simprints.infra.config.store.ConfigRepository diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCaseTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCaseTest.kt deleted file mode 100644 index 0a2c290c9a..0000000000 --- a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCaseTest.kt +++ /dev/null @@ -1,235 +0,0 @@ -package com.simprints.infra.sync.usecase - -import androidx.arch.core.executor.testing.InstantTaskExecutorRule -import com.google.common.truth.Truth.assertThat -import com.simprints.infra.events.EventRepository -import com.simprints.infra.events.event.domain.models.EventType -import com.simprints.infra.eventsync.status.models.DownSyncCounts -import com.simprints.infra.eventsync.sync.down.CountEventsToDownloadUseCase -import com.simprints.infra.eventsync.sync.down.EventDownSyncPeriodicCountUseCase -import com.simprints.infra.sync.SyncableCounts -import com.simprints.infra.sync.usecase.internal.ObserveEnrolmentRecordsCountUseCase -import com.simprints.infra.sync.usecase.internal.ObserveSamplesToUploadCountUseCase -import io.mockk.MockKAnnotations -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.every -import io.mockk.impl.annotations.MockK -import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.take -import kotlinx.coroutines.flow.toList -import kotlinx.coroutines.launch -import kotlinx.coroutines.test.advanceTimeBy -import kotlinx.coroutines.test.runCurrent -import kotlinx.coroutines.test.runTest -import org.junit.Before -import org.junit.Rule -import org.junit.Test - -@OptIn(ExperimentalCoroutinesApi::class) -class ObserveSyncableCountsUseCaseTest { - @get:Rule - val rule = InstantTaskExecutorRule() - - @MockK - private lateinit var eventRepository: EventRepository - - @MockK - private lateinit var countEventsToDownload: CountEventsToDownloadUseCase - - @Before - fun setup() { - MockKAnnotations.init(this, relaxed = true) - } - - @Test - fun `combines latest download and upload counts into EventCounts`() = runTest { - val totalRecordsCountFlow = MutableSharedFlow() - val recordEventsToDownloadCountFlow = MutableSharedFlow() - val eventsToUploadCountFlow = MutableSharedFlow() - val enrolmentsToUploadCountFlowV2 = MutableSharedFlow() - val enrolmentsToUploadCountFlowV4 = MutableSharedFlow() - val samplesToUploadCountFlow = MutableSharedFlow() - val eventDownSyncCount = io.mockk.mockk() - val countEnrolmentRecords = io.mockk.mockk() - val countSamplesToUpload = io.mockk.mockk() - every { eventDownSyncCount.invoke() } returns recordEventsToDownloadCountFlow - every { countEnrolmentRecords.invoke() } returns totalRecordsCountFlow - every { countSamplesToUpload.invoke() } returns samplesToUploadCountFlow - every { eventRepository.observeEventCount(null) } returns eventsToUploadCountFlow - every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns enrolmentsToUploadCountFlowV2 - every { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } returns enrolmentsToUploadCountFlowV4 - val useCase = - ObserveSyncableCountsUseCase( - countEnrolmentRecords, - countSamplesToUpload, - eventDownSyncCount, - eventRepository, - appScope = backgroundScope, - ) - val emitted = mutableListOf() - - val collectJob = launch { useCase().take(1).toList(emitted) } - - runCurrent() // ensure upstream flows are collected before emitting - val expected = SyncableCounts( - totalRecords = 5, - recordEventsToDownload = 10, - isRecordEventsToDownloadLowerBound = true, - eventsToUpload = 1, - enrolmentsToUpload = 5, // 2 of V2 + 3 of V4 - samplesToUpload = 4, - ) - totalRecordsCountFlow.emit(5) - recordEventsToDownloadCountFlow.emit(DownSyncCounts(count = 10, isLowerBound = true)) - eventsToUploadCountFlow.emit(1) - enrolmentsToUploadCountFlowV2.emit(2) - enrolmentsToUploadCountFlowV4.emit(3) - samplesToUploadCountFlow.emit(4) - runCurrent() - collectJob.join() - - assertThat(emitted).containsExactly(expected) - } - - @Test - fun `returns the same shared Flow across invocations`() = runTest { - val totalRecordsCountFlow = MutableSharedFlow(replay = 1) - val recordEventsToDownloadCountFlow = MutableSharedFlow(replay = 1) - val eventsToUploadCountFlow = MutableSharedFlow(replay = 1) - val enrolmentsToUploadCountFlowV2 = MutableSharedFlow(replay = 1) - val enrolmentsToUploadCountFlowV4 = MutableSharedFlow(replay = 1) - val samplesToUploadCountFlow = MutableSharedFlow(replay = 1) - totalRecordsCountFlow.tryEmit(0) - recordEventsToDownloadCountFlow.tryEmit(DownSyncCounts(count = 0, isLowerBound = false)) - eventsToUploadCountFlow.tryEmit(0) - enrolmentsToUploadCountFlowV2.tryEmit(0) - enrolmentsToUploadCountFlowV4.tryEmit(0) - samplesToUploadCountFlow.tryEmit(0) - val eventDownSyncCount = io.mockk.mockk() - val countEnrolmentRecords = io.mockk.mockk() - val countSamplesToUpload = io.mockk.mockk() - every { eventDownSyncCount.invoke() } returns recordEventsToDownloadCountFlow - every { countEnrolmentRecords.invoke() } returns totalRecordsCountFlow - every { countSamplesToUpload.invoke() } returns samplesToUploadCountFlow - every { eventRepository.observeEventCount(null) } returns eventsToUploadCountFlow - every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns enrolmentsToUploadCountFlowV2 - every { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } returns enrolmentsToUploadCountFlowV4 - val useCase = - ObserveSyncableCountsUseCase( - countEnrolmentRecords, - countSamplesToUpload, - eventDownSyncCount, - eventRepository, - appScope = backgroundScope, - ) - - val flow1 = useCase() - - val collectJob = launch { flow1.collect { } } - runCurrent() - - val flow2 = useCase() - - assertThat(flow1).isSameInstanceAs(flow2) - verify(exactly = 1) { eventDownSyncCount() } - verify(exactly = 1) { countEnrolmentRecords() } - verify(exactly = 1) { countSamplesToUpload() } - verify(exactly = 1) { eventRepository.observeEventCount(null) } - verify(exactly = 1) { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } - verify(exactly = 1) { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } - collectJob.cancel() - } - - @Test - fun `subscribes to down-sync counts only while collected`() = runTest { - val totalRecordsCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val recordEventsToDownloadCountFlow = MutableSharedFlow(replay = 1).apply { - tryEmit(DownSyncCounts(count = 0, isLowerBound = false)) - } - val eventsToUploadCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val enrolmentsToUploadCountFlowV2 = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val enrolmentsToUploadCountFlowV4 = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val samplesToUploadCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val eventDownSyncCount = io.mockk.mockk() - val countEnrolmentRecords = io.mockk.mockk() - val countSamplesToUpload = io.mockk.mockk() - every { eventDownSyncCount.invoke() } returns recordEventsToDownloadCountFlow - every { countEnrolmentRecords.invoke() } returns totalRecordsCountFlow - every { countSamplesToUpload.invoke() } returns samplesToUploadCountFlow - every { eventRepository.observeEventCount(null) } returns eventsToUploadCountFlow - every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns enrolmentsToUploadCountFlowV2 - every { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } returns enrolmentsToUploadCountFlowV4 - val useCase = - ObserveSyncableCountsUseCase( - countEnrolmentRecords, - countSamplesToUpload, - eventDownSyncCount, - eventRepository, - appScope = backgroundScope, - ) - - val sharedFlow = useCase() - runCurrent() - assertThat(recordEventsToDownloadCountFlow.subscriptionCount.value).isEqualTo(0) - - val collectJob = launch { sharedFlow.collect { } } - runCurrent() - assertThat(recordEventsToDownloadCountFlow.subscriptionCount.value).isEqualTo(1) - - collectJob.cancel() - runCurrent() - assertThat(recordEventsToDownloadCountFlow.subscriptionCount.value).isEqualTo(0) - } - - @Test - fun `stops down-sync periodic counting when downstream unsubscribed`() = runTest { - // integration test case with DownSyncCountsRepository, to check for its accidental overuse - coEvery { countEventsToDownload() } returnsMany listOf( - DownSyncCounts(count = 1, isLowerBound = false), // initial - DownSyncCounts(count = 2, isLowerBound = false), // immediate periodic on subscribe - DownSyncCounts(count = 3, isLowerBound = false), // would be after interval if still subscribed - ) - val eventDownSyncCount = EventDownSyncPeriodicCountUseCase( - countEventsToDownload, - appScope = backgroundScope, - ) - runCurrent() - advanceTimeBy(10_000L + 1) // debounce time, from EventDownSyncPeriodicCountUseCase - runCurrent() - val totalRecordsCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val eventsToUploadCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val enrolmentsToUploadCountFlowV2 = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val enrolmentsToUploadCountFlowV4 = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val samplesToUploadCountFlow = MutableSharedFlow(replay = 1).apply { tryEmit(0) } - val countEnrolmentRecords = io.mockk.mockk() - val countSamplesToUpload = io.mockk.mockk() - every { countEnrolmentRecords.invoke() } returns totalRecordsCountFlow - every { countSamplesToUpload.invoke() } returns samplesToUploadCountFlow - every { eventRepository.observeEventCount(null) } returns eventsToUploadCountFlow - every { eventRepository.observeEventCount(EventType.ENROLMENT_V2) } returns enrolmentsToUploadCountFlowV2 - every { eventRepository.observeEventCount(EventType.ENROLMENT_V4) } returns enrolmentsToUploadCountFlowV4 - - val useCase = - ObserveSyncableCountsUseCase( - countEnrolmentRecords, - countSamplesToUpload, - eventDownSyncCount, - eventRepository, - appScope = backgroundScope, - ) - - val collectJob = launch { useCase().collect { } } - runCurrent() - coVerify(exactly = 2) { countEventsToDownload() } - - collectJob.cancel() - runCurrent() - val downSyncIntervalMillis = 300_000L // from EventDownSyncPeriodicCountUseCase - advanceTimeBy(downSyncIntervalMillis * 2) - runCurrent() - coVerify(exactly = 2) { countEventsToDownload() } - } -} From 70371618ea1e97075236778480a9f2a4b977774a Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 11:19:39 +0300 Subject: [PATCH 07/10] MS-1537 Show device data state in troubleshooting overview --- feature/troubleshooting/build.gradle.kts | 1 + .../overview/OverviewFragment.kt | 3 + .../overview/OverviewViewModel.kt | 7 + .../usecase/CollectDeviceDataStateUseCase.kt | 49 +++++++ .../fragment_troubleshooting_overview.xml | 19 +++ .../overview/OverviewViewModelTest.kt | 8 ++ .../CollectDeviceDataStateUseCaseTest.kt | 127 ++++++++++++++++++ 7 files changed, 214 insertions(+) create mode 100644 feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt create mode 100644 feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt diff --git a/feature/troubleshooting/build.gradle.kts b/feature/troubleshooting/build.gradle.kts index 944ae47a41..ea681a4f88 100644 --- a/feature/troubleshooting/build.gradle.kts +++ b/feature/troubleshooting/build.gradle.kts @@ -14,6 +14,7 @@ dependencies { implementation(project(":infra:license")) implementation(project(":infra:network")) implementation(project(":infra:logging-persistent")) + implementation(project(":infra:sync")) implementation(project(":infra:enrolment-records:repository")) implementation(project(":fingerprint:infra:scanner")) diff --git a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewFragment.kt b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewFragment.kt index 41c9961c12..e7b79921e6 100644 --- a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewFragment.kt +++ b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewFragment.kt @@ -33,6 +33,9 @@ internal class OverviewFragment : Fragment(R.layout.fragment_troubleshooting_ove viewModel.configurationDetails.observe(viewLifecycleOwner) { binding.troubleshootOverviewConfiguration.text = it.orEmpty() } + viewModel.deviceDataState.observe(viewLifecycleOwner) { + binding.troubleshootOverviewDataState.text = it.orEmpty() + } viewModel.licenseStates.observe(viewLifecycleOwner) { binding.troubleshootOverviewLicences.text = it.ifBlank { "No licenses found" } } diff --git a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewViewModel.kt b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewViewModel.kt index 0a79bb85c3..72f9a282ce 100644 --- a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewViewModel.kt +++ b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.simprints.feature.troubleshooting.overview.usecase.CollectConfigurationDetailsUseCase +import com.simprints.feature.troubleshooting.overview.usecase.CollectDeviceDataStateUseCase import com.simprints.feature.troubleshooting.overview.usecase.CollectIdsUseCase import com.simprints.feature.troubleshooting.overview.usecase.CollectLicenceStatesUseCase import com.simprints.feature.troubleshooting.overview.usecase.CollectNetworkInformationUseCase @@ -21,6 +22,7 @@ import javax.inject.Inject internal class OverviewViewModel @Inject constructor( private val collectIds: CollectIdsUseCase, private val collectConfigurationDetails: CollectConfigurationDetailsUseCase, + private val collectDeviceDataState: CollectDeviceDataStateUseCase, private val collectLicenseStates: CollectLicenceStatesUseCase, private val collectNetworkInformation: CollectNetworkInformationUseCase, private val doServerPing: PingServerUseCase, @@ -35,6 +37,10 @@ internal class OverviewViewModel @Inject constructor( get() = _configurationDetails private val _configurationDetails = MutableLiveData(PLACEHOLDER_TEXT) + val deviceDataState: LiveData + get() = _deviceDataState + private val _deviceDataState = MutableLiveData(PLACEHOLDER_TEXT) + val licenseStates: LiveData get() = _licenseStates private val _licenseStates = MutableLiveData(PLACEHOLDER_TEXT) @@ -58,6 +64,7 @@ internal class OverviewViewModel @Inject constructor( fun collectData() { _projectIds.postValue(collectIds()) viewModelScope.launch { _configurationDetails.postValue(collectConfigurationDetails()) } + viewModelScope.launch { _deviceDataState.postValue(collectDeviceDataState()) } viewModelScope.launch { _licenseStates.postValue(collectLicenseStates()) } _networkStates.postValue(collectNetworkInformation()) viewModelScope.launch { _scannerState.postValue(collectScannerState()) } diff --git a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt new file mode 100644 index 0000000000..95dbe21d5c --- /dev/null +++ b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt @@ -0,0 +1,49 @@ +package com.simprints.feature.troubleshooting.overview.usecase + +import com.simprints.core.tools.time.TimeHelper +import com.simprints.core.tools.time.Timestamp +import com.simprints.infra.events.event.domain.models.scope.EventScopeType +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker +import javax.inject.Inject + +internal class CollectDeviceDataStateUseCase @Inject constructor( + private val deviceStateDataTracker: DeviceStateDataTracker, + private val timeHelper: TimeHelper, +) { + suspend operator fun invoke(): String { + val state = deviceStateDataTracker.getCurrentDeviceDataState() + + return """ + Project ID: ${state.projectId ?: SIGNED_OUT} + Records on device: ${state.recordCount.render()} + Pending events: ${state.pendingEvents.render()} + Pending enrolments: ${state.pendingEnrolments.render()} + Pending samples: ${state.pendingSamples.render()} + Pending scopes: ${state.pendingScopes.render()} + Last event sync: ${state.lastEventSyncAt.renderTime()} + Last sample sync: ${state.lastSampleSyncAt.renderTime()} + """.trimIndent() + } + + /** + * A count that could not be read must never read as 0 here - this screen is used to decide + * whether a device is holding data at risk. + */ + private fun Int?.render(): String = this?.toString() ?: UNKNOWN + + private fun Map?.render(): String = this + ?.entries + ?.joinToString { "${it.key}: ${it.value}" } + ?: UNKNOWN + + /** Null covers both "never synced on this install" and "the store could not be read". */ + private fun Long?.renderTime(): String = this + ?.let { timeHelper.readableBetweenNowAndTime(Timestamp(it)) } + ?: NEVER + + companion object { + private const val UNKNOWN = "unknown" + private const val NEVER = "never" + private const val SIGNED_OUT = "signed out" + } +} diff --git a/feature/troubleshooting/src/main/res/layout/fragment_troubleshooting_overview.xml b/feature/troubleshooting/src/main/res/layout/fragment_troubleshooting_overview.xml index 8b7dbcff5f..4d0fcd596c 100644 --- a/feature/troubleshooting/src/main/res/layout/fragment_troubleshooting_overview.xml +++ b/feature/troubleshooting/src/main/res/layout/fragment_troubleshooting_overview.xml @@ -38,6 +38,25 @@ android:textIsSelectable="true" tools:text="@tools:sample/lorem" /> + + + + ? = mapOf(EventScopeType.SESSION to 5), + pendingEvents: Int? = 22, + pendingEnrolments: Int? = 33, + pendingSamples: Int? = 44, + lastEventSyncAt: Long? = 1000L, + lastSampleSyncAt: Long? = 2000L, + ) = DeviceDataState( + projectId = projectId, + recordCount = recordCount, + pendingScopes = pendingScopes, + pendingEvents = pendingEvents, + pendingEnrolments = pendingEnrolments, + pendingSamples = pendingSamples, + lastEventSyncAt = lastEventSyncAt, + lastSampleSyncAt = lastSampleSyncAt, + ) + + companion object { + private const val PROJECT_ID = "projectId" + private const val READABLE_TIME = "2 minutes ago" + } +} From 1f48779734b4fc9b8e9cda54d3daa5d97422cd3b Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 28 Sep 2026 15:14:37 +0300 Subject: [PATCH 08/10] MS-1537 Add a refresh mechanism for DeviceStateDataTracker to recover from source crashes --- .../settings/syncinfo/SyncInfoViewModel.kt | 4 + .../syncinfo/SyncInfoViewModelTest.kt | 14 ++++ .../devicestate/DeviceStateDataTracker.kt | 7 ++ .../devicestate/DeviceStateDataTrackerImpl.kt | 75 ++++++++++++------- .../DeviceStateDataTrackerImplTest.kt | 43 +++++++++++ 5 files changed, 115 insertions(+), 28 deletions(-) diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModel.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModel.kt index ad2c1cc537..db40f19d04 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModel.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModel.kt @@ -17,6 +17,7 @@ import com.simprints.infra.config.store.models.isModuleSelectionAvailable import com.simprints.infra.recent.user.activity.RecentUserActivityManager import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableSharedFlow @@ -44,6 +45,7 @@ internal class SyncInfoViewModel @Inject constructor( private val timeHelper: TimeHelper, observeSyncInfo: ObserveSyncInfoUseCase, private val syncOrchestrator: SyncOrchestrator, + private val deviceStateDataTracker: DeviceStateDataTracker, @param:DispatcherIO private val ioDispatcher: CoroutineDispatcher, ) : ViewModel() { var isPreLogoutUpSync = false @@ -134,6 +136,8 @@ internal class SyncInfoViewModel @Inject constructor( } fun forceEventSync(canEmitSyncButtonClick: Boolean = true) { + // Sync is the user's "give me fresh data" gesture, so it also recovers any failed device state source + deviceStateDataTracker.refresh() viewModelScope.launch { if (canEmitSyncButtonClick) { val isEventSyncing = eventSyncStateFlow.firstOrNull()?.isSyncInProgress() == true diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModelTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModelTest.kt index 3d81640581..fa4e3e5d7b 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModelTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/SyncInfoViewModelTest.kt @@ -25,6 +25,7 @@ import com.simprints.infra.sync.ImageSyncStatus import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator import com.simprints.infra.sync.SyncStatus +import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import com.simprints.testtools.common.coroutines.TestCoroutineRule import com.simprints.testtools.common.livedata.getOrAwaitValue import com.simprints.testtools.common.livedata.getOrAwaitValues @@ -67,6 +68,9 @@ class SyncInfoViewModelTest { @MockK private lateinit var syncOrchestrator: SyncOrchestrator + @MockK + private lateinit var deviceStateDataTracker: DeviceStateDataTracker + @MockK private lateinit var mockProjectConfiguration: ProjectConfiguration @@ -162,6 +166,7 @@ class SyncInfoViewModelTest { timeHelper = timeHelper, observeSyncInfo = observeSyncInfo, syncOrchestrator = syncOrchestrator, + deviceStateDataTracker = deviceStateDataTracker, ioDispatcher = testCoroutineRule.testCoroutineDispatcher, ) } @@ -390,6 +395,15 @@ class SyncInfoViewModelTest { verify { syncOrchestrator.execute(OneTime.Events.restart(isDownSyncAllowed = false)) } } + @Test + fun `should refresh device data state when forcing event sync`() = runTest { + createViewModel() + + viewModel.forceEventSync() + + verify(exactly = 1) { deviceStateDataTracker.refresh() } + } + @Test fun `should stop current event sync before starting new one`() = runTest { createViewModel() diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt index bbd17b1bd8..1fb93d3f34 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt @@ -33,4 +33,11 @@ interface DeviceStateDataTracker { * Whether any events are still stored on the device. */ suspend fun hasPendingEvents(): Boolean + + /** + * Re-subscribes every source of [observeDeviceDataState]. + * A source that fails reads as `null` and stays that way, because the local reads behind it rarely recover on their own. + * Call this when the user asks for fresh data - otherwise recovery only happens when the screen is left and reopened. + */ + fun refresh() } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt index 916f265238..0d82ae368b 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt @@ -1,8 +1,10 @@ package com.simprints.infra.sync.devicestate import com.simprints.core.AppScope +import com.simprints.core.DispatcherIO import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository +import com.simprints.infra.enrolment.records.repository.domain.models.EnrolmentRecordQuery import com.simprints.infra.events.EventRepository import com.simprints.infra.events.event.domain.models.EventType import com.simprints.infra.eventsync.sync.common.EventSyncCache @@ -13,22 +15,25 @@ import com.simprints.infra.sync.ImageSyncTimestampProvider import com.simprints.infra.sync.devicestate.internal.ObserveEnrolmentRecordsCountUseCase import com.simprints.infra.sync.devicestate.internal.ObserveSamplesToUploadCountUseCase import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.retryWhen import kotlinx.coroutines.flow.shareIn +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Singleton -import kotlin.time.Duration.Companion.milliseconds @Singleton internal class DeviceStateDataTrackerImpl @Inject constructor( @@ -41,21 +46,30 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private val observeEnrolmentRecordsCount: ObserveEnrolmentRecordsCountUseCase, private val observeSamplesToUploadCount: ObserveSamplesToUploadCountUseCase, @param:AppScope private val appScope: CoroutineScope, + @param:DispatcherIO private val dispatcherIO: CoroutineDispatcher, ) : DeviceStateDataTracker { + /** Bumped by [refresh] to re-subscribe every source; see [deferred]. */ + private val refreshes = MutableStateFlow(0) + private val sharedState: SharedFlow by lazy { combine( deferred(PROJECT_ID) { observeProjectId() }, deferred(RECORD_COUNT) { observeEnrolmentRecordsCount() }, deferred(PENDING_SAMPLES) { observeSamplesToUploadCount() }, deferred(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts() }, - deferred(PENDING_EVENTS) { observePendingEventCounts() }, - ) { projectId, records, samples, scopes, events -> + // Combined only after each is made null-safe, so a failure in the enrolment counts + // cannot also blank the total, and each failure is logged against its own source. + combine( + deferred(PENDING_EVENTS) { observeTotalEventCount() }, + deferred(PENDING_ENROLMENTS) { observePendingEnrolmentCount() }, + ) { events, enrolments -> events to enrolments }, + ) { projectId, records, samples, scopes, (events, enrolments) -> DeviceDataState( projectId = projectId, recordCount = records, pendingScopes = scopes, - pendingEvents = events?.first, - pendingEnrolments = events?.second, + pendingEvents = events, + pendingEnrolments = enrolments, pendingSamples = samples, // Encrypted preferences have no change notification, so they are re-read per emission. lastEventSyncAt = readLastEventSyncAt(), @@ -70,6 +84,10 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( override fun observeDeviceDataState(): Flow = sharedState + override fun refresh() { + refreshes.update { it + 1 } + } + /** * Evaluates every source directly. Deliberately NOT [observeDeviceDataState] `.first()`, which * on a `shareIn(replay = 1)` flow would hand back the last cached emission. @@ -83,7 +101,7 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( recordCount = when { config == null -> null // could not read projectId == null -> 0 // signed out / nothing yet - else -> readOrNull(RECORD_COUNT) { countRecords() } + else -> readOrNull(RECORD_COUNT) { countRecordsInProject(projectId) } }, pendingScopes = readOrNull(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts().first() }, pendingEvents = readOrNull(PENDING_EVENTS) { countEvents() }, @@ -102,24 +120,28 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( override suspend fun hasPendingEvents(): Boolean = countEvents() > 0 - private suspend fun currentProjectId(): String? = configRepository - .getProjectConfiguration() - .projectId - .takeIf { it.isNotBlank() } - private fun observeProjectId(): Flow = configRepository .observeProjectConfiguration() .map { it.projectId.takeIf { id -> id.isNotBlank() } } .distinctUntilChanged() - private fun observePendingEventCounts(): Flow> = combine( - eventRepository.observeEventCount(type = null), + // distinctUntilChanged because Room invalidates the events table on every insert, and each + // emission re-runs the combine transform - including the encrypted timestamp reads. + private fun observeTotalEventCount(): Flow = eventRepository + .observeEventCount(type = null) + .distinctUntilChanged() + + private fun observePendingEnrolmentCount(): Flow = combine( eventRepository.observeEventCount(EventType.ENROLMENT_V2), eventRepository.observeEventCount(EventType.ENROLMENT_V4), - ) { events, enrolmentsV2, enrolmentsV4 -> events to (enrolmentsV2 + enrolmentsV4) } + ) { enrolmentsV2, enrolmentsV4 -> enrolmentsV2 + enrolmentsV4 }.distinctUntilChanged() + /** Every record on the device, whatever project it belongs to - see [getRecordCount]. */ private suspend fun countRecords(): Int = enrolmentRecordRepository.count() + private suspend fun countRecordsInProject(projectId: String): Int = + enrolmentRecordRepository.count(EnrolmentRecordQuery(projectId = projectId)) + private suspend fun countEvents(): Int = eventRepository.observeEventCount(type = null).first() private suspend fun countEnrolmentEvents(): Int = eventRepository.observeEventCount(EventType.ENROLMENT_V2).first() + @@ -129,8 +151,12 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( eventSyncCache.readLastSuccessfulSyncTime()?.ms } - private fun readLastSampleSyncAt(): Long? = readOrNull(LAST_SAMPLE_SYNC) { - imageSyncTimestampProvider.getLastImageSyncTimestamp() + /** + * `withContext` because this decrypts an EncryptedSharedPreferences entry and the combine + * transform runs on [appScope], which is the main dispatcher. + */ + private suspend fun readLastSampleSyncAt(): Long? = withContext(dispatcherIO) { + readOrNull(LAST_SAMPLE_SYNC) { imageSyncTimestampProvider.getLastImageSyncTimestamp() } } /** @@ -141,7 +167,7 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private fun deferred( source: String, block: () -> Flow, - ): Flow = flow { emitAll(block()) }.nullOnFailure(source) + ): Flow = refreshes.flatMapLatest { flow { emitAll(block()) }.nullOnFailure(source) } /** * Records a source failure as `null` rather than letting it abort the whole snapshot, or @@ -160,16 +186,11 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( } private fun Flow.nullOnFailure(source: String): Flow { - // Flow is covariant, so widening before retryWhen{} is what lets the failure be emitted as null. + // Flow is covariant, so widening before catch{} is what lets the failure be emitted as null. val nullable: Flow = this - return nullable.retryWhen { cause, _ -> - if (cause is CancellationException) return@retryWhen false + return nullable.catch { cause -> Simber.i("Could not observe $source for device data state", cause, tag = SYNC) - // Report the gap as null, then re-subscribe: catch{} would end the flow, leaving this - // source dark for as long as anything keeps collecting the shared state. emit(null) - delay(SOURCE_RETRY_DELAY_MS.milliseconds) - true } } @@ -182,7 +203,5 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private const val PENDING_SAMPLES = "pending samples" private const val LAST_EVENT_SYNC = "last event sync time" private const val LAST_SAMPLE_SYNC = "last sample sync time" - - private const val SOURCE_RETRY_DELAY_MS = 2_000L } } diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt index aeb362980d..27a7aa38db 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Before @@ -221,6 +222,47 @@ class DeviceStateDataTrackerImplTest { } } + @Test + fun `refresh re-subscribes a source that a failure left null`() = runTest { + var firstCollection = true + every { observeSamplesToUploadCount() } returns flow { + if (firstCollection) { + firstCollection = false + throw RuntimeException() + } + emit(SAMPLES) + } + val tracker = tracker(backgroundScope) + + tracker.observeDeviceDataState().test { + assertThat(awaitItem().pendingSamples).isNull() + + tracker.refresh() + // Every source is re-subscribed, so settle before reading rather than racing the + // order in which the five slots re-emit. + runCurrent() + + // The source is rebuilt rather than left dark for the lifetime of the shared upstream. + assertThat(expectMostRecentItem().pendingSamples).isEqualTo(SAMPLES) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `refresh leaves healthy sources alone`() = runTest { + val tracker = tracker(backgroundScope) + + tracker.observeDeviceDataState().test { + assertThat(awaitItem().recordCount).isEqualTo(RECORDS) + + tracker.refresh() + runCurrent() + + assertThat(expectMostRecentItem().recordCount).isEqualTo(RECORDS) + cancelAndIgnoreRemainingEvents() + } + } + // --- freshness --- @Test @@ -441,6 +483,7 @@ class DeviceStateDataTrackerImplTest { observeEnrolmentRecordsCount = observeEnrolmentRecordsCount, observeSamplesToUploadCount = observeSamplesToUploadCount, appScope = scope, + dispatcherIO = UnconfinedTestDispatcher(), ) companion object { From c45396e76c25e7c8b8f6bda910902bf4d14766d4 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Tue, 29 Sep 2026 16:53:20 +0300 Subject: [PATCH 09/10] MS-1537 Add failure sync failure reasons to the device data state --- .../usecase/ObserveSyncInfoUseCaseTest.kt | 2 + .../GetSyncInfoSectionImagesUseCaseTest.kt | 2 + .../GetSyncInfoSectionRecordsUseCaseTest.kt | 2 + .../usecase/CollectDeviceDataStateUseCase.kt | 13 +- .../CollectDeviceDataStateUseCaseTest.kt | 40 +++- .../core/domain/sync/SyncFailureReason.kt | 17 ++ .../simprints/core/domain/sync/SyncOutcome.kt | 12 ++ .../infra/eventsync/DeleteSyncInfoUseCase.kt | 2 +- .../eventsync/sync/common/EventSyncCache.kt | 54 ++++- .../eventsync/sync/common/WorkInfo.ext.kt | 10 + .../sync/master/EventEndSyncReporterWorker.kt | 15 +- .../eventsync/DeleteSyncInfoUseCaseTest.kt | 2 +- .../sync/common/EventSyncCacheTest.kt | 149 +++++++++++--- .../master/EventEndSyncReporterWorkerTest.kt | 22 +- .../infra/sync/ImageSyncTimestampProvider.kt | 86 ++++++-- .../infra/sync/SyncOrchestratorImpl.kt | 2 +- .../infra/sync/devicestate/DeviceDataState.kt | 10 +- .../devicestate/DeviceStateDataTrackerImpl.kt | 82 ++++---- .../infra/sync/files/FileUpSyncWorker.kt | 5 +- .../internal/ObserveImageSyncStatusUseCase.kt | 2 +- .../sync/ImageSyncTimestampProviderTest.kt | 194 +++++++++++++----- .../infra/sync/SyncOrchestratorImplTest.kt | 2 +- .../DeviceStateDataTrackerImplTest.kt | 95 +++++++-- .../infra/sync/files/FileUpSyncWorkerTest.kt | 8 +- .../ObserveImageSyncStatusUseCaseTest.kt | 27 ++- 25 files changed, 666 insertions(+), 189 deletions(-) create mode 100644 infra/core/src/main/java/com/simprints/core/domain/sync/SyncFailureReason.kt create mode 100644 infra/core/src/main/java/com/simprints/core/domain/sync/SyncOutcome.kt diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt index e630a2e718..4245e87eae 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt @@ -84,7 +84,9 @@ internal class ObserveSyncInfoUseCaseTest { pendingEnrolments = 0, pendingSamples = 0, lastEventSyncAt = null, + lastEventSyncFailure = null, lastSampleSyncAt = null, + lastSampleSyncFailure = null, ) const val TEST_PROJECT_ID = "test_project_id" diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt index 1699343609..47d924ada9 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt @@ -227,7 +227,9 @@ internal class GetSyncInfoSectionImagesUseCaseTest { pendingEnrolments = 0, pendingSamples = pendingSamples, lastEventSyncAt = null, + lastEventSyncFailure = null, lastSampleSyncAt = null, + lastSampleSyncFailure = null, ) @Test diff --git a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt index d43f7ecfdb..4c034b9f17 100644 --- a/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt +++ b/feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt @@ -772,7 +772,9 @@ internal class GetSyncInfoSectionRecordsUseCaseTest { pendingEnrolments = pendingEnrolments, pendingSamples = pendingSamples, lastEventSyncAt = null, + lastEventSyncFailure = null, lastSampleSyncAt = null, + lastSampleSyncFailure = null, ) private companion object { diff --git a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt index 95dbe21d5c..82f66c364c 100644 --- a/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt +++ b/feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt @@ -1,5 +1,6 @@ package com.simprints.feature.troubleshooting.overview.usecase +import com.simprints.core.domain.sync.SyncFailureReason import com.simprints.core.tools.time.TimeHelper import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.event.domain.models.scope.EventScopeType @@ -20,8 +21,8 @@ internal class CollectDeviceDataStateUseCase @Inject constructor( Pending enrolments: ${state.pendingEnrolments.render()} Pending samples: ${state.pendingSamples.render()} Pending scopes: ${state.pendingScopes.render()} - Last event sync: ${state.lastEventSyncAt.renderTime()} - Last sample sync: ${state.lastSampleSyncAt.renderTime()} + Last event sync: ${state.lastEventSyncAt.renderTime()} ${state.lastEventSyncFailure.renderFailure()} + Last sample sync: ${state.lastSampleSyncAt.renderTime()} ${state.lastSampleSyncFailure.renderFailure()} """.trimIndent() } @@ -36,13 +37,15 @@ internal class CollectDeviceDataStateUseCase @Inject constructor( ?.joinToString { "${it.key}: ${it.value}" } ?: UNKNOWN - /** Null covers both "never synced on this install" and "the store could not be read". */ - private fun Long?.renderTime(): String = this - ?.let { timeHelper.readableBetweenNowAndTime(Timestamp(it)) } + private fun Timestamp?.renderTime(): String = this + ?.let { timeHelper.readableBetweenNowAndTime(it) } ?: NEVER + private fun SyncFailureReason?.renderFailure(): String = this?.let { "($FAILED $it)" }.orEmpty() + companion object { private const val UNKNOWN = "unknown" + private const val FAILED = "failed:" private const val NEVER = "never" private const val SIGNED_OUT = "signed out" } diff --git a/feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt b/feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt index 84ef032ee9..41865fe929 100644 --- a/feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt +++ b/feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt @@ -1,13 +1,13 @@ package com.simprints.feature.troubleshooting.overview.usecase -import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.* +import com.simprints.core.domain.sync.SyncFailureReason import com.simprints.core.tools.time.TimeHelper +import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.sync.devicestate.DeviceDataState import com.simprints.infra.sync.devicestate.DeviceStateDataTracker -import io.mockk.MockKAnnotations -import io.mockk.coEvery -import io.mockk.every +import io.mockk.* import io.mockk.impl.annotations.MockK import kotlinx.coroutines.test.runTest import org.junit.Before @@ -91,6 +91,30 @@ class CollectDeviceDataStateUseCaseTest { assertThat(details).doesNotContain(READABLE_TIME) } + @Test + fun `a failed sync renders its reason alongside the time it failed`() = runTest { + coEvery { deviceStateDataTracker.getCurrentDeviceDataState() } returns deviceDataState( + lastEventSyncFailure = SyncFailureReason.BACKEND_MAINTENANCE, + lastSampleSyncFailure = SyncFailureReason.UNKNOWN, + ) + + val details = useCase() + + // The time has to stay: a device still trying and failing reads very differently from one + // that stopped syncing. + assertThat(details).contains("Last event sync: $READABLE_TIME (failed: BACKEND_MAINTENANCE)") + assertThat(details).contains("Last sample sync: $READABLE_TIME (failed: UNKNOWN)") + } + + @Test + fun `a succeeded sync renders no reason`() = runTest { + coEvery { deviceStateDataTracker.getCurrentDeviceDataState() } returns deviceDataState() + + val details = useCase() + + assertThat(details).doesNotContain("failed:") + } + @Test fun `signed out snapshot renders without throwing`() = runTest { coEvery { deviceStateDataTracker.getCurrentDeviceDataState() } returns deviceDataState(projectId = null) @@ -107,8 +131,10 @@ class CollectDeviceDataStateUseCaseTest { pendingEvents: Int? = 22, pendingEnrolments: Int? = 33, pendingSamples: Int? = 44, - lastEventSyncAt: Long? = 1000L, - lastSampleSyncAt: Long? = 2000L, + lastEventSyncAt: Timestamp? = Timestamp(1000L), + lastEventSyncFailure: SyncFailureReason? = null, + lastSampleSyncAt: Timestamp? = Timestamp(2000L), + lastSampleSyncFailure: SyncFailureReason? = null, ) = DeviceDataState( projectId = projectId, recordCount = recordCount, @@ -117,7 +143,9 @@ class CollectDeviceDataStateUseCaseTest { pendingEnrolments = pendingEnrolments, pendingSamples = pendingSamples, lastEventSyncAt = lastEventSyncAt, + lastEventSyncFailure = lastEventSyncFailure, lastSampleSyncAt = lastSampleSyncAt, + lastSampleSyncFailure = lastSampleSyncFailure, ) companion object { diff --git a/infra/core/src/main/java/com/simprints/core/domain/sync/SyncFailureReason.kt b/infra/core/src/main/java/com/simprints/core/domain/sync/SyncFailureReason.kt new file mode 100644 index 0000000000..2543abd8ec --- /dev/null +++ b/infra/core/src/main/java/com/simprints/core/domain/sync/SyncFailureReason.kt @@ -0,0 +1,17 @@ +package com.simprints.core.domain.sync + +import androidx.annotation.Keep +import com.simprints.core.ExcludedFromGeneratedTestCoverageReports + +@Keep +@ExcludedFromGeneratedTestCoverageReports("Enum") +enum class SyncFailureReason { + RELOGIN_REQUIRED, + CLOUD_INTEGRATION, + BACKEND_MAINTENANCE, + TOO_MANY_REQUESTS, + COMM_CARE_PERMISSION_MISSING, + UNKNOWN, +} + +fun String.toSyncFailureReason(): SyncFailureReason = SyncFailureReason.entries.firstOrNull { it.name == this } ?: SyncFailureReason.UNKNOWN diff --git a/infra/core/src/main/java/com/simprints/core/domain/sync/SyncOutcome.kt b/infra/core/src/main/java/com/simprints/core/domain/sync/SyncOutcome.kt new file mode 100644 index 0000000000..edb5a8467c --- /dev/null +++ b/infra/core/src/main/java/com/simprints/core/domain/sync/SyncOutcome.kt @@ -0,0 +1,12 @@ +package com.simprints.core.domain.sync + +import androidx.annotation.Keep +import com.simprints.core.tools.time.Timestamp + +@Keep +data class SyncOutcome( + val timestamp: Timestamp, + val failure: SyncFailureReason?, +) { + val succeeded: Boolean get() = failure == null +} diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCase.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCase.kt index 30a263ce2a..0349ec0eb7 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCase.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCase.kt @@ -17,6 +17,6 @@ class DeleteSyncInfoUseCase @Inject internal constructor( commCareSyncCache.clearAllSyncedCases() upSyncScopeRepo.deleteAll() eventSyncCache.clearProgresses() - eventSyncCache.storeLastSuccessfulSyncTime(null) + eventSyncCache.clearLastSyncOutcome() } } diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt index 21b240876d..9d81c89868 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt @@ -4,11 +4,17 @@ import android.annotation.SuppressLint import androidx.annotation.VisibleForTesting import androidx.core.content.edit import com.simprints.core.DispatcherIO +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.domain.sync.SyncOutcome +import com.simprints.core.domain.sync.toSyncFailureReason import com.simprints.core.tools.time.Timestamp import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import com.simprints.infra.security.SecurityManager import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Singleton @@ -25,6 +31,24 @@ class EventSyncCache @Inject constructor( securityManager.buildEncryptedSharedPreferences(FILENAME_FOR_PROGRESSES_SHARED_PREFS) private val sharedForLastSyncTime = securityManager.buildEncryptedSharedPreferences(FILENAME_FOR_LAST_SYNC_TIME_SHARED_PREFS) + private val lastSyncOutcome = MutableStateFlow(null) + + /** The last attempt and how it turned out, then every later one. */ + fun observeLastSyncOutcome(): Flow = + lastSyncOutcome.onStart { lastSyncOutcome.compareAndSet(null, readLastSyncOutcome()) } + + /** When event sync was last attempted and how it turned out; null when it never has been. */ + suspend fun readLastSyncOutcome(): SyncOutcome? = withContext(dispatcher) { + sharedForLastSyncTime + .getLong(LAST_ATTEMPT_TIME_KEY, -1) + .takeIf { it >= 0 } + ?.let { timestamp -> + SyncOutcome( + timestamp = Timestamp(timestamp), + failure = sharedForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null)?.toSyncFailureReason(), + ) + } + } suspend fun readLastSuccessfulSyncTime(): Timestamp? = withContext(dispatcher) { sharedForLastSyncTime @@ -33,10 +57,30 @@ class EventSyncCache @Inject constructor( ?.let { Timestamp(it) } } - suspend fun storeLastSuccessfulSyncTime(lastSyncTime: Timestamp?): Unit = withContext(dispatcher) { + /** Records an attempt: when it ended, and why it failed when it did. */ + suspend fun storeLastSyncOutcome( + timestamp: Timestamp, + failure: SyncFailureReason?, + ): Unit = withContext(dispatcher) { sharedForLastSyncTime.edit { - putLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, lastSyncTime?.ms ?: -1) + putLong(LAST_ATTEMPT_TIME_KEY, timestamp.ms) + if (failure == null) { + remove(LAST_ATTEMPT_FAILURE_KEY) + putLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, timestamp.ms) + } else { + putString(LAST_ATTEMPT_FAILURE_KEY, failure.name) + } } + lastSyncOutcome.value = SyncOutcome(timestamp, failure) + } + + suspend fun clearLastSyncOutcome(): Unit = withContext(dispatcher) { + sharedForLastSyncTime.edit { + remove(LAST_ATTEMPT_TIME_KEY) + remove(LAST_ATTEMPT_FAILURE_KEY) + remove(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY) + } + lastSyncOutcome.value = null } suspend fun readProgress(workerId: String): Int = withContext(dispatcher) { @@ -88,6 +132,12 @@ class EventSyncCache @Inject constructor( @VisibleForTesting const val PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY = "PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY" + @VisibleForTesting + const val LAST_ATTEMPT_TIME_KEY = "LAST_SYNC_ATTEMPT_TIME_KEY" + + @VisibleForTesting + const val LAST_ATTEMPT_FAILURE_KEY = "LAST_SYNC_ATTEMPT_FAILURE_KEY" + const val KEY_IGNORE_MAX = "IGNORE_MAX_VALUES" const val FILENAME_FOR_PROGRESSES_SHARED_PREFS = "CACHE_PROGRESSES" diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt index 16623a5b9e..6db61edfd1 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt @@ -1,6 +1,7 @@ package com.simprints.infra.eventsync.sync.common import androidx.work.WorkInfo +import com.simprints.core.domain.sync.SyncFailureReason internal const val OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED = "OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED" internal const val OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS = "OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS" @@ -32,3 +33,12 @@ internal fun WorkInfo.hasAnyFailureReason(): Boolean = didFailBecauseBackendMaintenance() || didFailBecauseTooManyRequests() || didFailBecauseCommCarePermissionMissing() + +internal fun List.firstFailureReason(): SyncFailureReason? = when { + any { it.didFailBecauseReloginRequired() } -> SyncFailureReason.RELOGIN_REQUIRED + any { it.didFailBecauseCloudIntegration() } -> SyncFailureReason.CLOUD_INTEGRATION + any { it.didFailBecauseBackendMaintenance() } -> SyncFailureReason.BACKEND_MAINTENANCE + any { it.didFailBecauseTooManyRequests() } -> SyncFailureReason.TOO_MANY_REQUESTS + any { it.didFailBecauseCommCarePermissionMissing() } -> SyncFailureReason.COMM_CARE_PERMISSION_MISSING + else -> null +} diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorker.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorker.kt index 267eb20a89..0809c0eccc 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorker.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorker.kt @@ -10,7 +10,7 @@ import com.simprints.infra.events.EventRepository import com.simprints.infra.events.event.domain.models.scope.EventScopeEndCause import com.simprints.infra.eventsync.sync.common.EventSyncCache import com.simprints.infra.eventsync.sync.common.SyncWorkersInfoProvider -import com.simprints.infra.eventsync.sync.common.hasAnyFailureReason +import com.simprints.infra.eventsync.sync.common.firstFailureReason import com.simprints.infra.logging.Simber import dagger.assisted.Assisted import dagger.assisted.AssistedInject @@ -20,7 +20,7 @@ import kotlinx.coroutines.withContext /** * It's executed at the end of the sync, when all workers succeed (downloaders and uploaders). - * It stores the "last successful timestamp" + * It stores when the sync was attempted, and how it turned out. */ @HiltWorker internal class EventEndSyncReporterWorker @AssistedInject constructor( @@ -50,15 +50,16 @@ internal class EventEndSyncReporterWorker @AssistedInject constructor( } if (!syncId.isNullOrEmpty()) { - val hasWorkerFailures = syncWorkersInfoProvider + val failure = syncWorkersInfoProvider .getSyncWorkerInfos(syncId) .firstOrNull() .orEmpty() - .any { it.hasAnyFailureReason() } + .firstFailureReason() - if (!hasWorkerFailures) { - syncCache.storeLastSuccessfulSyncTime(timeHelper.now()) - } + // Stored whatever the outcome, so that a device whose syncs keep failing cannot be + // mistaken for one that simply stopped syncing. Only a success moves the success + // time, which is what says how stale the data is. + syncCache.storeLastSyncOutcome(timeHelper.now(), failure) success() } else { throw IllegalArgumentException("SyncId missed") diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCaseTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCaseTest.kt index 3351dc7bf5..ec6e83bc18 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCaseTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/DeleteSyncInfoUseCaseTest.kt @@ -45,6 +45,6 @@ internal class DeleteSyncInfoUseCaseTest { coVerify(exactly = 1) { commCareSyncCache.clearAllSyncedCases() } coVerify(exactly = 1) { upSyncScopeRepository.deleteAll() } coVerify(exactly = 1) { eventSyncCache.clearProgresses() } - coVerify(exactly = 1) { eventSyncCache.storeLastSuccessfulSyncTime(null) } + coVerify(exactly = 1) { eventSyncCache.clearLastSyncOutcome() } } } diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt index 1041a59b9c..a6b8fc5d28 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt @@ -1,8 +1,13 @@ package com.simprints.infra.eventsync.sync.common import android.content.SharedPreferences +import app.cash.turbine.test import com.google.common.truth.Truth.assertThat +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.domain.sync.SyncOutcome import com.simprints.core.tools.time.Timestamp +import com.simprints.infra.eventsync.sync.common.EventSyncCache.Companion.LAST_ATTEMPT_FAILURE_KEY +import com.simprints.infra.eventsync.sync.common.EventSyncCache.Companion.LAST_ATTEMPT_TIME_KEY import com.simprints.infra.eventsync.sync.common.EventSyncCache.Companion.PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY import com.simprints.infra.security.SecurityManager import com.simprints.testtools.common.coroutines.TestCoroutineRule @@ -11,6 +16,7 @@ import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Rule @@ -53,47 +59,144 @@ class EventSyncCacheTest { } @Test - fun `readLastSuccessfulSyncTime should return the date if it's greater than -1`() = runTest { - every { - sharedPrefsForLastSyncTime.getLong( - PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, - -1, - ) - } returns 30 + fun `the success time keeps the key it has always been stored under`() = runTest { + // An upgrading device must carry it over rather than read as never synced, which would + // drop every CommCare project straight into fallback. + assertThat(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY).isEqualTo("PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY") + } + + @Test + fun `readLastSuccessfulSyncTime returns null until a sync has succeeded`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns -1 + + assertThat(eventSyncCache.readLastSuccessfulSyncTime()).isNull() + } + + @Test + fun `readLastSuccessfulSyncTime returns the time of the last success, not the last attempt`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 90 - val date = eventSyncCache.readLastSuccessfulSyncTime() - assertThat(date).isEqualTo(Timestamp(30)) + assertThat(eventSyncCache.readLastSuccessfulSyncTime()).isEqualTo(Timestamp(30)) } @Test - fun `readLastSuccessfulSyncTime should return the date if it's equal to -1`() = runTest { - every { - sharedPrefsForLastSyncTime.getLong( - PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, - -1, - ) - } returns -1 + fun `readLastSyncOutcome returns null until a sync has been attempted`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns -1 - val date = eventSyncCache.readLastSuccessfulSyncTime() - assertThat(date).isEqualTo(null) + assertThat(eventSyncCache.readLastSyncOutcome()).isNull() } @Test - fun `storeLastSuccessfulSyncTime should store the date if it's not null`() = runTest { + fun `readLastSyncOutcome returns the time of an attempt that succeeded`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns null + + assertThat(eventSyncCache.readLastSyncOutcome()).isEqualTo(SyncOutcome(Timestamp(30), failure = null)) + } + + @Test + fun `readLastSyncOutcome returns the time and reason of an attempt that failed`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns "BACKEND_MAINTENANCE" + + assertThat(eventSyncCache.readLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(30), SyncFailureReason.BACKEND_MAINTENANCE)) + } + + @Test + fun `readLastSyncOutcome decodes a name from another version as unknown`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns "SOMETHING_ELSE" + + // A downgrade must not break reading the device state. + assertThat(eventSyncCache.readLastSyncOutcome()?.failure).isEqualTo(SyncFailureReason.UNKNOWN) + } + + @Test + fun `storeLastSyncOutcome stores the time and the reason of a failure`() = runTest { val editor = mockk(relaxed = true) every { sharedPrefsForLastSyncTime.edit() } returns editor - eventSyncCache.storeLastSuccessfulSyncTime(Timestamp(30)) + eventSyncCache.storeLastSyncOutcome(Timestamp(30), SyncFailureReason.TOO_MANY_REQUESTS) + + verify(exactly = 1) { editor.putLong(LAST_ATTEMPT_TIME_KEY, 30) } + verify(exactly = 1) { editor.putString(LAST_ATTEMPT_FAILURE_KEY, "TOO_MANY_REQUESTS") } + } + + @Test + fun `storeLastSyncOutcome clears a previous reason on success`() = runTest { + val editor = mockk(relaxed = true) + every { sharedPrefsForLastSyncTime.edit() } returns editor + + eventSyncCache.storeLastSyncOutcome(Timestamp(30), null) + verify(exactly = 1) { editor.putLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, 30) } + verify(exactly = 1) { editor.remove(LAST_ATTEMPT_FAILURE_KEY) } + verify(exactly = 0) { editor.putString(LAST_ATTEMPT_FAILURE_KEY, any()) } } @Test - fun `storeLastSuccessfulSyncTime should store -1 if the date is null`() = runTest { + fun `storeLastSyncOutcome moves the attempt time even when the sync failed`() = runTest { val editor = mockk(relaxed = true) every { sharedPrefsForLastSyncTime.edit() } returns editor - eventSyncCache.storeLastSuccessfulSyncTime(null) - verify(exactly = 1) { editor.putLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } + eventSyncCache.storeLastSyncOutcome(Timestamp(30), SyncFailureReason.CLOUD_INTEGRATION) + + // A device whose syncs keep failing must not look like one that stopped syncing. + verify(exactly = 1) { editor.putLong(LAST_ATTEMPT_TIME_KEY, 30) } + } + + @Test + fun `clearLastSyncOutcome removes both keys`() = runTest { + val editor = mockk(relaxed = true) + every { sharedPrefsForLastSyncTime.edit() } returns editor + + eventSyncCache.clearLastSyncOutcome() + + verify(exactly = 1) { editor.remove(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY) } + verify(exactly = 1) { editor.remove(LAST_ATTEMPT_FAILURE_KEY) } + } + + @Test + fun `observeLastSyncOutcome starts from what is stored`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns "TOO_MANY_REQUESTS" + + assertThat(eventSyncCache.observeLastSyncOutcome().first()) + .isEqualTo(SyncOutcome(Timestamp(30), SyncFailureReason.TOO_MANY_REQUESTS)) + } + + @Test + fun `observeLastSyncOutcome publishes every write`() = runTest { + every { sharedPrefsForLastSyncTime.edit() } returns mockk(relaxed = true) + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns -1 + + eventSyncCache.observeLastSyncOutcome().test { + assertThat(awaitItem()).isNull() + + // Encrypted preferences have no change notification of their own. + eventSyncCache.storeLastSyncOutcome(Timestamp(30), SyncFailureReason.RELOGIN_REQUIRED) + + assertThat(awaitItem()).isEqualTo(SyncOutcome(Timestamp(30), SyncFailureReason.RELOGIN_REQUIRED)) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `observeLastSyncOutcome publishes a clear`() = runTest { + every { sharedPrefsForLastSyncTime.edit() } returns mockk(relaxed = true) + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns null + + eventSyncCache.observeLastSyncOutcome().test { + assertThat(awaitItem()).isNotNull() + + eventSyncCache.clearLastSyncOutcome() + + assertThat(awaitItem()).isNull() + cancelAndIgnoreRemainingEvents() + } } @Test diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt index df6ec44aae..f662e2b456 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt @@ -4,6 +4,7 @@ import android.os.PowerManager import androidx.work.ListenableWorker import androidx.work.workDataOf import com.google.common.truth.Truth.* +import com.simprints.core.domain.sync.SyncFailureReason import com.simprints.core.tools.time.TimeHelper import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.EventRepository @@ -51,7 +52,7 @@ internal class EventEndSyncReporterWorkerTest { val result = endSyncReportWorker.doWork() assertThat(result).isEqualTo(ListenableWorker.Result.failure()) - coVerify(exactly = 0) { syncCache.storeLastSuccessfulSyncTime(any()) } + coVerify(exactly = 0) { syncCache.storeLastSyncOutcome(any(), any()) } } @Test @@ -60,16 +61,16 @@ internal class EventEndSyncReporterWorkerTest { val result = endSyncReportWorker.doWork() assertThat(result).isEqualTo(ListenableWorker.Result.failure()) - coVerify(exactly = 0) { syncCache.storeLastSuccessfulSyncTime(any()) } + coVerify(exactly = 0) { syncCache.storeLastSyncOutcome(any(), any()) } } @Test - fun `doWork should succeed otherwise and save the last success time`() = runTest { + fun `doWork should succeed otherwise and record the attempt`() = runTest { val endSyncReportWorker = createWorker("sync id", null, null) val result = endSyncReportWorker.doWork() assertThat(result).isEqualTo(ListenableWorker.Result.success()) - coVerify(exactly = 1) { syncCache.storeLastSuccessfulSyncTime(any()) } + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(any(), any()) } } @Test @@ -91,7 +92,7 @@ internal class EventEndSyncReporterWorkerTest { } @Test - fun `doWork should not save last success time when workers contain failure flags`() = runTest { + fun `doWork still records the attempt when workers contain failure flags`() = runTest { every { syncWorkersInfoProvider.getSyncWorkerInfos(any()) } returns flowOf( listOf( mockk(relaxed = true) { @@ -104,7 +105,16 @@ internal class EventEndSyncReporterWorkerTest { val result = endSyncReportWorker.doWork() assertThat(result).isEqualTo(ListenableWorker.Result.success()) - coVerify(exactly = 0) { syncCache.storeLastSuccessfulSyncTime(any()) } + // The one timestamp moves whatever the outcome, so a device whose syncs keep failing + // cannot be mistaken for one that simply stopped syncing. + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), SyncFailureReason.CLOUD_INTEGRATION) } + } + + @Test + fun `doWork stores a succeeded outcome when no worker failed`() = runTest { + createWorker("sync id", null, null).doWork() + + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), null) } } private fun createWorker( diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt b/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt index 1c90d1bd01..474486fa81 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt @@ -1,39 +1,93 @@ package com.simprints.infra.sync import androidx.core.content.edit +import com.simprints.core.DispatcherIO +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.domain.sync.SyncOutcome +import com.simprints.core.domain.sync.toSyncFailureReason import com.simprints.core.tools.time.TimeHelper +import com.simprints.core.tools.time.Timestamp import com.simprints.infra.security.SecurityManager +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.onStart +import kotlinx.coroutines.withContext import javax.inject.Inject +import javax.inject.Singleton +/** + * When sample upload was last attempted, and how it turned out. + */ +@Singleton class ImageSyncTimestampProvider @Inject constructor( - securityManager: SecurityManager, + private val securityManager: SecurityManager, private val timeHelper: TimeHelper, + @param:DispatcherIO private val dispatcher: CoroutineDispatcher, ) { - private val securePrefs = securityManager.buildEncryptedSharedPreferences(SECURE_PREF_FILE_NAME) + private val securePrefs by lazy { securityManager.buildEncryptedSharedPreferences(SECURE_PREF_FILE_NAME) } + private val lastSyncOutcome = MutableStateFlow(null) - fun saveImageSyncCompletionTimestampNow() { - securePrefs.edit { putLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, timeHelper.now().ms) } + /** The last attempt and how it turned out, then every later one. */ + // compareAndSet, not assignment: the worker can write between this read and the store, and the + // stale value would otherwise win and stick until the next upload. + fun observeLastSyncOutcome(): Flow = + lastSyncOutcome.onStart { lastSyncOutcome.compareAndSet(null, getLastSyncOutcome()) } + + /** When the upload worker last ran and how it turned out; null when it never has. */ + suspend fun getLastSyncOutcome(): SyncOutcome? = withContext(dispatcher) { + securePrefs + .getLong(IMAGE_SYNC_ATTEMPT_TIME_MILLIS, 0) + .takeIf { securePrefs.contains(IMAGE_SYNC_ATTEMPT_TIME_MILLIS) } + ?.let { at -> + SyncOutcome( + timestamp = Timestamp(at), + failure = securePrefs.getString(IMAGE_SYNC_FAILURE_REASON, null)?.toSyncFailureReason(), + ) + } } - fun getMillisSinceLastImageSync(): Long? = - if (securePrefs.contains(IMAGE_SYNC_COMPLETION_TIME_MILLIS)) { - timeHelper.now().ms - securePrefs.getLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, 0) - } else { - null - } + /** + * When the last upload actually *completed*, which is what says how stale this device's + * samples are. The last attempt cannot answer it - it moves even when nothing uploaded. + */ + suspend fun getLastSuccessfulSyncTimestamp(): Long? = withContext(dispatcher) { + securePrefs + .getLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, 0) + .takeIf { securePrefs.contains(IMAGE_SYNC_COMPLETION_TIME_MILLIS) } + } - fun getLastImageSyncTimestamp(): Long? = securePrefs - .getLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, 0) - .takeIf { - securePrefs.contains(IMAGE_SYNC_COMPLETION_TIME_MILLIS) + suspend fun getMillisSinceLastImageSync(): Long? = getLastSuccessfulSyncTimestamp()?.let { timeHelper.now().ms - it } + + /** Records an attempt: when it ended, and why it failed when it did. */ + suspend fun saveImageSyncOutcomeNow(failure: SyncFailureReason?) { + val at = timeHelper.now() + withContext(dispatcher) { + securePrefs.edit { + putLong(IMAGE_SYNC_ATTEMPT_TIME_MILLIS, at.ms) + if (failure == null) { + remove(IMAGE_SYNC_FAILURE_REASON) + putLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, at.ms) + } else { + putString(IMAGE_SYNC_FAILURE_REASON, failure.name) + } + } } + lastSyncOutcome.value = SyncOutcome(at, failure) + } - fun clearTimestamp() { - securePrefs.edit { clear() } + suspend fun clearLastSyncOutcome() { + withContext(dispatcher) { securePrefs.edit { clear() } } + lastSyncOutcome.value = null } companion object { private const val SECURE_PREF_FILE_NAME = "93e98bc1-5b25-4805-94f6-f55ce0400747" + + // The completion time keeps the key it has always been stored under, so an upgrading + // device carries it over. The attempt keys are new and start empty on upgrade. private const val IMAGE_SYNC_COMPLETION_TIME_MILLIS = "IMAGE_SYNC_COMPLETION_TIME_MILLIS" + private const val IMAGE_SYNC_ATTEMPT_TIME_MILLIS = "IMAGE_SYNC_ATTEMPT_TIME_MILLIS" + private const val IMAGE_SYNC_FAILURE_REASON = "IMAGE_SYNC_FAILURE_REASON" } } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestratorImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestratorImpl.kt index ccdb3eaa72..364550762a 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestratorImpl.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestratorImpl.kt @@ -184,7 +184,7 @@ internal class SyncOrchestratorImpl @Inject constructor( override suspend fun deleteEventSyncInfo() { deleteSyncInfo() workManager.pruneWork() - imageSyncTimestampProvider.clearTimestamp() + imageSyncTimestampProvider.clearLastSyncOutcome() } override fun cleanupWorkers() { diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt index d625aac856..22a3d9b6c8 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt @@ -1,9 +1,11 @@ package com.simprints.infra.sync.devicestate +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.event.domain.models.scope.EventScopeType /** - * How much data is currently sitting on this device, and when each upload channel last succeeded. + * How much data is currently sitting on this device, and how each upload channel last fared. * * `null` means the underlying source could not be read, `0` means there is genuinely nothing. * Consumers must not collapse the two as they have very different consequences for a device that may be at risk. @@ -15,6 +17,8 @@ data class DeviceDataState( val pendingEvents: Int?, val pendingEnrolments: Int?, val pendingSamples: Int?, - val lastEventSyncAt: Long?, // Null when never synced on this install, or when the store could not be read. - val lastSampleSyncAt: Long?, // Null when never synced on this install, or when the store could not be read. + val lastEventSyncAt: Timestamp?, + val lastEventSyncFailure: SyncFailureReason?, + val lastSampleSyncAt: Timestamp?, + val lastSampleSyncFailure: SyncFailureReason?, ) diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt index 0d82ae368b..32681956d8 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt @@ -1,12 +1,13 @@ package com.simprints.infra.sync.devicestate import com.simprints.core.AppScope -import com.simprints.core.DispatcherIO +import com.simprints.core.domain.sync.SyncOutcome import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.enrolment.records.repository.EnrolmentRecordRepository import com.simprints.infra.enrolment.records.repository.domain.models.EnrolmentRecordQuery import com.simprints.infra.events.EventRepository import com.simprints.infra.events.event.domain.models.EventType +import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.eventsync.sync.common.EventSyncCache import com.simprints.infra.images.ImageRepository import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC @@ -15,7 +16,6 @@ import com.simprints.infra.sync.ImageSyncTimestampProvider import com.simprints.infra.sync.devicestate.internal.ObserveEnrolmentRecordsCountUseCase import com.simprints.infra.sync.devicestate.internal.ObserveSamplesToUploadCountUseCase import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -31,7 +31,6 @@ import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.update -import kotlinx.coroutines.withContext import javax.inject.Inject import javax.inject.Singleton @@ -46,7 +45,6 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private val observeEnrolmentRecordsCount: ObserveEnrolmentRecordsCountUseCase, private val observeSamplesToUploadCount: ObserveSamplesToUploadCountUseCase, @param:AppScope private val appScope: CoroutineScope, - @param:DispatcherIO private val dispatcherIO: CoroutineDispatcher, ) : DeviceStateDataTracker { /** Bumped by [refresh] to re-subscribe every source; see [deferred]. */ private val refreshes = MutableStateFlow(0) @@ -56,24 +54,20 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( deferred(PROJECT_ID) { observeProjectId() }, deferred(RECORD_COUNT) { observeEnrolmentRecordsCount() }, deferred(PENDING_SAMPLES) { observeSamplesToUploadCount() }, - deferred(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts() }, - // Combined only after each is made null-safe, so a failure in the enrolment counts - // cannot also blank the total, and each failure is logged against its own source. - combine( - deferred(PENDING_EVENTS) { observeTotalEventCount() }, - deferred(PENDING_ENROLMENTS) { observePendingEnrolmentCount() }, - ) { events, enrolments -> events to enrolments }, - ) { projectId, records, samples, scopes, (events, enrolments) -> + observeEventCounts(), + observeLastSyncs(), + ) { projectId, records, samples, eventCounts, (lastEventSync, lastSampleSync) -> DeviceDataState( projectId = projectId, recordCount = records, - pendingScopes = scopes, - pendingEvents = events, - pendingEnrolments = enrolments, + pendingScopes = eventCounts.pendingScopes, + pendingEvents = eventCounts.pendingEvents, + pendingEnrolments = eventCounts.pendingEnrolments, pendingSamples = samples, - // Encrypted preferences have no change notification, so they are re-read per emission. - lastEventSyncAt = readLastEventSyncAt(), - lastSampleSyncAt = readLastSampleSyncAt(), + lastEventSyncAt = lastEventSync?.timestamp, + lastEventSyncFailure = lastEventSync?.failure, + lastSampleSyncAt = lastSampleSync?.timestamp, + lastSampleSyncFailure = lastSampleSync?.failure, ) }.shareIn( appScope, @@ -95,6 +89,8 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( override suspend fun getCurrentDeviceDataState(): DeviceDataState { val config = readOrNull(PROJECT_ID) { configRepository.getProjectConfiguration() } val projectId = config?.projectId?.takeIf { it.isNotBlank() } + val lastEventSync = readOrNull(LAST_EVENT_SYNC) { eventSyncCache.readLastSyncOutcome() } + val lastSampleSync = readOrNull(LAST_SAMPLE_SYNC) { imageSyncTimestampProvider.getLastSyncOutcome() } return DeviceDataState( projectId = projectId, @@ -111,8 +107,10 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( projectId == null -> 0 // signed out / nothing yet else -> readOrNull(PENDING_SAMPLES) { imageRepository.getNumberOfImagesToUpload(projectId) } }, - lastEventSyncAt = readLastEventSyncAt(), - lastSampleSyncAt = readLastSampleSyncAt(), + lastEventSyncAt = lastEventSync?.timestamp, + lastEventSyncFailure = lastEventSync?.failure, + lastSampleSyncAt = lastSampleSync?.timestamp, + lastSampleSyncFailure = lastSampleSync?.failure, ) } @@ -120,16 +118,12 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( override suspend fun hasPendingEvents(): Boolean = countEvents() > 0 - private fun observeProjectId(): Flow = configRepository - .observeProjectConfiguration() - .map { it.projectId.takeIf { id -> id.isNotBlank() } } - .distinctUntilChanged() + private fun observeProjectId(): Flow = + configRepository.observeProjectConfiguration().map { it.projectId.takeIf { id -> id.isNotBlank() } }.distinctUntilChanged() // distinctUntilChanged because Room invalidates the events table on every insert, and each // emission re-runs the combine transform - including the encrypted timestamp reads. - private fun observeTotalEventCount(): Flow = eventRepository - .observeEventCount(type = null) - .distinctUntilChanged() + private fun observeTotalEventCount(): Flow = eventRepository.observeEventCount(type = null).distinctUntilChanged() private fun observePendingEnrolmentCount(): Flow = combine( eventRepository.observeEventCount(EventType.ENROLMENT_V2), @@ -144,20 +138,20 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private suspend fun countEvents(): Int = eventRepository.observeEventCount(type = null).first() - private suspend fun countEnrolmentEvents(): Int = eventRepository.observeEventCount(EventType.ENROLMENT_V2).first() + - eventRepository.observeEventCount(EventType.ENROLMENT_V4).first() + private suspend fun countEnrolmentEvents(): Int = eventRepository.observeEventCount(EventType.ENROLMENT_V2).first() + eventRepository + .observeEventCount(EventType.ENROLMENT_V4) + .first() - private suspend fun readLastEventSyncAt(): Long? = readOrNull(LAST_EVENT_SYNC) { - eventSyncCache.readLastSuccessfulSyncTime()?.ms - } + private fun observeEventCounts(): Flow = combine( + deferred(PENDING_SCOPES) { eventRepository.observeClosedEventScopeCounts() }, + deferred(PENDING_EVENTS) { observeTotalEventCount() }, + deferred(PENDING_ENROLMENTS) { observePendingEnrolmentCount() }, + ) { scopes, events, enrolments -> EventCounts(scopes, events, enrolments) } - /** - * `withContext` because this decrypts an EncryptedSharedPreferences entry and the combine - * transform runs on [appScope], which is the main dispatcher. - */ - private suspend fun readLastSampleSyncAt(): Long? = withContext(dispatcherIO) { - readOrNull(LAST_SAMPLE_SYNC) { imageSyncTimestampProvider.getLastImageSyncTimestamp() } - } + private fun observeLastSyncs(): Flow> = combine( + deferred(LAST_EVENT_SYNC) { eventSyncCache.observeLastSyncOutcome() }, + deferred(LAST_SAMPLE_SYNC) { imageSyncTimestampProvider.observeLastSyncOutcome() }, + ) { event, sample -> event to sample } /** * Builds the source flow only once something collects, so that merely holding the tracker @@ -194,6 +188,12 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( } } + private data class EventCounts( + val pendingScopes: Map?, + val pendingEvents: Int?, + val pendingEnrolments: Int?, + ) + companion object { private const val PROJECT_ID = "project id" private const val RECORD_COUNT = "record count" @@ -201,7 +201,7 @@ internal class DeviceStateDataTrackerImpl @Inject constructor( private const val PENDING_EVENTS = "pending events" private const val PENDING_ENROLMENTS = "pending enrolments" private const val PENDING_SAMPLES = "pending samples" - private const val LAST_EVENT_SYNC = "last event sync time" - private const val LAST_SAMPLE_SYNC = "last sample sync time" + private const val LAST_EVENT_SYNC = "last event sync outcome" + private const val LAST_SAMPLE_SYNC = "last sample sync outcome" } } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt b/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt index ddde86c0dc..6d140e214b 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt @@ -5,6 +5,7 @@ import androidx.hilt.work.HiltWorker import androidx.work.WorkerParameters import androidx.work.workDataOf import com.simprints.core.DispatcherBG +import com.simprints.core.domain.sync.SyncFailureReason import com.simprints.core.workers.SimCoroutineWorker import com.simprints.infra.authstore.AuthStore import com.simprints.infra.images.ImageRepository @@ -42,12 +43,14 @@ internal class FileUpSyncWorker @AssistedInject constructor( ) } if (uploadSuccessful) { - imageSyncTimestampProvider.saveImageSyncCompletionTimestampNow() + imageSyncTimestampProvider.saveImageSyncOutcomeNow(failure = null) success() } else { + imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) retry() } } catch (ex: Exception) { + imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) retry(ex) } } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCase.kt b/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCase.kt index e53d361b49..a485f75cc3 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCase.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCase.kt @@ -20,7 +20,7 @@ internal class ObserveImageSyncStatusUseCase @Inject constructor( .getWorkInfosFlow(WorkQuery.fromUniqueWorkNames(SyncConstants.FILE_UP_SYNC_WORK_NAME)) .associateWithIfSyncing() .map { (workInfos, isSyncing) -> - val lastUpdateTimestamp = imageSyncTimestampProvider.getLastImageSyncTimestamp() + val lastUpdateTimestamp = imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp() val currentIndex = workInfos .firstOrNull() ?.progress diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt index 9f953dd5d9..e054b42450 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt @@ -1,18 +1,28 @@ package com.simprints.infra.sync import android.content.SharedPreferences +import app.cash.turbine.test import com.google.common.truth.Truth.assertThat +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.domain.sync.SyncOutcome import com.simprints.core.tools.time.TimeHelper import com.simprints.core.tools.time.Timestamp import com.simprints.infra.security.SecurityManager +import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.verify +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest import org.junit.Before +import org.junit.Rule import org.junit.Test class ImageSyncTimestampProviderTest { + @get:Rule + val testCoroutineRule = TestCoroutineRule() + @MockK private lateinit var securityManager: SecurityManager @@ -34,100 +44,192 @@ class ImageSyncTimestampProviderTest { every { securityManager.buildEncryptedSharedPreferences(any()) } returns sharedPreferences every { sharedPreferences.edit() } returns editor every { editor.putLong(any(), any()) } returns editor + every { editor.putString(any(), any()) } returns editor + every { editor.remove(any()) } returns editor every { editor.clear() } returns editor + storedTimestamp(null) + storedSuccessTimestamp(null) + every { sharedPreferences.getString(FAILURE_KEY, null) } returns null imageSyncTimestampProvider = ImageSyncTimestampProvider( securityManager = securityManager, timeHelper = timeHelper, + dispatcher = testCoroutineRule.testCoroutineDispatcher, ) } @Test - fun `saveImageSyncCompletionTimestampNow saves current timestamp to secure preferences`() { - val currentTime = 1234567890L - every { timeHelper.now() } returns Timestamp(currentTime) + fun `getLastSyncOutcome returns null until an upload has been attempted`() = runTest { + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()).isNull() + } - imageSyncTimestampProvider.saveImageSyncCompletionTimestampNow() + @Test + fun `getLastSyncOutcome returns the time of an attempt that succeeded`() = runTest { + storedTimestamp(1234567890L) - verify { - sharedPreferences.edit() - editor.putLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", currentTime) - } + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(1234567890L), failure = null)) } @Test - fun `getMillisSinceLastImageSync returns null when no timestamp exists`() { - every { sharedPreferences.contains("IMAGE_SYNC_COMPLETION_TIME_MILLIS") } returns false - every { sharedPreferences.getLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 0) } returns 0 + fun `getLastSyncOutcome returns the time and reason of an attempt that failed`() = runTest { + storedTimestamp(1234567890L) + every { sharedPreferences.getString(FAILURE_KEY, null) } returns "UNKNOWN" + + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(1234567890L), SyncFailureReason.UNKNOWN)) + } - val result = imageSyncTimestampProvider.getMillisSinceLastImageSync() + @Test + fun `getLastSyncOutcome decodes a name from another version as unknown`() = runTest { + storedTimestamp(1234567890L) + every { sharedPreferences.getString(FAILURE_KEY, null) } returns "SOMETHING_ELSE" - assertThat(result).isNull() + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()?.failure).isEqualTo(SyncFailureReason.UNKNOWN) } @Test - fun `getMillisSinceLastImageSync returns null when timestamp is zero`() { - every { sharedPreferences.contains("IMAGE_SYNC_COMPLETION_TIME_MILLIS") } returns false - every { sharedPreferences.getLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 0) } returns 0 + fun `getMillisSinceLastImageSync returns null when nothing has succeeded`() = runTest { + assertThat(imageSyncTimestampProvider.getMillisSinceLastImageSync()).isNull() + } - val result = imageSyncTimestampProvider.getMillisSinceLastImageSync() + @Test + fun `getMillisSinceLastImageSync measures from the last success, not the last attempt`() = runTest { + storedSuccessTimestamp(1_000_000L) + storedTimestamp(1_004_000L) + every { timeHelper.now() } returns Timestamp(1_005_000L) - assertThat(result).isNull() + assertThat(imageSyncTimestampProvider.getMillisSinceLastImageSync()).isEqualTo(5_000L) } @Test - fun `getMillisSinceLastImageSync returns correct seconds when timestamp exists`() { - val lastSyncTimeMillis = 1000000L - val currentTimeMillis = 1005000L // 5 seconds later - val expectedMillis = 5_000L + fun `getLastSuccessfulSyncTimestamp returns the last completed upload`() = runTest { + storedSuccessTimestamp(1_000_000L) - every { sharedPreferences.contains("IMAGE_SYNC_COMPLETION_TIME_MILLIS") } returns true - every { sharedPreferences.getLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 0) } returns lastSyncTimeMillis - every { timeHelper.now() } returns Timestamp(currentTimeMillis) + assertThat(imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp()).isEqualTo(1_000_000L) + } - val result = imageSyncTimestampProvider.getMillisSinceLastImageSync() + @Test + fun `saveImageSyncOutcomeNow stores the time and the reason of a failure`() = runTest { + every { timeHelper.now() } returns Timestamp(1234567890L) - assertThat(result).isEqualTo(expectedMillis) + imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) + + verify { + editor.putLong(ATTEMPT_KEY, 1234567890L) + editor.putString(FAILURE_KEY, "UNKNOWN") + } + // A failing device must not read as freshly synced on the dashboard. + verify(exactly = 0) { editor.putLong(SUCCESS_KEY, any()) } } @Test - fun `getLastImageSyncTimestamp returns null when no timestamp exists`() { - every { sharedPreferences.contains("IMAGE_SYNC_COMPLETION_TIME_MILLIS") } returns false - every { sharedPreferences.getLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 0) } returns 0 + fun `saveImageSyncOutcomeNow clears a previous reason on success`() = runTest { + every { timeHelper.now() } returns Timestamp(1234567890L) - val result = imageSyncTimestampProvider.getLastImageSyncTimestamp() + imageSyncTimestampProvider.saveImageSyncOutcomeNow(failure = null) - assertThat(result).isNull() + verify { + editor.putLong(ATTEMPT_KEY, 1234567890L) + editor.putLong(SUCCESS_KEY, 1234567890L) + editor.remove(FAILURE_KEY) + } } @Test - fun `getLastImageSyncTimestamp returns correct timestamp when exists`() { - val storedTimestamp = 1234567890L - every { sharedPreferences.contains("IMAGE_SYNC_COMPLETION_TIME_MILLIS") } returns true - every { sharedPreferences.getLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 0) } returns storedTimestamp + fun `the success time keeps the key it has always been stored under`() = runTest { + every { timeHelper.now() } returns Timestamp(1234567890L) - val result = imageSyncTimestampProvider.getLastImageSyncTimestamp() + imageSyncTimestampProvider.saveImageSyncOutcomeNow(failure = null) - assertThat(result).isEqualTo(storedTimestamp) + // An upgrading device must carry it over rather than read as never synced. + verify(exactly = 1) { editor.putLong("IMAGE_SYNC_COMPLETION_TIME_MILLIS", 1234567890L) } } @Test - fun `clearTimestamp clears all timestamp preferences`() { - imageSyncTimestampProvider.clearTimestamp() + fun `observeLastSyncOutcome does not overwrite a value written while it was loading`() = runTest { + storedTimestamp(30L) + every { timeHelper.now() } returns Timestamp(90L) + // The worker got there first; the stale read below must not win. + imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) + + assertThat(imageSyncTimestampProvider.observeLastSyncOutcome().first()) + .isEqualTo(SyncOutcome(Timestamp(90L), SyncFailureReason.UNKNOWN)) + } - val result = imageSyncTimestampProvider.getMillisSinceLastImageSync() + @Test + fun `observeLastSyncOutcome starts from what is stored`() = runTest { + storedTimestamp(1234567890L) + every { sharedPreferences.getString(FAILURE_KEY, null) } returns "RELOGIN_REQUIRED" - assertThat(result).isNull() - verify { - sharedPreferences.edit() - editor.clear() + imageSyncTimestampProvider.observeLastSyncOutcome().test { + assertThat(awaitItem()).isEqualTo(SyncOutcome(Timestamp(1234567890L), SyncFailureReason.RELOGIN_REQUIRED)) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `observeLastSyncOutcome publishes every write`() = runTest { + every { timeHelper.now() } returns Timestamp(1234567890L) + + imageSyncTimestampProvider.observeLastSyncOutcome().test { + assertThat(awaitItem()).isNull() + + // Encrypted preferences have no change notification of their own. + imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) + + assertThat(awaitItem()).isEqualTo(SyncOutcome(Timestamp(1234567890L), SyncFailureReason.UNKNOWN)) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `clearLastSyncOutcome wipes the store and publishes the clear`() = runTest { + storedTimestamp(1234567890L) + + imageSyncTimestampProvider.observeLastSyncOutcome().test { + assertThat(awaitItem()).isNotNull() + + imageSyncTimestampProvider.clearLastSyncOutcome() + + assertThat(awaitItem()).isNull() + cancelAndIgnoreRemainingEvents() } + verify { editor.clear() } } @Test - fun `provider uses correct secure preference file name`() { + fun `provider uses correct secure preference file name`() = runTest { + imageSyncTimestampProvider.getLastSyncOutcome() + verify { securityManager.buildEncryptedSharedPreferences("93e98bc1-5b25-4805-94f6-f55ce0400747") } } + + @Test + fun `the encrypted file is not opened until something reads or writes it`() = runTest { + // Opening it is a file operation too, so it has to wait for a call that runs on IO. + verify(exactly = 0) { securityManager.buildEncryptedSharedPreferences(any()) } + + imageSyncTimestampProvider.getLastSyncOutcome() + + verify(exactly = 1) { securityManager.buildEncryptedSharedPreferences(any()) } + } + + private fun storedTimestamp(ms: Long?) { + every { sharedPreferences.contains(ATTEMPT_KEY) } returns (ms != null) + every { sharedPreferences.getLong(ATTEMPT_KEY, 0) } returns (ms ?: 0) + } + + private fun storedSuccessTimestamp(ms: Long?) { + every { sharedPreferences.contains(SUCCESS_KEY) } returns (ms != null) + every { sharedPreferences.getLong(SUCCESS_KEY, 0) } returns (ms ?: 0) + } + + private companion object { + const val ATTEMPT_KEY = "IMAGE_SYNC_ATTEMPT_TIME_MILLIS" + const val SUCCESS_KEY = "IMAGE_SYNC_COMPLETION_TIME_MILLIS" + const val FAILURE_KEY = "IMAGE_SYNC_FAILURE_REASON" + } } diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/SyncOrchestratorImplTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/SyncOrchestratorImplTest.kt index 1ac18bf18f..0dcf6cd291 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/SyncOrchestratorImplTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/SyncOrchestratorImplTest.kt @@ -135,7 +135,7 @@ class SyncOrchestratorImplTest { syncOrchestrator.deleteEventSyncInfo() coVerify { deleteSyncInfo() } verify { workManager.pruneWork() } - verify { imageSyncTimestampProvider.clearTimestamp() } + coVerify { imageSyncTimestampProvider.clearLastSyncOutcome() } } @Test diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt index 27a7aa38db..c96773ad83 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt @@ -2,6 +2,8 @@ package com.simprints.infra.sync.devicestate import app.cash.turbine.test import com.google.common.truth.Truth.* +import com.simprints.core.domain.sync.SyncFailureReason +import com.simprints.core.domain.sync.SyncOutcome import com.simprints.core.tools.time.Timestamp import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.config.store.models.ProjectConfiguration @@ -16,10 +18,6 @@ import com.simprints.infra.sync.ImageSyncTimestampProvider import com.simprints.infra.sync.devicestate.internal.ObserveEnrolmentRecordsCountUseCase import com.simprints.infra.sync.devicestate.internal.ObserveSamplesToUploadCountUseCase import io.mockk.* -import io.mockk.MockKAnnotations -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.every import io.mockk.impl.annotations.MockK import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -28,7 +26,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch -import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Before @@ -82,8 +79,11 @@ class DeviceStateDataTrackerImplTest { coEvery { imageRepository.getNumberOfImagesToUpload(any()) } returns SAMPLES every { observeSamplesToUploadCount() } returns flowOf(SAMPLES) - coEvery { eventSyncCache.readLastSuccessfulSyncTime() } returns Timestamp(EVENT_SYNC_AT) - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns SAMPLE_SYNC_AT + every { eventSyncCache.observeLastSyncOutcome() } returns flowOf(EVENT_SYNC_OUTCOME) + coEvery { eventSyncCache.readLastSyncOutcome() } returns EVENT_SYNC_OUTCOME + + every { imageSyncTimestampProvider.observeLastSyncOutcome() } returns flowOf(SAMPLE_SYNC_OUTCOME) + coEvery { imageSyncTimestampProvider.getLastSyncOutcome() } returns SAMPLE_SYNC_OUTCOME } // --- aggregation --- @@ -101,7 +101,9 @@ class DeviceStateDataTrackerImplTest { pendingEnrolments = ENROLMENTS_V2 + ENROLMENTS_V4, pendingSamples = SAMPLES, lastEventSyncAt = EVENT_SYNC_AT, + lastEventSyncFailure = null, lastSampleSyncAt = SAMPLE_SYNC_AT, + lastSampleSyncFailure = null, ), ) } @@ -118,7 +120,9 @@ class DeviceStateDataTrackerImplTest { pendingEnrolments = ENROLMENTS_V2 + ENROLMENTS_V4, pendingSamples = SAMPLES, lastEventSyncAt = EVENT_SYNC_AT, + lastEventSyncFailure = null, lastSampleSyncAt = SAMPLE_SYNC_AT, + lastSampleSyncFailure = null, ), ) cancelAndIgnoreRemainingEvents() @@ -189,14 +193,16 @@ class DeviceStateDataTrackerImplTest { } @Test - fun `snapshot reports failing sync timestamps as null and keeps the other fields`() = runTest { - coEvery { eventSyncCache.readLastSuccessfulSyncTime() } throws RuntimeException() - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } throws RuntimeException() + fun `snapshot reports failing sync outcomes as null and keeps the other fields`() = runTest { + coEvery { eventSyncCache.readLastSyncOutcome() } throws RuntimeException() + coEvery { imageSyncTimestampProvider.getLastSyncOutcome() } throws RuntimeException() val state = tracker(backgroundScope).getCurrentDeviceDataState() assertThat(state.lastEventSyncAt).isNull() + assertThat(state.lastEventSyncFailure).isNull() assertThat(state.lastSampleSyncAt).isNull() + assertThat(state.lastSampleSyncFailure).isNull() assertThat(state.recordCount).isEqualTo(RECORDS) } @@ -454,8 +460,8 @@ class DeviceStateDataTrackerImplTest { @Test fun `never-synced timestamps stay null rather than becoming zero`() = runTest { - coEvery { eventSyncCache.readLastSuccessfulSyncTime() } returns null - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns null + coEvery { eventSyncCache.readLastSyncOutcome() } returns null + coEvery { imageSyncTimestampProvider.getLastSyncOutcome() } returns null val state = tracker(backgroundScope).getCurrentDeviceDataState() @@ -469,8 +475,62 @@ class DeviceStateDataTrackerImplTest { tracker.getCurrentDeviceDataState() tracker.getCurrentDeviceDataState() - coVerify(exactly = 2) { eventSyncCache.readLastSuccessfulSyncTime() } - verify(exactly = 2) { imageSyncTimestampProvider.getLastImageSyncTimestamp() } + coVerify(exactly = 2) { eventSyncCache.readLastSyncOutcome() } + coVerify(exactly = 2) { imageSyncTimestampProvider.getLastSyncOutcome() } + } + + // --- last sync outcome --- + + @Test + fun `snapshot carries the failure reason of each channel`() = runTest { + coEvery { eventSyncCache.readLastSyncOutcome() } returns + SyncOutcome(EVENT_SYNC_AT, SyncFailureReason.BACKEND_MAINTENANCE) + coEvery { imageSyncTimestampProvider.getLastSyncOutcome() } returns + SyncOutcome(SAMPLE_SYNC_AT, SyncFailureReason.UNKNOWN) + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + assertThat(state.lastEventSyncFailure).isEqualTo(SyncFailureReason.BACKEND_MAINTENANCE) + assertThat(state.lastSampleSyncFailure).isEqualTo(SyncFailureReason.UNKNOWN) + } + + @Test + fun `a failure keeps the timestamp of the run that failed`() = runTest { + coEvery { eventSyncCache.readLastSyncOutcome() } returns + SyncOutcome(EVENT_SYNC_AT, SyncFailureReason.CLOUD_INTEGRATION) + + val state = tracker(backgroundScope).getCurrentDeviceDataState() + + // A device that keeps failing must not read as one that stopped syncing. + assertThat(state.lastEventSyncAt).isEqualTo(EVENT_SYNC_AT) + } + + @Test + fun `observed state re-emits when a sync fails and moves no count`() = runTest { + val outcomes = MutableStateFlow(EVENT_SYNC_OUTCOME) + every { eventSyncCache.observeLastSyncOutcome() } returns outcomes + + tracker(backgroundScope).observeDeviceDataState().test { + assertThat(awaitItem().lastEventSyncFailure).isNull() + + outcomes.value = SyncOutcome(EVENT_SYNC_AT, SyncFailureReason.RELOGIN_REQUIRED) + + assertThat(awaitItem().lastEventSyncFailure).isEqualTo(SyncFailureReason.RELOGIN_REQUIRED) + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `observed state reports a failing outcome store as null rather than failing the stream`() = runTest { + every { imageSyncTimestampProvider.observeLastSyncOutcome() } returns flow { throw RuntimeException() } + + tracker(backgroundScope).observeDeviceDataState().test { + val state = awaitItem() + assertThat(state.lastSampleSyncAt).isNull() + assertThat(state.lastSampleSyncFailure).isNull() + assertThat(state.lastEventSyncAt).isEqualTo(EVENT_SYNC_AT) + cancelAndIgnoreRemainingEvents() + } } private fun tracker(scope: CoroutineScope) = DeviceStateDataTrackerImpl( @@ -483,7 +543,6 @@ class DeviceStateDataTrackerImplTest { observeEnrolmentRecordsCount = observeEnrolmentRecordsCount, observeSamplesToUploadCount = observeSamplesToUploadCount, appScope = scope, - dispatcherIO = UnconfinedTestDispatcher(), ) companion object { @@ -493,8 +552,10 @@ class DeviceStateDataTrackerImplTest { private const val ENROLMENTS_V2 = 2 private const val ENROLMENTS_V4 = 3 private const val SAMPLES = 4 - private const val EVENT_SYNC_AT = 1000L - private const val SAMPLE_SYNC_AT = 2000L + private val EVENT_SYNC_AT = Timestamp(1000L) + private val SAMPLE_SYNC_AT = Timestamp(2000L) + private val EVENT_SYNC_OUTCOME = SyncOutcome(EVENT_SYNC_AT, failure = null) + private val SAMPLE_SYNC_OUTCOME = SyncOutcome(SAMPLE_SYNC_AT, failure = null) private val SCOPE_COUNTS = EventScopeType.entries.associateWith { 1 } } } diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt index 9da3c7d4b0..a29844a421 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt @@ -3,6 +3,7 @@ package com.simprints.infra.sync.files import android.os.PowerManager import androidx.work.ListenableWorker.Result import com.google.common.truth.* +import com.simprints.core.domain.sync.SyncFailureReason import com.simprints.infra.authstore.AuthStore import com.simprints.infra.images.ImageRepository import com.simprints.infra.sync.ImageSyncTimestampProvider @@ -64,7 +65,7 @@ class FileUpSyncWorkerTest { // Then Truth.assertThat(Result.success()).isEqualTo(result) coVerify(exactly = 1) { imageRepository.uploadStoredImagesAndDelete(PROJECT_ID, any()) } - coVerify(exactly = 1) { imageSyncTimestampProvider.saveImageSyncCompletionTimestampNow() } + coVerify(exactly = 1) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(failure = null) } } @Test @@ -78,7 +79,8 @@ class FileUpSyncWorkerTest { // Then Truth.assertThat(Result.retry()).isEqualTo(result) coVerify(exactly = 1) { imageRepository.uploadStoredImagesAndDelete(PROJECT_ID, any()) } - coVerify(exactly = 0) { imageSyncTimestampProvider.saveImageSyncCompletionTimestampNow() } + // A retry is still an attempt that ended without uploading anything. + coVerify(exactly = 1) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) } } @Test @@ -92,7 +94,7 @@ class FileUpSyncWorkerTest { // Then Truth.assertThat(Result.retry()).isEqualTo(result) coVerify(exactly = 1) { imageRepository.uploadStoredImagesAndDelete(any(), any()) } - coVerify(exactly = 0) { imageSyncTimestampProvider.saveImageSyncCompletionTimestampNow() } + coVerify(exactly = 1) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) } } @Test diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCaseTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCaseTest.kt index 3e78dcd40b..299c2e3d30 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCaseTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/usecase/internal/ObserveImageSyncStatusUseCaseTest.kt @@ -7,6 +7,7 @@ import com.google.common.truth.Truth.assertThat import com.simprints.infra.sync.ImageSyncTimestampProvider import com.simprints.infra.sync.SyncConstants import io.mockk.MockKAnnotations +import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk @@ -30,6 +31,7 @@ class ObserveImageSyncStatusUseCaseTest { @Before fun setup() { MockKAnnotations.init(this, relaxed = true) + coEvery { imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp() } returns null useCase = ObserveImageSyncStatusUseCase( workManager = workManager, imageSyncTimestampProvider = imageSyncTimestampProvider, @@ -40,8 +42,8 @@ class ObserveImageSyncStatusUseCaseTest { fun `image sync status returns syncing when worker is running`() = runTest { val workInfoFlow = flowOf(createWorkInfos(WorkInfo.State.RUNNING)) every { workManager.getWorkInfosFlow(any()) } returns workInfoFlow - every { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 30_000L - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns 1_234_567_890L + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 30_000L + coEvery { imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp() } returns 1_234_567_890L val status = useCase().first() @@ -53,8 +55,8 @@ class ObserveImageSyncStatusUseCaseTest { fun `image sync status returns not syncing when worker is cancelled`() = runTest { val workInfoFlow = flowOf(createWorkInfos(WorkInfo.State.CANCELLED)) every { workManager.getWorkInfosFlow(any()) } returns workInfoFlow - every { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 120_000L - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns 1_234_567_890L + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 120_000L + coEvery { imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp() } returns 1_234_567_890L val status = useCase().first() @@ -66,8 +68,8 @@ class ObserveImageSyncStatusUseCaseTest { fun `image sync status returns null timestamp when no sync has occurred`() = runTest { val workInfoFlow = flowOf(createWorkInfos(WorkInfo.State.CANCELLED)) every { workManager.getWorkInfosFlow(any()) } returns workInfoFlow - every { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns null - every { imageSyncTimestampProvider.getLastImageSyncTimestamp() } returns null + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns null + coEvery { imageSyncTimestampProvider.getLastSuccessfulSyncTimestamp() } returns null val status = useCase().first() @@ -81,7 +83,7 @@ class ObserveImageSyncStatusUseCaseTest { val workInfo2 = createWorkInfosWithProgress(WorkInfo.State.RUNNING) val workInfoFlow = flowOf(workInfo1, workInfo2) every { workManager.getWorkInfosFlow(any()) } returns workInfoFlow - every { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 0L + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 0L val status1 = useCase().first() assertThat(status1.progress).isEqualTo(5 to 10) @@ -94,7 +96,7 @@ class ObserveImageSyncStatusUseCaseTest { fun `image sync status returns syncing momentarily when worker succeeds quickly`() = runTest { val workInfoFlow = flowOf(createWorkInfos(WorkInfo.State.SUCCEEDED)) every { workManager.getWorkInfosFlow(any()) } returns workInfoFlow - every { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 0L + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 0L val status1 = useCase().first() assertThat(status1.isSyncing).isTrue() @@ -103,6 +105,15 @@ class ObserveImageSyncStatusUseCaseTest { assertThat(status2.isSyncing).isFalse() } + @Test + fun `image sync status does not pulse when the quick run uploaded nothing`() = runTest { + every { workManager.getWorkInfosFlow(any()) } returns flowOf(createWorkInfos(WorkInfo.State.SUCCEEDED)) + // A failed attempt leaves the completion time where it was, so it cannot read as "just now". + coEvery { imageSyncTimestampProvider.getMillisSinceLastImageSync() } returns 120_000L + + assertThat(useCase().first().isSyncing).isFalse() + } + private fun createWorkInfos(state: WorkInfo.State): List = listOf( WorkInfo(UUID.randomUUID(), state, emptySet()), ) From 4bea4f29d94a3003da4a5467bc51c9dca5a06f4c Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Wed, 30 Sep 2026 10:37:45 +0300 Subject: [PATCH 10/10] MS-1537 Addressing copilot review comments --- .../settings/about/AboutViewModel.kt | 3 + .../eventsync/sync/common/EventSyncCache.kt | 15 ++--- .../eventsync/sync/common/WorkInfo.ext.kt | 8 ++- .../BaseEventDownSyncDownloaderWorker.kt | 13 ++++ .../SimprintsEventDownSyncDownloaderWorker.kt | 2 + .../up/workers/EventUpSyncUploaderWorker.kt | 14 +++++ .../sync/common/EventSyncCacheTest.kt | 22 +++++++ ...printsEventDownSyncDownloaderWorkerTest.kt | 28 ++++++++- .../master/EventEndSyncReporterWorkerTest.kt | 61 +++++++++++++++++++ .../workers/EventUpSyncUploaderWorkerTest.kt | 34 ++++++++++- .../infra/sync/ImageSyncTimestampProvider.kt | 13 ++-- .../infra/sync/files/FileUpSyncWorker.kt | 5 ++ .../sync/ImageSyncTimestampProviderTest.kt | 20 ++++++ .../infra/sync/files/FileUpSyncWorkerTest.kt | 15 +++++ 14 files changed, 235 insertions(+), 18 deletions(-) diff --git a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt index e6cfd9df16..f2dfdb0420 100644 --- a/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt +++ b/feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt @@ -20,6 +20,7 @@ import com.simprints.infra.sync.OneTime import com.simprints.infra.sync.SyncOrchestrator import com.simprints.infra.sync.devicestate.DeviceStateDataTracker import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.launch import javax.inject.Inject @@ -82,6 +83,8 @@ internal class AboutViewModel @Inject constructor( private suspend fun hasEventsToUpload(): Boolean = try { deviceStateDataTracker.hasPendingEvents() + } catch (cancellation: CancellationException) { + throw cancellation } catch (t: Throwable) { Simber.i("Could not check pending events before logout", t, tag = SYNC) true diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt index 9d81c89868..b146b1a989 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/EventSyncCache.kt @@ -39,7 +39,7 @@ class EventSyncCache @Inject constructor( /** When event sync was last attempted and how it turned out; null when it never has been. */ suspend fun readLastSyncOutcome(): SyncOutcome? = withContext(dispatcher) { - sharedForLastSyncTime + val lastAttempt = sharedForLastSyncTime .getLong(LAST_ATTEMPT_TIME_KEY, -1) .takeIf { it >= 0 } ?.let { timestamp -> @@ -48,14 +48,15 @@ class EventSyncCache @Inject constructor( failure = sharedForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null)?.toSyncFailureReason(), ) } + lastAttempt ?: readLastSuccessfulSyncTimeBlocking()?.let { SyncOutcome(timestamp = it, failure = null) } } - suspend fun readLastSuccessfulSyncTime(): Timestamp? = withContext(dispatcher) { - sharedForLastSyncTime - .getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) - .takeIf { it >= 0 } - ?.let { Timestamp(it) } - } + suspend fun readLastSuccessfulSyncTime(): Timestamp? = withContext(dispatcher) { readLastSuccessfulSyncTimeBlocking() } + + private fun readLastSuccessfulSyncTimeBlocking(): Timestamp? = sharedForLastSyncTime + .getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) + .takeIf { it >= 0 } + ?.let { Timestamp(it) } /** Records an attempt: when it ended, and why it failed when it did. */ suspend fun storeLastSyncOutcome( diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt index 6db61edfd1..445ae7543d 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/common/WorkInfo.ext.kt @@ -8,6 +8,7 @@ internal const val OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS = "OUTPUT_FAILED_BECA internal const val OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION = "OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION" internal const val OUTPUT_FAILED_BECAUSE_BACKEND_MAINTENANCE = "OUTPUT_FAILED_BECAUSE_BACKEND_MAINTENANCE" internal const val OUTPUT_FAILED_BECAUSE_COMMCARE_PERMISSION_MISSING = "OUTPUT_FAILED_BECAUSE_COMMCARE_PERMISSION_MISSING" +internal const val OUTPUT_FAILED_UNEXPECTEDLY = "OUTPUT_FAILED_UNEXPECTEDLY" internal const val OUTPUT_ESTIMATED_MAINTENANCE_TIME = "OUTPUT_ESTIMATED_MAINTENANCE_TIME" internal fun WorkInfo.didFailBecauseReloginRequired(): Boolean = @@ -25,6 +26,8 @@ internal fun WorkInfo.didFailBecauseTooManyRequests(): Boolean = internal fun WorkInfo.didFailBecauseCommCarePermissionMissing(): Boolean = this.outputData.getBoolean(OUTPUT_FAILED_BECAUSE_COMMCARE_PERMISSION_MISSING, false) +internal fun WorkInfo.didFailUnexpectedly(): Boolean = this.outputData.getBoolean(OUTPUT_FAILED_UNEXPECTEDLY, false) + internal fun WorkInfo.getEstimatedOutageTime(): Long = this.outputData.getLong(OUTPUT_ESTIMATED_MAINTENANCE_TIME, 0L) internal fun WorkInfo.hasAnyFailureReason(): Boolean = @@ -32,7 +35,8 @@ internal fun WorkInfo.hasAnyFailureReason(): Boolean = didFailBecauseCloudIntegration() || didFailBecauseBackendMaintenance() || didFailBecauseTooManyRequests() || - didFailBecauseCommCarePermissionMissing() + didFailBecauseCommCarePermissionMissing() || + didFailUnexpectedly() internal fun List.firstFailureReason(): SyncFailureReason? = when { any { it.didFailBecauseReloginRequired() } -> SyncFailureReason.RELOGIN_REQUIRED @@ -40,5 +44,7 @@ internal fun List.firstFailureReason(): SyncFailureReason? = when { any { it.didFailBecauseBackendMaintenance() } -> SyncFailureReason.BACKEND_MAINTENANCE any { it.didFailBecauseTooManyRequests() } -> SyncFailureReason.TOO_MANY_REQUESTS any { it.didFailBecauseCommCarePermissionMissing() } -> SyncFailureReason.COMM_CARE_PERMISSION_MISSING + any { it.didFailUnexpectedly() } -> SyncFailureReason.UNKNOWN + any { it.state == WorkInfo.State.FAILED } -> SyncFailureReason.UNKNOWN else -> null } diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/BaseEventDownSyncDownloaderWorker.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/BaseEventDownSyncDownloaderWorker.kt index ea54461913..87fe43485d 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/BaseEventDownSyncDownloaderWorker.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/BaseEventDownSyncDownloaderWorker.kt @@ -16,6 +16,7 @@ import com.simprints.infra.eventsync.sync.common.WorkerProgressCountReporter import com.simprints.infra.eventsync.sync.down.tasks.BaseEventDownSyncTask import com.simprints.infra.logging.Simber import com.simprints.infra.serialization.SimJson +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext @@ -97,6 +98,18 @@ internal abstract class BaseEventDownSyncDownloaderWorker( "Total downloaded: $count / $max", ) } + } catch (cancellation: CancellationException) { + // Cancellation is not a sync failure, so it must not reach handleSyncException and be + // flagged as one. It must not propagate either: a worker that throws breaks the chain + // and the end reporter would never run to record the attempt. + Simber.i("Down-sync cancelled", cancellation, tag = tag) + success( + workDataOf( + OUTPUT_DOWN_SYNC to count, + OUTPUT_DOWN_MAX_SYNC to max, + ), + "Cancelled after: $count / $max", + ) } catch (t: Throwable) { handleSyncException(t, count, max) } diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorker.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorker.kt index 4ffc65fcca..8fb4033b27 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorker.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorker.kt @@ -17,6 +17,7 @@ import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_BACKEND_M import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS +import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_UNEXPECTEDLY import com.simprints.infra.eventsync.sync.down.tasks.BaseEventDownSyncTask import com.simprints.infra.eventsync.sync.down.tasks.SimprintsEventDownSyncTask import com.simprints.infra.logging.Simber @@ -86,6 +87,7 @@ internal class SimprintsEventDownSyncDownloaderWorker @AssistedInject constructo } else -> { + outputData.putBoolean(OUTPUT_FAILED_UNEXPECTEDLY, true) Simber.e("Down-sync completed with unexpected issue", t, tag = tag) } } diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorker.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorker.kt index 67a64f26d5..edded62eb4 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorker.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorker.kt @@ -20,6 +20,7 @@ import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_BACKEND_M import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS +import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_UNEXPECTEDLY import com.simprints.infra.eventsync.sync.common.WorkerProgressCountReporter import com.simprints.infra.eventsync.sync.up.tasks.EventUpSyncTask import com.simprints.infra.eventsync.sync.up.workers.EventUpSyncUploaderWorker.Companion.OUTPUT_UP_MAX_SYNC @@ -32,6 +33,7 @@ import com.simprints.infra.network.exceptions.SyncCloudIntegrationException import com.simprints.infra.serialization.SimJson import dagger.assisted.Assisted import dagger.assisted.AssistedInject +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.withContext @@ -92,6 +94,15 @@ internal class EventUpSyncUploaderWorker @AssistedInject constructor( createOutputData(count, max), "Total uploaded: $count / $max", ) + } catch (cancellation: CancellationException) { + // Cancellation is not a sync failure, so no failure flag is written. It must not + // propagate either: a worker that throws breaks the chain and the end reporter would + // never run to record the attempt. + Simber.i("Up-sync cancelled", cancellation, tag = tag) + success( + createOutputData(count, max), + "Cancelled after: $count / $max", + ) } catch (t: Throwable) { Simber.i("Up-sync completed with issue", t, tag = tag) success( @@ -119,6 +130,9 @@ internal class EventUpSyncUploaderWorker @AssistedInject constructor( outputDataBuilder .putBoolean(OUTPUT_FAILED_BECAUSE_BACKEND_MAINTENANCE, true) .putLong(OUTPUT_ESTIMATED_MAINTENANCE_TIME, t.estimatedOutage ?: 0L) + + null -> Unit + else -> outputDataBuilder.putBoolean(OUTPUT_FAILED_UNEXPECTEDLY, true) } return outputDataBuilder.build() diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt index a6b8fc5d28..11167e4a09 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/common/EventSyncCacheTest.kt @@ -83,10 +83,31 @@ class EventSyncCacheTest { @Test fun `readLastSyncOutcome returns null until a sync has been attempted`() = runTest { every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns -1 + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns -1 assertThat(eventSyncCache.readLastSyncOutcome()).isNull() } + @Test + fun `readLastSyncOutcome falls back to a success stored before attempts were recorded`() = runTest { + // Upgrading from a version that only wrote the success key: without the fallback the + // device would report never having synced until the next attempt. + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns -1 + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns 30 + + assertThat(eventSyncCache.readLastSyncOutcome()).isEqualTo(SyncOutcome(Timestamp(30), failure = null)) + } + + @Test + fun `readLastSyncOutcome prefers a recorded attempt over the legacy success`() = runTest { + every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 90 + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns 30 + every { sharedPrefsForLastSyncTime.getString(LAST_ATTEMPT_FAILURE_KEY, null) } returns SyncFailureReason.CLOUD_INTEGRATION.name + + assertThat(eventSyncCache.readLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(90), failure = SyncFailureReason.CLOUD_INTEGRATION)) + } + @Test fun `readLastSyncOutcome returns the time of an attempt that succeeded`() = runTest { every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns 30 @@ -171,6 +192,7 @@ class EventSyncCacheTest { fun `observeLastSyncOutcome publishes every write`() = runTest { every { sharedPrefsForLastSyncTime.edit() } returns mockk(relaxed = true) every { sharedPrefsForLastSyncTime.getLong(LAST_ATTEMPT_TIME_KEY, -1) } returns -1 + every { sharedPrefsForLastSyncTime.getLong(PEOPLE_SYNC_CACHE_LAST_SYNC_TIME_KEY, -1) } returns -1 eventSyncCache.observeLastSyncOutcome().test { assertThat(awaitItem()).isNull() diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorkerTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorkerTest.kt index 730e5cdde4..d48ea66b24 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorkerTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/down/workers/SimprintsEventDownSyncDownloaderWorkerTest.kt @@ -22,6 +22,7 @@ import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_BACKEND_M import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS +import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_UNEXPECTEDLY import com.simprints.infra.eventsync.sync.down.tasks.SimprintsEventDownSyncTask import com.simprints.infra.eventsync.sync.down.workers.BaseEventDownSyncDownloaderWorker.Companion.INPUT_DOWN_SYNC_OPS import com.simprints.infra.eventsync.sync.down.workers.BaseEventDownSyncDownloaderWorker.Companion.INPUT_EVENT_DOWN_SYNC_SCOPE_ID @@ -38,6 +39,7 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.mockk +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Rule @@ -249,7 +251,9 @@ internal class SimprintsEventDownSyncDownloaderWorkerTest { } @Test - fun worker_failForNetworkIssue_shouldSucceedWithoutFailureFlags() = runTest { + fun worker_failForUncategorisedIssue_shouldFlagUnexpectedFailure() = runTest { + // Nothing downloaded, so the run must not be indistinguishable from a clean one - the end + // reporter reads these flags to decide what outcome to store. coEvery { eventRepository.getEventScope(any()) } returns eventScope coEvery { downSyncTask.downSync(any(), any(), any(), any()) @@ -257,6 +261,28 @@ internal class SimprintsEventDownSyncDownloaderWorkerTest { val result = eventDownSyncDownloaderWorker.doWork() + assertThat(result).isEqualTo( + ListenableWorker.Result.success( + workDataOf( + OUTPUT_DOWN_SYNC to 0, + OUTPUT_DOWN_MAX_SYNC to 0, + OUTPUT_FAILED_UNEXPECTEDLY to true, + ), + ), + ) + } + + @Test + fun worker_cancelled_shouldSucceedWithoutFailureFlag() = runTest { + // Flagging cancellation would make the end reporter store an UNKNOWN failed attempt for a + // sync that was simply stopped, and throwing would break the chain before it ever runs. + coEvery { eventRepository.getEventScope(any()) } returns eventScope + coEvery { + downSyncTask.downSync(any(), any(), any(), any()) + } throws CancellationException("Cancelled") + + val result = eventDownSyncDownloaderWorker.doWork() + assertThat(result).isEqualTo( ListenableWorker.Result.success( workDataOf( diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt index f662e2b456..2eb0c7e028 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/master/EventEndSyncReporterWorkerTest.kt @@ -2,6 +2,7 @@ package com.simprints.infra.eventsync.sync.master import android.os.PowerManager import androidx.work.ListenableWorker +import androidx.work.WorkInfo import androidx.work.workDataOf import com.google.common.truth.Truth.* import com.simprints.core.domain.sync.SyncFailureReason @@ -10,6 +11,7 @@ import com.simprints.core.tools.time.Timestamp import com.simprints.infra.events.EventRepository import com.simprints.infra.eventsync.sync.common.EventSyncCache import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION +import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_UNEXPECTEDLY import com.simprints.infra.eventsync.sync.common.SyncWorkersInfoProvider import com.simprints.infra.eventsync.sync.master.EventEndSyncReporterWorker.Companion.EVENT_DOWN_SYNC_SCOPE_TO_CLOSE import com.simprints.infra.eventsync.sync.master.EventEndSyncReporterWorker.Companion.EVENT_UP_SYNC_SCOPE_TO_CLOSE @@ -112,11 +114,70 @@ internal class EventEndSyncReporterWorkerTest { @Test fun `doWork stores a succeeded outcome when no worker failed`() = runTest { + every { syncWorkersInfoProvider.getSyncWorkerInfos(any()) } returns flowOf( + listOf( + mockk(relaxed = true) { + every { outputData } returns workDataOf() + every { state } returns WorkInfo.State.SUCCEEDED + }, + ), + ) + createWorker("sync id", null, null).doWork() coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), null) } } + @Test + fun `doWork records an uncategorised worker error as a failure, not a success`() = runTest { + // Sync workers succeed with the reason in their output, so an error they could not + // categorise would otherwise be stored as a clean run. + every { syncWorkersInfoProvider.getSyncWorkerInfos(any()) } returns flowOf( + listOf( + mockk(relaxed = true) { + every { outputData } returns workDataOf(OUTPUT_FAILED_UNEXPECTEDLY to true) + every { state } returns WorkInfo.State.SUCCEEDED + }, + ), + ) + + createWorker("sync id", null, null).doWork() + + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), SyncFailureReason.UNKNOWN) } + } + + @Test + fun `doWork records a worker left in the failed state as a failure`() = runTest { + every { syncWorkersInfoProvider.getSyncWorkerInfos(any()) } returns flowOf( + listOf( + mockk(relaxed = true) { + every { outputData } returns workDataOf() + every { state } returns WorkInfo.State.FAILED + }, + ), + ) + + createWorker("sync id", null, null).doWork() + + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), SyncFailureReason.UNKNOWN) } + } + + @Test + fun `doWork keeps a categorised reason over the uncategorised fallback`() = runTest { + every { syncWorkersInfoProvider.getSyncWorkerInfos(any()) } returns flowOf( + listOf( + mockk(relaxed = true) { + every { outputData } returns workDataOf(OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION to true) + every { state } returns WorkInfo.State.FAILED + }, + ), + ) + + createWorker("sync id", null, null).doWork() + + coVerify(exactly = 1) { syncCache.storeLastSyncOutcome(Timestamp(1), SyncFailureReason.CLOUD_INTEGRATION) } + } + private fun createWorker( syncId: String?, downScopeId: String?, diff --git a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorkerTest.kt b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorkerTest.kt index 897d4ca671..461e698352 100644 --- a/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorkerTest.kt +++ b/infra/event-sync/src/test/java/com/simprints/infra/eventsync/sync/up/workers/EventUpSyncUploaderWorkerTest.kt @@ -23,6 +23,7 @@ import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_BACKEND_M import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_CLOUD_INTEGRATION import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_RELOGIN_REQUIRED import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_BECAUSE_TOO_MANY_REQUESTS +import com.simprints.infra.eventsync.sync.common.OUTPUT_FAILED_UNEXPECTEDLY import com.simprints.infra.eventsync.sync.up.EventUpSyncProgress import com.simprints.infra.eventsync.sync.up.tasks.EventUpSyncTask import com.simprints.infra.eventsync.sync.up.workers.EventUpSyncUploaderWorker.Companion.INPUT_EVENT_UP_SYNC_SCOPE_ID @@ -33,6 +34,7 @@ import com.simprints.infra.serialization.SimJson import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest @@ -103,7 +105,7 @@ internal class EventUpSyncUploaderWorkerTest { } @Test - fun worker_shouldSucceedWithoutFlagsIfNoEventScope() = runTest { + fun worker_shouldFlagUnexpectedFailureIfNoEventScope() = runTest { coEvery { eventRepository.getEventScope(any()) } returns null val result = init(projectScope).doWork() @@ -113,6 +115,7 @@ internal class EventUpSyncUploaderWorkerTest { workDataOf( EventUpSyncUploaderWorker.OUTPUT_UP_SYNC to 0, EventUpSyncUploaderWorker.OUTPUT_UP_MAX_SYNC to 12, + OUTPUT_FAILED_UNEXPECTEDLY to true, ), ), ) @@ -222,7 +225,9 @@ internal class EventUpSyncUploaderWorkerTest { } @Test - fun worker_shouldSucceedWithoutFlagsIfUnexpectedIssue() = runTest { + fun worker_shouldFlagUnexpectedFailureIfUnexpectedIssue() = runTest { + // Nothing uploaded, so the run must not be indistinguishable from a clean one - the end + // reporter reads these flags to decide what outcome to store. coEvery { eventRepository.getEventScope(any()) } returns eventScope coEvery { upSyncTask.upSync(any(), eventScope) @@ -230,6 +235,28 @@ internal class EventUpSyncUploaderWorkerTest { val result = init(projectScope).doWork() + assertThat(result).isEqualTo( + ListenableWorker.Result.success( + workDataOf( + EventUpSyncUploaderWorker.OUTPUT_UP_SYNC to 0, + EventUpSyncUploaderWorker.OUTPUT_UP_MAX_SYNC to 12, + OUTPUT_FAILED_UNEXPECTEDLY to true, + ), + ), + ) + } + + @Test + fun `worker should succeed without a failure flag when cancelled`() = runTest { + // Flagging cancellation would make the end reporter store an UNKNOWN failed attempt for a + // sync that was simply stopped, and throwing would break the chain before it ever runs. + coEvery { eventRepository.getEventScope(any()) } returns eventScope + coEvery { + upSyncTask.upSync(any(), eventScope) + } throws CancellationException("cancelled") + + val result = init(projectScope).doWork() + assertThat(result).isEqualTo( ListenableWorker.Result.success( workDataOf( @@ -282,7 +309,7 @@ internal class EventUpSyncUploaderWorkerTest { } @Test - fun `should succeed when input is null`() = runTest { + fun `should flag an unexpected failure when input is null`() = runTest { val eventUpSyncUploaderWorker = init(null) val result = eventUpSyncUploaderWorker.doWork() @@ -292,6 +319,7 @@ internal class EventUpSyncUploaderWorkerTest { workDataOf( EventUpSyncUploaderWorker.OUTPUT_UP_SYNC to 0, EventUpSyncUploaderWorker.OUTPUT_UP_MAX_SYNC to 12, + OUTPUT_FAILED_UNEXPECTEDLY to true, ), ), ) diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt b/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt index 474486fa81..8a7444f231 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/ImageSyncTimestampProvider.kt @@ -36,7 +36,7 @@ class ImageSyncTimestampProvider @Inject constructor( /** When the upload worker last ran and how it turned out; null when it never has. */ suspend fun getLastSyncOutcome(): SyncOutcome? = withContext(dispatcher) { - securePrefs + val lastAttempt = securePrefs .getLong(IMAGE_SYNC_ATTEMPT_TIME_MILLIS, 0) .takeIf { securePrefs.contains(IMAGE_SYNC_ATTEMPT_TIME_MILLIS) } ?.let { at -> @@ -45,17 +45,18 @@ class ImageSyncTimestampProvider @Inject constructor( failure = securePrefs.getString(IMAGE_SYNC_FAILURE_REASON, null)?.toSyncFailureReason(), ) } + lastAttempt ?: readCompletionTimestamp()?.let { SyncOutcome(timestamp = Timestamp(it), failure = null) } } /** * When the last upload actually *completed*, which is what says how stale this device's * samples are. The last attempt cannot answer it - it moves even when nothing uploaded. */ - suspend fun getLastSuccessfulSyncTimestamp(): Long? = withContext(dispatcher) { - securePrefs - .getLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, 0) - .takeIf { securePrefs.contains(IMAGE_SYNC_COMPLETION_TIME_MILLIS) } - } + suspend fun getLastSuccessfulSyncTimestamp(): Long? = withContext(dispatcher) { readCompletionTimestamp() } + + private fun readCompletionTimestamp(): Long? = securePrefs + .getLong(IMAGE_SYNC_COMPLETION_TIME_MILLIS, 0) + .takeIf { securePrefs.contains(IMAGE_SYNC_COMPLETION_TIME_MILLIS) } suspend fun getMillisSinceLastImageSync(): Long? = getLastSuccessfulSyncTimestamp()?.let { timeHelper.now().ms - it } diff --git a/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt b/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt index 6d140e214b..720a95f835 100644 --- a/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt +++ b/infra/sync/src/main/java/com/simprints/infra/sync/files/FileUpSyncWorker.kt @@ -13,6 +13,7 @@ import com.simprints.infra.sync.ImageSyncTimestampProvider import com.simprints.infra.sync.SyncConstants import dagger.assisted.Assisted import dagger.assisted.AssistedInject +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.withContext @@ -49,6 +50,10 @@ internal class FileUpSyncWorker @AssistedInject constructor( imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) retry() } + } catch (cancellation: CancellationException) { + // WorkManager cancelling the worker is not an upload attempt, and recording one here + // would both publish a failure that never happened and swallow the cancellation. + throw cancellation } catch (ex: Exception) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) retry(ex) diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt index e054b42450..1d9cbed355 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/ImageSyncTimestampProviderTest.kt @@ -63,6 +63,26 @@ class ImageSyncTimestampProviderTest { assertThat(imageSyncTimestampProvider.getLastSyncOutcome()).isNull() } + @Test + fun `getLastSyncOutcome falls back to a completion stored before attempts were recorded`() = runTest { + // Upgrading from a version that only wrote the completion key: without the fallback the + // device would report never having synced until the next attempt. + storedSuccessTimestamp(1234567890L) + + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(1234567890L), failure = null)) + } + + @Test + fun `getLastSyncOutcome prefers a recorded attempt over the legacy completion`() = runTest { + storedTimestamp(200L) + storedSuccessTimestamp(100L) + every { sharedPreferences.getString(FAILURE_KEY, null) } returns SyncFailureReason.CLOUD_INTEGRATION.name + + assertThat(imageSyncTimestampProvider.getLastSyncOutcome()) + .isEqualTo(SyncOutcome(Timestamp(200L), failure = SyncFailureReason.CLOUD_INTEGRATION)) + } + @Test fun `getLastSyncOutcome returns the time of an attempt that succeeded`() = runTest { storedTimestamp(1234567890L) diff --git a/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt b/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt index a29844a421..ec0a24254a 100644 --- a/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt +++ b/infra/sync/src/test/java/com/simprints/infra/sync/files/FileUpSyncWorkerTest.kt @@ -11,7 +11,9 @@ import com.simprints.infra.sync.SyncConstants import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Rule import org.junit.Test @@ -97,6 +99,19 @@ class FileUpSyncWorkerTest { coVerify(exactly = 1) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(SyncFailureReason.UNKNOWN) } } + @Test + fun `doWork lets cancellation through without recording an attempt`() = runBlocking { + // Cancelling the worker is not an upload attempt, and catching it here would also swallow + // cooperative cancellation. + coEvery { imageRepository.uploadStoredImagesAndDelete(any(), any()) } throws CancellationException("cancelled") + + assertThrows(CancellationException::class.java) { + runBlocking { fileUpSyncWorker.doWork() } + } + + coVerify(exactly = 0) { imageSyncTimestampProvider.saveImageSyncOutcomeNow(any()) } + } + @Test fun `doWork calls progress callback during image upload`() = runBlocking { // Given