aws-amplify / amplify-flutter

A declarative library with an easy-to-use interface for building Flutter applications on AWS.
https://docs.amplify.aws
Apache License 2.0
1.31k stars 242 forks source link

Push Notification Configuration fails (on Android only) when multiple API endpoints are present #5423

Open bitsplash-andy opened 1 week ago

bitsplash-andy commented 1 week ago

Description

I am unable to configure push notifications on Android using version 2.4.1 of all plugins when having two GraphQL APIs defined in the config. All functionality works as expected on iOS.

It looks as though the push notifications code has been updated to use Gen2 Amplify Outputs via: https://github.com/aws-amplify/amplify-flutter/pull/5073

There is a conditional platform statement on line 233 which encodes the config for storage in the SecureStorage.

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/notifications/push/amplify_push_notifications/lib/src/amplify_push_notifications_impl.dart#L233

This causes the following exception to be thrown

Converting object to an encodable object failed: Instance of 'AmplifyOutputs'
#0      _JsonStringifier.writeObject (dart:convert/json.dart:793:7)
#1      _JsonStringStringifier.printOn (dart:convert/json.dart:982:17)
#2      _JsonStringStringifier.stringify (dart:convert/json.dart:967:5)
#3      JsonEncoder.convert (dart:convert/json.dart:345:30)
#4      JsonCodec.encode (dart:convert/json.dart:231:45)
#5      jsonEncode (dart:convert/json.dart:114:10)
#6      AmplifyPushNotifications.configure (package:amplify_push_notifications/src/amplify_push_notifications_impl.dart:238:16)

The cause of this is:

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/amplify_core/lib/src/config/amplify_outputs/amplify_outputs.dart#L111C1-L125C2

As I say, this breaks Push Notifications on Android only when the API config contains more than one endpoint. All other categories function as expected if the Push Notification plugin is removed, and there are no issues on iOS even when plugin is present.

I'm assuming more things may break further down the line as the Gen2 code is adopted into more packages, but it's frustrating to have such an edge case appear after migrating the codebase to the latest version.

Categories

Steps to Reproduce

Add more then one API endpoint to the configuration

add AmplifyPushNotificationsPinpoint() via Amplify.addPlugins call Amplify.configure

Screenshots

No response

Platforms

Flutter Version

3.24.1

Amplify Flutter Version

2.4.1

Deployment Method

Custom Pipeline

Schema

No response

bitsplash-andy commented 1 week ago

Rolling back to version 2.1.0 prevents issue, but is far from ideal.

Need to understand why AmplifyOutputs contains the following:

/// Converts a map of [DataOutputs] to a single data json object.
///
/// This manual mapping is required since the Amplify Outputs schema only supports
/// a single data object, but Amplify Flutter supports more than 1.
Object? _dataToJson(Map<String, DataOutputs>? outputs) {
  if (outputs == null) return null;
  if (outputs.length > 1) {
    throw ConfigurationError(
      'Found ${outputs.length} endpoints.'
      ' Amplify Outputs does not support multiple GraphQL endpoints.',
    );
  }
  final data = outputs.values.firstOrNull;
  return data?.toJson();
}

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/amplify_core/lib/src/config/amplify_outputs/amplify_outputs.dart#L111C1-L125C2

and

/// Converts a single data json object to a map of [DataOutputs].
///
/// This manual mapping is required since the Amplify Outputs schema only supports
/// a single data object, but Amplify Flutter supports more than 1.
Map<String, DataOutputs>? _dataFromJson(Map<String, Object?>? object) {
  if (object == null) return null;
  return {
    _dataPluginName: DataOutputs.fromJson(object),
  };
}
bitsplash-andy commented 1 week ago

Failing a change to AmplifyOutputs, perhaps the following should be modified to use a custom JSON encoder to prevent the incompatibility for now:

await _amplifySecureStorage.write(
  key: configSecureStorageKey,
  value: jsonEncode(config),
);

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/notifications/push/amplify_push_notifications/lib/src/amplify_push_notifications_impl.dart#L238

and

final amplifyconfigStr = await amplifySecureStorage.read(
  key: configSecureStorageKey,
);

https://github.com/aws-amplify/amplify-flutter/blob/6aaf966e7907cba71b9d2cf6f84ae2dc6e4d50b0/packages/notifications/push/amplify_push_notifications_pinpoint/lib/src/push_notifications_background_processing.dart#L35

NikaHsn commented 1 week ago

Sorry that you are facing this issue and thanks for reporting it. We will look into this and get back to you when we have updates.

NikaHsn commented 1 week ago

@bitsplash-andy The Amplify CLI (both Generations 1 and 2) does not support multiple GraphQL API endpoints. Hence, the Amplify Flutter libraries are designed to handle configurations with only one GraphQL API endpoint. While the AmplifyOutputs type allows for multiple data endpoints, this is primarily for internal implementations and backwards compatibility. When using custom pipeline for deployment please make sure the amplify configuration is a valid josn for this schema.

bitsplash-andy commented 1 week ago

Hi @NikaHsn, thanks for your reply.

To clarify, we don’t use the Amplify CLI, everything is provisioned manually via CDK etc. We only use the client library.

The client library has supported multiple API endpoints without issue up until version 2.2.0, at which point this began failing in the push notification library as described above.

for example the following :

"api": {
      "plugins": {
          "awsAPIPlugin": {
              "Authenticated": {
                  "endpointType": "GraphQL",
                  "endpoint": "ENDPOINT_URL",
                  "region": "eu-west-2",
                  "authorizationType": "AMAZON_COGNITO_USER_POOLS"
              },
              "Unauthenticated": {
                  "endpointType": "GraphQL",
                  "endpoint": "ENDPOINT_URL",
                  "region": "eu-west-2",
                  "authorizationType": "API_KEY",
                  "apiKey": "KEY"
              }
          }
      }
    },

can be switched at runtime via

  final request = GraphQLRequest(
    apiName: 'Authenticated',
    document: '''query someQuery {}''',
  );

or

  final request = GraphQLRequest(
    apiName: 'Unauthenticated',
    document: '''query someQuery {}''',
  );

Docs here

Multiple API endpoint calls continue to work as expected if the push notifications plugin is disabled, but the enforced single endpoint schema on configuration serialisation described above causes the push notification plugin to throw an exception on initialisation if multiple API endpoints are present. As the configuration cannot be specified on a plugin basis, this means that the choice appears to be stay on version 2.1.0 or loose the ability to enable the push notifications plugin in newer releases.

Additionally, this only happens on Android. From a library consumption perspective for users that want to use existing, non-Amplify provisioned backends this is a real issue.

bitsplash-andy commented 1 week ago

Apologies, I've updated my previous comment a few times for clarity.

NikaHsn commented 1 week ago

thanks @bitsplash-andy for clarification. we will look into this and get back to you when we have update.