From 17699f2b8aa043f2024a6b5df56513abfabdab86 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 11 Dec 2017 12:50:45 +1300 Subject: [PATCH] Update from SS_Cache to Symfony/Cache --- src/Filters/CachedContentFilter.php | 5 ++- src/Filters/ContentFilter.php | 13 +++---- src/Filters/RateLimitFilter.php | 8 ++-- src/VersionFeed.php | 4 +- src/VersionFeedController.php | 20 ++-------- tests/VersionFeedFunctionalTest.php | 57 +++++++++++++++-------------- tests/VersionFeedTest.php | 2 +- 7 files changed, 49 insertions(+), 60 deletions(-) diff --git a/src/Filters/CachedContentFilter.php b/src/Filters/CachedContentFilter.php index 73dc2a5..acfb21f 100644 --- a/src/Filters/CachedContentFilter.php +++ b/src/Filters/CachedContentFilter.php @@ -28,12 +28,13 @@ class CachedContentFilter extends ContentFilter { $cacheEnabled = Config::inst()->get(get_class(), 'cache_enabled'); $result = (isset($_GET['flush']) || !$cacheEnabled) ? null - : $cache->load($key); + : $cache->get($key); if($result) return $result; // Fallback to generate result $result = parent::getContent($key, $callback); - $cache->save($result, $key); + $lifetime = Config::inst()->get(ContentFilter::class, 'cache_lifetime') ?: null; + $cache->set($result, $key, $lifetime); return $result; } } diff --git a/src/Filters/ContentFilter.php b/src/Filters/ContentFilter.php index e2670f3..e43edda 100644 --- a/src/Filters/ContentFilter.php +++ b/src/Filters/ContentFilter.php @@ -6,7 +6,8 @@ use SS_Cache; use SilverStripe\VersionFeed\VersionFeedController; use SilverStripe\Core\Config\Config; - +use Psr\SimpleCache\CacheInterface; +use SilverStripe\Core\Injector\Injector; @@ -41,13 +42,9 @@ abstract class ContentFilter { * @return Zend_Cache_Frontend */ protected function getCache() { - $cache = \SS_Cache::factory(VersionFeedController::class); - $cache->setOption('automatic_serialization', true); - - // Map 0 to null for unlimited lifetime - $lifetime = Config::inst()->get(get_class($this), 'cache_lifetime') ?: null; - $cache->setLifetime($lifetime); - return $cache; + return Injector::inst()->get( + CacheInterface::class . '.' . VersionFeedController::class + ); } /** diff --git a/src/Filters/RateLimitFilter.php b/src/Filters/RateLimitFilter.php index d6a232b..c03be35 100644 --- a/src/Filters/RateLimitFilter.php +++ b/src/Filters/RateLimitFilter.php @@ -94,7 +94,7 @@ class RateLimitFilter extends ContentFilter { // Generate result with rate limiting enabled $limitKey = $this->getCacheKey($key); $cache = $this->getCache(); - if($lockedUntil = $cache->load($limitKey)) { + if($lockedUntil = $cache->get($limitKey)) { if(time() < $lockedUntil) { // Politely inform visitor of limit $response = new HTTPResponse_Exception('Too Many Requests.', 429); @@ -102,9 +102,11 @@ class RateLimitFilter extends ContentFilter { throw $response; } } + + $lifetime = Config::inst()->get(ContentFilter::class, 'cache_lifetime') ?: null; // Apply rate limit - $cache->save(time() + $timeout, $limitKey); + $cache->set(time() + $timeout, $limitKey, $lifetime); // Generate results $result = parent::getContent($key, $callback); @@ -112,7 +114,7 @@ class RateLimitFilter extends ContentFilter { // Reset rate limit with optional cooldown if($cooldown = Config::inst()->get(get_class(), 'lock_cooldown')) { // Set cooldown on successful query execution - $cache->save(time() + $cooldown, $limitKey); + $cache->set(time() + $cooldown, $limitKey, $lifetime); } else { // Without cooldown simply disable lock $cache->remove($limitKey); diff --git a/src/VersionFeed.php b/src/VersionFeed.php index a13989b..3c71eb1 100644 --- a/src/VersionFeed.php +++ b/src/VersionFeed.php @@ -121,7 +121,7 @@ class VersionFeed extends SiteTreeExtension { $changed = true; } - // Copy the link so it can be cached by SS_Cache. + // Copy the link so it can be cached. $version->GeneratedLink = $version->AbsoluteLink(); } @@ -142,7 +142,7 @@ class VersionFeed extends SiteTreeExtension { $first = clone($previous); $first->DiffContent = new HTMLText(); $first->DiffContent->setValue('
' . $first->Content . '
'); - // Copy the link so it can be cached by SS_Cache. + // Copy the link so it can be cached. $first->GeneratedLink = $first->AbsoluteLink(); $changeList->push($first); } diff --git a/src/VersionFeedController.php b/src/VersionFeedController.php index bd551ca..6b6a0b4 100644 --- a/src/VersionFeedController.php +++ b/src/VersionFeedController.php @@ -2,17 +2,6 @@ namespace SilverStripe\VersionFeed; - - - - - - - - - - - use SilverStripe\Core\Config\Config; use SilverStripe\VersionFeed\VersionFeed; use SilverStripe\Control\RSS\RSSFeed; @@ -25,8 +14,7 @@ use SilverStripe\Versioned\Versioned_Version; use SilverStripe\Core\Convert; use SilverStripe\View\Requirements; use SilverStripe\Core\Extension; - - +use SilverStripe\VersionFeed\Filters\ContentFilter; class VersionFeed_Controller extends Extension { @@ -38,16 +26,16 @@ class VersionFeed_Controller extends Extension { /** * Content handler * - * @var \VersionFeed\Filters\ContentFilter + * @var ContentFilter */ protected $contentFilter; /** * Sets the content filter * - * @param \VersionFeed\Filters\ContentFilter $contentFilter + * @param ContentFilter $contentFilter */ - public function setContentFilter(\VersionFeed\Filters\ContentFilter $contentFilter) { + public function setContentFilter(ContentFilter $contentFilter) { $this->contentFilter = $contentFilter; } diff --git a/tests/VersionFeedFunctionalTest.php b/tests/VersionFeedFunctionalTest.php index e53be6e..e9a7344 100644 --- a/tests/VersionFeedFunctionalTest.php +++ b/tests/VersionFeedFunctionalTest.php @@ -2,16 +2,14 @@ namespace SilverStripe\VersionFeed\Tests; - -use SS_Cache; -use Zend_Cache; - - use Page; use SilverStripe\VersionFeed\VersionFeed; +use SilverStripe\VersionFeed\Filters\CachedContentFilter; +use SilverStripe\VersionFeed\Filters\RateLimitFilter; use SilverStripe\VersionFeed\VersionFeedController; use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Injector\Injector; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Versioned\Versioned; use SilverStripe\SiteConfig\SiteConfig; @@ -21,7 +19,7 @@ use SilverStripe\Dev\FunctionalTest; class VersionFeedFunctionalTest extends FunctionalTest { protected $usesDatabase = true; - protected $requiredExtensions = array( + protected $required_extensions = array( 'Page' => array(VersionFeed::class), 'Page_Controller' => array(VersionFeedController::class), ); @@ -31,8 +29,10 @@ class VersionFeedFunctionalTest extends FunctionalTest { public function setUp() { parent::setUp(); - $cache = SS_Cache::factory(VersionFeedController::class); - $cache->clean(Zend_Cache::CLEANING_MODE_ALL); + $cache = Injector::inst()->get( + CacheInterface::class . '.' . VersionFeedController::class + ); + $cache->clear(); $this->userIP = isset($_SERVER['HTTP_CLIENT_IP']) ? $_SERVER['HTTP_CLIENT_IP'] : null; @@ -42,11 +42,11 @@ class VersionFeedFunctionalTest extends FunctionalTest { Config::inst()->update(VersionFeed::class, 'allchanges_enabled', true); // Disable caching and locking by default - Config::inst()->update('VersionFeed\Filters\CachedContentFilter', 'cache_enabled', false); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_timeout', 0); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_bypage', false); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', false); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_cooldown', false); + Config::inst()->update(CachedContentFilter::class, 'cache_enabled', false); + Config::inst()->update(RateLimitFilter::class, 'lock_timeout', 0); + Config::inst()->update(RateLimitFilter::class, 'lock_bypage', false); + Config::inst()->update(RateLimitFilter::class, 'lock_byuserip', false); + Config::inst()->update(RateLimitFilter::class, 'lock_cooldown', false); } public function tearDown() { @@ -83,17 +83,18 @@ class VersionFeedFunctionalTest extends FunctionalTest { public function testRateLimiting() { // Re-enable locking just for this test - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_timeout', 20); - Config::inst()->update('VersionFeed\Filters\CachedContentFilter', 'cache_enabled', true); + Config::inst()->update(RateLimitFilter::class, 'lock_timeout', 20); + Config::inst()->update(CachedContentFilter::class, 'cache_enabled', true); $page1 = $this->createPageWithChanges(array('PublicHistory' => true, 'Title' => 'Page1')); $page2 = $this->createPageWithChanges(array('PublicHistory' => true, 'Title' => 'Page2')); // Artifically set cache lock - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', false); - $cache = SS_Cache::factory(VersionFeedController::class); - $cache->setOption('automatic_serialization', true); - $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX); + Config::inst()->update(RateLimitFilter::class, 'lock_byuserip', false); + $cache = Injector::inst()->get( + CacheInterface::class . '.' . VersionFeedController::class + ); + $cache->set(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX); // Test normal hit $response = $this->get($page1->RelativeLink('changes')); @@ -104,39 +105,39 @@ class VersionFeedFunctionalTest extends FunctionalTest { $this->assertGreaterThan(0, $response->getHeader('Retry-After')); // Test page specific lock - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_bypage', true); + Config::inst()->update(RateLimitFilter::class, 'lock_bypage', true); $key = implode('_', array( 'changes', $page1->ID, Versioned::get_versionnumber_by_stage(SiteTree::class, 'Live', $page1->ID, false) )); $key = \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5($key); - $cache->save(time() + 10, $key); + $cache->set(time() + 10, $key); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(429, $response->getStatusCode()); $this->assertGreaterThan(0, $response->getHeader('Retry-After')); $response = $this->get($page2->RelativeLink('changes')); $this->assertEquals(200, $response->getStatusCode()); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_bypage', false); + Config::inst()->update(RateLimitFilter::class, 'lock_bypage', false); // Test rate limit hit by IP - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', true); + Config::inst()->update(RateLimitFilter::class, 'lock_byuserip', true); $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.1'; - $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); + $cache->set(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(429, $response->getStatusCode()); $this->assertGreaterThan(0, $response->getHeader('Retry-After')); // Test rate limit doesn't hit other IP $_SERVER['HTTP_CLIENT_IP'] = '127.0.0.20'; - $cache->save(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); + $cache->set(time() + 10, \VersionFeed\Filters\RateLimitFilter::CACHE_PREFIX . '_' . md5('127.0.0.1')); $response = $this->get($page1->RelativeLink('changes')); $this->assertEquals(200, $response->getStatusCode()); // Restore setting - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_byuserip', false); - Config::inst()->update('VersionFeed\Filters\RateLimitFilter', 'lock_timeout', 0); - Config::inst()->update('VersionFeed\Filters\CachedContentFilter', 'cache_enabled', false); + Config::inst()->update(RateLimitFilter::class, 'lock_byuserip', false); + Config::inst()->update(RateLimitFilter::class, 'lock_timeout', 0); + Config::inst()->update(CachedContentFilter::class, 'cache_enabled', false); } public function testContainsChangesForPageOnly() { diff --git a/tests/VersionFeedTest.php b/tests/VersionFeedTest.php index 48cb591..ef45d19 100644 --- a/tests/VersionFeedTest.php +++ b/tests/VersionFeedTest.php @@ -13,7 +13,7 @@ class VersionFeedTest extends SapphireTest { protected $usesDatabase = true; - protected $requiredExtensions = array( + protected $required_extensions = array( 'SiteTree' => array(VersionFeed::class), 'ContentController' => array(VersionFeedController::class), );