Address review comments

AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/InitialSync/InitialSyncOrchestrator.swift

@@ -165,15 +165,7 @@ final class AWSInitialSyncOrchestrator: InitialSyncOrchestrator {
  underlyingError
  )
 
- // send success if atleast one model succeeded
- let syncableModelsCount = ModelRegistry.modelSchemas.filter { $0.isSyncable }.count
- if syncableModelsCount == syncErrors.count {
- return .failure(syncError)
- } else {
- self.log.verbose("\(#function) Atleast one model sync succeeded. Sending completion result as .success with error: \(syncError)")
- return .successfulVoid
- }
-
+ return .failure(syncError)
  }
 
  private func dispatchSyncQueriesStarted(for modelNames: [String]) {
@@ -254,6 +246,15 @@ extension AWSInitialSyncOrchestrator {
  case .unauthorized = AppSyncErrorType(errorTypeValue) {
  return true
  }
+
+ // Check is API error is of unauthorized type
+ if case let .api(amplifyError, _) = datastoreError,
+ let apiError = amplifyError as? APIError {
+ if apiError.isUnauthorized() {
+ return true
+ }
+ }
+
  return false
  }
 

AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Sync/Support/APIError+Unauthorized.swift

@@ -0,0 +1,63 @@
+//
+// Copyright Amazon.com Inc. or its affiliates.
+// All Rights Reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+import Amplify
+import AppSyncRealTimeClient
+
+extension APIError {
+
+ static let UnauthorizedMessageString: String = "Unauthorized"
+
+ /// By default, this consolidates unauthorized error scenarios coming from `APIError.httpStatusError` and
+ /// `APIError.operationError`. For `.httpStatusError`, this checks if the status code is 401 or 403. For
+ /// `.operationError`, this checks if the error description contains "Unauthorized".
+ ///
+ /// **Warning** Customized server responses that indicate unauthorized may not match the internal mapping done
+ /// in this API and return `false`. Check APIError enum directly or create your own `UnauthorizedDeterming` rule,
+ /// for example:
+ /// ```
+ /// static let customRule = UnauthorizedDetermining { error in
+ /// // Your custom logic to determine if `error` is unauthorized.
+ /// // return `true` or `false`.
+ /// }
+ /// ```
+ ///
+ /// - Parameter rule: Used to determine if the `APIError` is an unauthorized error.
+ /// - Returns: `true` if unauthorized error, `false` otherwise
+ public func isUnauthorized(rule: UnauthorizedDetermining = .default) -> Bool {
+ rule.isUnauthorized(self)
+ }
+
+ public struct UnauthorizedDetermining {
+ let isUnauthorized: (APIError) -> Bool
+
+ public init(isUnauthenticated: @escaping (APIError) -> Bool) {
+ self.isUnauthorized = isUnauthenticated
+ }
+
+ public static let `default` = UnauthorizedDetermining { error in
+ error.containsUnauthorizedMessageString || error.is401or403
+ }
+ }
+
+ private var is401or403: Bool {
+ switch self {
+ case .httpStatusError(let statusCode, _):
+ return statusCode == 401 || statusCode == 403
+ default:
+ return false
+ }
+ }
+
+ private var containsUnauthorizedMessageString: Bool {
+ switch self {
+ case .operationError(let errorDescription, _, _):
+ return errorDescription.range(of: APIError.UnauthorizedMessageString, options: .caseInsensitive) != nil
+ default: return false
+ }
+ }
+}

AmplifyPlugins/DataStore/Tests/AWSDataStorePluginTests/Sync/InitialSync/InitialSyncOrchestratorTests.swift

@@ -101,114 +101,13 @@ class InitialSyncOrchestratorTests: XCTestCase {
  Amplify.Hub.removeListener(hubListener)
  sink.cancel()
  }
-
- /// - Given: An InitialSyncOrchestrator with a model dependency graph, API is expected to return an error for all models
- /// - When:
- /// - The orchestrator starts up
- /// - Then:
- /// - Finish with an error when all sync queries fail
- func testFinishWithAPIErrorWhenAllSyncQueriesFail() async throws {
- ModelRegistry.reset()
- PostCommentModelRegistration().registerModels(registry: ModelRegistry.self)
- let responder = QueryRequestListenerResponder<PaginatedList<AnyModel>> { request, listener in
- if request.document.contains("SyncPosts") {
- let event: GraphQLOperation<PaginatedList<AnyModel>>.OperationResult =
- .failure(APIError.operationError("", "", nil))
- listener?(event)
- } else if request.document.contains("SyncComments") {
- let event: GraphQLOperation<PaginatedList<AnyModel>>.OperationResult =
- .failure(APIError.operationError("", "", nil))
- listener?(event)
- }
-
- return nil
- }
-
- let apiPlugin = MockAPICategoryPlugin()
- apiPlugin.responders[.queryRequestListener] = responder
-
- let storageAdapter = MockSQLiteStorageEngineAdapter()
- storageAdapter.returnOnQueryModelSyncMetadata(nil)
-
- let reconciliationQueue = MockReconciliationQueue()
-
- let orchestrator: AWSInitialSyncOrchestrator =
- AWSInitialSyncOrchestrator(dataStoreConfiguration: .default,
- authModeStrategy: AWSDefaultAuthModeStrategy(),
- api: apiPlugin,
- reconciliationQueue: reconciliationQueue,
- storageAdapter: storageAdapter)
-
- let syncCallbackReceived = expectation(description: "Sync callback received, sync operation is complete")
- let syncQueriesStartedReceived = expectation(description: "syncQueriesStarted received")
-
- let filter = HubFilters.forEventName(HubPayload.EventName.DataStore.syncQueriesStarted)
- let hubListener = Amplify.Hub.listen(to: .dataStore, isIncluded: filter) { payload in
- guard let syncQueriesStartedEvent = payload.data as? SyncQueriesStartedEvent else {
- XCTFail("Failed to cast payload data as SyncQueriesStartedEvent")
- return
- }
- XCTAssertEqual(syncQueriesStartedEvent.models.count, 2)
- syncQueriesStartedReceived.fulfill()
- }
-
- guard try await HubListenerTestUtilities.waitForListener(with: hubListener, timeout: 5.0) else {
- XCTFail("Listener not registered for hub")
- return
- }
-
- let syncStartedReceived = expectation(description: "Sync started received, sync operation started")
- syncStartedReceived.expectedFulfillmentCount = 2
- let finishedReceived = expectation(description: "InitialSyncOperation finished paginating and offering")
- finishedReceived.expectedFulfillmentCount = 2
- let failureCompletionReceived = expectation(description: "InitialSyncOrchestrator completed with failure")
- let sink = orchestrator
- .publisher
- .sink(receiveCompletion: { completion in
- switch completion {
- case .finished:
- XCTFail("Should have finished with failure")
- case .failure:
- failureCompletionReceived.fulfill()
- }
- }, receiveValue: { value in
- switch value {
- case .started:
- syncStartedReceived.fulfill()
- case .finished(let modelName, let error):
- if modelName == "Post" {
- guard case .api(let apiError, _) = error, case .operationError = apiError as? APIError else {
- XCTFail("Should be api error")
- return
- }
- } else if modelName == "Comment" {
- guard case .api(let apiError, _) = error, case .operationError = apiError as? APIError else {
- XCTFail("Should be api error")
- return
- }
- }
- finishedReceived.fulfill()
- default:
- break
- }
- })
-
- orchestrator.sync { _ in
- syncCallbackReceived.fulfill()
- }
-
- await waitForExpectations(timeout: 1)
- XCTAssertEqual(orchestrator.syncOperationQueue.maxConcurrentOperationCount, 1)
- Amplify.Hub.removeListener(hubListener)
- sink.cancel()
- }
 
  /// - Given: An InitialSyncOrchestrator with a model dependency graph, API is expected to return an error for certain models
  /// - When:
  /// - The orchestrator starts up
  /// - Then:
- /// - Finish with a success when at least one sync query succeeds
- func testFinishWithAPISuccessWhenAtlestOneSyncQuerySucceeds() async throws {
+ /// - Finish with an error for each sync query that fails.
+ func testFinishWithAPIError() async throws {
  ModelRegistry.reset()
  PostCommentModelRegistration().registerModels(registry: ModelRegistry.self)
  let responder = QueryRequestListenerResponder<PaginatedList<AnyModel>> { request, listener in
@@ -263,15 +162,15 @@ class InitialSyncOrchestratorTests: XCTestCase {
  syncStartedReceived.expectedFulfillmentCount = 2
  let finishedReceived = expectation(description: "InitialSyncOperation finished paginating and offering")
  finishedReceived.expectedFulfillmentCount = 2
- let successCompletionReceived = expectation(description: "InitialSyncOrchestrator completed with success")
+ let failureCompletionReceived = expectation(description: "InitialSyncOrchestrator completed with failure")
  let sink = orchestrator
  .publisher
  .sink(receiveCompletion: { completion in
  switch completion {
  case .finished:
- successCompletionReceived.fulfill()
+ XCTFail("Should have finished with failure")
  case .failure:
- XCTFail("Should have finished with success")
+ failureCompletionReceived.fulfill()
  }
  }, receiveValue: { value in
  switch value {