From 0374d66b3207bd412c3e1ac69558ec657db1c1f4 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 11:19:58 +1300 Subject: [PATCH] Convert to new cache layer, clean up other overlooked points in the ss4 upgrade --- .gitignore | 1 - .travis.yml | 2 +- _config/injector.yml | 8 ++++++-- src/Controllers/CMSExternalLinks.php | 9 +++++---- src/Jobs/CheckExternalLinksJob.php | 2 +- src/Model/BrokenExternalLink.php | 4 ++-- src/Model/BrokenExternalPageTrackStatus.php | 12 ++++++++---- src/Reports/BrokenExternalLinksReport.php | 7 +++---- src/Tasks/CheckExternalLinksTask.php | 2 +- src/Tasks/CurlLinkChecker.php | 12 ++++-------- tests/ExternalLinksTest.php | 6 +++--- 11 files changed, 34 insertions(+), 31 deletions(-) delete mode 100644 .gitignore diff --git a/.gitignore b/.gitignore deleted file mode 100644 index e43b0f9..0000000 --- a/.gitignore +++ /dev/null @@ -1 +0,0 @@ -.DS_Store diff --git a/.travis.yml b/.travis.yml index 8a176d9..adfb908 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,7 +29,7 @@ before_script: script: - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit; fi - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi - - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs --ignore=install.php src/ tests/ *.php; fi + - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs src/ tests/ *.php; fi after_success: - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi diff --git a/_config/injector.yml b/_config/injector.yml index ab86975..45b3195 100644 --- a/_config/injector.yml +++ b/_config/injector.yml @@ -1,5 +1,9 @@ --- Name: externallinksdependencies --- -Injector: - LinkChecker: CurlLinkChecker +SilverStripe\Core\Injector\Injector: + LinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker + Psr\SimpleCache\CacheInterface.CurlLinkChecker: + factory: SilverStripe\Core\Cache\CacheFactory + constructor: + namespace: 'curllinkchecker' diff --git a/src/Controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinks.php index 595e4db..2bafb3f 100644 --- a/src/Controllers/CMSExternalLinks.php +++ b/src/Controllers/CMSExternalLinks.php @@ -7,13 +7,14 @@ use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; use SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob; use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; use SilverStripe\Control\Controller; +use Symbiote\QueuedJobs\Services\QueuedJobService; class CMSExternalLinksController extends Controller { private static $allowed_actions = array('getJobStatus', 'start'); - /* + /** * Respond to Ajax requests for info on a running job * * @return string JSON string detailing status of the job @@ -41,7 +42,7 @@ class CMSExternalLinksController extends Controller } - /* + /** * Starts a broken external link check */ public function start() @@ -53,11 +54,11 @@ class CMSExternalLinksController extends Controller } // Create a new job - if (class_exists('QueuedJobService')) { + if (class_exists(QueuedJobService::class)) { // Force the creation of a new run BrokenExternalPageTrackStatus::create_status(); $checkLinks = new CheckExternalLinksJob(); - singleton('QueuedJobService')->queueJob($checkLinks); + singleton(QueuedJobService::class)->queueJob($checkLinks); } else { //TODO this hangs as it waits for the connection to be released // should return back and continue processing diff --git a/src/Jobs/CheckExternalLinksJob.php b/src/Jobs/CheckExternalLinksJob.php index fb2bfc2..642ce3e 100644 --- a/src/Jobs/CheckExternalLinksJob.php +++ b/src/Jobs/CheckExternalLinksJob.php @@ -19,7 +19,7 @@ class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob public function getTitle() { - return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); + return _t(__CLASS__ . '.TITLE', 'Checking for external broken links'); } public function getJobType() diff --git a/src/Model/BrokenExternalLink.php b/src/Model/BrokenExternalLink.php index 50a3c39..5d39f09 100644 --- a/src/Model/BrokenExternalLink.php +++ b/src/Model/BrokenExternalLink.php @@ -71,13 +71,13 @@ class BrokenExternalLink extends DataObject $code = $this->HTTPCode; if (empty($code)) { // Assume that $code = 0 means there was no response - $description = _t('BrokenExternalLink.NOTAVAILABLE', 'Server Not Available'); + $description = _t(__CLASS__ . '.NOTAVAILABLE', 'Server Not Available'); } elseif (($descriptions = Config::inst()->get(HTTPResponse::class, 'status_codes')) && isset($descriptions[$code]) ) { $description = $descriptions[$code]; } else { - $description = _t('BrokenExternalLink.UNKNOWNRESPONSE', 'Unknown Response Code'); + $description = _t(__CLASS__ . '.UNKNOWNRESPONSE', 'Unknown Response Code'); } return sprintf("%d (%s)", $code, $description); } diff --git a/src/Model/BrokenExternalPageTrackStatus.php b/src/Model/BrokenExternalPageTrackStatus.php index b1ef680..0a159be 100644 --- a/src/Model/BrokenExternalPageTrackStatus.php +++ b/src/Model/BrokenExternalPageTrackStatus.php @@ -33,7 +33,7 @@ class BrokenExternalPageTrackStatus extends DataObject /** * Get the latest track status * - * @return self + * @return BrokenExternalPageTrackStatus */ public static function get_latest() { @@ -72,6 +72,8 @@ class BrokenExternalPageTrackStatus extends DataObject /** * Get total pages count + * + * @return int */ public function getTotalPages() { @@ -80,6 +82,8 @@ class BrokenExternalPageTrackStatus extends DataObject /** * Get completed pages count + * + * @return int */ public function getCompletedPages() { @@ -92,7 +96,7 @@ class BrokenExternalPageTrackStatus extends DataObject /** * Returns the latest run, or otherwise creates a new one * - * @return self + * @return BrokenExternalPageTrackStatus */ public static function get_or_create() { @@ -106,10 +110,10 @@ class BrokenExternalPageTrackStatus extends DataObject return self::create_status(); } - /* + /** * Create and prepare a new status * - * @return self + * @return BrokenExternalPageTrackStatus */ public static function create_status() { diff --git a/src/Reports/BrokenExternalLinksReport.php b/src/Reports/BrokenExternalLinksReport.php index 1d2a57e..8b9ab90 100644 --- a/src/Reports/BrokenExternalLinksReport.php +++ b/src/Reports/BrokenExternalLinksReport.php @@ -12,7 +12,6 @@ use SilverStripe\Reports\Report; /** * Content side-report listing pages with external broken links * @package externallinks - * @subpackage content */ class BrokenExternalLinksReport extends Report @@ -25,7 +24,7 @@ class BrokenExternalLinksReport extends Report */ public function title() { - return _t('ExternalBrokenLinksReport.EXTERNALBROKENLINKS', "External broken links report"); + return _t(__CLASS__ . '.EXTERNALBROKENLINKS', "External broken links report"); } public function columns() @@ -78,7 +77,7 @@ class BrokenExternalLinksReport extends Report public function getCMSFields() { - Requirements::javascript('externallinks/javascript/BrokenExternalLinksReport.js'); + Requirements::javascript('silverstripe/externallinks: javascript/BrokenExternalLinksReport.js'); $fields = parent::getCMSFields(); $reportResultSpan = '

'; @@ -90,7 +89,7 @@ class BrokenExternalLinksReport extends Report 'runReport', sprintf( $button, - _t('ExternalBrokenLinksReport.RUNREPORT', 'Create new report') + _t(__CLASS__ . '.RUNREPORT', 'Create new report') ) ); $fields->push($runReportButton); diff --git a/src/Tasks/CheckExternalLinksTask.php b/src/Tasks/CheckExternalLinksTask.php index 617f12f..a588e86 100644 --- a/src/Tasks/CheckExternalLinksTask.php +++ b/src/Tasks/CheckExternalLinksTask.php @@ -137,7 +137,7 @@ class CheckExternalLinksTask extends BuildTask } // do we have any whitelisted codes - $ignoreCodes = Config::inst()->get('CheckExternalLinks', 'IgnoreCodes'); + $ignoreCodes = $this->config()->get('IgnoreCodes'); if (is_array($ignoreCodes) && in_array($httpCode, $ignoreCodes)) { return false; } diff --git a/src/Tasks/CurlLinkChecker.php b/src/Tasks/CurlLinkChecker.php index d86d26b..2e396a8 100644 --- a/src/Tasks/CurlLinkChecker.php +++ b/src/Tasks/CurlLinkChecker.php @@ -2,7 +2,7 @@ namespace SilverStripe\ExternalLinks\Tasks; -use SS_Cache; +use Psr\SimpleCache\CacheInterface; /** * Check links using curl @@ -17,11 +17,7 @@ class CurlLinkChecker implements LinkChecker */ protected function getCache() { - return SS_Cache::factory( - __CLASS__, - 'Output', - array('automatic_serialization' => true) - ); + return Injector::inst()->get(CacheInterface::class . '.CurlLinkChecker'); } /** @@ -39,7 +35,7 @@ class CurlLinkChecker implements LinkChecker // Check if we have a cached result $cacheKey = md5($href); - $result = $this->getCache()->load($cacheKey); + $result = $this->getCache()->get($cacheKey); if ($result !== false) { return $result; } @@ -54,7 +50,7 @@ class CurlLinkChecker implements LinkChecker curl_close($handle); // Cache result - $this->getCache()->save($httpCode, $cacheKey); + $this->getCache()->set($httpCode, $cacheKey); return $httpCode; } } diff --git a/tests/ExternalLinksTest.php b/tests/ExternalLinksTest.php index ad26e1a..d4de5e5 100644 --- a/tests/ExternalLinksTest.php +++ b/tests/ExternalLinksTest.php @@ -31,7 +31,7 @@ class ExternalLinksTest extends SapphireTest parent::setUpOnce(); } - public function setUp() + protected function setUp() { parent::setUp(); @@ -101,7 +101,7 @@ class ExternalLinksTest extends SapphireTest // Check all pages have had the correct HTML adjusted for ($i = 1; $i <= 5; $i++) { - $page = $this->objFromFixture('ExternalLinksTestPage', 'page'.$i); + $page = $this->objFromFixture(ExternalLinksTestPage::class, 'page'.$i); $this->assertNotEmpty($page->Content); $this->assertEquals( $page->ExpectedContent, @@ -154,7 +154,7 @@ class ExternalLinksTest extends SapphireTest $reports = Report::get_reports(); $reportNames = array(); foreach ($reports as $report) { - $reportNames[] = $report->class; + $reportNames[] = get_class($report); } $this->assertContains( BrokenExternalLinksReport::class,