diff --git a/_config/versionedstate.yml b/_config/versionedstate.yml index 653f5e6f..1813c8e5 100644 --- a/_config/versionedstate.yml +++ b/_config/versionedstate.yml @@ -7,7 +7,6 @@ SilverStripe\Control\RequestHandler: SilverStripe\ORM\DataObject: extensions: - SilverStripe\Versioned\VersionedStateExtension - - SilverStripe\Versioned\RecursiveStagesExtension --- Name: versionedstate-test --- diff --git a/src/RecursivePublishable.php b/src/RecursivePublishable.php index e02a19d0..79d636c8 100644 --- a/src/RecursivePublishable.php +++ b/src/RecursivePublishable.php @@ -12,7 +12,6 @@ use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\Queries\SQLUpdate; use SilverStripe\ORM\SS_List; -use SilverStripe\ORM\Tests\MySQLDatabaseTest\Data; /** * Provides owns / owned_by and recursive publishing API for all objects. @@ -448,4 +447,22 @@ public function onBeforeDuplicate($original, &$doWrite, &$relations) $owns = $this->owner->config()->get('owns'); $relations = array_intersect($allowed ?? [], $owns); } + + /** + * Make sure to flush cached data in case the data changes + * Extension point in @see DataObject::onAfterWrite() + */ + public function onAfterWrite(): void + { + RecursiveStagesService::reset(); + } + + /** + * Make sure to flush cached data in case the data changes + * Extension point in @see DataObject::onAfterDelete() + */ + public function onAfterDelete(): void + { + RecursiveStagesService::reset(); + } } diff --git a/src/RecursiveStagesExtension.php b/src/RecursiveStagesExtension.php deleted file mode 100644 index b77df6f8..00000000 --- a/src/RecursiveStagesExtension.php +++ /dev/null @@ -1,34 +0,0 @@ -ownedObjectsCache = []; } - /** - * @return void - * @throws NotFoundExceptionInterface - */ public static function reset(): void { /** @var RecursiveStagesInterface $service */ @@ -46,10 +40,6 @@ public static function reset(): void * 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 function stagesDifferRecursive(DataObject $object): bool { @@ -64,10 +54,6 @@ public function stagesDifferRecursive(DataObject $object): bool /** * Execution ownership hierarchy traversal and inspect individual models - * - * @param DataObject $object - * @return bool - * @throws Exception */ protected function determineStagesDifferRecursive(DataObject $object): bool { @@ -76,11 +62,12 @@ protected function determineStagesDifferRecursive(DataObject $object): bool return false; } + // Compare existing content (perform full ownership traversal) $models = [$object]; - // Compare existing content (perform full ownership traversal) + /** @var DataObject|Versioned $model */ while ($model = array_shift($models)) { - if ($this->isModelDirty($model)) { + if ($model->hasExtension(Versioned::class) && $model->isModifiedOnDraft()) { // Model is dirty, // we can return here as there is no need to check the rest of the hierarchy return true; @@ -101,11 +88,6 @@ protected function determineStagesDifferRecursive(DataObject $object): bool /** * Get unique identifiers for all owned objects, so we can easily compare them - * - * @param DataObject $object - * @param string $stage - * @return array - * @throws Exception */ protected function getOwnedIdentifiers(DataObject $object, string $stage): array { @@ -123,9 +105,10 @@ protected function getOwnedIdentifiers(DataObject $object, string $stage): array $identifiers = []; 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]); + // Compose a unique identifier, so we can easily compare models across stages + // Note that we can't use getUniqueKey() as that one contains stage fragments + // which prevents us from making cross-stage comparison + $identifiers[] = $model->ClassName . '-' . $model->ID; $relatedObjects = $this->getOwnedObjects($model); $models = array_merge($models, $relatedObjects); } @@ -141,10 +124,6 @@ protected function getOwnedIdentifiers(DataObject $object, string $stage): array /** * This lookup will attempt to find "owned" objects * This method uses the 'owns' relation, same as @see RecursivePublishable::publishRecursive() - * - * @param DataObject|RecursivePublishable $object - * @return array - * @throws Exception */ protected function getOwnedObjects(DataObject $object): array { @@ -154,7 +133,7 @@ protected function getOwnedObjects(DataObject $object): array // 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()); + $cacheKey = $object->getUniqueKey() . '-' . Versioned::get_stage(); if (!array_key_exists($cacheKey, $this->ownedObjectsCache)) { $this->ownedObjectsCache[$cacheKey] = $object @@ -166,22 +145,4 @@ protected function getOwnedObjects(DataObject $object): array return $this->ownedObjectsCache[$cacheKey]; } - - /** - * Determine if model is dirty (has draft changes that need publishing) - * Non-versioned models are supported - * - * @param DataObject $object - * @return bool - */ - protected function isModelDirty(DataObject $object): bool - { - if ($object->hasExtension(Versioned::class)) { - /** @var $object Versioned */ - return !$object->isPublished() || $object->stagesDiffer(); - } - - // Non-versioned models are never dirty - return false; - } } diff --git a/src/Versioned.php b/src/Versioned.php index 06079cb9..8e917cf0 100644 --- a/src/Versioned.php +++ b/src/Versioned.php @@ -4,7 +4,6 @@ use InvalidArgumentException; use LogicException; -use Psr\Container\NotFoundExceptionInterface; use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; use SilverStripe\Control\Director; @@ -2002,9 +2001,6 @@ public function stagesDiffer() /** * Determine if content differs on stages including nested objects * 'owns' configuration drives the relationship traversal - * - * @return bool - * @throws NotFoundExceptionInterface */ public function stagesDifferRecursive(): bool { diff --git a/tests/php/RecursiveStagesServiceTest.php b/tests/php/RecursiveStagesServiceTest.php index 33a83e53..9e9bc95e 100644 --- a/tests/php/RecursiveStagesServiceTest.php +++ b/tests/php/RecursiveStagesServiceTest.php @@ -2,9 +2,7 @@ namespace SilverStripe\Versioned\Tests; -use Psr\Container\NotFoundExceptionInterface; use SilverStripe\Dev\SapphireTest; -use SilverStripe\ORM\ValidationException; use SilverStripe\Versioned\Tests\RecursiveStagesServiceTest\ChildObject; use SilverStripe\Versioned\Tests\RecursiveStagesServiceTest\ColumnObject; use SilverStripe\Versioned\Tests\RecursiveStagesServiceTest\GroupObject; @@ -40,10 +38,6 @@ class RecursiveStagesServiceTest extends SapphireTest ], ]; - /** - * @return void - * @throws NotFoundExceptionInterface - */ public function testStageDiffersRecursiveWithInvalidObject(): void { Versioned::withVersionedMode(function (): void { @@ -57,13 +51,6 @@ public function testStageDiffersRecursiveWithInvalidObject(): void } /** - * @param string $class - * @param string $identifier - * @param bool $delete - * @param bool $expected - * @return void - * @throws ValidationException - * @throws NotFoundExceptionInterface * @dataProvider objectsProvider */ public function testStageDiffersRecursive(string $class, string $identifier, bool $delete, bool $expected): void