From 8a7d2ec9da84a128577777db6d5c6b6d8d53002d Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Mon, 30 Mar 2015 11:28:42 +1300 Subject: [PATCH] NEW Refactor diff API and add managed limits. Deprecate the getDiffedChanges method to get rid of confusing fullHistory parameter. Add a getDiff method to fetch just a single diff and a more versatile getDiffList method which allows specifying a limit. We now also cap the amount of items coming from the diffing process to a default value that we hope will result in generation times no greater than a few seconds. This can be reconfigured by developers. --- code/VersionFeed.php | 73 +++++++++++++++++++++++++++++---- code/VersionFeed_Controller.php | 12 +++--- tests/VersionFeedTest.php | 6 +-- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/code/VersionFeed.php b/code/VersionFeed.php index ce5ee3e..b8cb33d 100644 --- a/code/VersionFeed.php +++ b/code/VersionFeed.php @@ -13,7 +13,7 @@ class VersionFeed extends SiteTreeExtension { public function updateFieldLabels(&$labels) { $labels['PublicHistory'] = _t('RSSHistory.LABEL', 'Make history public'); } - + /** * Enable the allchanges feed * @@ -21,7 +21,15 @@ class VersionFeed extends SiteTreeExtension { * @var bool */ private static $allchanges_enabled = true; - + + /** + * Allchanges feed limit of items. + * + * @config + * @var int + */ + private static $allchanges_limit = 20; + /** * Enables RSS feed for page-specific changes * @@ -30,20 +38,31 @@ class VersionFeed extends SiteTreeExtension { */ private static $changes_enabled = true; + /** + * Changes feed limit of items. + * + * @config + * @var int + */ + private static $changes_limit = 100; + /** * Compile a list of changes to the current page, excluding non-published and explicitly secured versions. * * @param int $highestVersion Top version number to consider. - * @param boolean $fullHistory Whether to get the full change history or just the previous version. + * @param int $limit Limit to the amount of items returned. * * @returns ArrayList List of cleaned records. */ - public function getDiffedChanges($highestVersion = null, $fullHistory = true) { + public function getDiffList($highestVersion = null, $limit = 100) { // This can leak secured content if it was protected via inherited setting. // For now the users will need to be aware about this shortcoming. $offset = $highestVersion ? "AND \"SiteTree_versions\".\"Version\"<='".(int)$highestVersion."'" : ''; - $limit = $fullHistory ? null : 2; - $versions = $this->owner->allVersions("\"WasPublished\"='1' AND \"CanViewType\" IN ('Anyone', 'Inherit') $offset", "\"LastEdited\" DESC", $limit); + // Get just enough elements for diffing. We need one more than desired to have something to compare to. + $qLimit = (int)$limit + 1; + $versions = $this->owner->allVersions( + "\"WasPublished\"='1' AND \"CanViewType\" IN ('Anyone', 'Inherit') $offset", "\"LastEdited\" DESC", $qLimit + ); // Process the list to add the comparisons. $changeList = new ArrayList(); @@ -87,18 +106,54 @@ class VersionFeed extends SiteTreeExtension { $previous = $version; } - // Push the first version on to the list - only if we're looking at the full history or if it's the first - // version in the version history. - if ($previous && ($fullHistory || $versions->count() == 1)) { + // Make sure enough diff items have been generated to satisfy the $limit. If we ran out, add the final, + // non-diffed item (the initial version). This will also work for a single-diff request: if we are requesting + // a diff on the initial version we will just get that version, verbatim. + if ($previous && $versions->count()<$qLimit) { $first = clone($previous); $first->DiffContent = new HTMLText(); $first->DiffContent->setValue('
' . $first->obj('Content')->forTemplate() . '
'); + // Copy the link so it can be cached by SS_Cache. + $first->GeneratedLink = $first->AbsoluteLink(); $changeList->push($first); } return $changeList; } + /** + * Return a single diff representing this version. + * Returns the initial version if there is nothing to compare to. + * + * @returns DataObject Object with relevant fields diffed. + */ + public function getDiff() { + $changes = $this->getDiffList($this->owner->Version, 1); + if ($changes && $changes->Count()) { + return $changes->First(); + } + + return null; + } + + /** + * Compile a list of changes to the current page, excluding non-published and explicitly secured versions. + * + * @deprecated 2.0.0 Use VersionFeed::getDiffList instead + * + * @param int $highestVersion Top version number to consider. + * @param boolean $fullHistory Set to true to get the full change history, set to false for a single diff. + * @param int $limit Limit to the amount of items returned. + * + * @returns ArrayList List of cleaned records. + */ + public function getDiffedChanges($highestVersion = null, $fullHistory = true, $limit = 100) { + return $this->getDiffList( + $highestVersion, + $fullHistory ? $limit : 1 + ); + } + public function updateSettingsFields(FieldList $fields) { if(!Config::inst()->get(get_class(), 'changes_enabled')) return; diff --git a/code/VersionFeed_Controller.php b/code/VersionFeed_Controller.php index 68e0768..2a9a20d 100644 --- a/code/VersionFeed_Controller.php +++ b/code/VersionFeed_Controller.php @@ -56,11 +56,11 @@ class VersionFeed_Controller extends Extension { $target = $this->owner; $key = implode('_', array('changes', $this->owner->ID, $this->owner->Version)); $entries = $this->filterContent($key, function() use ($target) { - return $target->getDiffedChanges(); + return $target->getDiffList(null, Config::inst()->get('VersionFeed', 'changes_limit')); }); // Generate the output. - $title = sprintf(_t('RSSHistory.SINGLEPAGEFEEDTITLE', 'Updates to %s page'), $this->owner->Title); + $title = sprintf(_t('RSSHistory.SINGLEPAGEFEEDTITLE', 'Updates to %s page'), $this->owner->Title); $rss = new RSSFeed($entries, $this->owner->request->getURL(), $title, '', 'Title', '', null); $rss->setTemplate('Page_changes_rss'); return $rss->outputToBrowser(); @@ -77,13 +77,14 @@ class VersionFeed_Controller extends Extension { return $this->owner->httpError(404, 'Global history not viewable'); } + $limit = (int)Config::inst()->get('VersionFeed', 'allchanges_limit'); $latestChanges = DB::query(' SELECT * FROM "SiteTree_versions" WHERE "WasPublished" = \'1\' AND "CanViewType" IN (\'Anyone\', \'Inherit\') AND "ShowInSearch" = 1 AND ("PublicHistory" IS NULL OR "PublicHistory" = \'1\') - ORDER BY "LastEdited" DESC LIMIT 20' + ORDER BY "LastEdited" DESC LIMIT ' . $limit ); $lastChange = $latestChanges->record(); $latestChanges->rewind(); @@ -110,8 +111,9 @@ class VersionFeed_Controller extends Extension { // Get the diff to the previous version. $version = new Versioned_Version($record); - $changes = $version->getDiffedChanges($version->Version, false); - if ($changes && $changes->Count()) $changeList->push($changes->First()); + if ($diff = $version->getDiff()) { + $changeList->push($diff); + } } return $changeList; diff --git a/tests/VersionFeedTest.php b/tests/VersionFeedTest.php index 19cf1d1..f782c6f 100644 --- a/tests/VersionFeedTest.php +++ b/tests/VersionFeedTest.php @@ -39,15 +39,15 @@ class VersionFeedTest extends SapphireTest { $this->assertContains( str_replace(' ' ,'',_t('RSSHistory.TITLECHANGED', 'Title has changed:') . 'My Changed Title'), - array_map($cleanDiffOutput, $page->getDiffedChanges()->column('DiffTitle')), + array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle')), 'Detects published title changes' ); $this->assertNotContains( str_replace(' ' ,'',_t('RSSHistory.TITLECHANGED', 'Title has changed:') . 'My Unpublished Changed Title'), - array_map($cleanDiffOutput, $page->getDiffedChanges()->column('DiffTitle')), + array_map($cleanDiffOutput, $page->getDiffList()->column('DiffTitle')), 'Ignores unpublished title changes' ); } -} \ No newline at end of file +}