Skip to content

Commit b5f0197

Browse files
gatsbybotpieh
gatsbybot
and
pieh
authored
fix(gatsby-source-contentful): handle backreferences on data updates properly (#35214) (#35269)
Co-authored-by: axe312ger <[email protected]> (cherry picked from commit cf98027) Co-authored-by: Michal Piechowiak <[email protected]>
1 parent a527891 commit b5f0197

File tree

3 files changed

+243
-52
lines changed

3 files changed

+243
-52
lines changed

‎packages/gatsby-source-contentful/src/__tests__/gatsby-node.js

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,47 @@ const createMockCache = () => {
4040
}
4141

4242
describe(`gatsby-node`, () => {
43+
function deleteDescendants(node) {
44+
if (!node) {
45+
return
46+
}
47+
48+
for (const childId of node.children ?? []) {
49+
const child = currentNodeMap.get(childId)
50+
if (child) {
51+
deleteDescendants(child)
52+
currentNodeMap.delete(child.id)
53+
}
54+
}
55+
}
56+
4357
const actions = {
4458
createTypes: jest.fn(),
4559
setPluginStatus: jest.fn(),
4660
createNode: jest.fn(async node => {
61+
// similar checks as gatsby does
62+
if (!_.isPlainObject(node)) {
63+
throw new Error(
64+
`The node passed to the "createNode" action creator must be an object`
65+
)
66+
}
67+
68+
if (node.internal.owner) {
69+
throw new Error(
70+
`The node internal.owner field is set automatically by Gatsby and not by plugins`
71+
)
72+
}
73+
74+
// recursively delete children of node if it existed already
75+
deleteDescendants(currentNodeMap.get(node.id))
76+
4777
node.internal.owner = `gatsby-source-contentful`
4878
currentNodeMap.set(node.id, node)
4979
}),
5080
deleteNode: jest.fn(node => {
81+
// recursively delete children of deleted node same as gatsby
82+
deleteDescendants(node)
83+
5184
currentNodeMap.delete(node.id)
5285
}),
5386
touchNode: jest.fn(),
@@ -80,8 +113,12 @@ describe(`gatsby-node`, () => {
80113
const parentSpan = {}
81114
const createNodeId = jest.fn(value => value)
82115
let currentNodeMap
83-
const getNodes = () => Array.from(currentNodeMap.values())
84-
const getNode = id => currentNodeMap.get(id)
116+
117+
// returning clones of nodes to test if we are updating nodes correctly
118+
// as it's not enough to mutate a node, we need to call an update function
119+
// to actually update datastore
120+
const getNodes = () => Array.from(currentNodeMap.values()).map(_.cloneDeep)
121+
const getNode = id => _.cloneDeep(currentNodeMap.get(id))
85122

86123
const getFieldValue = (value, locale, defaultLocale) =>
87124
value[locale] ?? value[defaultLocale]
@@ -179,7 +216,9 @@ describe(`gatsby-node`, () => {
179216
const linkedNode = getNode(linkId)
180217
reference[referenceKey] =
181218
reference[referenceKey] || linkedNode[referenceKey] || []
182-
reference[referenceKey].push(nodeId)
219+
if (!reference[referenceKey].includes(nodeId)) {
220+
reference[referenceKey].push(nodeId)
221+
}
183222
references.set(linkId, reference)
184223
}
185224
break
@@ -549,7 +588,7 @@ describe(`gatsby-node`, () => {
549588
expect(getNode(blogEntry[`author___NODE`])).toBeTruthy()
550589
})
551590

552-
expect(actions.createNode).toHaveBeenCalledTimes(42)
591+
expect(actions.createNode).toHaveBeenCalledTimes(46)
553592
expect(actions.deleteNode).toHaveBeenCalledTimes(0)
554593
expect(actions.touchNode).toHaveBeenCalledTimes(32)
555594
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
@@ -564,7 +603,7 @@ describe(`gatsby-node`, () => {
564603
"Contentful: 0 deleted entries",
565604
],
566605
Array [
567-
"Contentful: 11 cached entries",
606+
"Contentful: 4 cached entries",
568607
],
569608
Array [
570609
"Contentful: 1 new assets",
@@ -639,7 +678,7 @@ describe(`gatsby-node`, () => {
639678
expect(getNode(blogEntry[`author___NODE`])).toBeTruthy()
640679
})
641680

642-
expect(actions.createNode).toHaveBeenCalledTimes(50)
681+
expect(actions.createNode).toHaveBeenCalledTimes(54)
643682
expect(actions.deleteNode).toHaveBeenCalledTimes(0)
644683
expect(actions.touchNode).toHaveBeenCalledTimes(72)
645684
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
@@ -654,7 +693,7 @@ describe(`gatsby-node`, () => {
654693
"Contentful: 0 deleted entries",
655694
],
656695
Array [
657-
"Contentful: 14 cached entries",
696+
"Contentful: 5 cached entries",
658697
],
659698
Array [
660699
"Contentful: 0 new assets",
@@ -701,6 +740,23 @@ describe(`gatsby-node`, () => {
701740
// initial sync
702741
await simulateGatsbyBuild()
703742

743+
for (const author of getNodes().filter(
744+
n => n.internal.type === `ContentfulPerson`
745+
)) {
746+
expect(author[`blog post___NODE`].length).toEqual(3)
747+
expect(author[`blog post___NODE`]).toEqual(
748+
expect.not.arrayContaining([
749+
makeId({
750+
spaceId: removedBlogEntry.sys.space.sys.id,
751+
currentLocale: author.node_locale,
752+
defaultLocale: locales[0],
753+
id: removedBlogEntry.sys.id,
754+
type: normalizedType,
755+
}),
756+
])
757+
)
758+
}
759+
704760
// create blog post
705761
await simulateGatsbyBuild()
706762

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

771+
for (const author of getNodes().filter(
772+
n => n.internal.type === `ContentfulPerson`
773+
)) {
774+
expect(author[`blog post___NODE`].length).toEqual(4)
775+
expect(author[`blog post___NODE`]).toEqual(
776+
expect.arrayContaining([
777+
makeId({
778+
spaceId: removedBlogEntry.sys.space.sys.id,
779+
currentLocale: author.node_locale,
780+
defaultLocale: locales[0],
781+
id: removedBlogEntry.sys.id,
782+
type: normalizedType,
783+
}),
784+
])
785+
)
786+
}
787+
715788
// remove blog post
716789
reporter.info.mockClear()
717790
await simulateGatsbyBuild()
@@ -734,14 +807,31 @@ describe(`gatsby-node`, () => {
734807
)
735808
)
736809

810+
for (const author of getNodes().filter(
811+
n => n.internal.type === `ContentfulPerson`
812+
)) {
813+
expect(author[`blog post___NODE`].length).toEqual(3)
814+
expect(author[`blog post___NODE`]).toEqual(
815+
expect.not.arrayContaining([
816+
makeId({
817+
spaceId: removedBlogEntry.sys.space.sys.id,
818+
currentLocale: author.node_locale,
819+
defaultLocale: locales[0],
820+
id: removedBlogEntry.sys.id,
821+
type: normalizedType,
822+
}),
823+
])
824+
)
825+
}
826+
737827
// check if references are gone
738828
authorIds.forEach(authorId => {
739829
expect(getNode(authorId)[`blog post___NODE`]).toEqual(
740830
expect.not.arrayContaining(deletedEntryIds)
741831
)
742832
})
743833

744-
expect(actions.createNode).toHaveBeenCalledTimes(44)
834+
expect(actions.createNode).toHaveBeenCalledTimes(52)
745835
expect(actions.deleteNode).toHaveBeenCalledTimes(2)
746836
expect(actions.touchNode).toHaveBeenCalledTimes(74)
747837
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
@@ -756,7 +846,7 @@ describe(`gatsby-node`, () => {
756846
"Contentful: 1 deleted entries",
757847
],
758848
Array [
759-
"Contentful: 14 cached entries",
849+
"Contentful: 5 cached entries",
760850
],
761851
Array [
762852
"Contentful: 0 new assets",
@@ -827,7 +917,7 @@ describe(`gatsby-node`, () => {
827917
locales
828918
)
829919

830-
expect(actions.createNode).toHaveBeenCalledTimes(44)
920+
expect(actions.createNode).toHaveBeenCalledTimes(54)
831921
expect(actions.deleteNode).toHaveBeenCalledTimes(2)
832922
expect(actions.touchNode).toHaveBeenCalledTimes(74)
833923
expect(reporter.info.mock.calls).toMatchInlineSnapshot(`
@@ -842,7 +932,7 @@ describe(`gatsby-node`, () => {
842932
"Contentful: 0 deleted entries",
843933
],
844934
Array [
845-
"Contentful: 14 cached entries",
935+
"Contentful: 5 cached entries",
846936
],
847937
Array [
848938
"Contentful: 0 new assets",

‎packages/gatsby-source-contentful/src/normalize.js

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ function prepareTextNode(id, node, key, text) {
196196
contentDigest: node.updatedAt,
197197
},
198198
sys: {
199-
type: node.sys.type,
199+
type: `TextNode`,
200200
},
201201
}
202202

@@ -220,7 +220,7 @@ function prepareJSONNode(id, node, key, content) {
220220
contentDigest: node.updatedAt,
221221
},
222222
sys: {
223-
type: node.sys.type,
223+
type: `JsonNode`,
224224
},
225225
}
226226

@@ -384,7 +384,7 @@ export const createNodesForContentType = ({
384384
)
385385

386386
const existingNode = getNode(entryNodeId)
387-
if (existingNode?.internal?.contentDigest === entryItem.sys.updatedAt) {
387+
if (existingNode?.updatedAt === entryItem.sys.updatedAt) {
388388
// The Contentful model has `.sys.updatedAt` leading for an entry. If the updatedAt value
389389
// of an entry did not change, then we can trust that none of its children were changed either.
390390
return null
@@ -552,9 +552,7 @@ export const createNodesForContentType = ({
552552
// of an entry did not change, then we can trust that none of its children were changed either.
553553
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
554554
const existingNode = getNode(textNodeId)
555-
if (
556-
existingNode?.internal?.contentDigest !== entryItem.sys.updatedAt
557-
) {
555+
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
558556
const textNode = prepareTextNode(
559557
textNodeId,
560558
entryNode,
@@ -620,9 +618,7 @@ export const createNodesForContentType = ({
620618
// of an entry did not change, then we can trust that none of its children were changed either.
621619
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
622620
const existingNode = getNode(jsonNodeId)
623-
if (
624-
existingNode?.internal?.contentDigest !== entryItem.sys.updatedAt
625-
) {
621+
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
626622
const jsonNode = prepareJSONNode(
627623
jsonNodeId,
628624
entryNode,
@@ -649,10 +645,7 @@ export const createNodesForContentType = ({
649645
// of an entry did not change, then we can trust that none of its children were changed either.
650646
// (That's why child nodes use the updatedAt of the parent node as their digest, too)
651647
const existingNode = getNode(jsonNodeId)
652-
if (
653-
existingNode?.internal?.contentDigest !==
654-
entryItem.sys.updatedAt
655-
) {
648+
if (existingNode?.updatedAt !== entryItem.sys.updatedAt) {
656649
const jsonNode = prepareJSONNode(
657650
jsonNodeId,
658651
entryNode,

0 commit comments

Comments
 (0)