From e1d039a1ec98ca0478c7c2d7136615c1628989d9 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 30 Jun 2021 21:15:50 -0700 Subject: [PATCH 01/27] add new config to batch properties calls --- API_DOCS.md | 2 + __tests__/implementation.test.js | 63 ++ examples/swapi/swapi-loaders.js | 918 ++++++++++++-------- examples/swapi/swapi.dataloader-config.yaml | 1 + schema.json | 4 + src/codegen.ts | 2 + src/config.ts | 1 + src/implementation.ts | 75 +- src/runtimeHelpers.ts | 190 +++- 9 files changed, 885 insertions(+), 371 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index d07d8af..25ea1b4 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -94,6 +94,7 @@ 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) typings: language: flow @@ -125,6 +126,7 @@ 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 nested list of optional properties we want to fetch. (e.g. usually 'properties' or 'features'). | ### `typings` diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index f4e3767..15507f3 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1217,3 +1217,66 @@ test('bail if errorHandler does not return an error', async () => { ]); }); }); + +test('batch endpoint with propertyBatchKey without reorderResultsByKey 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', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, bar }) => { + if (_.isEqual(foo_ids, [1, 2, 3])) { + 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])) { + 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).toMatchObject([ + { foo_id: 1, name: 'Shake Shack', rating: 4 }, + expect.toBeError( + 'Could not find newKey = "2" and propertyNewKey = "rating" in the response dict. Or your endpoint does not follow the contract we support.', + 'BatchItemNotFoundError', + ), + { foo_id: 3, rating: 3 }, + { foo_id: 4, rating: 3.5 }, + ]); + }); +}); diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index 4f840c5..f8f47de 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'; /** @@ -274,70 +276,91 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems([ - * { 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'll refer to each element in the group as a "request ID". - */ - const requestGroups = partitionItems('planet_id', keys); + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * We'll refer to each element in the group as a "request ID". + */ + 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,34 @@ 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', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -548,70 +597,91 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems([ - * { 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'll refer to each element in the group as a "request ID". - */ - const requestGroups = partitionItems('person_id', keys); + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * We'll refer to each element in the group as a "request ID". + */ + 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 +828,34 @@ 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', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -819,70 +915,91 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems([ - * { 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'll refer to each element in the group as a "request ID". - */ - const requestGroups = partitionItems('vehicle_id', keys); + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * We'll refer to each element in the group as a "request ID". + */ + 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 +1149,34 @@ 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', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -1102,70 +1245,91 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems([ - * { 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'll refer to each element in the group as a "request ID". - */ - const requestGroups = partitionItems('film_id', keys); + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * We'll refer to each element in the group as a "request ID". + */ + 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 +1474,30 @@ 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', + batchKeyPartition, + propertyBatchKeyPartiion, + requestGroups, + groupedResults, + ); + } else { + // Split the results back up into the order that they were requested + return unPartitionResults(requestGroups, groupedResults); + } }, { ...cacheKeyOptions, @@ -1361,7 +1547,8 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * "isBatchResource": true, * "batchKey": "film_ids", * "newKey": "film_id", - * "nestedPath": "properties" + * "nestedPath": "properties", + * "propertyBatchKey": "properties" * } * ``` */ @@ -1375,70 +1562,91 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems([ - * { 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'll refer to each element in the group as a "request ID". - */ - const requestGroups = partitionItems('film_id', keys); + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * We'll refer to each element in the group as a "request ID". + */ + 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 +1763,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 +1804,30 @@ 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', + 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..af98605 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -34,6 +34,7 @@ resources: batchKey: film_ids newKey: film_id nestedPath: properties + propertyBatchKey: properties getRoot: docsLink: https://swapi.dev/documentation#root isBatchResource: false diff --git a/schema.json b/schema.json index c4bfe37..10083ff 100644 --- a/schema.json +++ b/schema.json @@ -97,6 +97,10 @@ "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 nested list of optional properties we want to fetch. (e.g. usually 'properties' or 'features')" } } }, 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..d17284c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -20,6 +20,7 @@ export interface BatchResourceConfig { isBatchResource: true; batchKey: string; newKey: string; + propertyBatchKey: string; reorderResultsByKey?: string; nestedPath?: string; commaSeparatedBatchKey?: boolean; diff --git a/src/implementation.ts b/src/implementation.ts index e844ca2..e36ac07 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -153,19 +153,42 @@ 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: + * \`[ [ 0, 2 ], [ 1 ] ]\` * 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 +202,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 +295,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 +418,34 @@ 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}', + 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..6cd3d82 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,141 @@ 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 'bar_id' as newKey and 'properties' as propertyBatchKey, + * the resultGroups should look like this: + * [ + * [ { bar_id: 2, name: 'Burger King', rating: 3 } ], + * [ { bar_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 assciated 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', + * batchKeyPartition = [ [2, 2], [1] ], + * propertyBatchKeyPartion = [ [['name'], ['rating']], [['rating']] ], + * requestGroups = [ [0, 2], [1] ], + * resultGroups = [ + * [ { bar_id: 2, name: 'Burger King', rating: 3 } ], + * [ { bar_id: 1, name: 'In N Out', rating: 4 } ] + * ], + * ) + * ``` + * + * Returns: + * ``` + * [ + * { bar_id: 2, name: 'Burger King' }， + * { bar_id: 1, rating: 4 }， + * { bar_id: 2, rating: 3 }， + * ] + */ +export function unPartitionResultsByBatchKeyPartition>( + newKey: string, + propertyBatchKey: 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: { bar_id: 2, name: 'Burger King' }, + * { order: 2, result: { bar_id: 2, rating: 3 } }, + * ], + * [ + * { order: 1, result: { bar_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; + } + + if (Object.values(resultElement).includes(batchKeyPartition[i][j])) { + result = Object.assign( + {}, + ...[newKey, ...propertyBatchKeyPartion[i][j]].map((key) => ({ + [key]: resultElement[key], + })), + ); + 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 newKey = "${batchKeyPartition[i][j]}" and propertyNewKey = "${propertyBatchKeyPartion[i][j]}" in the response dict.`, + `Or your endpoint does not follow the contract we support.`, + ].join(' '), + ), + }; + } else { + return { order: id, result: result }; + } + }); + }, + ); + /** + * Flatten and sort the groups - e.g.: + * ```js + * [ + * { order: 0, result: { bar_id: 2, name: 'Burger King' } }, + * { order: 1, result: { bar_id: 1, rating: 4 } }, + * { order: 2, result: { bar_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 * From 1f0d45e306f55060b7bcf4576d791a24d2e652d1 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Thu, 1 Jul 2021 13:56:00 -0700 Subject: [PATCH 02/27] add unit tests to add coverage and add assert when batchKey and newKey don't exist for a batch resource --- __tests__/implementation.test.js | 171 ++++++++++++++++++++++++++++++- examples/swapi/swapi.js | 8 +- src/implementation.ts | 5 +- src/runtimeHelpers.ts | 2 +- 4 files changed, 178 insertions(+), 8 deletions(-) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 15507f3..1a6d3a8 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1218,7 +1218,61 @@ test('bail if errorHandler does not return an error', async () => { }); }); -test('batch endpoint with propertyBatchKey without reorderResultsByKey throws error for response with non existant items', 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', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, properties, include_extra_info }) => { + if (_.isEqual(foo_ids, [2, 1])) { + expect(include_extra_info).toBe(false); + return Promise.resolve([ + { foo_id: 1, rating: 3, name: 'Burger King' }, + { foo_id: 2, rating: 4, name: 'In N Out' }, + ]); + } + + if (_.isEqual(foo_ids, [3])) { + expect(include_extra_info).toBe(true); + return Promise.resolve([ + { + foo_id: 3, + rating: 5, + name: 'Shake Shack', + extra_stuff: 'lorem ipsum', + }, + ]); + } + }, + }; + + 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([ + { foo_id: 2, name: 'In N Out', rating: 4 }, + { foo_id: 1, rating: 3 }, + { foo_id: 3, rating: 5 }, + ]); + }); +}); + +test('batch endpoint with propertyBatchKey throws error for response with non existant items', async () => { const config = { resources: { foo: { @@ -1269,7 +1323,7 @@ test('batch endpoint with propertyBatchKey without reorderResultsByKey throws er { foo_id: 4, properties: ['rating'], include_extra_info: false }, ]); - expect(results).toMatchObject([ + expect(results).toEqual([ { foo_id: 1, name: 'Shake Shack', rating: 4 }, expect.toBeError( 'Could not find newKey = "2" and propertyNewKey = "rating" in the response dict. Or your endpoint does not follow the contract we support.', @@ -1280,3 +1334,116 @@ test('batch endpoint with propertyBatchKey without reorderResultsByKey throws er ]); }); }); + +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', + }, + }, + }; + + 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([ + { + foo_id: 2, + name: 'Burger King', + rating: 3, + }, + { + foo_id: 4, + name: 'In N Out', + rating: 3.5, + }, + { + foo_id: 5, + name: 'Shake Shack', + rating: 4, + extra_stuff: 'lorem ipsum', + }, + ]); + } + 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/), + { foo_id: 2, name: 'Burger King', rating: 3 }, + expect.toBeError(/yikes/), + { foo_id: 4, rating: 3.5 }, + { foo_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', + }, + }, + }; + + const resources = { + foo: ({ foo_ids, bar }) => { + if (_.isEqual(foo_ids, [1, 2, 3])) { + 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])) { + 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'], bar: true }, + { foo_id: 2, properties: ['name'], bar: true }, + { foo_id: 3, properties: ['rating'], bar: true }, + { foo_id: 4, properties: ['rating'], bar: false }, + ]); + + expect(results).toEqual([ + { foo_id: 1, rating: 3, name: 'Burger King' }, + expect.toBeError( + 'Could not find newKey = "2" and propertyNewKey = "name" in the response dict. Or your endpoint does not follow the contract we support.', + 'BatchItemNotFoundError', + ), + { foo_id: 3, rating: 4 }, + { foo_id: 4, rating: 5 }, + ]); + }); +}); diff --git a/examples/swapi/swapi.js b/examples/swapi/swapi.js index 7a4568e..0b7c9d4 100644 --- a/examples/swapi/swapi.js +++ b/examples/swapi/swapi.js @@ -63,10 +63,10 @@ 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, + title: ?string, + episode_id: ?number, + director: ?string, + producer: ?string, |}>, |}>; diff --git a/src/implementation.ts b/src/implementation.ts index e36ac07..8d19a94 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('.'); diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index 6cd3d82..3a8334f 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -380,7 +380,7 @@ export function unPartitionResultsByBatchKeyPartition ({ From 2231852c53396322a594c7ed308b1b1467c1aa62 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Thu, 1 Jul 2021 14:08:05 -0700 Subject: [PATCH 03/27] change BatchItemNotFoundError --- __tests__/implementation.test.js | 4 ++-- src/runtimeHelpers.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 1a6d3a8..6e112da 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1326,7 +1326,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex expect(results).toEqual([ { foo_id: 1, name: 'Shake Shack', rating: 4 }, expect.toBeError( - 'Could not find newKey = "2" and propertyNewKey = "rating" in the response dict. Or your endpoint does not follow the contract we support.', + 'Could not find newKey = "2" in the response dict. Or your endpoint does not follow the contract we support.', 'BatchItemNotFoundError', ), { foo_id: 3, rating: 3 }, @@ -1439,7 +1439,7 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp expect(results).toEqual([ { foo_id: 1, rating: 3, name: 'Burger King' }, expect.toBeError( - 'Could not find newKey = "2" and propertyNewKey = "name" in the response dict. Or your endpoint does not follow the contract we support.', + 'Could not find newKey = "2" in the response dict. Or your endpoint does not follow the contract we support.', 'BatchItemNotFoundError', ), { foo_id: 3, rating: 4 }, diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index 3a8334f..cee4c96 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -396,7 +396,7 @@ export function unPartitionResultsByBatchKeyPartition Date: Fri, 2 Jul 2021 10:42:10 -0700 Subject: [PATCH 04/27] add documentations for this config and fix review issues --- API_DOCS.md | 2 +- README.md | 37 ++++++++++++++++++++++++++++++++ __tests__/implementation.test.js | 2 -- schema.json | 2 +- src/runtimeHelpers.ts | 7 +----- 5 files changed, 40 insertions(+), 10 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 25ea1b4..5b85bc9 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -126,7 +126,7 @@ 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 nested list of optional properties we want to fetch. (e.g. usually 'properties' or 'features'). | +| `propertyBatchKey` | (Optional) The argument to the resource that represents the optional properties we want to fetch. (e.g. usually 'properties' or 'features'). | ### `typings` diff --git a/README.md b/README.md index 9496a31..55b0e1b 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,43 @@ resources: newKey: user_id ``` +### Batch Resources with `properties` paramerters + +Instead of accepting just a list of users (`user_ids`), a batch resouce could accept both a list of users (`user_ids`) and a list of properties (`properties`): + +```js +const getUserInfo = (args: { user_ids: Array, properties: Array }): Promise> => + fetch('/userInfo', args); +``` + +To batch up calls to this resouce with different `properties` for different `user_ids`, we specify `propertyBatchKey` in the config to describe the "properties" argument. + +The config for our `getUserInfoV2` would look like this: + +```yaml +resources: + getUserInfo: + isBatchResource: true + batchKey: user_ids + newKey: user_id + propertyBatchKey: properties +``` + +**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): + +1. The resouce accept two `list` type parameters, like this: + +``` +ids: str[] (should be the batchKey) +properties: str[] (should be the propertyBatchKey) +``` + +2. `properties` are not returned in a nested object, but spread at the same level as the `id`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) +3. All `properties` should be optional in the response object. +4. The resouce must have a one-to-one correspondence between the input "properties" and the output "properties". + (i.e. if we request property "name", the response must have "name" in it, and no extra data assciated with it.) + See [the full docs](./API_DOCS.md) for more information on how to configure resources. ## Contributing diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 6e112da..33476c8 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1248,7 +1248,6 @@ test('batch endpoint (multiple requests) with propertyBatchKey', async () => { foo_id: 3, rating: 5, name: 'Shake Shack', - extra_stuff: 'lorem ipsum', }, ]); } @@ -1367,7 +1366,6 @@ test('batch endpoint (multiple requests) with propertyBatchKey error handling', foo_id: 5, name: 'Shake Shack', rating: 4, - extra_stuff: 'lorem ipsum', }, ]); } diff --git a/schema.json b/schema.json index 10083ff..1449357 100644 --- a/schema.json +++ b/schema.json @@ -100,7 +100,7 @@ }, "propertyBatchKey": { "type": "string", - "description": "(Optional) The argument to the resource that represents the nested list of optional properties we want to fetch. (e.g. usually 'properties' or 'features')" + "description": "(Optional) The argument to the resource that represents the optional properties we want to fetch. (e.g. usually 'properties' or 'features')" } } }, diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index cee4c96..f1dddba 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -381,12 +381,7 @@ export function unPartitionResultsByBatchKeyPartition ({ - [key]: resultElement[key], - })), - ); + result = _.pick(resultElement, [newKey, ...propertyBatchKeyPartion[i][j]]); break; } } From 2d15a1151185de3f612b07c8a5eeef1099adfebf Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 20 Jul 2021 17:27:25 -0700 Subject: [PATCH 05/27] add swagger validation --- examples/swapi/swapi.dataloader-config.yaml | 5 ++ package.json | 1 + schema.json | 16 +++++ src/index.ts | 75 ++++++++++++++++++++- yarn.lock | 69 ++++++++++++++++++- 5 files changed, 163 insertions(+), 3 deletions(-) diff --git a/examples/swapi/swapi.dataloader-config.yaml b/examples/swapi/swapi.dataloader-config.yaml index af98605..3f80a51 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -35,6 +35,11 @@ resources: newKey: film_id nestedPath: properties propertyBatchKey: properties + swaggerFile: https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM + swaggerPath: /v2/business + swaggerMethod: get + swaggerRequiredId: + - id getRoot: docsLink: https://swapi.dev/documentation#root isBatchResource: false diff --git a/package.json b/package.json index 0c234a0..eab56d6 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "typescript": "^3.7.0-beta" }, "dependencies": { + "@apidevtools/swagger-parser": "^10.0.2", "@babel/preset-env": "^7.6.3", "aggregate-error": "^3.0.1", "ajv": "^6.11.0", diff --git a/schema.json b/schema.json index 1449357..0e6101f 100644 --- a/schema.json +++ b/schema.json @@ -101,6 +101,22 @@ "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')" + }, + "swaggerFile": { + "type": "string", + "description": "(Optional) " + }, + "swaggerPath": { + "type": "string", + "description": " " + }, + "swaggerMethod": { + "type": "string", + "description": "(Optional) " + }, + "swaggerRequiredId": { + "type": "array", + "description": "(Optional) " } } }, diff --git a/src/index.ts b/src/index.ts index 2f46bca..bb0d952 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,6 +6,7 @@ import path from 'path'; import yargs from 'yargs'; import codegen from './codegen'; import { getConfig } from './config'; +import SwaggerParser from '@apidevtools/swagger-parser'; interface CLIArgs { config: string; @@ -21,6 +22,76 @@ function writeLoaders(args: CLIArgs) { fs.writeFileSync(args.output, output); } +// return if two arrays have exact same values (order doesn't matter) +function arraysEqual(a: Array, b: Array) { + if (a === b) return true; + if (a == null || b == null) return false; + if (a.length !== b.length) return false; + a.sort(); + b.sort(); + for (var i = 0; i < a.length; ++i) { + if (a[i] !== b[i]) return false; + } + return true; +} + +// return all values of the key in object +function findVal(object: any, key: string, array: Array) { + Object.keys(object).some(function (k) { + if (k === key) { + array.push(...object[k]); + } + if (object[k] && typeof object[k] === 'object') { + findVal(object[k], key, array); + } else { + return; + } + }); + return array; +} + +function verifyBatchPropertyResource(args: CLIArgs) { + const resources: object = getConfig(args.config).resources; + + Object.entries(resources).forEach(([key, value]) => { + if (Object.keys(value).includes('propertyBatchKey')) { + const propertyBatchKey = value['propertyBatchKey']; + if ( + !value.hasOwnProperty('swaggerFile') || + !value.hasOwnProperty('swaggerPath') || + !value.hasOwnProperty('swaggerMethod') + ) { + throw new Error('Missing swagger info, please add them in yout dataloader-cofig.yaml file!'); + } + const swaggerFile = value['swaggerFile']; + const swaggerPath = value['swaggerPath']; + const swaggerMethod = value['swaggerMethod']; + + // parse swagger file, so that all $ref pointers will be resolved. + SwaggerParser.validate(swaggerFile, (err, api) => { + if (err) { + console.error(err); + } else { + var object = api.paths[swaggerPath][swaggerMethod]['responses']['200']; + // The resource may return the list of results in a nested path, we need to handle that + if (value.hasOwnProperty('nestedPath')) { + object = + api.paths[swaggerPath][swaggerMethod]['responses']['200']['schema'][value['nestedPath']]; + } + // Find all the fields that are listed `required` in the swagger spec + const requiredList = findVal(object, 'required', []); + const requiredId = value.hasOwnProperty('swaggerRequiredId') ? value['swaggerRequiredId'] : []; + if (!arraysEqual(requiredId, requiredList)) { + throw new Error( + 'Sorry, your endpoint does not match the requirement for using propertyBatchKey, please read.', + ); + } + } + }); + } + }); +} + function main() { const argv = yargs .options({ @@ -36,8 +107,8 @@ function main() { }, }) .help().argv; - - writeLoaders(argv); + verifyBatchPropertyResource(argv); + // writeLoaders(argv); } if (!process.env.NODE_ENV) { diff --git a/yarn.lock b/yarn.lock index 9d0d22f..a2b373a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,6 +2,37 @@ # yarn lockfile v1 +"@apidevtools/json-schema-ref-parser@^9.0.6": + version "9.0.7" + resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fjson-schema-ref-parser@9.0.7/@apidevtools-json-schema-ref-parser-9.0.7.64aa7f5b34e43d74ea9e408b90ddfba02050dde3.tgz#64aa7f5b34e43d74ea9e408b90ddfba02050dde3" + integrity sha1-ZKp/WzTkPXTqnkCLkN37oCBQ3eM= + dependencies: + "@jsdevtools/ono" "^7.1.3" + call-me-maybe "^1.0.1" + js-yaml "^3.13.1" + +"@apidevtools/openapi-schemas@^2.0.4": + version "2.1.0" + resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fopenapi-schemas@2.1.0/@apidevtools-openapi-schemas-2.1.0.9fa08017fb59d80538812f03fc7cac5992caaa17.tgz#9fa08017fb59d80538812f03fc7cac5992caaa17" + integrity sha1-n6CAF/tZ2AU4gS8D/HysWZLKqhc= + +"@apidevtools/swagger-methods@^3.0.2": + version "3.0.2" + resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fswagger-methods@3.0.2/@apidevtools-swagger-methods-3.0.2.b789a362e055b0340d04712eafe7027ddc1ac267.tgz#b789a362e055b0340d04712eafe7027ddc1ac267" + integrity sha1-t4mjYuBVsDQNBHEur+cCfdwawmc= + +"@apidevtools/swagger-parser@^10.0.2": + version "10.0.2" + resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fswagger-parser@10.0.2/@apidevtools-swagger-parser-10.0.2.f4145afb7c3a3bafe0376f003b5c3bdeae17a952.tgz#f4145afb7c3a3bafe0376f003b5c3bdeae17a952" + integrity sha1-9BRa+3w6O6/gN28AO1w73q4XqVI= + dependencies: + "@apidevtools/json-schema-ref-parser" "^9.0.6" + "@apidevtools/openapi-schemas" "^2.0.4" + "@apidevtools/swagger-methods" "^3.0.2" + "@jsdevtools/ono" "^7.1.3" + call-me-maybe "^1.0.1" + z-schema "^4.2.3" + "@babel/cli@^7.6.3": version "7.6.3" resolved "https://registry.yarnpkg.com/@babel/cli/-/cli-7.6.3.tgz#1b0c62098c8a5e01e4a4a59a52cba9682e7e0906" @@ -1044,6 +1075,11 @@ "@types/yargs" "^15.0.0" chalk "^3.0.0" +"@jsdevtools/ono@^7.1.3": + version "7.1.3" + resolved "https://npm.yelpcorp.com/_tarballs/%40jsdevtools%252Fono@7.1.3/@jsdevtools-ono-7.1.3.9df03bbd7c696a5c58885c34aa06da41c8543796.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" + integrity sha1-nfA7vXxpalxYiFw0qgbaQchUN5Y= + "@sinonjs/commons@^1.7.0": version "1.7.0" resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.7.0.tgz#f90ffc52a2e519f018b13b6c4da03cbff36ebed6" @@ -1535,6 +1571,11 @@ cache-base@^1.0.1: union-value "^1.0.0" unset-value "^1.0.0" +call-me-maybe@^1.0.1: + version "1.0.1" + resolved "https://npm.yelpcorp.com/_tarballs/call-me-maybe@1.0.1/call-me-maybe-1.0.1.26d208ea89e37b5cbde60250a15f031c16a4d66b.tgz#26d208ea89e37b5cbde60250a15f031c16a4d66b" + integrity sha1-JtII6onje1y95gJQoV8DHBak1ms= + callsites@^3.0.0: version "3.1.0" resolved "https://registry.yarnpkg.com/callsites/-/callsites-3.1.0.tgz#b3630abd8943432f54b3f0519238e33cd7df2f73" @@ -1695,7 +1736,7 @@ combined-stream@^1.0.6, combined-stream@~1.0.6: dependencies: delayed-stream "~1.0.0" -commander@^2.11.0: +commander@^2.11.0, commander@^2.7.1: version "2.20.3" resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33" integrity sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ== @@ -3282,6 +3323,16 @@ locate-path@^5.0.0: dependencies: p-locate "^4.1.0" +lodash.get@^4.4.2: + version "4.4.2" + resolved "https://npm.yelpcorp.com/_tarballs/lodash.get@4.4.2/lodash.get-4.4.2.2d177f652fa31e939b4438d5341499dfa3825e99.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" + integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk= + +lodash.isequal@^4.5.0: + version "4.5.0" + resolved "https://npm.yelpcorp.com/_tarballs/lodash.isequal@4.5.0/lodash.isequal-4.5.0.415c4478f2bcc30120c22ce10ed3226f7d3e18e0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" + integrity sha1-QVxEePK8wwEgwizhDtMib30+GOA= + lodash.sortby@^4.7.0: version "4.7.0" resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438" @@ -4777,6 +4828,11 @@ v8-to-istanbul@^4.0.1: convert-source-map "^1.6.0" source-map "^0.7.3" +validator@^12.0.0: + version "12.2.0" + resolved "https://npm.yelpcorp.com/_tarballs/validator@12.2.0/validator-12.2.0.660d47e96267033fd070096c3b1a6f2db4380a0a.tgz#660d47e96267033fd070096c3b1a6f2db4380a0a" + integrity sha1-Zg1H6WJnAz/QcAlsOxpvLbQ4Cgo= + verror@1.10.0: version "1.10.0" resolved "https://registry.yarnpkg.com/verror/-/verror-1.10.0.tgz#3a105ca17053af55d6e270c1f8288682e18da400" @@ -4973,3 +5029,14 @@ yargs@^15.0.0: which-module "^2.0.0" y18n "^4.0.0" yargs-parser "^16.1.0" + +z-schema@^4.2.3: + version "4.2.3" + resolved "https://npm.yelpcorp.com/_tarballs/z-schema@4.2.3/z-schema-4.2.3.85f7eea7e6d4fe59a483462a98f511bd78fe9882.tgz#85f7eea7e6d4fe59a483462a98f511bd78fe9882" + integrity sha1-hffup+bU/lmkg0YqmPURvXj+mII= + dependencies: + lodash.get "^4.4.2" + lodash.isequal "^4.5.0" + validator "^12.0.0" + optionalDependencies: + commander "^2.7.1" From 0d71c98cb3dd12b849b84f9e54b6d0b454d9ef80 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 13:16:38 -0700 Subject: [PATCH 06/27] add documentations for config --- examples/swapi/swapi.dataloader-config.yaml | 6 ++-- schema.json | 14 ++++---- src/index.ts | 39 ++++++++++++--------- 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/examples/swapi/swapi.dataloader-config.yaml b/examples/swapi/swapi.dataloader-config.yaml index 3f80a51..8f0ff5a 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -35,10 +35,10 @@ resources: newKey: film_id nestedPath: properties propertyBatchKey: properties - swaggerFile: https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM + swaggerLink: https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM swaggerPath: /v2/business - swaggerMethod: get - swaggerRequiredId: + httpMethod: get + swaggeRequiredKeys: - id getRoot: docsLink: https://swapi.dev/documentation#root diff --git a/schema.json b/schema.json index 0e6101f..e5a2a9f 100644 --- a/schema.json +++ b/schema.json @@ -102,21 +102,21 @@ "type": "string", "description": "(Optional) The argument to the resource that represents the optional properties we want to fetch. (e.g. usually 'properties' or 'features')" }, - "swaggerFile": { + "swaggerLink": { "type": "string", - "description": "(Optional) " + "description": "(Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces. " }, "swaggerPath": { "type": "string", - "description": " " + "description": "(Non-optional when propertyBatchKey is set)) The global paths section of the API specification. e.g. /users/v1" }, - "swaggerMethod": { + "httpMethod": { "type": "string", - "description": "(Optional) " + "description": "(Non-optional when propertyBatchKey is set)) The http method of the API specification. e.g. get, post, delete, etc." }, - "swaggerRequiredId": { + "swaggeRequiredKeys": { "type": "array", - "description": "(Optional) " + "description": "(Optional) The key in the response objects corresponds to an element in `batchKey`. This should be the only field that are marked as required in your swagger endpoint response, except nestedPath" } } }, diff --git a/src/index.ts b/src/index.ts index bb0d952..29f6673 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,7 +22,9 @@ function writeLoaders(args: CLIArgs) { fs.writeFileSync(args.output, output); } -// return if two arrays have exact same values (order doesn't matter) +/** + * If two arrays have the same items (order doesn't matter) + */ function arraysEqual(a: Array, b: Array) { if (a === b) return true; if (a == null || b == null) return false; @@ -35,7 +37,9 @@ function arraysEqual(a: Array, b: Array) { return true; } -// return all values of the key in object +/** + * Find all values of the intput key in an object recursively + */ function findVal(object: any, key: string, array: Array) { Object.keys(object).some(function (k) { if (k === key) { @@ -50,6 +54,9 @@ function findVal(object: any, key: string, array: Array) { return array; } +/** + * Throw erros when resource uses propertyBatchKey feature but doesn't have optional properties + */ function verifyBatchPropertyResource(args: CLIArgs) { const resources: object = getConfig(args.config).resources; @@ -57,33 +64,33 @@ function verifyBatchPropertyResource(args: CLIArgs) { if (Object.keys(value).includes('propertyBatchKey')) { const propertyBatchKey = value['propertyBatchKey']; if ( - !value.hasOwnProperty('swaggerFile') || + !value.hasOwnProperty('swaggerLink') || !value.hasOwnProperty('swaggerPath') || - !value.hasOwnProperty('swaggerMethod') + !value.hasOwnProperty('httpMethod') ) { - throw new Error('Missing swagger info, please add them in yout dataloader-cofig.yaml file!'); + throw new Error(`Missing swagger info for ${key}, please add them in the dataloader-cofig.yaml file!`); } - const swaggerFile = value['swaggerFile']; + const swaggerLink = value['swaggerLink']; const swaggerPath = value['swaggerPath']; - const swaggerMethod = value['swaggerMethod']; + const httpMethod = value['httpMethod']; // parse swagger file, so that all $ref pointers will be resolved. - SwaggerParser.validate(swaggerFile, (err, api) => { + SwaggerParser.validate(swaggerLink, (err, api) => { if (err) { console.error(err); } else { - var object = api.paths[swaggerPath][swaggerMethod]['responses']['200']; - // The resource may return the list of results in a nested path, we need to handle that + var object = api.paths[swaggerPath][httpMethod]['responses']['200']; + // The resource may return the list of results in a nested path and properties are under the nestedPath if (value.hasOwnProperty('nestedPath')) { - object = - api.paths[swaggerPath][swaggerMethod]['responses']['200']['schema'][value['nestedPath']]; + object = api.paths[swaggerPath][httpMethod]['responses']['200']['schema'][value['nestedPath']]; } - // Find all the fields that are listed `required` in the swagger spec + // Find all the fields that are `required` in the swagger spec const requiredList = findVal(object, 'required', []); - const requiredId = value.hasOwnProperty('swaggerRequiredId') ? value['swaggerRequiredId'] : []; - if (!arraysEqual(requiredId, requiredList)) { + const requiredKeys = value.hasOwnProperty('swaggeRequiredKeys') ? value['swaggeRequiredKeys'] : []; + if (!arraysEqual(requiredKeys, requiredList)) { throw new Error( - 'Sorry, your endpoint does not match the requirement for using propertyBatchKey, please read.', + 'Sorry, your endpoint does not match the requirement for using propertyBatchKey ' + + ', please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', ); } } From 89bfeb34a4258719d1e90f2ef6696875b9f8d449 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 13:41:31 -0700 Subject: [PATCH 07/27] add documentations for config --- package.json | 1 + src/index.ts | 33 +++++++++++++++++++++------------ yarn.lock | 5 +++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index eab56d6..2593ec3 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "js-yaml": "^3.13.1", "lodash": "^4.17.15", "object-hash": "^2.0.0", + "openapi-types": "^1.3.5", "prettier": "^1.19.1", "yargs": "^14.2.0" }, diff --git a/src/index.ts b/src/index.ts index 29f6673..4304285 100644 --- a/src/index.ts +++ b/src/index.ts @@ -79,19 +79,28 @@ function verifyBatchPropertyResource(args: CLIArgs) { if (err) { console.error(err); } else { - var object = api.paths[swaggerPath][httpMethod]['responses']['200']; - // The resource may return the list of results in a nested path and properties are under the nestedPath - if (value.hasOwnProperty('nestedPath')) { - object = api.paths[swaggerPath][httpMethod]['responses']['200']['schema'][value['nestedPath']]; - } - // Find all the fields that are `required` in the swagger spec - const requiredList = findVal(object, 'required', []); - const requiredKeys = value.hasOwnProperty('swaggeRequiredKeys') ? value['swaggeRequiredKeys'] : []; - if (!arraysEqual(requiredKeys, requiredList)) { + if (typeof api !== 'object' || typeof api.paths[swaggerPath][httpMethod] !== 'object') { throw new Error( - 'Sorry, your endpoint does not match the requirement for using propertyBatchKey ' + - ', please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', + `Cannot find the swagger response definition for ${key}, please make sure you have correct swagger info in the dataloader-cofig.yaml file!`, ); + } else { + var swaggerResponse: any = api.paths[swaggerPath][httpMethod]['responses']['200']; + // The resource may return the list of results in a nested path and properties are under the nestedPath + if (value.hasOwnProperty('nestedPath')) { + swaggerResponse = + api.paths[swaggerPath][httpMethod]['responses']['200']['schema'][value['nestedPath']]; + } + // Find all the fields that are `required` in the swagger spec + const requiredList = findVal(swaggerResponse, 'required', []); + const requiredKeys = value.hasOwnProperty('swaggeRequiredKeys') + ? value['swaggeRequiredKeys'] + : []; + if (!arraysEqual(requiredKeys, requiredList)) { + throw new Error( + 'Sorry, your swagger endpoint does not match the requirement of using propertyBatchKey ' + + ', please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', + ); + } } } }); @@ -115,7 +124,7 @@ function main() { }) .help().argv; verifyBatchPropertyResource(argv); - // writeLoaders(argv); + writeLoaders(argv); } if (!process.env.NODE_ENV) { diff --git a/yarn.lock b/yarn.lock index a2b373a..e9a9aa9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3721,6 +3721,11 @@ onetime@^5.1.0: dependencies: mimic-fn "^2.1.0" +openapi-types@^1.3.5: + version "1.3.5" + resolved "https://npm.yelpcorp.com/_tarballs/openapi-types@1.3.5/openapi-types-1.3.5.6718cfbc857fe6c6f1471f65b32bdebb9c10ce40.tgz#6718cfbc857fe6c6f1471f65b32bdebb9c10ce40" + integrity sha1-ZxjPvIV/5sbxRx9lsyveu5wQzkA= + optionator@^0.8.1: version "0.8.2" resolved "https://registry.yarnpkg.com/optionator/-/optionator-0.8.2.tgz#364c5e409d3f4d6301d6c0b4c05bba50180aeb64" From a0c5018a0a09fd216c754b0b06c8ce909f6d2d2d Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 14:41:06 -0700 Subject: [PATCH 08/27] add responseKey as the key in the response objects corresponds to --- API_DOCS.md | 5 ++++ README.md | 4 ++- __tests__/implementation.test.js | 32 ++++++++++++--------- examples/swapi/swapi.dataloader-config.yaml | 3 +- schema.json | 14 ++++----- src/config.ts | 1 + src/implementation.ts | 1 + src/runtimeHelpers.ts | 18 ++++++------ 8 files changed, 46 insertions(+), 32 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 5b85bc9..b145b1e 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -127,6 +127,11 @@ Describes the shape and behaviour of the resources object you will pass to `getL | `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` | (Optional) 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. | +| `swaggerLink` | (Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces. | +| `swaggerPath` | (Non-optional when propertyBatchKey is set)) The global paths section of the API specification. e.g. /users/v1. | +| `httpMethod` | (Non-optional when propertyBatchKey is set)) The http method of the API specification. e.g. get, post, delete, etc. | +| | ### `typings` diff --git a/README.md b/README.md index 55b0e1b..d43da99 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,7 @@ const getUserInfo = (args: { user_ids: Array, properties: Array ``` To batch up calls to this resouce 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: @@ -167,6 +168,7 @@ resources: batchKey: user_ids newKey: user_id propertyBatchKey: properties + responseKey: id ``` **IMPORTANT NOTE** @@ -180,7 +182,7 @@ properties: str[] (should be the propertyBatchKey) ``` 2. `properties` are not returned in a nested object, but spread at the same level as the `id`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) -3. All `properties` should be optional in the response object. +3. All `properties` should be optional in the response object. To enfore that, we require `swaggerLink`, `swaggerPath` and `httpMethod` defined in the config. 4. The resouce must have a one-to-one correspondence between the input "properties" and the output "properties". (i.e. if we request property "name", the response must have "name" in it, and no extra data assciated with it.) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 33476c8..bf91275 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1227,6 +1227,7 @@ test('batch endpoint (multiple requests) with propertyBatchKey', async () => { batchKey: 'foo_ids', newKey: 'foo_id', propertyBatchKey: 'properties', + responseKey: 'id', }, }, }; @@ -1236,8 +1237,8 @@ test('batch endpoint (multiple requests) with propertyBatchKey', async () => { if (_.isEqual(foo_ids, [2, 1])) { expect(include_extra_info).toBe(false); return Promise.resolve([ - { foo_id: 1, rating: 3, name: 'Burger King' }, - { foo_id: 2, rating: 4, name: 'In N Out' }, + { id: 1, rating: 3, name: 'Burger King' }, + { id: 2, rating: 4, name: 'In N Out' }, ]); } @@ -1245,7 +1246,7 @@ test('batch endpoint (multiple requests) with propertyBatchKey', async () => { expect(include_extra_info).toBe(true); return Promise.resolve([ { - foo_id: 3, + id: 3, rating: 5, name: 'Shake Shack', }, @@ -1264,9 +1265,9 @@ test('batch endpoint (multiple requests) with propertyBatchKey', async () => { ]); expect(results).toEqual([ - { foo_id: 2, name: 'In N Out', rating: 4 }, - { foo_id: 1, rating: 3 }, - { foo_id: 3, rating: 5 }, + { id: 2, name: 'In N Out', rating: 4 }, + { id: 1, rating: 3 }, + { id: 3, rating: 5 }, ]); }); }); @@ -1280,6 +1281,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex batchKey: 'foo_ids', newKey: 'foo_id', propertyBatchKey: 'properties', + responseKey: 'foo_id', }, }, }; @@ -1325,7 +1327,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex expect(results).toEqual([ { foo_id: 1, name: 'Shake Shack', rating: 4 }, expect.toBeError( - 'Could not find newKey = "2" in the response dict. Or your endpoint does not follow the contract we support.', + 'Could not find foo_id = 2 in the response dict. Or your endpoint does not follow the contract we support.', 'BatchItemNotFoundError', ), { foo_id: 3, rating: 3 }, @@ -1343,6 +1345,7 @@ test('batch endpoint (multiple requests) with propertyBatchKey error handling', batchKey: 'foo_ids', newKey: 'foo_id', propertyBatchKey: 'properties', + responseKey: 'id', }, }, }; @@ -1353,17 +1356,17 @@ test('batch endpoint (multiple requests) with propertyBatchKey error handling', expect(include_extra_info).toBe(true); return Promise.resolve([ { - foo_id: 2, + id: 2, name: 'Burger King', rating: 3, }, { - foo_id: 4, + id: 4, name: 'In N Out', rating: 3.5, }, { - foo_id: 5, + id: 5, name: 'Shake Shack', rating: 4, }, @@ -1389,10 +1392,10 @@ test('batch endpoint (multiple requests) with propertyBatchKey error handling', expect(results).toEqual([ expect.toBeError(/yikes/), - { foo_id: 2, name: 'Burger King', rating: 3 }, + { id: 2, name: 'Burger King', rating: 3 }, expect.toBeError(/yikes/), - { foo_id: 4, rating: 3.5 }, - { foo_id: 5, name: 'Shake Shack' }, + { id: 4, rating: 3.5 }, + { id: 5, name: 'Shake Shack' }, ]); }); }); @@ -1407,6 +1410,7 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp newKey: 'foo_id', reorderResultsByKey: 'foo_id', propertyBatchKey: 'properties', + responseKey: 'foo_id', }, }, }; @@ -1437,7 +1441,7 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp expect(results).toEqual([ { foo_id: 1, rating: 3, name: 'Burger King' }, expect.toBeError( - 'Could not find newKey = "2" in the response dict. Or your endpoint does not follow the contract we support.', + 'Could not find foo_id = 2 in the response dict. Or your endpoint does not follow the contract we support.', 'BatchItemNotFoundError', ), { foo_id: 3, rating: 4 }, diff --git a/examples/swapi/swapi.dataloader-config.yaml b/examples/swapi/swapi.dataloader-config.yaml index 8f0ff5a..abf2830 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -35,11 +35,10 @@ resources: newKey: film_id nestedPath: properties propertyBatchKey: properties + responseKey: id swaggerLink: https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM swaggerPath: /v2/business httpMethod: get - swaggeRequiredKeys: - - id getRoot: docsLink: https://swapi.dev/documentation#root isBatchResource: false diff --git a/schema.json b/schema.json index e5a2a9f..55f4a7c 100644 --- a/schema.json +++ b/schema.json @@ -102,21 +102,21 @@ "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 set) 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." + }, "swaggerLink": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces. " + "description": "(Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces." }, "swaggerPath": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set)) The global paths section of the API specification. e.g. /users/v1" + "description": "(Non-optional when propertyBatchKey is set) The global paths section of the API specification. e.g. /users/v1." }, "httpMethod": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set)) The http method of the API specification. e.g. get, post, delete, etc." - }, - "swaggeRequiredKeys": { - "type": "array", - "description": "(Optional) The key in the response objects corresponds to an element in `batchKey`. This should be the only field that are marked as required in your swagger endpoint response, except nestedPath" + "description": "(Non-optional when propertyBatchKey is set) The http method of the API specification. e.g. get, post, delete, etc." } } }, diff --git a/src/config.ts b/src/config.ts index d17284c..018e2ef 100644 --- a/src/config.ts +++ b/src/config.ts @@ -21,6 +21,7 @@ export interface BatchResourceConfig { batchKey: string; newKey: string; propertyBatchKey: string; + responseKey: string; reorderResultsByKey?: string; nestedPath?: string; commaSeparatedBatchKey?: boolean; diff --git a/src/implementation.ts b/src/implementation.ts index 8d19a94..47836dc 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -440,6 +440,7 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado return unPartitionResultsByBatchKeyPartition( '${resourceConfig.newKey}', '${resourceConfig.propertyBatchKey}', + '${resourceConfig.responseKey}', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index f1dddba..ea07262 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -307,8 +307,8 @@ export function unPartitionResults( * If we have 'bar_id' as newKey and 'properties' as propertyBatchKey, * the resultGroups should look like this: * [ - * [ { bar_id: 2, name: 'Burger King', rating: 3 } ], - * [ { bar_id: 1, name: 'In N Out', rating: 4 } ] + * [ { id: 2, name: 'Burger King', rating: 3 } ], + * [ { id: 1, name: 'In N Out', rating: 4 } ] * ], * * @@ -327,6 +327,7 @@ export function unPartitionResults( * unPartitionResultsByBatchKeyPartition( * newKey = 'bar_id', * propertyBatchKey = 'properties', + * responseKey = 'id' * batchKeyPartition = [ [2, 2], [1] ], * propertyBatchKeyPartion = [ [['name'], ['rating']], [['rating']] ], * requestGroups = [ [0, 2], [1] ], @@ -340,14 +341,15 @@ export function unPartitionResults( * Returns: * ``` * [ - * { bar_id: 2, name: 'Burger King' }， - * { bar_id: 1, rating: 4 }， - * { bar_id: 2, rating: 3 }， + * { 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 */ @@ -380,8 +382,8 @@ export function unPartitionResultsByBatchKeyPartition Date: Wed, 21 Jul 2021 14:50:52 -0700 Subject: [PATCH 09/27] add responseKey as the key in the response objects corresponds to --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 4304285..58a4429 100644 --- a/src/index.ts +++ b/src/index.ts @@ -93,7 +93,7 @@ function verifyBatchPropertyResource(args: CLIArgs) { // Find all the fields that are `required` in the swagger spec const requiredList = findVal(swaggerResponse, 'required', []); const requiredKeys = value.hasOwnProperty('swaggeRequiredKeys') - ? value['swaggeRequiredKeys'] + ? [value['swaggeRequiredKeys']] : []; if (!arraysEqual(requiredKeys, requiredList)) { throw new Error( From 10dab843c4247f7a87ef1fa91f04eaa15b092e52 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 15:12:48 -0700 Subject: [PATCH 10/27] change some helper function and docstring --- README.md | 4 ++-- src/index.ts | 27 ++++++++++++--------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d43da99..dc489ae 100644 --- a/README.md +++ b/README.md @@ -174,14 +174,14 @@ resources: **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): -1. The resouce accept two `list` type parameters, like this: +1. The resouce accept two non-optional `list` type parameters, like this: ``` ids: str[] (should be the batchKey) properties: str[] (should be the propertyBatchKey) ``` -2. `properties` are not returned in a nested object, but spread at the same level as the `id`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) +2. `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) 3. All `properties` should be optional in the response object. To enfore that, we require `swaggerLink`, `swaggerPath` and `httpMethod` defined in the config. 4. The resouce must have a one-to-one correspondence between the input "properties" and the output "properties". (i.e. if we request property "name", the response must have "name" in it, and no extra data assciated with it.) diff --git a/src/index.ts b/src/index.ts index 58a4429..c4e3b2d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,16 +23,12 @@ function writeLoaders(args: CLIArgs) { } /** - * If two arrays have the same items (order doesn't matter) + * If requiredList only contains responseKey */ -function arraysEqual(a: Array, b: Array) { - if (a === b) return true; - if (a == null || b == null) return false; - if (a.length !== b.length) return false; - a.sort(); - b.sort(); - for (var i = 0; i < a.length; ++i) { - if (a[i] !== b[i]) return false; +function validRequiredList(requiredList: Array, responseKey: string) { + if (requiredList.length > 1) return false; + if (requiredList.length == 1 && requiredList[0] !== responseKey) { + return false; } return true; } @@ -64,6 +60,7 @@ function verifyBatchPropertyResource(args: CLIArgs) { if (Object.keys(value).includes('propertyBatchKey')) { const propertyBatchKey = value['propertyBatchKey']; if ( + !value.hasOwnProperty('responseKey') || !value.hasOwnProperty('swaggerLink') || !value.hasOwnProperty('swaggerPath') || !value.hasOwnProperty('httpMethod') @@ -92,13 +89,13 @@ function verifyBatchPropertyResource(args: CLIArgs) { } // Find all the fields that are `required` in the swagger spec const requiredList = findVal(swaggerResponse, 'required', []); - const requiredKeys = value.hasOwnProperty('swaggeRequiredKeys') - ? [value['swaggeRequiredKeys']] - : []; - if (!arraysEqual(requiredKeys, requiredList)) { + const responseKey = value['responseKey']; + if (!validRequiredList(requiredList, responseKey)) { throw new Error( - 'Sorry, your swagger endpoint does not match the requirement of using propertyBatchKey ' + - ', please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', + [ + 'Sorry, your swagger endpoint does not match the requirement of using propertyBatchKey,', + 'please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', + ].join(' '), ); } } From 4c8c3753dd503b594846618e2451cd2d4985b2fe Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 16:46:17 -0700 Subject: [PATCH 11/27] use public registry not yelp one --- src/index.ts | 8 +++---- yarn.lock | 60 ++++++++++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/index.ts b/src/index.ts index c4e3b2d..ddfe47a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -67,16 +67,16 @@ function verifyBatchPropertyResource(args: CLIArgs) { ) { throw new Error(`Missing swagger info for ${key}, please add them in the dataloader-cofig.yaml file!`); } - const swaggerLink = value['swaggerLink']; - const swaggerPath = value['swaggerPath']; - const httpMethod = value['httpMethod']; + const swaggerLink: string = value['swaggerLink']; + const swaggerPath: string = value['swaggerPath']; + const httpMethod: string = value['httpMethod']; // parse swagger file, so that all $ref pointers will be resolved. SwaggerParser.validate(swaggerLink, (err, api) => { if (err) { console.error(err); } else { - if (typeof api !== 'object' || typeof api.paths[swaggerPath][httpMethod] !== 'object') { + if (!api || !api.paths[swaggerPath][httpMethod]) { throw new Error( `Cannot find the swagger response definition for ${key}, please make sure you have correct swagger info in the dataloader-cofig.yaml file!`, ); diff --git a/yarn.lock b/yarn.lock index e9a9aa9..ff4868e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3,28 +3,29 @@ "@apidevtools/json-schema-ref-parser@^9.0.6": - version "9.0.7" - resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fjson-schema-ref-parser@9.0.7/@apidevtools-json-schema-ref-parser-9.0.7.64aa7f5b34e43d74ea9e408b90ddfba02050dde3.tgz#64aa7f5b34e43d74ea9e408b90ddfba02050dde3" - integrity sha1-ZKp/WzTkPXTqnkCLkN37oCBQ3eM= + version "9.0.9" + resolved "https://registry.yarnpkg.com/@apidevtools/json-schema-ref-parser/-/json-schema-ref-parser-9.0.9.tgz#d720f9256e3609621280584f2b47ae165359268b" + integrity sha512-GBD2Le9w2+lVFoc4vswGI/TjkNIZSVp7+9xPf+X3uidBfWnAeUWmquteSyt0+VCrhNMWj/FTABISQrD3Z/YA+w== dependencies: "@jsdevtools/ono" "^7.1.3" + "@types/json-schema" "^7.0.6" call-me-maybe "^1.0.1" - js-yaml "^3.13.1" + js-yaml "^4.1.0" "@apidevtools/openapi-schemas@^2.0.4": version "2.1.0" - resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fopenapi-schemas@2.1.0/@apidevtools-openapi-schemas-2.1.0.9fa08017fb59d80538812f03fc7cac5992caaa17.tgz#9fa08017fb59d80538812f03fc7cac5992caaa17" - integrity sha1-n6CAF/tZ2AU4gS8D/HysWZLKqhc= + resolved "https://registry.yarnpkg.com/@apidevtools/openapi-schemas/-/openapi-schemas-2.1.0.tgz#9fa08017fb59d80538812f03fc7cac5992caaa17" + integrity sha512-Zc1AlqrJlX3SlpupFGpiLi2EbteyP7fXmUOGup6/DnkRgjP9bgMM/ag+n91rsv0U1Gpz0H3VILA/o3bW7Ua6BQ== "@apidevtools/swagger-methods@^3.0.2": version "3.0.2" - resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fswagger-methods@3.0.2/@apidevtools-swagger-methods-3.0.2.b789a362e055b0340d04712eafe7027ddc1ac267.tgz#b789a362e055b0340d04712eafe7027ddc1ac267" - integrity sha1-t4mjYuBVsDQNBHEur+cCfdwawmc= + resolved "https://registry.yarnpkg.com/@apidevtools/swagger-methods/-/swagger-methods-3.0.2.tgz#b789a362e055b0340d04712eafe7027ddc1ac267" + integrity sha512-QAkD5kK2b1WfjDS/UQn/qQkbwF31uqRjPTrsCs5ZG9BQGAkjwvqGFjjPqAuzac/IYzpPtRzjCP1WrTuAIjMrXg== "@apidevtools/swagger-parser@^10.0.2": version "10.0.2" - resolved "https://npm.yelpcorp.com/_tarballs/%40apidevtools%252Fswagger-parser@10.0.2/@apidevtools-swagger-parser-10.0.2.f4145afb7c3a3bafe0376f003b5c3bdeae17a952.tgz#f4145afb7c3a3bafe0376f003b5c3bdeae17a952" - integrity sha1-9BRa+3w6O6/gN28AO1w73q4XqVI= + resolved "https://registry.npmjs.org/@apidevtools/swagger-parser/-/swagger-parser-10.0.2.tgz#f4145afb7c3a3bafe0376f003b5c3bdeae17a952" + integrity sha512-JFxcEyp8RlNHgBCE98nwuTkZT6eNFPc1aosWV6wPcQph72TSEEu1k3baJD4/x1qznU+JiDdz8F5pTwabZh+Dhg== dependencies: "@apidevtools/json-schema-ref-parser" "^9.0.6" "@apidevtools/openapi-schemas" "^2.0.4" @@ -1077,8 +1078,8 @@ "@jsdevtools/ono@^7.1.3": version "7.1.3" - resolved "https://npm.yelpcorp.com/_tarballs/%40jsdevtools%252Fono@7.1.3/@jsdevtools-ono-7.1.3.9df03bbd7c696a5c58885c34aa06da41c8543796.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" - integrity sha1-nfA7vXxpalxYiFw0qgbaQchUN5Y= + resolved "https://registry.yarnpkg.com/@jsdevtools/ono/-/ono-7.1.3.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" + integrity sha512-4JQNk+3mVzK3xh2rqd6RB4J46qUR19azEHBneZyTZM+c456qOrbbM/5xcR8huNCCcbVt7+UmizG6GuUvPvKUYg== "@sinonjs/commons@^1.7.0": version "1.7.0" @@ -1158,6 +1159,11 @@ resolved "https://registry.yarnpkg.com/@types/js-yaml/-/js-yaml-3.12.1.tgz#5c6f4a1eabca84792fbd916f0cb40847f123c656" integrity sha512-SGGAhXLHDx+PK4YLNcNGa6goPf9XRWQNAUUbffkwVGGXIxmDKWyGGL4inzq2sPmExu431Ekb9aEMn9BkPqEYFA== +"@types/json-schema@^7.0.6": + version "7.0.8" + resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.8.tgz#edf1bf1dbf4e04413ca8e5b17b3b7d7d54b59818" + integrity sha512-YSBPTLTVm2e2OoQIDYx8HaeWJ5tTToLH67kXR7zYNGupXMEHa2++G8k+DczX2cFVgalypqtyZIcU19AFcmOpmg== + "@types/lodash@^4.14.144": version "4.14.144" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.144.tgz#12e57fc99064bce45e5ab3c8bc4783feb75eab8e" @@ -1343,6 +1349,11 @@ argparse@^1.0.7: dependencies: sprintf-js "~1.0.2" +argparse@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38" + integrity sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q== + arr-diff@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/arr-diff/-/arr-diff-4.0.0.tgz#d6461074febfec71e7e15235761a329a5dc7c520" @@ -1573,7 +1584,7 @@ cache-base@^1.0.1: call-me-maybe@^1.0.1: version "1.0.1" - resolved "https://npm.yelpcorp.com/_tarballs/call-me-maybe@1.0.1/call-me-maybe-1.0.1.26d208ea89e37b5cbde60250a15f031c16a4d66b.tgz#26d208ea89e37b5cbde60250a15f031c16a4d66b" + resolved "https://registry.yarnpkg.com/call-me-maybe/-/call-me-maybe-1.0.1.tgz#26d208ea89e37b5cbde60250a15f031c16a4d66b" integrity sha1-JtII6onje1y95gJQoV8DHBak1ms= callsites@^3.0.0: @@ -3187,6 +3198,13 @@ js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" +js-yaml@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.1.0.tgz#c1fb65f8f5017901cdd2c951864ba18458a10602" + integrity sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA== + dependencies: + argparse "^2.0.1" + jsbn@~0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/jsbn/-/jsbn-0.1.1.tgz#a5e654c2e5a2deb5f201d96cefbca80c0ef2f513" @@ -3325,12 +3343,12 @@ locate-path@^5.0.0: lodash.get@^4.4.2: version "4.4.2" - resolved "https://npm.yelpcorp.com/_tarballs/lodash.get@4.4.2/lodash.get-4.4.2.2d177f652fa31e939b4438d5341499dfa3825e99.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" + resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk= lodash.isequal@^4.5.0: version "4.5.0" - resolved "https://npm.yelpcorp.com/_tarballs/lodash.isequal@4.5.0/lodash.isequal-4.5.0.415c4478f2bcc30120c22ce10ed3226f7d3e18e0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" + resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" integrity sha1-QVxEePK8wwEgwizhDtMib30+GOA= lodash.sortby@^4.7.0: @@ -3723,8 +3741,8 @@ onetime@^5.1.0: openapi-types@^1.3.5: version "1.3.5" - resolved "https://npm.yelpcorp.com/_tarballs/openapi-types@1.3.5/openapi-types-1.3.5.6718cfbc857fe6c6f1471f65b32bdebb9c10ce40.tgz#6718cfbc857fe6c6f1471f65b32bdebb9c10ce40" - integrity sha1-ZxjPvIV/5sbxRx9lsyveu5wQzkA= + resolved "https://registry.npmjs.org/openapi-types/-/openapi-types-1.3.5.tgz#6718cfbc857fe6c6f1471f65b32bdebb9c10ce40" + integrity sha512-11oi4zYorsgvg5yBarZplAqbpev5HkuVNPlZaPTknPDzAynq+lnJdXAmruGWP0s+dNYZS7bjM+xrTpJw7184Fg== optionator@^0.8.1: version "0.8.2" @@ -4835,8 +4853,8 @@ v8-to-istanbul@^4.0.1: validator@^12.0.0: version "12.2.0" - resolved "https://npm.yelpcorp.com/_tarballs/validator@12.2.0/validator-12.2.0.660d47e96267033fd070096c3b1a6f2db4380a0a.tgz#660d47e96267033fd070096c3b1a6f2db4380a0a" - integrity sha1-Zg1H6WJnAz/QcAlsOxpvLbQ4Cgo= + resolved "https://registry.yarnpkg.com/validator/-/validator-12.2.0.tgz#660d47e96267033fd070096c3b1a6f2db4380a0a" + integrity sha512-jJfE/DW6tIK1Ek8nCfNFqt8Wb3nzMoAbocBF6/Icgg1ZFSBpObdnwVY2jQj6qUqzhx5jc71fpvBWyLGO7Xl+nQ== verror@1.10.0: version "1.10.0" @@ -5037,8 +5055,8 @@ yargs@^15.0.0: z-schema@^4.2.3: version "4.2.3" - resolved "https://npm.yelpcorp.com/_tarballs/z-schema@4.2.3/z-schema-4.2.3.85f7eea7e6d4fe59a483462a98f511bd78fe9882.tgz#85f7eea7e6d4fe59a483462a98f511bd78fe9882" - integrity sha1-hffup+bU/lmkg0YqmPURvXj+mII= + resolved "https://registry.yarnpkg.com/z-schema/-/z-schema-4.2.3.tgz#85f7eea7e6d4fe59a483462a98f511bd78fe9882" + integrity sha512-zkvK/9TC6p38IwcrbnT3ul9in1UX4cm1y/VZSs4GHKIiDCrlafc+YQBgQBUdDXLAoZHf2qvQ7gJJOo6yT1LH6A== dependencies: lodash.get "^4.4.2" lodash.isequal "^4.5.0" From 6c17d4f958c10f9554d33aa9911bb6aad2c64013 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 16:51:45 -0700 Subject: [PATCH 12/27] update docstring --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index ddfe47a..f0ab749 100644 --- a/src/index.ts +++ b/src/index.ts @@ -51,7 +51,7 @@ function findVal(object: any, key: string, array: Array) { } /** - * Throw erros when resource uses propertyBatchKey feature but doesn't have optional properties + * Throw erros when resource uses propertyBatchKey feature but doesn't match requirements */ function verifyBatchPropertyResource(args: CLIArgs) { const resources: object = getConfig(args.config).resources; From 192bb2c1b56d8a96fac9cf669bc578152b815401 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 17:03:02 -0700 Subject: [PATCH 13/27] update swapi example --- examples/swapi/swapi-loaders.js | 11 ++++++++++- examples/swapi/swapi.js | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index f8f47de..821a98a 100644 --- a/examples/swapi/swapi-loaders.js +++ b/examples/swapi/swapi-loaders.js @@ -529,6 +529,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade return unPartitionResultsByBatchKeyPartition( 'planet_id', 'undefined', + 'undefined', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, @@ -847,6 +848,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade return unPartitionResultsByBatchKeyPartition( 'person_id', 'undefined', + 'undefined', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, @@ -1168,6 +1170,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade return unPartitionResultsByBatchKeyPartition( 'vehicle_id', 'undefined', + 'undefined', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, @@ -1489,6 +1492,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade return unPartitionResultsByBatchKeyPartition( 'film_id', 'undefined', + 'undefined', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, @@ -1548,7 +1552,11 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * "batchKey": "film_ids", * "newKey": "film_id", * "nestedPath": "properties", - * "propertyBatchKey": "properties" + * "propertyBatchKey": "properties", + * "responseKey": "id", + * "swaggerLink": "https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM", + * "swaggerPath": "/v2/business", + * "httpMethod": "get" * } * ``` */ @@ -1819,6 +1827,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade return unPartitionResultsByBatchKeyPartition( 'film_id', 'properties', + 'id', batchKeyPartition, propertyBatchKeyPartiion, requestGroups, diff --git a/examples/swapi/swapi.js b/examples/swapi/swapi.js index 0b7c9d4..235bf2a 100644 --- a/examples/swapi/swapi.js +++ b/examples/swapi/swapi.js @@ -62,7 +62,7 @@ export type SWAPI_Film = $ReadOnly<{| export type SWAPI_Film_V2 = $ReadOnly<{| properties: $ReadOnlyArray<{| - film_id: number, + id: number, title: ?string, episode_id: ?number, director: ?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, From 60dbcc21500ae29ef3d72b53396fc8496c678ceb Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 21 Jul 2021 17:05:12 -0700 Subject: [PATCH 14/27] update API_DOCS --- API_DOCS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/API_DOCS.md b/API_DOCS.md index b145b1e..291d665 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -95,6 +95,10 @@ resources: 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 (must exist when propertyBatchKey is used) + swaggerLink: ?string (must exist when propertyBatchKey is used) + swaggerPath: ?string (must exist when propertyBatchKey is used) + httpMethod: ?string (must exist when propertyBatchKey is used) typings: language: flow From 678529c8eb0133105e5412e647ab45c44e2f8c25 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Thu, 22 Jul 2021 09:55:50 -0700 Subject: [PATCH 15/27] revise some documentation --- API_DOCS.md | 16 ++++++++-------- schema.json | 8 ++++---- src/index.ts | 4 ++-- src/runtimeHelpers.ts | 2 ++ 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 291d665..7e5b6ec 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -95,10 +95,10 @@ resources: 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 (must exist when propertyBatchKey is used) - swaggerLink: ?string (must exist when propertyBatchKey is used) - swaggerPath: ?string (must exist when propertyBatchKey is used) - httpMethod: ?string (must exist when propertyBatchKey is used) + responseKey: ?string (non-optional when propertyBatchKey is used) + swaggerLink: ?string (non-optional when propertyBatchKey is used) + swaggerPath: ?string (non-optional when propertyBatchKey is used) + httpMethod: ?string (non-optional when propertyBatchKey is used) typings: language: flow @@ -131,10 +131,10 @@ Describes the shape and behaviour of the resources object you will pass to `getL | `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` | (Optional) 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. | -| `swaggerLink` | (Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces. | -| `swaggerPath` | (Non-optional when propertyBatchKey is set)) The global paths section of the API specification. e.g. /users/v1. | -| `httpMethod` | (Non-optional when propertyBatchKey is set)) The http method of the API specification. e.g. get, post, delete, etc. | +| `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. | +| `swaggerLink` | (Non-optional when propertyBatchKey is used) The URL of your Swagger definition. Should point to a source of truth. | +| `swaggerPath` | (Non-optional when propertyBatchKey is used) The path section of the API specification. e.g. /users/v1. | +| `httpMethod` | (Non-optional when propertyBatchKey is used) The http method of the API specification. e.g. get, post, delete, etc. | | | ### `typings` diff --git a/schema.json b/schema.json index 55f4a7c..7b5b968 100644 --- a/schema.json +++ b/schema.json @@ -104,19 +104,19 @@ }, "responseKey": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set) 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." + "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." }, "swaggerLink": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set) The URL of your Swagger definition. Should point to a source of truth. Useful for others to verify information specified here is correct, and is used in stack traces." + "description": "(Non-optional when propertyBatchKey is used) The URL of your Swagger definition. Should point to a source of truth." }, "swaggerPath": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set) The global paths section of the API specification. e.g. /users/v1." + "description": "(Non-optional when propertyBatchKey is used) The path section of the API specification. e.g. /users/v1." }, "httpMethod": { "type": "string", - "description": "(Non-optional when propertyBatchKey is set) The http method of the API specification. e.g. get, post, delete, etc." + "description": "(Non-optional when propertyBatchKey is used) The http method of the API specification. e.g. get, post, delete, etc." } } }, diff --git a/src/index.ts b/src/index.ts index f0ab749..9346d61 100644 --- a/src/index.ts +++ b/src/index.ts @@ -51,7 +51,7 @@ function findVal(object: any, key: string, array: Array) { } /** - * Throw erros when resource uses propertyBatchKey feature but doesn't match requirements + * Throw erros when resource uses propertyBatchKey but doesn't match requirements */ function verifyBatchPropertyResource(args: CLIArgs) { const resources: object = getConfig(args.config).resources; @@ -81,7 +81,7 @@ function verifyBatchPropertyResource(args: CLIArgs) { `Cannot find the swagger response definition for ${key}, please make sure you have correct swagger info in the dataloader-cofig.yaml file!`, ); } else { - var swaggerResponse: any = api.paths[swaggerPath][httpMethod]['responses']['200']; + var swaggerResponse = api.paths[swaggerPath][httpMethod]['responses']['200']; // The resource may return the list of results in a nested path and properties are under the nestedPath if (value.hasOwnProperty('nestedPath')) { swaggerResponse = diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index ea07262..bf066a0 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -382,7 +382,9 @@ export function unPartitionResultsByBatchKeyPartition Date: Thu, 22 Jul 2021 10:01:07 -0700 Subject: [PATCH 16/27] update BatchResourceConfig --- src/config.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/config.ts b/src/config.ts index 018e2ef..28e4835 100644 --- a/src/config.ts +++ b/src/config.ts @@ -20,14 +20,17 @@ export interface BatchResourceConfig { isBatchResource: true; batchKey: string; newKey: string; - propertyBatchKey: string; - responseKey: string; reorderResultsByKey?: string; nestedPath?: string; commaSeparatedBatchKey?: boolean; // TODO: Assert somehow/somewhere that both isResponseDictionary and reorderResultsByKey aren't set isResponseDictionary?: boolean; isBatchKeyASet?: boolean; + propertyBatchKey?: string; + responseKey?: string; + swaggerLink?: string; + swaggerPath?: string; + httpMethod?: string; } export interface NonBatchResourceConfig { From 7bad34adfc689b3e32cfdd67b4451d4f3f34d77a Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Fri, 23 Jul 2021 16:48:11 -0700 Subject: [PATCH 17/27] update README.md --- README.md | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index dc489ae..d00989d 100644 --- a/README.md +++ b/README.md @@ -149,14 +149,26 @@ resources: ### Batch Resources with `properties` paramerters -Instead of accepting just a list of users (`user_ids`), a batch resouce could accept both a list of users (`user_ids`) and a list of properties (`properties`): +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 resouce with different `properties` for different `user_ids`, we specify `propertyBatchKey` in the config to describe the "properties" argument. +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: @@ -174,7 +186,7 @@ resources: **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): -1. The resouce accept two non-optional `list` type parameters, like this: +1. The resource accept two non-optional `list` type parameters, like this: ``` ids: str[] (should be the batchKey) @@ -182,9 +194,9 @@ properties: str[] (should be the propertyBatchKey) ``` 2. `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) -3. All `properties` should be optional in the response object. To enfore that, we require `swaggerLink`, `swaggerPath` and `httpMethod` defined in the config. -4. The resouce must have a one-to-one correspondence between the input "properties" and the output "properties". - (i.e. if we request property "name", the response must have "name" in it, and no extra data assciated with it.) +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.) See [the full docs](./API_DOCS.md) for more information on how to configure resources. From ebbfe287fc9ce87e4c3482983f45fbe81048a721 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Fri, 23 Jul 2021 16:50:14 -0700 Subject: [PATCH 18/27] update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d00989d..8e1597f 100644 --- a/README.md +++ b/README.md @@ -196,7 +196,7 @@ properties: str[] (should be the propertyBatchKey) 2. `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.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.) + - e.g. if we request property "name", the response must have "name" in it, and no extra data associated with it. See [the full docs](./API_DOCS.md) for more information on how to configure resources. From 4df94f8e88149fba5b2db576360443baadcb4b87 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Mon, 26 Jul 2021 16:04:34 -0700 Subject: [PATCH 19/27] remove the script and several config --- API_DOCS.md | 7 -- examples/swapi/swapi.dataloader-config.yaml | 3 - schema.json | 12 --- src/config.ts | 3 - src/index.ts | 85 +-------------------- 5 files changed, 1 insertion(+), 109 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 7e5b6ec..9fc1dfb 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -96,9 +96,6 @@ resources: isBatchKeyASet: ?boolean (can only use if isBatchResource=true) propertyBatchKey: ?string (can only use if isBatchResource=true) responseKey: ?string (non-optional when propertyBatchKey is used) - swaggerLink: ?string (non-optional when propertyBatchKey is used) - swaggerPath: ?string (non-optional when propertyBatchKey is used) - httpMethod: ?string (non-optional when propertyBatchKey is used) typings: language: flow @@ -132,10 +129,6 @@ 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. | -| `swaggerLink` | (Non-optional when propertyBatchKey is used) The URL of your Swagger definition. Should point to a source of truth. | -| `swaggerPath` | (Non-optional when propertyBatchKey is used) The path section of the API specification. e.g. /users/v1. | -| `httpMethod` | (Non-optional when propertyBatchKey is used) The http method of the API specification. e.g. get, post, delete, etc. | -| | ### `typings` diff --git a/examples/swapi/swapi.dataloader-config.yaml b/examples/swapi/swapi.dataloader-config.yaml index abf2830..08222da 100644 --- a/examples/swapi/swapi.dataloader-config.yaml +++ b/examples/swapi/swapi.dataloader-config.yaml @@ -36,9 +36,6 @@ resources: nestedPath: properties propertyBatchKey: properties responseKey: id - swaggerLink: https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM - swaggerPath: /v2/business - httpMethod: get getRoot: docsLink: https://swapi.dev/documentation#root isBatchResource: false diff --git a/schema.json b/schema.json index 7b5b968..627844d 100644 --- a/schema.json +++ b/schema.json @@ -105,18 +105,6 @@ "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." - }, - "swaggerLink": { - "type": "string", - "description": "(Non-optional when propertyBatchKey is used) The URL of your Swagger definition. Should point to a source of truth." - }, - "swaggerPath": { - "type": "string", - "description": "(Non-optional when propertyBatchKey is used) The path section of the API specification. e.g. /users/v1." - }, - "httpMethod": { - "type": "string", - "description": "(Non-optional when propertyBatchKey is used) The http method of the API specification. e.g. get, post, delete, etc." } } }, diff --git a/src/config.ts b/src/config.ts index 28e4835..a57f83d 100644 --- a/src/config.ts +++ b/src/config.ts @@ -28,9 +28,6 @@ export interface BatchResourceConfig { isBatchKeyASet?: boolean; propertyBatchKey?: string; responseKey?: string; - swaggerLink?: string; - swaggerPath?: string; - httpMethod?: string; } export interface NonBatchResourceConfig { diff --git a/src/index.ts b/src/index.ts index 9346d61..14c68ae 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,89 +22,6 @@ function writeLoaders(args: CLIArgs) { fs.writeFileSync(args.output, output); } -/** - * If requiredList only contains responseKey - */ -function validRequiredList(requiredList: Array, responseKey: string) { - if (requiredList.length > 1) return false; - if (requiredList.length == 1 && requiredList[0] !== responseKey) { - return false; - } - return true; -} - -/** - * Find all values of the intput key in an object recursively - */ -function findVal(object: any, key: string, array: Array) { - Object.keys(object).some(function (k) { - if (k === key) { - array.push(...object[k]); - } - if (object[k] && typeof object[k] === 'object') { - findVal(object[k], key, array); - } else { - return; - } - }); - return array; -} - -/** - * Throw erros when resource uses propertyBatchKey but doesn't match requirements - */ -function verifyBatchPropertyResource(args: CLIArgs) { - const resources: object = getConfig(args.config).resources; - - Object.entries(resources).forEach(([key, value]) => { - if (Object.keys(value).includes('propertyBatchKey')) { - const propertyBatchKey = value['propertyBatchKey']; - if ( - !value.hasOwnProperty('responseKey') || - !value.hasOwnProperty('swaggerLink') || - !value.hasOwnProperty('swaggerPath') || - !value.hasOwnProperty('httpMethod') - ) { - throw new Error(`Missing swagger info for ${key}, please add them in the dataloader-cofig.yaml file!`); - } - const swaggerLink: string = value['swaggerLink']; - const swaggerPath: string = value['swaggerPath']; - const httpMethod: string = value['httpMethod']; - - // parse swagger file, so that all $ref pointers will be resolved. - SwaggerParser.validate(swaggerLink, (err, api) => { - if (err) { - console.error(err); - } else { - if (!api || !api.paths[swaggerPath][httpMethod]) { - throw new Error( - `Cannot find the swagger response definition for ${key}, please make sure you have correct swagger info in the dataloader-cofig.yaml file!`, - ); - } else { - var swaggerResponse = api.paths[swaggerPath][httpMethod]['responses']['200']; - // The resource may return the list of results in a nested path and properties are under the nestedPath - if (value.hasOwnProperty('nestedPath')) { - swaggerResponse = - api.paths[swaggerPath][httpMethod]['responses']['200']['schema'][value['nestedPath']]; - } - // Find all the fields that are `required` in the swagger spec - const requiredList = findVal(swaggerResponse, 'required', []); - const responseKey = value['responseKey']; - if (!validRequiredList(requiredList, responseKey)) { - throw new Error( - [ - 'Sorry, your swagger endpoint does not match the requirement of using propertyBatchKey,', - 'please read https://github.com/Yelp/dataloader-codegen/blob/master/README.md', - ].join(' '), - ); - } - } - } - }); - } - }); -} - function main() { const argv = yargs .options({ @@ -120,7 +37,7 @@ function main() { }, }) .help().argv; - verifyBatchPropertyResource(argv); + writeLoaders(argv); } From 46030620428f4c5f14961c2462d662213d5c47e2 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Mon, 26 Jul 2021 16:09:08 -0700 Subject: [PATCH 20/27] remove swagger-parser package --- package.json | 2 -- yarn.lock | 92 +--------------------------------------------------- 2 files changed, 1 insertion(+), 93 deletions(-) diff --git a/package.json b/package.json index 2593ec3..0c234a0 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,6 @@ "typescript": "^3.7.0-beta" }, "dependencies": { - "@apidevtools/swagger-parser": "^10.0.2", "@babel/preset-env": "^7.6.3", "aggregate-error": "^3.0.1", "ajv": "^6.11.0", @@ -53,7 +52,6 @@ "js-yaml": "^3.13.1", "lodash": "^4.17.15", "object-hash": "^2.0.0", - "openapi-types": "^1.3.5", "prettier": "^1.19.1", "yargs": "^14.2.0" }, diff --git a/yarn.lock b/yarn.lock index ff4868e..9d0d22f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,38 +2,6 @@ # yarn lockfile v1 -"@apidevtools/json-schema-ref-parser@^9.0.6": - version "9.0.9" - resolved "https://registry.yarnpkg.com/@apidevtools/json-schema-ref-parser/-/json-schema-ref-parser-9.0.9.tgz#d720f9256e3609621280584f2b47ae165359268b" - integrity sha512-GBD2Le9w2+lVFoc4vswGI/TjkNIZSVp7+9xPf+X3uidBfWnAeUWmquteSyt0+VCrhNMWj/FTABISQrD3Z/YA+w== - dependencies: - "@jsdevtools/ono" "^7.1.3" - "@types/json-schema" "^7.0.6" - call-me-maybe "^1.0.1" - js-yaml "^4.1.0" - -"@apidevtools/openapi-schemas@^2.0.4": - version "2.1.0" - resolved "https://registry.yarnpkg.com/@apidevtools/openapi-schemas/-/openapi-schemas-2.1.0.tgz#9fa08017fb59d80538812f03fc7cac5992caaa17" - integrity sha512-Zc1AlqrJlX3SlpupFGpiLi2EbteyP7fXmUOGup6/DnkRgjP9bgMM/ag+n91rsv0U1Gpz0H3VILA/o3bW7Ua6BQ== - -"@apidevtools/swagger-methods@^3.0.2": - version "3.0.2" - resolved "https://registry.yarnpkg.com/@apidevtools/swagger-methods/-/swagger-methods-3.0.2.tgz#b789a362e055b0340d04712eafe7027ddc1ac267" - integrity sha512-QAkD5kK2b1WfjDS/UQn/qQkbwF31uqRjPTrsCs5ZG9BQGAkjwvqGFjjPqAuzac/IYzpPtRzjCP1WrTuAIjMrXg== - -"@apidevtools/swagger-parser@^10.0.2": - version "10.0.2" - resolved "https://registry.npmjs.org/@apidevtools/swagger-parser/-/swagger-parser-10.0.2.tgz#f4145afb7c3a3bafe0376f003b5c3bdeae17a952" - integrity sha512-JFxcEyp8RlNHgBCE98nwuTkZT6eNFPc1aosWV6wPcQph72TSEEu1k3baJD4/x1qznU+JiDdz8F5pTwabZh+Dhg== - dependencies: - "@apidevtools/json-schema-ref-parser" "^9.0.6" - "@apidevtools/openapi-schemas" "^2.0.4" - "@apidevtools/swagger-methods" "^3.0.2" - "@jsdevtools/ono" "^7.1.3" - call-me-maybe "^1.0.1" - z-schema "^4.2.3" - "@babel/cli@^7.6.3": version "7.6.3" resolved "https://registry.yarnpkg.com/@babel/cli/-/cli-7.6.3.tgz#1b0c62098c8a5e01e4a4a59a52cba9682e7e0906" @@ -1076,11 +1044,6 @@ "@types/yargs" "^15.0.0" chalk "^3.0.0" -"@jsdevtools/ono@^7.1.3": - version "7.1.3" - resolved "https://registry.yarnpkg.com/@jsdevtools/ono/-/ono-7.1.3.tgz#9df03bbd7c696a5c58885c34aa06da41c8543796" - integrity sha512-4JQNk+3mVzK3xh2rqd6RB4J46qUR19azEHBneZyTZM+c456qOrbbM/5xcR8huNCCcbVt7+UmizG6GuUvPvKUYg== - "@sinonjs/commons@^1.7.0": version "1.7.0" resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.7.0.tgz#f90ffc52a2e519f018b13b6c4da03cbff36ebed6" @@ -1159,11 +1122,6 @@ resolved "https://registry.yarnpkg.com/@types/js-yaml/-/js-yaml-3.12.1.tgz#5c6f4a1eabca84792fbd916f0cb40847f123c656" integrity sha512-SGGAhXLHDx+PK4YLNcNGa6goPf9XRWQNAUUbffkwVGGXIxmDKWyGGL4inzq2sPmExu431Ekb9aEMn9BkPqEYFA== -"@types/json-schema@^7.0.6": - version "7.0.8" - resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.8.tgz#edf1bf1dbf4e04413ca8e5b17b3b7d7d54b59818" - integrity sha512-YSBPTLTVm2e2OoQIDYx8HaeWJ5tTToLH67kXR7zYNGupXMEHa2++G8k+DczX2cFVgalypqtyZIcU19AFcmOpmg== - "@types/lodash@^4.14.144": version "4.14.144" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.144.tgz#12e57fc99064bce45e5ab3c8bc4783feb75eab8e" @@ -1349,11 +1307,6 @@ argparse@^1.0.7: dependencies: sprintf-js "~1.0.2" -argparse@^2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38" - integrity sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q== - arr-diff@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/arr-diff/-/arr-diff-4.0.0.tgz#d6461074febfec71e7e15235761a329a5dc7c520" @@ -1582,11 +1535,6 @@ cache-base@^1.0.1: union-value "^1.0.0" unset-value "^1.0.0" -call-me-maybe@^1.0.1: - version "1.0.1" - resolved "https://registry.yarnpkg.com/call-me-maybe/-/call-me-maybe-1.0.1.tgz#26d208ea89e37b5cbde60250a15f031c16a4d66b" - integrity sha1-JtII6onje1y95gJQoV8DHBak1ms= - callsites@^3.0.0: version "3.1.0" resolved "https://registry.yarnpkg.com/callsites/-/callsites-3.1.0.tgz#b3630abd8943432f54b3f0519238e33cd7df2f73" @@ -1747,7 +1695,7 @@ combined-stream@^1.0.6, combined-stream@~1.0.6: dependencies: delayed-stream "~1.0.0" -commander@^2.11.0, commander@^2.7.1: +commander@^2.11.0: version "2.20.3" resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33" integrity sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ== @@ -3198,13 +3146,6 @@ js-yaml@^3.13.1: argparse "^1.0.7" esprima "^4.0.0" -js-yaml@^4.1.0: - version "4.1.0" - resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-4.1.0.tgz#c1fb65f8f5017901cdd2c951864ba18458a10602" - integrity sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA== - dependencies: - argparse "^2.0.1" - jsbn@~0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/jsbn/-/jsbn-0.1.1.tgz#a5e654c2e5a2deb5f201d96cefbca80c0ef2f513" @@ -3341,16 +3282,6 @@ locate-path@^5.0.0: dependencies: p-locate "^4.1.0" -lodash.get@^4.4.2: - version "4.4.2" - resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" - integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk= - -lodash.isequal@^4.5.0: - version "4.5.0" - resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" - integrity sha1-QVxEePK8wwEgwizhDtMib30+GOA= - lodash.sortby@^4.7.0: version "4.7.0" resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438" @@ -3739,11 +3670,6 @@ onetime@^5.1.0: dependencies: mimic-fn "^2.1.0" -openapi-types@^1.3.5: - version "1.3.5" - resolved "https://registry.npmjs.org/openapi-types/-/openapi-types-1.3.5.tgz#6718cfbc857fe6c6f1471f65b32bdebb9c10ce40" - integrity sha512-11oi4zYorsgvg5yBarZplAqbpev5HkuVNPlZaPTknPDzAynq+lnJdXAmruGWP0s+dNYZS7bjM+xrTpJw7184Fg== - optionator@^0.8.1: version "0.8.2" resolved "https://registry.yarnpkg.com/optionator/-/optionator-0.8.2.tgz#364c5e409d3f4d6301d6c0b4c05bba50180aeb64" @@ -4851,11 +4777,6 @@ v8-to-istanbul@^4.0.1: convert-source-map "^1.6.0" source-map "^0.7.3" -validator@^12.0.0: - version "12.2.0" - resolved "https://registry.yarnpkg.com/validator/-/validator-12.2.0.tgz#660d47e96267033fd070096c3b1a6f2db4380a0a" - integrity sha512-jJfE/DW6tIK1Ek8nCfNFqt8Wb3nzMoAbocBF6/Icgg1ZFSBpObdnwVY2jQj6qUqzhx5jc71fpvBWyLGO7Xl+nQ== - verror@1.10.0: version "1.10.0" resolved "https://registry.yarnpkg.com/verror/-/verror-1.10.0.tgz#3a105ca17053af55d6e270c1f8288682e18da400" @@ -5052,14 +4973,3 @@ yargs@^15.0.0: which-module "^2.0.0" y18n "^4.0.0" yargs-parser "^16.1.0" - -z-schema@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/z-schema/-/z-schema-4.2.3.tgz#85f7eea7e6d4fe59a483462a98f511bd78fe9882" - integrity sha512-zkvK/9TC6p38IwcrbnT3ul9in1UX4cm1y/VZSs4GHKIiDCrlafc+YQBgQBUdDXLAoZHf2qvQ7gJJOo6yT1LH6A== - dependencies: - lodash.get "^4.4.2" - lodash.isequal "^4.5.0" - validator "^12.0.0" - optionalDependencies: - commander "^2.7.1" From a245c58c1143e8c65150425a772a321f857c5afe Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Mon, 26 Jul 2021 16:11:57 -0700 Subject: [PATCH 21/27] test swapi --- examples/swapi/swapi-loaders.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index 821a98a..fde0315 100644 --- a/examples/swapi/swapi-loaders.js +++ b/examples/swapi/swapi-loaders.js @@ -1553,10 +1553,7 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade * "newKey": "film_id", * "nestedPath": "properties", * "propertyBatchKey": "properties", - * "responseKey": "id", - * "swaggerLink": "https://github.yelpcorp.com/raw/clientlibs/search_business_properties/master/search_business_properties_clientlib_ng/swagger.json?token=AAAAIVXIYGB4EMDIUMF7CMTBABBJM", - * "swaggerPath": "/v2/business", - * "httpMethod": "get" + * "responseKey": "id" * } * ``` */ From 31a14c870313fb984493a401407448837693a1a7 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 27 Jul 2021 15:08:46 -0700 Subject: [PATCH 22/27] remove import --- src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 14c68ae..2f46bca 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,6 @@ import path from 'path'; import yargs from 'yargs'; import codegen from './codegen'; import { getConfig } from './config'; -import SwaggerParser from '@apidevtools/swagger-parser'; interface CLIArgs { config: string; From 344462dc8d534f5f3510b6cff4e46bf87bd8784f Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Tue, 27 Jul 2021 15:13:37 -0700 Subject: [PATCH 23/27] update comments --- src/runtimeHelpers.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index bf066a0..952b6e0 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -304,7 +304,7 @@ export function unPartitionResults( * 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 'bar_id' as newKey and 'properties' as propertyBatchKey, + * If we have 'id' as responseKey and 'properties' as propertyBatchKey, * the resultGroups should look like this: * [ * [ { id: 2, name: 'Burger King', rating: 3 } ], @@ -313,7 +313,7 @@ export function unPartitionResults( * * * 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 assciated with it. + * 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: @@ -332,8 +332,8 @@ export function unPartitionResults( * propertyBatchKeyPartion = [ [['name'], ['rating']], [['rating']] ], * requestGroups = [ [0, 2], [1] ], * resultGroups = [ - * [ { bar_id: 2, name: 'Burger King', rating: 3 } ], - * [ { bar_id: 1, name: 'In N Out', rating: 4 } ] + * [ { id: 2, name: 'Burger King', rating: 3 } ], + * [ { id: 1, name: 'In N Out', rating: 4 } ] * ], * ) * ``` @@ -362,11 +362,11 @@ export function unPartitionResultsByBatchKeyPartition Date: Wed, 28 Jul 2021 11:37:00 -0700 Subject: [PATCH 24/27] fix review issues --- README.md | 34 ++++++++++++++++++-------------- __tests__/implementation.test.js | 12 +++++++---- schema.json | 3 +++ src/implementation.ts | 2 +- src/runtimeHelpers.ts | 3 ++- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 8e1597f..af1f18c 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ resources: newKey: user_id ``` -### Batch Resources with `properties` paramerters +### 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: @@ -156,16 +156,18 @@ const getUserInfo = (args: { user_ids: Array, properties: Array fetch('/userInfo', args); const users = getUserInfo({ - user_ids: [1, 2, 3], - properties: ['firstName', 'age'] + 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 }, -] +/** + * 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. @@ -186,14 +188,16 @@ resources: **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): -1. The resource accept two non-optional `list` type parameters, like this: +**Contract** -``` -ids: str[] (should be the batchKey) -properties: str[] (should be the propertyBatchKey) -``` +1. The resource accepts two arguments; to specify the entity IDs and the properties for each entity to fetch: + + ``` + ids: str[] (should be the batchKey) + properties: str[] (should be the propertyBatchKey) + ``` -2. `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) +2. In the response, `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.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. diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index bf91275..2cf7283 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1289,6 +1289,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex const resources = { foo: ({ foo_ids, bar }) => { if (_.isEqual(foo_ids, [1, 2, 3])) { + expect(include_extra_info).toBe(true); return Promise.resolve([ { foo_id: 1, @@ -1303,6 +1304,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex }, ]); } else if (_.isEqual(foo_ids, [4])) { + expect(include_extra_info).toBe(false); return Promise.resolve([ { foo_id: 4, @@ -1418,12 +1420,14 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp const resources = { foo: ({ foo_ids, bar }) => { 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' }]); } }, @@ -1432,10 +1436,10 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp const loaders = getLoaders(resources); const results = await loaders.foo.loadMany([ - { foo_id: 1, properties: ['name', 'rating'], bar: true }, - { foo_id: 2, properties: ['name'], bar: true }, - { foo_id: 3, properties: ['rating'], bar: true }, - { foo_id: 4, properties: ['rating'], bar: false }, + { 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([ diff --git a/schema.json b/schema.json index 627844d..d8e834e 100644 --- a/schema.json +++ b/schema.json @@ -106,6 +106,9 @@ "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/implementation.ts b/src/implementation.ts index 47836dc..a09696d 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -163,7 +163,6 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado * ]) * \`\`\` * - * Returns: * \`[ [ 0, 2 ], [ 1 ] ]\` * @@ -181,6 +180,7 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado * * Returns: * \`[ [ 0, 2 ], [ 1 ] ]\` + * * We'll refer to each element in the group as a "request ID". */ let requestGroups; diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index 952b6e0..6b3c8e4 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -389,6 +389,7 @@ export function unPartitionResultsByBatchKeyPartition Date: Wed, 28 Jul 2021 11:43:56 -0700 Subject: [PATCH 25/27] fix tests --- __tests__/implementation.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index 2cf7283..ca40d41 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1287,7 +1287,7 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex }; const resources = { - foo: ({ foo_ids, bar }) => { + foo: ({ foo_ids, properties, include_extra_info }) => { if (_.isEqual(foo_ids, [1, 2, 3])) { expect(include_extra_info).toBe(true); return Promise.resolve([ @@ -1418,7 +1418,7 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp }; const resources = { - foo: ({ foo_ids, bar }) => { + foo: ({ foo_ids, properties, include_extra_info }) => { if (_.isEqual(foo_ids, [1, 2, 3])) { expect(include_extra_info).toBe(true); return Promise.resolve([ From 398fc7406769de35e79a8a0cc2ddb924956c0fe5 Mon Sep 17 00:00:00 2001 From: Yue Guo Date: Wed, 28 Jul 2021 13:57:27 -0700 Subject: [PATCH 26/27] fix review issues --- API_DOCS.md | 59 ++++++++++++++++++++++++++++++++ README.md | 55 ----------------------------- __tests__/implementation.test.js | 10 ++++-- src/runtimeHelpers.ts | 1 + 4 files changed, 68 insertions(+), 57 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 9fc1dfb..9d763ed 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`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.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: diff --git a/README.md b/README.md index af1f18c..9496a31 100644 --- a/README.md +++ b/README.md @@ -147,61 +147,6 @@ resources: newKey: user_id ``` -### 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 two arguments; to specify the entity IDs and the properties for each entity to fetch: - - ``` - ids: str[] (should be the batchKey) - properties: str[] (should be the propertyBatchKey) - ``` - -2. In the response, `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.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. - See [the full docs](./API_DOCS.md) for more information on how to configure resources. ## Contributing diff --git a/__tests__/implementation.test.js b/__tests__/implementation.test.js index ca40d41..b81d9de 100644 --- a/__tests__/implementation.test.js +++ b/__tests__/implementation.test.js @@ -1329,7 +1329,10 @@ test('batch endpoint with propertyBatchKey throws error for response with non ex 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.', + [ + '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 }, @@ -1445,7 +1448,10 @@ test('batch endpoint with propertyBatchKey with reorderResultsByKey handles resp 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.', + [ + '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 }, diff --git a/src/runtimeHelpers.ts b/src/runtimeHelpers.ts index 6b3c8e4..2cd4dfb 100644 --- a/src/runtimeHelpers.ts +++ b/src/runtimeHelpers.ts @@ -398,6 +398,7 @@ export function unPartitionResultsByBatchKeyPartition Date: Wed, 28 Jul 2021 14:10:42 -0700 Subject: [PATCH 27/27] some format issues --- API_DOCS.md | 2 +- examples/swapi/swapi-loaders.js | 780 ++++++++++++++++---------------- src/implementation.ts | 7 +- 3 files changed, 395 insertions(+), 394 deletions(-) diff --git a/API_DOCS.md b/API_DOCS.md index 9d763ed..c0c2101 100644 --- a/API_DOCS.md +++ b/API_DOCS.md @@ -131,7 +131,7 @@ To use this feature, there are several restrictions. (Please open an issue if yo }): Array ``` -2. In the response, `properties` are spread at the same level as the `responseKey`. (see `getFilmsV2` in [swapi example](./examples/swapi/swapi-server.js)) +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. diff --git a/examples/swapi/swapi-loaders.js b/examples/swapi/swapi-loaders.js index fde0315..5b7204f 100644 --- a/examples/swapi/swapi-loaders.js +++ b/examples/swapi/swapi-loaders.js @@ -276,84 +276,84 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - - * 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: - * `[ [ 0, 2 ], [ 1 ] ]` - * We'll refer to each element in the group as a "request ID". - */ + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We'll refer to each element in the group as a "request ID". + */ let requestGroups; if (false) { @@ -598,84 +598,84 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - - * 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: - * `[ [ 0, 2 ], [ 1 ] ]` - * We'll refer to each element in the group as a "request ID". - */ + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We'll refer to each element in the group as a "request ID". + */ let requestGroups; if (false) { @@ -917,84 +917,84 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - - * 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: - * `[ [ 0, 2 ], [ 1 ] ]` - * We'll refer to each element in the group as a "request ID". - */ + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We'll refer to each element in the group as a "request ID". + */ let requestGroups; if (false) { @@ -1248,84 +1248,84 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - - * 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: - * `[ [ 0, 2 ], [ 1 ] ]` - * We'll refer to each element in the group as a "request ID". - */ + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We'll refer to each element in the group as a "request ID". + */ let requestGroups; if (false) { @@ -1567,84 +1567,84 @@ export default function getLoaders(resources: ResourcesType, options?: DataLoade ); /** - * Chunk up the "keys" array to create a set of "request groups". - * - * We're about to hit a batch resource. In addition to the batch - * key, the resource may take other arguments too. When batching - * up requests, we'll want to look out for where those other - * arguments differ, and send multiple requests so we don't get - * back the wrong info. - * - * In other words, we'll potentially want to send _multiple_ - * requests to the underlying resource batch method in this - * dataloader body. - * - * ~~~ Why? ~~~ - * - * Consider what happens when we get called with arguments where - * the non-batch keys differ. - * - * Example: - * - * ```js - * loaders.foo.load({ foo_id: 2, include_private_data: true }); - * loaders.foo.load({ foo_id: 3, include_private_data: false }); - * loaders.foo.load({ foo_id: 4, include_private_data: false }); - * ``` - * - * If we collected everything up and tried to send the one - * request to the resource as a batch request, how do we know - * what the value for "include_private_data" should be? We're - * going to have to group these up up and send two requests to - * the resource to make sure we're requesting the right stuff. - * - * e.g. We'd need to make the following set of underlying resource - * calls: - * - * ```js - * foo({ foo_ids: [ 2 ], include_private_data: true }); - * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); - * ``` - * - * ~~~ tl;dr ~~~ - * - * When we have calls to .load with differing non batch key args, - * we'll need to send multiple requests to the underlying - * resource to make sure we get the right results back. - * - * Let's create the request groups, where each element in the - * group refers to a position in "keys" (i.e. a call to .load) - * - * Example: - * - * ```js - * partitionItems('bar_id', [ - * { bar_id: 7, include_extra_info: true }, - * { bar_id: 8, include_extra_info: false }, - * { bar_id: 9, include_extra_info: true }, - * ]) - * ``` - * - - * 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: - * `[ [ 0, 2 ], [ 1 ] ]` - * We'll refer to each element in the group as a "request ID". - */ + * Chunk up the "keys" array to create a set of "request groups". + * + * We're about to hit a batch resource. In addition to the batch + * key, the resource may take other arguments too. When batching + * up requests, we'll want to look out for where those other + * arguments differ, and send multiple requests so we don't get + * back the wrong info. + * + * In other words, we'll potentially want to send _multiple_ + * requests to the underlying resource batch method in this + * dataloader body. + * + * ~~~ Why? ~~~ + * + * Consider what happens when we get called with arguments where + * the non-batch keys differ. + * + * Example: + * + * ```js + * loaders.foo.load({ foo_id: 2, include_private_data: true }); + * loaders.foo.load({ foo_id: 3, include_private_data: false }); + * loaders.foo.load({ foo_id: 4, include_private_data: false }); + * ``` + * + * If we collected everything up and tried to send the one + * request to the resource as a batch request, how do we know + * what the value for "include_private_data" should be? We're + * going to have to group these up up and send two requests to + * the resource to make sure we're requesting the right stuff. + * + * e.g. We'd need to make the following set of underlying resource + * calls: + * + * ```js + * foo({ foo_ids: [ 2 ], include_private_data: true }); + * foo({ foo_ids: [ 3, 4 ], include_private_data: false }); + * ``` + * + * ~~~ tl;dr ~~~ + * + * When we have calls to .load with differing non batch key args, + * we'll need to send multiple requests to the underlying + * resource to make sure we get the right results back. + * + * Let's create the request groups, where each element in the + * group refers to a position in "keys" (i.e. a call to .load) + * + * Example: + * + * ```js + * partitionItems('bar_id', [ + * { bar_id: 7, include_extra_info: true }, + * { bar_id: 8, include_extra_info: false }, + * { bar_id: 9, include_extra_info: true }, + * ]) + * ``` + * + * 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: + * `[ [ 0, 2 ], [ 1 ] ]` + * + * We'll refer to each element in the group as a "request ID". + */ let requestGroups; if (true) { diff --git a/src/implementation.ts b/src/implementation.ts index a09696d..fef9086 100644 --- a/src/implementation.ts +++ b/src/implementation.ts @@ -186,9 +186,10 @@ function getBatchLoader(resourceConfig: BatchResourceConfig, resourcePath: Reado let requestGroups; if (${typeof resourceConfig.propertyBatchKey === 'string'}) { - requestGroups = partitionItems(['${resourceConfig.newKey}', '${ - resourceConfig.propertyBatchKey - }'], keys); + requestGroups = partitionItems([ + '${resourceConfig.newKey}', + '${resourceConfig.propertyBatchKey}' + ], keys); } else { requestGroups = partitionItems('${resourceConfig.newKey}', keys); }