-
Notifications
You must be signed in to change notification settings - Fork 259
chore: update configure apis to use AmplifyOutputs instead of AmplifyConfig #5017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
b60fb0a
9dc9c24
a9cb8f8
835ebfd
fb3c59c
7f458e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -111,24 +111,44 @@ abstract class AmplifyClass { | |||||
'Check if Amplify is already configured using Amplify.isConfigured.', | ||||||
); | ||||||
} | ||||||
|
||||||
late AmplifyConfig amplifyConfig; | ||||||
late final AmplifyOutputs amplifyOutputs; | ||||||
try { | ||||||
final Map<String, Object?> json; | ||||||
late final AmplifyConfig amplifyConfig; | ||||||
try { | ||||||
final json = jsonDecode(configuration) as Map; | ||||||
amplifyConfig = AmplifyConfig.fromJson(json.cast()); | ||||||
json = jsonDecode(configuration) as Map<String, Object?>; | ||||||
} on Object catch (e) { | ||||||
throw ConfigurationError( | ||||||
'The provided configuration is not a valid json. ' | ||||||
'Check underlyingException.', | ||||||
recoverySuggestion: | ||||||
'Inspect your amplifyconfiguration.dart and ensure that ' | ||||||
'the string is proper json', | ||||||
'Inspect your amplify_output.dart or amplifyconfiguration.dart ' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
'and ensure that the string is proper json', | ||||||
underlyingException: e, | ||||||
); | ||||||
} | ||||||
await _configurePlugins(amplifyConfig); | ||||||
_configCompleter.complete(amplifyConfig.toAmplifyOutputs()); | ||||||
try { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: The multiple try/catch blocks gets a little hard to follow. I would suggestion refactoring with small private functions. Future<void> configure(String configuration) async {
if (isConfigured) {
throw const AmplifyAlreadyConfiguredException(
'Amplify has already been configured and re-configuration is not supported.',
recoverySuggestion:
'Check if Amplify is already configured using Amplify.isConfigured.',
);
}
late final AmplifyOutputs amplifyOutputs;
try {
final json = _decodeJson(configuration);
amplifyOutputs = _parseJsonConfig(json);
await _configurePlugins(amplifyOutputs);
_configCompleter.complete(amplifyOutputs);
} on ConfigurationError catch (e, st) {
// Complete with the configuration error and reset the completer so
// that 1) `configure` can be called again and 2) listeners registered
// on `asyncConfig` are notified of the error so they can handle it as
// appropriate. For example, the Authenticator listens for `asyncConfig`
// to complete before updating the UI.
_configCompleter.completeError(e, st);
rethrow;
} on Object {
// At this point, configuration is complete in the sense that the
// configuration file has been validated and plugins have had their
// `configure` methods run with no `ConfigurationException`s.
//
// Any other errors which occur during plugin configuration should be
// handled by the developer, but since they are unrelated to
// configuration, listeners to `Amplify.asyncConfig` should be allowed to
// proceed with the validated configuration.
_configCompleter.complete(amplifyOutputs);
_configCompleter = Completer();
rethrow;
}
}
AmplifyOutputs _parseJsonConfig(Map<String, Object?> json) {
try {
return AmplifyOutputs.fromJson(json);
} on Object {
try {
final amplifyConfig = AmplifyConfig.fromJson(json);
return amplifyConfig.toAmplifyOutputs();
} on Object catch (e) {
throw ConfigurationError(
'The provided configuration can not be decoded to AmplifyOutputs '
'or AmplifyConfig. '
'Check underlyingException.',
recoverySuggestion:
'If using Amplify Gen 2 ensure that the json string '
'can be decoded to AmplifyOutputs type. '
'If using Amplify Gen 1 ensure that the json '
'string can be decoded to AmplifyConfig type.',
underlyingException: e,
);
}
}
}
Map<String, Object?> _decodeJson(String configuration) {
try {
return jsonDecode(configuration) as Map<String, Object?>;
} on Object catch (e) {
throw ConfigurationError(
'The provided configuration is not a valid json. '
'Check underlyingException.',
recoverySuggestion:
'Inspect your amplify_output.dart or amplifyconfiguration.dart '
'and ensure that the string is proper json',
underlyingException: e,
);
}
} |
||||||
amplifyOutputs = AmplifyOutputs.fromJson(json); | ||||||
} on Object { | ||||||
try { | ||||||
amplifyConfig = AmplifyConfig.fromJson(json); | ||||||
amplifyOutputs = amplifyConfig.toAmplifyOutputs(); | ||||||
} on Object catch (e) { | ||||||
throw ConfigurationError( | ||||||
'The provided configuration can not be decoded to AmplifyOutputs ' | ||||||
'or AmplifyConfig. ' | ||||||
'Check underlyingException.', | ||||||
recoverySuggestion: | ||||||
'If using Amplify Gen 2 ensure that the json string ' | ||||||
'can be decoded to AmplifyOutputs type. ' | ||||||
'If using Amplify Gen 1 ensure that the json ' | ||||||
'string can be decoded to AmplifyConfig type.', | ||||||
underlyingException: e, | ||||||
); | ||||||
} | ||||||
} | ||||||
await _configurePlugins(amplifyOutputs); | ||||||
_configCompleter.complete(amplifyOutputs); | ||||||
} on ConfigurationError catch (e, st) { | ||||||
// Complete with the configuration error and reset the completer so | ||||||
// that 1) `configure` can be called again and 2) listeners registered | ||||||
|
@@ -146,14 +166,14 @@ abstract class AmplifyClass { | |||||
// handled by the developer, but since they are unrelated to | ||||||
// configuration, listeners to `Amplify.asyncConfig` should be allowed to | ||||||
// proceed with the validated configuration. | ||||||
_configCompleter.complete(amplifyConfig.toAmplifyOutputs()); | ||||||
_configCompleter.complete(amplifyOutputs); | ||||||
_configCompleter = Completer(); | ||||||
rethrow; | ||||||
} | ||||||
} | ||||||
|
||||||
/// Configures all plugins in topologically-sorted order. | ||||||
Future<void> _configurePlugins(AmplifyConfig config) async { | ||||||
Future<void> _configurePlugins(AmplifyOutputs config) async { | ||||||
await Future.wait(_addPluginFutures); | ||||||
_addPluginFutures.clear(); | ||||||
final categories = <Category, AmplifyCategory>{ | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ class AmplifyOutputs | |
|
||
/// {@macro amplify_core.amplify_outputs.rest_api_outputs} | ||
@internal | ||
@JsonKey(includeToJson: false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: was there a particular reason to make this change? I think this change is fine, but am curious about what the motivation was. I had considered mvoing this change when I added rest API but decided not to because I thought it might be weird if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do Amplify iOS and/or Android consider the config invalid when there are extra key/value pairs? That seems potentially problematic since new key/value pairs would generally be considered non breaking. |
||
final Map<String, RestApiOutputs>? restApi; | ||
|
||
/// {@macro amplify_core.amplify_outputs.notifications_outputs} | ||
|
@@ -119,6 +120,6 @@ Object? _dataToJson(Map<String, DataOutputs>? outputs) { | |
' Amplify Outputs does not support multiple GraphQL endpoints.', | ||
); | ||
} | ||
final data = outputs[_dataPluginName]; | ||
final data = outputs.values.firstOrNull; | ||
return data?.toJson(); | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,9 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import 'package:amplify_core/amplify_core.dart'; | ||
import 'package:amplify_core/src/config/amplify_outputs/auth/auth_outputs.dart'; | ||
import 'package:json_annotation/json_annotation.dart'; | ||
import 'package:meta/meta.dart'; | ||
|
||
part 'oauth.g.dart'; | ||
|
||
|
@@ -27,6 +29,46 @@ class CognitoOAuthConfig | |
this.tokenUriQueryParameters, | ||
}); | ||
|
||
@internal | ||
factory CognitoOAuthConfig.fromAuthOutputs(AuthOutputs authOutputs) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P.S. this used in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a follow up task to remove use of |
||
if (authOutputs.userPoolClientId == null) { | ||
throw ConfigurationError('Invalid config: no User Pool Client Id found.'); | ||
} | ||
if (authOutputs.oauth == null) { | ||
throw ConfigurationError('Invalid config: no oAuth configuration found.'); | ||
} | ||
|
||
final appClientId = authOutputs.userPoolClientId!; | ||
final appClientSecret = authOutputs.appClientSecret; | ||
final scopes = authOutputs.oauth!.scopes; | ||
final signInUri = authOutputs.oauth!.signInUri; | ||
final signOutUri = authOutputs.oauth!.signOutUri; | ||
final signInUriQueryParameters = | ||
authOutputs.oauth!.signInUriQueryParameters; | ||
final signOutUriQueryParameters = | ||
authOutputs.oauth!.signOutUriQueryParameters; | ||
final signInRedirectUri = authOutputs.oauth!.redirectSignInUri.join(','); | ||
final signOutRedirectUri = authOutputs.oauth!.redirectSignOutUri.join(','); | ||
final webDomain = authOutputs.oauth!.domain; | ||
final tokenUri = authOutputs.oauth!.tokenUri; | ||
final tokenUriQueryParameters = authOutputs.oauth!.tokenUriQueryParameters; | ||
|
||
return CognitoOAuthConfig( | ||
appClientId: appClientId, | ||
appClientSecret: appClientSecret, | ||
scopes: scopes, | ||
signInUri: signInUri, | ||
signOutUri: signOutUri, | ||
signInRedirectUri: signInRedirectUri, | ||
signOutRedirectUri: signOutRedirectUri, | ||
webDomain: webDomain, | ||
signInUriQueryParameters: signInUriQueryParameters, | ||
signOutUriQueryParameters: signOutUriQueryParameters, | ||
tokenUri: tokenUri, | ||
tokenUriQueryParameters: tokenUriQueryParameters, | ||
); | ||
} | ||
|
||
factory CognitoOAuthConfig.fromJson(Map<String, Object?> json) => | ||
_$CognitoOAuthConfigFromJson(json); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import 'package:amplify_core/amplify_core.dart'; | ||
import 'package:amplify_core/src/config/amplify_outputs/auth/auth_outputs.dart'; | ||
import 'package:meta/meta.dart'; | ||
|
||
part 'user_pool.g.dart'; | ||
|
||
|
@@ -17,6 +19,29 @@ class CognitoUserPoolConfig | |
this.endpoint, | ||
}); | ||
|
||
@internal | ||
factory CognitoUserPoolConfig.fromAuthOutputs(AuthOutputs authOutputs) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P.S. this used in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as above. Do we have a follow up task to remove use of this? |
||
if (authOutputs.userPoolId == null) { | ||
throw ConfigurationError( | ||
'Invalid Cognito User Pool config: No User Pool Id found', | ||
); | ||
} | ||
if (authOutputs.userPoolClientId == null) { | ||
throw ConfigurationError( | ||
'Invalid Cognito User Pool config: No User Pool Client Id found', | ||
); | ||
} | ||
return CognitoUserPoolConfig( | ||
poolId: authOutputs.userPoolId!, | ||
appClientId: authOutputs.userPoolClientId!, | ||
appClientSecret: authOutputs.appClientSecret, | ||
region: authOutputs.awsRegion, | ||
hostedUI: authOutputs.oauth == null | ||
? null | ||
: CognitoOAuthConfig.fromAuthOutputs(authOutputs), | ||
); | ||
} | ||
|
||
factory CognitoUserPoolConfig.fromJson(Map<String, Object?> json) => | ||
_$CognitoUserPoolConfigFromJson(json); | ||
final String poolId; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
import 'package:amplify_core/amplify_core.dart'; | ||
import 'package:amplify_core/src/config/amplify_outputs/notifications/notifications_outputs.dart'; | ||
|
||
/// {@template amplify_core.push.service_provider_client} | ||
/// A base class for new service providers to implement and add functionality | ||
|
@@ -11,7 +12,7 @@ import 'package:amplify_core/amplify_core.dart'; | |
abstract class ServiceProviderClient { | ||
/// Initialize this client, used by the plugin during configuration. | ||
Future<void> init({ | ||
required NotificationsPinpointPluginConfig config, | ||
required NotificationsOutputs config, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Is this an internal type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, it is not exporeted. |
||
required AmplifyAuthProviderRepository authProviderRepo, | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,10 +73,10 @@ android { | |
} | ||
|
||
dependencies { | ||
implementation 'com.amplifyframework:aws-auth-cognito:2.15.0' | ||
implementation "com.amplifyframework:aws-api:2.15.0" | ||
implementation "com.amplifyframework:aws-datastore:2.15.0" | ||
implementation "com.amplifyframework:aws-api-appsync:2.15.0" | ||
implementation 'com.amplifyframework:aws-auth-cognito:2.19.1' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Should this go to main in a separate PR? |
||
implementation "com.amplifyframework:aws-api:2.19.1" | ||
implementation "com.amplifyframework:aws-datastore:2.19.1" | ||
implementation "com.amplifyframework:aws-api-appsync:2.19.1" | ||
implementation 'com.google.code.gson:gson:2.10.1' | ||
implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1' | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Can this be moved to where it is defined and
late
be removed?