From 2fc418d2075e0f2bc3a1eafda9e4db4bbe3063e4 Mon Sep 17 00:00:00 2001 From: Mauro Arce Date: Fri, 2 May 2025 09:31:46 -0700 Subject: [PATCH 1/4] Add maxBatchSize dataloader config support --- src/config.ts | 1 + src/implementation.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index a57f83d..92a9268 100644 --- a/src/config.ts +++ b/src/config.ts @@ -28,6 +28,7 @@ export interface BatchResourceConfig { isBatchKeyASet?: boolean; propertyBatchKey?: string; responseKey?: string; + maxBatchSize?: number; } export interface NonBatchResourceConfig { diff --git a/src/implementation.ts b/src/implementation.ts index 43287ef..9ac0dc8 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -514,7 +514,8 @@ function getPropertyBatchLoader(resourceConfig: BatchResourceConfig, resourcePat * flow errors :( */ '' } - ...cacheKeyOptions + ...cacheKeyOptions, + ${resourceConfig.maxBatchSize ? `maxBatchSize: ${resourceConfig.maxBatchSize},` : ''} } )`; } From 53698bba29924b16dd8115077ef14c20afdb721b Mon Sep 17 00:00:00 2001 From: Mauro Arce Date: Fri, 2 May 2025 10:08:02 -0700 Subject: [PATCH 2/4] Add test --- __tests__/implementation.test.js | 62 ++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 10725ce..d93243c 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1246,6 +1246,68 @@ test('bail if errorHandler does not return an error', async () => { }); }); +test('batch endpoint with propertyBatchKey and maxBatchSize', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + propertyBatchKey: 'properties', // This is required for maxBatchSize to work + responseKey: 'id', + maxBatchSize: 2, // Set a maximum batch size of 2 + }, + }, + }; + + // Track each batch of IDs that the resource function receives + const receivedBatches = []; + + const resources = { + foo: ({ foo_ids, properties }) => { + receivedBatches.push([...foo_ids]); + return Promise.resolve( + foo_ids.map(id => ({ + id, + name: `name-${id}`, + rating: id + 1 + })) + ); + }, + }; + + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + // Request 5 items at once, which should be split into 3 batches (2 + 2 + 1) + const results = await Promise.all([ + loaders.foo.load({ foo_id: 1, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 2, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 3, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 4, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 5, properties: ['name', 'rating'] }), + ]); + + // Verify that all results were returned correctly + expect(results).toEqual([ + { id: 1, name: 'name-1', rating: 2 }, + { id: 2, name: 'name-2', rating: 3 }, + { id: 3, name: 'name-3', rating: 4 }, + { id: 4, name: 'name-4', rating: 5 }, + { id: 5, name: 'name-5', rating: 6 }, + ]); + + // Verify that the requests were batched correctly + // We should have 3 batches with max 2 IDs. + expect(receivedBatches.map(batch => batch.length)).toEqual([2, 2, 1]); + + // Verify that all IDs were requested + const allRequestedIds = receivedBatches.flat().sort(); + expect(allRequestedIds).toEqual([1, 2, 3, 4, 5]); + }); +}); + test('batch endpoint (multiple requests) with propertyBatchKey', async () => { const config = { resources: { From fec60be5d0b23536221bff1c560b7b4c50a78f22 Mon Sep 17 00:00:00 2001 From: Mauro Arce Date: Fri, 2 May 2025 14:04:25 -0700 Subject: [PATCH 3/4] Add maxBatchSize to getBatchLoader as well --- __tests__/implementation.test.js | 65 +++++++++++++++++++++++++++++++- src/implementation.ts | 3 +- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index d93243c..6e3da3f 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1246,6 +1246,67 @@ test('bail if errorHandler does not return an error', async () => { }); }); +test('batch endpoint with maxBatchSize', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + responseKey: 'id', + maxBatchSize: 3, + }, + }, + }; + + // Track each batch of IDs that the resource function receives + const receivedBatches = []; + + const resources = { + foo: ({ foo_ids, properties }) => { + receivedBatches.push([...foo_ids]); + return Promise.resolve( + foo_ids.map(id => ({ + id, + name: `name-${id}`, + rating: id + 1 + })) + ); + }, + }; + + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + // Request 5 items at once, which should be split into 3 batches (2 + 2 + 1) + const results = await Promise.all([ + loaders.foo.load({ foo_id: 1, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 2, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 3, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 4, properties: ['name', 'rating'] }), + loaders.foo.load({ foo_id: 5, properties: ['name', 'rating'] }), + ]); + + // Verify that all results were returned correctly + expect(results).toEqual([ + { id: 1, name: 'name-1', rating: 2 }, + { id: 2, name: 'name-2', rating: 3 }, + { id: 3, name: 'name-3', rating: 4 }, + { id: 4, name: 'name-4', rating: 5 }, + { id: 5, name: 'name-5', rating: 6 }, + ]); + + // Verify that the requests were batched correctly + // We should have 3 batches with max 2 IDs. + expect(receivedBatches.map(batch => batch.length)).toEqual([3, 2]); + + // Verify that all IDs were requested + const allRequestedIds = receivedBatches.flat().sort(); + expect(allRequestedIds).toEqual([1, 2, 3, 4, 5]); + }); +}); + test('batch endpoint with propertyBatchKey and maxBatchSize', async () => { const config = { resources: { @@ -1254,9 +1315,9 @@ test('batch endpoint with propertyBatchKey and maxBatchSize', async () => { docsLink: 'example.com/docs/bar', batchKey: 'foo_ids', newKey: 'foo_id', - propertyBatchKey: 'properties', // This is required for maxBatchSize to work + propertyBatchKey: 'properties', responseKey: 'id', - maxBatchSize: 2, // Set a maximum batch size of 2 + maxBatchSize: 2, }, }, }; diff --git a/src/implementation.ts b/src/implementation.ts index 9ac0dc8..64849ab 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -416,7 +416,8 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado * flow errors :( */ '' } - ...cacheKeyOptions + ...cacheKeyOptions, + ${resourceConfig.maxBatchSize ? `maxBatchSize: ${resourceConfig.maxBatchSize},` : ''} } )`; } From 732f246ff0d56b0069bba013162a15ad648a06fc Mon Sep 17 00:00:00 2001 From: Mauro Arce Date: Fri, 2 May 2025 14:19:44 -0700 Subject: [PATCH 4/4] Update documentation and some inline comments --- API_DOCS.md | 1 + __tests__/implementation.test.js | 6 ++---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index c0c2101..cb0b5cb 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -188,6 +188,7 @@ Describes the shape and behaviour of the resources object you will pass to `getL | `isBatchKeyASet` | (Optional) Set to true if the interface of the resource takes the batch key as a set (rather than an array). For example, when using a generated clientlib based on swagger where `uniqueItems: true` is set for the batchKey parameter. Default: false. | | `propertyBatchKey` | (Optional) The argument to the resource that represents the optional properties we want to fetch. (e.g. usually 'properties' or 'features'). | | `responseKey` | (Non-optional when propertyBatchKey is used) The key in the response objects corresponds to `batchKey`. This should be the only field that are marked as required in your swagger endpoint response, except nestedPath. | +| `maxBatchSize` | (Optional) Limits the number of items that can be batched together in a single request. When more items are requested than this limit, multiple requests will be made. This can help prevent hitting URI length limits or timeouts for large batches. | ### `typings` diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 6e3da3f..09d93f3 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1279,7 +1279,7 @@ test('batch endpoint with maxBatchSize', async () => { await createDataLoaders(config, async (getLoaders) => { const loaders = getLoaders(resources); - // Request 5 items at once, which should be split into 3 batches (2 + 2 + 1) + // Request 5 items at once, which should be split by maxBatchSize later in the test. const results = await Promise.all([ loaders.foo.load({ foo_id: 1, properties: ['name', 'rating'] }), loaders.foo.load({ foo_id: 2, properties: ['name', 'rating'] }), @@ -1298,7 +1298,6 @@ test('batch endpoint with maxBatchSize', async () => { ]); // Verify that the requests were batched correctly - // We should have 3 batches with max 2 IDs. expect(receivedBatches.map(batch => batch.length)).toEqual([3, 2]); // Verify that all IDs were requested @@ -1341,7 +1340,7 @@ test('batch endpoint with propertyBatchKey and maxBatchSize', async () => { await createDataLoaders(config, async (getLoaders) => { const loaders = getLoaders(resources); - // Request 5 items at once, which should be split into 3 batches (2 + 2 + 1) + // Request 5 items at once, which should be split by maxBatchSize later in the test. const results = await Promise.all([ loaders.foo.load({ foo_id: 1, properties: ['name', 'rating'] }), loaders.foo.load({ foo_id: 2, properties: ['name', 'rating'] }), @@ -1360,7 +1359,6 @@ test('batch endpoint with propertyBatchKey and maxBatchSize', async () => { ]); // Verify that the requests were batched correctly - // We should have 3 batches with max 2 IDs. expect(receivedBatches.map(batch => batch.length)).toEqual([2, 2, 1]); // Verify that all IDs were requested