Skip to content

Commit

Permalink
PR fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
mfendeksilverstripe committed Oct 18, 2023
1 parent d2efe5a commit d6490ce
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 103 deletions.
1 change: 0 additions & 1 deletion _config/versionedstate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ SilverStripe\Control\RequestHandler:
SilverStripe\ORM\DataObject:
extensions:
- SilverStripe\Versioned\VersionedStateExtension
- SilverStripe\Versioned\RecursiveStagesExtension
---
Name: versionedstate-test
---
Expand Down
19 changes: 18 additions & 1 deletion src/RecursivePublishable.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
}
34 changes: 0 additions & 34 deletions src/RecursiveStagesExtension.php

This file was deleted.

3 changes: 0 additions & 3 deletions src/RecursiveStagesInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ interface RecursiveStagesInterface
{
/**
* Determine if content differs on stages including nested objects
*
* @param DataObject $object
* @return bool
*/
public function stagesDifferRecursive(DataObject $object): bool;
}
55 changes: 8 additions & 47 deletions src/RecursiveStagesService.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace SilverStripe\Versioned;

use Exception;
use Psr\Container\NotFoundExceptionInterface;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Resettable;
Expand All @@ -25,10 +23,6 @@ public function flushCachedData(): void
$this->ownedObjectsCache = [];
}

/**
* @return void
* @throws NotFoundExceptionInterface
*/
public static function reset(): void
{
/** @var RecursiveStagesInterface $service */
Expand All @@ -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
{
Expand All @@ -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
{
Expand All @@ -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;
Expand All @@ -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
{
Expand All @@ -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);
}
Expand All @@ -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
{
Expand All @@ -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
Expand All @@ -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;
}
}
4 changes: 0 additions & 4 deletions src/Versioned.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use InvalidArgumentException;
use LogicException;
use Psr\Container\NotFoundExceptionInterface;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Cookie;
use SilverStripe\Control\Director;
Expand Down Expand Up @@ -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
{
Expand Down
13 changes: 0 additions & 13 deletions tests/php/RecursiveStagesServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -40,10 +38,6 @@ class RecursiveStagesServiceTest extends SapphireTest
],
];

/**
* @return void
* @throws NotFoundExceptionInterface
*/
public function testStageDiffersRecursiveWithInvalidObject(): void
{
Versioned::withVersionedMode(function (): void {
Expand All @@ -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
Expand Down

0 comments on commit d6490ce

Please sign in to comment.