Stop validating related elements recursively

Resolves #13904
craftcms · Nov 7, 2023 · ae45e59 · ae45e59
1 parent b032852
commit ae45e59
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 - Added the `db/drop-table-prefix` command.
 - Top-level disabled related/nested elements are now included in “Extended” element exports. ([#13496](https://github.com/craftcms/cms/issues/13496))
+- Related element validation is no longer recursive. ([#13904](https://github.com/craftcms/cms/issues/13904))
 - Addresses’ owner elements are now automatically set on them during initialization, if they were queried with the `owner` address query param.
 - Entry Title fields are no longer shown when “Show the Title field” is disabled and there’s a validation error on the `title` attribute. ([#13876](https://github.com/craftcms/cms/issues/13876))
 - Improved the reliability of image dimension detection. ([#13886](https://github.com/craftcms/cms/pull/13886))

diff --git a/src/base/ElementTrait.php b/src/base/ElementTrait.php
@@ -163,6 +163,12 @@ trait ElementTrait
      */
     public bool $propagating = false;
 
+    /**
+     * @var bool Whether the element is currently being validated via BaseRelationField::validateRelatedElements()
+     * @since 4.5.10
+     */
+    public bool $validatingRelatedElement = false;
+
     /**
      * @var bool Whether all element attributes should be propagated across all its supported sites, even if that means
      * overwriting existing site-specific values.

diff --git a/src/fields/BaseRelationField.php b/src/fields/BaseRelationField.php
@@ -26,7 +26,6 @@
 use craft\elements\ElementCollection;
 use craft\errors\SiteNotFoundException;
 use craft\events\ElementCriteriaEvent;
-use craft\events\ElementEvent;
 use craft\fields\conditions\RelationalFieldConditionRule;
 use craft\helpers\ArrayHelper;
 use craft\helpers\Cp;
@@ -104,18 +103,6 @@ public static function valueType(): string
         return sprintf('\\%s|\\%s<\\%s>', ElementQueryInterface::class, ElementCollection::class, ElementInterface::class);
     }
 
-    /**
-     * @var array Related elements that have been validated
-     * @see _validateRelatedElement()
-     */
-    private static array $_relatedElementValidates = [];
-
-    /**
-     * @var bool Whether we're listening for related element saves yet
-     * @see _validateRelatedElement()
-     */
-    private static bool $_listeningForRelatedElementSave = false;
-
     /**
      * @var string|string[]|null The source keys that this field can relate elements from (used if [[allowMultipleSources]] is set to true)
      */
@@ -470,10 +457,10 @@ public function validateRelationCount(ElementInterface $element): void
      */
     public function validateRelatedElements(ElementInterface $element): void
     {
-        // Prevent circular relations from worrying about this element
-        $sourceId = $element->getCanonicalId();
-        $sourceValidates = self::$_relatedElementValidates[$sourceId][$element->siteId] ?? null;
-        self::$_relatedElementValidates[$sourceId][$element->siteId] = true;
+        // Only enforce this when the source element is being saved directly
+        if ($element->validatingRelatedElement) {
+            return;
+        }
 
         /** @var ElementQueryInterface|Collection $value */
         $value = $element->getFieldValue($this->handle);
@@ -485,25 +472,23 @@ public function validateRelatedElements(ElementInterface $element): void
                 ->preferSites([$this->targetSiteId($element)]);
         }
 
+        $sourceId = $element->getCanonicalId();
         $errorCount = 0;
 
         foreach ($value->all() as $i => $related) {
             /** @var Element $related */
-            if ($related->enabled && $related->getEnabledForSite()) {
+            if (
+                $related->enabled &&
+                $related->getEnabledForSite() &&
+                $related->getCanonicalId() !== $sourceId
+            ) {
                 if (!self::_validateRelatedElement($related)) {
                     $element->addModelErrors($related, "$this->handle[$i]");
                     $errorCount++;
                 }
             }
         }
 
-        // Reset self::$_relatedElementValidates[$sourceId][$element->siteId] to its original value
-        if ($sourceValidates !== null) {
-            self::$_relatedElementValidates[$sourceId][$element->siteId] = $sourceValidates;
-        } else {
-            unset(self::$_relatedElementValidates[$sourceId][$element->siteId]);
-        }
-
         if ($errorCount) {
             /** @var ElementInterface|string $elementType */
             $elementType = static::elementType();
@@ -522,25 +507,14 @@ public function validateRelatedElements(ElementInterface $element): void
      */
     private static function _validateRelatedElement(ElementInterface $element): bool
     {
-        if (isset(self::$_relatedElementValidates[$element->id][$element->siteId])) {
-            return self::$_relatedElementValidates[$element->id][$element->siteId];
-        }
-
-        // If this is the first time we are validating a related element,
-        // listen for future element saves so we can clear our cache
-        if (!self::$_listeningForRelatedElementSave) {
-            Event::on(Elements::class, Elements::EVENT_AFTER_SAVE_ELEMENT, function(ElementEvent $e) {
-                $element = $e->element;
-                unset(self::$_relatedElementValidates[$element->id][$element->siteId]);
-            });
-            self::$_listeningForRelatedElementSave = true;
-        }
-
-        // Prevent an infinite loop if there are circular relations
-        self::$_relatedElementValidates[$element->id][$element->siteId] = true;
+        // Prevent relational fields on this element from enforcing related element validation
+        $element->validatingRelatedElement = true;
 
         $element->setScenario(Element::SCENARIO_LIVE);
-        return self::$_relatedElementValidates[$element->id][$element->siteId] = $element->validate();
+        $validates = $element->validate();
+
+        $element->validatingRelatedElement = false;
+        return $validates;
     }
 
     /**