Closed Supereg closed 4 weeks ago
codecov[bot]
commented
1 month ago Attention: Patch coverage is 83.74628% with 492 lines in your changes missing coverage. Please review.
Project coverage is 84.44%. Comparing base (
e3344aa) to head (3a54f5d). Report is 1 commits behind head on main.
Supereg
commented
1 month ago @PSchmiedmayer markdown lint is currently failing as we are linking to the FirebaseAccountService that is not yet deployed in the documentation catalog.
PSchmiedmayer
commented
3 weeks ago Great job to get this merged @Supereg; a huge lift!
Model Account Service as singleton with ability to provide multiple Identity Providers
:recycle: Current situation & Problems
The current design of SpeziAccount allows to have multiple
AccountServices that each implement exactly one identity provider (e.g., signup with password credentials or SSO using Sign in with Apple). We are currently missing support for a singleAccountServiceto combine multiple identity providers. Secondly, we found that there is generally no need for an application to support multiple, entirely different account services.Further, the current infrastructure is relatively strict on how to define account services and their UI components. This is done through multiple different sub-protocols to the based
AccountServiceprotocol. Each protocol makes certain assumptions how an account service might look and is, therefore, (magically) providing certain default implementations or default UI components. While the defaults are great, we found that this design is pretty inflexible. If an account service doesn't quite fit the requirements of one of the default protocols, it has to implement a lot of the things completely from scratch. Most of the default UI components shipped with SpeziAccount are tightly coupled with these protocols and can therefore not be easily reused. We would love a design that provides great defaults, but not at the cost of flexibility.Further, account notifications and external storage are currently realized using
Standardconstraints. Specifically with external storage we found that these are often provided by external components and that the constraint was always just used to forward the implementation to such a component. Removing the constraint and directly supplying the external storage provider to theAccountConfigurationwould be much more ideal. In an attempt to transparently support notifications and external storage, internal wrapper account services were used to automatically implement support for these operations. This design required several workarounds when it was required to interact with the underlying account service and was easy to get wrong. Not ideal.Lastly, there was typically a lot of boiler plate code required to create new account keys, making it a bigger effort than necessary. Notably, creating non-String-based account keys involved creating display and entry view components as there wasn't anything provided out of the box for most primitive types. All in all, it made fast prototyping harder as considerable effort had to be spent when implementing new account keys.
The release notes sections lists how all of these problems have been addressed in this PR.
:gear: Release Notes
AccountServiceis now a singleton design that is capable of providing multipleIdentityProviderAccountServiceprotocol.IdentityProvidercan be completely customized by the account service.AccountSetupProviderView(previously theUserIdPasswordEmbeddedView) and aSignInWithAppleview that integrates nicely with SpeziAccountAccountNotificationsmodule. The.deletingAccountevent must now be manually emitted by anAccountService.ExternalAccountStoragemodule serves as the bridge between anAccountServiceand anAccountStorageProvider.ExternalAccountStoragemodule.ExternalAccountStoragecan now communicate back changes from the external data store.InMemoryAccountServiceand theInMemoryAccountStorageProvider. Both are great for SwiftUI previews and UI testing.AccountKeyandKeyEntrymacro that can be used to define new account keys.AccountDetailsstorage infrastructure, removing the differentiating betweenSignupDetails,ModifiedAccountDetails,PartialAccountDetailsandRemovedAccountDetails. All were replaced by the singleAccountDetailscontainer. TheAccountValuesBuilderwas also replaced by making all accessed toAccountDetailsmutable.FollowUpInfoSheetis now much more customizable, allowing to change behavior inAccountSetupusing theFollowUpBehaviorconfiguration. See https://github.com/StanfordSpezi/SpeziAccount/pull/53#issuecomment-2273202062.verifyRequiredAccountDetails(_:)modifier was removed as it is now automatically added through theAccountConfiguration.:books: Documentation
The documentation catalog was restructured. Generally, nesting was reduced and it aims to move types that are relevant to framework users to the root documentation page to make it more easily navigable and discoverable. Documentation that is tailored towards developers implementing account services or external storage providers got their own group. Generally, all documentation was updated to reflect the new functionality.
:white_check_mark: Testing
Existing tests were used to verify existing behavior. Some new tests were added to tests new functionalities.
:pencil: Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: