Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Migrate AsyncStorage to mmkv #10153

Merged
merged 20 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b6969e6
AsyncStorage to mmkv migration
kylanhurt Jun 27, 2024
ad9283d
Remove unnecessary console logs
kylanhurt Jun 27, 2024
f363877
Remove unnecessary awaits for synchronous mmkv
kylanhurt Jun 27, 2024
356e887
Have asyncStorage attempt to provide value (and set to mmkv) if no mm…
kylanhurt Jun 28, 2024
6c3b286
Add some relevant comments regarding eventual (complete) migration aw…
kylanhurt Jun 28, 2024
aa2809e
Fix asyncStorage vs mmkv null vs undefined values
kylanhurt Jun 28, 2024
6ee0960
Adjust storage wrapper to pass e2e tests
kylanhurt Jul 1, 2024
00dd7eb
Update app/store/async-storage-wrapper.js
kylanhurt Jul 2, 2024
53d0e03
Update app/store/async-storage-wrapper.js
kylanhurt Jul 2, 2024
8310170
Update app/store/async-storage-wrapper.js
kylanhurt Jul 2, 2024
6947075
Some fixes to storage wrapper migration
kylanhurt Jul 2, 2024
b7ad9a6
Merge branch 'kylan/mmkv-final' of https://github.com/MetaMask/metama…
kylanhurt Jul 2, 2024
bb14c0e
Minor formatting fix
kylanhurt Jul 2, 2024
c428383
More minor fixes to asynt storage wrapper
kylanhurt Jul 2, 2024
fff043d
Merge branch 'main' into kylan/mmkv-final
kylanhurt Jul 2, 2024
05ce522
Merge branch 'main' into kylan/mmkv-final
kylanhurt Jul 3, 2024
7180c74
Merge branch 'main' into kylan/mmkv-final
kylanhurt Jul 3, 2024
8bbb23b
Remove asyncStorage limit from gradle properties
kylanhurt Jul 3, 2024
4b0e45a
Merge branch 'kylan/mmkv-final' of https://github.com/MetaMask/metama…
kylanhurt Jul 3, 2024
f002f17
Remove unnecessary console.error
kylanhurt Jul 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions android/gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,5 @@ hermesEnabled=true
# TODO: explain following config options
android.disableResourceValidation=true
android.enableDexingArtifactTransform.desugaring=false
# will be able to get rid of this after migration to mmkv (from AsyncStorage)
AsyncStorage_db_size_in_MB=10
4 changes: 2 additions & 2 deletions app/core/Authentication/Authentication.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import AsyncStorage from '@react-native-async-storage/async-storage';
import AsyncStorage from '../../store/async-storage-wrapper';
import {
BIOMETRY_CHOICE_DISABLED,
TRUE,
Expand Down Expand Up @@ -27,7 +27,7 @@ describe('Authentication', () => {
});

afterEach(() => {
AsyncStorage.clear();
AsyncStorage.clearAll();
jest.restoreAllMocks();
});

Expand Down
30 changes: 25 additions & 5 deletions app/store/async-storage-wrapper.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,49 @@
import ReadOnlyNetworkStore from '../util/test/network-store';
import AsyncStorage from '@react-native-async-storage/async-storage';
import { isE2E } from '../util/test/utils';
import { MMKV } from 'react-native-mmkv';

/**
* Wrapper class for AsyncStorage.
* (Will want to eventuall re-name since no longer async once migratted to mmkv)
*/
class AsyncStorageWrapper {
constructor() {
/**
* The underlying storage implementation.
* Use `ReadOnlyNetworkStore` in test mode otherwise use `AsyncStorage`.
*/
this.storage = isE2E ? ReadOnlyNetworkStore : AsyncStorage;
this.storage = isE2E ? ReadOnlyNetworkStore : new MMKV();
}

async getItem(key) {
try {
return await this.storage.getItem(key);
// asyncStorage returns null for no value
// mmkv returns undefined for no value
// therefore must return null if no value is found
// to keep app behavior consistent
let value = (await this.storage.getString(key)) || null;
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
if (!value) {
const asyncStorageValue = await AsyncStorage.getItem(key);
if (asyncStorageValue) {
value = asyncStorageValue;
this.storage.set(key, value);
}
}
return value;
} catch (error) {
if (isE2E) {
// Fall back to AsyncStorage in test mode if ReadOnlyNetworkStore fails
return AsyncStorage.getItem(key);
return await AsyncStorage.getItem(key);
}
throw error;
}
}

async setItem(key, value) {
try {
return await this.storage.setItem(key, value);
const response = await this.storage.set(key, value);
return response;
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
if (isE2E) {
// Fall back to AsyncStorage in test mode if ReadOnlyNetworkStore fails
Expand All @@ -40,7 +55,8 @@ class AsyncStorageWrapper {

async removeItem(key) {
try {
return await this.storage.removeItem(key);
const response = await this.storage.delete(key);
return response;
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
} catch (error) {
if (isE2E) {
// Fall back to AsyncStorage in test mode if ReadOnlyNetworkStore fails
Expand All @@ -49,6 +65,10 @@ class AsyncStorageWrapper {
throw error;
}
}

async clearAll() {
await this.storage.clearAll();
}
}

export default new AsyncStorageWrapper();
31 changes: 31 additions & 0 deletions app/store/migrations/049.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import migrate, { storage as mmkvStorage } from './049';
import AsyncStorage from '@react-native-async-storage/async-storage';

kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
const asyncStorageItems: { [key: string]: string } = {
valueA: 'a',
valueB: 'true',
valueC: 'myValue',
};

describe('Migration #49', () => {
it('migrates asyncStorage values to mmkv ', async () => {
// set asyncStorageItems to AsyncStorage
for (const key in asyncStorageItems) {
await AsyncStorage.setItem(key, asyncStorageItems[key]);
}

await migrate({});

// make sure all AsyncStorage items are removed
const keys = await AsyncStorage.getAllKeys();
// loop through all AsyncStorage keys and make sure empty
for (const key of keys) {
expect(await AsyncStorage.getItem(key)).toBeNull();
}

// now check that all MMKV values match original AsyncStorage values
for (const key in asyncStorageItems) {
expect(mmkvStorage.getString(key)).toEqual(asyncStorageItems[key]);
}
});
});
32 changes: 32 additions & 0 deletions app/store/migrations/049.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import AsyncStorage from '@react-native-async-storage/async-storage';
import { MMKV } from 'react-native-mmkv';

export const storage = new MMKV();

export const hasMigratedFromAsyncStorage = storage.getString(
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
'hasMigratedFromAsyncStorage',
);

export default async function migrate(state: unknown) {
const keys = await AsyncStorage.getAllKeys();
for (const key of keys) {
try {
const value = await AsyncStorage.getItem(key);

if (value != null) {
storage.set(key, value);
}
await AsyncStorage.removeItem(key);
} catch (error) {
console.error(
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
`Failed to migrate key "${key}" from AsyncStorage to MMKV!`,
error,
);
throw error;
kylanhurt marked this conversation as resolved.
Show resolved Hide resolved
}
}

storage.set('hasMigratedFromAsyncStorage', 'true');

return state;
}
2 changes: 2 additions & 0 deletions app/store/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import migration45 from './045';
import migration46 from './046';
import migration47 from './047';
import migration48 from './048';
import migration49 from './049';

type MigrationFunction = (state: unknown) => unknown;
type AsyncMigrationFunction = (state: unknown) => Promise<unknown>;
Expand Down Expand Up @@ -110,6 +111,7 @@ export const migrationList: MigrationsList = {
46: migration46,
47: migration47,
48: migration48,
49: migration49,
};

// Enable both synchronous and asynchronous migrations
Expand Down
6 changes: 3 additions & 3 deletions app/util/test/network-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,18 @@ class ReadOnlyNetworkStore {
}

// Async Storage
async getItem(key) {
async getString(key) {
await this._initIfRequired();
const value = this._asyncState[key];
return value !== undefined ? value : null;
}

async setItem(key, value) {
async set(key, value) {
await this._initIfRequired();
this._asyncState[key] = value;
}

async removeItem(key) {
async delete(key) {
await this._initIfRequired();
delete this._asyncState[key];
}
Expand Down
Loading