Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows the node has changed (so knows to re-run queries) #33328

Merged
merged 1 commit into from
Sep 29, 2021
Merged

fix(gatsby-source-drupal): Clone nodes before adding back references so Gatsby knows the node has changed (so knows to re-run queries) #33328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions packages/gatsby-source-drupal/src/__tests__/fixtures/1593545806.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,38 @@
"id": "does-not-exist",
"type": "file--file"
},
{
"jsonapi": {
"version": "1.0",
"meta": {
"links": {
"self": {
"href": "http://jsonapi.org/format/1.0/"
}
}
}
},
"data": {
"type": "node--article",
"id": "article-9",
"attributes": {
"id": 24,
"uuid": "article-9",
"title": "Article #9 - new",
"body": "Aliquam non varius libero, sit amet consequat ex. Aenean porta turpis quis vulputate blandit. Suspendisse in porta erat. Sed sit amet scelerisque turpis, at rutrum mauris. Sed tempor eleifend lobortis. Proin maximus, massa sed dignissim sollicitudin, quam risus mattis justo, sit amet aliquam odio ligula quis urna. Interdum et malesuada fames ac ante ipsum primis in faucibus. Ut mollis leo nisi, at interdum urna fermentum ut. Fusce id suscipit neque, eu fermentum lacus. Donec egestas laoreet felis ac luctus. Vestibulum molestie mattis ante, a vulputate nunc ullamcorper at. Ut hendrerit ipsum eget gravida ultricies."
},
"relationships": {
"field_tags": {
"data": [
{
"type": "taxonomy_term--tags",
"id": "tag-2"
}
]
}
}
}
},
{
"jsonapi": {
"version": "1.0",
Expand Down
4 changes: 4 additions & 0 deletions packages/gatsby-source-drupal/src/__tests__/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@ describe(`gatsby-source-drupal`, () => {
expect(
nodes[createNodeId(`und.tag-2`)].relationships[`node__article___NODE`]
).toContain(createNodeId(`und.article-3`))
// Created a new node article-9 with reference to tag-2
expect(
nodes[createNodeId(`und.tag-2`)].relationships[`node__article___NODE`]
).toContain(createNodeId(`und.article-9`))
})
})
})
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby-source-drupal/src/gatsby-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ ${JSON.stringify(webhookBody, null, 4)}
nodes.forEach(node => {
handleReferences(node, {
getNode: nodes.get.bind(nodes),
mutateNode: true,
createNodeId,
entityReferenceRevisions,
})
Expand Down
84 changes: 49 additions & 35 deletions packages/gatsby-source-drupal/src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ const {

const { getOptions } = require(`./plugin-options`)

const backRefsNamesLookup = new WeakMap()
const referencedNodesLookup = new WeakMap()
const backRefsNamesLookup = new Map()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to regular maps w/ IDs as we're not holding consistent references to the same node object anymore.

const referencedNodesLookup = new Map()

const handleReferences = (
node,
{ getNode, createNodeId, entityReferenceRevisions = [] }
{ getNode, mutateNode = false, createNodeId, entityReferenceRevisions = [] }
) => {
const relationships = node.relationships
const rootNodeLanguage = getOptions().languageConfig ? node.langcode : `und`

const backReferencedNodes = []
if (node.drupal_relationships) {
const referencedNodes = []
_.each(node.drupal_relationships, (v, k) => {
Expand Down Expand Up @@ -68,6 +69,7 @@ const handleReferences = (
relationships[nodeFieldName] = referencedNodeId
referencedNodes.push(referencedNodeId)
}

// If there's meta on the field and it's not an existing/internal one
// create a new node's field with that meta. It can't exist on both
// @see https://jsonapi.org/format/#document-resource-object-fields
Expand All @@ -78,11 +80,16 @@ const handleReferences = (
})

delete node.drupal_relationships
referencedNodesLookup.set(node, referencedNodes)
referencedNodesLookup.set(node.id, referencedNodes)
if (referencedNodes.length) {
const nodeFieldName = `${node.internal.type}___NODE`
referencedNodes.forEach(nodeID => {
const referencedNode = getNode(nodeID)
let referencedNode
if (mutateNode) {
referencedNode = getNode(nodeID)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the initial sourcing, it does make sense to directly mutate nodes as none of them have been created yet.

} else {
referencedNode = _.cloneDeep(getNode(nodeID))
}
if (!referencedNode.relationships[nodeFieldName]) {
referencedNode.relationships[nodeFieldName] = []
}
Expand All @@ -91,20 +98,23 @@ const handleReferences = (
referencedNode.relationships[nodeFieldName].push(node.id)
}

let backRefsNames = backRefsNamesLookup.get(referencedNode)
let backRefsNames = backRefsNamesLookup.get(referencedNode.id)
if (!backRefsNames) {
backRefsNames = []
backRefsNamesLookup.set(referencedNode, backRefsNames)
backRefsNamesLookup.set(referencedNode.id, backRefsNames)
}

if (!backRefsNames.includes(nodeFieldName)) {
backRefsNames.push(nodeFieldName)
}
backReferencedNodes.push(referencedNode)
})
}
}

node.relationships = relationships

return backReferencedNodes
}

exports.handleReferences = handleReferences
Expand All @@ -117,7 +127,7 @@ const handleDeletedNode = async ({
createContentDigest,
entityReferenceRevisions,
}) => {
const deletedNode = getNode(
let deletedNode = getNode(
createNodeId(
createNodeIdWithVersion(
node.id,
Expand All @@ -135,20 +145,25 @@ const handleDeletedNode = async ({
return deletedNode
}

// Clone node so we're not mutating the original node.
deletedNode = _.cloneDeep(deletedNode)

// Remove the deleted node from backRefsNamesLookup and referencedNodesLookup
backRefsNamesLookup.delete(deletedNode)
referencedNodesLookup.delete(deletedNode)
backRefsNamesLookup.delete(deletedNode.id)
referencedNodesLookup.delete(deletedNode.id)

// Remove relationships from other nodes and re-create them.
Object.keys(deletedNode.relationships).forEach(key => {
let ids = deletedNode.relationships[key]
ids = [].concat(ids)
ids.forEach(id => {
const node = getNode(id)
let node = getNode(id)

// The referenced node might have already been deleted.
if (node) {
let referencedNodes = referencedNodesLookup.get(node)
// Clone node so we're not mutating the original node.
node = _.cloneDeep(node)
let referencedNodes = referencedNodesLookup.get(node.id)
if (referencedNodes?.includes(deletedNode.id)) {
// Loop over relationships and cleanup references.
Object.entries(node.relationships).forEach(([key, value]) => {
Expand All @@ -174,7 +189,7 @@ const handleDeletedNode = async ({
referencedNodes = referencedNodes.filter(
nId => nId !== deletedNode.id
)
referencedNodesLookup.set(node, referencedNodes)
referencedNodesLookup.set(node.id, referencedNodes)
}

// Recreate the referenced node with its now cleaned-up relationships.
Expand Down Expand Up @@ -230,33 +245,46 @@ ${JSON.stringify(nodeToUpdate, null, 4)}

const nodesToUpdate = [newNode]

handleReferences(newNode, {
const oldNodeReferencedNodes = referencedNodesLookup.get(newNode.id)
const backReferencedNodes = handleReferences(newNode, {
getNode,
mutateNode: false,
createNodeId,
entityReferenceRevisions: pluginOptions.entityReferenceRevisions,
})

const oldNode = getNode(newNode.id)
nodesToUpdate.push(...backReferencedNodes)

let oldNode = getNode(newNode.id)
if (oldNode) {
// Clone node so we're not mutating the original node.
oldNode = _.cloneDeep(oldNode)
// copy over back references from old node
const backRefsNames = backRefsNamesLookup.get(oldNode)
const backRefsNames = backRefsNamesLookup.get(oldNode.id)
if (backRefsNames) {
backRefsNamesLookup.set(newNode, backRefsNames)
backRefsNamesLookup.set(newNode.id, backRefsNames)
backRefsNames.forEach(backRefFieldName => {
newNode.relationships[backRefFieldName] =
oldNode.relationships[backRefFieldName]
})
}

const oldNodeReferencedNodes = referencedNodesLookup.get(oldNode)
const newNodeReferencedNodes = referencedNodesLookup.get(newNode)
const newNodeReferencedNodes = referencedNodesLookup.get(newNode.id)

// see what nodes are no longer referenced and remove backRefs from them
const removedReferencedNodes = _.difference(
let removedReferencedNodes = _.difference(
oldNodeReferencedNodes,
newNodeReferencedNodes
).map(id => getNode(id))

removedReferencedNodes = removedReferencedNodes.map(node => {
if (node) {
return _.cloneDeep(node)
} else {
return node
}
})

nodesToUpdate.push(...removedReferencedNodes)

const nodeFieldName = `${newNode.internal.type}___NODE`
Expand All @@ -271,23 +299,9 @@ ${JSON.stringify(nodeToUpdate, null, 4)}
)
}
})

// see what nodes are newly referenced, and make sure to call `createNode` on them
const addedReferencedNodes = _.difference(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't needed as it just duplicates what handleReferences does. We now return nodes with updated backreferences from that function so we can eliminate this code.

newNodeReferencedNodes,
oldNodeReferencedNodes
).map(id => getNode(id))

nodesToUpdate.push(...addedReferencedNodes)
} else {
// if we are inserting new node, we need to update all referenced nodes
const newNodes = referencedNodesLookup.get(newNode)
if (typeof newNodes !== `undefined`) {
newNodes.forEach(id => nodesToUpdate.push(getNode(id)))
}
}

// download file
// Download file.
const { skipFileDownloads } = pluginOptions
if (isFileNode(newNode) && !skipFileDownloads) {
await downloadFile(
Expand Down