From dd361929db0bba5a47ed4d4a15b51b1662dd3588 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 13 Dec 2017 16:13:46 +1300 Subject: [PATCH] FIX Use cached SiteTree object for history instead of creating a temporary object --- src/VersionFeedController.php | 10 ++++++---- tests/VersionFeedFunctionalTest.php | 29 ++++++++++++++--------------- tests/VersionFeedTest.php | 16 ++++++---------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/VersionFeedController.php b/src/VersionFeedController.php index ed20188..130aa27 100644 --- a/src/VersionFeedController.php +++ b/src/VersionFeedController.php @@ -4,6 +4,8 @@ namespace SilverStripe\VersionFeed; use SilverStripe\Core\Config\Config; use SilverStripe\Control\RSS\RSSFeed; +use SilverStripe\ORM\DataObject; +use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\ORM\DB; use SilverStripe\Security\Member; @@ -118,7 +120,7 @@ class VersionFeedController extends Extension // Cache the diffs to remove DOS possibility. $key = 'allchanges' . preg_replace('#[^a-zA-Z0-9_]#', '', $lastChange['LastEdited']) - . (Member::currentUserID() ?: 'public'); + . (Security::getCurrentUser() ? Security::getCurrentUser()->ID : 'public'); $changeList = $this->filterContent($key, function () use ($latestChanges) { $changeList = new ArrayList(); $canView = array(); @@ -126,16 +128,16 @@ class VersionFeedController extends Extension // Check if the page should be visible. // WARNING: although we are providing historical details, we check the current configuration. $id = $record['RecordID']; + $page = DataObject::get_by_id(SiteTree::class, $id); if (!isset($canView[$id])) { - $page = SiteTree::get()->byID($id); - $canView[$id] = $page && $page->canView(new Member()); + $canView[$id] = $page && $page->canView(Security::getCurrentUser()); } if (!$canView[$id]) { continue; } // Get the diff to the previous version. - $version = SiteTree::create($record); + $version = $page; if ($diff = $version->getDiff()) { $changeList->push($diff); } diff --git a/tests/VersionFeedFunctionalTest.php b/tests/VersionFeedFunctionalTest.php index ee06523..89b7d8e 100644 --- a/tests/VersionFeedFunctionalTest.php +++ b/tests/VersionFeedFunctionalTest.php @@ -3,19 +3,18 @@ namespace SilverStripe\VersionFeed\Tests; use Page; - -use SilverStripe\VersionFeed\VersionFeed; -use SilverStripe\VersionFeed\Filters\CachedContentFilter; -use SilverStripe\VersionFeed\Filters\RateLimitFilter; -use SilverStripe\VersionFeed\VersionFeedController; +use Psr\SimpleCache\CacheInterface; +use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Control\Director; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; -use SilverStripe\CMS\Model\SiteTree; -use SilverStripe\Versioned\Versioned; -use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\Dev\FunctionalTest; -use SilverStripe\Control\Director; -use Psr\SimpleCache\CacheInterface; +use SilverStripe\SiteConfig\SiteConfig; +use SilverStripe\Versioned\Versioned; +use SilverStripe\VersionFeed\Filters\CachedContentFilter; +use SilverStripe\VersionFeed\Filters\RateLimitFilter; +use SilverStripe\VersionFeed\VersionFeed; +use SilverStripe\VersionFeed\VersionFeedController; class VersionFeedFunctionalTest extends FunctionalTest { @@ -181,7 +180,7 @@ class VersionFeedFunctionalTest extends FunctionalTest $response = $this->get($page1->RelativeLink('allchanges')); $xml = simplexml_load_string($response->getBody()); $titles = array_map(function ($item) { - return (string)$item->title; + return str_replace('Changed: ', '', (string) $item->title); }, $xml->xpath('//item')); $this->assertContains('Page1', $titles); $this->assertContains('Page2', $titles); @@ -197,21 +196,21 @@ class VersionFeedFunctionalTest extends FunctionalTest ), $seed); $page->update($seed); $page->write(); - $page->publish('Stage', 'Live'); + $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); $page->update(array( 'Title' => 'Changed: ' . $seed['Title'], 'Content' => 'Changed: ' . $seed['Content'], )); $page->write(); - $page->publish('Stage', 'Live'); + $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); $page->update(array( 'Title' => 'Changed again: ' . $seed['Title'], 'Content' => 'Changed again: ' . $seed['Content'], )); $page->write(); - $page->publish('Stage', 'Live'); + $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); $page->update(array( 'Title' => 'Unpublished: ' . $seed['Title'], @@ -223,7 +222,7 @@ class VersionFeedFunctionalTest extends FunctionalTest } /** - * Tests response code for globally disabled feedss + * Tests response code for globally disabled feeds */ public function testFeedViewability() { diff --git a/tests/VersionFeedTest.php b/tests/VersionFeedTest.php index 2e77ebf..e7ee39c 100644 --- a/tests/VersionFeedTest.php +++ b/tests/VersionFeedTest.php @@ -3,15 +3,15 @@ namespace SilverStripe\VersionFeed\Tests; use Page; +use SilverStripe\CMS\Controllers\ContentController; +use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Dev\SapphireTest; +use SilverStripe\Versioned\Versioned; use SilverStripe\VersionFeed\VersionFeed; use SilverStripe\VersionFeed\VersionFeedController; -use SilverStripe\Dev\SapphireTest; -use SilverStripe\CMS\Model\SiteTree; -use SilverStripe\CMS\Controllers\ContentController; class VersionFeedTest extends SapphireTest { - protected $usesDatabase = true; protected static $required_extensions = [ @@ -19,10 +19,6 @@ class VersionFeedTest extends SapphireTest ContentController::class => [VersionFeedController::class], ]; - protected $illegalExtensions = [ - 'SiteTree' => ['Translatable'] - ]; - public function testDiffedChangesExcludesRestrictedItems() { $this->markTestIncomplete(); @@ -37,11 +33,11 @@ class VersionFeedTest extends SapphireTest { $page = new Page(['Title' => 'My Title']); $page->write(); - $page->publish('Stage', 'Live'); + $page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE); $page->Title = 'My Changed Title'; $page->write(); - $page->publish('Stage', 'Live'); + $page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE); $page->Title = 'My Unpublished Changed Title'; $page->write();