From 9271866017f399092ae832b5db194af0770181e3 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 16 Jul 2026 13:36:02 +0200 Subject: [PATCH 1/3] feat(base-data-service): Add struct option for validation --- packages/base-data-service/package.json | 1 + .../src/BaseDataService.test.ts | 29 ++++++++++++ .../base-data-service/src/BaseDataService.ts | 47 +++++++++++++------ packages/base-data-service/src/utils.ts | 23 +++++++++ .../tests/ExampleDataService.ts | 23 +++++++-- yarn.lock | 1 + 6 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 packages/base-data-service/src/utils.ts diff --git a/packages/base-data-service/package.json b/packages/base-data-service/package.json index 977267e002..060aab96f3 100644 --- a/packages/base-data-service/package.json +++ b/packages/base-data-service/package.json @@ -58,6 +58,7 @@ "dependencies": { "@metamask/messenger": "^2.0.0", "@metamask/storage-service": "^1.0.2", + "@metamask/superstruct": "^3.1.0", "@metamask/utils": "^11.11.0", "@tanstack/query-core": "^4.43.0", "cockatiel": "^3.1.2", diff --git a/packages/base-data-service/src/BaseDataService.test.ts b/packages/base-data-service/src/BaseDataService.test.ts index af231b5b3a..b402e5f794 100644 --- a/packages/base-data-service/src/BaseDataService.test.ts +++ b/packages/base-data-service/src/BaseDataService.test.ts @@ -222,6 +222,35 @@ describe('BaseDataService', () => { expect(publishSpy).toHaveBeenCalledTimes(8); }); + describe('validation', () => { + beforeAll(() => { + jest.useRealTimers(); + }); + + afterAll(() => { + jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate'] }); + }); + + beforeEach(() => { + cleanAll(); + }); + + it('throws when fetchQuery response fails struct validation', async () => { + const messenger = new Messenger({ namespace: serviceName }); + const service = new ExampleDataService(messenger); + + mockAssets({ status: 200, body: { foo: 'bar' } }); + mockAssets({ status: 200, body: { foo: 'bar' } }); + mockAssets({ status: 200, body: { foo: 'bar' } }); + + await expect(service.getAssets(MOCK_ASSETS)).rejects.toThrow( + /ExampleDataService:getAssets.*returned an invalid response:/u, + ); + + service.destroy(); + }); + }); + describe('service policy', () => { beforeAll(() => { jest.useRealTimers(); diff --git a/packages/base-data-service/src/BaseDataService.ts b/packages/base-data-service/src/BaseDataService.ts index 3b45961225..0af9ebedf9 100644 --- a/packages/base-data-service/src/BaseDataService.ts +++ b/packages/base-data-service/src/BaseDataService.ts @@ -8,6 +8,7 @@ import type { StorageServiceRemoveItemAction, StorageServiceSetItemAction, } from '@metamask/storage-service'; +import { Struct, validate } from '@metamask/superstruct'; import { Duration, inMilliseconds } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import { @@ -33,6 +34,7 @@ import { CreateServicePolicyOptions, ServicePolicy, } from './createServicePolicy'; +import { processQueryResponse } from './utils'; // Data service queries use the following format: ['ServiceActionName', ...params] export type QueryKey = [string, ...Json[]]; @@ -81,6 +83,10 @@ type DataServiceEvents = | DataServiceCacheUpdatedEvent | DataServiceGranularCacheUpdatedEvent; +type AdditionalQueryOptions = { + struct?: Struct; +}; + // Defaults to apply to all data service queries if no default option specified const QUERY_CLIENT_DEFAULTS: DefaultOptions = { queries: { @@ -247,19 +253,24 @@ export class BaseDataService< TError = unknown, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, - >( - options: WithRequired< - OmitKeyof< - FetchQueryOptions, - 'retry' | 'retryDelay' - >, - 'queryKey' | 'queryFn' + >({ + struct, + ...options + }: WithRequired< + OmitKeyof< + FetchQueryOptions, + 'retry' | 'retryDelay' >, - ): Promise { + 'queryKey' | 'queryFn' + > & + AdditionalQueryOptions): Promise { return this.#queryClient.fetchQuery({ ...options, queryFn: (context) => - this.#policy.execute(() => options.queryFn(context)), + this.#policy.execute(async () => { + const result = await options.queryFn(context); + return processQueryResponse(options.queryKey, result, struct); + }), }); } @@ -278,13 +289,17 @@ export class BaseDataService< TQueryKey extends QueryKey = QueryKey, TPageParam extends Json = Json, >( - options: WithRequired< + { + struct, + ...options + }: WithRequired< OmitKeyof< FetchInfiniteQueryOptions, 'retry' | 'retryDelay' >, 'queryKey' | 'queryFn' - >, + > & + AdditionalQueryOptions, pageParam?: TPageParam, ): Promise { const cache = this.#queryClient.getQueryCache(); @@ -297,12 +312,14 @@ export class BaseDataService< const result = await this.#queryClient.fetchInfiniteQuery({ ...options, queryFn: (context) => - this.#policy.execute(() => - options.queryFn({ + this.#policy.execute(async () => { + const result = await options.queryFn({ ...context, pageParam: context.pageParam ?? pageParam, - }), - ), + }); + + return processQueryResponse(options.queryKey, result, struct); + }), }); return result.pages[0]; diff --git a/packages/base-data-service/src/utils.ts b/packages/base-data-service/src/utils.ts new file mode 100644 index 0000000000..bd4fb0103d --- /dev/null +++ b/packages/base-data-service/src/utils.ts @@ -0,0 +1,23 @@ +import { Struct, validate } from '@metamask/superstruct'; + +import { QueryKey } from './BaseDataService'; + +export function processQueryResponse( + queryKey: QueryKey, + response: Response, + struct?: Struct, +) { + if (!struct) { + return response; + } + + const [error, result] = validate(response, struct); + + if (error) { + throw new Error( + `${queryKey} returned an invalid response: ${error.message}.`, + ); + } + + return result; +} diff --git a/packages/base-data-service/tests/ExampleDataService.ts b/packages/base-data-service/tests/ExampleDataService.ts index f6eb3ff93b..c2625dbd06 100644 --- a/packages/base-data-service/tests/ExampleDataService.ts +++ b/packages/base-data-service/tests/ExampleDataService.ts @@ -1,5 +1,12 @@ import { Messenger } from '@metamask/messenger'; -import { CaipAssetId, Duration, inMilliseconds, Json } from '@metamask/utils'; +import { object, number, string, array } from '@metamask/superstruct'; +import { + CaipAssetType, + CaipAssetTypeStruct, + Duration, + inMilliseconds, + Json, +} from '@metamask/utils'; import { ConstantBackoff } from 'cockatiel'; import { @@ -28,11 +35,20 @@ export type ExampleMessenger = Messenger< >; export type GetAssetsResponse = { - assetId: CaipAssetId; + assetId: CaipAssetType; decimals: number; name: string; symbol: string; -}; +}[]; + +const GetAssetsResponseStruct = array( + object({ + assetId: CaipAssetTypeStruct, + decimals: number(), + name: string(), + symbol: string(), + }), +); export type GetActivityResponse = { data: Json[]; @@ -102,6 +118,7 @@ export class ExampleDataService extends BaseDataService< }, staleTime: inMilliseconds(1, Duration.Day), cacheTime: inMilliseconds(1, Duration.Day), + struct: GetAssetsResponseStruct, }); } diff --git a/yarn.lock b/yarn.lock index 1e4085d18f..dc7594ed82 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5893,6 +5893,7 @@ __metadata: "@metamask/auto-changelog": "npm:^6.1.0" "@metamask/messenger": "npm:^2.0.0" "@metamask/storage-service": "npm:^1.0.2" + "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^11.11.0" "@tanstack/query-core": "npm:^4.43.0" "@ts-bridge/cli": "npm:^0.6.4" From d04e322f76e1b70430e7dc7f063f02480d156199 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 16 Jul 2026 14:23:41 +0200 Subject: [PATCH 2/3] Improve types and JSDoc --- .../base-data-service/src/BaseDataService.ts | 21 +++++++++++-------- packages/base-data-service/src/utils.ts | 15 ++++++++++--- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/base-data-service/src/BaseDataService.ts b/packages/base-data-service/src/BaseDataService.ts index 0af9ebedf9..2101919d38 100644 --- a/packages/base-data-service/src/BaseDataService.ts +++ b/packages/base-data-service/src/BaseDataService.ts @@ -8,7 +8,7 @@ import type { StorageServiceRemoveItemAction, StorageServiceSetItemAction, } from '@metamask/storage-service'; -import { Struct, validate } from '@metamask/superstruct'; +import { Struct } from '@metamask/superstruct'; import { Duration, inMilliseconds } from '@metamask/utils'; import type { Json } from '@metamask/utils'; import { @@ -83,8 +83,8 @@ type DataServiceEvents = | DataServiceCacheUpdatedEvent | DataServiceGranularCacheUpdatedEvent; -type AdditionalQueryOptions = { - struct?: Struct; +type AdditionalQueryOptions = { + struct?: Struct; }; // Defaults to apply to all data service queries if no default option specified @@ -246,6 +246,7 @@ export class BaseDataService< * * @param options - The options defining the query. Keep in mind that `queryKey` and `queryFn` are required when using data services. * Additionally `retry` and `retryDelay` are not available, retries can be customized using the `servicePolicyOptions`. + * @param options.struct - An optional struct for validating the response of the query function. * @returns The query results. */ protected async fetchQuery< @@ -263,13 +264,14 @@ export class BaseDataService< >, 'queryKey' | 'queryFn' > & - AdditionalQueryOptions): Promise { + AdditionalQueryOptions): Promise { return this.#queryClient.fetchQuery({ ...options, queryFn: (context) => + // TODO: Consider if the validation should happen outside the policy executor this.#policy.execute(async () => { - const result = await options.queryFn(context); - return processQueryResponse(options.queryKey, result, struct); + const response = await options.queryFn(context); + return processQueryResponse(options.queryKey, response, struct); }), }); } @@ -279,6 +281,7 @@ export class BaseDataService< * * @param options - The options defining the query. Keep in mind that `queryKey` and `queryFn` are required when using data services. * Additionally `retry` and `retryDelay` are not available, retries can be customized using the `servicePolicyOptions`. + * @param options.struct - An optional struct for validating the response of the query function. * @param pageParam - An optional page parameter. * @returns The query result, exclusively the requested page is returned. */ @@ -299,7 +302,7 @@ export class BaseDataService< >, 'queryKey' | 'queryFn' > & - AdditionalQueryOptions, + AdditionalQueryOptions, pageParam?: TPageParam, ): Promise { const cache = this.#queryClient.getQueryCache(); @@ -313,12 +316,12 @@ export class BaseDataService< ...options, queryFn: (context) => this.#policy.execute(async () => { - const result = await options.queryFn({ + const response = await options.queryFn({ ...context, pageParam: context.pageParam ?? pageParam, }); - return processQueryResponse(options.queryKey, result, struct); + return processQueryResponse(options.queryKey, response, struct); }), }); diff --git a/packages/base-data-service/src/utils.ts b/packages/base-data-service/src/utils.ts index bd4fb0103d..8f3ed02a8d 100644 --- a/packages/base-data-service/src/utils.ts +++ b/packages/base-data-service/src/utils.ts @@ -2,11 +2,20 @@ import { Struct, validate } from '@metamask/superstruct'; import { QueryKey } from './BaseDataService'; +/** + * Process query responses, validating them using Superstruct if a struct is defined. + * + * @param queryKey - The query key. + * @param response - The query response + * @param struct - The struct defining the schema for the query response. + * @returns The query response, coerced by Superstruct if needed. + * @throws If the query response does not match the struct. + */ export function processQueryResponse( queryKey: QueryKey, response: Response, - struct?: Struct, -) { + struct?: Struct, +): Response { if (!struct) { return response; } @@ -15,7 +24,7 @@ export function processQueryResponse( if (error) { throw new Error( - `${queryKey} returned an invalid response: ${error.message}.`, + `${queryKey[0]} returned an invalid response: ${error.message}.`, ); } From d4d9f6a054aafafb9715145d8ee30410e9a5aca2 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 16 Jul 2026 14:25:28 +0200 Subject: [PATCH 3/3] Fix lint --- packages/base-data-service/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/base-data-service/src/utils.ts b/packages/base-data-service/src/utils.ts index 8f3ed02a8d..6b2b9f95d4 100644 --- a/packages/base-data-service/src/utils.ts +++ b/packages/base-data-service/src/utils.ts @@ -4,7 +4,7 @@ import { QueryKey } from './BaseDataService'; /** * Process query responses, validating them using Superstruct if a struct is defined. - * + * * @param queryKey - The query key. * @param response - The query response * @param struct - The struct defining the schema for the query response.