Repository navigation
MS-1537 Bonus refactoring - device state tracker - #1845
luhmirin-s wants to merge 10 commits into
Conversation
… from source crashes
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve the two critical tracker issues and preserve coroutine cancellation handling.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Refactors device-state collection into a shared tracker used by sync UI, troubleshooting, logout handling, and session payload generation.
Changes:
- Replaces
SyncableCountswithDeviceStateDataTrackerandDeviceDataState. - Adds reactive event scope counting and migrates consumers.
- Updates troubleshooting UI and related tests.
| File | Reviewed change |
|---|---|
infra/sync/src/test/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCaseTest.kt |
Removes obsolete tests |
infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCaseTest.kt |
Updates package and tests |
infra/sync/src/test/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCaseTest.kt |
Updates package and tests |
infra/sync/src/test/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImplTest.kt |
Adds tracker coverage |
infra/sync/src/test/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCaseTest.kt |
Tests tracker-based logout decisions |
infra/sync/src/main/java/com/simprints/infra/sync/usecase/ObserveSyncableCountsUseCase.kt |
Removes obsolete use case |
infra/sync/src/main/java/com/simprints/infra/sync/SyncModule.kt |
Binds the new tracker |
infra/sync/src/main/java/com/simprints/infra/sync/SyncableCounts.kt |
Removes obsolete model |
infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveSamplesToUploadCountUseCase.kt |
Relocates sample counting |
infra/sync/src/main/java/com/simprints/infra/sync/devicestate/internal/ObserveEnrolmentRecordsCountUseCase.kt |
Relocates enrolment counting |
infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTrackerImpl.kt |
Implements state aggregation |
infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceStateDataTracker.kt |
Defines tracker API |
infra/sync/src/main/java/com/simprints/infra/sync/devicestate/DeviceDataState.kt |
Defines aggregated state |
infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/HandleProjectStateUseCase.kt |
Uses tracker for logout decisions |
infra/events/src/test/java/com/simprints/infra/events/event/local/SessionScopeRoomDaoTest.kt |
Tests scope count queries |
infra/events/src/test/java/com/simprints/infra/events/event/local/EventLocalDataSourceTest.kt |
Tests count mapping and recovery |
infra/events/src/main/java/com/simprints/infra/events/EventRepositoryImpl.kt |
Exposes scope counts |
infra/events/src/main/java/com/simprints/infra/events/EventRepository.kt |
Adds scope-count API |
infra/events/src/main/java/com/simprints/infra/events/event/local/SessionScopeRoomDao.kt |
Adds grouped count query |
infra/events/src/main/java/com/simprints/infra/events/event/local/models/DbScopeTypeCount.kt |
Adds Room projection |
infra/events/src/main/java/com/simprints/infra/events/event/local/EventLocalDataSource.kt |
Maps and zero-fills counts |
feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCaseTest.kt |
Tests state formatting |
feature/troubleshooting/src/test/java/com/simprints/feature/troubleshooting/overview/OverviewViewModelTest.kt |
Tests state collection |
feature/troubleshooting/src/main/res/layout/fragment_troubleshooting_overview.xml |
Adds device-data section |
feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/usecase/CollectDeviceDataStateUseCase.kt |
Formats device state |
feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewViewModel.kt |
Loads device state |
feature/troubleshooting/src/main/java/com/simprints/feature/troubleshooting/overview/OverviewFragment.kt |
Displays device state |
feature/troubleshooting/build.gradle.kts |
Adds sync dependency |
feature/login-check/src/test/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCaseTest.kt |
Updates payload tests |
feature/login-check/src/main/java/com/simprints/feature/logincheck/usecases/UpdateSessionScopePayloadUseCase.kt |
Uses tracker counts |
feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCaseTest.kt |
Updates sync-info tests |
feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCaseTest.kt |
Tests record counters |
feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCaseTest.kt |
Tests image counters |
feature/dashboard/src/test/java/com/simprints/feature/dashboard/settings/about/AboutViewModelTest.kt |
Tests logout checks |
feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/ObserveSyncInfoUseCase.kt |
Combines tracker and down-sync state |
feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionRecordsUseCase.kt |
Reads device-state counters |
feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/usecase/internal/GetSyncInfoSectionImagesUseCase.kt |
Reads sample counters |
feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/about/AboutViewModel.kt |
Uses tracker for logout checks |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Seven unresolved findings remain, including a critical event-sync failure-classification issue.
Review effort: Lite
Findings: 3
Open (8)
Preserve UNKNOWN failures for unclassified worker errors · New Make readOrNull accept suspending repository operations Count only uploadable events before entering logout sync Fall back to legacy sync timestamp for last successful outcome · New Classify uncategorized failed work as UNKNOWN · New Use legacy completion time when attempt timestamp is absent · New Rethrow cancellation instead of recording failed uploads · New Do not swallow CancellationException in database flow handling
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Review effort: Lite
Findings: 3
Open (3)
Resolved since last review (8)
Preserve UNKNOWN failures for unclassified worker errors Make readOrNull accept suspending repository operations Count only uploadable events before entering logout sync Rethrow cancellation instead of recording failed uploads Use legacy completion time when attempt timestamp is absent Classify uncategorized failed work as UNKNOWN Fall back to legacy sync timestamp for last successful outcome Do not swallow CancellationException in database flow handling
a5f9db1 to
a50be41
Compare
|





JIRA ticket
Will be released in: 2027.1.0
This change is a ground work to simplify/unify local device data state collection for the "hearthbeat" worker. Since the required data is very close to what we already collect for sync dashboard, it makes sense to move this part out of dashboard to a common infra layer.
Notable changes
DeviceStateDataTracker(infra/sync/devicestate) — single entry point for "how much data is on this device", replacingSyncableCounts/ObserveSyncableCountsUseCase(both deleted). Exposes a conflated flow for UI and one-shot methods for calls in worker.SyncOutcomeandSyncFailureReasonin infra/core to use across modules.EventSyncCacheandImageSyncTimestampProvidernow persist last attempt (with failure reason) alongside the existing last success key, each observable as a flow. The old completion key is preserved so upgrading devices keep their history; attempt keys start empty.OUTPUT_FAILED_UNEXPECTEDLYworker failure reason, plusList<WorkInfo>.firstFailureReason()mapping worker output toSyncFailureReason.AboutViewModel,UpdateSessionScopePayloadUseCase(session-scope record count), and the project-ending sign-out gate inHandleProjectStateUseCase(fixes a stale event count there) all go through the tracker. * Troubleshooting overview gained a device-data-state section for manual monitoring.ImageSyncTimestampProvideris now a@Singletonwith IO-dispatched, suspending reads and lazy prefs, where it was previously synchronous on the caller's thread.Testing guidance
Additional work checklist