Skip to content

fix: Run sync request synchronously [DHIS2-21606][2.42]#24291

Draft
muilpp wants to merge 4 commits into
2.42from
DHIS2-21606_2.42
Draft

fix: Run sync request synchronously [DHIS2-21606][2.42]#24291
muilpp wants to merge 4 commits into
2.42from
DHIS2-21606_2.42

Conversation

@muilpp

@muilpp muilpp commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

When syncing single events to a remote instance, two issues could cause unnecessary failures:

  • Already deleted events: If the target had already processed an event deletion, importing the same deletion again raised error E1082, causing the sync to fail. This error is now downgraded to a warning for DELETE imports. Additionally, PersistablesFilter skips persisting entities that are already marked as deleted in the preheat, avoiding redundant write attempts.

  • Run the request synchronously: The sync request was executed asynchronously, but its status was never polled. Instead, it was always assumed to have succeeded. As a result, single events could be marked as synchronized even if the import had failed, leaving the source and target instances out of sync without detection. The request is now executed synchronously (matching the platform behavior), and single events are marked as synchronized only if the remote import completes successfully.

The overload of SyncUtils.runSyncRequest is added to accept a caller-supplied ResponseExtractor<T>. The existing overload only handled ImportSummary/ImportSummaries (the old DXF2 format), but the tracker endpoint returns ImportReport, making it incompatible. The new overload preserves the retry-on-5xx behavior while letting callers deserialize the response into any type. It will be reused in a follow up PR that applies the same fix to TrackerDataSynchronizationService.

@sonarqubecloud

Copy link
Copy Markdown

@enricocolasante enricocolasante left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused by the overlapping of this PR and the other one already opened.
Only single events is going to be sync?
The delete an already deleted single events wasn't an issue before right? Because an already deleted event would have been already synchronized and it should be present in the paylaod at all. The already deleted issue is only an issue for entities without the synchronized field (enrollments and from now on tracker events)

@muilpp

muilpp commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

I am confused by the overlapping of this PR and the other one already opened. Only single events is going to be sync? The delete an already deleted single events wasn't an issue before right? Because an already deleted event would have been already synchronized and it should be present in the paylaod at all. The already deleted issue is only an issue for entities without the synchronized field (enrollments and from now on tracker events)

I mentioned in the description that I'll also make the tracker data sync job synchronous.

and it should be present in the payload at all.

I'm not sure what you mean by this.

I applied the fix here as well for consistency with the other import validations. Also, if the remote instance soft deletes an event and it is then synchronized to the local instance, the import fails with the same error. If the local instance already has that event soft deleted, I think it makes more sense to report it as a warning instead of an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants