PR fixes.

_config/versionedownership.yml

@@ -4,6 +4,10 @@ Name: versionedownership
 SilverStripe\ORM\DataObject:
   extensions:
     RecursivePublishable: SilverStripe\Versioned\RecursivePublishable
+
+SilverStripe\Core\Injector\Injector:
+  SilverStripe\Versioned\RecursiveStagesInterface:
+    class: SilverStripe\Versioned\RecursiveStagesService
 ---
 Name: versionedownership-admin
 OnlyIf:
@@ -12,9 +16,3 @@ OnlyIf:
 SilverStripe\Admin\LeftAndMain:
   extensions:
     RecursivePublishableHandler: SilverStripe\Versioned\RecursivePublishableHandler
----
-Name: versionedrecursivestages
----
-SilverStripe\Core\Injector\Injector:
-  SilverStripe\Versioned\RecursiveStagesInterface:
-    class: SilverStripe\Versioned\RecursiveStagesService

_config/versionedstate.yml

@@ -7,6 +7,7 @@ SilverStripe\Control\RequestHandler:
 SilverStripe\ORM\DataObject:
   extensions:
     - SilverStripe\Versioned\VersionedStateExtension
+    - SilverStripe\Versioned\RecursiveStagesExtension
 ---
 Name: versionedstate-test
 ---

src/RecursiveStagesExtension.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace SilverStripe\Versioned;
+
+use Psr\Container\NotFoundExceptionInterface;
+use SilverStripe\Core\Extension;
+use SilverStripe\ORM\DataObject;
+
+class RecursiveStagesExtension extends Extension
+{
+    /**
+     * Make sure to flush cached data in case the data changes
+     * Extension point in @see DataObject::onAfterWrite()
+     *
+     * @return void
+     * @throws NotFoundExceptionInterface
+     */
+    public function onAfterWrite(): void
+    {
+        RecursiveStagesService::reset();
+    }
+
+    /**
+     * Make sure to flush cached data in case the data changes
+     * Extension point in @see DataObject::onAfterDelete()
+     *
+     * @return void
+     * @throws NotFoundExceptionInterface
+     */
+    public function onAfterDelete(): void
+    {
+        RecursiveStagesService::reset();
+    }
+}

src/RecursiveStagesInterface.php

@@ -7,18 +7,16 @@
 /**
  * Interface RecursiveStagesInterface
  *
- * Interface for @see RecursiveStagesService to provide the capability to for "smart" durty model state
+ * Interface for @see RecursiveStagesService to provide the capability to for "smart" dirty model state
  * which can cover nested models
  */
 interface RecursiveStagesInterface
 {
-
     /**
      * Determine if content differs on stages including nested objects
      *
      * @param DataObject $object
-     * @param string $mode
      * @return bool
      */
-    public function stagesDifferRecursive(DataObject $object, string $mode): bool;
+    public function stagesDifferRecursive(DataObject $object): bool;
 }

src/RecursiveStagesService.php

@@ -2,165 +2,186 @@
 
 namespace SilverStripe\Versioned;
 
+use Exception;
+use Psr\Container\NotFoundExceptionInterface;
 use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Core\Resettable;
 use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\SS_List;
 
 /**
  * Functionality for detecting the need of publishing nested objects owned by common parent / ancestor object
  */
