Merge pull request #17 from mateusz/optimise-and-limit

Optimise diff generation on small diferrences, and hard cap the list.
This commit is contained in:
Damian Mooyman 2015-03-30 12:57:10 +13:00
commit d37785ff7b
3 changed files with 83 additions and 22 deletions

View File

@ -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();
@ -52,24 +71,28 @@ class VersionFeed extends SiteTreeExtension {
foreach ($versions as $version) {
$changed = false;
// Check if we have something to compare with.
if (isset($previous)) {
// We have something to compare with.
$diff = $this->owner->compareVersions($version->Version, $previous->Version);
// Produce the diff fields for use in the template.
if ($version->Title != $previous->Title) {
$diffTitle = Diff::compareHTML($version->Title, $previous->Title);
$version->DiffTitle = new HTMLText();
$version->DiffTitle->setValue(
sprintf(
'<div><em>%s</em>' . $diff->Title . '</div>',
'<div><em>%s</em> ' . $diffTitle . '</div>',
_t('RSSHistory.TITLECHANGED', 'Title has changed:')
)
);
$changed = true;
}
if ($version->Content != $previous->Content) {
$diffContent = Diff::compareHTML($version->Content, $previous->Content);
$version->DiffContent = new HTMLText();
$version->DiffContent->setValue('<div>'.$diff->obj('Content')->forTemplate().'</div>');
$version->DiffContent->setValue('<div>'.$diffContent.'</div>');
$changed = true;
}
@ -87,18 +110,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('<div>' . $first->obj('Content')->forTemplate() . '</div>');
$first->DiffContent->setValue('<div>' . $first->Content . '</div>');
// 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;

View File

@ -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;

View File

@ -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'
);
}
}
}