feat(NOTIFY-1146): use better type system for user storage (#4907)

## Explanation This PR adds a readonly object that stores feature names so we stop relying on string values for user storage operations. This object is also now exported from the packages, in both SDK and UserStorage paths, so it can conveniently be used in external applications. ## References https://consensyssoftware.atlassian.net/browse/NOTIFY-1146 Fixes #4921 ## Changelog ### `@metamask/profile-sync-controller` - **CHANGED**: Better type system for user storage, using and exporting a new `USER_STORAGE_FEATURE_NAMES` readonly object ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
MetaMask · Nov 15, 2024 · d699559 · d699559
1 parent f63889b
commit d699559
Show file tree

Hide file tree

Showing 16 changed files with 311 additions and 187 deletions.
diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts
diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts
@@ -23,10 +23,11 @@ import type { NetworkConfiguration } from '@metamask/network-controller';
 import type { HandleSnapRequest } from '@metamask/snaps-controllers';
 
 import { createSHA256Hash } from '../../shared/encryption';
-import type {
-  UserStoragePathWithFeatureAndKey,
-  UserStoragePathWithFeatureOnly,
-  UserStoragePathWithKeyOnly,
+import type { UserStorageFeatureKeys } from '../../shared/storage-schema';
+import {
+  USER_STORAGE_FEATURE_NAMES,
+  type UserStoragePathWithFeatureAndKey,
+  type UserStoragePathWithFeatureOnly,
 } from '../../shared/storage-schema';
 import type { NativeScrypt } from '../../shared/types/encryption';
 import { createSnapSignMessageRequest } from '../authentication/auth-snap-requests';
@@ -340,7 +341,9 @@ export default class UserStorageController extends BaseController<
       UserStorageAccount[] | null
     > => {
       const rawAccountsListResponse =
-        await this.performGetStorageAllFeatureEntries('accounts');
+        await this.performGetStorageAllFeatureEntries(
+          USER_STORAGE_FEATURE_NAMES.accounts,
+        );
 
       return (
         rawAccountsListResponse?.map((rawAccount) => JSON.parse(rawAccount)) ??
@@ -355,7 +358,7 @@ export default class UserStorageController extends BaseController<
         mapInternalAccountToUserStorageAccount(internalAccount);
 
       await this.performSetStorage(
-        `accounts.${internalAccount.address}`,
+        `${USER_STORAGE_FEATURE_NAMES.accounts}.${internalAccount.address}`,
         JSON.stringify(mappedAccount),
       );
     },
@@ -371,7 +374,7 @@ export default class UserStorageController extends BaseController<
         internalAccountsList.map(mapInternalAccountToUserStorageAccount);
 
       await this.performBatchSetStorage(
-        'accounts',
+        USER_STORAGE_FEATURE_NAMES.accounts,
         internalAccountsListFormattedForUserStorage.map((account) => [
           account.a,
           JSON.stringify(account),
@@ -671,9 +674,11 @@ export default class UserStorageController extends BaseController<
    * @param values - data to store, in the form of an array of `[entryKey, entryValue]` pairs
    * @returns nothing. NOTE that an error is thrown if fails to store data.
    */
-  public async performBatchSetStorage(
-    path: UserStoragePathWithFeatureOnly,
-    values: [UserStoragePathWithKeyOnly, string][],
+  public async performBatchSetStorage<
+    FeatureName extends UserStoragePathWithFeatureOnly,
+  >(
+    path: FeatureName,
+    values: [UserStorageFeatureKeys<FeatureName>, string][],
   ): Promise<void> {
     this.#assertProfileSyncingEnabled();
 
@@ -959,7 +964,7 @@ export default class UserStorageController extends BaseController<
 
       // Save the internal accounts list to the user storage
       await this.performBatchSetStorage(
-        'accounts',
+        USER_STORAGE_FEATURE_NAMES.accounts,
         internalAccountsToBeSavedToUserStorage.map((account) => [
           account.address,
           JSON.stringify(mapInternalAccountToUserStorageAccount(account)),

diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockResponses.ts
@@ -2,7 +2,10 @@ import type {
   UserStoragePathWithFeatureAndKey,
   UserStoragePathWithFeatureOnly,
 } from '../../../shared/storage-schema';
-import { createEntryPath } from '../../../shared/storage-schema';
+import {
+  createEntryPath,
+  USER_STORAGE_FEATURE_NAMES,
+} from '../../../shared/storage-schema';
 import type {
   GetUserStorageAllFeatureEntriesResponse,
   GetUserStorageResponse,
@@ -72,7 +75,7 @@ export async function createMockAllFeatureEntriesResponse(
  * @returns mock GET API request. Can be used by e2e or unit mock servers
  */
 export async function getMockUserStorageGetResponse(
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
 ) {
   return {
     url: getMockUserStorageEndpoint(path),
@@ -88,7 +91,7 @@ export async function getMockUserStorageGetResponse(
  * @returns mock GET ALL API request. Can be used by e2e or unit mock servers
  */
 export async function getMockUserStorageAllFeatureEntriesResponse(
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
   dataArr?: string[],
 ) {
   return {
@@ -99,7 +102,7 @@ export async function getMockUserStorageAllFeatureEntriesResponse(
 }
 
 export const getMockUserStoragePutResponse = (
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
 ) => {
   return {
     url: getMockUserStorageEndpoint(path),
@@ -109,7 +112,7 @@ export const getMockUserStoragePutResponse = (
 };
 
 export const getMockUserStorageBatchPutResponse = (
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
 ) => {
   return {
     url: getMockUserStorageEndpoint(path),
@@ -119,7 +122,7 @@ export const getMockUserStorageBatchPutResponse = (
 };
 
 export const deleteMockUserStorageResponse = (
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
 ) => {
   return {
     url: getMockUserStorageEndpoint(path),
@@ -129,7 +132,7 @@ export const deleteMockUserStorageResponse = (
 };
 
 export const deleteMockUserStorageAllFeatureEntriesResponse = (
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
 ) => {
   return {
     url: getMockUserStorageEndpoint(path),

diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockServices.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockServices.ts
@@ -1,8 +1,9 @@
 import nock from 'nock';
 
-import type {
-  UserStoragePathWithFeatureAndKey,
-  UserStoragePathWithFeatureOnly,
+import {
+  USER_STORAGE_FEATURE_NAMES,
+  type UserStoragePathWithFeatureAndKey,
+  type UserStoragePathWithFeatureOnly,
 } from '../../../shared/storage-schema';
 import {
   getMockUserStorageGetResponse,
@@ -19,7 +20,7 @@ type MockReply = {
 };
 
 export const mockEndpointGetUserStorageAllFeatureEntries = async (
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
   mockReply?: MockReply,
 ) => {
   const mockResponse = await getMockUserStorageAllFeatureEntriesResponse(path);
@@ -36,7 +37,7 @@ export const mockEndpointGetUserStorageAllFeatureEntries = async (
 };
 
 export const mockEndpointGetUserStorage = async (
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
   mockReply?: MockReply,
 ) => {
   const mockResponse = await getMockUserStorageGetResponse(path);
@@ -53,7 +54,7 @@ export const mockEndpointGetUserStorage = async (
 };
 
 export const mockEndpointUpsertUserStorage = (
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
   mockReply?: Pick<MockReply, 'status'>,
   expectCallback?: (requestBody: nock.Body) => Promise<void>,
 ) => {
@@ -67,7 +68,7 @@ export const mockEndpointUpsertUserStorage = (
 };
 
 export const mockEndpointBatchUpsertUserStorage = (
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
   mockReply?: Pick<MockReply, 'status'>,
   callback?: (uri: string, requestBody: nock.Body) => Promise<void>,
 ) => {
@@ -81,7 +82,7 @@ export const mockEndpointBatchUpsertUserStorage = (
 };
 
 export const mockEndpointDeleteUserStorage = (
-  path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
+  path: UserStoragePathWithFeatureAndKey = `${USER_STORAGE_FEATURE_NAMES.notifications}.notification_settings`,
   mockReply?: MockReply,
 ) => {
   const mockResponse = deleteMockUserStorageResponse(path);
@@ -95,7 +96,7 @@ export const mockEndpointDeleteUserStorage = (
 };
 
 export const mockEndpointDeleteUserStorageAllFeatureEntries = (
-  path: UserStoragePathWithFeatureOnly = 'notifications',
+  path: UserStoragePathWithFeatureOnly = USER_STORAGE_FEATURE_NAMES.notifications,
   mockReply?: MockReply,
 ) => {
   const mockResponse = deleteMockUserStorageAllFeatureEntriesResponse(path);

diff --git a/packages/profile-sync-controller/src/controllers/user-storage/index.ts b/packages/profile-sync-controller/src/controllers/user-storage/index.ts
@@ -6,3 +6,4 @@ export default UserStorageController;
 export * from './UserStorageController';
 export * as Mocks from './__fixtures__';
 export * from '../../shared/encryption';
+export * from '../../shared/storage-schema';
diff --git a/...ges/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts b/...ges/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts
@@ -1,3 +1,4 @@
+import { USER_STORAGE_FEATURE_NAMES } from '../../../shared/storage-schema';
 import {
   MOCK_STORAGE_KEY,
   createMockAllFeatureEntriesResponse,
@@ -44,7 +45,7 @@ describe('network-syncing/services - getAllRemoteNetworks()', () => {
 
     return {
       mockGetAllAPI: await mockEndpointGetUserStorageAllFeatureEntries(
-        'networks',
+        USER_STORAGE_FEATURE_NAMES.networks,
         payload,
       ),
     };
@@ -101,7 +102,9 @@ describe('network-syncing/services - upsertRemoteNetwork()', () => {
     return {
       storageOps: storageOpts,
       mockNetwork,
-      mockUpsertAPI: mockEndpointUpsertUserStorage('networks.0x1337'),
+      mockUpsertAPI: mockEndpointUpsertUserStorage(
+        `${USER_STORAGE_FEATURE_NAMES.networks}.0x1337`,
+      ),
     };
   };
 

diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts
@@ -1,3 +1,4 @@
+import { USER_STORAGE_FEATURE_NAMES } from '../../../shared/storage-schema';
 import type { UserStorageBaseOptions } from '../services';
 import {
   getUserStorageAllFeatureEntries,
@@ -35,7 +36,7 @@ export async function getAllRemoteNetworks(
   const rawResults =
     (await getUserStorageAllFeatureEntries({
       ...opts,
-      path: 'networks',
+      path: USER_STORAGE_FEATURE_NAMES.networks,
     })) ?? [];
 
   const results = rawResults

diff --git a/...ofile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts b/...ofile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts
@@ -1,5 +1,6 @@
 import type { NetworkConfiguration } from '@metamask/network-controller';
 
+import { USER_STORAGE_FEATURE_NAMES } from '../../../shared/storage-schema';
 import { MOCK_STORAGE_KEY } from '../__fixtures__';
 import { mockEndpointUpsertUserStorage } from '../__fixtures__/mockServices';
 import type { UserStorageBaseOptions } from '../services';
@@ -37,7 +38,9 @@ const testMatrix = [
 describe('network-syncing/sync - updateNetwork() / addNetwork() / deleteNetwork()', () => {
   it.each(testMatrix)('should successfully call $fnName', async ({ act }) => {
     const mockNetwork = arrangeMockNetwork();
-    const mockUpsertAPI = mockEndpointUpsertUserStorage('networks.0x1337');
+    const mockUpsertAPI = mockEndpointUpsertUserStorage(
+      `${USER_STORAGE_FEATURE_NAMES.networks}.0x1337`,
+    );
     await act(mockNetwork);
     expect(mockUpsertAPI.isDone()).toBe(true);
   });
@@ -46,9 +49,12 @@ describe('network-syncing/sync - updateNetwork() / addNetwork() / deleteNetwork(
     'should throw error when calling $fnName when API fails',
     async ({ act }) => {
       const mockNetwork = arrangeMockNetwork();
-      const mockUpsertAPI = mockEndpointUpsertUserStorage('networks.0x1337', {
-        status: 500,
-      });
+      const mockUpsertAPI = mockEndpointUpsertUserStorage(
+        `${USER_STORAGE_FEATURE_NAMES.networks}.0x1337`,
+        {
+          status: 500,
+        },
+      );
       await expect(async () => await act(mockNetwork)).rejects.toThrow(
         expect.any(Error),
       );