This repository was archived by the owner on Feb 24, 2025. It is now read-only.
Add support for per-model Sync Data Providers initialization #433
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bwaresiak
approved these changes
Jul 26, 2023
| @@ -230,6 +230,7 @@ public class DDGSync: DDGSyncing { | |||
| dependencies.scheduler.isEnabled = false | |||
| startSyncCancellable?.cancel() | |||
| syncQueueCancellable?.cancel() | |||
| try syncQueue?.dataProviders.forEach { try $0.deregisterFeature() } | |||
| syncQueue = nil | |||
| authState = .inactive | |||
| try dependencies.secureStore.removeAccount() | |||
There was a problem hiding this comment.
I wonder if we should handle any of these try calls somehow... pixel?
There was a problem hiding this comment.
Let me add a task for it. I'm sure it's not the only place we need to handle.
Sources/DDGSync/DataProvider.swift
Outdated
| open class DataProvider: DataProviding { | ||
|
|
||
| public let feature: Feature | ||
| public let metadataStore: SyncMetadataStore |
There was a problem hiding this comment.
Should this be public, or rather encapsulated? I don't think we want anyone to use it directly.
There was a problem hiding this comment.
Of course we don't want to expose it 👍 thanks!
afterxleep
pushed a commit
that referenced
this pull request
Aug 2, 2023
Task/Issue URL: https://app.asana.com/0/0/1205093266805582/f Rework Sync initialization mechanism by moving sync data models initialization from DDGSync (where it was tied to account signup/login flow) to data models themselves. Sync Metadata is leveraged to store sync setup state per data model. Two values are possible: needs remote data fetch (a.k.a initial sync) or ready to sync. Decoupling account flow from data models initialization allows to initialize data models independently of user turning on sync, which is a requirement to support initial sync for new syncable models added with client app updates. Also DataProvider class has been added – it's a dedicated sync data providers superclass that also encapsulates common logic, making individual data providers simpler.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/0/1205093266805582/f
iOS PR: duckduckgo/iOS#1863
macOS PR: duckduckgo/macos-browser#1387
What kind of version bump will this require?: Major
Description:
Rework Sync initialization mechanism by moving sync data models initialization from
DDGSync (where it was tied to account signup/login flow) to data models themselves.
Sync Metadata is leveraged to store sync setup state per data model. Two values are
possible: needs remote data fetch (a.k.a initial sync) or ready to sync.
Decoupling account flow from data models initialization allows to initialize data models
independently of user turning on sync, which is a requirement to support initial sync
for new syncable models added with client app updates.
Also DataProvider class has been added – it's a dedicated sync data providers superclass
that also encapsulates common logic, making individual data providers simpler.
Steps to test this PR:
iOS:
was triggered for an active account).
macOS:
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template