Skip to content

Commit

Permalink
Attempting refactor of priority requirement. Peer review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrispenny
chrispenny committed Apr 4, 2023
1 parent 92f4617 commit 8c24101
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 188 deletions.
2 changes: 1 addition & 1 deletion src/Contract/StaticallyPublishable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface StaticallyPublishable
*
* Note: include the URL of the object itself!
*
* @return array associative array of URL (string) => Priority (int)
* @return array associative array of URL (string)
*/
public function urlsToCache(): array;
}
119 changes: 57 additions & 62 deletions src/Extension/Engine/SiteTreePublishingEngine.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ class SiteTreePublishingEngine extends SiteTreeExtension implements Resettable
/**
* Queues the urls to be flushed into the queue.
*/
private array|SS_List $toUpdate = [];
private array $urlsToUpdate = [];

/**
* Queues the urls to be deleted as part of a next flush operation.
*/
private array|SS_List $toDelete = [];
private array $urlsToDelete = [];

public static function reset(): void
{
Expand All @@ -57,50 +57,31 @@ public static function reset(): void
* Force inject queue service
* Used for unit tests only to cover edge cases where Injector doesn't cover
*
*
* @param QueuedJobService $service
*/
public static function setQueueService(QueuedJobService $service): void
{
static::$queueService = $service;
}

/**
* @return array
*/
public function getToUpdate()
public function getUrlsToUpdate(): array
{
return $this->toUpdate;
return $this->urlsToUpdate;
}

/**
* @return array
*/
public function getToDelete()
public function getUrlsToDelete(): array
{
return $this->toDelete;
return $this->urlsToDelete;
}

/**
* @param array $toUpdate
* @return $this
*/
public function setToUpdate($toUpdate)
public function setUrlsToUpdate(array $urlsToUpdate): void
{
$this->toUpdate = $toUpdate;

return $this;
$this->urlsToUpdate = $urlsToUpdate;
}

/**
* @param array $toDelete
* @return $this
*/
public function setToDelete($toDelete)
public function setUrlsToDelete(array $urlsToDelete): void
{
$this->toDelete = $toDelete;

return $this;
$this->urlsToDelete = $urlsToDelete;
}

/**
Expand All @@ -113,20 +94,20 @@ public function onBeforePublishRecursive($original)
return;
}

$owner = $this->getOwner();

// We want to find out if the URL for this page has changed at all. That can happen 2 ways: Either the page is
// moved in the SiteTree (ParentID changes), or the URLSegment is updated
// Apparently ParentID can sometimes be string, so make sure we cast to (int) for our comparison
if ((int) $original->ParentID !== (int) $this->getOwner()->ParentID
|| $original->URLSegment !== $this->getOwner()->URLSegment
if ((int) $original->ParentID !== (int) $owner->ParentID
|| $original->URLSegment !== $owner->URLSegment
) {
// We have detected a change to the URL. We need to purge the old URLs for this page and any children
$context = [
'action' => self::ACTION_UNPUBLISH,
];
// We'll collect these changes now, but they won't be actioned until onAfterPublishRecursive()
$original->collectChanges($context);
// We need to flushChanges() immediately so that Jobs are queued for the Pages while they still have their
// old URLs
$original->flushChanges();
}
}

Expand All @@ -138,18 +119,22 @@ public function onAfterPublishRecursive($original)
$parentId = $original->ParentID ?? null;
$urlSegment = $original->URLSegment ?? null;

$owner = $this->getOwner();

// Apparently ParentID can sometimes be string, so make sure we cast to (int) for our comparison
$parentChanged = $parentId && (int) $parentId !== (int) $this->getOwner()->ParentID;
$urlChanged = $urlSegment && $original->URLSegment !== $this->getOwner()->URLSegment;
$parentChanged = $parentId && (int) $parentId !== (int) $owner->ParentID;
$urlSegmentChanged = $urlSegment && $original->URLSegment !== $owner->URLSegment;

$context = [
'action' => self::ACTION_PUBLISH,
// If a URL change has been detected, then we need to force the recursive regeneration of all child
// pages
'urlChanged' => $parentChanged || $urlChanged,
'urlSegmentChanged' => $parentChanged || $urlSegmentChanged,
];

// Collect any additional changes (noting that some could already have been added in onBeforePublishRecursive())
$this->collectChanges($context);
// Flush any/all changes that we have detected
$this->flushChanges();
}

Expand All @@ -158,7 +143,13 @@ public function onBeforeUnpublish()
$context = [
'action' => self::ACTION_UNPUBLISH,
];
// We'll collect these changes now, but they won't be actioned until onAfterUnpublish()
$this->collectChanges($context);
}

public function onAfterUnpublish()
{
// Flush any/all changes that we have detected
$this->flushChanges();
}

Expand All @@ -174,24 +165,38 @@ public function collectChanges(array $context): void
// Collection of changes needs to happen within the context of our Published/LIVE state
Versioned::set_stage(Versioned::LIVE);

$owner = $this->getOwner();

// Re-fetch our page, now within a LIVE context
$page = DataObject::get($this->getOwner()->ClassName)->byID($this->getOwner()->ID);
$siteTree = DataObject::get($owner->ClassName)->byID($owner->ID);

// This page isn't LIVE/Published, so there is nothing for us to do here
if (!$page?->exists()) {
if (!$siteTree?->exists()) {
return;
}

// The page does not include the required extension, and it doesn't implement a Trigger
if (!$page->hasExtension(PublishableSiteTree::class) && !$page instanceof StaticPublishingTrigger) {
if (!$siteTree->hasExtension(PublishableSiteTree::class) && !$siteTree instanceof StaticPublishingTrigger) {
return;
}

$toUpdate = $page->objectsToUpdate($context);
$this->setToUpdate($toUpdate);
// Fetch our objects to be actioned
$objectsToUpdate = $siteTree->objectsToUpdate($context);
$objectsToDelete = $siteTree->objectsToDelete($context);
// Start collecting URLs from the above objects
$urlsToUpdate = [];
$urlsToDelete = [];

foreach ($objectsToUpdate as $objectToUpdate) {
$urlsToUpdate = array_merge($urlsToUpdate, $objectToUpdate->urlsToCache());
}

foreach ($objectsToDelete as $objectToDelete) {
$urlsToDelete = array_merge($urlsToDelete, $objectToDelete->urlsToCache());
}

$toDelete = $page->objectsToDelete($context);
$this->setToDelete($toDelete);
$this->setUrlsToUpdate($urlsToUpdate);
$this->setUrlsToDelete($urlsToDelete);
});
}

Expand All @@ -201,45 +206,35 @@ public function collectChanges(array $context): void
public function flushChanges()
{
$queueService = static::$queueService ?? QueuedJobService::singleton();
$urlsToUpdate = $this->getUrlsToUpdate();
$urlsToDelete = $this->getUrlsToDelete();

if ($this->toUpdate) {
if ($urlsToUpdate) {
/** @var UrlBundleInterface $urlService */
$urlService = Injector::inst()->create(UrlBundleInterface::class);

foreach ($this->toUpdate as $item) {
$urls = $item->urlsToCache();
ksort($urls);
$urls = array_keys($urls);
$urlService->addUrls($urls);
}
$urlService->addUrls($urlsToUpdate);

$jobs = $urlService->getJobsForUrls(GenerateStaticCacheJob::class, 'Building URLs', $this->owner);

foreach ($jobs as $job) {
$queueService->queueJob($job);
}

$this->toUpdate = [];
$this->urlsToUpdate = [];
}

