From 6e556423f63a1cdaa8dce79bee6dc4005dad8f7e Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Thu, 11 Apr 2019 09:29:19 -0400 Subject: [PATCH] Correct resolver batching behavior (#1495) * Fix issue where queriedGroup sometimes isn't an array * Refactor batch loader to handle duplicates, slugs * Resolve corner cases with batch loader 1. Normalizes the input to the loader 2. Removes the notion of defaults 3. Ensures list results return an array 4. Ensures multiple items are batched correctly with loadMany * Update batching to work with gravity batch param --- src/data/complete.queryMap.json | 2 +- src/data/vortex.graphql | 7 +- src/lib/cache.ts | 4 +- src/lib/helpers.ts | 17 +- src/lib/loaders/__tests__/batchLoader.test.ts | 164 +++++++++------ src/lib/loaders/batchLoader.ts | 189 ++++++++++++------ .../loaders_without_authentication/gravity.ts | 4 +- src/schema/object_identification.ts | 5 +- 8 files changed, 260 insertions(+), 132 deletions(-) diff --git a/src/data/complete.queryMap.json b/src/data/complete.queryMap.json index f269c8c17e..989082f549 100644 --- a/src/data/complete.queryMap.json +++ b/src/data/complete.queryMap.json @@ -483,4 +483,4 @@ "4534ea8e115e15baa7f77f2a652902a6": "query ShowTestsQuery {\n show(id: \"anderson-fine-art-gallery-flickinger-collection\") {\n ...Show_show\n __id\n }\n}\n\nfragment Show_show on Show {\n ...Detail_show\n ...MoreInfo_show\n ...ShowArtists_show\n ...ShowArtworks_show\n __id\n}\n\nfragment Detail_show on Show {\n _id\n id\n name\n description\n city\n is_local_discovery\n images {\n id\n }\n ...ShowHeader_show\n ...ShowArtworksPreview_show\n ...ShowArtistsPreview_show\n ...Shows_show\n location {\n ...LocationMap_location\n openingHours {\n __typename\n ... on OpeningHoursArray {\n schedules {\n days\n hours\n }\n }\n ... on OpeningHoursText {\n text\n }\n }\n __id\n }\n artists_without_artworks {\n id\n __id\n }\n counts {\n artworks\n artists\n }\n status\n partner {\n __typename\n ... on Partner {\n name\n type\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n __id\n}\n\nfragment MoreInfo_show on Show {\n _id\n id\n exhibition_period\n pressReleaseUrl\n openingReceptionText\n partner {\n __typename\n ... on Partner {\n website\n type\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n press_release\n events {\n ...ShowEventSection_event\n }\n __id\n}\n\nfragment ShowArtists_show on Show {\n _id\n id\n artists_grouped_by_name {\n letter\n items {\n ...ArtistListItem_artist\n sortable_id\n href\n __id\n }\n }\n __id\n}\n\nfragment ShowArtworks_show on Show {\n __id\n id\n _id\n filteredArtworks(size: 0, medium: \"*\", price_range: \"*-*\", aggregations: [MEDIUM, PRICE_RANGE, TOTAL]) {\n ...FilteredInfiniteScrollGrid_filteredArtworks\n __id\n }\n}\n\nfragment FilteredInfiniteScrollGrid_filteredArtworks on FilterArtworks {\n ...Filters_filteredArtworks\n ...ArtworksGridPaginationContainer_filteredArtworks\n __id\n}\n\nfragment Filters_filteredArtworks on FilterArtworks {\n aggregations {\n slice\n counts {\n id\n name\n __id\n }\n }\n __id\n}\n\nfragment ArtworksGridPaginationContainer_filteredArtworks on FilterArtworks {\n __id\n artworks: artworks_connection(first: 10) {\n pageInfo {\n hasNextPage\n startCursor\n endCursor\n }\n edges {\n node {\n id\n __id\n image {\n aspect_ratio\n }\n ...Artwork_artwork\n __typename\n }\n cursor\n }\n }\n}\n\nfragment Artwork_artwork on Artwork {\n title\n date\n sale_message\n is_in_auction\n is_biddable\n is_acquireable\n is_offerable\n id\n sale {\n is_auction\n is_live_open\n is_open\n is_closed\n display_timely_at\n __id\n }\n sale_artwork {\n opening_bid {\n display\n }\n current_bid {\n display\n }\n bidder_positions_count\n sale {\n is_closed\n __id\n }\n __id\n }\n image {\n url(version: \"large\")\n aspect_ratio\n }\n artists(shallow: true) {\n name\n __id\n }\n partner {\n name\n __id\n }\n href\n __id\n}\n\nfragment ArtistListItem_artist on Artist {\n __id\n _id\n id\n name\n is_followed\n nationality\n birthday\n deathday\n image {\n url\n }\n}\n\nfragment ShowEventSection_event on PartnerShowEventType {\n event_type\n description\n start_at\n end_at\n}\n\nfragment ShowHeader_show on Show {\n id\n _id\n __id\n name\n press_release\n is_followed\n end_at\n exhibition_period\n status\n isStubShow\n partner {\n __typename\n ... on Partner {\n name\n id\n href\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n images {\n url\n aspect_ratio\n }\n followedArtists(first: 3) {\n edges {\n node {\n artist {\n name\n href\n id\n _id\n __id\n }\n }\n }\n }\n artists {\n name\n href\n id\n _id\n __id\n }\n}\n\nfragment ShowArtworksPreview_show on Show {\n __id\n artworks(size: 6) {\n ...GenericGrid_artworks\n __id\n }\n counts {\n artworks\n }\n}\n\nfragment ShowArtistsPreview_show on Show {\n _id\n id\n artists {\n _id\n id\n href\n ...ArtistListItem_artist\n __id\n }\n artists_without_artworks {\n _id\n id\n href\n ...ArtistListItem_artist\n __id\n }\n __id\n}\n\nfragment Shows_show on Show {\n nearbyShows(first: 20) {\n edges {\n node {\n __id\n ...ShowItem_show\n }\n }\n }\n __id\n}\n\nfragment LocationMap_location on Location {\n __id\n id\n city\n address\n address_2\n postal_code\n summary\n coordinates {\n lat\n lng\n }\n day_schedules {\n start_time\n end_time\n day_of_week\n }\n openingHours {\n __typename\n ... on OpeningHoursArray {\n schedules {\n days\n hours\n }\n }\n ... on OpeningHoursText {\n text\n }\n }\n}\n\nfragment ShowItem_show on Show {\n _id\n id\n name\n exhibition_period\n end_at\n images {\n url\n aspect_ratio\n }\n partner {\n __typename\n ... on Partner {\n name\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n __id\n}\n\nfragment GenericGrid_artworks on Artwork {\n __id\n id\n image {\n aspect_ratio\n }\n ...Artwork_artwork\n}\n", "edb302bf331c6fc8a0c7ecaf3c107bcb": "query ShowsQuery(\n $count: Int!\n $cursor: String\n) {\n me {\n ...Shows_me_1G22uz\n __id\n }\n}\n\nfragment Shows_me_1G22uz on Me {\n followsAndSaves {\n shows(first: $count, after: $cursor) {\n pageInfo {\n startCursor\n endCursor\n hasPreviousPage\n hasNextPage\n }\n edges {\n node {\n __id\n ...ShowItemRow_show\n __typename\n }\n cursor\n }\n }\n }\n __id\n}\n\nfragment ShowItemRow_show on Show {\n id\n _id\n __id\n is_followed\n name\n isStubShow\n partner {\n __typename\n ... on Partner {\n name\n profile {\n image {\n url(version: \"square\")\n }\n __id\n }\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n href\n exhibition_period\n status\n cover_image {\n url\n aspect_ratio\n }\n is_fair_booth\n start_at\n end_at\n}\n", "e5c878c2ad186621b8ed4f9737f81519": "query indexTestsQuery {\n show(id: \"anderson-fine-art-gallery-flickinger-collection\") {\n ...Show_show\n __id\n }\n}\n\nfragment Show_show on Show {\n ...Detail_show\n ...MoreInfo_show\n ...ShowArtists_show\n ...ShowArtworks_show\n __id\n}\n\nfragment Detail_show on Show {\n _id\n id\n name\n description\n city\n is_local_discovery\n images {\n id\n }\n ...ShowHeader_show\n ...ShowArtworksPreview_show\n ...ShowArtistsPreview_show\n ...Shows_show\n location {\n ...LocationMap_location\n openingHours {\n __typename\n ... on OpeningHoursArray {\n schedules {\n days\n hours\n }\n }\n ... on OpeningHoursText {\n text\n }\n }\n __id\n }\n artists_without_artworks {\n id\n __id\n }\n counts {\n artworks\n artists\n }\n status\n partner {\n __typename\n ... on Partner {\n name\n type\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n __id\n}\n\nfragment MoreInfo_show on Show {\n _id\n id\n exhibition_period\n pressReleaseUrl\n openingReceptionText\n partner {\n __typename\n ... on Partner {\n website\n type\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n press_release\n events {\n ...ShowEventSection_event\n }\n __id\n}\n\nfragment ShowArtists_show on Show {\n _id\n id\n artists_grouped_by_name {\n letter\n items {\n ...ArtistListItem_artist\n sortable_id\n href\n __id\n }\n }\n __id\n}\n\nfragment ShowArtworks_show on Show {\n __id\n id\n _id\n filteredArtworks(size: 0, medium: \"*\", price_range: \"*-*\", aggregations: [MEDIUM, PRICE_RANGE, TOTAL]) {\n ...FilteredInfiniteScrollGrid_filteredArtworks\n __id\n }\n}\n\nfragment FilteredInfiniteScrollGrid_filteredArtworks on FilterArtworks {\n ...Filters_filteredArtworks\n ...ArtworksGridPaginationContainer_filteredArtworks\n __id\n}\n\nfragment Filters_filteredArtworks on FilterArtworks {\n aggregations {\n slice\n counts {\n id\n name\n __id\n }\n }\n __id\n}\n\nfragment ArtworksGridPaginationContainer_filteredArtworks on FilterArtworks {\n __id\n artworks: artworks_connection(first: 10) {\n pageInfo {\n hasNextPage\n startCursor\n endCursor\n }\n edges {\n node {\n id\n __id\n image {\n aspect_ratio\n }\n ...Artwork_artwork\n __typename\n }\n cursor\n }\n }\n}\n\nfragment Artwork_artwork on Artwork {\n title\n date\n sale_message\n is_in_auction\n is_biddable\n is_acquireable\n is_offerable\n id\n sale {\n is_auction\n is_live_open\n is_open\n is_closed\n display_timely_at\n __id\n }\n sale_artwork {\n opening_bid {\n display\n }\n current_bid {\n display\n }\n bidder_positions_count\n sale {\n is_closed\n __id\n }\n __id\n }\n image {\n url(version: \"large\")\n aspect_ratio\n }\n artists(shallow: true) {\n name\n __id\n }\n partner {\n name\n __id\n }\n href\n __id\n}\n\nfragment ArtistListItem_artist on Artist {\n __id\n _id\n id\n name\n is_followed\n nationality\n birthday\n deathday\n image {\n url\n }\n}\n\nfragment ShowEventSection_event on PartnerShowEventType {\n event_type\n description\n start_at\n end_at\n}\n\nfragment ShowHeader_show on Show {\n id\n _id\n __id\n name\n press_release\n is_followed\n end_at\n exhibition_period\n status\n isStubShow\n partner {\n __typename\n ... on Partner {\n name\n id\n href\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n images {\n url\n aspect_ratio\n }\n followedArtists(first: 3) {\n edges {\n node {\n artist {\n name\n href\n id\n _id\n __id\n }\n }\n }\n }\n artists {\n name\n href\n id\n _id\n __id\n }\n}\n\nfragment ShowArtworksPreview_show on Show {\n __id\n artworks(size: 6) {\n ...GenericGrid_artworks\n __id\n }\n counts {\n artworks\n }\n}\n\nfragment ShowArtistsPreview_show on Show {\n _id\n id\n artists {\n _id\n id\n href\n ...ArtistListItem_artist\n __id\n }\n artists_without_artworks {\n _id\n id\n href\n ...ArtistListItem_artist\n __id\n }\n __id\n}\n\nfragment Shows_show on Show {\n nearbyShows(first: 20) {\n edges {\n node {\n __id\n ...ShowItem_show\n }\n }\n }\n __id\n}\n\nfragment LocationMap_location on Location {\n __id\n id\n city\n address\n address_2\n postal_code\n summary\n coordinates {\n lat\n lng\n }\n day_schedules {\n start_time\n end_time\n day_of_week\n }\n openingHours {\n __typename\n ... on OpeningHoursArray {\n schedules {\n days\n hours\n }\n }\n ... on OpeningHoursText {\n text\n }\n }\n}\n\nfragment ShowItem_show on Show {\n _id\n id\n name\n exhibition_period\n end_at\n images {\n url\n aspect_ratio\n }\n partner {\n __typename\n ... on Partner {\n name\n }\n ... on Node {\n __id\n }\n ... on ExternalPartner {\n __id\n }\n }\n __id\n}\n\nfragment GenericGrid_artworks on Artwork {\n __id\n id\n image {\n aspect_ratio\n }\n ...Artwork_artwork\n}\n" -} \ No newline at end of file +} diff --git a/src/data/vortex.graphql b/src/data/vortex.graphql index 27bb53ad66..2b83ca9f96 100644 --- a/src/data/vortex.graphql +++ b/src/data/vortex.graphql @@ -173,7 +173,12 @@ type Query { """ Pricing Context Histograms """ - pricingContext(artistId: String!, category: PricingContextCategoryEnum!, heightCm: Int!, widthCm: Int!): PricingContext + pricingContext( + artistId: String! + category: PricingContextCategoryEnum! + heightCm: Int! + widthCm: Int! + ): PricingContext } enum QueryPeriodEnum { diff --git a/src/lib/cache.ts b/src/lib/cache.ts index f1d043107e..b273ccf4ab 100644 --- a/src/lib/cache.ts +++ b/src/lib/cache.ts @@ -134,7 +134,9 @@ function _set(key, data, options: CacheOptions) { const timestamp = new Date().getTime() /* eslint-disable no-param-reassign */ if (isArray(data)) { - data.forEach(datum => (datum.cached = timestamp)) + data.forEach(datum => { + datum && (datum.cached = timestamp) + }) } else { data.cached = timestamp } diff --git a/src/lib/helpers.ts b/src/lib/helpers.ts index 4d7f679db0..68b2ebacbe 100644 --- a/src/lib/helpers.ts +++ b/src/lib/helpers.ts @@ -78,10 +78,19 @@ export const truncate = (string, length, append = "…") => { return x.length > limit ? x.slice(0, limit) + append : x } export const toQueryString = (options = {}) => - stringify(options, { - arrayFormat: "brackets", - sort: (a, b) => a.localeCompare(b), - }) + /** + * In the case of batched requests we want to explicitly _not_ sort the + * params because the order matters to dataloader + */ + // @ts-ignore + options.batched + ? stringify(options, { + arrayFormat: "brackets", + }) + : stringify(options, { + arrayFormat: "brackets", + sort: (a, b) => a.localeCompare(b), + }) export const toKey = (path, options = {}) => `${path}?${toQueryString(options)}` export const exclude = (values?: any[], property?: any) => xs => reject(xs, x => includes(values, x[property])) diff --git a/src/lib/loaders/__tests__/batchLoader.test.ts b/src/lib/loaders/__tests__/batchLoader.test.ts index af9dad2111..aea5cf5ce6 100644 --- a/src/lib/loaders/__tests__/batchLoader.test.ts +++ b/src/lib/loaders/__tests__/batchLoader.test.ts @@ -58,11 +58,11 @@ describe("batchLoader", () => { ) const batch = batchLoader({ multipleLoader }) - await batch({ id: "foo", param: "bar" }) + await batch({ id: ["foo"], param: "bar" }) expect(multipleLoader).toBeCalledWith({ + batched: true, id: ["foo"], param: "bar", - size: 1, }) }) @@ -79,55 +79,58 @@ describe("batchLoader", () => { expect(multipleLoader).toBeCalledTimes(1) }) + }) +}) - it("should return a default value when an id is missing", async () => { - const multipleLoader = jest.fn(({ id: ids }) => - ids.slice(1).map(id => ({ - _id: id, - })) - ) - const batch = batchLoader({ multipleLoader, defaultResult: {} }) - - const results = await Promise.all([ - batch("123"), - batch("456"), - batch("789"), - ]) - expect(results).toEqual([ - {}, - { - _id: "456", - }, - { - _id: "789", - }, - ]) +describe("serializeParams", () => { + const { serializeParams } = require("../batchLoader") - expect(multipleLoader).toBeCalledWith({ - id: ["123", "456", "789"], - size: 3, - }) - }) + it("should return an empty string for a param object with only id", () => { + expect(serializeParams({ id: "a" })).toBe("") + }) + it("should return key=value for every entry in the params object", () => { + expect(serializeParams({ id: "a", foo: "bar", bleep: "bloop" })).toBe( + "bleep=bloop&foo=bar" + ) }) }) -const { groupKeys } = require("../batchLoader") -describe("groupKeys", () => { +describe("groupByParams", () => { + const { groupByParams } = require("../batchLoader") it("should handle a single key", () => { - expect(groupKeys(["a"])).toEqual([ - { - id: ["a"], - size: 1, - }, + expect(groupByParams([{ id: "a" }])).toEqual([ + [""], + [ + [ + { + id: "a", + }, + ], + ], ]) }) it("should group single keys together", () => { - expect(groupKeys(["a", "b", "c"])).toEqual([ + const keys = [ { - id: ["a", "b", "c"], - size: 3, + id: "a", }, + { + id: "b", + }, + ] + expect(groupByParams(keys)).toEqual([ + [""], + [ + [ + { + id: "a", + }, + { + id: "b", + }, + ], + ], ]) }) @@ -142,38 +145,75 @@ describe("groupKeys", () => { foo: "bar", }, ] - expect(groupKeys(keys)).toEqual([ - { - id: ["a", "b"], - foo: "bar", - size: 2, - }, + expect(groupByParams(keys)).toEqual([ + ["foo=bar"], + [ + [ + { + id: "a", + foo: "bar", + }, + + { + id: "b", + foo: "bar", + }, + ], + ], ]) }) it("should separate keys with different parameters", () => { const keys = [ - "a", + { id: "a" }, { id: "b", foo: "bar" }, { id: "c", foo: "bar", boo: "baz" }, ] - expect(groupKeys(keys)).toEqual([ - { - id: ["a"], - size: 1, - }, - { - id: ["b"], - foo: "bar", - size: 1, - }, - { - id: ["c"], - foo: "bar", - boo: "baz", - size: 1, - }, + expect(groupByParams(keys)).toEqual([ + ["", "foo=bar", "boo=baz&foo=bar"], + [ + [ + { + id: "a", + }, + ], + [ + { + id: "b", + foo: "bar", + }, + ], + [ + { + id: "c", + foo: "bar", + boo: "baz", + }, + ], + ], ]) }) }) + +describe("cacheKeyFn", () => { + const { cacheKeyFn } = require("../batchLoader") + it("should not treat two objects with the same id but different params as different", () => { + expect(cacheKeyFn({ id: "123" })).not.toEqual( + cacheKeyFn({ id: "123", foo: "bar" }) + ) + }) + + it("should treat two objects with the same params and id as equal", () => { + expect(cacheKeyFn({ id: "123", foo: "bar" })).toEqual( + cacheKeyFn({ id: "123", foo: "bar" }) + ) + }) + + it("should treat two objects with different ids as different", () => { + expect(cacheKeyFn({ id: "456", foo: "bar" })).not.toEqual({ + id: "567", + foo: "bar", + }) + }) +}) diff --git a/src/lib/loaders/batchLoader.ts b/src/lib/loaders/batchLoader.ts index 56d6d81d9a..c85f7e0428 100644 --- a/src/lib/loaders/batchLoader.ts +++ b/src/lib/loaders/batchLoader.ts @@ -1,19 +1,40 @@ import DataLoader from "dataloader" -import { chain, flatten, chunk } from "lodash" +import { chain } from "lodash" import config from "config" import { StaticPathLoader, DynamicPathLoader } from "./api/loader_interface" const { ENABLE_RESOLVER_BATCHING } = config +interface IdWithParams { + id: string + [key: string]: any +} + +type SerializedParams = string[] + +interface BatchedParams { + id: string[] + [key: string]: any +} + /** - * This is just used to compare options sent into the dataloader. - * If the options are an object this stingifies the object in a way - * that they can be grouped as similar. + * This function is used by the dataloader to determine the uniqueness of keys. + * Because keys can be objects, it's important to be able to differentiate between + * their contents. If an object is empty except for an id field, we just use id. + * Otherwise we stringify the object so it can be compared via strict comparison. */ -const renderParams = key => { - if (typeof key === "string") { - return "" +export const cacheKeyFn = (key: IdWithParams): string => { + // If id is the *only* property of the key object + if (Object.keys(key).length === 1) { + return key.id } + return JSON.stringify(key) +} + +/** + * This is just used to compare options sent into the dataloader. + */ +export const serializeParams = (key: IdWithParams) => { const { id, ...params } = key return Object.entries(params) .map(entry => entry.join("=")) @@ -21,35 +42,43 @@ const renderParams = key => { .join("&") } -interface GroupKeysResult { - id: string[] - size: number - [key: string]: any -} -export const groupKeys = ( - requestedKeys: string | { id } -): GroupKeysResult[] => { - const grouped = chain(requestedKeys) - .groupBy(renderParams) - .values() - .map(values => chunk(values, 20)) - .flatten() - .map((keys: string[] | { id }[]) => { - if (typeof keys[0] === "string") { - return { id: keys, size: keys.length } - } - return { - // @ts-ignore - ...keys[0], - id: keys.map(k => k.id), - size: keys.length, +/** + * A tuple containing a list of stringify params (minus id) and + * an array of arrays of params with Id. Each of the inner arrays + * contain param objects that have all the same params besides id. + */ +type ParamGrouping = [SerializedParams, IdWithParams[][]] +/** + * Takes an array of parameter objects and groups them by all their params except for id. + * @returns a tuple of an array of group strings and an array of grouped keys + */ +export const groupByParams = (params: IdWithParams[]): ParamGrouping => { + const paramGrouping: ParamGrouping = chain(params) + .groupBy(serializeParams) + .entries() + .thru(entries => { + const result: ParamGrouping = [[], []] + for (let i = 0; i < entries.length; ++i) { + result[0].push(entries[i][0]) + result[1].push(entries[i][1]) } + return result }) .value() - return (grouped as unknown) as GroupKeysResult[] + return paramGrouping } +/** + * Takes an array of like parameters with different ids and joins them into one + * object with an array of ids. This is the format that gravity's list endpoints + * consume. + */ +export const flattenedParams = (keys: IdWithParams[]): BatchedParams => ({ + ...keys[0], + id: keys.map(key => key.id), +}) + interface BatchLoaderArgs { /** * For resolvers designed to hit a list endpoint @@ -59,39 +88,89 @@ interface BatchLoaderArgs { */ singleLoader?: any multipleLoader: any - defaultResult?: any } export const batchLoader = ({ singleLoader, multipleLoader, - defaultResult = null, }: BatchLoaderArgs) => { if (!ENABLE_RESOLVER_BATCHING) { return singleLoader ? singleLoader : multipleLoader } - const dl = new DataLoader(keys => { - let groupedKeys = groupKeys(keys as any) - - return Promise.all( - groupedKeys.map(keys => { - if (keys.id.length === 1 && singleLoader) { - return singleLoader(keys.id[0]) - } else { - return multipleLoader(keys) - } - }) - ).then(data => { - const normalizedResults = data.map((queriedGroup, groupIndex) => { - return groupedKeys[groupIndex].id.map( - id => queriedGroup.find(r => r._id === id) || defaultResult - ) + const dl = new DataLoader( + async (idWithParamsList: IdWithParams[]) => { + const [paramGroups, groupedParams] = groupByParams(idWithParamsList) + const data = await Promise.all( + groupedParams.map(flattenedParams).map(params => { + if ( + params.id.length === 1 && + singleLoader && + Object.keys(params).length === 1 + ) { + return singleLoader(params.id[0]) + } else { + return multipleLoader({ ...params, batched: true }) + } + }) + ) + const normalizedData = data.map( + datum => (Array.isArray(datum) ? datum.reverse() : [datum]) + ) + const results = idWithParamsList.map(params => { + const paramGroup = serializeParams(params) + const groupIndex = paramGroups.indexOf(paramGroup) + return normalizedData[groupIndex].pop() }) - return flatten(normalizedResults) - }) - }) + return results + }, + { + maxBatchSize: 20, + cacheKeyFn, + } + ) + + type Key = string | { id: string[] } + + /** + * This is an abstraction around the data loader to keep api parity with the + * existing gravityLoader api + */ + return (key: Key) => { + /** + * This section covers the case when an endpoint is being requested that supports + * parameters. An example would be the `sales` endpoint which has filters like + * `live` or `is_auction`. + * + * The assumption here is that things are being requested from a list endpoint + * so the results are always formatted into an array + */ + if (typeof key === "object" && key !== null) { + if (key.id.length === 1) { + return dl + .load({ ...key, id: key.id[0] }) + .then( + result => (result === undefined || result === null ? [] : [result]) + ) + } - return key => dl.load(key) + return dl + .loadMany( + key.id.map(id => ({ + ...key, + id: id, + })) + ) + .then(results => results.filter(result => result !== null)) + /** + * This section covers the case when a single resource endpoint is being requested. + * Take the `sale` endpoint for example. `key` is expected to be a string of `id` or `slug`. + */ + } else if (typeof key === "string") { + return dl.load({ id: key }) + } else { + throw new Error("Requested an invalid key type for batchLoader") + } + } } type PathLoader = StaticPathLoader | DynamicPathLoader @@ -105,23 +184,17 @@ export const createBatchLoaders = < >({ singleLoader, multipleLoader, - singleDefault, - multipleDefault, }: { singleLoader: SL multipleLoader: ML - singleDefault?: any - multipleDefault?: any }): [SL, ML] => { return [ batchLoader({ singleLoader, multipleLoader, - defaultResult: singleDefault, }), batchLoader({ multipleLoader, - defaultResult: multipleDefault, }), ] } diff --git a/src/lib/loaders/loaders_without_authentication/gravity.ts b/src/lib/loaders/loaders_without_authentication/gravity.ts index 9e46a844ae..5da4e8770b 100644 --- a/src/lib/loaders/loaders_without_authentication/gravity.ts +++ b/src/lib/loaders/loaders_without_authentication/gravity.ts @@ -11,9 +11,7 @@ export default opts => { const [batchSaleLoader, batchSalesLoader] = createBatchLoaders({ singleLoader: gravityLoader(id => `sale/${id}`), - multipleLoader: gravityLoader<{ id: string, is_auction: boolean }[]>('sales'), - singleDefault: null, - multipleDefault: [] + multipleLoader: gravityLoader<{ id: string, is_auction: boolean }[]>('sales') }) return { diff --git a/src/schema/object_identification.ts b/src/schema/object_identification.ts index abc296f950..3d73350eba 100644 --- a/src/schema/object_identification.ts +++ b/src/schema/object_identification.ts @@ -156,8 +156,9 @@ export const GlobalIDField = { type: new GraphQLNonNull(GraphQLID), // Ensure we never encode a null `id`, as it would silently work. Instead return `null`, so that // e.g. Relay will complain about the result not matching the type specified in the schema. - resolve: (obj, _args, _request, info) => - obj.id && toGlobalId(info.parentType.name, obj.id), + resolve: (obj, _args, _request, info) => { + return obj.id && toGlobalId(info.parentType.name, obj.id) + }, } export const IDFields = {