[MS-1299] Sync architecture revamp, part 2

[alex-vt]

JIRA ticket
Will be released in: 2026.2.0

Boundaries for the changes - what's intentionally out of scope:

• WorkManager and workers. This is already the optimal foundation for sync - both foreground and background.
• SyncInfoViewModel. Already observes sync state.
• UI/UX. Nothing visually new.

Previously done in phase 1:

• ObserveSyncInfoUseCase made fully reactive - without suspension points.
• SyncUseCase, a unified (events + images) sync status reactive observation use case introduced. It is a StateFlow, with up-to-date sync status available as .value syncronously from anywhere.
• CountSyncableUseCase, a unified (events + images) counters reactive observation use case introduced.

Notable changes

• observeSyncState(): StateFlow<SyncStatus> introduced in SyncOrchestrator
• executeOneTime(command: OneTime): Job introduced in SyncOrchestrator
• executeSchedulingCommand(command: ScheduleCommand): Job introduced in SyncOrchestrator

Testing guidance

• Sync counters and buttons on dashboard, sync info and logout screens should work correctly.
• Some usecase calls were changes in other places - full regression testing recommended when the work is completed.

Additional work checklist

• Effect on other features and security has been considered
• Design document marked as "In development" (if applicable)
• External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
• Test cases in Testiny are up to date (or ticket created)
• Other teams notified about the changes (if applicable)

[Copilot]

Pull request overview

This PR implements phase 2 of a comprehensive sync architecture revamp, introducing a structured command-based API for sync operations. The changes replace the imperative sync orchestration with a reactive, command-driven approach using expressive DSL-style builders.

Changes:

