Skip to content

Commit b4a3748

Browse files
authored
Migrate SampleGasPriceService to BaseDataService (#8343)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> With the release of `@metamask/base-data-service`, the `SampleGasPricesService` class needs to be updated so that new data services follow our best practices. ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> https://consensyssoftware.atlassian.net/browse/WPC-942 ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Introduces breaking API changes (service inheritance and removed `onRetry`/`onBreak`/`onDegraded`) and changes request behavior to be query-cached/deduplicated, which could affect consumers and error/retry semantics. > > **Overview** > Migrates `SampleGasPricesService` to extend `@metamask/base-data-service`’s `BaseDataService`, switching `fetchGasPrices` to use `fetchQuery` (TanStack Query) so API calls are **cached and deduplicated** and adding optional `queryClientConfig` for cache/client tuning. > > Adds new messenger surface area for cache management/observability (`:invalidateQueries` action plus `cacheUpdated` events), exports the new types from `src/index.ts`, and updates tests accordingly while removing the previous `onRetry`/`onBreak`/`onDegraded` hooks. Dependency and TS project references are updated to include `@metamask/base-data-service`, `@tanstack/query-core`, and `@metamask/superstruct` (used for response validation). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3a60272. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 011fcad commit b4a3748

8 files changed

Lines changed: 155 additions & 355 deletions

File tree

packages/sample-controllers/CHANGELOG.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add actions and events for accessing and interacting with the new query cache for `SampleGasPricesService` ([#8343](https://github.com/MetaMask/core/pull/8343))
13+
- New actions and events are:
14+
- `SampleGasPricesServiceInvalidateQueriesAction`
15+
- `SampleGasPricesServiceCacheUpdatedEvent`
16+
- `SampleGasPricesServiceGranularCacheUpdatedEvent`
17+
- Additionally, the actions are available within `SampleGasPricesServiceActions` and the events are available within `SampleGasPricesServiceEvents`
18+
- Add optional `queryClientConfig` constructor argument which can be used to configure the underlying TanStack Query client ([#8343](https://github.com/MetaMask/core/pull/8343))
19+
- Add `destroy` method to `BaseDataService` ([#8343](https://github.com/MetaMask/core/pull/8343))
20+
21+
### Changed
22+
23+
- **BREAKING:** `SampleGasPricesService` now inherits from `BaseDataService` from `@metamask/base-data-service` ([#8343](https://github.com/MetaMask/core/pull/8343))
24+
- Update `SampleGasPricesService.fetchGasPrices` (and messenger action) so requests to API will be cached and/or deduplicated ([#8343](https://github.com/MetaMask/core/pull/8343))
25+
- Add new dependencies ([#8343](https://github.com/MetaMask/core/pull/8343))
26+
- Add `@metamask/base-data-service` ^0.1.1
27+
- Add `@tanstack/query-core` ^4.43.0
28+
- Add `@metamask/superstruct` ^3.2.1
29+
30+
### Removed
31+
32+
- **BREAKING:** Remove `onRetry`, `onBreak`, and `onDegraded` ([#8343](https://github.com/MetaMask/core/pull/8343))
33+
- You are free to implement these methods in your "real" service class if you need them, but we no longer require you to do so.
34+
1035
## [4.0.4]
1136

1237
### Changed

packages/sample-controllers/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,12 @@
4949
},
5050
"dependencies": {
5151
"@metamask/base-controller": "^9.0.1",
52+
"@metamask/base-data-service": "^0.1.1",
5253
"@metamask/messenger": "^1.0.0",
5354
"@metamask/network-controller": "^30.0.1",
54-
"@metamask/utils": "^11.9.0"
55+
"@metamask/superstruct": "^3.1.0",
56+
"@metamask/utils": "^11.9.0",
57+
"@tanstack/query-core": "^4.43.0"
5558
},
5659
"devDependencies": {
5760
"@metamask/auto-changelog": "^3.4.4",

packages/sample-controllers/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
export type {
2+
SampleGasPricesServiceInvalidateQueriesAction,
23
SampleGasPricesServiceActions,
4+
SampleGasPricesServiceCacheUpdatedEvent,
5+
SampleGasPricesServiceGranularCacheUpdatedEvent,
36
SampleGasPricesServiceEvents,
47
SampleGasPricesServiceMessenger,
58
} from './sample-gas-prices-service/sample-gas-prices-service';

packages/sample-controllers/src/sample-gas-prices-service/sample-gas-prices-service.test.ts

Lines changed: 22 additions & 207 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { HttpError } from '@metamask/controller-utils';
1+
import { DEFAULT_MAX_RETRIES } from '@metamask/controller-utils';
22
import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger';
33
import type {
44
MockAnyNamespace,
@@ -11,14 +11,6 @@ import type { SampleGasPricesServiceMessenger } from './sample-gas-prices-servic
1111
import { SampleGasPricesService } from './sample-gas-prices-service';
1212

1313
describe('SampleGasPricesService', () => {
14-
beforeEach(() => {
15-
jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'] });
16-
});
17-
18-
afterEach(() => {
19-
jest.useRealTimers();
20-
});
21-
2214
describe('SampleGasPricesService:fetchGasPrices', () => {
2315
it('returns the low, average, and high gas prices from the API', async () => {
2416
nock('https://api.example.com')
@@ -31,7 +23,7 @@ describe('SampleGasPricesService', () => {
3123
high: 15,
3224
},
3325
});
34-
const { rootMessenger } = getService();
26+
const { rootMessenger } = createService();
3527

3628
const gasPricesResponse = await rootMessenger.call(
3729
'SampleGasPricesService:fetchGasPrices',
@@ -45,6 +37,19 @@ describe('SampleGasPricesService', () => {
4537
});
4638
});
4739

40+
it('throws if the API returns a non-200 status', async () => {
41+
nock('https://api.example.com')
42+
.get('/gas-prices')
43+
.query({ chainId: 'eip155:1' })
44+
.times(DEFAULT_MAX_RETRIES + 1)
45+
.reply(500);
46+
const { rootMessenger } = createService();
47+
48+
await expect(
49+
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
50+
).rejects.toThrow("Gas prices API failed with status '500'");
51+
});
52+
4853
it.each([
4954
'not an object',
5055
{ missing: 'data' },
@@ -62,202 +67,13 @@ describe('SampleGasPricesService', () => {
6267
.get('/gas-prices')
6368
.query({ chainId: 'eip155:1' })
6469
.reply(200, JSON.stringify(response));
65-
const { rootMessenger } = getService();
70+
const { rootMessenger } = createService();
6671

6772
await expect(
6873
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
6974
).rejects.toThrow('Malformed response received from gas prices API');
7075
},
7176
);
72-
73-
it('calls onDegraded listeners if the request takes longer than 5 seconds to resolve', async () => {
74-
nock('https://api.example.com')
75-
.get('/gas-prices')
76-
.query({ chainId: 'eip155:1' })
77-
.reply(200, () => {
78-
jest.advanceTimersByTime(6000);
79-
return {
80-
data: {
81-
low: 5,
82-
average: 10,
83-
high: 15,
84-
},
85-
};
86-
});
87-
const { service, rootMessenger } = getService();
88-
const onDegradedListener = jest.fn();
89-
service.onDegraded(onDegradedListener);
90-
91-
await rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1');
92-
93-
expect(onDegradedListener).toHaveBeenCalled();
94-
});
95-
96-
it('allows the degradedThreshold to be changed', async () => {
97-
nock('https://api.example.com')
98-
.get('/gas-prices')
99-
.query({ chainId: 'eip155:1' })
100-
.reply(200, () => {
101-
jest.advanceTimersByTime(1000);
102-
return {
103-
data: {
104-
low: 5,
105-
average: 10,
106-
high: 15,
107-
},
108-
};
109-
});
110-
const { service, rootMessenger } = getService({
111-
options: {
112-
policyOptions: { degradedThreshold: 500 },
113-
},
114-
});
115-
const onDegradedListener = jest.fn();
116-
service.onDegraded(onDegradedListener);
117-
118-
await rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1');
119-
120-
expect(onDegradedListener).toHaveBeenCalled();
121-
});
122-
123-
it('attempts a request that responds with non-200 up to 4 times, throwing if it never succeeds', async () => {
124-
nock('https://api.example.com')
125-
.get('/gas-prices')
126-
.query({ chainId: 'eip155:1' })
127-
.times(4)
128-
.reply(500);
129-
const { service, rootMessenger } = getService();
130-
service.onRetry(() => {
131-
jest.advanceTimersToNextTimerAsync().catch(console.error);
132-
});
133-
134-
await expect(
135-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
136-
).rejects.toThrow(
137-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
138-
);
139-
});
140-
141-
it('calls onDegraded listeners when the maximum number of retries is exceeded', async () => {
142-
nock('https://api.example.com')
143-
.get('/gas-prices')
144-
.query({ chainId: 'eip155:1' })
145-
.times(4)
146-
.reply(500);
147-
const { service, rootMessenger } = getService();
148-
service.onRetry(() => {
149-
jest.advanceTimersToNextTimerAsync().catch(console.error);
150-
});
151-
const onDegradedListener = jest.fn();
152-
service.onDegraded(onDegradedListener);
153-
154-
await expect(
155-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
156-
).rejects.toThrow(
157-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
158-
);
159-
expect(onDegradedListener).toHaveBeenCalled();
160-
});
161-
162-
it('intercepts requests and throws a circuit break error after the 4th failed attempt, running onBreak listeners', async () => {
163-
nock('https://api.example.com')
164-
.get('/gas-prices')
165-
.query({ chainId: 'eip155:1' })
166-
.times(12)
167-
.reply(500);
168-
const { service, rootMessenger } = getService();
169-
service.onRetry(() => {
170-
jest.advanceTimersToNextTimerAsync().catch(console.error);
171-
});
172-
const onBreakListener = jest.fn();
173-
service.onBreak(onBreakListener);
174-
175-
// Should make 4 requests
176-
await expect(
177-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
178-
).rejects.toThrow(
179-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
180-
);
181-
// Should make 4 requests
182-
await expect(
183-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
184-
).rejects.toThrow(
185-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
186-
);
187-
// Should make 4 requests
188-
await expect(
189-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
190-
).rejects.toThrow(
191-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
192-
);
193-
// Should not make an additional request (we only mocked 12 requests
194-
// above)
195-
await expect(
196-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
197-
).rejects.toThrow(
198-
'Execution prevented because the circuit breaker is open',
199-
);
200-
expect(onBreakListener).toHaveBeenCalledWith({
201-
error: new HttpError(
202-
500,
203-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
204-
),
205-
});
206-
});
207-
208-
it('resumes requests after the circuit break duration passes, returning the API response if the request ultimately succeeds', async () => {
209-
const circuitBreakDuration = 5_000;
210-
nock('https://api.example.com')
211-
.get('/gas-prices')
212-
.query({ chainId: 'eip155:1' })
213-
.times(12)
214-
.reply(500)
215-
.get('/gas-prices')
216-
.query({ chainId: 'eip155:1' })
217-
.reply(200, {
218-
data: {
219-
low: 5,
220-
average: 10,
221-
high: 15,
222-
},
223-
});
224-
const { service, rootMessenger } = getService({
225-
options: {
226-
policyOptions: { circuitBreakDuration },
227-
},
228-
});
229-
service.onRetry(() => {
230-
jest.advanceTimersToNextTimerAsync().catch(console.error);
231-
});
232-
233-
await expect(
234-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
235-
).rejects.toThrow(
236-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
237-
);
238-
await expect(
239-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
240-
).rejects.toThrow(
241-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
242-
);
243-
await expect(
244-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
245-
).rejects.toThrow(
246-
"Fetching 'https://api.example.com/gas-prices?chainId=eip155%3A1' failed with status '500'",
247-
);
248-
await expect(
249-
rootMessenger.call('SampleGasPricesService:fetchGasPrices', '0x1'),
250-
).rejects.toThrow(
251-
'Execution prevented because the circuit breaker is open',
252-
);
253-
await jest.advanceTimersByTimeAsync(circuitBreakDuration);
254-
const gasPricesResponse = await service.fetchGasPrices('0x1');
255-
expect(gasPricesResponse).toStrictEqual({
256-
low: 5,
257-
average: 10,
258-
high: 15,
259-
});
260-
});
26177
});
26278

26379
describe('fetchGasPrices', () => {
@@ -272,7 +88,7 @@ describe('SampleGasPricesService', () => {
27288
high: 15,
27389
},
27490
});
275-
const { service } = getService();
91+
const { service } = createService();
27692

27793
const gasPricesResponse = await service.fetchGasPrices('0x1');
27894

@@ -301,7 +117,7 @@ type RootMessenger = Messenger<
301117
*
302118
* @returns The root messenger.
303119
*/
304-
function getRootMessenger(): RootMessenger {
120+
function createRootMessenger(): RootMessenger {
305121
return new Messenger({ namespace: MOCK_ANY_NAMESPACE });
306122
}
307123

@@ -312,7 +128,7 @@ function getRootMessenger(): RootMessenger {
312128
* events required by the controller's messenger.
313129
* @returns The service-specific messenger.
314130
*/
315-
function getMessenger(
131+
function createServiceMessenger(
316132
rootMessenger: RootMessenger,
317133
): SampleGasPricesServiceMessenger {
318134
return new Messenger({
@@ -330,7 +146,7 @@ function getMessenger(
330146
* `messenger`).
331147
* @returns The new service, root messenger, and service messenger.
332148
*/
333-
function getService({
149+
function createService({
334150
options = {},
335151
}: {
336152
options?: Partial<ConstructorParameters<typeof SampleGasPricesService>[0]>;
@@ -339,10 +155,9 @@ function getService({
339155
rootMessenger: RootMessenger;
340156
messenger: SampleGasPricesServiceMessenger;
341157
} {
342-
const rootMessenger = getRootMessenger();
343-
const messenger = getMessenger(rootMessenger);
158+
const rootMessenger = createRootMessenger();
159+
const messenger = createServiceMessenger(rootMessenger);
344160
const service = new SampleGasPricesService({
345-
fetch,
346161
messenger,
347162
...options,
348163
});

0 commit comments

Comments
 (0)