Add a deleteAndDispose API to allow storage reinitialisation at runtime #138
Conversation
To allow runtime reconfiguration of storage options. After calling the new method, you must call getStorage again, perhaps with new authentication parameters.
There was a problem hiding this comment.
Summary of Changes
Hello @luckyrat, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new deleteAndDispose API to the biometric storage library, addressing the need for runtime reinitialization of storage with different configurations. This enhancement allows library consumers to effectively reset and reconfigure storage instances, resolving previous ambiguities related to the forceInit option and contributing to broader fork consolidation efforts. The changes include the addition of the API, its underlying disposal mechanism, and updated examples to demonstrate its usage.
Highlights
- New
deleteAndDisposeAPI: A newdeleteAndDisposeAPI has been added to theBiometricStorageFileclass. This method allows consumers to clear the stored data and then dispose of the storage instance, making it ready for reinitialization with potentially different configurations. - Internal
disposemethod: An internaldisposemethod was introduced in theBiometricStorageabstract class and itsMethodChannelBiometricStorageimplementation to handle platform-specific disposal logic. This method is called by the newdeleteAndDisposeAPI. - Example Usage Update: The example application (
example/lib/main.dart) has been updated to include a UI button that demonstrates how to use the newdeleteAndDisposeAPI across different types of biometric storage instances. - Platform-Specific Disposal Implementations: Platform-specific implementations for
disposehave been added for web and Win32, though they are currently no-operations as these platforms do not require explicit disposal for the underlying storage mechanisms.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a deleteAndDispose API to allow for re-initialization of storage at runtime. The changes in the Dart code look reasonable, and I've suggested a small refactoring in the example app to improve code clarity.
However, there is a critical issue. The new dispose method is added to the method channel, but the provided diffs do not include the necessary native implementations for Android, iOS, and macOS. Without these, the new functionality will not work on native platforms and will result in runtime errors. This needs to be addressed before merging.
| Future<void> dispose(String name, PromptInfo promptInfo) => _transformErrors( | ||
| _channel.invokeMethod('dispose', <String, dynamic>{ | ||
| 'name': name, | ||
| ..._promptInfoForCurrentPlatform(promptInfo), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This adds a method channel call for dispose. However, based on the provided diffs, the native implementations for this call on Android, iOS, and macOS are missing. This will cause a runtime error when deleteAndDispose is called on these platforms.
Please ensure that the native implementations are added to this pull request. The native dispose handler should clean up any resources associated with the storage instance, such as removing it from the in-memory map of active storages (e.g., storageFiles on Android and stores on iOS/macOS).
| if (_authStorage != null) { | ||
| await _authStorage!.deleteAndDispose(); | ||
| _authStorage = null; | ||
| _logger.finer( | ||
| 'Deleted and disposed authenticated storage'); | ||
| } | ||
| if (_storage != null) { | ||
| await _storage!.deleteAndDispose(); | ||
| _storage = null; | ||
| _logger.finer( | ||
| 'Deleted and disposed unauthenticated storage'); | ||
| } | ||
| if (_customPrompt != null) { | ||
| await _customPrompt!.deleteAndDispose(); | ||
| _customPrompt = null; | ||
| _logger.finer( | ||
| 'Deleted and disposed custom prompt storage'); | ||
| } |
There was a problem hiding this comment.
This block of code for deleting and disposing the different storages is quite repetitive. You can refactor this to reduce duplication and improve maintainability by iterating over a list of the storages. This makes the code cleaner and easier to modify if more storage types are added in the future.
final storagesToDispose = {
'authenticated': _authStorage,
'unauthenticated': _storage,
'custom prompt': _customPrompt,
};
for (final entry in storagesToDispose.entries) {
if (entry.value != null) {
await entry.value!.deleteAndDispose();
_logger.finer('Deleted and disposed ${entry.key} storage');
}
}
_authStorage = null;
_storage = null;
_customPrompt = null;
Following from our old discussion (#75) about the mismatch between the intended usage of the
forceInitoption and my understanding of it, this PR introduces the deleteAndDispose API so that library consumers can reinitialise a storage with a different configuration (e.g. authentication grace period).This is part of the fork consolidation efforts in issue #137