if ($this->toDelete) {
if ($urlsToDelete) {
/** @var UrlBundleInterface $urlService */
$urlService = Injector::inst()->create(UrlBundleInterface::class);

foreach ($this->toDelete as $item) {
$urls = $item->urlsToCache();
ksort($urls);
$urls = array_keys($urls);
$urlService->addUrls($urls);
}
$urlService->addUrls($urlsToDelete);

$jobs = $urlService->getJobsForUrls(DeleteStaticCacheJob::class, 'Purging URLs', $this->owner);

foreach ($jobs as $job) {
$queueService->queueJob($job);
}

$this->toDelete = [];
$this->urlsToDelete = [];
}
}
}
43 changes: 26 additions & 17 deletions src/Extension/Publishable/PublishableSiteTree.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use SilverStripe\Control\Director;
use SilverStripe\ORM\DataExtension;
use SilverStripe\ORM\SS_List;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\StaticPublishQueue\Contract\StaticallyPublishable;
use SilverStripe\StaticPublishQueue\Contract\StaticPublishingTrigger;
use SilverStripe\StaticPublishQueue\Extension\Engine\SiteTreePublishingEngine;
Expand Down Expand Up @@ -42,15 +43,14 @@ public function getMyVirtualPages()
public function objectsToUpdate(array $context)
{
$list = [];
$page = $this->getOwner();
$urlChanged = $context['urlChanged'] ?? false;
$siteTree = $this->getOwner();

if ($context['action'] === SiteTreePublishingEngine::ACTION_PUBLISH) {
// Trigger refresh of the page itself
$list[] = $page;
$list[] = $siteTree;

// Refresh related virtual pages
$virtualPages = $page->getMyVirtualPages();
$virtualPages = $siteTree->getMyVirtualPages();

if ($virtualPages->exists()) {
foreach ($virtualPages as $virtual) {
Expand All @@ -62,24 +62,25 @@ public function objectsToUpdate(array $context)
// *not* include children
// Note: This configuration is only relevant for the 'publish' action because for an 'unpublish' action
// we specifically have to purge all children (we now that they have also been unpublished)
$childInclusion = $page->config()->get('regenerate_children');
$childInclusion = $siteTree->config()->get('regenerate_children');
$forceRecursiveInclusion = $context['urlSegmentChanged'] ?? false;

if ($childInclusion || $urlChanged) {
$recursive = $childInclusion === self::RELATION_INCLUDE_RECURSIVE || $urlChanged;
if ($childInclusion || $forceRecursiveInclusion) {
$recursive = $childInclusion === self::RELATION_INCLUDE_RECURSIVE || $forceRecursiveInclusion;

$this->addChildren($list, $page, $recursive);
$this->addChildren($list, $siteTree, $recursive);
}
}

// For any of our defined actions, we will update parents when configured to do so. Config value or 0
// is to *not* include parents
$parentInclusion = $page->config()->get('regenerate_parents');
$parentInclusion = $siteTree->config()->get('regenerate_parents');

if ($parentInclusion) {
// You can also choose whether to update only the direct parent, or the entire tree
$recursive = $parentInclusion === self::RELATION_INCLUDE_RECURSIVE;

$this->addParents($list, $page, $recursive);
$this->addParents($list, $siteTree, $recursive);
}

return $list;
Expand All @@ -98,23 +99,31 @@ public function objectsToDelete(array $context)
}

$list = [];
$page = $this->getOwner();
$siteTree = $this->getOwner();

// Trigger cache removal for this page
$list[] = $page;
$list[] = $siteTree;

// Trigger removal of the related virtual pages
$virtualPages = $page->getMyVirtualPages();
$virtualPages = $siteTree->getMyVirtualPages();

if ($virtualPages->exists()) {
foreach ($virtualPages as $virtual) {
$list[] = $virtual;
}
}

// When a parent is unpublished or the URL has changed, then all existing caches for all children have also
// been invalided and must be purged. In this context, this is not optional/configurable
$this->addChildren($list, $page, true);
// Check if you specifically want children included in all actions
$childInclusion = $siteTree->config()->get('regenerate_children');
// Check to see if SiteTree enforces strict hierarchy (that being, parents must be published in order for
// children to be viewed)
$forceRecursiveInclusion = $siteTree->config()->get('enforce_strict_hierarchy');

if ($childInclusion || $forceRecursiveInclusion) {
$recursive = $childInclusion === self::RELATION_INCLUDE_RECURSIVE || $forceRecursiveInclusion;

$this->addChildren($list, $siteTree, $recursive);
}

return $list;
}
Expand All @@ -134,7 +143,7 @@ public function urlsToCache(): array
$link = $page->Link();
}

return [Director::absoluteURL($link) => 0];
return [Director::absoluteURL($link)];
}

protected function addChildren(array &$list, SiteTree $currentPage, bool $recursive = false): void
Expand Down
Loading

0 comments on commit 8c24101

Please sign in to comment.