Skip to content

Commit 2bf59a4

Browse files
palpatimpalpatim
authored
Fix delete merging (#243)
- Convert ReconcileAndLocalSaveOperation to AsynchronousOperation - Make Reconciler respect only local metadata - Unit test refactoring & additional tests - Added MockResponder convenience type for easier declaration of Mock type responder callbacks - Remove ModelGraphs in favor of existing 'sortByDependencyOrder'; added some unit tests to 'sortByDependencyOrder' - Feedback from previous PR
1 parent f4b0423 commit 2bf59a4

File tree

55 files changed

+1503
-742
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1503
-742
lines changed

‎Amplify.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@
237237
FA3152A0233D645B00DE78E7 /* StorageRemoveRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA31529F233D645A00DE78E7 /* StorageRemoveRequest.swift */; };
238238
FA47B8362350C2D60031A0E3 /* AutoUnsubscribeOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA47B8352350C2D60031A0E3 /* AutoUnsubscribeOperationTests.swift */; };
239239
FA47B8382350C58B0031A0E3 /* AutoUnsubscribeHubListenToOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA47B8372350C58B0031A0E3 /* AutoUnsubscribeHubListenToOperationTests.swift */; };
240+
FA4A955F239ADEBD008E876E /* MockResponder.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA4A955E239ADEBD008E876E /* MockResponder.swift */; };
240241
FA4B38AB238482B100E20DAB /* DefaultLoggingPluginTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA4B38AA238482B100E20DAB /* DefaultLoggingPluginTests.swift */; };
241242
FA4E730A232828EA003B8EEB /* Amplify+Reset.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA4E7309232828EA003B8EEB /* Amplify+Reset.swift */; };
242243
FA4E730C23282917003B8EEB /* Amplify+Resolve.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA4E730B23282917003B8EEB /* Amplify+Resolve.swift */; };
@@ -872,6 +873,7 @@
872873
FA317107232AE8DF009BC140 /* SerialDispatcherPerformanceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SerialDispatcherPerformanceTests.swift; sourceTree = "<group>"; };
873874
FA47B8352350C2D60031A0E3 /* AutoUnsubscribeOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutoUnsubscribeOperationTests.swift; sourceTree = "<group>"; };
874875
FA47B8372350C58B0031A0E3 /* AutoUnsubscribeHubListenToOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutoUnsubscribeHubListenToOperationTests.swift; sourceTree = "<group>"; };
876+
FA4A955E239ADEBD008E876E /* MockResponder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockResponder.swift; sourceTree = "<group>"; };
875877
FA4B38AA238482B100E20DAB /* DefaultLoggingPluginTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultLoggingPluginTests.swift; sourceTree = "<group>"; };
876878
FA4E7309232828EA003B8EEB /* Amplify+Reset.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Amplify+Reset.swift"; sourceTree = "<group>"; };
877879
FA4E730B23282917003B8EEB /* Amplify+Resolve.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Amplify+Resolve.swift"; sourceTree = "<group>"; };
@@ -2272,6 +2274,7 @@
22722274
FAC23589227A45D500424678 /* MockHubCategoryPlugin.swift */,
22732275
FAC0A2BB22B4603800B50912 /* MockLoggingCategoryPlugin.swift */,
22742276
B4BD6B3623708C6700A1F0A7 /* MockPredictionsCategoryPlugin.swift */,
2277+
FA4A955E239ADEBD008E876E /* MockResponder.swift */,
22752278
FAC23553227A056600424678 /* MockStorageCategoryPlugin.swift */,
22762279
);
22772280
path = Mocks;
@@ -3675,6 +3678,7 @@
36753678
FACF52042329633500646E10 /* TestExtensions.swift in Sources */,
36763679
B9FAA11823879A57009414B4 /* Author+Schema.swift in Sources */,
36773680
B9521836237E21BA00F53237 /* Post+Schema.swift in Sources */,
3681+
FA4A955F239ADEBD008E876E /* MockResponder.swift in Sources */,
36783682
B9FAA11223878C96009414B4 /* UserAccount+Schema.swift in Sources */,
36793683
FACA36152327FC39000E74F6 /* MessageReporter.swift in Sources */,
36803684
FAF512AE23986791001ADF4E /* AmplifyModels.swift in Sources */,

