Skip to content

Commit

Permalink
fix: update profile-sync notification settings path hash (#4711)
Browse files Browse the repository at this point in the history
## Explanation

This is a critical fix that ensures that we use the correct path hash
used in extension for creating UserStorage for notification settings.

```diff
- notificationSettings
+ notification_settings
```

## References

https://consensyssoftware.atlassian.net/browse/NOTIFY-1115

## Changelog

### `@metamask/profile-sync-controller`

- **CHANGED**: **BREAKING** Updates the profile sync
`notification_settings` path hash.
  - This means that the hash for the storage entry will change.

### `@metamask/notification-services-controller`

- **CHANGED**: changes `notificationSettings` to `notification_settings`
to use correct storage schema.

## 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
  • Loading branch information
Prithpal-Sooriya authored Sep 17, 2024
1 parent b4ccb42 commit 091b8ec
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,13 @@ export default class NotificationServicesController extends BaseController<
getNotificationStorage: async () => {
return await this.messagingSystem.call(
'UserStorageController:performGetStorage',
'notifications.notificationSettings',
'notifications.notification_settings',
);
},
setNotificationStorage: async (state: string) => {
return await this.messagingSystem.call(
'UserStorageController:performSetStorage',
'notifications.notificationSettings',
'notifications.notification_settings',
state,
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('user-storage/user-storage-controller - performGetStorage() tests', ()
});

const result = await controller.performGetStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
);
mockAPI.done();
expect(result).toBe(MOCK_STORAGE_DATA);
Expand All @@ -91,7 +91,7 @@ describe('user-storage/user-storage-controller - performGetStorage() tests', ()
});

await expect(
controller.performGetStorage('notifications.notificationSettings'),
controller.performGetStorage('notifications.notification_settings'),
).rejects.toThrow(expect.any(Error));
});

Expand Down Expand Up @@ -126,7 +126,7 @@ describe('user-storage/user-storage-controller - performGetStorage() tests', ()
});

await expect(
controller.performGetStorage('notifications.notificationSettings'),
controller.performGetStorage('notifications.notification_settings'),
).rejects.toThrow(expect.any(Error));
},
);
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('user-storage/user-storage-controller - performSetStorage() tests', ()
});

await controller.performSetStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
'new data',
);
expect(mockAPI.isDone()).toBe(true);
Expand All @@ -242,7 +242,7 @@ describe('user-storage/user-storage-controller - performSetStorage() tests', ()

await expect(
controller.performSetStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
'new data',
),
).rejects.toThrow(expect.any(Error));
Expand Down Expand Up @@ -280,7 +280,7 @@ describe('user-storage/user-storage-controller - performSetStorage() tests', ()

await expect(
controller.performSetStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
'new data',
),
).rejects.toThrow(expect.any(Error));
Expand All @@ -290,7 +290,7 @@ describe('user-storage/user-storage-controller - performSetStorage() tests', ()
it('rejects if api call fails', async () => {
const { messengerMocks } = arrangeMocks({
mockAPI: mockEndpointUpsertUserStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
{ status: 500 },
),
});
Expand All @@ -300,7 +300,7 @@ describe('user-storage/user-storage-controller - performSetStorage() tests', ()
});
await expect(
controller.performSetStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
'new data',
),
).rejects.toThrow(expect.any(Error));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const createMockAllFeatureEntriesResponse = async (
);

