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

[google_sign_in_web] Migrate to null-safety #3628

Merged
merged 11 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 10 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_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.10.0

* Migrate to null-safety.

## 0.9.2+1

* Update Flutter SDK constraint.
Expand Down
21 changes: 21 additions & 0 deletions packages/google_sign_in/google_sign_in_web/example/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Testing

This package utilizes the `integration_test` package to run its tests in a web browser.

See [flutter.dev > Integration testing](https://flutter.dev/docs/testing/integration-tests) for more info.

## Running the tests

Make sure you have updated to the latest Flutter master.

1. Check what version of Chrome is running on the machine you're running tests on.

2. Download and install driver for that version from here:
* <https://chromedriver.chromium.org/downloads>

3. Start the driver using `chromedriver --port=4444`

4. Run tests: `flutter drive -d web-server --browser-name=chrome --driver=test_driver/integration_driver.dart --target=integration_test/TEST_NAME.dart`, or (in Linux):

* Single: `./run_test.sh integration_test/TEST_NAME.dart`
* All: `./run_test.sh`
ditman marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:google_sign_in_platform_interface/google_sign_in_platform_interf
import 'package:google_sign_in_web/google_sign_in_web.dart';
import 'gapi_mocks/gapi_mocks.dart' as gapi_mocks;
import 'src/test_utils.dart';
import 'package:js/js_util.dart' as js_util;
ditman marked this conversation as resolved.
Show resolved Hide resolved

void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
Expand All @@ -25,7 +26,7 @@ void main() {
idToken: expectedTokenData.idToken,
);

GoogleSignInPlugin plugin;
late GoogleSignInPlugin plugin;

group('plugin.init() throws a catchable exception', () {
setUp(() {
Expand Down Expand Up @@ -54,15 +55,16 @@ void main() {
);
fail('plugin.init should have thrown an exception!');
} catch (e) {
expect(e.code, 'idpiframe_initialization_failed');
final String code = js_util.getProperty(e, 'code') as String;
expect(code, 'idpiframe_initialization_failed');
}
});
});

