[google_sign_in] Convert Android to Pigeon #4344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replaces the direct method channel implementation with Pigeon. Since `google_sign_in`, unlike most of our plugins, exposes an API that's intended for direct cross-plugin native use, the existing methods are all left in place, but refactored as passthroughs to the new Pigeon versions. To ensure that they aren't broken, the existing Java tests are preserved unchanged (as a "legacy" copy) with `onMethodCall` left in place for now just to allow the tests to continue to run as-is. Since that dispatches to the legacy methods, this keeps the existing coverage of those methods. The new tests are a copy of the legacy tests, minimally translated to use the new Pigeon variants, to ensure continuity of testing to the new version. Part of flutter/flutter#117908
stuartmorgan
commented
Jun 29, 2023
There was a problem hiding this comment.
This file is an exact copy of the original GoogleSignInTest.java, with only the class name changed to match the file rename. (I did it this way, instead of giving the new test a new name, so that the new tests could be reviewed as a diff against the old ones, since that's much more valuable for review—and git history/blame—than having the legacy tests show as a move and the updated tests show as completely new.)
| // TODO(stuartmorgan): Test with games sign-in; according to docs these could be null | ||
| // as the games login request is currently constructed, but the public Dart API | ||
| // assumes they are non-null, so the sign-in query may need to change to | ||
| // include requestEmail() and requestProfile(). |
There was a problem hiding this comment.
This is very likely a latent bug (maybe nobody uses games sign in, or maybe it usually populates them?) but I didn't want to change any behavior, so I just documented it.
| final @Nullable Messages.Result<Messages.UserData> userDataResult; | ||
| final @Nullable Messages.Result<Void> voidResult; | ||
| final @Nullable Messages.Result<Boolean> boolResult; | ||
| final @Nullable Messages.Result<String> stringResult; |
There was a problem hiding this comment.
This is a bit clunky, but it allowed introducing the type safe responses without major structural changes to the plugin. (And in at least some cases storing them is unavoidable due to the way the intent callbacks work.)
2 tasks
camsim99
approved these changes
Jun 30, 2023
stuartmorgan
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 3, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 3, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 3, 2023
flutter/packages@53ed5a0...cdae854 2023-07-01 engine-flutter-autoroll@skia.org Roll Flutter from ff838bc to aa5f4a2 (29 revisions) (flutter/packages#4363) 2023-07-01 stuartmorgan@google.com [tool] Add a flag to skip cleanup (flutter/packages#4357) 2023-07-01 stuartmorgan@google.com [file_selector] Endorse Android (flutter/packages#4329) 2023-07-01 stuartmorgan@google.com [google_sign_in] Convert Android to Pigeon (flutter/packages#4344) 2023-06-30 tarrinneal@gmail.com [Pigeon] readme updates (flutter/packages#3705) 2023-06-30 43054281+camsim99@users.noreply.github.com [camera_android] Support concurrently image capture and image streaming (flutter/packages#4332) 2023-06-30 kustermann.martin@gmail.com Prepare for utf8.encode() to return more precise Uint8List type (flutter/packages#4342) 2023-06-30 tarrinneal@gmail.com [shared_preferences] Adds allowList to setPrefix method. (flutter/packages#3794) 2023-06-30 engine-flutter-autoroll@skia.org Roll Flutter from 51bef1b to ff838bc (12 revisions) (flutter/packages#4346) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Jul 7, 2023
11 tasks
11 tasks
11 tasks
auto-submit bot
pushed a commit
that referenced
this pull request
Sep 18, 2023
Replaces the manual method channel code with Pigeon. The Pigeon definition and the Dart code and Dart unit tests are heavily based on the [previous Android migration](#4344). Fixes flutter/flutter#117908
HugoOlthof
pushed a commit
to moneybird/packages
that referenced
this pull request
Dec 13, 2023
Replaces the manual method channel code with Pigeon. The Pigeon definition and the Dart code and Dart unit tests are heavily based on the [previous Android migration](flutter#4344). Fixes flutter/flutter#117908
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replaces the direct method channel implementation with Pigeon.
Since
google_sign_in, unlike most of our plugins, exposes an API that's intended for direct cross-plugin native use, the existing methods are all left in place, but refactored as passthroughs to the new Pigeon versions. To ensure that they aren't broken, the existing Java tests are preserved unchanged (as a "legacy" copy) withonMethodCallleft in place for now just to allow the tests to continue to run as-is. Since that dispatches to the legacy methods, this keeps the existing coverage of those methods.The new tests are a copy of the legacy tests, minimally translated to use the new Pigeon variants, to ensure continuity of testing to the new version.
Part of flutter/flutter#117908
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).