From 31536e56ce6f80d0698b0902fa0870932c12afc9 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 2 Mar 2021 12:46:51 -0330 Subject: [PATCH] Add BaseControllerV2 state metadata (#371) State metadata has been added to the new BaseController constructor as a required constructor parameter. The metadata describes how to derive the state that should be persisted, and how to derive an 'anonymized' representation of the controller state. The metadata describes top-level properties only, but it allows you to define a function to derive the anonymized or persistent state for each property. The only requirement of this derivation function is that the output is also valid JSON. This is part of the controller redesign (#337). --- src/BaseControllerV2.test.ts | 309 +++++++++++++++++++++++++++++++++-- src/BaseControllerV2.ts | 106 +++++++++++- 2 files changed, 400 insertions(+), 15 deletions(-) diff --git a/src/BaseControllerV2.test.ts b/src/BaseControllerV2.test.ts index 5dd8f66aa23..dd1161497c1 100644 --- a/src/BaseControllerV2.test.ts +++ b/src/BaseControllerV2.test.ts @@ -1,12 +1,19 @@ import type { Draft } from 'immer'; import * as sinon from 'sinon'; -import { BaseController } from './BaseControllerV2'; +import { BaseController, getAnonymizedState, getPersistentState } from './BaseControllerV2'; type MockControllerState = { count: number; }; +const mockControllerStateMetadata = { + count: { + persist: true, + anonymous: true, + }, +}; + class MockController extends BaseController { update(callback: (state: Draft) => void | MockControllerState) { super.update(callback); @@ -19,13 +26,19 @@ class MockController extends BaseController { describe('BaseController', () => { it('should set initial state', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); expect(controller.state).toEqual({ count: 0 }); }); + it('should set initial schema', () => { + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); + + expect(controller.metadata).toEqual(mockControllerStateMetadata); + }); + it('should not allow mutating state directly', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); expect(() => { controller.state = { count: 1 }; @@ -33,7 +46,7 @@ describe('BaseController', () => { }); it('should allow updating state by modifying draft', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); controller.update((draft) => { draft.count += 1; @@ -43,7 +56,7 @@ describe('BaseController', () => { }); it('should allow updating state by return a value', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); controller.update(() => { return { count: 1 }; @@ -53,7 +66,7 @@ describe('BaseController', () => { }); it('should throw an error if update callback modifies draft and returns value', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); expect(() => { controller.update((draft) => { @@ -64,7 +77,7 @@ describe('BaseController', () => { }); it('should inform subscribers of state changes', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); const listener2 = sinon.stub(); @@ -81,7 +94,7 @@ describe('BaseController', () => { }); it('should inform a subscriber of each state change once even after multiple subscriptions', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); controller.subscribe(listener1); @@ -95,7 +108,7 @@ describe('BaseController', () => { }); it('should no longer inform a subscriber about state changes after unsubscribing', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); controller.subscribe(listener1); @@ -108,7 +121,7 @@ describe('BaseController', () => { }); it('should no longer inform a subscriber about state changes after unsubscribing once, even if they subscribed many times', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); controller.subscribe(listener1); @@ -122,7 +135,7 @@ describe('BaseController', () => { }); it('should allow unsubscribing listeners who were never subscribed', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); expect(() => { @@ -131,7 +144,7 @@ describe('BaseController', () => { }); it('should no longer update subscribers after being destroyed', () => { - const controller = new MockController({ count: 0 }); + const controller = new MockController({ count: 0 }, mockControllerStateMetadata); const listener1 = sinon.stub(); const listener2 = sinon.stub(); @@ -146,3 +159,275 @@ describe('BaseController', () => { expect(listener2.callCount).toEqual(0); }); }); + +describe('getAnonymizedState', () => { + it('should return empty state', () => { + expect(getAnonymizedState({}, {})).toEqual({}); + }); + + it('should return empty state when no properties are anonymized', () => { + const anonymizedState = getAnonymizedState({ count: 1 }, { count: { anonymous: false, persist: false } }); + expect(anonymizedState).toEqual({}); + }); + + it('should return state that is already anonymized', () => { + const anonymizedState = getAnonymizedState( + { + password: 'secret password', + privateKey: '123', + network: 'mainnet', + tokens: ['DAI', 'USDC'], + }, + { + password: { + anonymous: false, + persist: false, + }, + privateKey: { + anonymous: false, + persist: false, + }, + network: { + anonymous: true, + persist: false, + }, + tokens: { + anonymous: true, + persist: false, + }, + }, + ); + expect(anonymizedState).toEqual({ network: 'mainnet', tokens: ['DAI', 'USDC'] }); + }); + + it('should use anonymizing function to anonymize state', () => { + const anonymizeTransactionHash = (hash: string) => { + return hash.split('').reverse().join(''); + }; + + const anonymizedState = getAnonymizedState( + { + transactionHash: '0x1234', + }, + { + transactionHash: { + anonymous: anonymizeTransactionHash, + persist: false, + }, + }, + ); + + expect(anonymizedState).toEqual({ transactionHash: '4321x0' }); + }); + + it('should allow returning a partial object from an anonymizing function', () => { + const anonymizeTxMeta = (txMeta: { hash: string; value: number }) => { + return { value: txMeta.value }; + }; + + const anonymizedState = getAnonymizedState( + { + txMeta: { + hash: '0x123', + value: 10, + }, + }, + { + txMeta: { + anonymous: anonymizeTxMeta, + persist: false, + }, + }, + ); + + expect(anonymizedState).toEqual({ txMeta: { value: 10 } }); + }); + + it('should allow returning a nested partial object from an anonymizing function', () => { + const anonymizeTxMeta = (txMeta: { hash: string; value: number; history: { hash: string; value: number }[] }) => { + return { + history: txMeta.history.map((entry) => { + return { value: entry.value }; + }), + value: txMeta.value, + }; + }; + + const anonymizedState = getAnonymizedState( + { + txMeta: { + hash: '0x123', + history: [ + { + hash: '0x123', + value: 9, + }, + ], + value: 10, + }, + }, + { + txMeta: { + anonymous: anonymizeTxMeta, + persist: false, + }, + }, + ); + + expect(anonymizedState).toEqual({ txMeta: { history: [{ value: 9 }], value: 10 } }); + }); + + it('should allow transforming types in an anonymizing function', () => { + const anonymizedState = getAnonymizedState( + { + count: '1', + }, + { + count: { + anonymous: (count) => Number(count), + persist: false, + }, + }, + ); + + expect(anonymizedState).toEqual({ count: 1 }); + }); +}); + +describe('getPersistentState', () => { + it('should return empty state', () => { + expect(getPersistentState({}, {})).toEqual({}); + }); + + it('should return empty state when no properties are persistent', () => { + const persistentState = getPersistentState({ count: 1 }, { count: { anonymous: false, persist: false } }); + expect(persistentState).toEqual({}); + }); + + it('should return persistent state', () => { + const persistentState = getPersistentState( + { + password: 'secret password', + privateKey: '123', + network: 'mainnet', + tokens: ['DAI', 'USDC'], + }, + { + password: { + anonymous: false, + persist: true, + }, + privateKey: { + anonymous: false, + persist: true, + }, + network: { + anonymous: false, + persist: false, + }, + tokens: { + anonymous: false, + persist: false, + }, + }, + ); + expect(persistentState).toEqual({ password: 'secret password', privateKey: '123' }); + }); + + it('should use function to derive persistent state', () => { + const normalizeTransacitonHash = (hash: string) => { + return hash.toLowerCase(); + }; + + const persistentState = getPersistentState( + { + transactionHash: '0X1234', + }, + { + transactionHash: { + anonymous: false, + persist: normalizeTransacitonHash, + }, + }, + ); + + expect(persistentState).toEqual({ transactionHash: '0x1234' }); + }); + + it('should allow returning a partial object from a persist function', () => { + const getPersistentTxMeta = (txMeta: { hash: string; value: number }) => { + return { value: txMeta.value }; + }; + + const persistentState = getPersistentState( + { + txMeta: { + hash: '0x123', + value: 10, + }, + }, + { + txMeta: { + anonymous: false, + persist: getPersistentTxMeta, + }, + }, + ); + + expect(persistentState).toEqual({ txMeta: { value: 10 } }); + }); + + it('should allow returning a nested partial object from a persist function', () => { + const getPersistentTxMeta = (txMeta: { + hash: string; + value: number; + history: { hash: string; value: number }[]; + }) => { + return { + history: txMeta.history.map((entry) => { + return { value: entry.value }; + }), + value: txMeta.value, + }; + }; + + const persistentState = getPersistentState( + { + txMeta: { + hash: '0x123', + history: [ + { + hash: '0x123', + value: 9, + }, + ], + value: 10, + }, + }, + { + txMeta: { + anonymous: false, + persist: getPersistentTxMeta, + }, + }, + ); + + expect(persistentState).toEqual({ txMeta: { history: [{ value: 9 }], value: 10 } }); + }); + + it('should allow transforming types in a persist function', () => { + const persistentState = getPersistentState( + { + count: '1', + }, + { + count: { + anonymous: false, + persist: (count) => Number(count), + }, + }, + ); + + expect(persistentState).toEqual({ count: 1 }); + }); +}); diff --git a/src/BaseControllerV2.ts b/src/BaseControllerV2.ts index f5821a4a980..8dae16f132c 100644 --- a/src/BaseControllerV2.ts +++ b/src/BaseControllerV2.ts @@ -7,7 +7,15 @@ import type { Draft, Patch } from 'immer'; enablePatches(); /** - * State change callbacks + * A state change listener. + * + * This function will get called for each state change, and is given a copy of + * the new state along with a set of patches describing the changes since the + * last update. + * + * @param state - The new controller state + * @param patches - A list of patches describing any changes (see here for more + * information: https://immerjs.github.io/immer/docs/patches) */ export type Listener = (state: T, patches: Patch[]) => void; @@ -43,20 +51,63 @@ export type IsJsonable = never; /** - * Controller class that provides state management and subscriptions + * An function to derive state. + * + * This function will accept one piece of the controller state (one property), + * and will return some derivation of that state. + * + * @param value - A piece of controller state + * @returns Something derived from controller state + */ +export type StateDeriver = (value: IsJsonable) => IsJsonable; + +/** + * State metadata. + * + * This metadata describes which parts of state should be persisted, and how to + * get an anonymized representation of the state. + */ +export type StateMetadata = { + [P in keyof T]: StatePropertyMetadata; +}; + +/** + * Metadata for a single state property + * + * @property persist - Indicates whether this property should be persisted + * (`true` for persistent, `false` for transient), or is set to a function + * that derives the persistent state from the state. + * @property anonymous - Indicates whether this property is already anonymous, + * (`true` for anonymous, `false` if it has potential to be personally + * identifiable), or is set to a function that returns an anonymized + * representation of this state. + */ +export interface StatePropertyMetadata { + persist: boolean | StateDeriver; + anonymous: boolean | StateDeriver; +} + +type Json = null | boolean | number | string | Json[] | { [prop: string]: Json } | Partial>; +/** + * Controller class that provides state management, subscriptions, and state metadata */ export class BaseController> { private internalState: IsJsonable; private internalListeners: Set> = new Set(); + public readonly metadata: StateMetadata; + /** * Creates a BaseController instance. * * @param state - Initial controller state + * @param metadata - State metadata, describing how to "anonymize" the state, + * and which parts should be persisted. */ - constructor(state: IsJsonable) { + constructor(state: IsJsonable, metadata: StateMetadata) { this.internalState = state; + this.metadata = metadata; } /** @@ -120,3 +171,52 @@ export class BaseController> { this.internalListeners.clear(); } } + +/** + * Returns an anonymized representation of the controller state. + * + * By "anonymized" we mean that it should not contain any information that could be personally + * identifiable. + * + * @param state - The controller state + * @param metadata - The controller state metadata, which describes how to derive the + * anonymized state + * @returns The anonymized controller state + */ +export function getAnonymizedState>( + state: IsJsonable, + metadata: StateMetadata, +): IsJsonable> { + return deriveStateFromMetadata(state, metadata, 'anonymous'); +} + +/** + * Returns the subset of state that should be persisted + * + * @param state - The controller state + * @param metadata - The controller state metadata, which describes which pieces of state should be persisted + * @returns The subset of controller state that should be persisted + */ +export function getPersistentState>( + state: IsJsonable, + metadata: StateMetadata, +): IsJsonable> { + return deriveStateFromMetadata(state, metadata, 'persist'); +} + +function deriveStateFromMetadata>( + state: IsJsonable, + metadata: StateMetadata, + metadataProperty: 'anonymous' | 'persist', +): IsJsonable> { + return Object.keys(state).reduce((persistedState, key) => { + const propertyMetadata = metadata[key as keyof S][metadataProperty]; + const stateProperty = state[key]; + if (typeof propertyMetadata === 'function') { + persistedState[key as string] = propertyMetadata(stateProperty); + } else if (propertyMetadata) { + persistedState[key as string] = stateProperty; + } + return persistedState; + }, {} as IsJsonable>); +}