Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_sign_in] Port plugin to use the federated Platform Interface #2266

Merged
merged 5 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/google_sign_in/google_sign_in/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 4.0.14

* Port plugin code to use the federated Platform Interface, instead of a MethodChannel directly.

## 4.0.13

* Fix `GoogleUserCircleAvatar` to handle new style profile image URLs.
Expand Down
130 changes: 64 additions & 66 deletions packages/google_sign_in/google_sign_in/lib/google_sign_in.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,36 @@
import 'dart:async';
import 'dart:ui' show hashValues;

import 'package:flutter/services.dart' show MethodChannel, PlatformException;
import 'package:meta/meta.dart' show visibleForTesting;
import 'package:flutter/services.dart' show PlatformException;
import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart';

import 'src/common.dart';

export 'src/common.dart';
export 'widgets.dart';

enum SignInOption { standard, games }

class GoogleSignInAuthentication {
GoogleSignInAuthentication._(this._data);

final Map<String, dynamic> _data;
final GoogleSignInTokenData _data;

/// An OpenID Connect ID token that identifies the user.
String get idToken => _data['idToken'];
String get idToken => _data.idToken;

/// The OAuth2 access token to access Google services.
String get accessToken => _data['accessToken'];
String get accessToken => _data.accessToken;

@override
String toString() => 'GoogleSignInAuthentication:$_data';
}

