From 01502220fb903c62f0b33b9a703fb45daafae141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8Dcaro=20Azevedo?= Date: Mon, 21 Feb 2022 13:20:32 -0300 Subject: [PATCH] fix: allCollections query pagination and pagetype timeout errors (#1140) --- packages/api/package.json | 3 +- .../vtex/clients/commerce/types/Portal.ts | 21 +++++--- .../src/platforms/vtex/loaders/collection.ts | 50 +++++++++++++++++++ .../api/src/platforms/vtex/loaders/index.ts | 3 ++ .../platforms/vtex/resolvers/collection.ts | 32 ++++++------ .../api/src/platforms/vtex/resolvers/query.ts | 40 ++++++--------- packages/gatsby-source-store/src/paginate.ts | 11 ++-- yarn.lock | 7 ++- 8 files changed, 112 insertions(+), 55 deletions(-) create mode 100644 packages/api/src/platforms/vtex/loaders/collection.ts diff --git a/packages/api/package.json b/packages/api/package.json index 4d98bc8e43..54d3ab7a43 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -26,7 +26,8 @@ "@sindresorhus/slugify": "^1.1.2", "dataloader": "^2.0.0", "fast-deep-equal": "^3.1.3", - "isomorphic-unfetch": "^3.1.0" + "isomorphic-unfetch": "^3.1.0", + "p-limit": "^3.1.0" }, "devDependencies": { "@graphql-codegen/cli": "2.2.0", diff --git a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts index 2a589413ce..b6cee9ae6c 100644 --- a/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts +++ b/packages/api/src/platforms/vtex/clients/commerce/types/Portal.ts @@ -1,14 +1,19 @@ -export interface PortalPagetype { +export type PortalPagetype = CollectionPageType | FallbackPageType + +export interface CollectionPageType { id: number name: string url: string title: string metaTagDescription: string - pageType: - | 'Brand' - | 'Category' - | 'Department' - | 'Subcategory' - | 'FullText' - | 'NotFound' + pageType: 'Brand' | 'Category' | 'Department' | 'Subcategory' +} + +export interface FallbackPageType { + id: null + name: null | string + url: null | string + title: null + metaTagDescription: null + pageType: 'NotFound' | 'FullText' } diff --git a/packages/api/src/platforms/vtex/loaders/collection.ts b/packages/api/src/platforms/vtex/loaders/collection.ts new file mode 100644 index 0000000000..a1c66d9d10 --- /dev/null +++ b/packages/api/src/platforms/vtex/loaders/collection.ts @@ -0,0 +1,50 @@ +import DataLoader from 'dataloader' +import pLimit from 'p-limit' + +import { NotFoundError } from '../utils/errors' +import type { CollectionPageType } from '../clients/commerce/types/Portal' +import type { Options } from '..' +import type { Clients } from '../clients' + +// Limits concurrent requests to 20 so that they don't timeout +const CONCURRENT_REQUESTS_MAX = 20 + +const collectionPageTypes = new Set([ + 'brand', + 'category', + 'department', + 'subcategory', +] as const) + +export const isCollectionPageType = (x: any): x is CollectionPageType => + typeof x.pageType === 'string' && + collectionPageTypes.has(x.pageType.toLowerCase()) + +export const getCollectionLoader = (_: Options, clients: Clients) => { + const limit = pLimit(CONCURRENT_REQUESTS_MAX) + + const loader = async ( + slugs: readonly string[] + ): Promise => { + return Promise.all( + slugs.map((slug: string) => + limit(async () => { + const page = await clients.commerce.catalog.portal.pagetype(slug) + + if (isCollectionPageType(page)) { + return page + } + + throw new NotFoundError( + `Catalog returned ${page.pageType} for slug: ${slug}. This usually happens when there is more than one category with the same name in the same category tree level.` + ) + }) + ) + ) + } + + return new DataLoader(loader, { + // DataLoader is being used to cache requests, not to batch them + batch: false, + }) +} diff --git a/packages/api/src/platforms/vtex/loaders/index.ts b/packages/api/src/platforms/vtex/loaders/index.ts index aa86ea268a..b0f73e81a0 100644 --- a/packages/api/src/platforms/vtex/loaders/index.ts +++ b/packages/api/src/platforms/vtex/loaders/index.ts @@ -1,5 +1,6 @@ import { getSimulationLoader } from './simulation' import { getSkuLoader } from './sku' +import { getCollectionLoader } from './collection' import type { Context, Options } from '..' export type Loaders = ReturnType @@ -7,9 +8,11 @@ export type Loaders = ReturnType export const getLoaders = (options: Options, { clients }: Context) => { const skuLoader = getSkuLoader(options, clients) const simulationLoader = getSimulationLoader(options, clients) + const collectionLoader = getCollectionLoader(options, clients) return { skuLoader, simulationLoader, + collectionLoader, } } diff --git a/packages/api/src/platforms/vtex/resolvers/collection.ts b/packages/api/src/platforms/vtex/resolvers/collection.ts index 3dfd585fce..537156038d 100644 --- a/packages/api/src/platforms/vtex/resolvers/collection.ts +++ b/packages/api/src/platforms/vtex/resolvers/collection.ts @@ -1,22 +1,20 @@ +import { isCollectionPageType } from '../loaders/collection' import { slugify as baseSlugify } from '../utils/slugify' import type { Resolver } from '..' import type { Brand } from '../clients/commerce/types/Brand' import type { CategoryTree } from '../clients/commerce/types/CategoryTree' -import type { PortalPagetype } from '../clients/commerce/types/Portal' +import type { CollectionPageType } from '../clients/commerce/types/Portal' -type Root = Brand | (CategoryTree & { level: number }) | PortalPagetype +type Root = Brand | (CategoryTree & { level: number }) | CollectionPageType const isBrand = (x: any): x is Brand => x.type === 'brand' -const isPortalPageType = (x: any): x is PortalPagetype => - typeof x.pageType === 'string' - const slugify = (root: Root) => { if (isBrand(root)) { - return baseSlugify(root.name) + return baseSlugify(root.name.toLowerCase()) } - if (isPortalPageType(root)) { + if (isCollectionPageType(root)) { return new URL(`https://${root.url}`).pathname.slice(1) } @@ -27,7 +25,7 @@ export const StoreCollection: Record> = { id: ({ id }) => id.toString(), slug: (root) => slugify(root), seo: (root) => - isBrand(root) || isPortalPageType(root) + isBrand(root) || isCollectionPageType(root) ? { title: root.title, description: root.metaTagDescription, @@ -39,7 +37,7 @@ export const StoreCollection: Record> = { type: (root) => isBrand(root) ? 'Brand' - : isPortalPageType(root) + : isCollectionPageType(root) ? root.pageType : root.level === 0 ? 'Department' @@ -51,7 +49,7 @@ export const StoreCollection: Record> = { } : { selectedFacets: new URL( - isPortalPageType(root) ? `https://${root.url}` : root.url + isCollectionPageType(root) ? `https://${root.url}` : root.url ).pathname .slice(1) .split('/') @@ -62,7 +60,7 @@ export const StoreCollection: Record> = { }, breadcrumbList: async (root, _, ctx) => { const { - clients: { commerce }, + loaders: { collectionLoader }, } = ctx const slug = slugify(root) @@ -78,17 +76,17 @@ export const StoreCollection: Record> = { segments.slice(0, index + 1).join('/') ) - const pageTypes = await Promise.all( - slugs.map((s) => commerce.catalog.portal.pagetype(s)) + const collections = await Promise.all( + slugs.map((s) => collectionLoader.load(s)) ) return { - itemListElement: pageTypes.map((pageType, index) => ({ - item: new URL(`https://${pageType.url}`).pathname.toLowerCase(), - name: pageType.name, + itemListElement: collections.map((collection, index) => ({ + item: new URL(`https://${collection.url}`).pathname.toLowerCase(), + name: collection.name, position: index + 1, })), - numberOfItems: pageTypes.length, + numberOfItems: collections.length, } }, } diff --git a/packages/api/src/platforms/vtex/resolvers/query.ts b/packages/api/src/platforms/vtex/resolvers/query.ts index bcff325964..ecaef20f3a 100644 --- a/packages/api/src/platforms/vtex/resolvers/query.ts +++ b/packages/api/src/platforms/vtex/resolvers/query.ts @@ -1,5 +1,4 @@ import { enhanceSku } from '../utils/enhanceSku' -import { NotFoundError } from '../utils/errors' import { transformSelectedFacet } from '../utils/facets' import { SORT_MAP } from '../utils/sort' import { StoreCollection } from './collection' @@ -29,24 +28,12 @@ export const Query = { return skuLoader.load(locator.map(transformSelectedFacet)) }, - collection: async ( - _: unknown, - { slug }: QueryCollectionArgs, - ctx: Context - ) => { + collection: (_: unknown, { slug }: QueryCollectionArgs, ctx: Context) => { const { - clients: { commerce }, + loaders: { collectionLoader }, } = ctx - const result = await commerce.catalog.portal.pagetype(slug) - - const whitelist = ['Brand', 'Category', 'Department', 'Subcategory'] - - if (whitelist.includes(result.pageType)) { - return result - } - - throw new NotFoundError(`Not Found: ${slug}`) + return collectionLoader.load(slug) }, search: async ( _: unknown, @@ -100,6 +87,7 @@ export const Query = { endCursor: products.total.toString(), totalCount: products.total, }, + // after + index is bigger than after+first itself because of the array flat() above edges: skus.map((sku, index) => ({ node: sku, cursor: (after + index).toString(), @@ -140,20 +128,24 @@ export const Query = { ...categories, ] + const validCollections = collections + // Nullable slugs may cause one route to override the other + .filter((node) => Boolean(StoreCollection.slug(node, null, ctx, null))) + return { pageInfo: { - hasNextPage: false, - hasPreviousPage: false, + hasNextPage: validCollections.length - after > first, + hasPreviousPage: after > 0, startCursor: '0', - endCursor: '0', + endCursor: ( + Math.min(first, validCollections.length - after) - 1 + ).toString(), }, - edges: collections - // Nullable slugs may cause one route to override the other - .filter((node) => Boolean(StoreCollection.slug(node, null, ctx, null))) - .slice(after, first) + edges: validCollections + .slice(after, after + first) .map((node, index) => ({ node, - cursor: index.toString(), + cursor: (after + index).toString(), })), } }, diff --git a/packages/gatsby-source-store/src/paginate.ts b/packages/gatsby-source-store/src/paginate.ts index e492768bf8..a995e6bed6 100644 --- a/packages/gatsby-source-store/src/paginate.ts +++ b/packages/gatsby-source-store/src/paginate.ts @@ -22,11 +22,14 @@ export const RelayForward = ( } }, next(state, page) { - const tail = page.edges[page.edges.length - 1] - const first = - Number(state.variables.first) ?? Math.min(DEFAULT_PAGE_SIZE, maxItems) + const { first: rawFirst, after: maybeStateAfter } = state.variables - const after = tail?.cursor + const stateAfter = Number.isNaN(Number(maybeStateAfter)) + ? 0 + : Number(maybeStateAfter) + + const first = Number(rawFirst) ?? DEFAULT_PAGE_SIZE + const after = (stateAfter + first).toString() return { variables: { first, after }, diff --git a/yarn.lock b/yarn.lock index a86f5b59a4..09ea68ab42 100644 --- a/yarn.lock +++ b/yarn.lock @@ -30854,7 +30854,12 @@ ws@^7.3.0, ws@^7.3.1, ws@~7.4.2: resolved "https://registry.yarnpkg.com/ws/-/ws-7.4.5.tgz#a484dd851e9beb6fdb420027e3885e8ce48986c1" integrity sha512-xzyu3hFvomRfXKH8vOFMU3OguG6oOvhXMo3xsGy3xWExqaM2dxBbVxuD99O7m3ZUFMvvscsZDqxfgMaRr/Nr1g== -ws@^8.1.0, ws@^8.2.3: +ws@^8.1.0: + version "8.5.0" + resolved "https://registry.yarnpkg.com/ws/-/ws-8.5.0.tgz#bfb4be96600757fe5382de12c670dab984a1ed4f" + integrity sha512-BWX0SWVgLPzYwF8lTzEy1egjhS4S4OEAHfsO8o65WOVsrnSRGaSiUaa9e0ggGlkMTtBlmOpEXiie9RUcBO86qg== + +ws@^8.2.3: version "8.3.0" resolved "https://registry.yarnpkg.com/ws/-/ws-8.3.0.tgz#7185e252c8973a60d57170175ff55fdbd116070d" integrity sha512-Gs5EZtpqZzLvmIM59w4igITU57lrtYVFneaa434VROv4thzJyV6UjIL3D42lslWlI+D4KzLYnxSwtfuiO79sNw==