Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(sync): allow concurrent sync start stop [WPB-15262] #3888

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

vitorhugods
Copy link
Member

@vitorhugods vitorhugods commented Feb 21, 2025

TaskWPB-15262 [Android] Sync engine refactoring


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Kalium is changing to allow concurrent sync start/stop. We need to address breaking changes on the app.

Solutions

Remove a lot of sensitive code and let Kalium figure it out.

Instead of having ConnectionPolicyManager to figure out if the connection policy can go up or down, we just make a SyncRequest when needed.

Most of the additions in this PR are for extra tests and inclusion of Fakes.

Considering that ConnectionPolicy doesn't exist anymore, this PR also changed ConnectionPolicyManager to SyncLifecycleManager, which is only responsible for requesting Sync when a push notification comes, or when the app is on the foreground.
It doesn't need to care about Persistent WebSocket anymore either, now that concurrent sync is possible.

The PersistentWebsocketService can request sync completely independently from SyncLifecycleManager.

Dependencies

Needs releases with:

Testing

Test Coverage

  • I have added automated test to this contribution

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

…ager

Replaced ConnectionPolicyManager with SyncLifecycleManager to streamline sync handling and improve lifecycle management. Updated associated services, tests, and application initialization to reflect the changes.
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 74.00000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 45.58%. Comparing base (f017b0f) to head (6bc66a1).

Files with missing lines Patch % Lines
...ire/android/services/PersistentWebSocketService.kt 0.00% 8 Missing ⚠️
...rc/main/kotlin/com/wire/android/WireApplication.kt 0.00% 3 Missing ⚠️
...ire/android/util/lifecycle/SyncLifecycleManager.kt 94.44% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3888      +/-   ##
===========================================
+ Coverage    45.51%   45.58%   +0.07%     
===========================================
  Files          491      491              
  Lines        17072    17066       -6     
  Branches      2853     2849       -4     
===========================================
+ Hits          7770     7780      +10     
+ Misses        8507     8491      -16     
  Partials       795      795              
Files with missing lines Coverage Δ
...re/android/notification/WireNotificationManager.kt 80.97% <100.00%> (-0.09%) ⬇️
...ire/android/util/lifecycle/SyncLifecycleManager.kt 94.44% <94.44%> (ø)
...rc/main/kotlin/com/wire/android/WireApplication.kt 0.00% <0.00%> (ø)
...ire/android/services/PersistentWebSocketService.kt 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f017b0f...6bc66a1. Read the comment docs.

Copy link
Contributor

Built wire-android-staging-compat-pr-3888.apk is available for download

Copy link
Contributor

Built wire-android-dev-debug-pr-3888.apk is available for download

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants