Skip to content

Commit

Permalink
Update PollingController to immediately call _executePoll if no p…
Browse files Browse the repository at this point in the history
…olling is currently active for the key on start (#1874)

## Explanation

Currently when PollingController polling is started via
`startPollingByNetworkClientId()`, the first poll execution doesn't
occur until `intervalLength` time has passed. Some controllers such as
the `CurrencyRateController` want to execute their polling logic
immediately on polling start that way it is available to the UI as soon
as possible. ~~This PR adds a `executeImmediately` flag to the
`PollingController`. When set to `true`, it causes `_executePoll()` to
immediately be called when `startPollingByNetworkClientId()` is called,
subsequently on each `intervalLength` time period. This value defaults
to false.~~ This PR updates `#poll()` to call `_executePoll()` if no
polling is currently active for the key

## References

* Related #1805

## Changelog

### `@metamask/polling-controller`

- **BREAKING**: `_executePoll()` is called immediately on start if no
polling interval is already active for the networkClientId + options
combination

## 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

---------

Co-authored-by: Elliot Winkler <[email protected]>
Co-authored-by: Alex <[email protected]>
  • Loading branch information
3 people authored Oct 24, 2023
1 parent fa106c5 commit 9e00912
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 49 deletions.
19 changes: 15 additions & 4 deletions packages/assets-controllers/src/NftDetectionController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ type ApprovalActions = AddApprovalRequest;

const controllerName = 'NftController' as const;

const flushPromises = () => {
return new Promise(jest.requireActual('timers').setImmediate);
};

describe('NftDetectionController', () => {
let nftDetection: NftDetectionController;
let preferences: PreferencesController;
Expand Down Expand Up @@ -294,19 +298,26 @@ describe('NftDetectionController', () => {
testNftDetection.startPollingByNetworkClientId('mainnet', {
address: '0x1',
});
await Promise.all([jest.advanceTimersByTime(0), flushPromises]);
expect(spy.mock.calls).toHaveLength(1);
await Promise.all([
jest.advanceTimersByTime(DEFAULT_INTERVAL),
Promise.resolve(),
jest.advanceTimersByTime(DEFAULT_INTERVAL / 2),
flushPromises(),
]);
expect(spy.mock.calls).toHaveLength(1);
await Promise.all([
jest.advanceTimersByTime(DEFAULT_INTERVAL),
Promise.resolve(),
jest.advanceTimersByTime(DEFAULT_INTERVAL / 2),
flushPromises(),
]);
expect(spy.mock.calls).toHaveLength(2);
await Promise.all([
jest.advanceTimersByTime(DEFAULT_INTERVAL),
flushPromises(),
]);
expect(spy.mock.calls).toMatchObject([
['mainnet', '0x1'],
['mainnet', '0x1'],
['mainnet', '0x1'],
]);
nftDetection.stopAllPolling();
jest.runOnlyPendingTimers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ const setupTokenListController = (
return { tokenList, tokenListMessenger };
};

const flushPromises = () => {
return new Promise(jest.requireActual('timers').setImmediate);
};

describe('TokenDetectionController', () => {
let tokenDetection: TokenDetectionController;
let preferences: PreferencesController;
Expand Down Expand Up @@ -611,7 +615,7 @@ describe('TokenDetectionController', () => {
});
await Promise.all([
jest.advanceTimersByTime(DEFAULT_INTERVAL),
Promise.resolve(),
flushPromises(),
]);
expect(spy.mock.calls).toMatchObject([
[{ networkClientId: 'mainnet', accountAddress: '0x1' }],
Expand Down
28 changes: 18 additions & 10 deletions packages/assets-controllers/src/TokenListController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1368,20 +1368,28 @@ describe('TokenListController', () => {
);

controller.startPollingByNetworkClientId('goerli');
jest.advanceTimersByTime(pollingIntervalTime / 2);
jest.advanceTimersByTime(0);
await flushPromises();
expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(0);
jest.advanceTimersByTime(pollingIntervalTime / 2);
await flushPromises();

expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(1);
await Promise.all([
jest.advanceTimersByTime(pollingIntervalTime / 2),
flushPromises(),
]);
expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(1);
await Promise.all([
jest.advanceTimersByTime(pollingIntervalTime / 2),
jest.runOnlyPendingTimers(),
flushPromises(),
]);

expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(2);
await Promise.all([
jest.advanceTimersByTime(pollingIntervalTime),
flushPromises(),
]);

await Promise.all([jest.runOnlyPendingTimers(), flushPromises()]);
expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(2);
expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(3);
});

it('should update tokenList state and tokensChainsCache', async () => {
Expand Down Expand Up @@ -1441,7 +1449,7 @@ describe('TokenListController', () => {
expect(controller.state).toStrictEqual(startingState);

// start polling for sepolia
await controller.startPollingByNetworkClientId('sepolia');
const pollingToken = controller.startPollingByNetworkClientId('sepolia');
// wait a polling interval
jest.advanceTimersByTime(pollingIntervalTime);
await flushPromises();
Expand All @@ -1457,10 +1465,10 @@ describe('TokenListController', () => {
data: sampleSepoliaTokensChainCache,
},
});
controller.stopPollingByPollingToken(pollingToken);

// start polling for binance
await controller.startPollingByNetworkClientId(
'binance-network-client-id',
);
controller.startPollingByNetworkClientId('binance-network-client-id');
jest.advanceTimersByTime(pollingIntervalTime);
await flushPromises();

Expand Down
16 changes: 13 additions & 3 deletions packages/gas-fee-controller/src/GasFeeController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ describe('GasFeeController', () => {
});

describe('polling (by networkClientId)', () => {
it('should call determineGasFeeCalculations (via _executePoll) with a URL that contains the chainId corresponding to the networkClientId after the interval passed via the constructor', async () => {
it('should call determineGasFeeCalculations (via _executePoll) with a URL that contains the chainId corresponding to the networkClientId immedaitely and after each interval passed via the constructor', async () => {
const pollingInterval = 10000;
await setupGasFeeController({
getIsEIP1559Compatible: jest.fn().mockResolvedValue(false),
Expand Down Expand Up @@ -1013,10 +1013,20 @@ describe('GasFeeController', () => {
});

gasFeeController.startPollingByNetworkClientId('goerli');
await clock.tickAsync(0);
expect(mockedDetermineGasFeeCalculations).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal(
ChainId.goerli,
)}`,
}),
);
await clock.tickAsync(pollingInterval / 2);
expect(mockedDetermineGasFeeCalculations).not.toHaveBeenCalled();
expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledTimes(1);
await clock.tickAsync(pollingInterval / 2);
expect(mockedDetermineGasFeeCalculations).toHaveBeenCalledWith(
expect(mockedDetermineGasFeeCalculations).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
fetchGasEstimatesUrl: `https://some-eip-1559-endpoint/${convertHexToDecimal(
ChainId.goerli,
Expand Down
Loading

0 comments on commit 9e00912

Please sign in to comment.