Skip to content

Commit

Permalink
remove queuedRequestPreference
Browse files Browse the repository at this point in the history
  • Loading branch information
adonesky1 committed Nov 19, 2024
1 parent 0f69373 commit 3196bc4
Show file tree
Hide file tree
Showing 4 changed files with 393 additions and 743 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ describe('createQueuedRequestMiddleware', () => {
const mockEnqueueRequest = getMockEnqueueRequest();
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: mockEnqueueRequest,
useRequestQueue: () => true,
});

const request = {
Expand All @@ -105,7 +104,7 @@ describe('createQueuedRequestMiddleware', () => {
const mockEnqueueRequest = getMockEnqueueRequest();
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: mockEnqueueRequest,
useRequestQueue: () => true,

shouldEnqueueRequest: ({ method }) =>
method === 'method_with_confirmation',
});
Expand Down Expand Up @@ -145,7 +144,6 @@ describe('createQueuedRequestMiddleware', () => {
it('calls next after a request is queued and processed', async () => {
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: getMockEnqueueRequest(),
useRequestQueue: () => true,
});
const request = {
...getRequestDefaults(),
Expand All @@ -167,7 +165,7 @@ describe('createQueuedRequestMiddleware', () => {
enqueueRequest: jest
.fn()
.mockRejectedValue(new Error('enqueuing error')),
useRequestQueue: () => true,

shouldEnqueueRequest: () => true,
});
const request = {
Expand All @@ -191,7 +189,7 @@ describe('createQueuedRequestMiddleware', () => {
enqueueRequest: jest
.fn()
.mockRejectedValue(new Error('enqueuing error')),
useRequestQueue: () => true,

shouldEnqueueRequest: () => true,
});
const request = {
Expand Down Expand Up @@ -271,7 +269,6 @@ function buildQueuedRequestMiddleware(
) {
const options = {
enqueueRequest: getMockEnqueueRequest(),
useRequestQueue: () => false,
shouldEnqueueRequest: () => false,
...overrideOptions,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,23 @@ function hasRequiredMetadata(
*
* @param options - Configuration options.
* @param options.enqueueRequest - A method for enqueueing a request.
* @param options.useRequestQueue - A function that determines if the request queue feature is enabled.
* @param options.shouldEnqueueRequest - A function that returns if a request should be handled by the QueuedRequestController.
* @returns The JSON-RPC middleware that manages queued requests.
*/
export const createQueuedRequestMiddleware = ({
enqueueRequest,
useRequestQueue,
shouldEnqueueRequest,
}: {
enqueueRequest: QueuedRequestController['enqueueRequest'];
useRequestQueue: () => boolean;
shouldEnqueueRequest: (
request: QueuedRequestMiddlewareJsonRpcRequest,
) => boolean;
}): JsonRpcMiddleware<JsonRpcParams, Json> => {
return createAsyncMiddleware(async (req: JsonRpcRequest, res, next) => {
hasRequiredMetadata(req);

// if the request queue feature is turned off, or this method is not a confirmation method
// bypass the queue completely
if (!useRequestQueue() || !shouldEnqueueRequest(req)) {
// if this method is not a confirmation method bypass the queue completely
if (!shouldEnqueueRequest(req)) {
return await next();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger<
export type SelectedNetworkControllerOptions = {
state?: SelectedNetworkControllerState;
messenger: SelectedNetworkControllerMessenger;
useRequestQueuePreference: boolean;
onPreferencesStateChange: (
listener: (preferencesState: { useRequestQueue: boolean }) => void,
) => void;
domainProxyMap: Map<Domain, NetworkProxy>;
};

Expand All @@ -124,23 +120,17 @@ export class SelectedNetworkController extends BaseController<
> {
#domainProxyMap: Map<Domain, NetworkProxy>;

#useRequestQueuePreference: boolean;

/**
* Construct a SelectedNetworkController controller.
*
* @param options - The controller options.
* @param options.messenger - The restricted controller messenger for the EncryptionPublicKey controller.
* @param options.state - The controllers initial state.
* @param options.useRequestQueuePreference - A boolean indicating whether to use the request queue preference.
* @param options.onPreferencesStateChange - A callback that is called when the preference state changes.
* @param options.domainProxyMap - A map for storing domain-specific proxies that are held in memory only during use.
*/
constructor({
messenger,
state = getDefaultState(),
useRequestQueuePreference,
onPreferencesStateChange,
domainProxyMap,
}: SelectedNetworkControllerOptions) {
super({
Expand All @@ -149,7 +139,6 @@ export class SelectedNetworkController extends BaseController<
messenger,
state,
});
this.#useRequestQueuePreference = useRequestQueuePreference;
this.#domainProxyMap = domainProxyMap;
this.#registerMessageHandlers();

Expand Down Expand Up @@ -247,21 +236,6 @@ export class SelectedNetworkController extends BaseController<
}
},
);

onPreferencesStateChange(({ useRequestQueue }) => {
if (this.#useRequestQueuePreference !== useRequestQueue) {
if (!useRequestQueue) {
// Loop through all domains and points each domain's proxy
// to the NetworkController's own proxy of the globally selected networkClient
Object.keys(this.state.domains).forEach((domain) => {
this.#unsetNetworkClientIdForDomain(domain);
});
} else {
this.#resetAllPermissionedDomains();
}
this.#useRequestQueuePreference = useRequestQueue;
}
});
}

#registerMessageHandlers(): void {
Expand Down Expand Up @@ -326,31 +300,10 @@ export class SelectedNetworkController extends BaseController<
);
}

// Loop through all domains and for those with permissions it points that domain's proxy
// to an unproxied instance of the globally selected network client.
// NOT the NetworkController's proxy of the globally selected networkClient
#resetAllPermissionedDomains() {
this.#domainProxyMap.forEach((_: NetworkProxy, domain: string) => {
const { selectedNetworkClientId } = this.messagingSystem.call(
'NetworkController:getState',
);
// can't use public setNetworkClientIdForDomain because it will throw an error
// rather than simply skip if the domain doesn't have permissions which can happen
// in this case since proxies are added for each site the user visits
if (this.#domainHasPermissions(domain)) {
this.#setNetworkClientIdForDomain(domain, selectedNetworkClientId);
}
});
}

setNetworkClientIdForDomain(
domain: Domain,
networkClientId: NetworkClientId,
) {
if (!this.#useRequestQueuePreference) {
return;
}

if (domain === METAMASK_DOMAIN) {
throw new Error(
`NetworkClientId for domain "${METAMASK_DOMAIN}" cannot be set on the SelectedNetworkController`,
Expand All @@ -373,9 +326,6 @@ export class SelectedNetworkController extends BaseController<
getNetworkClientIdForDomain(domain: Domain): NetworkClientId {
const { selectedNetworkClientId: metamaskSelectedNetworkClientId } =
this.messagingSystem.call('NetworkController:getState');
if (!this.#useRequestQueuePreference) {
return metamaskSelectedNetworkClientId;
}
return this.state.domains[domain] ?? metamaskSelectedNetworkClientId;
}

Expand Down Expand Up @@ -403,15 +353,15 @@ export class SelectedNetworkController extends BaseController<
let networkProxy = this.#domainProxyMap.get(domain);
if (networkProxy === undefined) {
let networkClient;
if (
this.#useRequestQueuePreference &&
this.#domainHasPermissions(domain)
) {
if (this.#domainHasPermissions(domain)) {
const networkClientId = this.getNetworkClientIdForDomain(domain);
networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
networkClientId,
);
if (networkClient === undefined) {
throw new Error('Selected network not initialized');
}
} else {
networkClient = this.messagingSystem.call(
'NetworkController:getSelectedNetworkClient',
Expand Down
Loading

0 comments on commit 3196bc4

Please sign in to comment.