Skip to content

Commit 6696bac

Browse files
lawmichalawmicha
authored
fix(DataStore): drop failed constraint violation reconciliations (#1321)
* fix(DataStore): drop failed constraint violation reconciliations * address PR comments * add more tests and address PR comments
1 parent 06d7ddc commit 6696bac

File tree

11 files changed

+189
-20
lines changed

11 files changed

+189
-20
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,13 @@ protocol ModelStorageBehavior {
4444
completion: DataStoreCallback<[M]>)
4545

4646
}
47+
48+
protocol ModelStorageErrorBehavior {
49+
func shouldIgnoreError(error: DataStoreError) -> Bool
50+
}
51+
52+
extension ModelStorageErrorBehavior {
53+
func shouldIgnoreError(error: DataStoreError) -> Bool {
54+
return false
55+
}
56+
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,15 @@ final class SQLiteStorageEngineAdapter: StorageEngineAdapter {
360360
completion(.successfulVoid)
361361
}
362362

363+
func shouldIgnoreError(error: DataStoreError) -> Bool {
364+
if let sqliteError = SQLiteResultError(from: error),
365+
case .constraintViolation = sqliteError {
366+
return true
367+
}
368+
369+
return false
370+
}
371+
363372
static func clearIfNewVersion(version: String,
364373
dbFilePath: URL,
365374
userDefaults: UserDefaults = UserDefaults.standard,

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

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

12-
protocol StorageEngineAdapter: AnyObject, ModelStorageBehavior {
12+
protocol StorageEngineAdapter: AnyObject, ModelStorageBehavior, ModelStorageErrorBehavior {
1313

1414
static var maxNumberOfPredicates: Int { get }
1515

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/SubscriptionSync/ReconcileAndLocalSave/ReconcileAndLocalSaveOperation.swift‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -277,33 +277,33 @@ class ReconcileAndLocalSaveOperation: AsynchronousOperation {
277277

278278
let publishers = dispositions.map { disposition ->
279279
Publishers.FlatMap<Future<Void, DataStoreError>,
280-
Future<ReconcileAndLocalSaveOperation.RemoteModel, DataStoreError>> in
280+
Future<ReconcileAndLocalSaveOperation.ApplyRemoteModelResult, DataStoreError>> in
281281

282282
switch disposition {
283283
case .create(let remoteModel):
284284
let publisher = self.save(storageAdapter: storageAdapter,
285285
remoteModel: remoteModel)
286-
.flatMap { inProcessModel in
286+
.flatMap { applyResult in
287287
self.saveMetadata(storageAdapter: storageAdapter,
288-
inProcessModel: inProcessModel,
288+
applyResult: applyResult,
289289
mutationType: .create)
290290
}
291291
return publisher
292292
case .update(let remoteModel):
293293
let publisher = self.save(storageAdapter: storageAdapter,
294294
remoteModel: remoteModel)
295-
.flatMap { inProcessModel in
295+
.flatMap { applyResult in
296296
self.saveMetadata(storageAdapter: storageAdapter,
297-
inProcessModel: inProcessModel,
297+
applyResult: applyResult,
298298
mutationType: .update)
299299
}
300300
return publisher
301301
case .delete(let remoteModel):
302302
let publisher = self.delete(storageAdapter: storageAdapter,
303303
remoteModel: remoteModel)
304-
.flatMap { inProcessModel in
304+
.flatMap { applyResult in
305305
self.saveMetadata(storageAdapter: storageAdapter,
306-
inProcessModel: inProcessModel,
306+
applyResult: applyResult,
307307
mutationType: .delete)
308308
}
309309
return publisher
@@ -326,9 +326,14 @@ class ReconcileAndLocalSaveOperation: AsynchronousOperation {
326326
}
327327
}
328328

329+
enum ApplyRemoteModelResult {
330+
case applied(RemoteModel)
331+
case dropped
332+
}
333+
329334
private func delete(storageAdapter: StorageEngineAdapter,
330-
remoteModel: RemoteModel) -> Future<RemoteModel, DataStoreError> {
331-
Future<RemoteModel, DataStoreError> { promise in
335+
remoteModel: RemoteModel) -> Future<ApplyRemoteModelResult, DataStoreError> {
336+
Future<ApplyRemoteModelResult, DataStoreError> { promise in
332337
guard let modelType = ModelRegistry.modelType(from: self.modelSchema.name) else {
333338
let error = DataStoreError.invalidModelName(self.modelSchema.name)
334339
promise(.failure(error))
@@ -341,21 +346,31 @@ class ReconcileAndLocalSaveOperation: AsynchronousOperation {
341346
predicate: nil) { response in
342347
switch response {
343348
case .failure(let dataStoreError):
344-
promise(.failure(dataStoreError))
349+
if storageAdapter.shouldIgnoreError(error: dataStoreError) {
350+
self.notifyDropped(modelName: remoteModel.model.modelName)
351+
promise(.success(.dropped))
352+
} else {
353+
promise(.failure(dataStoreError))
354+
}
345355
case .success:
346-
promise(.success(remoteModel))
356+
promise(.success(.applied(remoteModel)))
347357
}
348358
}
349359
}
350360
}
351361

352362
private func save(storageAdapter: StorageEngineAdapter,
353-
remoteModel: RemoteModel) -> Future<RemoteModel, DataStoreError> {
354-
Future<RemoteModel, DataStoreError> { promise in
363+
remoteModel: RemoteModel) -> Future<ApplyRemoteModelResult, DataStoreError> {
364+
Future<ApplyRemoteModelResult, DataStoreError> { promise in
355365
storageAdapter.save(untypedModel: remoteModel.model.instance) { response in
356366
switch response {
357367
case .failure(let dataStoreError):
358-
promise(.failure(dataStoreError))
368+
if storageAdapter.shouldIgnoreError(error: dataStoreError) {
369+
self.notifyDropped(modelName: remoteModel.model.modelName)
370+
promise(.success(.dropped))
371+
} else {
372+
promise(.failure(dataStoreError))
373+
}
359374
case .success(let savedModel):
360375
let anyModel: AnyModel
361376
do {
@@ -366,16 +381,21 @@ class ReconcileAndLocalSaveOperation: AsynchronousOperation {
366381
return
367382
}
368383
let inProcessModel = MutationSync(model: anyModel, syncMetadata: remoteModel.syncMetadata)
369-
promise(.success(inProcessModel))
384+
promise(.success(.applied(inProcessModel)))
370385
}
371386
}
372387
}
373388
}
374389

375390
private func saveMetadata(storageAdapter: StorageEngineAdapter,
376-
inProcessModel: AppliedModel,
391+
applyResult: ApplyRemoteModelResult,
377392
mutationType: MutationEvent.MutationType) -> Future<Void, DataStoreError> {
378393
Future<Void, DataStoreError> { promise in
394+
guard case let .applied(inProcessModel) = applyResult else {
395+
promise(.successfulVoid)
396+
return
397+
}
398+
379399
storageAdapter.save(inProcessModel.syncMetadata, condition: nil) { result in
380400
switch result {
381401
case .failure(let dataStoreError):

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Sync/Support/SQLiteResultError.swift‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import SQLite
9+
import Amplify
10+
11+
/// Checks for specific SQLLite error codes
12+
/// See https://sqlite.org/rescode.html#primary_result_code_list
13+
enum SQLiteResultError {
14+
15+
/// Constraint Violation, such as foreign key constraint violation, occurs when trying to process a SQL statement
16+
/// where the insert/update statement is performed for a child object and its parent does not exist.
17+
/// See https://sqlite.org/rescode.html#constraint for more details
18+
case constraintViolation(statement: Statement?)
19+
20+
/// Represents a SQLite specific [error code](https://sqlite.org/rescode.html)
21+
///
22+
/// - message: English-language text that describes the error
23+
/// - code: SQLite [error code](https://sqlite.org/rescode.html#primary_result_code_list)
24+
/// - statement: the statement which produced the error
25+
case error(message: String, code: Int32, statement: Statement?)
26+
27+
init?(from dataStoreError: DataStoreError) {
28+
guard case let .invalidOperation(error) = dataStoreError,
29+
let resultError = error as? Result,
30+
case .error(let message, let code, let statement) = resultError else {
31+
return nil
32+
}
33+
34+
if code == SQLITE_CONSTRAINT {
35+
self = .constraintViolation(statement: statement)
36+
return
37+
}
38+
39+
self = .error(message: message, code: code, statement: statement)
40+
}
41+
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Core/SQLiteStorageEngineAdapterTests.swift‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//
77

88
import XCTest
9+
import SQLite
910

1011
@testable import Amplify
1112
@testable import AWSPluginsCore
@@ -644,4 +645,22 @@ class SQLiteStorageEngineAdapterTests: BaseDataStoreTests {
644645

645646
wait(for: [querySuccess], timeout: 1)
646647
}
648+
649+
func testShouldIgnoreConstraintViolationError() {
650+
let constraintViolationError = Result.error(message: "Foreign Key Constraint Violation",
651+
code: SQLITE_CONSTRAINT,
652+
statement: nil)
653+
let dataStoreError = DataStoreError.invalidOperation(causedBy: constraintViolationError)
654+
655+
XCTAssertTrue(storageAdapter.shouldIgnoreError(error: dataStoreError))
656+
}
657+
658+
func testShouldIgnoreErrorFalse() {
659+
let constraintViolationError = Result.error(message: "",
660+
code: SQLITE_BUSY,
661+
statement: nil)
662+
let dataStoreError = DataStoreError.invalidOperation(causedBy: constraintViolationError)
663+
664+
XCTAssertFalse(storageAdapter.shouldIgnoreError(error: dataStoreError))
665+
}
647666
}

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Sync/MutationQueue/ProcessMutationErrorFromCloudOperationTests.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ class ProcessMutationErrorFromCloudOperationTests: XCTestCase {
549549
storageAdapter.shouldReturnErrorOnDeleteMutation = false
550550
storageAdapter.responders[.deleteUntypedModel] = DeleteUntypedModelCompletionResponder { _ in
551551
modelDeletedEvent.fulfill()
552+
return .emptyResult
552553
}
553554
storageAdapter.responders[.saveModelCompletion] =
554555
SaveModelCompletionResponder<MutationSyncMetadata> { metadata, completion in

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Sync/SubscriptionSync/ReconcileAndLocalSaveOperationTests.swift‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,7 @@ class ReconcileAndLocalSaveOperationTests: XCTestCase {
653653
let deleteResponder = DeleteUntypedModelCompletionResponder { _, id in
654654
XCTAssertEqual(id, self.anyPostMutationSync.model.id)
655655
stoargeExpect.fulfill()
656+
return .emptyResult
656657
}
657658
storageAdapter.responders[.deleteUntypedModel] = deleteResponder
658659

@@ -728,6 +729,7 @@ class ReconcileAndLocalSaveOperationTests: XCTestCase {
728729
let deleteResponder = DeleteUntypedModelCompletionResponder { _, id in
729730
XCTAssertEqual(id, self.anyPostMutationSync.model.id)
730731
stoargeExpect.fulfill()
732+
return .emptyResult
731733
}
732734
storageAdapter.responders[.deleteUntypedModel] = deleteResponder
733735

@@ -803,6 +805,62 @@ class ReconcileAndLocalSaveOperationTests: XCTestCase {
803805
waitForExpectations(timeout: 1)
804806
}
805807

808+
func testApplyRemoteModels_failWithConstraintViolationShouldBeSuccessful() {
809+
let expect = expectation(description: "should complete successfully")
810+
expect.expectedFulfillmentCount = 2
811+
let dispositions: [RemoteSyncReconciler.Disposition] = [.create(anyPostMutationSync),
812+
.create(anyPostMutationSync),
813+
.update(anyPostMutationSync),
814+
.update(anyPostMutationSync),
815+
.delete(anyPostMutationSync),
816+
.delete(anyPostMutationSync),
817+
.create(anyPostMutationSync),
818+
.update(anyPostMutationSync),
819+
.delete(anyPostMutationSync)]
820+
let expectDropped = expectation(description: "should notify dropped")
821+
expectDropped.expectedFulfillmentCount = dispositions.count
822+
let dataStoreError = DataStoreError.internalOperation("Failed to save", "")
823+
let saveResponder = SaveUntypedModelResponder { _, completion in
824+
completion(.failure(dataStoreError))
825+
}
826+
storageAdapter.shouldIgnoreError = true
827+
storageAdapter.responders[.saveUntypedModel] = saveResponder
828+
let deleteResponder = DeleteUntypedModelCompletionResponder { _, _ in
829+
return .failure(dataStoreError)
830+
}
831+
storageAdapter.responders[.deleteUntypedModel] = deleteResponder
832+
833+
operation.publisher
834+
.sink { completion in
835+
switch completion {
836+
case .finished:
837+
XCTFail("Unexpected completion")
838+
case .failure(let error):
839+
XCTFail("Unexpected error \(error)")
840+
}
841+
} receiveValue: { event in
842+
switch event {
843+
case .mutationEventDropped(let name):
844+
expectDropped.fulfill()
845+
default:
846+
break
847+
}
848+
}.store(in: &cancellables)
849+
850+
operation.applyRemoteModelsDispositions(dispositions)
851+
.sink(receiveCompletion: { completion in
852+
switch completion {
853+
case .failure:
854+
XCTFail("Unexpected failure")
855+
case .finished:
856+
expect.fulfill()
857+
}
858+
}, receiveValue: { _ in
859+
expect.fulfill()
860+
}).store(in: &cancellables)
861+
waitForExpectations(timeout: 1)
862+
}
863+
806864
func testApplyRemoteModels_deleteFail() {
807865
let dispositions: [RemoteSyncReconciler.Disposition] = [.create(anyPostMutationSync),
808866
.create(anyPostMutationSync),

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Sync/SubscriptionSync/Support/MockSQLiteStorageEngineAdapter.swift‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ class MockSQLiteStorageEngineAdapter: StorageEngineAdapter {
2828

2929
var shouldReturnErrorOnSaveMetadata: Bool
3030
var shouldReturnErrorOnDeleteMutation: Bool
31+
var shouldIgnoreError: Bool
3132

3233
var resultForQueryModelSyncMetadata: ModelSyncMetadata?
3334
var listenerForModelSyncMetadata: BasicClosure?
3435

3536
init() {
3637
self.shouldReturnErrorOnSaveMetadata = false
3738
self.shouldReturnErrorOnDeleteMutation = false
39+
self.shouldIgnoreError = false
3840
self.resultForQueryMutationSyncMetadatas = [MutationSyncMetadata]()
3941
}
4042

@@ -96,8 +98,8 @@ class MockSQLiteStorageEngineAdapter: StorageEngineAdapter {
9698
predicate: QueryPredicate? = nil,
9799
completion: (Result<Void, DataStoreError>) -> Void) {
98100
if let responder = responders[.deleteUntypedModel] as? DeleteUntypedModelCompletionResponder {
99-
responder.callback((modelType, id))
100-
completion(.emptyResult)
101+
let result = responder.callback((modelType, id))
102+
completion(result)
101103
return
102104
}
103105

@@ -231,9 +233,14 @@ class MockSQLiteStorageEngineAdapter: StorageEngineAdapter {
231233
}
232234
try basicClosure()
233235
}
236+
234237
func clear(completion: @escaping DataStoreCallback<Void>) {
235238
XCTFail("Not expected to execute")
236239
}
240+
241+
func shouldIgnoreError(error: DataStoreError) -> Bool {
242+
return shouldIgnoreError
243+
}
237244
}
238245

239246
class MockStorageEngineBehavior: StorageEngineBehavior {

‎AmplifyPlugins/DataStore/AWSDataStoreCategoryPluginTests/Sync/SubscriptionSync/Support/MockSQLiteStorageEngineAdapterResponders.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ typealias SaveModelCompletionResponder<M: Model> = MockResponder<(M, DataStoreCa
3737

3838
typealias SaveUntypedModelResponder = MockResponder<(Model, DataStoreCallback<Model>), Void>
3939

40-
typealias DeleteUntypedModelCompletionResponder = MockResponder<(Model.Type, String), Void>
40+
typealias DeleteUntypedModelCompletionResponder = MockResponder<(Model.Type, String), DataStoreResult<Void>>
4141

4242
extension MockStorageEngineBehavior {
4343
enum ResponderKeys {

0 commit comments

Comments
 (0)