Problem/Motivation

When there's a circular relationship between entity reference fields, the "gatsby_instantpreview" module will enter an infinite loop while saving the node. In the scenario where this was discovered, we had a content type which included taxonomy entity references, and those taxonomy terms had a computed field to display which content types referenced them. When saving one of the nodes that had a taxonomy term, an infinite recursion loop would be created.

Steps to reproduce

  • Enable the gatsby_instantpreview module
  • Create a new content type.
  • Create an entity reference field on the content type that can reference the same content type.
  • Go to the manage display tab of the content type and set the reference field to use the rendered entity display setting.
  • Create a new node in this content type.
  • Create a second node that references the first node.
  • Edit the first node and set it to reference the second node.
  • After re-saving the first node, the page should timeout due to the infinite loop caused by the circular references.

Proposed resolution

This appears to be caused by the `buildRelationshipJson()` method in the `GatsbyInstantPreview` class. At an initial read through it shouldn't be an issue, as there's a pass by reference `$entity_data` array that should be able to determine if a given entity has already been handled and exit early. However, that doesn't appear to be the case. I haven't done a deep analysis on why that's the case, but I have created a patch that adds an additional (optional) `$previous_items` array that behaves similar and will exit early.

Remaining tasks

Review the patch and investigate if an alternative solution should be used. One possible difference is that in my patch I'm updating the array prior to a continue statement that may be preventing the expected behavior otherwise.

User interface changes

n/a

API changes

n/a

Data model changes

n/a

CommentFileSizeAuthor
#8 3175899-pass.patch2.61 KBlarowlan
#8 3175899-fail.patch1.73 KBlarowlan
#7 3175899-gatsby-preview-recursion-07.patch841 bytesnickmans
#4 3175899-gatsby-preview-recursion-04.patch1.42 KBiansholtys
#3 3175899-gatsby-preview-recursion-03.patch1.48 KBAsacolips
#2 3175899-gatsby-preview-recursion.patch2 KBAsacolips
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Asacolips created an issue. See original summary.

Asacolips’s picture

Asacolips CreditAttribution: Asacolips at Mediacurrent commented
FileSize
3175899-gatsby-preview-recursion.patch2 KB
Asacolips’s picture

Asacolips CreditAttribution: Asacolips at Mediacurrent commented
FileSize
3175899-gatsby-preview-recursion-03.patch1.48 KB

Rerolled the patch against the latest dev and refactored to clean up the approach a bit. This also resolves an error with buildRelationshipJson() being called elsewhere with two arguments instead of three.

iansholtys’s picture

iansholtys CreditAttribution: iansholtys at Mediacurrent commented
FileSize
3175899-gatsby-preview-recursion-04.patch1.42 KB

Noticed an issue with #3 when using fastbuilds.

The value was set to TRUE before checking to see if a valid entity could be loaded. If the uuid is "virtual", as can be seen with the parent relationship on a root-level taxonomy term, $entity_data will contain a mapping of 'virtual' to TRUE. Fastbuilds will include TRUE as one of the "data" objects, which doesn't make much sense.

Simple fix is to only add the placeholder after we know for sure it's actually an entity.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Active » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for Service NSW commented
Priority: Normal » Major
Issue tags: +Needs tests
nickmans’s picture

nickmans CreditAttribution: nickmans at The Reference commented
FileSize
3175899-gatsby-preview-recursion-07.patch841 bytes

I think we can have an alternative approach much easier. I think that the logic is already implemented, but there's a bug. We should add the processed entity to the array before building the relationships.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for Service NSW commented
Issue tags: -Needs tests
FileSize
3175899-fail.patch1.73 KB
3175899-pass.patch2.61 KB

Here's a test, interdiff is the fail patch

  • larowlan committed 1946f4f on 8.x-1.x 
    Issue #3175899 by Asacolips, larowlan, iansholtys, nickmans: Infinite...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for Service NSW commented
Status: Needs review » Fixed

As expected, the fail patch doesn't complete, it just loops endlessly

I killed it to save $$$ on drupalci.

Fixed on commit 

--- a/modules/gatsby_instantpreview/tests/src/Kernel/GatsbyInstantPreviewTest.php
+++ b/modules/gatsby_instantpreview/tests/src/Kernel/GatsbyInstantPreviewTest.php
@@ -66,7 +66,7 @@ class GatsbyInstantPreviewTest extends KernelTestBase {
     $this->setUpCurrentUser([], [
       'access content',
     ]);
-    $this->createEntityReferenceField('node', 'page', 'field_reference', 'Referencs','node');
+    $this->createEntityReferenceField('node', 'page', 'field_reference', 'Referencs', 'node');
   }

Thanks all

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

teemuaro’s picture

teemuaro CreditAttribution: teemuaro commented

We had the same issue with circular relationships with rc3. Updating to latest -dev fixed the issue that nodes are not saved, but caused a new problem that cloud builds are not triggered, with the error in logs

Notice: Undefined variable: preview_path funktiossa Drupal\gatsby_instantpreview\GatsbyInstantPreview->gatsbyPrepareData() (row 82 in file .../web/modules/contrib/gatsby/modules/gatsby_instantpreview/src/GatsbyInstantPreview.php)

Commenting out that row fixes the issue somewhat, but is of course not ideal. Now "normal", ie. nodes with no circular relationships save and trigger builds normally, but nodes with circular relationship save normally but don't trigger builds. This probably needs some work since row 82's $preview_path is never filled. Just removing it as per GatsbyPreview->gatsbyPrepareData didn't help either.

This might be related/caused by this? https://www.drupal.org/project/gatsby/issues/3204395

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext for Service NSW commented

hi @teemuaro - could you open a new issue for that, sounds like it might be a recent change

teemuaro’s picture

teemuaro CreditAttribution: teemuaro commented

Sure thing, thanks for the suggestion.

https://www.drupal.org/project/gatsby/issues/3212557