Migrate all AsyncStorage direct calls to our storage wrapper #1169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zatteo
force-pushed
the
refactor/use-storage-wrapper
branch
from
bd26fc5
to
1db64a8
Compare
zatteo
changed the title
Ldoppea
approved these changes
Feb 20, 2024
| @@ -66,8 +64,6 @@ export const asyncLogout = async (client?: CozyClient): Promise<null> => { | |||
|
|
|||
| await sendKonnectorsLogs(client) | |||
| await client.logout() | |||
| await clearClient() | |||
| await resetSessionToken() | |||
There was a problem hiding this comment.
seems like resetSessionToken is not called anymore so we can remove the method implementation?
clearClient seems to be used in a single place (in iconTable.ts) but i'm wondering if this is not a potential bug?
There was a problem hiding this comment.
I removed resetSessionToken.
Why do you think clearClient in iconTable.ts may be a potential bug ? cc @acezard if you have insights here
There was a problem hiding this comment.
I anticipate that clearing the client only without cleaning related CozyPersistedStorageKeys may produce bugs.
There was a problem hiding this comment.
Oh so you mean we could replace this we the clearData function. Good idea.
This method will be used to clear EVERYTHING in local storage. Will be used only in tests environment for the moment.
…ect AsyncStorage calls We add a new enum called DevicePersistedStorageKeys dedicated to values that : - are linked to the device (so only one value can be stored contrary to UserPersistedStorageKeys) - are not removed when logging out
…syncStorage calls
… AsyncStorage calls
Since Oauth and SessionCreated are now part of the StorageKeys object, they are also removed when calling clearData().
zatteo
force-pushed
the
refactor/use-storage-wrapper
branch
from
1db64a8
to
4e01eb4
Compare
Ldoppea
approved these changes
Feb 23, 2024
**Previously** - StorageKey = key where data is persisted for a cozy life (so removed at log out) - DevicePersistedStorageKeys = key where data is persisted for the device life (so NOT removed at log out) **Now** - CozyPersistedStorageKeys = key where data is persisted for a cozy (so until log out) - DevicePersistedStorageKeys = key where data is persisted for the device life (so NOT removed at log out) StorageKey = CozyPersistedStorageKeys | DevicePersistedStorageKeys There is still a 3rd enum called UserPersistedStorageKeys but it is not used with the same storage wrapper so it can not be included in the new StorageKeys.
zatteo
force-pushed
the
refactor/use-storage-wrapper
branch
from
f17f3a4
to
dadc43f
Compare
We previously renamed StorageKeys to CozyPersistedDeviceKeys. clearData method was clearing StorageKeys. It now clears CozyPersistedDeviceKeys. Let's be explicit by also renaming clearData.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now every react-native-async-storage calls are centralized in our storage wrapper. This is a prerequisite to potentially migrate to react-native-mmkv or another library.
Some cases I encountered
AsyncStorage.setItemand is parsed afterAsyncStorage.getItem=> because getData and storeData already does the stringify/parse, I removed the remaining
JSON.stringifyandJSON.parsecalls when migrating to getData and storeData. Ex. : Bundle, OauthAsyncStorage.setItem=> getData will try to parse a not stringified string if there is already a value stored directly with
AsyncStorage.setItem. I added a compatibilty check. Ex. : DefaultRedirectionUrlWhat did I test
Checklist
Before merging this PR, the following things must have been done if relevant: