refactor: remove site key from options; require explicit pass to recaptcha provider - #16409
Conversation
Remove recaptchaSiteKey from FirebaseOptions and explicitly pass it into FIRRecaptchaProvider instead. This aligns with API design principles since it's a specific requirement for AppCheck.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the recaptchaSiteKey property from FirebaseOptions and updates RecaptchaProvider and RecaptchaProviderFactory to explicitly require a siteKey parameter during initialization. Feedback suggests validating the siteKey immediately in FIRRecaptchaProviderFactory to fail fast, and changing the initializer's return type to non-nullable instancetype to simplify the Swift API.
…initialization Update RecaptchaProviderFactory to explicitly require a non-empty siteKey parameter during initialization and raise an exception if invalid. This replaces the previous behavior of reading it from FirebaseOptions. Also updates the changelog entries for FirebaseAppCheck and FirebaseCore to reflect the changes to the public preview reCAPTCHA provider.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request removes the recaptchaSiteKey property from FirebaseOptions and updates RecaptchaProvider and RecaptchaProviderFactory to explicitly require a siteKey parameter during initialization. Feedback includes declaring the siteKey property as readonly in FIRRecaptchaProviderFactory, overriding the unavailable init method in the implementation to prevent runtime invocation, and adding a unit test to verify that initializing the factory with an empty site key throws an exception.
- Make siteKey property readonly - Override init to return nil and flag as NS_UNAVAILABLE - Add unit test to verify empty siteKey throws an exception
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request removes the recaptchaSiteKey property from FirebaseOptions in FirebaseCore and updates RecaptchaProvider and RecaptchaProviderFactory in FirebaseAppCheck to explicitly require a siteKey parameter during initialization. The corresponding tests and changelogs have been updated to reflect these changes. There are no review comments, so I have no feedback to provide.
|
@paulb777, I made some changes that warrant a re-review. This will be staged for the next release, M184. |
Co-authored-by: Paul Beusterien <paulbeusterien@google.com>
Co-authored-by: Paul Beusterien <paulbeusterien@google.com>
2 participants
Remove recaptchaSiteKey from FirebaseOptions and explicitly pass it into FIRRecaptchaProvider instead.
cc: @rlazo
#no-changelog