From 758a335bbde540d7a47293603c746ad96285fda5 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 09:48:46 -0500 Subject: [PATCH 01/20] fix(datastore): pass auth mode from swift --- .../pigeons/NativePluginBindings.kt | 7 ++-- .../api/GraphQLRequest+Extension.swift | 5 ++- .../pigeons/NativePluginBindings.swift | 6 +++- .../lib/amplify_datastore.dart | 11 ++++--- .../lib/src/native_plugin.g.dart | 5 +++ .../lib/src/utils/native_api_helpers.dart | 33 +++++++++++++++++-- .../pigeons/native_plugin.dart | 1 + 7 files changed, 57 insertions(+), 11 deletions(-) diff --git a/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt b/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt index 9a832cdd1a..58656c37c9 100644 --- a/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt +++ b/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt @@ -210,7 +210,8 @@ data class NativeGraphQLRequest ( val variablesJson: String? = null, val responseType: String? = null, val decodePath: String? = null, - val options: String? = null + val options: String? = null, + val authMode: String? = null ) { companion object { @@ -222,7 +223,8 @@ data class NativeGraphQLRequest ( val responseType = list[3] as String? val decodePath = list[4] as String? val options = list[5] as String? - return NativeGraphQLRequest(document, apiName, variablesJson, responseType, decodePath, options) + val authMode = list[6] as String? + return NativeGraphQLRequest(document, apiName, variablesJson, responseType, decodePath, options, authMode) } } fun toList(): List { @@ -233,6 +235,7 @@ data class NativeGraphQLRequest ( responseType, decodePath, options, + authMode, ) } } diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift index 9aedf6fc22..0472b99f26 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift @@ -13,13 +13,16 @@ extension GraphQLRequest { let variablesJson = self.variables .flatMap { try? JSONSerialization.data(withJSONObject: $0, options: []) } .flatMap { String(data: $0, encoding: .utf8) } + + let datastoreOptions = self.options?.pluginOptions as? AWSAPIPluginDataStoreOptions return NativeGraphQLRequest( document: self.document, apiName: self.apiName, variablesJson: variablesJson ?? "{}", responseType: String(describing: self.responseType), - decodePath: self.decodePath + decodePath: self.decodePath, + authMode: (datastoreOptions?.authType ?? self.authMode).map { String(describing: $0) } ) } } diff --git a/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift b/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift index be5850a1a8..34cd02efd6 100644 --- a/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift +++ b/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift @@ -215,6 +215,7 @@ struct NativeGraphQLRequest { var responseType: String? = nil var decodePath: String? = nil var options: String? = nil + var authMode: String? = nil static func fromList(_ list: [Any?]) -> NativeGraphQLRequest? { let document = list[0] as! String @@ -223,6 +224,7 @@ struct NativeGraphQLRequest { let responseType: String? = nilOrValue(list[3]) let decodePath: String? = nilOrValue(list[4]) let options: String? = nilOrValue(list[5]) + let authMode: String? = nilOrValue(list[6]) return NativeGraphQLRequest( document: document, @@ -230,7 +232,8 @@ struct NativeGraphQLRequest { variablesJson: variablesJson, responseType: responseType, decodePath: decodePath, - options: options + options: options, + authMode: authMode ) } func toList() -> [Any?] { @@ -241,6 +244,7 @@ struct NativeGraphQLRequest { responseType, decodePath, options, + authMode, ] } } diff --git a/packages/amplify_datastore/lib/amplify_datastore.dart b/packages/amplify_datastore/lib/amplify_datastore.dart index 40c2ad1579..95ebed3bc0 100644 --- a/packages/amplify_datastore/lib/amplify_datastore.dart +++ b/packages/amplify_datastore/lib/amplify_datastore.dart @@ -343,10 +343,13 @@ class _NativeAmplifyApi final subscription = operation.listen( (GraphQLResponse event) => - sendNativeDataEvent(flutterRequest.id, event.data), - onError: (error) { - // TODO(equartey): verify that error.toString() is the correct payload format. Should match AppSync - final errorPayload = error.toString(); + sendSubscriptionEvent(flutterRequest.id, event), + onError: (Object error) { + final errorPayload = jsonEncode({ + 'errors': [ + {'message': error.toString()} + ] + }); sendNativeErrorEvent(flutterRequest.id, errorPayload); }, onDone: () => sendNativeCompleteEvent(flutterRequest.id)); diff --git a/packages/amplify_datastore/lib/src/native_plugin.g.dart b/packages/amplify_datastore/lib/src/native_plugin.g.dart index f06960829b..08996512e4 100644 --- a/packages/amplify_datastore/lib/src/native_plugin.g.dart +++ b/packages/amplify_datastore/lib/src/native_plugin.g.dart @@ -214,6 +214,7 @@ class NativeGraphQLRequest { this.responseType, this.decodePath, this.options, + this.authMode, }); String document; @@ -228,6 +229,8 @@ class NativeGraphQLRequest { String? options; + String? authMode; + Object encode() { return [ document, @@ -236,6 +239,7 @@ class NativeGraphQLRequest { responseType, decodePath, options, + authMode, ]; } @@ -248,6 +252,7 @@ class NativeGraphQLRequest { responseType: result[3] as String?, decodePath: result[4] as String?, options: result[5] as String?, + authMode: result[6] as String?, ); } } diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index f283f4a435..894b5a0189 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -11,9 +11,30 @@ GraphQLRequest nativeRequestToGraphQLRequest( document: request.document, variables: jsonDecode(request.variablesJson ?? '{}'), apiName: request.apiName, + authorizationMode: nativeToApiAuthorizationType(request.authMode), ); } +/// Converts the Amplify Swift type [AWSAuthorizationType.value] to [APIAuthorizationType] +APIAuthorizationType? nativeToApiAuthorizationType(String? authMode) { + switch (authMode) { + case 'apiKey': + return APIAuthorizationType.apiKey; + case 'awsIAM': + return APIAuthorizationType.iam; + case 'openIDConnect': + return APIAuthorizationType.oidc; + case 'amazonCognitoUserPools': + return APIAuthorizationType.userPools; + case 'function': + return APIAuthorizationType.userPools; + case 'none': + return APIAuthorizationType.none; + default: + return null; + } +} + /// Convert a [GraphQLResponse] to a [NativeGraphQLResponse] NativeGraphQLResponse graphQLResponseToNativeResponse( GraphQLResponse response) { @@ -48,12 +69,18 @@ void sendNativeStartAckEvent(String subscriptionId) { _sendSubscriptionEvent(event); } -/// Send a data event for the given [subscriptionId] and [payloadJson] -void sendNativeDataEvent(String subscriptionId, String? payloadJson) { +/// Send a subscription event for the given [subscriptionId] and [GraphQLResponse] +/// If the response has errors, the event type will be `error`, otherwise `data` +void sendSubscriptionEvent( + String subscriptionId, GraphQLResponse response) { + final payloadJson = response.hasErrors + ? jsonEncode({"errors": response.errors}) + : jsonEncode(response.data); + final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, payloadJson: payloadJson, - type: 'data', + type: response.hasErrors ? "error" : "data", ); _sendSubscriptionEvent(event); } diff --git a/packages/amplify_datastore/pigeons/native_plugin.dart b/packages/amplify_datastore/pigeons/native_plugin.dart index 25584aa1ce..dc9f704701 100644 --- a/packages/amplify_datastore/pigeons/native_plugin.dart +++ b/packages/amplify_datastore/pigeons/native_plugin.dart @@ -126,4 +126,5 @@ class NativeGraphQLRequest { String? responseType; String? decodePath; String? options; + String? authMode; } From a99d900ea591ca9e910b1821c49fe707be7aac63 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 10:24:50 -0500 Subject: [PATCH 02/20] chore: removed authMode check as it will always be nil --- .../ios/Classes/api/GraphQLRequest+Extension.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift index 0472b99f26..dd5b23da39 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLRequest+Extension.swift @@ -22,7 +22,7 @@ extension GraphQLRequest { variablesJson: variablesJson ?? "{}", responseType: String(describing: self.responseType), decodePath: self.decodePath, - authMode: (datastoreOptions?.authType ?? self.authMode).map { String(describing: $0) } + authMode: datastoreOptions?.authType.map { String(describing: $0) } ) } } From 9a8062fd522989124ef315152ecdbd996bbee784 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 10:25:08 -0500 Subject: [PATCH 03/20] fix: add modelName to decoding logic --- .../amplify_datastore/ios/Classes/FlutterApiPlugin.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index bc447bc77e..0e1219d578 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -99,10 +99,13 @@ public class FlutterApiPlugin: APICategoryPlugin guard let payload else { throw DataStoreError.decodingError("Request payload could not be empty", "") } + + let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions return GraphQLResponse.fromAppSyncResponse( string: payload, - decodePath: request.decodePath + decodePath: request.decodePath, + modelName: datastoreOptions?.modelName ) } From da85f66a0d3db7ef744afb37c21e8a20efb8bc74 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 11:17:20 -0500 Subject: [PATCH 04/20] chore: made modelName for decode logic required --- .../Classes/api/GraphQLResponse+Decode.swift | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index ebc8a68819..f837587d31 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -22,7 +22,7 @@ extension GraphQLResponse { public static func fromAppSyncResponse( string: String, decodePath: String?, - modelName: String? = nil + modelName: String ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -44,7 +44,7 @@ extension GraphQLResponse { public static func fromAppSyncSubscriptionResponse( string: String, decodePath: String?, - modelName: String? = nil + modelName: String ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -98,7 +98,7 @@ extension GraphQLResponse { static func fromAppSyncResponse( json: JSONValue, decodePath: String?, - modelName: String? + modelName: String ) -> Result, APIError> { let data = decodePath != nil ? json.value(at: decodePath!) : json let errors = json.errors?.asArray @@ -147,23 +147,17 @@ extension GraphQLResponse { static func decodeDataPayload( _ dataPayload: JSONValue, - modelName: String? + modelName: String ) -> Result { if R.self == String.self { return encodeDataPayloadToString(dataPayload).map { $0 as! R } } - let dataPayloadWithTypeName = modelName.flatMap { - dataPayload.asObject?.merging( - ["__typename": .string($0)] - ) { a, _ in a } - }.map { JSONValue.object($0) } ?? dataPayload - if R.self == AnyModel.self { - return decodeDataPayloadToAnyModel(dataPayloadWithTypeName).map { $0 as! R } + return decodeDataPayloadToAnyModel(dataPayload, modelName: modelName).map { $0 as! R } } - return fromJson(dataPayloadWithTypeName) + return fromJson(dataPayload) .flatMap { data in Result { try jsonDecoder.decode(R.self, from: data) } .mapError { APIError.operationError("Could not decode json to type \(R.self)", "", $0)} @@ -171,29 +165,19 @@ extension GraphQLResponse { } static func decodeDataPayloadToAnyModel( - _ dataPayload: JSONValue + _ dataPayload: JSONValue, + modelName: String ) -> Result { - guard let typeName = dataPayload.__typename?.stringValue else { - return .failure(.operationError( - "Could not retrieve __typename from object", - """ - Could not retrieve the `__typename` attribute from the return value. Be sure to include __typename in \ - the selection set of the GraphQL operation. GraphQL: - \(dataPayload) - """ - )) - } - return encodeDataPayloadToString(dataPayload).flatMap { underlyingModelString in do { return .success(.init(try ModelRegistry.decode( - modelName: typeName, + modelName: modelName, from: underlyingModelString, jsonDecoder: jsonDecoder ))) } catch { return .failure(.operationError( - "Could not decode to \(typeName) with \(underlyingModelString)", + "Could not decode to \(modelName) with \(underlyingModelString)", "" )) } From e6c95e1ec71d5482cfab8a65b5eed39db90ab48a Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 11:26:36 -0500 Subject: [PATCH 05/20] fix: added null guards --- .../ios/Classes/FlutterApiPlugin.swift | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index 0e1219d578..5ce6c27abb 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -101,11 +101,15 @@ public class FlutterApiPlugin: APICategoryPlugin } let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions + + guard let modelName = datastoreOptions?.modelName else { + throw DataStoreError.decodingError("Request modelName can not be null", "") + } return GraphQLResponse.fromAppSyncResponse( string: payload, decodePath: request.decodePath, - modelName: datastoreOptions?.modelName + modelName: modelName ) } @@ -116,10 +120,17 @@ public class FlutterApiPlugin: APICategoryPlugin guard let payload else { throw DataStoreError.decodingError("Request payload could not be empty", "") } + + let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions + + guard let modelName = datastoreOptions?.modelName else { + throw DataStoreError.decodingError("Request modelName can not be null", "") + } return GraphQLResponse.fromAppSyncSubscriptionResponse( string: payload, - decodePath: request.decodePath + decodePath: request.decodePath, + modelName: modelName ) } From 9682b523048901547617373f17b8badd7447d3f5 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 12:59:25 -0500 Subject: [PATCH 06/20] fix: subscription data event --- .../amplify_datastore/lib/src/utils/native_api_helpers.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index 894b5a0189..34fc5ad8bc 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -75,7 +75,7 @@ void sendSubscriptionEvent( String subscriptionId, GraphQLResponse response) { final payloadJson = response.hasErrors ? jsonEncode({"errors": response.errors}) - : jsonEncode(response.data); + : response.data; final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, From 0938618de60570ea02d542dee2d4612d671669df Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 13:27:14 -0500 Subject: [PATCH 07/20] revert modelName requirement changes --- .../ios/Classes/FlutterApiPlugin.swift | 12 +----- .../Classes/api/GraphQLResponse+Decode.swift | 43 ++++++++++++++----- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index 5ce6c27abb..b23182c352 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -102,14 +102,10 @@ public class FlutterApiPlugin: APICategoryPlugin let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions - guard let modelName = datastoreOptions?.modelName else { - throw DataStoreError.decodingError("Request modelName can not be null", "") - } - return GraphQLResponse.fromAppSyncResponse( string: payload, decodePath: request.decodePath, - modelName: modelName + modelName: datastoreOptions?.modelName ) } @@ -122,15 +118,11 @@ public class FlutterApiPlugin: APICategoryPlugin } let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions - - guard let modelName = datastoreOptions?.modelName else { - throw DataStoreError.decodingError("Request modelName can not be null", "") - } return GraphQLResponse.fromAppSyncSubscriptionResponse( string: payload, decodePath: request.decodePath, - modelName: modelName + modelName: datastoreOptions?.modelName ) } diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index f837587d31..314751179d 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -22,7 +22,7 @@ extension GraphQLResponse { public static func fromAppSyncResponse( string: String, decodePath: String?, - modelName: String + modelName: String? = nil ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -30,6 +30,12 @@ extension GraphQLResponse { .unknown("Unable to deserialize json data", "Check the event structure.") )) } + + // we expect when flutter calls this to provide a modelName + guard let modelName = modelName else { + return .failure(.transformationError(string, .unknown("Unable to get modelName", "Please provide a modelName"))) + } + let result: Result, APIError> = toJson(data: data).flatMap { fromAppSyncResponse(json: $0, decodePath: decodePath, modelName: modelName) @@ -44,7 +50,7 @@ extension GraphQLResponse { public static func fromAppSyncSubscriptionResponse( string: String, decodePath: String?, - modelName: String + modelName: String? = nil ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -98,7 +104,7 @@ extension GraphQLResponse { static func fromAppSyncResponse( json: JSONValue, decodePath: String?, - modelName: String + modelName: String? ) -> Result, APIError> { let data = decodePath != nil ? json.value(at: decodePath!) : json let errors = json.errors?.asArray @@ -147,17 +153,24 @@ extension GraphQLResponse { static func decodeDataPayload( _ dataPayload: JSONValue, - modelName: String + modelName: String? ) -> Result { if R.self == String.self { return encodeDataPayloadToString(dataPayload).map { $0 as! R } } + + /// This is included to allow multi-platform support. Requests that do not have `__typename` + let dataPayloadWithTypeName = modelName.flatMap { + dataPayload.asObject?.merging( + ["__typename": .string($0)] + ) { a, _ in a } + }.map { JSONValue.object($0) } ?? dataPayload if R.self == AnyModel.self { - return decodeDataPayloadToAnyModel(dataPayload, modelName: modelName).map { $0 as! R } + return decodeDataPayloadToAnyModel(dataPayloadWithTypeName).map { $0 as! R } } - return fromJson(dataPayload) + return fromJson(dataPayloadWithTypeName) .flatMap { data in Result { try jsonDecoder.decode(R.self, from: data) } .mapError { APIError.operationError("Could not decode json to type \(R.self)", "", $0)} @@ -165,19 +178,29 @@ extension GraphQLResponse { } static func decodeDataPayloadToAnyModel( - _ dataPayload: JSONValue, - modelName: String + _ dataPayload: JSONValue ) -> Result { + guard let typeName = dataPayload.__typename?.stringValue else { + return .failure(.operationError( + "Could not retrieve __typename from object", + """ + Could not retrieve the `__typename` attribute from the return value. Be sure to include __typename in \ + the selection set of the GraphQL operation. GraphQL: + \(dataPayload) + """ + )) + } + return encodeDataPayloadToString(dataPayload).flatMap { underlyingModelString in do { return .success(.init(try ModelRegistry.decode( - modelName: modelName, + modelName: typeName, from: underlyingModelString, jsonDecoder: jsonDecoder ))) } catch { return .failure(.operationError( - "Could not decode to \(modelName) with \(underlyingModelString)", + "Could not decode to \(typeName) with \(underlyingModelString)", "" )) } From 5fac126fdedd416f760890e18818b7669c070f25 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 13:28:01 -0500 Subject: [PATCH 08/20] remove guard check --- .../ios/Classes/api/GraphQLResponse+Decode.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index 314751179d..acb977c8e4 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -30,12 +30,6 @@ extension GraphQLResponse { .unknown("Unable to deserialize json data", "Check the event structure.") )) } - - // we expect when flutter calls this to provide a modelName - guard let modelName = modelName else { - return .failure(.transformationError(string, .unknown("Unable to get modelName", "Please provide a modelName"))) - } - let result: Result, APIError> = toJson(data: data).flatMap { fromAppSyncResponse(json: $0, decodePath: decodePath, modelName: modelName) From 15ec356aac4f50a530caf38b36957965a8cd0106 Mon Sep 17 00:00:00 2001 From: Di Wu Date: Fri, 31 May 2024 19:02:02 +0000 Subject: [PATCH 09/20] fix(datastore): keep swift graphql decoding with non optional modelName (#4955) --- .../GraphQLResponse+DecodeTests.swift | 44 ++++++++++++++----- .../ios/Classes/FlutterApiPlugin.swift | 14 +++--- .../Classes/api/GraphQLResponse+Decode.swift | 17 ++++--- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift b/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift index 3c5d1071af..c046b6c341 100644 --- a/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift +++ b/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift @@ -116,7 +116,7 @@ class GraphQLResponseDecodeTests: XCTestCase { "a": ["b"] ] let expectedJson = "{\"a\":[\"b\"]}" - let result: Result = GraphQLResponse.decodeDataPayload(json, modelName: nil) + let result: Result = GraphQLResponse.decodeDataPayload(json, modelName: "Post") XCTAssertNoThrow(try result.get()) XCTAssertEqual(expectedJson, try result.get()) } @@ -137,7 +137,7 @@ class GraphQLResponseDecodeTests: XCTestCase { "author": "authorId", ], modelName: "Post")) - let result: Result = GraphQLResponse.decodeDataPayload(json, modelName: nil) + let result: Result = GraphQLResponse.decodeDataPayload(json, modelName: "Post") XCTAssertNoThrow(try result.get()) XCTAssertEqual(expectedModel.modelName, try result.get().modelName) XCTAssertEqual(expectedModel.id, try result.get().id) @@ -227,7 +227,11 @@ class GraphQLResponseDecodeTests: XCTestCase { "author": "authorId", ], modelName: "Post")) - let result: Result>, APIError> = .fromAppSyncResponse(json: json, decodePath: "onCreatePost", modelName: nil) + let result: Result>, APIError> = .fromAppSyncResponse( + json: json, + decodePath: "onCreatePost", + modelName: "Post" + ) XCTAssertNoThrow(try result.get()) let mutationSync = try! result.get() XCTAssertNoThrow(try mutationSync.get()) @@ -244,7 +248,11 @@ class GraphQLResponseDecodeTests: XCTestCase { ] ] - let result: Result>, APIError> = .fromAppSyncResponse(json: json, decodePath: "onCreatePost", modelName: nil) + let result: Result>, APIError> = .fromAppSyncResponse( + json: json, + decodePath: "onCreatePost", + modelName: "Post" + ) XCTAssertNoThrow(try result.get()) let mutationSync = try! result.get() XCTAssertThrowsError(try mutationSync.get()) { error in @@ -283,7 +291,11 @@ class GraphQLResponseDecodeTests: XCTestCase { "author": "authorId", ], modelName: "Post")) - let result: Result>, APIError> = .fromAppSyncResponse(json: json, decodePath: "onCreatePost", modelName: nil) + let result: Result>, APIError> = .fromAppSyncResponse( + json: json, + decodePath: "onCreatePost", + modelName: "Post" + ) XCTAssertNoThrow(try result.get()) let mutationSync = try! result.get() XCTAssertThrowsError(try mutationSync.get()) { error in @@ -305,7 +317,11 @@ class GraphQLResponseDecodeTests: XCTestCase { "a": "b" ] - let result: Result>, APIError> = .fromAppSyncResponse(json: json, decodePath: "onCreatePost", modelName: nil) + let result: Result>, APIError> = .fromAppSyncResponse( + json: json, + decodePath: "onCreatePost", + modelName: "Post" + ) XCTAssertThrowsError(try result.get()) { error in if case .unknown(let description, _, _) = (error as! APIError) { XCTAssertEqual("Failed to get data object or errors from GraphQL response", description) @@ -325,7 +341,7 @@ class GraphQLResponseDecodeTests: XCTestCase { ] let jsonString = String(data: try! JSONEncoder().encode(json), encoding: .utf8)! - let response: GraphQLResponse = .fromAppSyncResponse(string: jsonString, decodePath: nil) + let response: GraphQLResponse = .fromAppSyncResponse(string: jsonString, decodePath: nil, modelName: "Post") XCTAssertNoThrow(try response.get()) XCTAssertEqual(json.id?.stringValue, try response.get().identifier) XCTAssertEqual(json.__typename?.stringValue, try response.get().modelName) @@ -334,7 +350,7 @@ class GraphQLResponseDecodeTests: XCTestCase { func testFromAppSyncResponse_withBrokenJsonString_failWithTransformationError() { SchemaData.modelSchemaRegistry.registerModels(registry: ModelRegistry.self) let jsonString = "{" - let response: GraphQLResponse = .fromAppSyncResponse(string: jsonString, decodePath: nil) + let response: GraphQLResponse = .fromAppSyncResponse(string: jsonString, decodePath: nil, modelName: "Post") XCTAssertThrowsError(try response.get()) { error in guard case .transformationError = error as! GraphQLResponseError else { XCTFail("Should failed with transformationError") @@ -358,7 +374,11 @@ class GraphQLResponseDecodeTests: XCTestCase { ] let jsonString = String(data: try! JSONEncoder().encode(payloadJson), encoding: .utf8)! - let response: GraphQLResponse> = .fromAppSyncResponse(string: jsonString, decodePath: "onCreatePost") + let response: GraphQLResponse> = .fromAppSyncResponse( + string: jsonString, + decodePath: "onCreatePost", + modelName: "Post" + ) XCTAssertNoThrow(try response.get()) let mutationSync = try! response.get() XCTAssertEqual(payloadJson.onCreatePost?.id?.stringValue, mutationSync.model.identifier) @@ -368,7 +388,11 @@ class GraphQLResponseDecodeTests: XCTestCase { func testFromAppSyncSubscriptionResponse_withWrongJsonString_failWithTransformationError() { SchemaData.modelSchemaRegistry.registerModels(registry: ModelRegistry.self) let jsonString = "{" - let response: GraphQLResponse> = .fromAppSyncSubscriptionResponse(string: jsonString, decodePath: nil) + let response: GraphQLResponse> = .fromAppSyncSubscriptionResponse( + string: jsonString, + decodePath: nil, + modelName: "Post" + ) XCTAssertThrowsError(try response.get()) { error in guard case .transformationError = error as! GraphQLResponseError> else { XCTFail("Should failed with transformationError") diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index b23182c352..0ceb6ffa02 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -100,12 +100,14 @@ public class FlutterApiPlugin: APICategoryPlugin throw DataStoreError.decodingError("Request payload could not be empty", "") } - let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions - + guard let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions else { + throw DataStoreError.decodingError("Failed to decode the GraphQLRequest due to a missing options field.", "") + } + return GraphQLResponse.fromAppSyncResponse( string: payload, decodePath: request.decodePath, - modelName: datastoreOptions?.modelName + modelName: datastoreOptions.modelName ) } @@ -117,12 +119,14 @@ public class FlutterApiPlugin: APICategoryPlugin throw DataStoreError.decodingError("Request payload could not be empty", "") } - let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions + guard let datastoreOptions = request.options?.pluginOptions as? AWSAPIPluginDataStoreOptions else { + throw DataStoreError.decodingError("Failed to decode the GraphQLRequest due to a missing options field.", "") + } return GraphQLResponse.fromAppSyncSubscriptionResponse( string: payload, decodePath: request.decodePath, - modelName: datastoreOptions?.modelName + modelName: datastoreOptions.modelName ) } diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index acb977c8e4..630fdd0636 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -22,7 +22,7 @@ extension GraphQLResponse { public static func fromAppSyncResponse( string: String, decodePath: String?, - modelName: String? = nil + modelName: String ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -44,7 +44,7 @@ extension GraphQLResponse { public static func fromAppSyncSubscriptionResponse( string: String, decodePath: String?, - modelName: String? = nil + modelName: String ) -> GraphQLResponse { guard let data = string.data(using: .utf8) else { return .failure(.transformationError( @@ -98,7 +98,7 @@ extension GraphQLResponse { static func fromAppSyncResponse( json: JSONValue, decodePath: String?, - modelName: String? + modelName: String ) -> Result, APIError> { let data = decodePath != nil ? json.value(at: decodePath!) : json let errors = json.errors?.asArray @@ -147,18 +147,17 @@ extension GraphQLResponse { static func decodeDataPayload( _ dataPayload: JSONValue, - modelName: String? + modelName: String ) -> Result { if R.self == String.self { return encodeDataPayloadToString(dataPayload).map { $0 as! R } } /// This is included to allow multi-platform support. Requests that do not have `__typename` - let dataPayloadWithTypeName = modelName.flatMap { - dataPayload.asObject?.merging( - ["__typename": .string($0)] - ) { a, _ in a } - }.map { JSONValue.object($0) } ?? dataPayload + let dataPayloadWithTypeName = (dataPayload.asObject?.merging( + ["__typename": .string(modelName)], + uniquingKeysWith: { a, _ in a } + )).map { JSONValue.object($0) } ?? dataPayload if R.self == AnyModel.self { return decodeDataPayloadToAnyModel(dataPayloadWithTypeName).map { $0 as! R } From f53ab59474727cbe9af0293e1f5a914736f08362 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 14:54:46 -0500 Subject: [PATCH 10/20] fix: reuse sendErrorEvent helper --- .../lib/src/utils/native_api_helpers.dart | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index 34fc5ad8bc..fcb4399e11 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -73,20 +73,22 @@ void sendNativeStartAckEvent(String subscriptionId) { /// If the response has errors, the event type will be `error`, otherwise `data` void sendSubscriptionEvent( String subscriptionId, GraphQLResponse response) { - final payloadJson = response.hasErrors - ? jsonEncode({"errors": response.errors}) - : response.data; + if (response.hasErrors) { + final errorPayload = jsonEncode({"errors": response.errors}); + sendNativeErrorEvent(subscriptionId, errorPayload); + return; + } final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, - payloadJson: payloadJson, - type: response.hasErrors ? "error" : "data", + payloadJson: response.data, + type: "data", ); _sendSubscriptionEvent(event); } /// Send an error event for the given [subscriptionId] and [errorPayload] -void sendNativeErrorEvent(String subscriptionId, String errorPayload) { +void sendNativeErrorEvent(String subscriptionId, String? errorPayload) { final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, payloadJson: errorPayload, From 4b449caed586453e1caf7a6bb9a6c781b012399d Mon Sep 17 00:00:00 2001 From: Di Wu Date: Fri, 31 May 2024 21:48:35 +0000 Subject: [PATCH 11/20] fix(datastore): crash when insert cancellables (#4956) --- .../amplify_datastore/ios/Classes/FlutterApiPlugin.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index 0ceb6ffa02..c7cd9988cc 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -2,15 +2,13 @@ import Foundation import Flutter import Combine - - public class FlutterApiPlugin: APICategoryPlugin { public var key: PluginKey = "awsAPIPlugin" private let apiAuthFactory: APIAuthProviderFactory private let nativeApiPlugin: NativeApiPlugin private let nativeSubscriptionEvents: PassthroughSubject - private var cancellables = Set() + private var cancellables = AtomicDictionary() init( apiAuthProviderFactory: APIAuthProviderFactory, @@ -82,7 +80,9 @@ public class FlutterApiPlugin: APICategoryPlugin } } .toAmplifyAsyncThrowingSequence() - cancellables.insert(cancellable) // the subscription is bind with class instance lifecycle, it should be released when stream is finished or unsubscribed + + cancellables.set(value: (), forKey: cancellable) // the subscription is bind with class instance lifecycle, it should be released when stream is finished or unsubscribed + sequence.send(.connection(.connecting)) DispatchQueue.main.async { self.nativeApiPlugin.subscribe(request: request.toNativeGraphQLRequest()) { response in From 1b4057f6304cbf3eecc9e66d68dd9f0ca576ba82 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 17:30:34 -0500 Subject: [PATCH 12/20] fix: incorrect transform type --- .../amplify_datastore/lib/src/utils/native_api_helpers.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index fcb4399e11..9e1493176a 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -27,7 +27,7 @@ APIAuthorizationType? nativeToApiAuthorizationType(String? authMode) { case 'amazonCognitoUserPools': return APIAuthorizationType.userPools; case 'function': - return APIAuthorizationType.userPools; + return APIAuthorizationType.function; case 'none': return APIAuthorizationType.none; default: From 32c7e09c8967db74c9abbd4c96d42751e4360586 Mon Sep 17 00:00:00 2001 From: Di Wu Date: Fri, 31 May 2024 18:37:59 -0700 Subject: [PATCH 13/20] fix(datastore): reclassify unauthorized error --- .../amplify_datastore/ios/Classes/FlutterApiPlugin.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index c7cd9988cc..39ab466551 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -79,6 +79,15 @@ public class FlutterApiPlugin: APICategoryPlugin return nil } } + .flatMap { (event: GraphQLSubscriptionEvent) -> AnyPublisher, Error> in + if case .data(.failure(let graphQLResponseError)) = event, + case .error(let errors) = graphQLResponseError, + errors.contains(where: self.isUnauthorizedError(graphQLError:)) { + return Fail(error: APIError.operationError("Unauthorized", "", nil)).eraseToAnyPublisher() + } + return Just(event).setFailureType(to: Error.self).eraseToAnyPublisher() + } + .eraseToAnyPublisher() .toAmplifyAsyncThrowingSequence() cancellables.set(value: (), forKey: cancellable) // the subscription is bind with class instance lifecycle, it should be released when stream is finished or unsubscribed From 8c1409cc036b2e43b02d4c4dffcf4b1f69dc5169 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 31 May 2024 21:33:15 -0500 Subject: [PATCH 14/20] fix: consolidated NativeGraphQLResponse to one payload to match AppSync --- .../pigeons/NativePluginBindings.kt | 7 +-- .../GraphQLResponse+DecodeTests.swift | 58 ++++++++++--------- .../ios/Classes/FlutterApiPlugin.swift | 1 + .../Classes/api/GraphQLResponse+Decode.swift | 3 +- .../pigeons/NativePluginBindings.swift | 6 +- .../lib/src/native_plugin.g.dart | 5 -- .../lib/src/utils/native_api_helpers.dart | 14 +++-- .../pigeons/native_plugin.dart | 1 - 8 files changed, 46 insertions(+), 49 deletions(-) diff --git a/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt b/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt index 58656c37c9..9dd0e1ee4e 100644 --- a/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt +++ b/packages/amplify_datastore/android/src/main/kotlin/com/amazonaws/amplify/amplify_datastore/pigeons/NativePluginBindings.kt @@ -158,22 +158,19 @@ data class NativeAWSCredentials ( /** Generated class from Pigeon that represents data sent in messages. */ data class NativeGraphQLResponse ( - val payloadJson: String? = null, - val errorsJson: String? = null + val payloadJson: String? = null ) { companion object { @Suppress("UNCHECKED_CAST") fun fromList(list: List): NativeGraphQLResponse { val payloadJson = list[0] as String? - val errorsJson = list[1] as String? - return NativeGraphQLResponse(payloadJson, errorsJson) + return NativeGraphQLResponse(payloadJson) } } fun toList(): List { return listOf( payloadJson, - errorsJson, ) } } diff --git a/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift b/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift index c046b6c341..a08f53f8c9 100644 --- a/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift +++ b/packages/amplify_datastore/example/ios/unit_tests/GraphQLResponse+DecodeTests.swift @@ -209,14 +209,16 @@ class GraphQLResponseDecodeTests: XCTestCase { func testFromAppSyncResponse_withOnlyData_decodePayload() { SchemaData.modelSchemaRegistry.registerModels(registry: ModelRegistry.self) let json: JSONValue = [ - "onCreatePost": [ - "__typename": "Post", - "id": "123", - "title": "test", - "author": "authorId", - "_version": 1, - "_deleted": nil, - "_lastChangedAt": 1707773705221 + "data": [ + "onCreatePost": [ + "__typename": "Post", + "id": "123", + "title": "test", + "author": "authorId", + "_version": 1, + "_deleted": nil, + "_lastChangedAt": 1707773705221 + ] ] ] @@ -269,14 +271,16 @@ class GraphQLResponseDecodeTests: XCTestCase { func testFromAppSyncResponse_withDataAndErrors_decodePayload() { SchemaData.modelSchemaRegistry.registerModels(registry: ModelRegistry.self) let json: JSONValue = [ - "onCreatePost": [ - "__typename": "Post", - "id": "123", - "title": "test", - "author": "authorId", - "_version": 1, - "_deleted": nil, - "_lastChangedAt": 1707773705221 + "data": [ + "onCreatePost": [ + "__typename": "Post", + "id": "123", + "title": "test", + "author": "authorId", + "_version": 1, + "_deleted": nil, + "_lastChangedAt": 1707773705221 + ], ], "errors": [ ["message": "error1"], @@ -362,14 +366,16 @@ class GraphQLResponseDecodeTests: XCTestCase { func testFromAppSyncSubscriptionResponse_withJsonString_decodeCorrectly() { SchemaData.modelSchemaRegistry.registerModels(registry: ModelRegistry.self) let payloadJson: JSONValue = [ - "onCreatePost": [ - "__typename": "Post", - "id": "123", - "title": "test", - "author": "authorId", - "_version": 1, - "_deleted": nil, - "_lastChangedAt": 1707773705221 + "data": [ + "onCreatePost": [ + "__typename": "Post", + "id": "123", + "title": "test", + "author": "authorId", + "_version": 1, + "_deleted": nil, + "_lastChangedAt": 1707773705221 + ] ] ] @@ -381,8 +387,8 @@ class GraphQLResponseDecodeTests: XCTestCase { ) XCTAssertNoThrow(try response.get()) let mutationSync = try! response.get() - XCTAssertEqual(payloadJson.onCreatePost?.id?.stringValue, mutationSync.model.identifier) - XCTAssertEqual(payloadJson.onCreatePost?.__typename?.stringValue, mutationSync.model.modelName) + XCTAssertEqual(payloadJson.data?.onCreatePost?.id?.stringValue, mutationSync.model.identifier) + XCTAssertEqual(payloadJson.data?.onCreatePost?.__typename?.stringValue, mutationSync.model.modelName) } func testFromAppSyncSubscriptionResponse_withWrongJsonString_failWithTransformationError() { diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index 39ab466551..c6f26eb91d 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -22,6 +22,7 @@ public class FlutterApiPlugin: APICategoryPlugin public func query(request: GraphQLRequest) async throws -> GraphQLTask.Success where R : Decodable { let response = await asyncQuery(nativeRequest: request.toNativeGraphQLRequest()) + return try decodeGraphQLPayloadJson(request: request, payload: response.payloadJson) } diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index 630fdd0636..ccaef82ef0 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -100,8 +100,9 @@ extension GraphQLResponse { decodePath: String?, modelName: String ) -> Result, APIError> { - let data = decodePath != nil ? json.value(at: decodePath!) : json + var data = decodePath != nil ? json.data?.value(at: decodePath!) : json let errors = json.errors?.asArray + data = data?.isNull == true ? nil : data switch (data, errors) { case (.some(let data), .none): return decodeDataPayload(data, modelName: modelName).map { .success($0) } diff --git a/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift b/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift index 34cd02efd6..a619cbb998 100644 --- a/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift +++ b/packages/amplify_datastore/ios/Classes/pigeons/NativePluginBindings.swift @@ -162,21 +162,17 @@ struct NativeAWSCredentials { /// Generated class from Pigeon that represents data sent in messages. struct NativeGraphQLResponse { var payloadJson: String? = nil - var errorsJson: String? = nil static func fromList(_ list: [Any?]) -> NativeGraphQLResponse? { let payloadJson: String? = nilOrValue(list[0]) - let errorsJson: String? = nilOrValue(list[1]) return NativeGraphQLResponse( - payloadJson: payloadJson, - errorsJson: errorsJson + payloadJson: payloadJson ) } func toList() -> [Any?] { return [ payloadJson, - errorsJson, ] } } diff --git a/packages/amplify_datastore/lib/src/native_plugin.g.dart b/packages/amplify_datastore/lib/src/native_plugin.g.dart index 08996512e4..327c28c658 100644 --- a/packages/amplify_datastore/lib/src/native_plugin.g.dart +++ b/packages/amplify_datastore/lib/src/native_plugin.g.dart @@ -152,17 +152,13 @@ class NativeAWSCredentials { class NativeGraphQLResponse { NativeGraphQLResponse({ this.payloadJson, - this.errorsJson, }); String? payloadJson; - String? errorsJson; - Object encode() { return [ payloadJson, - errorsJson, ]; } @@ -170,7 +166,6 @@ class NativeGraphQLResponse { result as List; return NativeGraphQLResponse( payloadJson: result[0] as String?, - errorsJson: result[1] as String?, ); } } diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index 9e1493176a..c9b7581e01 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -38,12 +38,14 @@ APIAuthorizationType? nativeToApiAuthorizationType(String? authMode) { /// Convert a [GraphQLResponse] to a [NativeGraphQLResponse] NativeGraphQLResponse graphQLResponseToNativeResponse( GraphQLResponse response) { - final errorJson = jsonEncode( - response.errors.whereNotNull().map((e) => e.toJson()).toList()); - return NativeGraphQLResponse( - payloadJson: response.data, - errorsJson: errorJson, - ); + final errorJson = + response.errors.whereNotNull().map((e) => e.toJson()).toList(); + final data = jsonDecode(response.data ?? '{}'); + final payload = jsonEncode({ + 'data': data, + 'errors': errorJson, + }); + return NativeGraphQLResponse(payloadJson: payload); } /// Returns a connecting event [NativeGraphQLResponse] for the given [subscriptionId] diff --git a/packages/amplify_datastore/pigeons/native_plugin.dart b/packages/amplify_datastore/pigeons/native_plugin.dart index dc9f704701..d44a729c8f 100644 --- a/packages/amplify_datastore/pigeons/native_plugin.dart +++ b/packages/amplify_datastore/pigeons/native_plugin.dart @@ -110,7 +110,6 @@ class LegacyCredentialStoreData { class NativeGraphQLResponse { String? payloadJson; - String? errorsJson; } class NativeGraphQLSubscriptionResponse { From 234876b15a61d948503859f6d337c9261a3e05f8 Mon Sep 17 00:00:00 2001 From: Di Wu Date: Fri, 31 May 2024 17:02:42 -0700 Subject: [PATCH 15/20] pass api failure to sequence --- .../ios/Classes/FlutterApiPlugin.swift | 10 +++++++++- .../ios/Classes/api/GraphQLResponse+Decode.swift | 9 ++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift index c6f26eb91d..bd3fa4c122 100644 --- a/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift +++ b/packages/amplify_datastore/ios/Classes/FlutterApiPlugin.swift @@ -48,6 +48,7 @@ public class FlutterApiPlugin: APICategoryPlugin // TODO: shouldn't there be a timeout if there is no start_ack returned in a certain period of time let (sequence, cancellable) = nativeSubscriptionEvents + .setFailureType(to: Error.self) .receive(on: DispatchQueue.global()) .filter { $0.subscriptionId == subscriptionId } .handleEvents(receiveCompletion: {_ in @@ -139,7 +140,14 @@ public class FlutterApiPlugin: APICategoryPlugin modelName: datastoreOptions.modelName ) } - + + private func isUnauthorizedError(graphQLError: GraphQLError) -> Bool { + guard case let .string(errorTypeValue) = graphQLError.extensions?["errorType"] else { + return false + } + return errorTypeValue == "Unauthorized" + } + func asyncQuery(nativeRequest: NativeGraphQLRequest) async -> NativeGraphQLResponse { await withCheckedContinuation { continuation in DispatchQueue.main.async { diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index ccaef82ef0..5f41c66500 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -129,11 +129,18 @@ extension GraphQLResponse { return nil } - let extensions = errorObject.enumerated().filter { !["message", "locations", "path", "extensions"].contains($0.element.key) } + var extensions = errorObject.enumerated().filter { !["message", "locations", "path", "extensions"].contains($0.element.key) } .reduce([String: JSONValue]()) { partialResult, item in partialResult.merging([item.element.key: item.element.value]) { $1 } } + if error.message?.stringValue?.contains("Unauthorized") == true { + extensions = extensions.merging( + ["errorType": "Unauthorized"], + uniquingKeysWith: { _, a in a } + ) + } + return (try? jsonEncoder.encode(error)) .flatMap { try? jsonDecoder.decode(GraphQLError.self, from: $0) } .map { From 303a0da71821f9139337a71c2c1f2a261344c05f Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Sun, 2 Jun 2024 09:54:28 -0500 Subject: [PATCH 16/20] fix: standardized decoding for data & error for query, mutate, and subscriptions --- .../Classes/api/GraphQLResponse+Decode.swift | 3 ++- ...yncSubscriptionEventToAnyModelMapper.swift | 2 +- .../lib/src/utils/native_api_helpers.dart | 25 ++++++++++--------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index 5f41c66500..620ff31830 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -101,7 +101,8 @@ extension GraphQLResponse { modelName: String ) -> Result, APIError> { var data = decodePath != nil ? json.data?.value(at: decodePath!) : json - let errors = json.errors?.asArray + let errorsArray = json.errors?.asArray + let errors = errorsArray.isEmpty ? nil : errorsArray data = data?.isNull == true ? nil : data switch (data, errors) { case (.some(let data), .none): diff --git a/packages/amplify_datastore/ios/internal/AWSDataStorePlugin/Sync/SubscriptionSync/IncomingAsyncSubscriptionEventToAnyModelMapper.swift b/packages/amplify_datastore/ios/internal/AWSDataStorePlugin/Sync/SubscriptionSync/IncomingAsyncSubscriptionEventToAnyModelMapper.swift index cf33218e63..3c28e60aee 100644 --- a/packages/amplify_datastore/ios/internal/AWSDataStorePlugin/Sync/SubscriptionSync/IncomingAsyncSubscriptionEventToAnyModelMapper.swift +++ b/packages/amplify_datastore/ios/internal/AWSDataStorePlugin/Sync/SubscriptionSync/IncomingAsyncSubscriptionEventToAnyModelMapper.swift @@ -81,7 +81,7 @@ final class IncomingAsyncSubscriptionEventToAnyModelMapper: Subscriber, AmplifyC case .success(let mutationSync): modelsFromSubscription.send(.payload(mutationSync)) case .failure(let failure): - log.error(error: failure) + log.error(failure.errorDescription) } } diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index c9b7581e01..a7229909f8 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -38,14 +38,19 @@ APIAuthorizationType? nativeToApiAuthorizationType(String? authMode) { /// Convert a [GraphQLResponse] to a [NativeGraphQLResponse] NativeGraphQLResponse graphQLResponseToNativeResponse( GraphQLResponse response) { - final errorJson = - response.errors.whereNotNull().map((e) => e.toJson()).toList(); + final payload = _buildPayloadJson(response); + return NativeGraphQLResponse(payloadJson: payload); +} + +/// Build payloadJson for a [NativeGraphQLResponse] and [NativeGraphQLSubscriptionResponse] +/// from a [GraphQLResponse] +String _buildPayloadJson(GraphQLResponse response) { + final errors = response.errors.whereNotNull().map((e) => e.toJson()).toList(); final data = jsonDecode(response.data ?? '{}'); - final payload = jsonEncode({ + return jsonEncode({ 'data': data, - 'errors': errorJson, + 'errors': errors, }); - return NativeGraphQLResponse(payloadJson: payload); } /// Returns a connecting event [NativeGraphQLResponse] for the given [subscriptionId] @@ -75,16 +80,12 @@ void sendNativeStartAckEvent(String subscriptionId) { /// If the response has errors, the event type will be `error`, otherwise `data` void sendSubscriptionEvent( String subscriptionId, GraphQLResponse response) { - if (response.hasErrors) { - final errorPayload = jsonEncode({"errors": response.errors}); - sendNativeErrorEvent(subscriptionId, errorPayload); - return; - } + final payload = _buildPayloadJson(response); final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, - payloadJson: response.data, - type: "data", + payloadJson: payload, + type: response.hasErrors ? 'error' : "data", ); _sendSubscriptionEvent(event); } From f90476c62da03d73f7dff52bc31a3adfb2ebfed3 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Sun, 2 Jun 2024 10:02:27 -0500 Subject: [PATCH 17/20] fix: renamed helper --- packages/amplify_datastore/lib/amplify_datastore.dart | 2 +- .../amplify_datastore/lib/src/utils/native_api_helpers.dart | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/amplify_datastore/lib/amplify_datastore.dart b/packages/amplify_datastore/lib/amplify_datastore.dart index 95ebed3bc0..22e76d7c58 100644 --- a/packages/amplify_datastore/lib/amplify_datastore.dart +++ b/packages/amplify_datastore/lib/amplify_datastore.dart @@ -350,7 +350,7 @@ class _NativeAmplifyApi {'message': error.toString()} ] }); - sendNativeErrorEvent(flutterRequest.id, errorPayload); + sendSubscriptionStreamErrorEvent(flutterRequest.id, errorPayload); }, onDone: () => sendNativeCompleteEvent(flutterRequest.id)); diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index a7229909f8..7c54359a63 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -91,7 +91,8 @@ void sendSubscriptionEvent( } /// Send an error event for the given [subscriptionId] and [errorPayload] -void sendNativeErrorEvent(String subscriptionId, String? errorPayload) { +void sendSubscriptionStreamErrorEvent( + String subscriptionId, String? errorPayload) { final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, payloadJson: errorPayload, From 2a6973eb42ffa75b5192ea6c89a2e90664f68c2c Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Sun, 2 Jun 2024 22:50:33 -0500 Subject: [PATCH 18/20] fix: handle API exceptions that block initial sync --- .../lib/amplify_datastore.dart | 29 ++++++++------ .../lib/src/utils/native_api_helpers.dart | 38 ++++++++++++++++++- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/packages/amplify_datastore/lib/amplify_datastore.dart b/packages/amplify_datastore/lib/amplify_datastore.dart index 22e76d7c58..fa58a61dca 100644 --- a/packages/amplify_datastore/lib/amplify_datastore.dart +++ b/packages/amplify_datastore/lib/amplify_datastore.dart @@ -321,16 +321,26 @@ class _NativeAmplifyApi @override Future mutate(NativeGraphQLRequest request) async { - final flutterRequest = nativeRequestToGraphQLRequest(request); - final response = await Amplify.API.mutate(request: flutterRequest).response; - return graphQLResponseToNativeResponse(response); + try { + final flutterRequest = nativeRequestToGraphQLRequest(request); + final response = + await Amplify.API.mutate(request: flutterRequest).response; + return graphQLResponseToNativeResponse(response); + } on Exception catch (e) { + return handleGraphQLOperationException(e, request); + } } @override Future query(NativeGraphQLRequest request) async { - final flutterRequest = nativeRequestToGraphQLRequest(request); - final response = await Amplify.API.query(request: flutterRequest).response; - return graphQLResponseToNativeResponse(response); + try { + final flutterRequest = nativeRequestToGraphQLRequest(request); + final response = + await Amplify.API.query(request: flutterRequest).response; + return graphQLResponseToNativeResponse(response); + } on Exception catch (e) { + return handleGraphQLOperationException(e, request); + } } @override @@ -345,12 +355,7 @@ class _NativeAmplifyApi (GraphQLResponse event) => sendSubscriptionEvent(flutterRequest.id, event), onError: (Object error) { - final errorPayload = jsonEncode({ - 'errors': [ - {'message': error.toString()} - ] - }); - sendSubscriptionStreamErrorEvent(flutterRequest.id, errorPayload); + sendSubscriptionStreamErrorEvent(flutterRequest.id, error); }, onDone: () => sendNativeCompleteEvent(flutterRequest.id)); diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index 7c54359a63..dfb2d240d3 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -35,6 +35,40 @@ APIAuthorizationType? nativeToApiAuthorizationType(String? authMode) { } } +// TODO(equartey): Migrate string matching to use status codes when available on +// exceptions to more closely match the behavior of Amplify Swift. +// In addition to Unauthorized errors, Amplify Swift checks for status codes 401 & 403 for silent failures. +// https://github.com/aws-amplify/amplify-swift/blob/8534d75277701bb6cb9844cf66d1e2ef2a78c37e/AmplifyPlugins/API/Sources/AWSAPIPlugin/APIError%2BUnauthorized.swift#L41 +// +/// Transform a exception to an error payload json. +/// And tag the error to fail silently, allowing DataStore sync to continue. +String _transformExceptionToErrorPayloadJson(Object e) { + final _silentFailExceptions = ["SignedOutException"]; + + Map error = { + 'message': "${e.toString()}", + }; + if (e is AmplifyException) { + final isUnAuthorized = _silentFailExceptions + .any((x) => e.underlyingException?.toString().contains(x) ?? false); + // preface the error message with "Unauthorized" if the exception should be silent + error['message'] = isUnAuthorized + ? "Unauthorized - ${e.message} - ${e.underlyingException}" + : error['message']; + } + var errorPayload = { + 'errors': [error] + }; + return jsonEncode(errorPayload); +} + +/// Handle GraphQL operation Exceptions and return a [NativeGraphQLResponse] +NativeGraphQLResponse handleGraphQLOperationException( + Exception e, NativeGraphQLRequest request) { + final errorPayload = _transformExceptionToErrorPayloadJson(e); + return NativeGraphQLResponse(payloadJson: errorPayload); +} + /// Convert a [GraphQLResponse] to a [NativeGraphQLResponse] NativeGraphQLResponse graphQLResponseToNativeResponse( GraphQLResponse response) { @@ -91,8 +125,8 @@ void sendSubscriptionEvent( } /// Send an error event for the given [subscriptionId] and [errorPayload] -void sendSubscriptionStreamErrorEvent( - String subscriptionId, String? errorPayload) { +void sendSubscriptionStreamErrorEvent(String subscriptionId, Object e) { + final errorPayload = _transformExceptionToErrorPayloadJson(e); final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, payloadJson: errorPayload, From 77e8dabdd6ab1d965a0b8546bee47e7f85c529ab Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Mon, 3 Jun 2024 08:27:10 -0500 Subject: [PATCH 19/20] nit: a comment --- .../ios/Classes/api/GraphQLResponse+Decode.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift index 620ff31830..289fa1d2e6 100644 --- a/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift +++ b/packages/amplify_datastore/ios/Classes/api/GraphQLResponse+Decode.swift @@ -162,7 +162,8 @@ extension GraphQLResponse { return encodeDataPayloadToString(dataPayload).map { $0 as! R } } - /// This is included to allow multi-platform support. Requests that do not have `__typename` + /// This allows multi-platform support. Not all platform requests include `__typename` + /// in the selection set. This adds it to the response based on the model name for proper decoding. let dataPayloadWithTypeName = (dataPayload.asObject?.merging( ["__typename": .string(modelName)], uniquingKeysWith: { a, _ in a } From 86a6663db559e2469f0cf720526fac8cbb65a8e0 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Mon, 3 Jun 2024 11:05:41 -0500 Subject: [PATCH 20/20] fix: handle json payload exceptions --- .../lib/src/utils/native_api_helpers.dart | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart index dfb2d240d3..b59aceb856 100644 --- a/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart +++ b/packages/amplify_datastore/lib/src/utils/native_api_helpers.dart @@ -72,21 +72,38 @@ NativeGraphQLResponse handleGraphQLOperationException( /// Convert a [GraphQLResponse] to a [NativeGraphQLResponse] NativeGraphQLResponse graphQLResponseToNativeResponse( GraphQLResponse response) { - final payload = _buildPayloadJson(response); + var payload = ""; + try { + payload = _buildPayloadJson(response); + } on Exception catch (e) { + payload = _handlePayloadException(e); + } return NativeGraphQLResponse(payloadJson: payload); } /// Build payloadJson for a [NativeGraphQLResponse] and [NativeGraphQLSubscriptionResponse] /// from a [GraphQLResponse] String _buildPayloadJson(GraphQLResponse response) { - final errors = response.errors.whereNotNull().map((e) => e.toJson()).toList(); final data = jsonDecode(response.data ?? '{}'); + final errors = response.errors.whereNotNull().map((e) => e.toJson()).toList(); return jsonEncode({ 'data': data, 'errors': errors, }); } +/// Handle payload json parsing exceptions +String _handlePayloadException(Exception e) { + return jsonEncode({ + 'data': {}, + 'errors': [ + { + 'message': 'Error parsing payload json: ${e.toString()}', + } + ], + }); +} + /// Returns a connecting event [NativeGraphQLResponse] for the given [subscriptionId] NativeGraphQLSubscriptionResponse getConnectingEvent(String subscriptionId) { final event = NativeGraphQLSubscriptionResponse( @@ -114,12 +131,20 @@ void sendNativeStartAckEvent(String subscriptionId) { /// If the response has errors, the event type will be `error`, otherwise `data` void sendSubscriptionEvent( String subscriptionId, GraphQLResponse response) { - final payload = _buildPayloadJson(response); + var payload = ""; + var hasErrors = response.hasErrors; + + try { + payload = _buildPayloadJson(response); + } on Exception catch (e) { + payload = _handlePayloadException(e); + hasErrors = true; + } final event = NativeGraphQLSubscriptionResponse( subscriptionId: subscriptionId, payloadJson: payload, - type: response.hasErrors ? 'error' : "data", + type: hasErrors ? 'error' : "data", ); _sendSubscriptionEvent(event); }