class GoogleSignInAccount implements GoogleIdentity {
GoogleSignInAccount._(this._googleSignIn, Map<String, dynamic> data)
: displayName = data['displayName'],
email = data['email'],
id = data['id'],
photoUrl = data['photoUrl'],
_idToken = data['idToken'] {
GoogleSignInAccount._(this._googleSignIn, GoogleSignInUserData data)
: displayName = data.displayName,
email = data.email,
id = data.id,
photoUrl = data.photoUrl,
_idToken = data.idToken {
assert(id != null);
}

Expand Down Expand Up @@ -78,18 +76,16 @@ class GoogleSignInAccount implements GoogleIdentity {
throw StateError('User is no longer signed in.');
}

final Map<String, dynamic> response =
await GoogleSignIn.channel.invokeMapMethod<String, dynamic>(
'getTokens',
<String, dynamic>{
'email': email,
'shouldRecoverAuth': true,
},
final GoogleSignInTokenData response =
await GoogleSignInPlatform.instance.getTokens(
email: email,
shouldRecoverAuth: true,
);

// On Android, there isn't an API for refreshing the idToken, so re-use
// the one we obtained on login.
if (response['idToken'] == null) {
response['idToken'] = _idToken;
if (response.idToken == null) {
response.idToken = _idToken;
}
return GoogleSignInAuthentication._(response);
}
Expand All @@ -108,10 +104,7 @@ class GoogleSignInAccount implements GoogleIdentity {
/// this method and grab `authHeaders` once again.
Future<void> clearAuthCache() async {
final String token = (await authentication).accessToken;
await GoogleSignIn.channel.invokeMethod<void>(
'clearAuthCache',
<String, dynamic>{'token': token},
);
await GoogleSignInPlatform.instance.clearAuthCache(token: token);
}

@override
Expand Down Expand Up @@ -146,7 +139,7 @@ class GoogleSignIn {
/// Initializes global sign-in configuration settings.
///
/// The [signInOption] determines the user experience. [SigninOption.games]
/// must not be used on iOS.
/// must not be used on iOS or web.
ditman marked this conversation as resolved.
Show resolved Hide resolved
///
/// The list of [scopes] are OAuth scope codes to request when signing in.
/// These scope codes will determine the level of data access that is granted
Expand All @@ -157,18 +150,25 @@ class GoogleSignIn {
/// The [hostedDomain] argument specifies a hosted domain restriction. By
/// setting this, sign in will be restricted to accounts of the user in the
/// specified domain. By default, the list of accounts will not be restricted.
GoogleSignIn({this.signInOption, this.scopes, this.hostedDomain});
GoogleSignIn({
this.signInOption = SignInOption.standard,
this.scopes = const <String>[],
this.hostedDomain,
});

/// Factory for creating default sign in user experience.
factory GoogleSignIn.standard({List<String> scopes, String hostedDomain}) {
factory GoogleSignIn.standard({
List<String> scopes = const <String>[],
String hostedDomain,
}) {
return GoogleSignIn(
signInOption: SignInOption.standard,
scopes: scopes,
hostedDomain: hostedDomain);
}

/// Factory for creating sign in suitable for games. This option must not be
/// used on iOS because the games API is not supported.
/// used on iOS or web because the games API is not supported.
ditman marked this conversation as resolved.
Show resolved Hide resolved
factory GoogleSignIn.games() {
return GoogleSignIn(signInOption: SignInOption.games);
}
Expand All @@ -186,13 +186,8 @@ class GoogleSignIn {
/// Error code indicating that attempt to sign in failed.
static const String kSignInFailedError = 'sign_in_failed';

/// The [MethodChannel] over which this class communicates.
@visibleForTesting
static const MethodChannel channel =
MethodChannel('plugins.flutter.io/google_sign_in');

/// Option to determine the sign in user experience. [SignInOption.games] must
/// not be used on iOS.
/// not be used on iOS or web.
ditman marked this conversation as resolved.
Show resolved Hide resolved
final SignInOption signInOption;

/// The list of [scopes] are OAuth scope codes requested when signing in.
Expand All @@ -211,12 +206,12 @@ class GoogleSignIn {
// Future that completes when we've finished calling `init` on the native side
Future<void> _initialization;

Future<GoogleSignInAccount> _callMethod(String method) async {
Future<GoogleSignInAccount> _callMethod(Function method) async {
await _ensureInitialized();

final Map<String, dynamic> response =
await channel.invokeMapMethod<String, dynamic>(method);
return _setCurrentUser(response != null && response.isNotEmpty
final dynamic response = await method();
Copy link
Contributor

Choose a reason for hiding this comment

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

what would happen if the method is not an async function.

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! It seems to work, though, check this DartPad!


return _setCurrentUser(response != null && response is GoogleSignInUserData
? GoogleSignInAccount._(this, response)
: null);
}
Expand All @@ -230,16 +225,14 @@ class GoogleSignIn {
}

Future<void> _ensureInitialized() {
return _initialization ??=
channel.invokeMethod<void>('init', <String, dynamic>{
'signInOption': (signInOption ?? SignInOption.standard).toString(),
'scopes': scopes ?? <String>[],
'hostedDomain': hostedDomain,
})
..catchError((dynamic _) {
// Invalidate initialization if it errored out.
_initialization = null;
});
return _initialization ??= GoogleSignInPlatform.instance.init(
signInOption: signInOption,
scopes: scopes,
hostedDomain: hostedDomain,
)..catchError((dynamic _) {
// Invalidate initialization if it errors out.
_initialization = null;
});
}

/// The most recently scheduled method call.
Expand All @@ -251,6 +244,7 @@ class GoogleSignIn {
final Completer<void> completer = Completer<void>();
future.whenComplete(completer.complete).catchError((dynamic _) {
// Ignore if previous call completed with an error.
// TODO: Should we log errors here, if debug or similar?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. let's log an error here. Personally, I think we should be clear about things like this error is completely harmless and it is logged for debug purpose only.
@mehmetf I saw that this is added in 4eeb744
Did you recall why did we ignore this error?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll revisit this to add logging, so you guys can discuss how much of it we need :)

(I'd recommend that this gets logged at the lowest level that makes sense, like trace or similar)

});
return completer.future;
}
Expand All @@ -259,26 +253,25 @@ class GoogleSignIn {
///
/// At most one in flight call is allowed to prevent concurrent (out of order)
/// updates to [currentUser] and [onCurrentUserChanged].
Future<GoogleSignInAccount> _addMethodCall(String method) async {
Future<GoogleSignInAccount> _addMethodCall(
ditman marked this conversation as resolved.
Show resolved Hide resolved
Function method, {
bool canSkipCall = false,
}) async {
Future<GoogleSignInAccount> response;
if (_lastMethodCall == null) {
response = _callMethod(method);
} else {
response = _lastMethodCall.then((_) {
// If after the last completed call `currentUser` is not `null` and requested
// method is a sign in method, re-use the same authenticated user
// method can be skipped (`canSkipCall`), re-use the same authenticated user
// instead of making extra call to the native side.
const List<String> kSignInMethods = <String>[
'signIn',
'signInSilently'
];
if (kSignInMethods.contains(method) && _currentUser != null) {
if (canSkipCall && _currentUser != null) {
return _currentUser;
} else {
return _callMethod(method);
}
return _callMethod(method);
});
}
// Add the current response to the currently running Promise of all pending responses
_lastMethodCall = _waitFor(response);
return response;
}
Expand All @@ -303,10 +296,12 @@ class GoogleSignIn {
/// returned Future completes with [PlatformException] whose `code` can be
/// either [kSignInRequiredError] (when there is no authenticated user) or
/// [kSignInFailedError] (when an unknown error occurred).
Future<GoogleSignInAccount> signInSilently(
{bool suppressErrors = true}) async {
Future<GoogleSignInAccount> signInSilently({
bool suppressErrors = true,
}) async {
try {
return await _addMethodCall('signInSilently');
return await _addMethodCall(GoogleSignInPlatform.instance.signInSilently,
canSkipCall: true);
} catch (_) {
if (suppressErrors) {
return null;
Expand All @@ -319,7 +314,7 @@ class GoogleSignIn {
/// Returns a future that resolves to whether a user is currently signed in.
Future<bool> isSignedIn() async {
await _ensureInitialized();
return await channel.invokeMethod<bool>('isSignedIn');
return GoogleSignInPlatform.instance.isSignedIn();
}

/// Starts the interactive sign-in process.
Expand All @@ -333,16 +328,19 @@ class GoogleSignIn {
///
/// Re-authentication can be triggered only after [signOut] or [disconnect].
Future<GoogleSignInAccount> signIn() {
final Future<GoogleSignInAccount> result = _addMethodCall('signIn');
final Future<GoogleSignInAccount> result =
_addMethodCall(GoogleSignInPlatform.instance.signIn, canSkipCall: true);
bool isCanceled(dynamic error) =>
error is PlatformException && error.code == kSignInCanceledError;
return result.catchError((dynamic _) => null, test: isCanceled);
}

/// Marks current user as being in the signed out state.
Future<GoogleSignInAccount> signOut() => _addMethodCall('signOut');
Future<GoogleSignInAccount> signOut() =>
_addMethodCall(GoogleSignInPlatform.instance.signOut);

/// Disconnects the current user from the app and revokes previous
/// authentication.
Future<GoogleSignInAccount> disconnect() => _addMethodCall('disconnect');
Future<GoogleSignInAccount> disconnect() =>
_addMethodCall(GoogleSignInPlatform.instance.disconnect);
}
3 changes: 2 additions & 1 deletion packages/google_sign_in/google_sign_in/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
for signing in with a Google account on Android and iOS.
author: Flutter Team <flutter-dev@googlegroups.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in/google_sign_in
version: 4.0.13
version: 4.0.14

flutter:
plugin:
Expand All @@ -12,6 +12,7 @@ flutter:
pluginClass: GoogleSignInPlugin

dependencies:
google_sign_in_platform_interface: ^1.0.0
flutter:
sdk: flutter
meta: ^1.0.4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';

import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:google_sign_in_platform_interface/google_sign_in_platform_interface.dart';
import 'package:google_sign_in/google_sign_in.dart';
import 'package:google_sign_in/testing.dart';

Expand Down Expand Up @@ -391,7 +392,9 @@ void main() {
GoogleSignIn googleSignIn;

setUp(() {
GoogleSignIn.channel.setMockMethodCallHandler(
final MethodChannelGoogleSignIn platformInstance =
GoogleSignInPlatform.instance;
platformInstance.channel.setMockMethodCallHandler(
(FakeSignInBackend()..user = kUserData).handleMethodCall);
googleSignIn = GoogleSignIn();
});
Expand Down
10 changes: 9 additions & 1 deletion script/build_all_plugins_app.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,15 @@ readonly REPO_DIR="$(dirname "$SCRIPT_DIR")"
source "$SCRIPT_DIR/common.sh"
check_changed_packages > /dev/null

(cd "$REPO_DIR" && pub global run flutter_plugin_tools all-plugins-app --exclude instrumentation_adapter,url_launcher_platform_interface)
readonly EXCLUDED_PLUGINS_LIST=(
"instrumentation_adapter"
"url_launcher_platform_interface"
"google_sign_in_platform_interface"
)
# Comma-separated string of the list above
readonly EXCLUDED=$(IFS=, ; echo "${EXCLUDED_PLUGINS_LIST[*]}")

(cd "$REPO_DIR" && pub global run flutter_plugin_tools all-plugins-app --exclude $EXCLUDED)

function error() {
echo "$@" 1>&2
Expand Down