Skip to content

Conversation

@Zfinix
Copy link
Contributor

@Zfinix Zfinix commented Mar 21, 2022

This creates methods to clear data from SecureDataStore, KeyInfoStorage and PlainDataStore

@Zfinix Zfinix requested review from andrzejchm and wal33d006 March 21, 2022 08:18
@Zfinix Zfinix self-assigned this Mar 21, 2022
@Zfinix Zfinix changed the title Feat/clear local storage feat: clear local storage Mar 21, 2022
final _secureCleared = await _secureDataStore.clearAllData();
final _plainCleared = await _plainDataStore.clearAllData();
_secureCleared.fold(left, right);
return _plainCleared.fold(left, right);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secure and plain data clearing should be in separate methods both returning Future<Either<CredentialsStorageFailure, bool>> and both of them separately being called in this clearCredentials function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this should be wrapped in try catch block.

await storageFile.delete();
return right(true);
} catch (e) {
return right(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be

catch(ex, stack) {
return left(CredentialsStorageFailure(
              'Error while clearing data in Biometric storage',
              cause: ex,
              stack: stack,
            ))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Contributor

@wal33d006 wal33d006 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
Just a few minor comments!

Copy link
Contributor

@andrzejchm andrzejchm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work! just few minor comments from my side

@Zfinix
Copy link
Contributor Author

Zfinix commented Mar 26, 2022

This is finally ready to be merged @andrzejchm @wal33d006

@andrzejchm
Copy link
Contributor

love it!

@andrzejchm andrzejchm merged commit 6420c05 into main Mar 28, 2022
@andrzejchm andrzejchm deleted the feat/clear-local-storage branch March 28, 2022 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants