Skip to content

Commit

Permalink
Fetch multiple CMS template pages instead of just the first (#2314)
Browse files Browse the repository at this point in the history
## What's the purpose of this pull request?

Currently, when trying to load contentTypes from the Headless CMS we
just fetch the first page and ignore the hasNextPage response from the
cmsClient.

## How it works?

This PR changes that to load all publishedContentTypes before returning.
This might be troublesome if a given account has thousands of content
types of the same type (PLP, for instance). We can either increase the
page size to make fewer requests or investigate if we can move the
filtering to the CMS Client.

## How to test it?

I've added tests, but this can be replicated by copying the
`server/cms/index.ts` code to a store's node_modules folder and you
should see the effect.
  • Loading branch information
gvc committed May 16, 2024
1 parent a066917 commit f783663
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"css-loader": "^6.7.1",
"deepmerge": "^4.3.1",
"draftjs-to-html": "^0.9.1",
"graphql": "^15.0.0",
"graphql": "^15.6.0",
"include-media": "^1.4.10",
"next": "^13.5.6",
"next-seo": "^6.4.0",
Expand Down
74 changes: 70 additions & 4 deletions packages/core/src/server/cms/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ import MissingContentError from 'src/sdk/error/MissingContentError'
import MultipleContentError from 'src/sdk/error/MultipleContentError'
import config from '../../../faststore.config'

type Cache<T> = {
[key: string]: { data: Array<T> }
}
type ExtraOptions = {
cmsClient?: ClientCMS
cache?: Cache<ContentData>
}

export type Options =
| Locator
| {
Expand Down Expand Up @@ -45,10 +53,68 @@ export const clientCMS = new ClientCMS({
tenant: config.api.storeId,
})

export const getCMSPage = async (options: Options) => {
return await (isLocator(options)
? clientCMS.getCMSPage(options).then((page) => ({ data: [page] }))
: clientCMS.getCMSPagesByContentType(options.contentType, options.filters))
/*
* This in memory cache exists because for each page (think category or department)
* we are fetching all the pages of the same content type from the headless CMS to
* find the one that matches the slug.
*
* So instead of making multiple request for the Headless CMS API for each page we make
* one for each content-type and reuse the results for the next page.
*
* Since we rebuild on a CMS publication the server will go away and will "invalidate"
* the cache
*/
const getCMSPageCache = {}

export const getCMSPage = async (
options: Options,
extraOptions?: ExtraOptions
) => {
const cmsClient = extraOptions?.cmsClient ?? clientCMS
const cache = extraOptions?.cache ?? getCMSPageCache

if (isLocator(options)) {
return await cmsClient
.getCMSPage(options)
.then((page) => ({ data: [page] }))
}

if (!cache[options.contentType]) {
const pages = []
let page = 1
const perPage = 10
const response = await cmsClient.getCMSPagesByContentType(
options.contentType,
{ ...options.filters, page: page, perPage }
)

pages.push(...response.data)

const totalPagesToFetch = Math.ceil(response.totalItems / perPage) // How many pages have content
const pagesToFetch = Array.from(
{ length: totalPagesToFetch - 1 }, // We want all those pages minus the first one that we fetched
(_, i) => i + 2 // + 1 because indices are 0 based, and + 1 because we already fetched the first
)

if (response.totalItems > pages.length) {
const restOfPages = await Promise.all(
pagesToFetch.map((i) =>
cmsClient.getCMSPagesByContentType(options.contentType, {
...options.filters,
page: i,
perPage,
})
)
)

restOfPages.forEach((response) => {
pages.push(...response.data)
})
}
cache[options.contentType] = { data: pages }
}

return cache[options.contentType]
}

export const getPage = async <T extends ContentData>(options: Options) => {
Expand Down
95 changes: 95 additions & 0 deletions packages/core/test/server/cms/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { clientCMS, getCMSPage } from '../../../src/server/cms'
import { jest } from '@jest/globals'
import { ContentData } from '@vtex/client-cms'

describe('CMS Integration', () => {
const mockData = (count = 1) => {
const data: ContentData[] = []
for (let i = 0; i < count; i = i + 1) {
data.push({
id: `data-id-${i}`,
name: `data-name-${i}`,
status: `data-status-${i}`,
type: `data-type-${i}`,
sections: [],
releaseId: `release-${i}`,
})
}
return data
}

describe('getCMSPage', () => {
it('returns the first page if there is only one page', async () => {
const mockFunction = jest.fn(() => {
return Promise.resolve({
data: mockData(3),
hasNextPage: false,
totalItems: 3,
})
})
clientCMS.getCMSPagesByContentType = mockFunction

const result = await getCMSPage(
{ contentType: 'plp' },
{ cmsClient: clientCMS }
)

expect(mockFunction.mock.calls.length).toBe(1)
expect(result.data.length).toBe(3)
})

it('loads multiple pages', async () => {
const mockFunction: jest.Mock<typeof clientCMS.getCMSPagesByContentType> =
jest.fn()

mockFunction.mockImplementationOnce(() => {
return Promise.resolve({
data: mockData(10),
hasNextPage: true,
totalItems: 15,
})
})
mockFunction.mockImplementationOnce(() => {
return Promise.resolve({
data: mockData(5),
hasNextPage: false,
totalItems: 15,
})
})

clientCMS.getCMSPagesByContentType = mockFunction

const result = await getCMSPage(
{ contentType: 'plp' },
{ cmsClient: clientCMS, cache: {} }
)

expect(mockFunction.mock.calls.length).toBe(2)
expect(result.data.length).toBe(15)
})

it('it makes no request if the cache is filled', async () => {
const mockFunction: jest.Mock<typeof clientCMS.getCMSPagesByContentType> =
jest.fn()

mockFunction.mockImplementationOnce(() => {
return Promise.resolve({
data: mockData(10),
hasNextPage: true,
totalItems: 15,
})
})

clientCMS.getCMSPagesByContentType = mockFunction

const cache = { plp: { data: [] } }
const result = await getCMSPage(
{ contentType: 'plp' },
{ cmsClient: clientCMS, cache: cache }
)

expect(mockFunction.mock.calls.length).toBe(0)
expect(result.data.length).toBe(0)
})
})
})
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9146,7 +9146,7 @@ graphql-ws@5.12.1:
resolved "https://registry.yarnpkg.com/graphql-ws/-/graphql-ws-5.12.1.tgz#c62d5ac54dbd409cc6520b0b39de374b3d59d0dd"
integrity sha512-umt4f5NnMK46ChM2coO36PTFhHouBrK9stWWBczERguwYrGnPNxJ9dimU6IyOBfOkC6Izhkg4H8+F51W/8CYDg==

graphql@^15.0.0, graphql@^15.6.0:
graphql@^15.6.0:
version "15.8.0"
resolved "https://registry.yarnpkg.com/graphql/-/graphql-15.8.0.tgz#33410e96b012fa3bdb1091cc99a94769db212b38"
integrity sha512-5gghUc24tP9HRznNpV2+FIoq3xKkj5dTQqf4v0CpdPbFVwFkWoxOM+o+2OC9ZSvjEMTjfmG9QT+gcvggTwW1zw==
Expand Down

0 comments on commit f783663

Please sign in to comment.