Skip to content

Commit

Permalink
feat: make polling input generic (#4752)
Browse files Browse the repository at this point in the history
## Explanation

Allow the polling controllers to accept generic input when starting a
poll.

Today they accept:
```
networkClientId: NetworkClientId,
options: Json
```

But controllers may want unique polling loops over other data.

This PR allows controllers to use any serializable type as their polling
input. Controllers pass their input type as a generic argument to the
base class, making it more type safe than then previous `options: Json`.

An example of how this would be used in the future, is how
`CurrencyRateController` only needs a polling loop for each unique
native currency. So it would define:

```
type CurrencyRatePollingInput = {
  nativeCurrency: string;
};
```

And you would initiate polling with:
```
const currencyRateController = new(...);
const token1 = currencyRateController.startPolling({nativeCurrency: 'ETH'});
const token2 = currencyRateController.startPolling({nativeCurrency: 'POL'});
```

## References


## Changelog



<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/polling-controller`

- **BREAKING**: The input to start polling is now a generic type that
can be any input, instead of requiring a network client id like before.
The functions interfaces are updated to accommodate this.
`startPollingByNetworkClientId` is now `startPolling`. And
`onPollingComplete` now returns the entire input object, instead of a
network client id.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
bergeron authored Oct 10, 2024
1 parent c29b8a0 commit b1f5475
Show file tree
Hide file tree
Showing 22 changed files with 426 additions and 310 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,9 @@ describe('AccountTrackerController', () => {
.spyOn(controller, 'refresh')
.mockResolvedValue();

controller.startPollingByNetworkClientId(networkClientId1);
controller.startPolling({
networkClientId: networkClientId1,
});

await advanceTime({ clock, duration: 0 });
expect(refreshSpy).toHaveBeenNthCalledWith(1, networkClientId1);
Expand All @@ -516,8 +518,9 @@ describe('AccountTrackerController', () => {
expect(refreshSpy).toHaveBeenNthCalledWith(2, networkClientId1);
expect(refreshSpy).toHaveBeenCalledTimes(2);

const pollToken =
controller.startPollingByNetworkClientId(networkClientId2);
const pollToken = controller.startPolling({
networkClientId: networkClientId2,
});

await advanceTime({ clock, duration: 0 });
expect(refreshSpy).toHaveBeenNthCalledWith(3, networkClientId2);
Expand Down
14 changes: 11 additions & 3 deletions packages/assets-controllers/src/AccountTrackerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,15 @@ export type AccountTrackerControllerMessenger = RestrictedControllerMessenger<
AllowedEvents['type']
>;

/** The input to start polling for the {@link AccountTrackerController} */
type AccountTrackerPollingInput = {
networkClientId: NetworkClientId;
};

/**
* Controller that tracks the network balances for all user accounts.
*/
export class AccountTrackerController extends StaticIntervalPollingController<
export class AccountTrackerController extends StaticIntervalPollingController<AccountTrackerPollingInput>()<
typeof controllerName,
AccountTrackerControllerState,
AccountTrackerControllerMessenger
Expand Down Expand Up @@ -309,9 +314,12 @@ export class AccountTrackerController extends StaticIntervalPollingController<
/**
* Refreshes the balances of the accounts using the networkClientId
*
* @param networkClientId - The network client ID used to get balances.
* @param input - The input for the poll.
* @param input.networkClientId - The network client ID used to get balances.
*/
async _executePoll(networkClientId: string): Promise<void> {
async _executePoll({
networkClientId,
}: AccountTrackerPollingInput): Promise<void> {
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.refresh(networkClientId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('CurrencyRateController', () => {
messenger,
});

controller.startPollingByNetworkClientId('mainnet');
controller.startPolling({ networkClientId: 'mainnet' });
await advanceTime({ clock, duration: 0 });
expect(fetchExchangeRateStub).toHaveBeenCalledTimes(1);
expect(controller.state.currencyRates).toStrictEqual({
Expand Down Expand Up @@ -192,7 +192,7 @@ describe('CurrencyRateController', () => {
messenger,
});

controller.startPollingByNetworkClientId('sepolia');
controller.startPolling({ networkClientId: 'sepolia' });

await advanceTime({ clock, duration: 0 });

Expand All @@ -217,15 +217,15 @@ describe('CurrencyRateController', () => {
fetchExchangeRate: fetchExchangeRateStub,
messenger,
});
controller.startPollingByNetworkClientId('sepolia');
controller.startPolling({ networkClientId: 'sepolia' });
await advanceTime({ clock, duration: 0 });

controller.stopAllPolling();

// called once upon initial start
expect(fetchExchangeRateStub).toHaveBeenCalledTimes(1);

controller.startPollingByNetworkClientId('sepolia');
controller.startPolling({ networkClientId: 'sepolia' });
await advanceTime({ clock, duration: 0 });

expect(fetchExchangeRateStub).toHaveBeenCalledTimes(2);
Expand Down
15 changes: 11 additions & 4 deletions packages/assets-controllers/src/CurrencyRateController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,16 @@ const defaultState = {
},
};

/** The input to start polling for the {@link CurrencyRateController} */
type CurrencyRatePollingInput = {
networkClientId: NetworkClientId;
};

/**
* Controller that passively polls on a set interval for an exchange rate from the current network
* asset to the user's preferred currency.
*/
export class CurrencyRateController extends StaticIntervalPollingController<
export class CurrencyRateController extends StaticIntervalPollingController<CurrencyRatePollingInput>()<
typeof name,
CurrencyRateState,
CurrencyRateMessenger
Expand Down Expand Up @@ -237,10 +242,12 @@ export class CurrencyRateController extends StaticIntervalPollingController<
/**
* Updates exchange rate for the current currency.
*
* @param networkClientId - The network client ID used to get a ticker value.
* @returns The controller state.
* @param input - The input for the poll.
* @param input.networkClientId - The network client ID used to get a ticker value.
*/
async _executePoll(networkClientId: NetworkClientId): Promise<void> {
async _executePoll({
networkClientId,
}: CurrencyRatePollingInput): Promise<void> {
const networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
networkClientId,
Expand Down
11 changes: 7 additions & 4 deletions packages/assets-controllers/src/TokenDetectionController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ describe('TokenDetectionController', () => {
});
});

describe('startPollingByNetworkClientId', () => {
describe('startPolling', () => {
let clock: sinon.SinonFakeTimers;
beforeEach(() => {
clock = sinon.useFakeTimers();
Expand Down Expand Up @@ -1904,13 +1904,16 @@ describe('TokenDetectionController', () => {
return Promise.resolve();
});

controller.startPollingByNetworkClientId('mainnet', {
controller.startPolling({
networkClientId: 'mainnet',
address: '0x1',
});
controller.startPollingByNetworkClientId('sepolia', {
controller.startPolling({
networkClientId: 'sepolia',
address: '0xdeadbeef',
});
controller.startPollingByNetworkClientId('goerli', {
controller.startPolling({
networkClientId: 'goerli',
address: '0x3',
});
await advanceTime({ clock, duration: 0 });
Expand Down
18 changes: 12 additions & 6 deletions packages/assets-controllers/src/TokenDetectionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger<
AllowedEvents['type']
>;

/** The input to start polling for the {@link TokenDetectionController} */
type TokenDetectionPollingInput = {
networkClientId: NetworkClientId;
address: string;
};

/**
* Controller that passively polls on a set interval for Tokens auto detection
* @property intervalId - Polling interval used to fetch new token rates
Expand All @@ -148,7 +154,7 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger<
* @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController
* @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network
*/
export class TokenDetectionController extends StaticIntervalPollingController<
export class TokenDetectionController extends StaticIntervalPollingController<TokenDetectionPollingInput>()<
typeof controllerName,
TokenDetectionState,
TokenDetectionControllerMessenger
Expand Down Expand Up @@ -432,16 +438,16 @@ export class TokenDetectionController extends StaticIntervalPollingController<
};
}

async _executePoll(
networkClientId: NetworkClientId,
options: { address: string },
): Promise<void> {
async _executePoll({
networkClientId,
address,
}: TokenDetectionPollingInput): Promise<void> {
if (!this.isActive) {
return;
}
await this.detectTokens({
networkClientId,
selectedAddress: options.address,
selectedAddress: address,
});
}

Expand Down
14 changes: 9 additions & 5 deletions packages/assets-controllers/src/TokenListController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,7 @@ describe('TokenListController', () => {
});
});

describe('startPollingByNetworkClient', () => {
describe('startPolling', () => {
let clock: sinon.SinonFakeTimers;
const pollingIntervalTime = 1000;
beforeEach(() => {
Expand Down Expand Up @@ -1200,7 +1200,7 @@ describe('TokenListController', () => {
expiredCacheExistingState.tokenList,
);

controller.startPollingByNetworkClientId('sepolia');
controller.startPolling({ networkClientId: 'sepolia' });
await advanceTime({ clock, duration: 0 });

expect(fetchTokenListByChainIdSpy.mock.calls[0]).toStrictEqual(
Expand Down Expand Up @@ -1236,7 +1236,7 @@ describe('TokenListController', () => {
expiredCacheExistingState.tokenList,
);

controller.startPollingByNetworkClientId('goerli');
controller.startPolling({ networkClientId: 'goerli' });
await advanceTime({ clock, duration: 0 });

expect(fetchTokenListByChainIdSpy).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -1306,7 +1306,9 @@ describe('TokenListController', () => {
expect(controller.state).toStrictEqual(startingState);

// start polling for sepolia
const pollingToken = controller.startPollingByNetworkClientId('sepolia');
const pollingToken = controller.startPolling({
networkClientId: 'sepolia',
});
// wait a polling interval
await advanceTime({ clock, duration: pollingIntervalTime });

Expand All @@ -1324,7 +1326,9 @@ describe('TokenListController', () => {
controller.stopPollingByPollingToken(pollingToken);

// start polling for binance
controller.startPollingByNetworkClientId('binance-network-client-id');
controller.startPolling({
networkClientId: 'binance-network-client-id',
});
await advanceTime({ clock, duration: pollingIntervalTime });

// expect fetchTokenListByChain to be called for binance, but not for sepolia
Expand Down
20 changes: 14 additions & 6 deletions packages/assets-controllers/src/TokenListController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,15 @@ export const getDefaultTokenListState = (): TokenListState => {
};
};

/** The input to start polling for the {@link TokenListController} */
type TokenListPollingInput = {
networkClientId: NetworkClientId;
};

/**
* Controller that passively polls on a set interval for the list of tokens from metaswaps api
*/
export class TokenListController extends StaticIntervalPollingController<
export class TokenListController extends StaticIntervalPollingController<TokenListPollingInput>()<
typeof name,
TokenListState,
TokenListControllerMessenger
Expand Down Expand Up @@ -211,15 +216,15 @@ export class TokenListController extends StaticIntervalPollingController<
if (!isTokenListSupportedForNetwork(this.chainId)) {
return;
}
await this.startPolling();
await this.#startPolling();
}

/**
* Restart polling for the token list.
*/
async restart() {
this.stopPolling();
await this.startPolling();
await this.#startPolling();
}

/**
Expand Down Expand Up @@ -248,7 +253,7 @@ export class TokenListController extends StaticIntervalPollingController<
/**
* Starts a new polling interval.
*/
private async startPolling(): Promise<void> {
async #startPolling(): Promise<void> {
await safelyExecute(() => this.fetchTokenList());
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/no-misused-promises
Expand All @@ -261,10 +266,13 @@ export class TokenListController extends StaticIntervalPollingController<
* Fetching token list from the Token Service API.
*
* @private
* @param networkClientId - The ID of the network client triggering the fetch.
* @param input - The input for the poll.
* @param input.networkClientId - The ID of the network client triggering the fetch.
* @returns A promise that resolves when this operation completes.
*/
async _executePoll(networkClientId: string): Promise<void> {
async _executePoll({
networkClientId,
}: TokenListPollingInput): Promise<void> {
return this.fetchTokenList(networkClientId);
}

Expand Down
21 changes: 15 additions & 6 deletions packages/assets-controllers/src/TokenRatesController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,9 @@ describe('TokenRatesController', () => {
},
},
async ({ controller }) => {
controller.startPollingByNetworkClientId('mainnet');
controller.startPolling({
networkClientId: 'mainnet',
});

await advanceTime({ clock, duration: 0 });
expect(tokenPricesService.fetchTokenPrices).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -1268,7 +1270,9 @@ describe('TokenRatesController', () => {
},
},
async ({ controller }) => {
controller.startPollingByNetworkClientId('mainnet');
controller.startPolling({
networkClientId: 'mainnet',
});
await advanceTime({ clock, duration: 0 });

expect(controller.state).toStrictEqual({
Expand Down Expand Up @@ -1372,7 +1376,9 @@ describe('TokenRatesController', () => {
},
},
async ({ controller }) => {
controller.startPollingByNetworkClientId('mainnet');
controller.startPolling({
networkClientId: 'mainnet',
});
// flush promises and advance setTimeouts they enqueue 3 times
// needed because fetch() doesn't resolve immediately, so any
// downstream promises aren't flushed until the next advanceTime loop
Expand Down Expand Up @@ -1472,7 +1478,9 @@ describe('TokenRatesController', () => {
},
},
async ({ controller }) => {
controller.startPollingByNetworkClientId('mainnet');
controller.startPolling({
networkClientId: 'mainnet',
});
// flush promises and advance setTimeouts they enqueue 3 times
// needed because fetch() doesn't resolve immediately, so any
// downstream promises aren't flushed until the next advanceTime loop
Expand Down Expand Up @@ -1513,8 +1521,9 @@ describe('TokenRatesController', () => {
},
},
async ({ controller }) => {
const pollingToken =
controller.startPollingByNetworkClientId('mainnet');
const pollingToken = controller.startPolling({
networkClientId: 'mainnet',
});
await advanceTime({ clock, duration: 0 });
expect(tokenPricesService.fetchTokenPrices).toHaveBeenCalledTimes(
1,
Expand Down
15 changes: 11 additions & 4 deletions packages/assets-controllers/src/TokenRatesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,16 @@ export const getDefaultTokenRatesControllerState =
};
};

/** The input to start polling for the {@link TokenRatesController} */
export type TokenRatesPollingInput = {
networkClientId: NetworkClientId;
};

/**
* Controller that passively polls on a set interval for token-to-fiat exchange rates
* for tokens stored in the TokensController
*/
export class TokenRatesController extends StaticIntervalPollingController<
export class TokenRatesController extends StaticIntervalPollingController<TokenRatesPollingInput>()<
typeof controllerName,
TokenRatesControllerState,
TokenRatesControllerMessenger
Expand Down Expand Up @@ -594,10 +599,12 @@ export class TokenRatesController extends StaticIntervalPollingController<
/**
* Updates token rates for the given networkClientId
*
* @param networkClientId - The network client ID used to get a ticker value.
* @returns The controller state.
* @param input - The input for the poll.
* @param input.networkClientId - The network client ID used to get a ticker value.
*/
async _executePoll(networkClientId: NetworkClientId): Promise<void> {
async _executePoll({
networkClientId,
}: TokenRatesPollingInput): Promise<void> {
const networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
networkClientId,
Expand Down
Loading

0 comments on commit b1f5475

Please sign in to comment.