Skip to content

Commit

Permalink
fix(gatsby-source-contentful): handle backreferences on data updates …
Browse files Browse the repository at this point in the history 
…properly (#35214) (#35269)

Co-authored-by: axe312ger <opensource@axe312.dev>
(cherry picked from commit cf98027)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
  • Loading branch information
@gatsbybot @pieh
gatsbybot and pieh authored Mar 31, 2022
1 parent a527891 commit b5f0197
Show file tree
Hide file tree
Showing 3 changed files with 243 additions and 52 deletions.
112 changes: 101 additions & 11 deletions packages/gatsby-source-contentful/src/__tests__/gatsby-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,47 @@ const createMockCache = () => {
}

describe(`gatsby-node`, () => {
function deleteDescendants(node) {
if (!node) {
return
}

for (const childId of node.children ?? []) {
const child = currentNodeMap.get(childId)
if (child) {
deleteDescendants(child)
currentNodeMap.delete(child.id)
}
}
}

const actions = {
createTypes: jest.fn(),
setPluginStatus: jest.fn(),
createNode: jest.fn(async node => {
// similar checks as gatsby does
if (!_.isPlainObject(node)) {
throw new Error(
`The node passed to the "createNode" action creator must be an object`
)
}

if (node.internal.owner) {
throw new Error(
`The node internal.owner field is set automatically by Gatsby and not by plugins`
)
}

// recursively delete children of node if it existed already
deleteDescendants(currentNodeMap.get(node.id))

node.internal.owner = `gatsby-source-contentful`
currentNodeMap.set(node.id, node)
}),
deleteNode: jest.fn(node => {
// recursively delete children of deleted node same as gatsby
deleteDescendants(node)

currentNodeMap.delete(node.id)
}),
touchNode: jest.fn(),
Expand Down Expand Up @@ -80,8 +113,12 @@ describe(`gatsby-node`, () => {
const parentSpan = {}
const createNodeId = jest.fn(value => value)
let currentNodeMap
const getNodes = () => Array.from(currentNodeMap.values())
const getNode = id => currentNodeMap.get(id)

// returning clones of nodes to test if we are updating nodes correctly
// as it's not enough to mutate a node, we need to call an update function
// to actually update datastore
const getNodes = () => Array.from(currentNodeMap.values()).map(_.cloneDeep)
const getNode = id => _.cloneDeep(currentNodeMap.get(id))

const getFieldValue = (value, locale, defaultLocale) =>
value[locale] ?? value[defaultLocale]
Expand Down Expand Up @@ -179,7 +216,9 @@ describe(`gatsby-node`, () => {
const linkedNode = getNode(linkId)
reference[referenceKey] =
reference[referenceKey] || linkedNode[referenceKey] || []
reference[referenceKey].push(nodeId)
if (!reference[referenceKey].includes(nodeId)) {
reference[referenceKey].push(nodeId)
}
references.set(linkId, reference)
}
break
Expand Down Expand Up @@ -549,7 +588,7 @@ describe(`gatsby-node`, () => {
expect(getNode(blogEntry[`author___NODE`])).toBeTruthy()
})

expect(actions.createNode).toHaveBeenCalledTimes(42)
expect(actions.createNode).toHaveBeenCalledTimes(46)
expect(actions.deleteNode).toHaveBeenCalledTimes(0)
expect(actions.touchNode).toHaveBeenCalledTimes(32)
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
Expand All @@ -564,7 +603,7 @@ describe(`gatsby-node`, () => {
"Contentful: 0 deleted entries",
],
Array [
"Contentful: 11 cached entries",
"Contentful: 4 cached entries",
],
Array [
"Contentful: 1 new assets",
Expand Down Expand Up @@ -639,7 +678,7 @@ describe(`gatsby-node`, () => {
expect(getNode(blogEntry[`author___NODE`])).toBeTruthy()
})

expect(actions.createNode).toHaveBeenCalledTimes(50)
expect(actions.createNode).toHaveBeenCalledTimes(54)
expect(actions.deleteNode).toHaveBeenCalledTimes(0)
expect(actions.touchNode).toHaveBeenCalledTimes(72)
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
Expand All @@ -654,7 +693,7 @@ describe(`gatsby-node`, () => {
"Contentful: 0 deleted entries",
],
Array [
"Contentful: 14 cached entries",
"Contentful: 5 cached entries",
],
Array [
"Contentful: 0 new assets",
Expand Down Expand Up @@ -701,6 +740,23 @@ describe(`gatsby-node`, () => {
// initial sync
await simulateGatsbyBuild()

for (const author of getNodes().filter(
n => n.internal.type === `ContentfulPerson`
)) {
expect(author[`blog post___NODE`].length).toEqual(3)
expect(author[`blog post___NODE`]).toEqual(
expect.not.arrayContaining([
makeId({
spaceId: removedBlogEntry.sys.space.sys.id,
currentLocale: author.node_locale,
defaultLocale: locales[0],
id: removedBlogEntry.sys.id,
type: normalizedType,
}),
])
)
}

// create blog post
await simulateGatsbyBuild()

Expand All @@ -712,6 +768,23 @@ describe(`gatsby-node`, () => {
expect(blogEntry).not.toBeUndefined()
})

for (const author of getNodes().filter(
n => n.internal.type === `ContentfulPerson`
)) {
expect(author[`blog post___NODE`].length).toEqual(4)
expect(author[`blog post___NODE`]).toEqual(
expect.arrayContaining([
makeId({
spaceId: removedBlogEntry.sys.space.sys.id,
currentLocale: author.node_locale,
defaultLocale: locales[0],
id: removedBlogEntry.sys.id,
type: normalizedType,
}),
])
)
}

// remove blog post
reporter.info.mockClear()
await simulateGatsbyBuild()
Expand All @@ -734,14 +807,31 @@ describe(`gatsby-node`, () => {
)
)

for (const author of getNodes().filter(
n => n.internal.type === `ContentfulPerson`
)) {
expect(author[`blog post___NODE`].length).toEqual(3)
expect(author[`blog post___NODE`]).toEqual(
expect.not.arrayContaining([
makeId({
spaceId: removedBlogEntry.sys.space.sys.id,
currentLocale: author.node_locale,
defaultLocale: locales[0],
id: removedBlogEntry.sys.id,
type: normalizedType,
}),
])
)
}

// check if references are gone
authorIds.forEach(authorId => {
expect(getNode(authorId)[`blog post___NODE`]).toEqual(
expect.not.arrayContaining(deletedEntryIds)
)
})

expect(actions.createNode).toHaveBeenCalledTimes(44)
expect(actions.createNode).toHaveBeenCalledTimes(52)
expect(actions.deleteNode).toHaveBeenCalledTimes(2)
expect(actions.touchNode).toHaveBeenCalledTimes(74)
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
Expand All @@ -756,7 +846,7 @@ describe(`gatsby-node`, () => {
"Contentful: 1 deleted entries",
],
Array [
"Contentful: 14 cached entries",
"Contentful: 5 cached entries",
],
Array [
"Contentful: 0 new assets",
Expand Down Expand Up @@ -827,7 +917,7 @@ describe(`gatsby-node`, () => {
locales
)

expect(actions.createNode).toHaveBeenCalledTimes(44)
expect(actions.createNode).toHaveBeenCalledTimes(54)
expect(actions.deleteNode).toHaveBeenCalledTimes(2)
expect(actions.touchNode).toHaveBeenCalledTimes(74)
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
Expand All @@ -842,7 +932,7 @@ describe(`gatsby-node`, () => {
"Contentful: 0 deleted entries",
],
Array [
"Contentful: 14 cached entries",
"Contentful: 5 cached entries",
],
Array [
"Contentful: 0 new assets",
Expand Down
19 changes: 6 additions & 13 deletions packages/gatsby-source-contentful/src/normalize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ function prepareTextNode(id, node, key, text) {
contentDigest: node.updatedAt,
},
sys: {
type: node.sys.type,
type: `TextNode`,
},
}

Expand All @@ -220,7 +220,7 @@ function prepareJSONNode(id, node, key, content) {
contentDigest: node.updatedAt,
},
sys: {
type: node.sys.type,
type: `JsonNode`,
},
}

Expand Down Expand Up @@ -384,7 +384,7 @@ export const createNodesForContentType = ({
)

const existingNode = getNode(entryNodeId)
if (existingNode?.internal?.contentDigest === entryItem.sys.updatedAt) {
if (existingNode?.updatedAt === entryItem.sys.updatedAt) {
// The Contentful model has `.sys.updatedAt` leading for an entry. If the updatedAt value
// of an entry did not change, then we can trust that none of its children were changed either.
return null
Expand Down Expand Up @@ -552,9 +552,7 @@ export const createNodesForContentType = ({
// of an entry did not change, then we can trust that none of its children were changed either.
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
const existingNode = getNode(textNodeId)
if (
existingNode?.internal?.contentDigest !== entryItem.sys.updatedAt
) {
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
const textNode = prepareTextNode(
textNodeId,
entryNode,
Expand Down Expand Up @@ -620,9 +618,7 @@ export const createNodesForContentType = ({
// of an entry did not change, then we can trust that none of its children were changed either.
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
const existingNode = getNode(jsonNodeId)
if (
existingNode?.internal?.contentDigest !== entryItem.sys.updatedAt
) {
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
const jsonNode = prepareJSONNode(
jsonNodeId,
entryNode,
Expand All @@ -649,10 +645,7 @@ export const createNodesForContentType = ({
// of an entry did not change, then we can trust that none of its children were changed either.
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
const existingNode = getNode(jsonNodeId)
if (
existingNode?.internal?.contentDigest !==
entryItem.sys.updatedAt
) {
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
const jsonNode = prepareJSONNode(
jsonNodeId,
entryNode,
Expand Down
Loading

0 comments on commit b5f0197

Please sign in to comment.