Skip to content
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

refactor(auth)!: Plugin options #2691

Merged
merged 20 commits into from Mar 17, 2023
Merged

refactor(auth)!: Plugin options #2691

merged 20 commits into from Mar 17, 2023

Conversation

dnys1
Copy link
Contributor

@dnys1 dnys1 commented Feb 22, 2023

Migrates all Auth APIs to new plugin options

Dillon Nys added 6 commits March 13, 2023 14:03
Migrates `Amplify.Auth.signIn` to use new plugin options.
Migrates `Amplify.Auth.signUp` to use new plugin options.
Migrates `Amplify.Auth.confirmSignUp` to use new plugin options.
Migrates `Amplify.Auth.resendSignUpCode` to use new plugin options.
Migrates `Amplify.Auth.confirmSignIn` to use new plugin options.
Migrates `Amplify.Auth.signInWithWebUI` to use new plugin options.
@dnys1 dnys1 changed the title chore(auth): PoC of plugin options refactor(auth)!: Plugin options Mar 13, 2023
Dillon Nys added 11 commits March 13, 2023 14:28
Migrates `Amplify.Auth.signOut` to use new plugin options.
Migrates `Amplify.Auth.fetchUserAttributes` to use new plugin options.
Migrates `Amplify.Auth.updateUserAttribute` to use new plugin options.
Migrates `Amplify.Auth.updateUserAttributes` to use new plugin options.
Migrates `Amplify.Auth.confirmUserAttribute` to use new plugin options.
Migrates `Amplify.Auth.resendUserAttributeConfirmationCode` to use new plugin options.
Migrates `Amplify.Auth.updatePassword` to use new plugin options.
Migrates `Amplify.Auth.resetPassword` to use new plugin options.
Migrates `Amplify.Auth.confirmResetPassword` to use new plugin options.
Migrates `Amplify.Auth.getCurrentUser` to use new plugin options.
Migrates `Amplify.Auth.fetchAuthSession` to use new plugin options.
@dnys1 dnys1 marked this pull request as ready for review March 13, 2023 21:29
@dnys1 dnys1 requested a review from a team as a code owner March 13, 2023 21:29
@@ -98,39 +81,40 @@ class AmplifyAuthCognitoDart extends AuthPluginInterface<
CognitoUserAttributeKey,
AuthUserAttribute<CognitoUserAttributeKey>,
CognitoDevice,
CognitoSignUpOptions,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So when are not going to strongly type pluginOptions when using Auth.getPlugin(AuthPluginKey)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Comment on lines 9 to 59
abstract class ConfirmUserAttributeOptions
with AWSSerializable<Map<String, Object?>> {
with
AWSEquatable<ConfirmUserAttributeOptions>,
AWSSerializable<Map<String, Object?>>,
AWSDebuggable {
/// {@macro amplify_core.types.auth.confirm_user_attribute_options}
const ConfirmUserAttributeOptions();
const factory ConfirmUserAttributeOptions({
ConfirmUserAttributePluginOptions? pluginOptions,
}) = _ConfirmUserAttributeOptions;

/// Base constructor for subclassing.
const ConfirmUserAttributeOptions.base();

/// {@macro amplify_core.auth.confirm_user_attribute_plugin_options}
ConfirmUserAttributePluginOptions? get pluginOptions;

@override
List<Object?> get props => [pluginOptions];

@Deprecated('Use toJson instead')
Map<String, Object?> serializeAsMap() => toJson();

@override
Map<String, Object?> toJson() => {
'pluginOptions': pluginOptions?.toJson(),
};
}

class _ConfirmUserAttributeOptions extends ConfirmUserAttributeOptions {
const _ConfirmUserAttributeOptions({
this.pluginOptions,
}) : super.base();

@override
final ConfirmUserAttributePluginOptions? pluginOptions;

@override
String get runtimeTypeName => 'ConfirmUserAttributeOptions';
}

/// {@template amplify_core.auth.confirm_user_attribute_plugin_options}
/// Plugin-specific options for `Amplify.Auth.confirmUserAttribute`.
/// {@endtemplate}
abstract class ConfirmUserAttributePluginOptions
with
AWSEquatable<ConfirmUserAttributePluginOptions>,
AWSSerializable<Map<String, Object?>>,
AWSDebuggable {
/// {@macro amplify_core.auth.confirm_user_attribute_plugin_options}
const ConfirmUserAttributePluginOptions();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the implementation of Options and PluginOptions seems different than API change review's approved proposal. here we are using abstract classes and typed pluginOptions instead of using generics. am I missing something here ? @HuiSF wdyt?

Make options type non-abstract classes and keep plugin options as abstract classes.
/// Cognito options for `Amplify.Auth.resendUserAttributeConfirmationCode`.
/// {@endtemplate}
@zAmplifySerializable
class CognitoResendUserAttributeConfirmationCodePluginOptions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about to move PluginOptions classes to their own file so that they follow the class -> file naming convention. I mean instead of having CognitoResendUserAttributeConfirmationCodePluginOptions defined in cognito_resend_user_attribute_confirmation_code_options.dart it will be defined in cognito_resend_user_attribute_confirmation_code_plugin_options.dart

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm do you think we should do the same for the category plugin options as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so bc they are abstract classes and used only by category level options and so it is okay to have them inside the options file.

@dnys1 dnys1 merged commit b164ba6 into next Mar 17, 2023
68 of 71 checks passed
@dnys1 dnys1 deleted the feat/auth/plugin-options branch March 17, 2023 21:19
dnys1 pushed a commit that referenced this pull request Mar 22, 2023
### Breaking Changes
- chore(datastore)!: Reorganize + import cleanup ([#2760](#2760))
- feat(secure_storage)!: Use Local App Data Directory on Windows ([#2744](#2744))
- refactor(auth)!: Plugin options ([#2691](#2691))

### Fixes
- fix(analytics): event retry logic & max fail tries ([#2713](#2713))
- fix(android): Bump Amplify Android to 2.4.1
- fix(auth): Always pass client metadata
- fix(authenticator): Hub event processing
- fix(authenticator): Verify attribute state
- fix(aws_common): Explicitly serialize values
- fix(core): Refine `toJson` outputs when `createFactory = false`
- fix(datastore): support use of java8 features in the example App
- fix(db_common): Windows build
- fix(ios): Bump Amplify iOS to 1.29.1
- fix(secure_storage): Only init Windows config on Windows ([#2751](#2751))
- fix(smithy): Handle uncaught exception ([#2720](#2720))

### Features
- feat(authenticator): Add AutoFill Capabilities ([#2306](#2306))

Updated-Components: aws_common, smithy, Amplify Flutter, Amplify Dart, Amplify UI, DB Common, Secure Storage
dnys1 pushed a commit that referenced this pull request Mar 22, 2023
### Breaking Changes
- chore(datastore)!: Reorganize + import cleanup ([#2760](#2760))
- feat(secure_storage)!: Use Local App Data Directory on Windows ([#2744](#2744))
- refactor(auth)!: Plugin options ([#2691](#2691))

### Fixes
- fix(analytics): event retry logic & max fail tries ([#2713](#2713))
- fix(android): Bump Amplify Android to 2.4.1
- fix(auth): Always pass client metadata
- fix(authenticator): Hub event processing
- fix(authenticator): Verify attribute state
- fix(aws_common): Explicitly serialize values
- fix(core): Refine `toJson` outputs when `createFactory = false`
- fix(datastore): support use of java8 features in the example App
- fix(db_common): Windows build
- fix(ios): Bump Amplify iOS to 1.29.1
- fix(secure_storage): Only init Windows config on Windows ([#2751](#2751))
- fix(smithy): Handle uncaught exception ([#2720](#2720))

### Features
- feat(authenticator): Add AutoFill Capabilities ([#2306](#2306))

Updated-Components: aws_common, smithy, Amplify Flutter, Amplify Dart, Amplify UI, DB Common, Secure Storage
dnys1 pushed a commit that referenced this pull request Mar 24, 2023
Clean up from #2691. Moves plugin options to their own files.
dnys1 pushed a commit that referenced this pull request Mar 24, 2023
Clean up from #2691. Moves plugin options to their own files.
dnys1 pushed a commit that referenced this pull request Mar 24, 2023
Clean up from #2691. Moves plugin options to their own files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants