From 2898b577171293237742bddc68858a91367d26be Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Mon, 23 Jan 2023 17:17:56 -0800 Subject: [PATCH 1/9] Slight cleanup in GoogleSignInPlugin 1) Use switch instead of if/else and show all possible states 2) Handle the case of synchronous failure in signInSilently() by checking isComplete() instead of isSuccessful() --- .../googlesignin/GoogleSignInPlugin.java | 28 +++++++++++-------- .../googlesignin/GoogleSignInTest.java | 20 +++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java b/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java index d345d4976c63..a2631b772683 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java @@ -404,9 +404,9 @@ public void init( public void signInSilently(Result result) { checkAndSetPendingOperation(METHOD_SIGN_IN_SILENTLY, result); Task task = signInClient.silentSignIn(); - if (task.isSuccessful()) { + if (task.isComplete()) { // There's immediate result available. - onSignInAccount(task.getResult()); + onSignInResult(task); } else { task.addOnCompleteListener( new OnCompleteListener() { @@ -516,7 +516,7 @@ private void onSignInResult(Task completedTask) { GoogleSignInAccount account = completedTask.getResult(ApiException.class); onSignInAccount(account); } catch (ApiException e) { - // Forward all errors and let Dart side decide how to handle. + // Forward all errors and let Dart decide how to handle. String errorCode = errorCodeForStatus(e.getStatusCode()); finishWithError(errorCode, e.toString()); } catch (RuntimeExecutionException e) { @@ -538,14 +538,20 @@ private void onSignInAccount(GoogleSignInAccount account) { } private String errorCodeForStatus(int statusCode) { - if (statusCode == GoogleSignInStatusCodes.SIGN_IN_CANCELLED) { - return ERROR_REASON_SIGN_IN_CANCELED; - } else if (statusCode == CommonStatusCodes.SIGN_IN_REQUIRED) { - return ERROR_REASON_SIGN_IN_REQUIRED; - } else if (statusCode == CommonStatusCodes.NETWORK_ERROR) { - return ERROR_REASON_NETWORK_ERROR; - } else { - return ERROR_REASON_SIGN_IN_FAILED; + switch (statusCode) { + case GoogleSignInStatusCodes.SIGN_IN_CANCELLED: + return ERROR_REASON_SIGN_IN_CANCELED; + case CommonStatusCodes.SIGN_IN_REQUIRED: + return ERROR_REASON_SIGN_IN_REQUIRED; + case CommonStatusCodes.NETWORK_ERROR: + return ERROR_REASON_NETWORK_ERROR; + case GoogleSignInStatusCodes.SIGN_IN_CURRENTLY_IN_PROGRESS: + case GoogleSignInStatusCodes.SIGN_IN_FAILED: + case CommonStatusCodes.INVALID_ACCOUNT: + case CommonStatusCodes.INTERNAL_ERROR: + return ERROR_REASON_SIGN_IN_FAILED; + default: + return ERROR_REASON_SIGN_IN_FAILED; } } diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index 9692417390a5..2a7307b0dbb0 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -17,7 +17,10 @@ import com.google.android.gms.auth.api.signin.GoogleSignInAccount; import com.google.android.gms.auth.api.signin.GoogleSignInClient; import com.google.android.gms.auth.api.signin.GoogleSignInOptions; +import com.google.android.gms.common.api.ApiException; import com.google.android.gms.common.api.Scope; +import com.google.android.gms.common.api.Status; +import com.google.android.gms.tasks.Task; import io.flutter.plugin.common.BinaryMessenger; import io.flutter.plugin.common.MethodCall; import io.flutter.plugin.common.MethodChannel; @@ -43,6 +46,7 @@ public class GoogleSignInTest { @Mock GoogleSignInWrapper mockGoogleSignIn; @Mock GoogleSignInAccount account; @Mock GoogleSignInClient mockClient; + @Mock Task mockSignInTask; private GoogleSignInPlugin plugin; @Before @@ -204,6 +208,22 @@ public void signInThrowsWithoutActivity() { plugin.onMethodCall(new MethodCall("signIn", null), null); } + @Test + public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError() { + final String clientId = "fakeClientId"; + MethodCall methodCall = buildInitMethodCall(clientId, null); + initAndAssertServerClientId(methodCall, clientId); + + ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); + when(mockClient.silentSignIn()).thenReturn(mockSignInTask); + when(mockSignInTask.isComplete()).thenReturn(true); + when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); + + MethodCall methodCall = new MethodCall("signInSilently", null); + plugin.onMethodCall(methodCall, result); + verify(result).error("sign_in_required", "Error text", null); + } + @Test public void init_LoadsServerClientIdFromResources() { final String packageName = "fakePackageName"; From 82a3823f69d33c419e44816902c28d4502b27d92 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 00:08:48 -0800 Subject: [PATCH 2/9] Run formatter and fix some compilation errors --- .../plugins/googlesignin/GoogleSignInPlugin.java | 10 +++++----- .../flutter/plugins/googlesignin/GoogleSignInTest.java | 7 ++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java b/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java index a2631b772683..8963a5169e89 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/main/java/io/flutter/plugins/googlesignin/GoogleSignInPlugin.java @@ -540,18 +540,18 @@ private void onSignInAccount(GoogleSignInAccount account) { private String errorCodeForStatus(int statusCode) { switch (statusCode) { case GoogleSignInStatusCodes.SIGN_IN_CANCELLED: - return ERROR_REASON_SIGN_IN_CANCELED; + return ERROR_REASON_SIGN_IN_CANCELED; case CommonStatusCodes.SIGN_IN_REQUIRED: - return ERROR_REASON_SIGN_IN_REQUIRED; + return ERROR_REASON_SIGN_IN_REQUIRED; case CommonStatusCodes.NETWORK_ERROR: - return ERROR_REASON_NETWORK_ERROR; + return ERROR_REASON_NETWORK_ERROR; case GoogleSignInStatusCodes.SIGN_IN_CURRENTLY_IN_PROGRESS: case GoogleSignInStatusCodes.SIGN_IN_FAILED: case CommonStatusCodes.INVALID_ACCOUNT: case CommonStatusCodes.INTERNAL_ERROR: - return ERROR_REASON_SIGN_IN_FAILED; + return ERROR_REASON_SIGN_IN_FAILED; default: - return ERROR_REASON_SIGN_IN_FAILED; + return ERROR_REASON_SIGN_IN_FAILED; } } diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index 2a7307b0dbb0..b19aed529cc6 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -18,6 +18,7 @@ import com.google.android.gms.auth.api.signin.GoogleSignInClient; import com.google.android.gms.auth.api.signin.GoogleSignInOptions; import com.google.android.gms.common.api.ApiException; +import com.google.android.gms.common.api.CommonStatusCodes; import com.google.android.gms.common.api.Scope; import com.google.android.gms.common.api.Status; import com.google.android.gms.tasks.Task; @@ -214,13 +215,13 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError MethodCall methodCall = buildInitMethodCall(clientId, null); initAndAssertServerClientId(methodCall, clientId); - ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); + ApiException exception = + new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); when(mockClient.silentSignIn()).thenReturn(mockSignInTask); when(mockSignInTask.isComplete()).thenReturn(true); when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); - MethodCall methodCall = new MethodCall("signInSilently", null); - plugin.onMethodCall(methodCall, result); + plugin.onMethodCall(new MethodCall("signInSilently", null), result); verify(result).error("sign_in_required", "Error text", null); } From ef8ec2fe46f8ae42e16b2725acd81514b8af4c35 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 00:16:09 -0800 Subject: [PATCH 3/9] Version bump --- packages/google_sign_in/google_sign_in_android/CHANGELOG.md | 3 ++- packages/google_sign_in/google_sign_in_android/pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_android/CHANGELOG.md b/packages/google_sign_in/google_sign_in_android/CHANGELOG.md index 5d1b2d24d7d1..6ce3cb97e8db 100644 --- a/packages/google_sign_in/google_sign_in_android/CHANGELOG.md +++ b/packages/google_sign_in/google_sign_in_android/CHANGELOG.md @@ -1,5 +1,6 @@ -## NEXT +## 6.1.6 +* Minor implementation cleanup * Updates minimum Flutter version to 3.0. ## 6.1.5 diff --git a/packages/google_sign_in/google_sign_in_android/pubspec.yaml b/packages/google_sign_in/google_sign_in_android/pubspec.yaml index b6b581a75764..4be89f27286a 100644 --- a/packages/google_sign_in/google_sign_in_android/pubspec.yaml +++ b/packages/google_sign_in/google_sign_in_android/pubspec.yaml @@ -2,7 +2,7 @@ name: google_sign_in_android description: Android implementation of the google_sign_in plugin. repository: https://github.com/flutter/plugins/tree/main/packages/google_sign_in/google_sign_in_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22 -version: 6.1.5 +version: 6.1.6 environment: sdk: ">=2.14.0 <3.0.0" From 7d74bdde20f34e6484e18936e88cd24a019990fc Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 08:54:04 -0800 Subject: [PATCH 4/9] Declare throws --- .../java/io/flutter/plugins/googlesignin/GoogleSignInTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index b19aed529cc6..a935d96cea58 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -210,7 +210,8 @@ public void signInThrowsWithoutActivity() { } @Test - public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError() { + public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError() + throws ApiException { final String clientId = "fakeClientId"; MethodCall methodCall = buildInitMethodCall(clientId, null); initAndAssertServerClientId(methodCall, clientId); From 2801767d43ed1b2b4374c79f73c3c7a3fe9b1822 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 09:26:45 -0800 Subject: [PATCH 5/9] Update error text expectation --- .../java/io/flutter/plugins/googlesignin/GoogleSignInTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index a935d96cea58..f424a94282cd 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -223,7 +223,8 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); plugin.onMethodCall(new MethodCall("signInSilently", null), result); - verify(result).error("sign_in_required", "Error text", null); + verify(result) + .error("sign_in_required", CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", null); } @Test From eadf792932a386469d52cd62de957dade796c5ed Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 12:04:50 -0800 Subject: [PATCH 6/9] Diagnose failing test --- .../plugins/googlesignin/GoogleSignInTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index f424a94282cd..0f890ba3f936 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -44,6 +44,7 @@ public class GoogleSignInTest { @Mock PluginRegistry.Registrar mockRegistrar; @Mock BinaryMessenger mockMessenger; @Spy MethodChannel.Result result; + @Mock MethodChannel.Result mockResult; @Mock GoogleSignInWrapper mockGoogleSignIn; @Mock GoogleSignInAccount account; @Mock GoogleSignInClient mockClient; @@ -216,15 +217,24 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError MethodCall methodCall = buildInitMethodCall(clientId, null); initAndAssertServerClientId(methodCall, clientId); + ArgumentCaptor code = ArgumentCaptor.forClass(String.class); + ArgumentCaptor message = ArgumentCaptor.forClass(String.class); + verify(mockResult).error(code.capture(), message.capture(), null); + ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); when(mockClient.silentSignIn()).thenReturn(mockSignInTask); when(mockSignInTask.isComplete()).thenReturn(true); when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); - plugin.onMethodCall(new MethodCall("signInSilently", null), result); + plugin.onMethodCall(new MethodCall("signInSilently", null), mockResult); + System.out.println("~!@ :: " + code.getValue()); + System.out.println("~!@ :: " + message.getValue()); + Assert.assertEquals(CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", message.getValue()); + /* verify(result) .error("sign_in_required", CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", null); + */ } @Test From 6608fbba19eaeb59dde66027fb29d0799fb0815d Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 12:41:01 -0800 Subject: [PATCH 7/9] Code --- .../java/io/flutter/plugins/googlesignin/GoogleSignInTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index 0f890ba3f936..f7b526aad2c8 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -219,7 +219,7 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError ArgumentCaptor code = ArgumentCaptor.forClass(String.class); ArgumentCaptor message = ArgumentCaptor.forClass(String.class); - verify(mockResult).error(code.capture(), message.capture(), null); + verify(mockResult).error(code.capture(), message.capture(), any()); ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); From 7663e8d9c5f0316cd286901d58e1c3056c2305b8 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 12:48:04 -0800 Subject: [PATCH 8/9] Code --- .../java/io/flutter/plugins/googlesignin/GoogleSignInTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index f7b526aad2c8..81a9d78bacd4 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -219,7 +219,6 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError ArgumentCaptor code = ArgumentCaptor.forClass(String.class); ArgumentCaptor message = ArgumentCaptor.forClass(String.class); - verify(mockResult).error(code.capture(), message.capture(), any()); ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); @@ -228,6 +227,7 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); plugin.onMethodCall(new MethodCall("signInSilently", null), mockResult); + verify(mockResult).error(code.capture(), message.capture(), any()); System.out.println("~!@ :: " + code.getValue()); System.out.println("~!@ :: " + message.getValue()); Assert.assertEquals(CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", message.getValue()); From 75ccc3aeef262658749083c58b9f9ed8e7dd12c0 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Tue, 24 Jan 2023 13:52:14 -0800 Subject: [PATCH 9/9] Fix test --- .../plugins/googlesignin/GoogleSignInTest.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java index 81a9d78bacd4..78568460c9e6 100644 --- a/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java +++ b/packages/google_sign_in/google_sign_in_android/android/src/test/java/io/flutter/plugins/googlesignin/GoogleSignInTest.java @@ -44,7 +44,6 @@ public class GoogleSignInTest { @Mock PluginRegistry.Registrar mockRegistrar; @Mock BinaryMessenger mockMessenger; @Spy MethodChannel.Result result; - @Mock MethodChannel.Result mockResult; @Mock GoogleSignInWrapper mockGoogleSignIn; @Mock GoogleSignInAccount account; @Mock GoogleSignInClient mockClient; @@ -217,24 +216,18 @@ public void signInSilentlyThatImmediatelyCompletesWithoutResultFinishesWithError MethodCall methodCall = buildInitMethodCall(clientId, null); initAndAssertServerClientId(methodCall, clientId); - ArgumentCaptor code = ArgumentCaptor.forClass(String.class); - ArgumentCaptor message = ArgumentCaptor.forClass(String.class); - ApiException exception = new ApiException(new Status(CommonStatusCodes.SIGN_IN_REQUIRED, "Error text")); when(mockClient.silentSignIn()).thenReturn(mockSignInTask); when(mockSignInTask.isComplete()).thenReturn(true); when(mockSignInTask.getResult(ApiException.class)).thenThrow(exception); - plugin.onMethodCall(new MethodCall("signInSilently", null), mockResult); - verify(mockResult).error(code.capture(), message.capture(), any()); - System.out.println("~!@ :: " + code.getValue()); - System.out.println("~!@ :: " + message.getValue()); - Assert.assertEquals(CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", message.getValue()); - /* + plugin.onMethodCall(new MethodCall("signInSilently", null), result); verify(result) - .error("sign_in_required", CommonStatusCodes.SIGN_IN_REQUIRED + ": Error text", null); - */ + .error( + "sign_in_required", + "com.google.android.gms.common.api.ApiException: 4: Error text", + null); } @Test