group('other methods also throw catchable exceptions on init fail', () {
// This function ensures that init gets called, but for some reason, we
// ignored that it has thrown stuff...
void _discardInit() async {
Future<void> _discardInit() async {
try {
await plugin.init(
hostedDomain: 'foo',
Expand Down Expand Up @@ -135,13 +137,13 @@ void main() {
});

testWidgets('signInSilently', (WidgetTester tester) async {
GoogleSignInUserData actualUser = await plugin.signInSilently();
GoogleSignInUserData actualUser = (await plugin.signInSilently())!;

expect(actualUser, expectedUserData);
});

testWidgets('signIn', (WidgetTester tester) async {
GoogleSignInUserData actualUser = await plugin.signIn();
GoogleSignInUserData actualUser = (await plugin.signIn())!;

expect(actualUser, expectedUserData);
});
Expand Down Expand Up @@ -185,7 +187,8 @@ void main() {
await plugin.signIn();
fail('plugin.signIn() should have thrown an exception!');
} catch (e) {
expect(e.code, 'popup_closed_by_user');
final String code = js_util.getProperty(e, 'code') as String;
expect(code, 'popup_closed_by_user');
}
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import 'src/test_utils.dart';
void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

gapiUrl = toBase64Url(gapi_mocks.auth2InitSuccess(GoogleSignInUserData()));
gapiUrl = toBase64Url(gapi_mocks.auth2InitSuccess(
GoogleSignInUserData(email: 'test@test.com', id: '1234')));

testWidgets('Plugin is initialized after GAPI fully loads and init is called',
(WidgetTester tester) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,16 @@ import 'package:integration_test/integration_test.dart';

import 'package:google_sign_in_web/src/generated/gapiauth2.dart' as gapi;
import 'package:google_sign_in_web/src/utils.dart';
import 'package:mockito/mockito.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

import 'package:test/fake.dart?

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 didn't know where this was coming from, it seemed to come from some implicit import!

Copy link
Member Author

@ditman ditman Feb 25, 2021

Choose a reason for hiding this comment

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

This was concerning me, so I dug a little bit. Fake now (also) comes from flutter_test:

https://master-api.flutter.dev/flutter/flutter_test/flutter_test-library.html

No need to import package:test anymore!

(It's just a little bit "too new", hence the link to the "master" docs :P)


class MockGoogleUser extends Mock implements gapi.GoogleUser {}

class MockBasicProfile extends Mock implements gapi.BasicProfile {}

void main() {
// The non-null use cases are covered by the auth2_test.dart file.
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

group('gapiUserToPluginUserData', () {
var mockUser;
late FakeGoogleUser mockUser;
ditman marked this conversation as resolved.
Show resolved Hide resolved

setUp(() {
mockUser = MockGoogleUser();
mockUser = FakeGoogleUser();
});

testWidgets('null user -> null response', (WidgetTester tester) async {
Expand All @@ -30,21 +25,45 @@ void main() {

testWidgets('not signed-in user -> null response',
(WidgetTester tester) async {
when(mockUser.isSignedIn()).thenReturn(false);
expect(gapiUserToPluginUserData(mockUser), isNull);
});

testWidgets('signed-in, but null profile user -> null response',
(WidgetTester tester) async {
when(mockUser.isSignedIn()).thenReturn(true);
mockUser.setIsSignedIn(true);
expect(gapiUserToPluginUserData(mockUser), isNull);
});

testWidgets('signed-in, null userId in profile user -> null response',
(WidgetTester tester) async {
when(mockUser.isSignedIn()).thenReturn(true);
when(mockUser.getBasicProfile()).thenReturn(MockBasicProfile());
mockUser.setIsSignedIn(true);
mockUser.setBasicProfile(FakeBasicProfile());
expect(gapiUserToPluginUserData(mockUser), isNull);
});
});
}

class FakeGoogleUser extends Fake implements gapi.GoogleUser {
bool _isSignedIn = false;
gapi.BasicProfile? _basicProfile;

@override
bool isSignedIn() => _isSignedIn;
@override
gapi.BasicProfile? getBasicProfile() => _basicProfile;

void setIsSignedIn(bool isSignedIn) {
_isSignedIn = isSignedIn;
}

void setBasicProfile(gapi.BasicProfile basicProfile) {
_basicProfile = basicProfile;
}
}

class FakeBasicProfile extends Fake implements gapi.BasicProfile {
String? _id;

@override
String? getId() => _id;
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
name: regular_integration_tests
name: google_sign_in_web_integration_tests
publish_to: none

environment:
sdk: ">=2.2.2 <3.0.0"
sdk: ">=2.12.0-259.9.beta <3.0.0"
flutter: ">=1.27.0-0" # For integration_test from sdk

dependencies:
flutter:
sdk: flutter

dev_dependencies:
google_sign_in: ^4.5.3
http: ^0.13.0
js: ^0.6.3
flutter_driver:
sdk: flutter
flutter_test:
sdk: flutter
http: ^0.12.2
mockito: ^4.1.1
ditman marked this conversation as resolved.
Show resolved Hide resolved
integration_test:
path: ../../../integration_test
sdk: flutter

dependency_overrides:
google_sign_in_web:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
#!/usr/bin/bash

if pgrep -lf chromedriver > /dev/null; then
echo "chromedriver is running."

if [ $# -eq 0 ]; then
echo "No target specified, running all tests..."
find test_driver/ -iname *_integration.dart | xargs -n1 -i -t flutter drive -d web-server --web-port=7357 --browser-name=chrome --target='{}'
find integration_test/ -iname *_test.dart | xargs -n1 -i -t flutter drive -d web-server --web-port=7357 --browser-name=chrome --driver=test_driver/integration_driver.dart --target='{}'
else
echo "Running test target: $1..."
set -x
flutter drive -d web-server --web-port=7357 --browser-name=chrome --target=$1
flutter drive -d web-server --web-port=7357 --browser-name=chrome --driver=test_driver/integration_driver.dart --target=$1
fi

else
echo "chromedriver is not running."
fi



Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
_isGapiInitialized = gapi.inject(gapiUrl).then((_) => gapi.init());
}

Future<void> _isGapiInitialized;
Future<void> _isAuthInitialized;
late Future<void> _isGapiInitialized;
late Future<void> _isAuthInitialized;
bool _isInitCalled = false;

// This method throws if init hasn't been called at some point in the past.
Expand All @@ -58,20 +58,21 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
return Future.wait([_isGapiInitialized, _isAuthInitialized]);
}

String _autoDetectedClientId;
String? _autoDetectedClientId;

/// Factory method that initializes the plugin with [GoogleSignInPlatform].
static void registerWith(Registrar registrar) {
GoogleSignInPlatform.instance = GoogleSignInPlugin();
}

@override
Future<void> init(
{@required String hostedDomain,
List<String> scopes = const <String>[],
SignInOption signInOption = SignInOption.standard,
String clientId}) async {
final String appClientId = clientId ?? _autoDetectedClientId;
Future<void> init({
List<String> scopes = const <String>[],
SignInOption signInOption = SignInOption.standard,
String? hostedDomain,
String? clientId,
}) async {
final String? appClientId = clientId ?? _autoDetectedClientId;
assert(
appClientId != null,
'ClientID not set. Either set it on a '
Expand All @@ -90,7 +91,7 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
hosted_domain: hostedDomain,
// The js lib wants a space-separated list of values
scope: scopes.join(' '),
client_id: appClientId,
client_id: appClientId!,
));

Completer<void> isAuthInitialized = Completer<void>();
Expand Down Expand Up @@ -119,18 +120,18 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {
}

@override
Future<GoogleSignInUserData> signInSilently() async {
Future<GoogleSignInUserData?> signInSilently() async {
await initialized;

return gapiUserToPluginUserData(
await auth2.getAuthInstance().currentUser.get());
await auth2.getAuthInstance()?.currentUser?.get());
}

@override
Future<GoogleSignInUserData> signIn() async {
Future<GoogleSignInUserData?> signIn() async {
await initialized;
try {
return gapiUserToPluginUserData(await auth2.getAuthInstance().signIn());
return gapiUserToPluginUserData(await auth2.getAuthInstance()?.signIn());
} on auth2.GoogleAuthSignInError catch (reason) {
throw PlatformException(
code: reason.error,
Expand All @@ -143,47 +144,53 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {

@override
Future<GoogleSignInTokenData> getTokens(
{@required String email, bool shouldRecoverAuth}) async {
{required String email, bool? shouldRecoverAuth}) async {
await initialized;

final auth2.GoogleUser currentUser =
final auth2.GoogleUser? currentUser =
auth2.getAuthInstance()?.currentUser?.get();
final auth2.AuthResponse response = currentUser.getAuthResponse();
final auth2.AuthResponse? response = currentUser?.getAuthResponse();

return GoogleSignInTokenData(
idToken: response.id_token, accessToken: response.access_token);
idToken: response?.id_token, accessToken: response?.access_token);
}

@override
Future<void> signOut() async {
await initialized;

return auth2.getAuthInstance().signOut();
return auth2.getAuthInstance()?.signOut();
}

@override
Future<void> disconnect() async {
await initialized;

final auth2.GoogleUser currentUser =
final auth2.GoogleUser? currentUser =
auth2.getAuthInstance()?.currentUser?.get();

if (currentUser == null) return;

return currentUser.disconnect();
}

@override
Future<bool> isSignedIn() async {
await initialized;

final auth2.GoogleUser currentUser =
final auth2.GoogleUser? currentUser =
auth2.getAuthInstance()?.currentUser?.get();

if (currentUser == null) return false;

return currentUser.isSignedIn();
}

@override
Future<void> clearAuthCache({String token}) async {
Future<void> clearAuthCache({required String token}) async {
await initialized;

return auth2.getAuthInstance().disconnect();
return auth2.getAuthInstance()?.disconnect();
}

@override
Expand All @@ -194,14 +201,15 @@ class GoogleSignInPlugin extends GoogleSignInPlatform {

if (currentUser == null) return false;

final grantedScopes = currentUser.getGrantedScopes();
final grantedScopes = currentUser.getGrantedScopes() ?? '';
final missingScopes =
scopes.where((scope) => !grantedScopes.contains(scope));

if (missingScopes.isEmpty) return true;

return currentUser
.grant(auth2.SigninOptions(scope: missingScopes.join(" "))) ??
false;
final response = await currentUser
.grant(auth2.SigninOptions(scope: missingScopes.join(" ")));
ditman marked this conversation as resolved.
Show resolved Hide resolved

return response != null;
}
}
Loading