From 543af36a54a6aad78c06d8572fc66870c67f5446 Mon Sep 17 00:00:00 2001 From: Chris Penny Date: Tue, 2 May 2023 08:30:35 +1200 Subject: [PATCH] Remove public API changes from changeset --- README.md | 25 ---- src/Contract/StaticPublishingTrigger.php | 4 +- src/Contract/StaticallyPublishable.php | 2 +- .../Engine/SiteTreePublishingEngine.php | 119 ++++++++++++++---- .../Publishable/PublishableSiteTree.php | 8 +- 5 files changed, 100 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 0a561b90..2d72a716 100644 --- a/README.md +++ b/README.md @@ -26,31 +26,6 @@ It generates the cache files using the [QueuedJobs module](https://github.com/sy * "symbiote/silverstripe-queuedjobs": "^5", * "silverstripe/versioned": "^2" -## Migrating from v4 - -### `SiteTreePublishingEngine` changes - -The following methods have been removed: - -* `getToUpdate()` -* `setToUpdate()` -* `getToDelete()` -* `setToDelete()` - -These methods used to deal with `DataObjects`. We are now collecting URLs instead of `DataObjects`, and these URLs are -being stored and manipulated by the following methods. - -* `getUrlsToUpdate()` -* `addUrlsToUpdate()` -* `setUrlsToUpdate()` -* `getUrlsToDelete()` -* `addUrlsToDelete()` -* `setUrlsToDelete()` - -Why did this change happen? TL;DR: We realised that purging URLs for pages that change their URL (EG: their parent page -changes, or their `URLSegment` changes) was not functioning as expected. This was really the only way to enable this -functionality to work correctly while maintaining cache integrety. - ## Unit-testing with StaticPublisherState to disable queuedjobs for unit-tests You can use `StaticPublisherState` to disable queuejobs job queueing and logging in unit-testing to improve performance. diff --git a/src/Contract/StaticPublishingTrigger.php b/src/Contract/StaticPublishingTrigger.php index 2bd76eb1..6472c882 100644 --- a/src/Contract/StaticPublishingTrigger.php +++ b/src/Contract/StaticPublishingTrigger.php @@ -15,7 +15,7 @@ interface StaticPublishingTrigger * @param array $context An associative array with extra engine-specific information. * @return array|SS_List */ - public function objectsToUpdate(array $context); + public function objectsToUpdate($context); /** * Provides a SS_list of objects that need to be deleted. @@ -23,5 +23,5 @@ public function objectsToUpdate(array $context); * @param array $context An associative array with extra engine-specific information. * @return array|SS_List */ - public function objectsToDelete(array $context); + public function objectsToDelete($context); } diff --git a/src/Contract/StaticallyPublishable.php b/src/Contract/StaticallyPublishable.php index 022938f7..2d730e51 100644 --- a/src/Contract/StaticallyPublishable.php +++ b/src/Contract/StaticallyPublishable.php @@ -22,5 +22,5 @@ interface StaticallyPublishable * * @return array associative array of URL (string) => Priority (int) */ - public function urlsToCache(): array; + public function urlsToCache(); } diff --git a/src/Extension/Engine/SiteTreePublishingEngine.php b/src/Extension/Engine/SiteTreePublishingEngine.php index d0154645..a69e3cc1 100644 --- a/src/Extension/Engine/SiteTreePublishingEngine.php +++ b/src/Extension/Engine/SiteTreePublishingEngine.php @@ -8,7 +8,6 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Resettable; use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\SS_List; use SilverStripe\StaticPublishQueue\Contract\StaticPublishingTrigger; use SilverStripe\StaticPublishQueue\Extension\Publishable\PublishableSiteTree; use SilverStripe\StaticPublishQueue\Job\DeleteStaticCacheJob; @@ -35,8 +34,10 @@ class SiteTreePublishingEngine extends SiteTreeExtension implements Resettable /** * Queued job service injection property * Used for unit tests only to cover edge cases where Injector doesn't cover + * + * @var QueuedJobService|null */ - protected static ?QueuedJobService $queueService = null; + protected static $queueService = null; /** * Queues the urls to be flushed into the queue. @@ -48,6 +49,82 @@ class SiteTreePublishingEngine extends SiteTreeExtension implements Resettable */ private array $urlsToDelete = []; + /** + * Queues the urls to be flushed into the queue. + * + * @var array + * @deprecated 6.0.0 use {@link $urlsToDelete} + */ + private $toUpdate = []; + + /** + * Queues the urls to be deleted as part of a next flush operation. + * + * @var array + * @deprecated 6.0.0 use {@link $urlsToDelete} + */ + private $toDelete = []; + + /** + * @return array + * @deprecated 6.0.0 use {@link getUrlsToUpdate()} + */ + public function getToUpdate() + { + return $this->toUpdate; + } + + /** + * @return array + * @deprecated 6.0.0 use {@link getUrlsToDelete()} + */ + public function getToDelete() + { + return $this->toDelete; + } + + /** + * @param array $toUpdate + * @return $this + * @deprecated 6.0.0 use {@link setUrlsToUpdate()} This method is still used internally for backwards compatability, + * but will be replaced asap + */ + public function setToUpdate($toUpdate) + { + $urlsToUpdate = []; + + foreach ($toUpdate as $objectToUpdate) { + $urlsToUpdate = array_merge($urlsToUpdate, array_keys($objectToUpdate->urlsToCache())); + } + + $this->setUrlsToUpdate($urlsToUpdate); + // Legacy support so that getToUpdate() still returns the expected array of DataObjects + $this->toUpdate = $toUpdate; + + return $this; + } + + /** + * @param array $toDelete + * @return $this + * @deprecated 6.0.0 use {@link setUrlsToDelete()} This method is still used internally for backwards compatability, + * but will be replaced asap + */ + public function setToDelete($toDelete) + { + $urlsToDelete = []; + + foreach ($toDelete as $objectToDelete) { + $urlsToDelete = array_merge($urlsToDelete, array_keys($objectToDelete->urlsToCache())); + } + + $this->setUrlsToDelete($urlsToDelete); + // Legacy support so that getToDelete() still returns the expected array of DataObjects + $this->toDelete = $toDelete; + + return $this; + } + public static function reset(): void { static::$queueService = null; @@ -69,24 +146,14 @@ protected function getUrlsToUpdate(): array return $this->urlsToUpdate; } - protected function getUrlsToDelete(): array - { - return $this->urlsToDelete; - } - - protected function addUrlsToUpdate(array $urlsToUpdate): void - { - $this->urlsToUpdate = array_unique(array_merge($this->getUrlsToUpdate(), $urlsToUpdate)); - } - - protected function addUrlsToDelete(array $urlsToDelete): void + protected function setUrlsToUpdate(array $urlsToUpdate): void { - $this->urlsToDelete = array_unique(array_merge($this->getUrlsToDelete(), $urlsToDelete)); + $this->urlsToUpdate = $urlsToUpdate; } - protected function setUrlsToUpdate(array $urlsToUpdate): void + protected function getUrlsToDelete(): array { - $this->urlsToUpdate = $urlsToUpdate; + return $this->urlsToDelete; } protected function setUrlsToDelete(array $urlsToDelete): void @@ -126,6 +193,9 @@ public function onBeforePublishRecursive($original) */ public function onAfterPublishRecursive($original) { + // Flush any/all changes that we might have collected from onBeforePublishRecursive() + $this->flushChanges(); + $parentId = $original->ParentID ?? null; $urlSegment = $original->URLSegment ?? null; @@ -165,8 +235,11 @@ public function onAfterUnpublish() /** * Collect all changes for the given context. + * + * @param array $context + * @return void */ - public function collectChanges(array $context): void + public function collectChanges($context) { Environment::increaseMemoryLimitTo(); Environment::increaseTimeLimitTo(); @@ -191,16 +264,8 @@ public function collectChanges(array $context): void } // Fetch our objects to be actioned - $objectsToUpdate = $siteTree->objectsToUpdate($context); - $objectsToDelete = $siteTree->objectsToDelete($context); - - foreach ($objectsToUpdate as $objectToUpdate) { - $this->addUrlsToUpdate(array_keys($objectToUpdate->urlsToCache())); - } - - foreach ($objectsToDelete as $objectToDelete) { - $this->addUrlsToDelete(array_keys($objectToDelete->urlsToCache())); - } + $this->setToUpdate($siteTree->objectsToUpdate($context)); + $this->setToDelete($siteTree->objectsToDelete($context)); }); } diff --git a/src/Extension/Publishable/PublishableSiteTree.php b/src/Extension/Publishable/PublishableSiteTree.php index 071f55cd..6013666e 100644 --- a/src/Extension/Publishable/PublishableSiteTree.php +++ b/src/Extension/Publishable/PublishableSiteTree.php @@ -40,7 +40,7 @@ public function getMyVirtualPages() /** * @return array|SS_List */ - public function objectsToUpdate(array $context) + public function objectsToUpdate($context) { $list = []; $siteTree = $this->getOwner(); @@ -91,7 +91,7 @@ public function objectsToUpdate(array $context) * * @return array|SS_List */ - public function objectsToDelete(array $context) + public function objectsToDelete($context) { // This context isn't one of our valid actions, so there's nothing to do here if ($context['action'] !== SiteTreePublishingEngine::ACTION_UNPUBLISH) { @@ -130,8 +130,10 @@ public function objectsToDelete(array $context) /** * The only URL belonging to this object is its own URL. + * + * @return array */ - public function urlsToCache(): array + public function urlsToCache() { $page = $this->getOwner();