feat!: типизированный контракт signIn() и стандартизация кодов ошибок…#4
Conversation
… (1.3.0) Breaking changes: - signIn() возвращает Future<YandexAuthResult> (non-null); null больше не возвращается - Отмена/ошибки выбрасывают типизированные исключения вместо PlatformException: * YandexAuthCancelledException — пользователь отменил вход * YandexAuthFailedException — прочие ошибки, с YandexAuthErrorCode - Стандартизованы коды ошибок между Android и iOS: cancelled/activation/ concurrent/no_activity/sdk_error/unknown - Android-пакет переименован com.example.yandex_auth → com.coolswood.yandex_auth Прочее: - YandexAuthResult: добавлены == / hashCode - Android: восстановлен корректный config-change flow (результат не теряется) - iOS: добавлено распознавание отмены через ASWebAuthenticationSessionError - Починен тест yandex_auth_method_channel_test, добавлены 5 новых кейсов - README/CHANGELOG обновлены, example переведён на новый контракт - .iml-файлы убраны из git, расширен .gitignore
- iOS YandexLoginSDK: 3.1.0 → 3.1.1 - Android authsdk: 3.2.0 → 3.2.1 - example: cupertino_icons upgraded
P1: - Добавлен YandexAuth.logout(): на iOS очищает кеш JWT, на Android no-op (Yandex Auth SDK stateless). Тесты на успех и ошибку SDK. - Публичный API полностью задокументирован dartdoc'ом. - Подтверждена корректность config-change flow на Android. P2: - Включены строгие линтеры: strict-casts, strict-raw-types, public_member_api_docs, prefer_single_quotes, require_trailing_commas. - iOS: убран deprecated fallback через UIApplication.shared.windows; поиск root view controller единым путём через UIWindowScene.keyWindow. - Добавлен GitHub Actions CI: dart format --set-exit-if-changed, flutter analyze, flutter test. Версия: 1.3.0 → 1.3.1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughPR обновляет контракт авторизации: ChangesCore API, native plugins, and supporting updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant YandexAuth
participant MethodChannelYandexAuth
participant AndroidOriOSPlugin
App->>YandexAuth: signIn()
YandexAuth->>MethodChannelYandexAuth: signIn()
MethodChannelYandexAuth->>AndroidOriOSPlugin: invokeMethod('signIn')
AndroidOriOSPlugin-->>MethodChannelYandexAuth: success / PlatformException
MethodChannelYandexAuth-->>YandexAuth: YandexAuthResult / YandexAuthException
YandexAuth-->>App: Future<YandexAuthResult>
App->>YandexAuth: logout()
YandexAuth->>MethodChannelYandexAuth: logout()
MethodChannelYandexAuth->>AndroidOriOSPlugin: invokeMethod('logout')
AndroidOriOSPlugin-->>MethodChannelYandexAuth: success / PlatformException
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the yandex_auth plugin to version 1.3.1, introducing standardized error handling with typed exceptions, a new logout() method, and updated native SDK dependencies. It also renames the Android package, refactors iOS root view controller lookup using UIWindowScene, and enforces stricter linter rules. The review feedback identifies two important issues: a potential memory leak on Android due to contract holding a reference to a destroyed Activity during configuration changes, and a potential UIKit presentation failure on iOS if a modal is already active, which can be resolved by recursively finding the top-most view controller.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| override fun onDetachedFromActivityForConfigChanges() { | ||
| // Note: при config change мы НЕ очищаем pendingResult — результат | ||
| // активити будет доставлен в новую активити после onReattached. | ||
| activityBinding?.removeActivityResultListener(this) | ||
| activityBinding = null | ||
| activity = null | ||
| } |
There was a problem hiding this comment.
В методе onDetachedFromActivityForConfigChanges не сбрасывается переменная contract. Поскольку contract (экземпляр YandexAuthSdkContract) инициализируется с использованием YandexAuthOptions(binding.activity, true), он хранит сильную ссылку на старую Activity. При изменении конфигурации (например, повороте экрана) старая Activity уничтожается, но ссылка на неё остаётся в плагине через contract, что приводит к утечке памяти (Memory Leak). Рекомендуется обнулять contract при отсоединении от Activity.
| override fun onDetachedFromActivityForConfigChanges() { | |
| // Note: при config change мы НЕ очищаем pendingResult — результат | |
| // активити будет доставлен в новую активити после onReattached. | |
| activityBinding?.removeActivityResultListener(this) | |
| activityBinding = null | |
| activity = null | |
| } | |
| override fun onDetachedFromActivityForConfigChanges() { | |
| // Note: при config change мы НЕ очищаем pendingResult — результат | |
| // активити будет доставлен в новую активити после onReattached. | |
| activityBinding?.removeActivityResultListener(this) | |
| activityBinding = null | |
| activity = null | |
| contract = null | |
| } |
There was a problem hiding this comment.
✅ Применено в 141c49e. contract теперь обнуляется в onDetachedFromActivityForConfigChanges. Дополнительно: добавил комментарий с пояснением, почему это безопасно — contract пересоздаётся в onReattachedToActivityForConfigChanges через attachTo(), а в окне между detach/reattach onActivityResult аккуратно возвращает no_activity вместо утечки.
| do { | ||
| try YandexLoginSDK.shared.authorize(with: validRootViewController) | ||
| try YandexLoginSDK.shared.authorize(with: rootViewController) |
There was a problem hiding this comment.
Если в приложении уже отображается какой-либо модальный экран (например, диалоговое окно или экран другого плагина), то scene.keyWindow?.rootViewController вернёт корневой контроллер, который уже что-то презентует. Попытка вызвать YandexLoginSDK.shared.authorize(with: rootViewController) в этом случае приведёт к ошибке UIKit (так как контроллер не может презентовать новый экран, если он уже презентует другой). Рекомендуется находить самый верхний (top-most) UIViewController путём рекурсивного обхода presentedViewController.
var topViewController = rootViewController
while let presented = topViewController.presentedViewController {
topViewController = presented
}
do {
try YandexLoginSDK.shared.authorize(with: topViewController)There was a problem hiding this comment.
✅ Применено в 141c49e. Добавил цикл while let presented = topViewController.presentedViewController для спуска к top-most VC перед authorize(with:). Хороший кейс — реальный сбой при наличии модала над root.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
android/src/test/kotlin/com/coolswood/yandex_auth/YandexAuthPluginTest.kt (1)
16-31: 🎯 Functional Correctness | 🔵 TrivialДобавь пару тестов — раз уж плагин обзавёлся новыми ветками, пусть и тесты подрастут вместе с ним.
Сейчас единственный тест проверяет только
no_activity. А ведь появились новые интересные пути:logout()(успех),ERROR_CONCURRENTпри повторномsignIn,ERROR_SDK_ERRORв catch-блоке при исключении изcreateIntent, иERROR_CANCELLED/ERROR_SDK_ERRORвonActivityResult. Стоило бы покрыть их — благоYandexAuthPluginуже неплохо тестируется черезMockito.mock.Хочешь, накидаю черновик тестов для этих веток?
[medium_effort_and_high_reward]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/test/kotlin/com/coolswood/yandex_auth/YandexAuthPluginTest.kt` around lines 16 - 31, The test suite for YandexAuthPlugin only covers the no_activity branch and should be expanded to cover the new control-flow paths. Add focused unit tests around YandexAuthPlugin.onMethodCall and onActivityResult for logout success, the ERROR_CONCURRENT path on repeated signIn, the ERROR_SDK_ERROR catch path when createIntent throws, and both ERROR_CANCELLED and ERROR_SDK_ERROR outcomes from onActivityResult. Use Mockito.mock and existing MethodChannel.Result assertions to verify the exact error/success callbacks for each branch.ios/yandex_auth/Sources/yandex_auth/YandexAuthPlugin.swift (1)
89-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winСтоит отдавать приоритет
foregroundActive-сцене.
connectedScenes.first(where:)берёт первую попавшуюся сцену, удовлетворяющую условиюforegroundActive || foregroundInactive. При нескольких сценах (multi-window / iPad) это может оказаться неактивная сцена, у которойkeyWindowокажетсяnil→ пользователь получит ложныйno_activity, хотя активное окно есть. Предлагаю сперва искать активную сцену, а неактивную использовать как fallback.♻️ Вариант с приоритетом активной сцены
- guard - let scene = UIApplication.shared.connectedScenes - .first(where: { - $0.activationState == .foregroundActive - || $0.activationState == .foregroundInactive - }) as? UIWindowScene, - let rootViewController = scene.keyWindow?.rootViewController + let windowScenes = UIApplication.shared.connectedScenes + .compactMap { $0 as? UIWindowScene } + let scene = windowScenes.first { $0.activationState == .foregroundActive } + ?? windowScenes.first { $0.activationState == .foregroundInactive } + guard + let scene, + let rootViewController = scene.keyWindow?.rootViewController🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/yandex_auth/Sources/yandex_auth/YandexAuthPlugin.swift` around lines 89 - 97, Update the scene lookup in YandexAuthPlugin’s root view controller resolution to prefer a foregroundActive UIWindowScene before falling back to foregroundInactive. The current UIApplication.shared.connectedScenes.first(where:) condition can pick an inactive scene in multi-window cases, causing scene.keyWindow?.rootViewController to be nil and a false no_activity. Adjust the selection logic in the root view controller helper so the active scene is chosen first, with inactive as fallback.test/yandex_auth_test.dart (1)
7-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winМок для
logout()готов, а теста для него — нет!
MockYandexAuthPlatform.logout()добавлен, но нигде не проверяется, что публичныйYandexAuth().logout()реально делегирует вызов в платформу — по аналогии с тестомsignInна строке 25. Раз это новый публичный метод, стоит закрыть его тестом для симметрии и уверенности в контракте.✅ Предлагаемый тест
test('signIn возвращает результат из платформенной реализации', () async { YandexAuth yandexAuthPlugin = YandexAuth(); MockYandexAuthPlatform fakePlatform = MockYandexAuthPlatform(); YandexAuthPlatform.instance = fakePlatform; final result = await yandexAuthPlugin.signIn(); expect(result.token, '42'); }); + + test('logout делегирует вызов в платформенную реализацию', () async { + YandexAuth yandexAuthPlugin = YandexAuth(); + YandexAuthPlatform.instance = MockYandexAuthPlatform(); + + await expectLater(yandexAuthPlugin.logout(), completes); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/yandex_auth_test.dart` around lines 7 - 32, Add a test for the new YandexAuth.logout public API in the existing yandex_auth_test suite, mirroring the current signIn delegation test. Use MockYandexAuthPlatform and YandexAuthPlatform.instance to verify that YandexAuth().logout() delegates to the platform implementation and completes successfully, so the logout contract is covered alongside signIn.example/lib/main.dart (1)
20-36: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueАккуратно разобрал оба доменных исключения — красиво!
Раз уж показываешь пример использования плагина другим разработчикам, можно чуть подстраховаться catch-all блоком на случай непредвиденной ошибки вне контракта
YandexAuthException— это не обязательно, но сделает пример ещё нагляднее как best practice.🛡️ Опциональное дополнение
} on YandexAuthFailedException catch (e) { if (!mounted) return; setState(() { _status = 'Ошибка (${e.code.value}): ${e.message}'; }); + } catch (e) { + if (!mounted) return; + setState(() => _status = 'Неизвестная ошибка: $e'); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example/lib/main.dart` around lines 20 - 36, Add a fallback catch-all in _signIn() after the existing YandexAuthCancelledException and YandexAuthFailedException handlers so the example also demonstrates handling unexpected errors outside the YandexAuthException contract. Keep the current mounted checks and setState pattern, and update the _status in the new generic catch block with a clear failure message that includes the caught error from _signIn() in example/lib/main.dart.test/yandex_auth_method_channel_test.dart (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winНе дублируй имя канала в тесте — используй
platform.methodChannel
platform.methodChannelуже помечен@visibleForTesting, поэтому его можно переиспользовать прямо в тесте вместоMethodChannel('yandex_auth'). Так убирается лишняя точка рассинхрона: при смене имени канала тест не начнёт молча пропускать вызовы.♻️ Предлагаемый рефактор
late MethodChannelYandexAuth platform; - const MethodChannel channel = MethodChannel('yandex_auth'); setUp(() { platform = MethodChannelYandexAuth(); }); tearDown(() { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger - .setMockMethodCallHandler(channel, null); + .setMockMethodCallHandler(platform.methodChannel, null); }); void mockHandler(Future<Object?>? Function(MethodCall) handler) { TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger - .setMockMethodCallHandler(channel, handler); + .setMockMethodCallHandler(platform.methodChannel, handler); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/yandex_auth_method_channel_test.dart` around lines 10 - 11, The test is duplicating the channel name by creating a separate MethodChannel instead of reusing the already exposed platform.methodChannel. Update the yandex_auth_method_channel_test setup to reference platform.methodChannel directly, so the test stays aligned with MethodChannelYandexAuth and does not drift if the channel name changes.lib/src/models/yandex_auth_result.dart (1)
2-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueНебольшое улучшение: сделайте класс
@immutableсconst-конструктором.Раз вы добавили
operator ==/hashCode, класс фактически стал value-типом с неизменяемыми полями. Пометка@immutableиconst-конструктор — идиоматичный подход в Flutter: он документирует неизменяемость, позволяет создаватьconst-экземпляры и защищает от случайного добавления мутабельного состояния в будущем.♻️ Предлагаемое изменение
+import 'package:meta/meta.dart'; + /// Результат успешной авторизации через Yandex Auth. +@immutable class YandexAuthResult { /// Создаёт результат авторизации. - YandexAuthResult({required this.token, this.expiresIn}); + const YandexAuthResult({required this.token, this.expiresIn});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/src/models/yandex_auth_result.dart` around lines 2 - 4, Make YandexAuthResult explicitly immutable by annotating the class with `@immutable` and changing its constructor to const; update the YandexAuthResult declaration so it remains a value type with final fields and keeps its operator ==/hashCode semantics intact. Use the YandexAuthResult class name to locate the change and ensure the constructor still supports token and optional expiresIn while becoming const-friendly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/dart.yml:
- Around line 15-16: The checkout step in the Dart workflow leaves GITHUB_TOKEN
persisted in the local git config, so update the actions/checkout usage to
disable credential persistence by setting persist-credentials to false. Make
this change on the existing checkout step in the workflow so later steps like
flutter pub get do not retain the token in the workspace.
In `@ios/yandex_auth/Sources/yandex_auth/YandexAuthPlugin.swift`:
- Around line 150-157: The cancellation check in isCancellation is using a
fragile string match on YandexLoginSDKError.message for “closed the view
controller”; replace that fallback with the SDK’s typed cancellation error/code
instead, while keeping the existing ASWebAuthenticationSessionError
domain/canceledLogin check intact. Update the logic in
YandexAuthPlugin.isCancellation to branch on the YandexLoginSDK error type or
code rather than message text so cancellation detection remains stable.
---
Nitpick comments:
In `@android/src/test/kotlin/com/coolswood/yandex_auth/YandexAuthPluginTest.kt`:
- Around line 16-31: The test suite for YandexAuthPlugin only covers the
no_activity branch and should be expanded to cover the new control-flow paths.
Add focused unit tests around YandexAuthPlugin.onMethodCall and onActivityResult
for logout success, the ERROR_CONCURRENT path on repeated signIn, the
ERROR_SDK_ERROR catch path when createIntent throws, and both ERROR_CANCELLED
and ERROR_SDK_ERROR outcomes from onActivityResult. Use Mockito.mock and
existing MethodChannel.Result assertions to verify the exact error/success
callbacks for each branch.
In `@example/lib/main.dart`:
- Around line 20-36: Add a fallback catch-all in _signIn() after the existing
YandexAuthCancelledException and YandexAuthFailedException handlers so the
example also demonstrates handling unexpected errors outside the
YandexAuthException contract. Keep the current mounted checks and setState
pattern, and update the _status in the new generic catch block with a clear
failure message that includes the caught error from _signIn() in
example/lib/main.dart.
In `@ios/yandex_auth/Sources/yandex_auth/YandexAuthPlugin.swift`:
- Around line 89-97: Update the scene lookup in YandexAuthPlugin’s root view
controller resolution to prefer a foregroundActive UIWindowScene before falling
back to foregroundInactive. The current
UIApplication.shared.connectedScenes.first(where:) condition can pick an
inactive scene in multi-window cases, causing
scene.keyWindow?.rootViewController to be nil and a false no_activity. Adjust
the selection logic in the root view controller helper so the active scene is
chosen first, with inactive as fallback.
In `@lib/src/models/yandex_auth_result.dart`:
- Around line 2-4: Make YandexAuthResult explicitly immutable by annotating the
class with `@immutable` and changing its constructor to const; update the
YandexAuthResult declaration so it remains a value type with final fields and
keeps its operator ==/hashCode semantics intact. Use the YandexAuthResult class
name to locate the change and ensure the constructor still supports token and
optional expiresIn while becoming const-friendly.
In `@test/yandex_auth_method_channel_test.dart`:
- Around line 10-11: The test is duplicating the channel name by creating a
separate MethodChannel instead of reusing the already exposed
platform.methodChannel. Update the yandex_auth_method_channel_test setup to
reference platform.methodChannel directly, so the test stays aligned with
MethodChannelYandexAuth and does not drift if the channel name changes.
In `@test/yandex_auth_test.dart`:
- Around line 7-32: Add a test for the new YandexAuth.logout public API in the
existing yandex_auth_test suite, mirroring the current signIn delegation test.
Use MockYandexAuthPlatform and YandexAuthPlatform.instance to verify that
YandexAuth().logout() delegates to the platform implementation and completes
successfully, so the logout contract is covered alongside signIn.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6a3b081-c395-412b-afae-d406773419b6
⛔ Files ignored due to path filters (1)
example/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.github/workflows/dart.yml.gitignoreCHANGELOG.mdREADME.mdanalysis_options.yamlandroid/build.gradle.ktsandroid/src/main/AndroidManifest.xmlandroid/src/main/kotlin/com/coolswood/yandex_auth/YandexAuthPlugin.ktandroid/src/test/kotlin/com/coolswood/yandex_auth/YandexAuthPluginTest.ktexample/lib/main.dartios/yandex_auth/Package.swiftios/yandex_auth/Sources/yandex_auth/YandexAuthPlugin.swiftlib/src/exceptions/yandex_auth_exception.dartlib/src/models/yandex_auth_result.dartlib/yandex_auth.dartlib/yandex_auth_method_channel.dartlib/yandex_auth_platform_interface.dartpubspec.yamltest/yandex_auth_method_channel_test.darttest/yandex_auth_test.dartyandex_auth.iml
💤 Files with no reviewable changes (1)
- yandex_auth.iml
Android (gemini-code-assist, high): - onDetachedFromActivityForConfigChanges теперь обнуляет contract, чтобы избежать утечки уничтожаемой Activity (contract держал на неё сильную ссылку через YandexAuthOptions). contract пересоздаётся в onReattached. iOS (gemini-code-assist, medium): - signIn ищет top-most view controller через рекурсивный обход presentedViewController — иначе презентация авторизации поверх root падает, если над ним уже показан модал (диалог/экран другого плагина). iOS (coderabbitai, medium): - isCancellation: заменил хрупкий message.contains на точное сравнение с фиксированным сообщением SDK, вынесенным в константу. Публичного типизированного кода отмены в YandexLoginSDK нет (CoreLoginSDKError приватный), проверка по протоколу+сообщению — лучший доступный вариант. CI (coderabbitai, medium): - actions/checkout: persist-credentials: false — не оставляем GITHUB_TOKEN в .git/config на время джоба (artipacked, zizmor).
… (1.3.0)
Breaking changes:
concurrent/no_activity/sdk_error/unknown
Прочее:
Summary by CodeRabbit
logout()для выхода из аккаунта.signIn()теперь возвращаетFuture<YandexAuthResult>и сигнализирует о проблемах через типизированные исключения.