export const getMockUserStorageGetResponse = async (
path: UserStoragePathWithFeatureAndKey = 'notifications.notificationSettings',
path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
) => {
return {
url: getMockUserStorageEndpoint(path),
Expand All @@ -72,7 +72,7 @@ export const getMockUserStorageAllFeatureEntriesResponse = async (
};

export const getMockUserStoragePutResponse = (
path: UserStoragePathWithFeatureAndKey = 'notifications.notificationSettings',
path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
) => {
return {
url: getMockUserStorageEndpoint(path),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const mockEndpointGetUserStorageAllFeatureEntries = async (
};

export const mockEndpointGetUserStorage = async (
path: UserStoragePathWithFeatureAndKey = 'notifications.notificationSettings',
path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
mockReply?: MockReply,
) => {
const mockResponse = await getMockUserStorageGetResponse(path);
Expand All @@ -50,7 +50,7 @@ export const mockEndpointGetUserStorage = async (
};

export const mockEndpointUpsertUserStorage = (
path: UserStoragePathWithFeatureAndKey = 'notifications.notificationSettings',
path: UserStoragePathWithFeatureAndKey = 'notifications.notification_settings',
mockReply?: Pick<MockReply, 'status'>,
) => {
const mockResponse = getMockUserStoragePutResponse(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ describe('user-storage/schema.ts', () => {
it('should correctly construct user storage url', () => {
expect(
createEntryPath(
'notifications.notificationSettings',
'notifications.notification_settings',
'dbdc994804e591f7bef6695e525543712358dd5c952bd257560b629887972588',
),
).toBe(
'notifications/2072257b71d53b6cb8e72bab8e801e3d66faa0d5e1b822c88af466127e5e763b',
'notifications/94739860a3472f61e0802706abbbbf7c8d843f8ec0ad0bef3964e52fb9b72132',
);
});

Expand Down Expand Up @@ -49,11 +49,11 @@ describe('user-storage/schema.ts', () => {
});

it('should return feature and key from path', () => {
const path = 'notifications.notificationSettings';
const path = 'notifications.notification_settings';
const result = getFeatureAndKeyFromPath(path);
expect(result).toStrictEqual({
feature: 'notifications',
key: 'notificationSettings',
key: 'notification_settings',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { createSHA256Hash } from './encryption';
const ALLOW_ARBITRARY_KEYS = 'ALLOW_ARBITRARY_KEYS' as const;

export const USER_STORAGE_SCHEMA = {
notifications: ['notificationSettings'],
notifications: ['notification_settings'],
accounts: [ALLOW_ARBITRARY_KEYS], // keyed by account addresses
networks: [ALLOW_ARBITRARY_KEYS], // keyed by chains/networks
} as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('user-storage/services.ts - getUserStorage() tests', () => {
const actCallGetUserStorage = async () => {
return await getUserStorage({
bearerToken: 'MOCK_BEARER_TOKEN',
path: 'notifications.notificationSettings',
path: 'notifications.notification_settings',
storageKey: MOCK_STORAGE_KEY,
});
};
Expand All @@ -34,7 +34,7 @@ describe('user-storage/services.ts - getUserStorage() tests', () => {

it('returns null if endpoint does not have entry', async () => {
const mockGetUserStorage = await mockEndpointGetUserStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
{ status: 404 },
);
const result = await actCallGetUserStorage();
Expand All @@ -45,7 +45,7 @@ describe('user-storage/services.ts - getUserStorage() tests', () => {

it('returns null if endpoint fails', async () => {
const mockGetUserStorage = await mockEndpointGetUserStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
{ status: 500 },
);
const result = await actCallGetUserStorage();
Expand All @@ -60,7 +60,7 @@ describe('user-storage/services.ts - getUserStorage() tests', () => {
Data: 'Bad Encrypted Data',
};
const mockGetUserStorage = await mockEndpointGetUserStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
{
status: 200,
body: badResponseData,
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('user-storage/services.ts - upsertUserStorage() tests', () => {
const encryptedData = await MOCK_ENCRYPTED_STORAGE_DATA();
return await upsertUserStorage(encryptedData, {
bearerToken: 'MOCK_BEARER_TOKEN',
path: 'notifications.notificationSettings',
path: 'notifications.notification_settings',
storageKey: MOCK_STORAGE_KEY,
});
};
Expand All @@ -149,7 +149,7 @@ describe('user-storage/services.ts - upsertUserStorage() tests', () => {

it('throws error if unable to upsert user storage', async () => {
const mockUpsertUserStorage = mockEndpointUpsertUserStorage(
'notifications.notificationSettings',
'notifications.notification_settings',
{
status: 500,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type MockReply = {
// Example mock notifications storage entry (wildcard)
const MOCK_STORAGE_URL = STORAGE_URL(
Env.DEV,
'notifications/notificationSettings',
'notifications/notification_settings',
);
const MOCK_STORAGE_URL_ALL_FEATURE_ENTRIES = STORAGE_URL(
Env.DEV,
Expand Down
16 changes: 8 additions & 8 deletions packages/profile-sync-controller/src/sdk/user-storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ describe('User Storage', () => {

// Test Set
const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
await userStorage.setItem('notifications.notificationSettings', data);
await userStorage.setItem('notifications.notification_settings', data);
expect(mockPut.isDone()).toBe(true);
expect(mockGet.isDone()).toBe(false);

// Test Get (we expect the mocked encrypted data to be decrypt-able with the given Mock Storage Key)
const response = await userStorage.getItem(
'notifications.notificationSettings',
'notifications.notification_settings',
);
expect(mockGet.isDone()).toBe(true);
expect(response).toBe(data);
Expand All @@ -63,13 +63,13 @@ describe('User Storage', () => {

// Test Set
const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
await userStorage.setItem('notifications.notificationSettings', data);
await userStorage.setItem('notifications.notification_settings', data);
expect(mockPut.isDone()).toBe(true);
expect(mockGet.isDone()).toBe(false);

// Test Get (we expect the mocked encrypted data to be decrypt-able with the given Mock Storage Key)
const response = await userStorage.getItem(
'notifications.notificationSettings',
'notifications.notification_settings',
);
expect(mockGet.isDone()).toBe(true);
expect(response).toBe(data);
Expand Down Expand Up @@ -103,7 +103,7 @@ describe('User Storage', () => {

const data = JSON.stringify(MOCK_NOTIFICATIONS_DATA);
await expect(
userStorage.setItem('notifications.notificationSettings', data),
userStorage.setItem('notifications.notification_settings', data),
).rejects.toThrow(UserStorageError);
});

Expand All @@ -120,7 +120,7 @@ describe('User Storage', () => {
});

await expect(
userStorage.getItem('notifications.notificationSettings'),
userStorage.getItem('notifications.notification_settings'),
).rejects.toThrow(UserStorageError);
});

Expand All @@ -137,7 +137,7 @@ describe('User Storage', () => {
});

await expect(
userStorage.getItem('notifications.notificationSettings'),
userStorage.getItem('notifications.notification_settings'),
).rejects.toThrow(NotFoundError);
});

Expand All @@ -152,7 +152,7 @@ describe('User Storage', () => {
handleMockUserStoragePut();

await userStorage.setItem(
'notifications.notificationSettings',
'notifications.notification_settings',
'some fake data',
);
expect(mockAuthSignMessage).toHaveBeenCalled(); // SignMessage called since generating new key
Expand Down

0 comments on commit 091b8ec

Please sign in to comment.