From a89e54a7c90125568e6f91893d9d9e2e7649c220 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Tue, 20 Apr 2021 21:35:06 +0200 Subject: [PATCH] Use redux-persist migration on settings files (#3937) * Use redux-persist migration on settings files * Stop assuming assets exists on accounts assets * Remove useless checks and custom migrations on import * Fix tests * add verification for foreigner keys in settings file * remove console.log * fix test * clarify canImport * handle existing rates in settings * add yarn.lock * revert yarn.lock changes * remove canIMport * fix test --- src/services/Store/store/account.slice.ts | 4 +- src/services/Store/store/helpers.spec.ts | 50 +------------------ src/services/Store/store/helpers.ts | 28 ----------- src/services/Store/store/persist.config.ts | 8 +-- src/services/Store/store/root.reducer.test.ts | 15 ++++-- src/services/Store/store/root.reducer.ts | 22 ++++---- 6 files changed, 32 insertions(+), 95 deletions(-) diff --git a/src/services/Store/store/account.slice.ts b/src/services/Store/store/account.slice.ts index 78ccb94d49c..cba3cbbd5b2 100644 --- a/src/services/Store/store/account.slice.ts +++ b/src/services/Store/store/account.slice.ts @@ -121,12 +121,12 @@ export const selectTxsByStatus = (status: ITxStatus) => export const getAccountsAssets = createSelector([getAccounts, (s) => s], (a, s) => a .flatMap((a) => a.assets) - .reduce((acc, asset) => [...acc, getAssetByUUID(asset.uuid)(s)!], [] as ExtendedAsset[]) + .reduce((acc, asset) => [...acc, getAssetByUUID(asset.uuid)(s)], [] as ExtendedAsset[]) ); export const getAccountsAssetsMappings = createSelector([getAccountsAssets], (assets) => assets.reduce( - (acc, a) => ({ ...acc, [a.uuid]: a.mappings }), + (acc, a) => (a ? { ...acc, [a.uuid]: a.mappings } : acc), {} as Record ) ); diff --git a/src/services/Store/store/helpers.spec.ts b/src/services/Store/store/helpers.spec.ts index e77873d1866..a3a43f7c7d6 100644 --- a/src/services/Store/store/helpers.spec.ts +++ b/src/services/Store/store/helpers.spec.ts @@ -1,14 +1,11 @@ -import { fAccount, fAccounts, fAssets, fLocalStorage, fNetworks, fRates } from '@fixtures'; -import { deMarshallState, marshallState } from '@services/Store/DataManager/utils'; -import { IProvidersMappings, LocalStorage, LSKeys, NodeOptions, StoreAsset, TUuid } from '@types'; +import { fAccount, fAccounts, fAssets, fNetworks } from '@fixtures'; +import { IProvidersMappings, NodeOptions, StoreAsset, TUuid } from '@types'; import { buildCoinGeckoIdMapping, - canImport, destructureCoinGeckoIds, mergeAssets, mergeNetworks, - migrateConfig, serializeAccount, serializeNotification } from './helpers'; @@ -144,49 +141,6 @@ describe('mergeAssets', () => { }); }); -describe('canImport()', () => { - const persistable = deMarshallState(marshallState(fLocalStorage)); - it('returns true with valid import file', () => { - const actual = canImport(fLocalStorage, persistable); - expect(actual).toBe(true); - }); - - it('returns false with mismatching versions', () => { - const validate = () => canImport({ ...fLocalStorage, version: 'v0.0' }, persistable); - expect(validate()).toBe(false); - }); - - it('returns false with missing keys', () => { - const { accounts, ...lsWithoutAccounts } = fLocalStorage; - const actual = canImport(lsWithoutAccounts, persistable); - expect(actual).toBe(false); - }); -}); - -describe('migrateConfig()', () => { - it('Migrate rates outside of settings', () => { - const toMigrate = { - [LSKeys.SETTINGS]: { - rates: fRates - } - }; - const result = migrateConfig((toMigrate as unknown) as Partial); - - expect(result.rates).toEqual(toMigrate.settings.rates); - expect(result.settings).not.toContain(toMigrate.settings.rates); - }); - it('Creates trackedAsset object if missing', () => { - const toMigrate = { - [LSKeys.SETTINGS]: { - rates: fRates - } - }; - const result = migrateConfig((toMigrate as unknown) as Partial); - - expect(result.trackedAssets).toEqual({}); - }); -}); - describe('destructureCoinGeckoIds()', () => { it('returns a new object with asset uuid as key', () => { const rates = { diff --git a/src/services/Store/store/helpers.ts b/src/services/Store/store/helpers.ts index 7cb64bb2e1d..09e8303fb75 100644 --- a/src/services/Store/store/helpers.ts +++ b/src/services/Store/store/helpers.ts @@ -4,7 +4,6 @@ import { IAccount, IProvidersMappings, IRates, - LocalStorage, Network, NodeOptions, StoreAccount, @@ -12,13 +11,10 @@ import { } from '@types'; import { bigify, isBigish, isVoid } from '@utils'; import { - difference, - dissoc, either, identity, ifElse, isNil, - keys, lensPath, lensProp, map, @@ -94,30 +90,6 @@ export const mergeAssets = (inbound: ExtendedAsset[], original: ExtendedAsset[]) }) .concat(inbound.filter((i) => !original.find((o) => o.uuid === i.uuid))); -/** - * Compare json to import with our persist state - */ -export const canImport = (toImport: Partial, store: LocalStorage) => { - if (toImport.version !== store.version) { - return false; - } else { - // Check that all the keys in the store exist in the file to import - const diff = difference(keys(store), keys(toImport)); - return diff.length === 0; - } -}; - -export const migrateConfig = (toImport: Partial) => { - return { - ...toImport, - // @ts-expect-error rates are present in settings on data to be migrated, want to move it at root of persistence layer - rates: toImport.settings?.rates ? toImport.settings.rates : toImport.rates, - trackedAssets: toImport.trackedAssets ? toImport.trackedAssets : {}, - // @ts-expect-error rates are present in settings on data to be migrated, want to move it at root of persistence layer - settings: toImport.settings?.rates ? dissoc('rates', toImport.settings) : toImport.settings - } as LocalStorage; -}; - export const destructureCoinGeckoIds = ( rates: IRates, coinGeckoIdMapping: Record diff --git a/src/services/Store/store/persist.config.ts b/src/services/Store/store/persist.config.ts index 287219562ca..ceb8b4e2487 100644 --- a/src/services/Store/store/persist.config.ts +++ b/src/services/Store/store/persist.config.ts @@ -158,7 +158,7 @@ export const migrations = { return { ...state, // @ts-expect-error rates are present in settings on data to be migrated, want to move it at root of persistence layer - rates: state.settings.rates && state.settings.rates, + rates: state.rates ? state.rates : state.settings.rates ? state.settings.rates : [], trackedAssets: state.trackedAssets ? state.trackedAssets : [], settings: dissoc('rates', state.settings) }; @@ -171,6 +171,9 @@ export const migrations = { } }; +// @ts-expect-error: bad type for migrations +export const migrate = createMigrate(migrations, { debug: IS_DEV }); + export const APP_PERSIST_CONFIG: PersistConfig = { version: 5, key: 'Storage', @@ -182,8 +185,7 @@ export const APP_PERSIST_CONFIG: PersistConfig = { // @ts-expect-error: deserialize is redux-persist internal deserialize: customDeserializer, debug: IS_DEV, - // @ts-expect-error: bad type for migrations - migrate: createMigrate(migrations, { debug: IS_DEV }) + migrate }; export const createPersistReducer = (reducer: Reducer) => diff --git a/src/services/Store/store/root.reducer.test.ts b/src/services/Store/store/root.reducer.test.ts index fdcbb962885..76e6ca7607d 100644 --- a/src/services/Store/store/root.reducer.test.ts +++ b/src/services/Store/store/root.reducer.test.ts @@ -1,4 +1,4 @@ -import { put } from 'redux-saga-test-plan/matchers'; +import { call, put } from 'redux-saga-test-plan/matchers'; import { expectSaga, mockAppState } from 'test-utils'; import { fLocalStorage } from '@fixtures'; @@ -6,6 +6,7 @@ import { marshallState } from '@services/Store/DataManager/utils'; import { omit } from '@vendor'; import importSlice from './import.slice'; +import { APP_PERSIST_CONFIG, migrate } from './persist.config'; import { appReset, exportState, importSaga, importState } from './root.reducer'; describe('Import - Export', () => { @@ -17,19 +18,27 @@ describe('Import - Export', () => { it('importSaga(): updates the app state with the provided data', async () => { const importable = JSON.stringify(fLocalStorage); + const migrated = await migrate( + // @ts-expect-error: We don't provide _persist object to migrate + marshallState(JSON.parse(importable)), + APP_PERSIST_CONFIG.version! + ); return expectSaga(importSaga) .withState(mockAppState()) .dispatch(importState(importable)) .silentRun() .then(({ effects }) => { expect(effects.put).toHaveLength(2); - expect(effects.put[0]).toEqual(put(appReset(marshallState(fLocalStorage)))); + expect(effects.call[0]).toEqual( + call(migrate, marshallState(JSON.parse(importable)), APP_PERSIST_CONFIG.version!) + ); + expect(effects.put[0]).toEqual(put(appReset(migrated))); expect(effects.put[1]).toEqual(put(importSlice.actions.success())); }); }); it('importSaga(): sets error state on failure', () => { - const errorMessage = new Error('Invalid import file'); + const errorMessage = new TypeError('Cannot convert undefined or null to object'); const importable = JSON.stringify({ foo: 'made to fail' }); return expectSaga(importSaga) .withState(mockAppState()) diff --git a/src/services/Store/store/root.reducer.ts b/src/services/Store/store/root.reducer.ts index dc25c8731f0..6d13d0b986b 100644 --- a/src/services/Store/store/root.reducer.ts +++ b/src/services/Store/store/root.reducer.ts @@ -1,16 +1,15 @@ import { AnyAction, createAction, createSelector, PayloadAction } from '@reduxjs/toolkit'; import { combineReducers } from 'redux'; -import { put } from 'redux-saga-test-plan/matchers'; -import { select, takeLatest } from 'redux-saga/effects'; +import { call, put, takeLatest } from 'redux-saga/effects'; import { featureFlagSlice } from '@services/FeatureFlag'; import { deMarshallState, marshallState } from '@services/Store/DataManager/utils'; +import { LocalStorage } from '@types'; -import { canImport, migrateConfig } from './helpers'; import importSlice from './import.slice'; import { initialLegacyState } from './legacy.initialState'; import membershipSlice from './membership.slice'; -import { createPersistReducer } from './persist.config'; +import { APP_PERSIST_CONFIG, createPersistReducer, migrate } from './persist.config'; import persistenceSlice from './persistence.slice'; import { getAppState } from './selectors'; import tokenScanningSlice from './tokenScanning.slice'; @@ -59,15 +58,16 @@ export function* importSaga() { } function* importWorker({ payload }: PayloadAction) { - const persistable = yield select(exportState); try { - const json = migrateConfig(JSON.parse(payload)); + const settings = JSON.parse(payload); - if (!canImport(json, persistable)) { - throw new Error('Invalid import file'); - } - - yield put(appReset(marshallState(json))); + const migrated: LocalStorage = yield call( + // @ts-expect-error: bad type choice on call effect + migrate, + marshallState(settings), + APP_PERSIST_CONFIG.version! + ); + yield put(appReset(migrated)); yield put(importSlice.actions.success()); } catch (err) { yield put(importSlice.actions.error(err));