FIX Use cached SiteTree object for history instead of creating a temporary object

This commit is contained in:
Robbie Averill 2017-12-13 16:13:46 +13:00
parent 921b7c7fb5
commit dd361929db
3 changed files with 26 additions and 29 deletions

View File

@ -4,6 +4,8 @@ namespace SilverStripe\VersionFeed;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use SilverStripe\Control\RSS\RSSFeed; use SilverStripe\Control\RSS\RSSFeed;
use SilverStripe\ORM\DataObject;
use SilverStripe\Security\Security;
use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\ORM\DB; use SilverStripe\ORM\DB;
use SilverStripe\Security\Member; use SilverStripe\Security\Member;
@ -118,7 +120,7 @@ class VersionFeedController extends Extension
// Cache the diffs to remove DOS possibility. // Cache the diffs to remove DOS possibility.
$key = 'allchanges' $key = 'allchanges'
. preg_replace('#[^a-zA-Z0-9_]#', '', $lastChange['LastEdited']) . 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 = $this->filterContent($key, function () use ($latestChanges) {
$changeList = new ArrayList(); $changeList = new ArrayList();
$canView = array(); $canView = array();
@ -126,16 +128,16 @@ class VersionFeedController extends Extension
// Check if the page should be visible. // Check if the page should be visible.
// WARNING: although we are providing historical details, we check the current configuration. // WARNING: although we are providing historical details, we check the current configuration.
$id = $record['RecordID']; $id = $record['RecordID'];
$page = DataObject::get_by_id(SiteTree::class, $id);
if (!isset($canView[$id])) { if (!isset($canView[$id])) {
$page = SiteTree::get()->byID($id); $canView[$id] = $page && $page->canView(Security::getCurrentUser());
$canView[$id] = $page && $page->canView(new Member());
} }
if (!$canView[$id]) { if (!$canView[$id]) {
continue; continue;
} }
// Get the diff to the previous version. // Get the diff to the previous version.
$version = SiteTree::create($record); $version = $page;
if ($diff = $version->getDiff()) { if ($diff = $version->getDiff()) {
$changeList->push($diff); $changeList->push($diff);
} }

View File

@ -3,19 +3,18 @@
namespace SilverStripe\VersionFeed\Tests; namespace SilverStripe\VersionFeed\Tests;
use Page; use Page;
use Psr\SimpleCache\CacheInterface;
use SilverStripe\VersionFeed\VersionFeed; use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\VersionFeed\Filters\CachedContentFilter; use SilverStripe\Control\Director;
use SilverStripe\VersionFeed\Filters\RateLimitFilter;
use SilverStripe\VersionFeed\VersionFeedController;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\Injector;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Versioned\Versioned;
use SilverStripe\SiteConfig\SiteConfig;
use SilverStripe\Dev\FunctionalTest; use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Control\Director; use SilverStripe\SiteConfig\SiteConfig;
use Psr\SimpleCache\CacheInterface; 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 class VersionFeedFunctionalTest extends FunctionalTest
{ {
@ -181,7 +180,7 @@ class VersionFeedFunctionalTest extends FunctionalTest
$response = $this->get($page1->RelativeLink('allchanges')); $response = $this->get($page1->RelativeLink('allchanges'));
$xml = simplexml_load_string($response->getBody()); $xml = simplexml_load_string($response->getBody());
$titles = array_map(function ($item) { $titles = array_map(function ($item) {
return (string)$item->title; return str_replace('Changed: ', '', (string) $item->title);
}, $xml->xpath('//item')); }, $xml->xpath('//item'));
$this->assertContains('Page1', $titles); $this->assertContains('Page1', $titles);
$this->assertContains('Page2', $titles); $this->assertContains('Page2', $titles);
@ -197,21 +196,21 @@ class VersionFeedFunctionalTest extends FunctionalTest
), $seed); ), $seed);
$page->update($seed); $page->update($seed);
$page->write(); $page->write();
$page->publish('Stage', 'Live'); $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
$page->update(array( $page->update(array(
'Title' => 'Changed: ' . $seed['Title'], 'Title' => 'Changed: ' . $seed['Title'],
'Content' => 'Changed: ' . $seed['Content'], 'Content' => 'Changed: ' . $seed['Content'],
)); ));
$page->write(); $page->write();
$page->publish('Stage', 'Live'); $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
$page->update(array( $page->update(array(
'Title' => 'Changed again: ' . $seed['Title'], 'Title' => 'Changed again: ' . $seed['Title'],
'Content' => 'Changed again: ' . $seed['Content'], 'Content' => 'Changed again: ' . $seed['Content'],
)); ));
$page->write(); $page->write();
$page->publish('Stage', 'Live'); $page->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE);
$page->update(array( $page->update(array(
'Title' => 'Unpublished: ' . $seed['Title'], '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() public function testFeedViewability()
{ {

View File

@ -3,15 +3,15 @@
namespace SilverStripe\VersionFeed\Tests; namespace SilverStripe\VersionFeed\Tests;
use Page; 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\VersionFeed;
use SilverStripe\VersionFeed\VersionFeedController; use SilverStripe\VersionFeed\VersionFeedController;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Controllers\ContentController;
class VersionFeedTest extends SapphireTest class VersionFeedTest extends SapphireTest
{ {
protected $usesDatabase = true; protected $usesDatabase = true;
protected static $required_extensions = [ protected static $required_extensions = [
@ -19,10 +19,6 @@ class VersionFeedTest extends SapphireTest
ContentController::class => [VersionFeedController::class], ContentController::class => [VersionFeedController::class],
]; ];
protected $illegalExtensions = [
'SiteTree' => ['Translatable']
];
public function testDiffedChangesExcludesRestrictedItems() public function testDiffedChangesExcludesRestrictedItems()
{ {
$this->markTestIncomplete(); $this->markTestIncomplete();
@ -37,11 +33,11 @@ class VersionFeedTest extends SapphireTest
{ {
$page = new Page(['Title' => 'My Title']); $page = new Page(['Title' => 'My Title']);
$page->write(); $page->write();
$page->publish('Stage', 'Live'); $page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE);
$page->Title = 'My Changed Title'; $page->Title = 'My Changed Title';
$page->write(); $page->write();
$page->publish('Stage', 'Live'); $page->copyVersionToStage(Versioned::STAGE, Versioned::LIVE);
$page->Title = 'My Unpublished Changed Title'; $page->Title = 'My Unpublished Changed Title';
$page->write(); $page->write();