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

feat(amplify_flutter): allow customers to override AmplifyClass methods #1325

Merged

Conversation

Jordan-Nelson
Copy link
Member

Issue #, if available: n/a

Goal:

  • Allow Amplify Auth to be stubbed for use in the browser, so that a live Amplify Authenticator demo (with a stubbed backend) can be imbedded in the docs website.

Issue description:

AmplifyClass has a setter for the private _instance field. This allows a customer to pass in their own AmplifyClass implementation. However, the instance they pass in isn't used (except in one location, where a private method is invoked, and private methods can't be overridden from outside the lib anyway). This means that customers can technically create their own AmplifyClass implementation, but they can't override any of the methods, which defeats the whole point.

Description of changes:

  • Move all implementations from AmplifyClass to MethodChannelAmplify.
    • Note: I think it is debatable if all of the methods belong in the method channel class. We could create a new class, which would be the default implementation of AmplifyClass, and this class could call through to the method channel class when needed. The pattern of putting all of the implementations in the method channel class is consistent with the other plugins though. I think it is worth revisiting this possibly in the future, but for now we just move them all over.
  • Replace all implementation with calls to the appropriate method on the instance

Example:

Here is an example where AmplifyClass is stubbed for the purpose of running a (mocked) demo of the Amplify Authenticator in the browser. The implementation here is similar to the implementation in the real class. The key difference is that the method channel is never invoked. This is important because the method channel will not be available in the browser.

A similar stub could be used to allow Amplify.configure() and Amplify.addPlugin() to be called from within a widget/unit tests. Right now those methods can only be called from within integ tests. We had an issue opened in the past related to this.

This also would in theory allow a custom to write their own implementation for a platform we don't currently support, although I think that is an unlikely use case.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Jordan-Nelson Jordan-Nelson requested a review from a team as a code owner February 2, 2022 18:50
@@ -28,7 +31,6 @@ export 'src/config/api/api_config.dart';
export 'src/config/auth/auth_config.dart';
export 'src/config/config_map.dart';
export 'src/config/storage/storage_config.dart';

// Utilities
export 'src/utils/equatable.dart';
export 'src/utils/json.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't leave a comment on the line for some reason but this:

/// Top level singleton Amplify object.
final AmplifyClass Amplify = AmplifyClass.protected();

should become this:

/// Top level singleton Amplify object.
AmplifyClass get Amplify => AmplifyClass.instance;

Copy link
Contributor

Choose a reason for hiding this comment

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

Also maybe a setter as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call. Updated and added a setter.

@Jordan-Nelson Jordan-Nelson merged commit 40751c0 into aws-amplify:main Feb 2, 2022
@Jordan-Nelson Jordan-Nelson deleted the feat/configure-platform-override branch February 2, 2022 23:06
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.

3 participants