• Introduced SyncCommands DSL builders with autocomplete-friendly structure for scheduling and one-time sync operations
• Created SyncResponse with await() extension for suspendable command execution
• Migrated most SyncOrchestrator and EventSyncManager functionality into ExecuteSyncCommandUseCase and updated SyncUseCase
• Updated all callsites throughout features (login-check, dashboard, validate-subject-pool) and Application.kt to use new sync commands

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
infra/sync/src/main/java/com/simprints/infra/sync/SyncCommands.kt	New DSL-based command builder infrastructure with expressive API
infra/sync/src/main/java/com/simprints/infra/sync/SyncResponse.kt	Response wrapper with suspend-aware await() for command completion
infra/sync/src/main/java/com/simprints/infra/sync/usecase/internal/ExecuteSyncCommandUseCase.kt	Core command execution logic extracted from SyncOrchestrator
infra/sync/src/main/java/com/simprints/infra/sync/usecase/SyncUseCase.kt	Updated to accept and execute sync commands
infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestratorImpl.kt	Slimmed down - removed methods migrated to ExecuteSyncCommandUseCase
infra/sync/src/main/java/com/simprints/infra/sync/SyncOrchestrator.kt	Interface reduced with methods moved to use cases
infra/sync/src/main/java/com/simprints/infra/sync/config/usecase/*.kt	Config use cases updated to use new sync commands with stopAndStartAround
id/src/main/java/com/simprints/id/Application.kt	Application startup now uses sync commands for background work scheduling
id/src/main/java/com/simprints/id/services/sync/events/down/EventDownSyncResetService.kt	Service updated to use sync commands with await()
feature/login-check/src/main/java/com/simprints/feature/logincheck/*.kt	Login flow uses sync commands with proper await() semantics
feature/dashboard/src/main/java/com/simprints/feature/dashboard/settings/syncinfo/*.kt	Dashboard sync UI migrated to sync commands
feature/validate-subject-pool/src/main/java/com/simprints/feature/validatepool/usecase/*.kt	Validation flow updated with sync commands
All test files	Comprehensive test coverage for new command infrastructure and updated existing tests

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[luhmirin-s]

I have couple of concerns regarding the complexity of this solution, as it does not seem to simplify sync usage compared to the original sync orchestration.

[luhmirin-s]

This alone would disqualify this class from being called a "use case" in my opinion, amount of other stuff this class does is also a factor.

This is a basically an orchestrator/manager(/state machine?) with a single entry point and there is nothing wrong with calling it that.

[alex-vt]

These are internals. There's 1 public (also only within the module) invoke function there. It's a universal/flexible usecase, where the sync command is explicitly defined. Orchestrator was architecturally a standing out class (neither usecase nor repository), and had too loose coupling of different things it did, basically a so called "god class".

[luhmirin-s]

Sorry, I was not very clear with my initial comment. My point is that once a class starts storing a state or running a app-scoped task, it loses the "use case" status.
The whole point of the "use case" is to have self-contained (if possible, pure function) and re-usable piece of business logic that can be (although not required) called from multiple places and that can be tested in isolation. Once it start handling global state it is not "reusable" anymore. The amount of entry points/functions is irrelevant to the definition.

The old sync orchestrator wasn't really "god class", since it had a pretty narrow responsibility - "abstracting worker scheduling logic into simple method calls".

[alex-vt]

This it what SyncOrchestrator usage used to be like:

flowchart LR
  subgraph SyncOrchestrator["SyncOrchestrator"]

    SO_scheduleBackgroundWork["scheduleBackgroundWork"]
    SO_cancelBackgroundWork["cancelBackgroundWork"]

    SO_rescheduleEventSync["rescheduleEventSync"]
    SO_cancelEventSync["cancelEventSync"]
    SO_startEventSync["startEventSync"]
    SO_stopEventSync["stopEventSync"]

    SO_startImageSync["startImageSync"]
    SO_stopImageSync["stopImageSync"]
    SO_rescheduleImageUpSync["rescheduleImageUpSync"]

    SO_startConfigSync["startConfigSync"]
    SO_refreshConfiguration["refreshConfiguration"]

    SO_uploadEnrolmentRecords["uploadEnrolmentRecords"]

    SO_deleteEventSyncInfo["deleteEventSyncInfo"]
    SO_cleanupWorkers["cleanupWorkers"]

  end

  SO_scheduleBackgroundWork --> SO_rescheduleEventSync
  SO_cancelBackgroundWork --> SO_stopEventSync
  SO_refreshConfiguration --> SO_startConfigSync
  SO_cancelEventSync --> SO_stopEventSync
  SO_startImageSync --> SO_stopImageSync

  subgraph SyncOrchestratorImpl["SyncOrchestratorImpl : SyncOrchestrator"]
    direction TB
    SOI_init["init"]
  end

  subgraph RunBlockingEventSyncUseCase["RunBlockingEventSyncUseCase"]
    direction TB
    RBESU_invoke["invoke"]
  end

  subgraph LoginCheckViewModel["LoginCheckViewModel"]
    direction TB
    LCVM_startSignInAttempt["startSignInAttempt"]
  end

  subgraph StartBackgroundSyncUseCase["StartBackgroundSyncUseCase"]
    direction TB
    SBSU_invoke["invoke"]
  end

  subgraph DashboardLogoutUseCase["LogoutUseCase (feature.dashboard)"]
    direction TB
    DLU_invoke["invoke"]
  end

  subgraph DebugFragment["DebugFragment"]
    direction TB
    DF_onViewCreated["onViewCreated"]
  end

  subgraph SyncInfoViewModel["SyncInfoViewModel"]
    direction TB
    SIVM_forceEventSync["forceEventSync"]
    SIVM_toggleImageSync["toggleImageSync"]
    SIVM_syncImagesAfterEventsWhenRequired["syncImagesAfterEventsWhenRequired"]
  end

  subgraph ModuleSelectionViewModel["ModuleSelectionViewModel"]
    direction TB
    MSVM_saveModules["saveModules"]
  end

  subgraph SettingsViewModel["SettingsViewModel"]
    direction TB
    SEVM_scheduleConfigUpdate["scheduleConfigUpdate"]
  end

  subgraph Application["Application : Configuration.Provider"]
    direction TB
    APP_initApplication["initApplication"]
  end

  subgraph EventDownSyncResetService["EventDownSyncResetService"]
    direction TB
    EDSRS_onStartCommand["onStartCommand"]
  end

  subgraph SyncConfigScheduleReceiver["SyncConfigScheduleReceiver"]
    direction TB
    SCSR_onReceive["onReceive"]
  end

  subgraph DeviceConfigDownSyncWorker["DeviceConfigDownSyncWorker"]
    direction TB
    DCDSW_doWork["doWork"]
  end

  subgraph RescheduleWorkersIfConfigChangedUseCase["RescheduleWorkersIfConfigChangedUseCase"]
    direction TB
    RWICC_invoke["invoke"]
  end

  subgraph SyncConfigLogoutUseCase["LogoutUseCase (infra.sync.config)"]
    direction TB
    SCLU_invoke["invoke"]
  end

  subgraph ResetLocalRecordsIfConfigChangedUseCase["ResetLocalRecordsIfConfigChangedUseCase"]
    direction TB
    RLRC_invoke["invoke"]
  end

  SOI_init --> SO_rescheduleImageUpSync

  RBESU_invoke --> SO_startEventSync
  LCVM_startSignInAttempt --> SO_cancelBackgroundWork
  SBSU_invoke --> SO_scheduleBackgroundWork

  DLU_invoke --> SO_cancelBackgroundWork
  DLU_invoke --> SO_deleteEventSyncInfo

  DF_onViewCreated --> SO_startEventSync
  DF_onViewCreated --> SO_stopEventSync
  DF_onViewCreated --> SO_rescheduleEventSync
  DF_onViewCreated --> SO_cancelEventSync

  SIVM_forceEventSync --> SO_stopEventSync
  SIVM_forceEventSync --> SO_startEventSync
  SIVM_toggleImageSync --> SO_stopImageSync
  SIVM_toggleImageSync --> SO_startImageSync
  SIVM_syncImagesAfterEventsWhenRequired --> SO_startImageSync

  MSVM_saveModules --> SO_stopEventSync
  MSVM_saveModules --> SO_startEventSync

  SEVM_scheduleConfigUpdate --> SO_refreshConfiguration

  APP_initApplication --> SO_cleanupWorkers
  APP_initApplication --> SO_scheduleBackgroundWork

  EDSRS_onStartCommand --> SO_startEventSync
  SCSR_onReceive --> SO_startConfigSync
  DCDSW_doWork --> SO_uploadEnrolmentRecords
  RWICC_invoke --> SO_rescheduleImageUpSync

  SCLU_invoke --> SO_cancelBackgroundWork
  SCLU_invoke --> SO_deleteEventSyncInfo

  RLRC_invoke --> SO_cancelEventSync
  RLRC_invoke --> SO_rescheduleEventSync

Loading

Now a lot of these point to the usecase. It's a definite relief on the developer's mindspace around sync. Should we keep this small compromise of the init block, at least while it's only one of its kind? Or I'll extract it in a separate usecase to be invoked when the app starts?

[luhmirin-s]

Not sure I understand the point you are making?
Before there were multiple methods for specific actions, now all of those arrows will point to a single method and it will have lots of hidden details in the parameters. The complexity is still there, tho. And both are dealing with the same responsibility.

I don't see the need to "compromise of the init block" since I am not saying "not use init block or global state" or do any other significant internal changes.

What I am saying - lets drop the notion that this is a "use case" (since it is not) and make a clear external API fro the class; that would remove the arbitrary limitation on having the "single method" and allow you to simplify the DX. For example, based on the diagram above and the SyncCommand class, I can see an option where we have separate observeState(), executeOneTime(OneTime.Images.start()) and executeSchedulingCommand(ScheduleCommand.Images.reschedule()).

[alex-vt]

observeSyncState(), executeOneTime(OneTime.Images.start()) (for OneTime commands) and executeSchedulingCommand(ScheduleCommand.Images.reschedule()) (for ScheduleCommand commands) are now in the SyncOrchestrator.

[luhmirin-s]

I am starting to think that mixing the "observeOnly" with actionable commands needlessly complicates thing.
I would suggest simply splitting this into observe(): Flow and execute(): Job instead of trying to squeeze this into "use case" box.

[alex-vt]

The single entry point for sync is the point. It's great for consistent developer experience without looking up which of the many usecases or functions to use. For anything about sync, the developer declares the usecase and just types sync(SyncCommands., then follows the autocomplete - a breeze!

[luhmirin-s]

It is not obvious that you need to do a "command" to only observe the current state. And autocomplete is a non-argument in this case because typing sync.o + TAB is even "faster".

The main point is not to conflate things of completely different types for the sake of preserving "single entry point", there is no such requirement and it makes the implementation and DX more complex.

[alex-vt]

Separating an executable command and ObserveOnly would just make 2 usecases instead of currently 1. A single point of entry is an elegant solution here, the types aren't different enough to lost the advantage of that. SyncUseCase isn't even big despite being all-in-one, the actual impl is just a few lines.

[luhmirin-s]

Why does it need to be a use case?

[alex-vt]

It's now back in the SyncOrchestrator.

[luhmirin-s]

This is a confusing declaration since "schedule" has been always used a verb before and it clear what "reschedule" means.

On the other hand "Schedule.<>.start" is ambiguous - it can be either "start scheduling sync" or "start executing scheduled sync".

[alex-vt]

It's kinda a verb and a noun 2-in-1. "Start scheduling sync" or "start executing scheduled sync" is an inner abstraction - the calling site intends to begin/initiate/launch the kind of sync that is scheduled/periodic/background, and does so, with whatever internal spec it is configured at the worker-level.

[BurningAXE]

Am I missing anything or are 'Reschedule' and 'Schedule....start' with very different meanings?

[luhmirin-s]

It's kinda a verb and a noun 2-in-1. "Start scheduling sync" or "start executing scheduled sync"

But the code says "Schedule Image...start" and "start executing image schedule" in this case.

My point is that is more ambiguous than it should be, not that you cannot interpret it the way you do, tho.

[alex-vt]

Updated with OneTimeNow and ScheduleOf - this should resolve the ambiguity.

[luhmirin-s]

As I have mentioned in one previous comments, this structure might lead ambiguity on the call site because "Schedule.Everything.start()" can be interpreted in different ways.

I would suggest dropping the "Schedule/OneTime" part of the DSL and instead use more specific verb for the final commands - "schedule/reschedule/unschedule/cancel" for periodic work management and "start/restart/stop" for one-shot work,

[alex-vt]

Keeping start/stop/stopAndStart semantics uniform and concise in the number of choices is intentional, it's one of the points. Schedule can be named Periodic though, is it ok what do you think?

[luhmirin-s]

Just using Periodic reduces the confusion, but does not remove it, IMO.

SyncCommand.Periodic.Image.start() - does this mean to "start the image sync right now and also schedule it periodically" or only "schedule image sync execution for later"?

[alex-vt]

Updated with OneTimeNow and ScheduleOf - this should resolve the ambiguity. Now it's SyncCommand.ScheduleOf.Images.start() - clear that it's the schedule that's being controlled, not the individual syncs in it.

[luhmirin-s]

IMO, SyncCommand.ScheduleOf.Image.start() has the same issue.

My initial comment still stands - it would be much cleaner with separate set of actions for one-time work and scheduling.

[BurningAXE]

What started out as an attempt to simplify sync VM implementation has spiralled into a general sync refactoring. Given that we are discussing architecture and not implementation in the PR - I think it makes sense to "go back to the drawing board" and actually pass this through a design doc. There we can discuss the pros and cons of different approaches. Once we agree on the architecture (which shouldn't necessarily change (much)), we can move to the implementation part.

[alex-vt]

What started out as an attempt to simplify sync VM implementation has spiralled into a general sync refactoring. Given that we are discussing architecture and not implementation in the PR - I think it makes sense to "go back to the drawing board" and actually pass this through a design doc. There we can discuss the pros and cons of different approaches. Once we agree on the architecture (which shouldn't necessarily change (much)), we can move to the implementation part.

Just to keep the scope already narrowed before re-planning: the part about the usage of workers isn't up to a change, at least not recommended - that was already researched before Phase 1. Workers are already the reliable solution and shouldn't be replaced, even if the app is in the foreground - at any moment it isn't anymore, and other solutions to keep the sync going would be no better. Also, the VM is already using reactive pattern as well. So, the part about reactive sync architecture focuses on the SyncOrchestrator.

Also, the Phase 1 had already outlined the work that's being done here. It was the exploration part, marked as "spike", and is now in main. Here we have the planned implementation of the SyncUseCase stub from Phase 1.