diff --git a/API_DOCS.md b/API_DOCS.md index d07d8af..c0c2101 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -77,6 +77,65 @@ getLoaders(resources[, options]) To see an example call to `getLoaders`, [check out the SWAPI example](./examples/swapi/swapi-server.js) or [the tests](./__tests__/implementation.test.js). +## Batch Resources with `properties` parameters + +Instead of accepting just a list of users (`user_ids`), a batch resource could accept both a list of users (`user_ids`) and a list of properties (`properties`) to fetch about that user: + +```js +const getUserInfo = (args: { user_ids: Array, properties: Array }): Promise> => + fetch('/userInfo', args); + +const users = getUserInfo({ + user_ids: [1, 2, 3], + properties: ['firstName', 'age'], +}); + +/** + * e.g. users => + * [ + * { id: 1, firstName: 'Alice', age: 42 }, + * { id: 2, firstName: 'Bob', age: 70 }, + * { id: 3, firstName: 'Carol', age: 50 }, + * ] + */ +``` + +To batch up calls to this resource with different `properties` for different `user_ids`, we specify `propertyBatchKey` in the config to describe the "properties" argument. +We specify `responseKey` in the config as the key in the response objects corresponds to `batchKey`. + +The config for our `getUserInfoV2` would look like this: + +```yaml +resources: + getUserInfo: + isBatchResource: true + batchKey: user_ids + newKey: user_id + propertyBatchKey: properties + responseKey: id +``` + +**IMPORTANT NOTE** +To use this feature, there are several restrictions. (Please open an issue if you're interested in helping us support other use cases): + +**Contract** + +1. The resource accepts a list of `ids` and a list of `properties`; to specify the entity IDs and the properties for each entity to fetch: + + ```js + ({ + // this is the batchKey + ids: Array, + // this is the propertyBatchKey + properties: Array, + }): Array + ``` + +2. In the response, `properties` are spread at the same level as the `responseKey`. (Check out `getFilmsV2` in [swapi example](./examples/swapi/swapi.js).) +3. All `properties` must be optional in the response object. The flow types currently don't handle the nullability of these properties correctly, so to enforce this, we recommend a build step to ensure that the underlying types are always set as maybe types. +4. The resource must have a one-to-one correspondence between the input "properties" and the output "properties". + - e.g. if we request property "name", the response must have "name" in it, and no extra data associated with it. + ## Config File The config file should be a [YAML](https://yaml.org/) file in the following format: @@ -94,6 +153,8 @@ resources: commaSeparatedBatchKey: ?string (can only use if isBatchResource=true) isResponseDictionary: ?boolean (can only use if isBatchResource=true) isBatchKeyASet: ?boolean (can only use if isBatchResource=true) + propertyBatchKey: ?string (can only use if isBatchResource=true) + responseKey: ?string (non-optional when propertyBatchKey is used) typings: language: flow @@ -125,6 +186,8 @@ Describes the shape and behaviour of the resources object you will pass to `getL | `commaSeparatedBatchKey` | (Optional) Set to true if the interface of the resource takes the batch key as a comma separated list (rather than an array of IDs, as is more common). Default: false | | `isResponseDictionary` | (Optional) Set to true if the batch resource returns the results as a dictionary with key mapped to values (instead of a list of items). If this option is supplied `reorderResultsByKey` should not be. Default: false | | `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. | ### `typings` diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index f4e3767..b81d9de 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1217,3 +1217,245 @@ test('bail if errorHandler does not return an error', async () => { ]); }); }); + +test('batch endpoint (multiple requests) with propertyBatchKey', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + propertyBatchKey: 'properties', + responseKey: 'id', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, properties, include_extra_info }) => { + if (_.isEqual(foo_ids, [2, 1])) { + expect(include_extra_info).toBe(false); + return Promise.resolve([ + { id: 1, rating: 3, name: 'Burger King' }, + { id: 2, rating: 4, name: 'In N Out' }, + ]); + } + + if (_.isEqual(foo_ids, [3])) { + expect(include_extra_info).toBe(true); + return Promise.resolve([ + { + id: 3, + rating: 5, + name: 'Shake Shack', + }, + ]); + } + }, + }; + + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + const results = await loaders.foo.loadMany([ + { foo_id: 2, properties: ['name', 'rating'], include_extra_info: false }, + { foo_id: 1, properties: ['rating'], include_extra_info: false }, + { foo_id: 3, properties: ['rating'], include_extra_info: true }, + ]); + + expect(results).toEqual([ + { id: 2, name: 'In N Out', rating: 4 }, + { id: 1, rating: 3 }, + { id: 3, rating: 5 }, + ]); + }); +}); + +test('batch endpoint with propertyBatchKey throws error for response with non existant items', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + propertyBatchKey: 'properties', + responseKey: 'foo_id', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, properties, include_extra_info }) => { + if (_.isEqual(foo_ids, [1, 2, 3])) { + expect(include_extra_info).toBe(true); + return Promise.resolve([ + { + foo_id: 1, + name: 'Shake Shack', + rating: 4, + }, + // deliberately omit 2 + { + foo_id: 3, + name: 'Burger King', + rating: 3, + }, + ]); + } else if (_.isEqual(foo_ids, [4])) { + expect(include_extra_info).toBe(false); + return Promise.resolve([ + { + foo_id: 4, + name: 'In N Out', + rating: 3.5, + }, + ]); + } + }, + }; + + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + const results = await loaders.foo.loadMany([ + { foo_id: 1, properties: ['name', 'rating'], include_extra_info: true }, + { foo_id: 2, properties: ['rating'], include_extra_info: true }, + { foo_id: 3, properties: ['rating'], include_extra_info: true }, + { foo_id: 4, properties: ['rating'], include_extra_info: false }, + ]); + + expect(results).toEqual([ + { foo_id: 1, name: 'Shake Shack', rating: 4 }, + expect.toBeError( + [ + 'Could not find foo_id = 2 in the response dict. Or your endpoint does not follow the contract we support.', + 'Please read https://github.com/Yelp/dataloader-codegen/blob/master/API_DOCS.md.', + ].join(' '), + 'BatchItemNotFoundError', + ), + { foo_id: 3, rating: 3 }, + { foo_id: 4, rating: 3.5 }, + ]); + }); +}); + +test('batch endpoint (multiple requests) with propertyBatchKey error handling', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + propertyBatchKey: 'properties', + responseKey: 'id', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, properties, include_extra_info }) => { + if (_.isEqual(foo_ids, [2, 4, 5])) { + expect(include_extra_info).toBe(true); + return Promise.resolve([ + { + id: 2, + name: 'Burger King', + rating: 3, + }, + { + id: 4, + name: 'In N Out', + rating: 3.5, + }, + { + id: 5, + name: 'Shake Shack', + rating: 4, + }, + ]); + } + if (_.isEqual(foo_ids, [1, 3])) { + expect(include_extra_info).toBe(false); + throw new Error('yikes'); + } + }, + }; + + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + const results = await loaders.foo.loadMany([ + { foo_id: 1, properties: ['name', 'rating'], include_extra_info: false }, + { foo_id: 2, properties: ['name', 'rating'], include_extra_info: true }, + { foo_id: 3, properties: ['name'], include_extra_info: false }, + { foo_id: 4, properties: ['rating'], include_extra_info: true }, + { foo_id: 5, properties: ['name'], include_extra_info: true }, + ]); + + expect(results).toEqual([ + expect.toBeError(/yikes/), + { id: 2, name: 'Burger King', rating: 3 }, + expect.toBeError(/yikes/), + { id: 4, rating: 3.5 }, + { id: 5, name: 'Shake Shack' }, + ]); + }); +}); + +test('batch endpoint with propertyBatchKey with reorderResultsByKey handles response with non existant items', async () => { + const config = { + resources: { + foo: { + isBatchResource: true, + docsLink: 'example.com/docs/bar', + batchKey: 'foo_ids', + newKey: 'foo_id', + reorderResultsByKey: 'foo_id', + propertyBatchKey: 'properties', + responseKey: 'foo_id', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, properties, include_extra_info }) => { + if (_.isEqual(foo_ids, [1, 2, 3])) { + expect(include_extra_info).toBe(true); + return Promise.resolve([ + { foo_id: 3, rating: 4, name: 'Shake Shack' }, + { foo_id: 1, rating: 3, name: 'Burger King' }, + // deliberately omit 2 + ]); + } else if (_.isEqual(foo_ids, [4])) { + expect(include_extra_info).toBe(false); + return Promise.resolve([{ foo_id: 4, rating: 5, name: 'In N Out' }]); + } + }, + }; + await createDataLoaders(config, async (getLoaders) => { + const loaders = getLoaders(resources); + + const results = await loaders.foo.loadMany([ + { foo_id: 1, properties: ['name', 'rating'], include_extra_info: true }, + { foo_id: 2, properties: ['name'], include_extra_info: true }, + { foo_id: 3, properties: ['rating'], include_extra_info: true }, + { foo_id: 4, properties: ['rating'], include_extra_info: false }, + ]); + + expect(results).toEqual([ + { foo_id: 1, rating: 3, name: 'Burger King' }, + expect.toBeError( + [ + 'Could not find foo_id = 2 in the response dict. Or your endpoint does not follow the contract we support.', + 'Please read https://github.com/Yelp/dataloader-codegen/blob/master/API_DOCS.md.', + ].join(' '), + 'BatchItemNotFoundError', + ), + { foo_id: 3, rating: 4 }, + { foo_id: 4, rating: 5 }, + ]); + }); +}); diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index 4f840c5..5b7204f 100644 --- a/examples/swapi/swapi-loaders.js +++ b/examples/swapi/swapi-loaders.js @@ -13,10 +13,12 @@ import { cacheKeyOptions, CaughtResourceError, defaultErrorHandler, + getBatchKeysForPartitionItems, partitionItems, resultsDictToList, sortByKeys, unPartitionResults, + unPartitionResultsByBatchKeyPartition, } from 'dataloader-codegen/lib/runtimeHelpers'; /** @@ -325,11 +327,26 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * ``` + * + * Returns: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We could also have more than one batch key. + * + * Example: + * + * ```js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * ``` * * Returns: @@ -337,7 +354,13 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('planet_id', keys); + let requestGroups; + + if (false) { + requestGroups = partitionItems(['planet_id', 'undefined'], keys); + } else { + requestGroups = partitionItems('planet_id', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -487,8 +510,35 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (false) { + const batchKeyPartition = getBatchKeysForPartitionItems( + 'planet_id', + ['planet_id', 'undefined'], + keys, + ); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'undefined', + ['planet_id', 'undefined'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'planet_id', + 'undefined', + 'undefined', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -599,11 +649,26 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * ``` + * + * Returns: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We could also have more than one batch key. + * + * Example: + * + * ```js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * ``` * * Returns: @@ -611,7 +676,13 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('person_id', keys); + let requestGroups; + + if (false) { + requestGroups = partitionItems(['person_id', 'undefined'], keys); + } else { + requestGroups = partitionItems('person_id', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -758,8 +829,35 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (false) { + const batchKeyPartition = getBatchKeysForPartitionItems( + 'person_id', + ['person_id', 'undefined'], + keys, + ); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'undefined', + ['person_id', 'undefined'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'person_id', + 'undefined', + 'undefined', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -870,11 +968,26 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * ``` + * + * Returns: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We could also have more than one batch key. + * + * Example: + * + * ```js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * ``` * * Returns: @@ -882,7 +995,13 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('vehicle_id', keys); + let requestGroups; + + if (false) { + requestGroups = partitionItems(['vehicle_id', 'undefined'], keys); + } else { + requestGroups = partitionItems('vehicle_id', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1032,8 +1151,35 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (false) { + const batchKeyPartition = getBatchKeysForPartitionItems( + 'vehicle_id', + ['vehicle_id', 'undefined'], + keys, + ); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'undefined', + ['vehicle_id', 'undefined'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'vehicle_id', + 'undefined', + 'undefined', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -1153,11 +1299,26 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * ``` + * + * Returns: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We could also have more than one batch key. + * + * Example: + * + * ```js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * ``` * * Returns: @@ -1165,7 +1326,13 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('film_id', keys); + let requestGroups; + + if (false) { + requestGroups = partitionItems(['film_id', 'undefined'], keys); + } else { + requestGroups = partitionItems('film_id', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1310,8 +1477,31 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (false) { + const batchKeyPartition = getBatchKeysForPartitionItems('film_id', ['film_id', 'undefined'], keys); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'undefined', + ['film_id', 'undefined'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'film_id', + 'undefined', + 'undefined', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -1361,7 +1551,9 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * "isBatchResource": true, * "batchKey": "film_ids", * "newKey": "film_id", - * "nestedPath": "properties" + * "nestedPath": "properties", + * "propertyBatchKey": "properties", + * "responseKey": "id" * } * ``` */ @@ -1426,11 +1618,26 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * Example: * * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * ``` + * + * Returns: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We could also have more than one batch key. + * + * Example: + * + * ```js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * ``` * * Returns: @@ -1438,7 +1645,13 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('film_id', keys); + let requestGroups; + + if (true) { + requestGroups = partitionItems(['film_id', 'properties'], keys); + } else { + requestGroups = partitionItems('film_id', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all( @@ -1555,32 +1768,6 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade } } - if (!(response instanceof Error)) { - /** - * Check to see the resource contains the same number - * of items that we requested. If not, since there's - * no "reorderResultsByKey" specified for this resource, - * we don't know _which_ key's response is missing. Therefore - * it's unsafe to return the response array back. - */ - if (response.length !== requests.length) { - /** - * We must return errors for all keys in this group :( - */ - response = new BatchItemNotFoundError( - [ - `[dataloader-codegen :: getFilmsV2] Resource returned ${response.length} items, but we requested ${requests.length} items.`, - 'Add reorderResultsByKey to the config for this resource to be able to handle a partial response.', - ].join(' '), - ); - - // Tell flow that BatchItemNotFoundError extends Error. - // It's an issue with flowgen package, but not an issue with Flow. - // @see https://github.com/Yelp/dataloader-codegen/pull/35#discussion_r394777533 - invariant(response instanceof Error, 'expected BatchItemNotFoundError to be an Error'); - } - } - /** * If the resource returns an Error, we'll want to copy and * return that error as the return value for every request in @@ -1622,8 +1809,31 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade }), ); - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (true) { + const batchKeyPartition = getBatchKeysForPartitionItems('film_id', ['film_id', 'properties'], keys); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + 'properties', + ['film_id', 'properties'], + keys, + ); + return unPartitionResultsByBatchKeyPartition( + 'film_id', + 'properties', + 'id', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, diff --git a/examples/swapi/swapi.dataloader-config.yaml b/examples/swapi/swapi.dataloader-config.yaml index 205eab6..08222da 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -34,6 +34,8 @@ resources: batchKey: film_ids newKey: film_id nestedPath: properties + propertyBatchKey: properties + responseKey: id getRoot: docsLink: https://swapi.dev/documentation#root isBatchResource: false diff --git a/examples/swapi/swapi.js b/examples/swapi/swapi.js index 7a4568e..235bf2a 100644 --- a/examples/swapi/swapi.js +++ b/examples/swapi/swapi.js @@ -62,11 +62,11 @@ export type SWAPI_Film = $ReadOnly<{| export type SWAPI_Film_V2 = $ReadOnly<{| properties: $ReadOnlyArray<{| - film_id: number, - title: string, - episode_id: number, - director: string, - producer: string, + id: number, + title: ?string, + episode_id: ?number, + director: ?string, + producer: ?string, |}>, |}>; @@ -118,7 +118,7 @@ module.exports = function (): SWAPIClientlibTypes { return Promise.resolve({ properties: [ { - film_id: 4, + id: 4, director: 'George Lucas', producer: 'Rick McCallum', episode_id: 1, diff --git a/schema.json b/schema.json index c4bfe37..d8e834e 100644 --- a/schema.json +++ b/schema.json @@ -97,7 +97,18 @@ "isBatchKeyASet": { "type": "boolean", "description": "(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": { + "type": "string", + "description": "(Optional) The argument to the resource that represents the optional properties we want to fetch. (e.g. usually 'properties' or 'features')" + }, + "responseKey": { + "type": "string", + "description": "(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." } + }, + "dependencies": { + "propertyBatchKey": { "required": ["responseKey"] } } }, "nonBatchInfo": { diff --git a/src/codegen.ts b/src/codegen.ts index 95803bf..7102225 100644 --- a/src/codegen.ts +++ b/src/codegen.ts @@ -61,10 +61,12 @@ export default function codegen( cacheKeyOptions, CaughtResourceError, defaultErrorHandler, + getBatchKeysForPartitionItems, partitionItems, resultsDictToList, sortByKeys, unPartitionResults, + unPartitionResultsByBatchKeyPartition, } from '${runtimeHelpers}'; diff --git a/src/config.ts b/src/config.ts index ffe7301..a57f83d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -26,6 +26,8 @@ export interface BatchResourceConfig { // TODO: Assert somehow/somewhere that both isResponseDictionary and reorderResultsByKey aren't set isResponseDictionary?: boolean; isBatchKeyASet?: boolean; + propertyBatchKey?: string; + responseKey?: string; } export interface NonBatchResourceConfig { diff --git a/src/implementation.ts b/src/implementation.ts index e844ca2..fef9086 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -85,7 +85,10 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado resourceConfig.isBatchResource === true, `${errorPrefix(resourcePath)} Expected getBatchLoader to be called with a batch resource config`, ); - + assert( + typeof resourceConfig.batchKey === 'string' && typeof resourceConfig.newKey === 'string', + `${errorPrefix(resourcePath)} Expected both batchKey and newKey for a batch resource`, + ); // The reference at runtime to where the underlying resource lives const resourceReference = ['resources', ...resourcePath].join('.'); @@ -153,11 +156,26 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado * Example: * * \`\`\`js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 7, include_extra_info: true }, * { bar_id: 8, include_extra_info: false }, * { bar_id: 9, include_extra_info: true }, - * ], 'bar_id') + * ]) + * \`\`\` + * + * Returns: + * \`[ [ 0, 2 ], [ 1 ] ]\` + * + * We could also have more than one batch key. + * + * Example: + * + * \`\`\`js + * partitionItems(['bar_id', 'properties'], [ + * { bar_id: 7, properties: ['property_1'], include_extra_info: true }, + * { bar_id: 8, properties: ['property_2'], include_extra_info: false }, + * { bar_id: 9, properties: ['property_3'], include_extra_info: true }, + * ]) * \`\`\` * * Returns: @@ -165,7 +183,16 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado * * We'll refer to each element in the group as a "request ID". */ - const requestGroups = partitionItems('${resourceConfig.newKey}', keys); + let requestGroups; + + if (${typeof resourceConfig.propertyBatchKey === 'string'}) { + requestGroups = partitionItems([ + '${resourceConfig.newKey}', + '${resourceConfig.propertyBatchKey}' + ], keys); + } else { + requestGroups = partitionItems('${resourceConfig.newKey}', keys); + } // Map the request groups to a list of Promises - one for each request const groupedResults = await Promise.all(requestGroups.map(async requestIDs => { @@ -179,7 +206,7 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado const requests = requestIDs.map(id => keys[id]); ${(() => { - const { batchKey, newKey, commaSeparatedBatchKey } = resourceConfig; + const { batchKey, newKey, commaSeparatedBatchKey, propertyBatchKey } = resourceConfig; let batchKeyParam = `['${batchKey}']: requests.map(k => k['${newKey}'])`; if (commaSeparatedBatchKey === true) { @@ -272,9 +299,17 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado } ${(() => { - const { reorderResultsByKey, isResponseDictionary } = resourceConfig; - - if (!isResponseDictionary && reorderResultsByKey == null) { + const { reorderResultsByKey, isResponseDictionary, propertyBatchKey } = resourceConfig; + if ( + !isResponseDictionary && + reorderResultsByKey == null && + /** + * When there's propertyBatchKey and propertyNewKey, the resource might + * contain less number of items that we requested. It's valid, so we + * should skip the check. + */ + !(typeof propertyBatchKey === 'string') + ) { return ` if (!(response instanceof Error)) { /** @@ -387,8 +422,35 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado return response; })) - // Split the results back up into the order that they were requested - return unPartitionResults(requestGroups, groupedResults); + /** + * When there's propertyBatchKey, the resource might contain less number of items that we requested. + * We need the value of batchKey and propertyBatchKey in requests group to help us split the results + * back up into the order that they were requested. + */ + if (${typeof resourceConfig.propertyBatchKey === 'string'}) { + const batchKeyPartition = getBatchKeysForPartitionItems( + '${resourceConfig.newKey}', + ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], + keys + ); + const propertyBatchKeyPartiion = getBatchKeysForPartitionItems( + '${resourceConfig.propertyBatchKey}', + ['${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}'], + keys + ); + return unPartitionResultsByBatchKeyPartition( + '${resourceConfig.newKey}', + '${resourceConfig.propertyBatchKey}', + '${resourceConfig.responseKey}', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ${ diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index 97375ee..2cd4dfb 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -62,17 +62,17 @@ export const cacheKeyOptions = { /** * Take in all objects passed to .load(), and bucket them by the non - * batchKey attributes. + * batch keys (i.e. `batchKey` and `propertyBatchKey`) attributes. * * We use this to chunk up the requests to the resource. * * Example: * ```js - * partitionItems([ + * partitionItems('bar_id', [ * { bar_id: 2, include_extra_info: true }, * { bar_id: 3, include_extra_info: false }, * { bar_id: 4, include_extra_info: true }, - * ], 'bar_id') + * ]) * ``` * * Returns: @@ -80,13 +80,16 @@ export const cacheKeyOptions = { * * TODO: add generic instead of 'object' for the items array argument */ -export function partitionItems(ignoreKey: string, items: ReadonlyArray): ReadonlyArray> { +export function partitionItems( + ignoreKeys: Array | string, + items: ReadonlyArray, +): ReadonlyArray> { const groups: { [key: string]: Array; } = {}; items.forEach((item, i) => { - const hash = objectHash(_.omit(item, ignoreKey), { algorithm: 'passthrough' }); + const hash = objectHash(_.omit(item, ignoreKeys), { algorithm: 'passthrough' }); groups[hash] = groups[hash] || []; groups[hash].push(i); }); @@ -94,6 +97,48 @@ export function partitionItems(ignoreKey: string, items: ReadonlyArray): return Object.values(groups); } +/** + * Take in all objects passed to .load(), and bucket them by the non + * batch keys (i.e. `batchKey` and `propertyBatchKey`) attributes. + * Return batch keys value for each partition items. + * + * This function is only called when we have propertyBatchKey, and it's + * used to map result to the order of requests. + * + * Example: + * ```js + * getBatchKeyForPartitionItems( + * 'bar_id', + * ['bar_id', 'properties'], + * [ + * { bar_id: 2, properties: ['name'], include_extra_info: true }, + * { bar_id: 3, properties: ['rating'], include_extra_info: false }, + * { bar_id: 2, properties: ['rating'], include_extra_info: true }, + * ]) + * ``` + * + * Returns: + * `[ [ 2, 2 ], [ 3 ] ]` + * + * TODO: add generic instead of 'object' for the items array argument + */ +export function getBatchKeysForPartitionItems( + batchKey: string, + ignoreKeys: Array, + items: ReadonlyArray, +): ReadonlyArray> { + const groups: { + [key: string]: Array; + } = {}; + items.forEach((item, i) => { + const hash = objectHash(_.omit(item, ignoreKeys), { algorithm: 'passthrough' }); + groups[hash] = groups[hash] || []; + groups[hash].push(items[i][batchKey]); + }); + + return Object.values(groups); +} + /** * Utility function to sort array of objects by a list of corresponding IDs * @@ -253,6 +298,142 @@ export function unPartitionResults( }); } +/** + * Perform the inverse mapping from partitionItems on the nested results we get + * back from the service. This function is only called when we have propertyBatchKey. + * We currently only support one specific response contract. + * + * propertyBatchKey is not returned in a nested object, but spread at top level as well. + * If we have 'id' as responseKey and 'properties' as propertyBatchKey, + * the resultGroups should look like this: + * [ + * [ { id: 2, name: 'Burger King', rating: 3 } ], + * [ { id: 1, name: 'In N Out', rating: 4 } ] + * ], + * + * + * IMPORTANT NOTE: The contract must have a one-to-one correspondence between the input propertyBatchKey and the output propertyBatchKey. + * i.e. if we have property: 'name' in the request, the response must have 'name' in it, and no extra data associated with it. + * + * Example + * Request args: + * [ + * { bar_id: 2, properties: ['name'], include_extra_info: true }, + * { bar_id: 1, properties: ['rating'], include_extra_info: false }, + * { bar_id: 2, properties: ['rating'], include_extra_info: true }, + * ] + * + * ```js + * unPartitionResultsByBatchKeyPartition( + * newKey = 'bar_id', + * propertyBatchKey = 'properties', + * responseKey = 'id' + * batchKeyPartition = [ [2, 2], [1] ], + * propertyBatchKeyPartion = [ [['name'], ['rating']], [['rating']] ], + * requestGroups = [ [0, 2], [1] ], + * resultGroups = [ + * [ { id: 2, name: 'Burger King', rating: 3 } ], + * [ { id: 1, name: 'In N Out', rating: 4 } ] + * ], + * ) + * ``` + * + * Returns: + * ``` + * [ + * { id: 2, name: 'Burger King' }, + * { id: 1, rating: 4 }, + * { id: 2, rating: 3 }, + * ] + */ +export function unPartitionResultsByBatchKeyPartition>( + newKey: string, + propertyBatchKey: string, + responseKey: string, + batchKeyPartition: ReadonlyArray>, + propertyBatchKeyPartion: ReadonlyArray>, + /** Should be a nested array of IDs, as generated by partitionItems */ + requestGroups: ReadonlyArray>, + /** The results back from the service, in the same shape as groups */ + resultGroups: ReadonlyArray>, +): ReadonlyArray { + /** + * e.g. with our inputs, produce: + * ```js + * [ + * [ + * { order: 0, result: { id: 2, name: 'Burger King' }, + * { order: 2, result: { id: 2, rating: 3 } }, + * ], + * [ + * { order: 1, result: { id: 1, rating: 4 } }, + * ] + * ] + * ``` + */ + const zippedGroups: ReadonlyArray> = requestGroups.map( + (ids, i) => { + return ids.map((id, j) => { + let result = null; + for (const resultElement of Object.values(resultGroups)[i]) { + // There's error in the result we should return + if (resultElement instanceof CaughtResourceError) { + result = resultElement; + break; + } + + // Find the response that matches the requested batchKey + if (resultElement[responseKey] === batchKeyPartition[i][j]) { + // Only responseKey and the requested properties will be in the final result. + result = _.pick(resultElement, [responseKey, ...propertyBatchKeyPartion[i][j]]); + break; + } + } + + // If requested property doesn't exist in resultElement, we should throw BatchItemNotFoundError error. + if (result === null) { + return { + order: id, + result: new BatchItemNotFoundError( + [ + `Could not find ${responseKey} = ${batchKeyPartition[i][j]} in the response dict.`, + `Or your endpoint does not follow the contract we support.`, + `Please read https://github.com/Yelp/dataloader-codegen/blob/master/API_DOCS.md.`, + ].join(' '), + ), + }; + } else { + return { order: id, result }; + } + }); + }, + ); + /** + * Flatten and sort the groups - e.g.: + * ```js + * [ + * { order: 0, result: { id: 2, name: 'Burger King' } }, + * { order: 1, result: { id: 1, rating: 4 } }, + * { order: 2, result: { id: 2, rating: 3 } } + * ] + * ``` + */ + const sortedResults: ReadonlyArray<{ order: number; result: T | Error }> = _.sortBy(_.flatten(zippedGroups), [ + 'order', + ]); + + // Now that we have a sorted array, return the actual results! + return sortedResults + .map((r) => r.result) + .map((result) => { + if (result instanceof CaughtResourceError) { + return result.cause; + } else { + return result; + } + }); +} + /** * Turn a dictionary of results into an ordered list *