‎Amplify/Core/Support/AsychronousOperation.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ open class AsynchronousOperation: Operation {
2222
}
2323

2424
/// Synchronizes access to `state`.
25-
private let stateQueue = DispatchQueue(label: "com.amazonaws.amplify.AsyncOperation.state",
25+
private let stateQueue = DispatchQueue(label: "com.amazonaws.amplify.AsynchronousOperation.state",
2626
target: DispatchQueue.global())
2727

2828
/// Private backing stored property for `state`.

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/AWSDataStorePlugin.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ final public class AWSDataStorePlugin: DataStoreCategoryPlugin {
2626
public init(modelRegistration: AmplifyModelRegistration) {
2727
self.modelRegistration = modelRegistration
2828
self.isSyncEnabled = false
29-
if #available(iOS 13, *) {
29+
if #available(iOS 13.0, *) {
3030
self.dataStorePublisher = DataStorePublisher()
3131
} else {
3232
self.dataStorePublisher = nil
@@ -81,10 +81,10 @@ final public class AWSDataStorePlugin: DataStoreCategoryPlugin {
8181

8282
public func reset(onComplete: @escaping (() -> Void)) {
8383
let group = DispatchGroup()
84-
if let awsStorageEngine = storageEngine as? StorageEngine {
84+
if let resettable = storageEngine as? Resettable {
8585
group.enter()
8686
DispatchQueue.global().async {
87-
awsStorageEngine.reset {
87+
resettable.reset {
8888
group.leave()
8989
}
9090
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/Model+SQLite.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ extension Array where Element == Model.Type {
144144
sortMap[key] = modelType
145145
}
146146
}
147-
forEach(walkAssociatedModels(of:))
147+
148+
let sortedStartList = sorted { $0.modelName < $1.modelName }
149+
sortedStartList.forEach(walkAssociatedModels(of:))
148150
return sortedKeys.map { sortMap[$0]! }
149151
}
150152

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/StorageEngineAdapter+SQLite.swift‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,7 @@ final class SQLiteStorageEngineAdapter: StorageEngineAdapter {
181181
let rows = try connection.prepare(sql).bind(ids)
182182

183183
let syncMetadataList = try rows.convert(to: MutationSyncMetadata.self)
184-
let mutationSyncList = try syncMetadataList.map {
185-
(syncMetadata: MutationSyncMetadata) -> MutationSync<AnyModel> in
184+
let mutationSyncList = try syncMetadataList.map { syncMetadata -> MutationSync<AnyModel> in
186185
guard let model = modelById[syncMetadata.id] else {
187186
throw DataStoreError.invalidOperation(causedBy: nil)
188187
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/StorageEngine.swift‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ final class StorageEngine: StorageEngineBehavior {
4141
let storageAdapter = try SQLiteStorageEngineAdapter(databaseName: databaseName ?? "app")
4242

4343
try storageAdapter.setUp(models: StorageEngine.systemModels)
44-
if #available(iOS 13, *) {
44+
if #available(iOS 13.0, *) {
4545
let syncEngine = isSyncEnabled ? try? RemoteSyncEngine(storageAdapter: storageAdapter) : nil
4646
self.init(storageAdapter: storageAdapter, syncEngine: syncEngine)
4747
} else {
@@ -78,7 +78,7 @@ final class StorageEngine: StorageEngineBehavior {
7878
return
7979
}
8080

81-
if #available(iOS 13, *) {
81+
if #available(iOS 13.0, *) {
8282
self.log.verbose("\(#function) syncing mutation for \(savedModel)")
8383
self.syncMutation(of: savedModel,
8484
mutationType: mutationType,
@@ -107,7 +107,7 @@ final class StorageEngine: StorageEngineBehavior {
107107
return
108108
}
109109

110-
if #available(iOS 13, *) {
110+
if #available(iOS 13.0, *) {
111111
// TODO: Add a delete-specific APICategory API that allows delete mutations with just sync metadata
112112
// like type, ID, and version
113113
self.syncDeletion(of: modelType, withId: id, syncEngine: syncEngine, completion: completion)
@@ -132,12 +132,12 @@ final class StorageEngine: StorageEngineBehavior {
132132
func reset(onComplete: () -> Void) {
133133
// TOOD: Perform cleanup on StorageAdapter, including releasing its `Connection` if needed
134134
let group = DispatchGroup()
135-
if #available(iOS 13, *) {
135+
if #available(iOS 13.0, *) {
136136

137-
if let remoteSyncEngine = syncEngine as? RemoteSyncEngine {
137+
if let resettable = syncEngine as? Resettable {
138138
group.enter()
139139
DispatchQueue.global().async {
140-
remoteSyncEngine.reset {
140+
resettable.reset {
141141
group.leave()
142142
}
143143
}
@@ -147,7 +147,7 @@ final class StorageEngine: StorageEngineBehavior {
147147
onComplete()
148148
}
149149

150-
@available(iOS 13, *)
150+
@available(iOS 13.0, *)
151151
private func syncDeletion<M: Model>(of modelType: M.Type,
152152
withId id: Model.Identifier,
153153
syncEngine: RemoteSyncEngineBehavior,
@@ -174,7 +174,7 @@ final class StorageEngine: StorageEngineBehavior {
174174
completion: mutationEventCallback)
175175
}
176176

177-
@available(iOS 13, *)
177+
@available(iOS 13.0, *)
178178
private func syncMutation<M: Model>(of savedModel: M,
179179
mutationType: MutationEvent.MutationType,
180180
syncEngine: RemoteSyncEngineBehavior,
@@ -203,7 +203,7 @@ final class StorageEngine: StorageEngineBehavior {
203203
completion: mutationEventCallback)
204204
}
205205

206-
@available(iOS 13, *)
206+
@available(iOS 13.0, *)
207207
private func submitToSyncEngine(mutationEvent: MutationEvent,
208208
syncEngine: RemoteSyncEngineBehavior,
209209
completion: @escaping DataStoreCallback<MutationEvent>) {

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/InitialSync/InitialSyncOperation.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ final class InitialSyncOperation: AsynchronousOperation {
117117
}
118118

119119
guard let reconciliationQueue = reconciliationQueue else {
120-
finish(result: .failure(DataStoreError.nilReconciliationQueues()))
120+
finish(result: .failure(DataStoreError.nilReconciliationQueue()))
121121
return
122122
}
123123

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/InitialSync/InitialSyncOrchestrator.swift‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ protocol InitialSyncOrchestrator {
1212
func sync(completion: @escaping (Result<Void, DataStoreError>) -> Void)
1313
}
1414

15+
// For testing
16+
@available(iOS 13.0, *)
17+
typealias InitialSyncOrchestratorFactory =
18+
(APICategoryGraphQLBehavior?, IncomingEventReconciliationQueue?, StorageEngineAdapter?) -> InitialSyncOrchestrator
19+
1520
@available(iOS 13.0, *)
1621
final class AWSInitialSyncOrchestrator: InitialSyncOrchestrator {
1722
typealias SyncOperationResult = Result<Void, DataStoreError>
@@ -50,11 +55,8 @@ final class AWSInitialSyncOrchestrator: InitialSyncOrchestrator {
5055
self.completion = completion
5156

5257
log.info("Beginning initial sync")
53-
let roots = getModelRoots()
5458

55-
for root in roots {
56-
enqueueSyncOperation(for: root)
57-
}
59+
enqueueSyncableModels()
5860

5961
// This operation is intentionally not cancel-aware; we always want resolveCompletion to execute
6062
// as the last item
@@ -65,37 +67,33 @@ final class AWSInitialSyncOrchestrator: InitialSyncOrchestrator {
6567
syncOperationQueue.isSuspended = false
6668
}
6769

68-
/// Creates a graph of registered models and returns the roots.
69-
private func getModelRoots() -> [DirectedGraphNode<Model.Type>] {
70-
let syncableModels = ModelRegistry.models.filter { !$0.schema.isSystem }
71-
let modelGraphs = ModelGraphs(models: syncableModels)
72-
let roots = modelGraphs.roots
73-
return roots
70+
private func enqueueSyncableModels() {
71+
let syncableModels = ModelRegistry.models.filter { $0.schema.isSyncable }
72+
let sortedModels = syncableModels.sortByDependencyOrder()
73+
for model in sortedModels {
74+
enqueueSyncOperation(for: model)
75+
}
7476
}
7577

76-
/// Recursively enqueues sync operations for models and downstream dependencies
77-
private func enqueueSyncOperation(for root: DirectedGraphNode<Model.Type>) {
78+
/// Enqueues sync operations for models and downstream dependencies
79+
private func enqueueSyncOperation(for modelType: Model.Type) {
7880
let syncOperationCompletion: SyncOperationResultHandler = { result in
7981
if case .failure(let dataStoreError) = result {
8082
let syncError = DataStoreError.sync(
81-
"An error occurred syncing \(root.value.modelName)",
83+
"An error occurred syncing \(modelType.modelName)",
8284
"",
8385
dataStoreError)
8486
self.syncErrors.append(syncError)
8587
}
8688
}
8789

88-
let initialSyncForModel = InitialSyncOperation(modelType: root.value,
90+
let initialSyncForModel = InitialSyncOperation(modelType: modelType,
8991
api: api,
9092
reconciliationQueue: reconciliationQueue,
9193
storageAdapter: storageAdapter,
9294
completion: syncOperationCompletion)
9395

9496
syncOperationQueue.addOperation(initialSyncForModel)
95-
96-
for downstream in root.downstream {
97-
enqueueSyncOperation(for: downstream)
98-
}
9997
}
10098

10199
private func resolveCompletion() {
@@ -117,3 +115,12 @@ final class AWSInitialSyncOrchestrator: InitialSyncOrchestrator {
117115

118116
@available(iOS 13.0, *)
119117
extension AWSInitialSyncOrchestrator: DefaultLogger { }
118+
119+
@available(iOS 13.0, *)
120+
extension AWSInitialSyncOrchestrator: Resettable {
121+
func reset(onComplete: @escaping BasicClosure) {
122+
syncOperationQueue.cancelAllOperations()
123+
syncOperationQueue.waitUntilAllOperationsAreFinished()
124+
onComplete()
125+
}
126+
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/MutationSync/MutationEvent/AWSMutationEventPublisher.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Amplify
99
import Combine
1010

1111
/// Note: This publisher accepts only a single subscriber
12-
@available(iOS 13, *)
12+
@available(iOS 13.0, *)
1313
final class AWSMutationEventPublisher: Publisher {
1414
typealias Output = MutationEvent
1515
typealias Failure = DataStoreError
@@ -79,7 +79,7 @@ extension AWSMutationEventPublisher: MutationEventPublisher {
7979
}
8080
}
8181

82-
@available(iOS 13, *)
82+
@available(iOS 13.0, *)
8383
extension AWSMutationEventPublisher: DefaultLogger { }
8484

8585
@available(iOS 13.0, *)

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/MutationSync/MutationEvent/MutationEventPublisher.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import Amplify
99
import Combine
1010

1111
/// Publishes mutation events to downstream subscribers for subsequent sync to the API.
12-
@available(iOS 13, *)
12+
@available(iOS 13.0, *)
1313
protocol MutationEventPublisher: class {
1414
var publisher: AnyPublisher<MutationEvent, DataStoreError> { get }
1515
}

0 commit comments

Comments
 (0)