-class RecursiveStagesService implements RecursiveStagesInterface
+class RecursiveStagesService implements RecursiveStagesInterface, Resettable
 {
     use Injectable;
 
+    private array $stagesDifferCache = [];
+    private array $ownedObjectsCache = [];
+
+    public function flushCachedData(): void
+    {
+        $this->stagesDifferCache = [];
+        $this->ownedObjectsCache = [];
+    }
+
     /**
-     * Strong ownership uses 'owns' configuration to determine relationships
+     * @return void
+     * @throws NotFoundExceptionInterface
      */
-    public const OWNERSHIP_STRONG = 'strong';
+    public static function reset(): void
+    {
+        /** @var RecursiveStagesInterface $service */
+        $service = Injector::inst()->get(RecursiveStagesInterface::class);
+
+        if (!$service instanceof RecursiveStagesService) {
+            // This covers the case where the service is overridden
+            return;
+        }
+
+        $service->flushCachedData();
+    }
 
     /**
-     * Weak ownership uses 'cascade_duplicates' configuration to determine relationships
+     * Determine if content differs on stages including nested objects
+     * This method also supports non-versioned models to allow traversal of hierarchy
+     * which includes both versioned and non-versioned models
+     *
+     * @param DataObject $object
+     * @return bool
+     * @throws Exception
      */
-    public const OWNERSHIP_WEAK = 'weak';
+    public function stagesDifferRecursive(DataObject $object): bool
+    {
+        $cacheKey = $object->getUniqueKey();
+
+        if (!array_key_exists($cacheKey, $this->stagesDifferCache)) {
+            $this->stagesDifferCache[$cacheKey] = $this->determineStagesDifferRecursive($object);
+        }
+
+        return $this->stagesDifferCache[$cacheKey];
+    }
 
     /**
-     * Determine if content differs on stages including nested objects
+     * Execution ownership hierarchy traversal and inspect individual models
      *
      * @param DataObject $object
-     * @param string $mode
      * @return bool
+     * @throws Exception
      */
-    public function stagesDifferRecursive(DataObject $object, string $mode): bool
+    protected function determineStagesDifferRecursive(DataObject $object): bool
     {
         if (!$object->exists()) {
+            // Model hasn't been saved to DB, so we can just bail out as there is nothing to inspect
             return false;
         }
 
-        $items = [$object];
+        $models = [$object];
 
-        // compare existing content
-        while ($item = array_shift($items)) {
-            if ($this->checkNeedPublishingItem($item)) {
+        // Compare existing content (perform full ownership traversal)
+        while ($model = array_shift($models)) {
+            if ($this->isModelDirty($model)) {
+                // Model is dirty,
+                // we can return here as there is no need to check the rest of the hierarchy
                 return true;
             }
 
-            $relatedObjects = $this->findOwnedObjects($item, $mode);
-            $items = array_merge($items, $relatedObjects);
+            // Discover and add owned objects for inspection
+            $relatedObjects = $this->getOwnedObjects($model);
+            $models = array_merge($models, $relatedObjects);
         }
 
-        // compare deleted content
-        $draftIdentifiers = $this->findOwnedIdentifiers($object, $mode, Versioned::DRAFT);
-        $liveIdentifiers = $this->findOwnedIdentifiers($object, $mode, Versioned::LIVE);
+        // Compare deleted content,
+        // this wouldn't be covered in hierarchy traversal as deleted models are no longer present
+        $draftIdentifiers = $this->getOwnedIdentifiers($object, Versioned::DRAFT);
+        $liveIdentifiers = $this->getOwnedIdentifiers($object, Versioned::LIVE);
 
         return $draftIdentifiers !== $liveIdentifiers;
     }
 
     /**
-     * Find all identifiers for owned objects
+     * Get unique identifiers for all owned objects, so we can easily compare them
      *
      * @param DataObject $object
-     * @param string $mode
      * @param string $stage
      * @return array
+     * @throws Exception
      */
-    protected function findOwnedIdentifiers(DataObject $object, string $mode, string $stage): array
+    protected function getOwnedIdentifiers(DataObject $object, string $stage): array
     {
-        $ids = Versioned::withVersionedMode(function () use ($object, $mode, $stage): array {
+        $identifiers = Versioned::withVersionedMode(function () use ($object, $stage): array {
             Versioned::set_stage($stage);
 
-            $object = DataObject::get_by_id($object->ClassName, $object->ID);
+            /** @var DataObject $stagedObject */
+            $stagedObject = DataObject::get_by_id($object->ClassName, $object->ID);
 
-            if ($object === null) {
+            if ($stagedObject === null) {
                 return [];
             }
 
-            $items = [$object];
-            $ids = [];
+            $models = [$stagedObject];
+            $identifiers = [];
 
-            while ($object = array_shift($items)) {
-                $ids[] = implode('_', [$object->baseClass(), $object->ID]);
-                $relatedObjects = $this->findOwnedObjects($object, $mode);
-                $items = array_merge($items, $relatedObjects);
+            while ($model = array_shift($models)) {
+                // Compose a unique identifier, so we can easily compare models
+                // Note that we intentionally use base class here, so we can cover the situation where model class changes
+                $identifiers[] = implode('_', [$model->baseClass(), $model->ID]);
+                $relatedObjects = $this->getOwnedObjects($model);
+                $models = array_merge($models, $relatedObjects);
             }
 
-            return $ids;
+            return $identifiers;
         });
 
-        sort($ids, SORT_STRING);
+        sort($identifiers, SORT_STRING);
 
-        return array_values($ids);
+        return array_values($identifiers);
     }
 
     /**
-     * This lookup will attempt to find "Strongly owned" objects
-     * such objects are unable to exist without the current object
-     * We will use "cascade_duplicates" setting for this purpose as we can assume that if an object needs to be
-     * duplicated along with the owner object, it uses the strong ownership relation
-     *
-     * "Weakly owned" objects could be looked up via "owns" setting
-     * Such objects can exist even without the owner objects as they are often used as shared objects
-     * managed independently of their owners
+     * This lookup will attempt to find "owned" objects
+     * This method uses the 'owns' relation, same as @see RecursivePublishable::publishRecursive()
      *
-     * @param DataObject $object
-     * @param string $mode
+     * @param DataObject|RecursivePublishable $object
      * @return array
+     * @throws Exception
      */
-    protected function findOwnedObjects(DataObject $object, string $mode): array
+    protected function getOwnedObjects(DataObject $object): array
     {
-        $ownershipType = $mode === self::OWNERSHIP_WEAK
-            ? 'owns'
-            : 'cascade_duplicates';
-
-        $relations = (array) $object->config()->get($ownershipType);
-        $relations = array_unique($relations);
-        $result = [];
-
-        foreach ($relations as $relation) {
-            $relation = (string) $relation;
-
-            if (!$relation) {
-                continue;
-            }
-
-            $relationData = $object->{$relation}();
-
-            if ($relationData instanceof DataObject) {
-                if (!$relationData->exists()) {
-                    continue;
-                }
-
-                $result[] = $relationData;
-
-                continue;
-            }
-
-            if (!$relationData instanceof SS_List) {
-                continue;
-            }
+        if (!$object->hasExtension(RecursivePublishable::class)) {
+            return [];
+        }
 
-            foreach ($relationData as $relatedObject) {
-                if (!$relatedObject instanceof DataObject || !$relatedObject->exists()) {
-                    continue;
-                }
+        // Add versioned stage to cache key to cover the case where non-versioned model owns versioned models
+        // In this situation the versioned models can have different cached state which we need to cover
+        $cacheKey = sprintf('%s-%s', $object->getUniqueKey(), Versioned::get_stage());
 
-                $result[] = $relatedObject;
-            }
+        if (!array_key_exists($cacheKey, $this->ownedObjectsCache)) {
+            $this->ownedObjectsCache[$cacheKey] = $object
+                // We intentionally avoid recursive traversal here as it's not memory efficient,
+                // stack based approach is used instead for better performance
+                ->findOwned(false)
+                ->toArray();
         }
 
-        return $result;
+        return $this->ownedObjectsCache[$cacheKey];
     }
 
     /**
-     * @param DataObject|Versioned $item
+     * Determine if model is dirty (has draft changes that need publishing)
+     * Non-versioned models are supported
+     *
+     * @param DataObject $object
      * @return bool
      */
-    protected function checkNeedPublishingItem(DataObject $item): bool
+    protected function isModelDirty(DataObject $object): bool
     {
-        if ($item->hasExtension(Versioned::class)) {
-            /** @var $item Versioned */
-            return !$item->isPublished() || $item->stagesDiffer();
+        if ($object->hasExtension(Versioned::class)) {
+            /** @var $object Versioned */
+            return !$object->isPublished() || $object->stagesDiffer();
         }
 
+        // Non-versioned models are never dirty
         return false;
     }
 }

src/Versioned.php

@@ -4,6 +4,7 @@
 
 use InvalidArgumentException;
 use LogicException;
+use Psr\Container\NotFoundExceptionInterface;
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Cookie;
 use SilverStripe\Control\Director;
@@ -2000,19 +2001,17 @@ public function stagesDiffer()
 
     /**
      * Determine if content differs on stages including nested objects
-     * $mode determines which relation will be used to traverse the ownership tree
-     * "strong" will use "cascade_duplicates"
-     * "weak" will use "owns"
+     * 'owns' configuration drives the relationship traversal
      *
-     * @param string $mode "strong" or "weak"
      * @return bool
+     * @throws NotFoundExceptionInterface
      */
-    public function stagesDifferRecursive(string $mode = RecursiveStagesService::OWNERSHIP_STRONG): bool
+    public function stagesDifferRecursive(): bool
     {
         /** @var RecursiveStagesInterface $service */
         $service = Injector::inst()->get(RecursiveStagesInterface::class);
 
-        return $service->stagesDifferRecursive($this->owner, $mode);
+        return $service->stagesDifferRecursive($this->owner);
     }
 
     /**