From 3c69b3fa77da96618cb9bf486d67c0d709375083 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Wed, 22 Nov 2017 11:07:33 +1300 Subject: [PATCH 01/15] begin SilverStripe v4 compatibility update --- composer.json | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index e358e3c..757ad21 100644 --- a/composer.json +++ b/composer.json @@ -1,7 +1,7 @@ { "name": "silverstripe/externallinks", "description": "Adds tracking of external broken links to the SilverStripe CMS", - "type": "silverstripe-module", + "type": "silverstripe-vendormodule", "keywords": ["silverstripe", "broken", "links", "href"], "license": "BSD-3-Clause", "authors": [ @@ -11,19 +11,18 @@ } ], "require": { - "silverstripe/framework": "~3.1", - "silverstripe/cms": "~3.1" + "silverstripe/recipe-cms": "^1.0" }, "require-dev": { "hafriedlander/silverstripe-phockito": "*", - "phpunit/PHPUnit": "~3.7@stable" + "phpunit/PHPUnit": "^3.7" }, "suggest": { "silverstripe/queuedjobs": "Speeds up running the job for Content Editors fropm the report" }, "extra": { "branch-alias": { - "dev-master": "1.0.x-dev" + "dev-master": "2.0.x-dev" } } } From 4b59fdba02a381abdba5968eeea9b05e466c68fc Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Wed, 22 Nov 2017 13:05:03 +1300 Subject: [PATCH 02/15] Update supporting items for SilverStripe 4 conventions Update versions and configurations for tests and code checkers, such as phpunit and the SilverStripe CI tools. Altered the layout of the repository to be more in line with other SilverStripe 4 modules (including core ones). --- .gitattributes | 1 + .gitignore | 1 + .scrutinizer.yml | 70 +++---------------- .travis.yml | 47 ++++++------- .tx/config | 8 --- codecov.yml | 2 + composer.json | 64 ++++++++++------- license.md | 2 +- phpcs.xml.dist | 9 +++ phpunit.xml.dist | 13 ++++ .../Controllers}/CMSExternalLinks.php | 0 .../Jobs}/CheckExternalLinksJob.php | 0 .../Model}/BrokenExternalLink.php | 0 .../Model}/BrokenExternalPageTrack.php | 0 .../Model}/BrokenExternalPageTrackStatus.php | 0 .../Reports}/BrokenExternalLinksReport.php | 0 .../Tasks}/CheckExternalLinksTask.php | 0 {code/tasks => src/Tasks}/CurlLinkChecker.php | 0 {code/tasks => src/Tasks}/LinkChecker.php | 0 19 files changed, 96 insertions(+), 121 deletions(-) create mode 100644 .gitignore delete mode 100644 .tx/config create mode 100644 codecov.yml create mode 100644 phpcs.xml.dist create mode 100644 phpunit.xml.dist rename {code/controllers => src/Controllers}/CMSExternalLinks.php (100%) rename {code/jobs => src/Jobs}/CheckExternalLinksJob.php (100%) rename {code/model => src/Model}/BrokenExternalLink.php (100%) rename {code/model => src/Model}/BrokenExternalPageTrack.php (100%) rename {code/model => src/Model}/BrokenExternalPageTrackStatus.php (100%) rename {code/reports => src/Reports}/BrokenExternalLinksReport.php (100%) rename {code/tasks => src/Tasks}/CheckExternalLinksTask.php (100%) rename {code/tasks => src/Tasks}/CurlLinkChecker.php (100%) rename {code/tasks => src/Tasks}/LinkChecker.php (100%) diff --git a/.gitattributes b/.gitattributes index 475f5f2..89eb187 100644 --- a/.gitattributes +++ b/.gitattributes @@ -4,3 +4,4 @@ /.gitignore export-ignore /.travis.yml export-ignore /.scrutinizer.yml export-ignore +/codecov.yml export-ignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e43b0f9 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +.DS_Store diff --git a/.scrutinizer.yml b/.scrutinizer.yml index d1ebd80..051ef9a 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -1,69 +1,15 @@ inherit: true +build: + nodes: + analysis: + tests: + override: [php-scrutinizer-run] + checks: php: - verify_property_names: true - verify_argument_usable_as_reference: true - verify_access_scope_valid: true - useless_calls: true - use_statement_alias_conflict: true - variable_existence: true - unused_variables: true - unused_properties: true - unused_parameters: true - unused_methods: true - unreachable_code: true - too_many_arguments: true - sql_injection_vulnerabilities: true - simplify_boolean_return: true - side_effects_or_types: true - security_vulnerabilities: true - return_doc_comments: true - return_doc_comment_if_not_inferrable: true - require_scope_for_properties: true - require_scope_for_methods: true - require_php_tag_first: true - psr2_switch_declaration: true - psr2_class_declaration: true - property_assignments: true - prefer_while_loop_over_for_loop: true - precedence_mistakes: true - precedence_in_conditions: true - phpunit_assertions: true - php5_style_constructor: true - parse_doc_comments: true - parameter_non_unique: true - parameter_doc_comments: true - param_doc_comment_if_not_inferrable: true - optional_parameters_at_the_end: true - one_class_per_file: true - no_unnecessary_if: true - no_trailing_whitespace: true - no_property_on_interface: true - no_non_implemented_abstract_methods: true - no_error_suppression: true - no_duplicate_arguments: true - no_commented_out_code: true - newline_at_end_of_file: true - missing_arguments: true - method_calls_on_non_object: true - instanceof_class_exists: true - foreach_traversable: true - fix_line_ending: true - fix_doc_comments: true - duplication: true - deprecated_code_usage: true - deadlock_detection_in_loops: true code_rating: true - closure_use_not_conflicting: true - catch_class_exists: true - blank_line_after_namespace_declaration: false - avoid_multiple_statements_on_same_line: true - avoid_duplicate_types: true - avoid_conflicting_incrementers: true - avoid_closing_tag: true - assignment_of_null_return: true - argument_type_checks: true + duplication: true filter: - paths: [code/*, tests/*] + paths: [src/*, tests/*] diff --git a/.travis.yml b/.travis.yml index 75153e6..8a176d9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,36 +1,35 @@ -# See https://github.com/silverstripe-labs/silverstripe-travis-support for setup details - -sudo: false - language: php -php: - - 5.3 - - 5.4 - - 5.5 - - 5.6 - - 7.0 - env: - - DB=MYSQL CORE_RELEASE=3.2 + global: + - COMPOSER_ROOT_VERSION=2.0.x-dev matrix: include: - php: 5.6 - env: DB=MYSQL CORE_RELEASE=3 - - php: 5.6 - env: DB=MYSQL CORE_RELEASE=3.1 - - php: 5.6 - env: DB=PGSQL CORE_RELEASE=3.2 - allow_failures: + env: DB=MYSQL PHPCS_TEST=1 PHPUNIT_TEST=1 - php: 7.0 + env: DB=MYSQL PHPUNIT_TEST=1 + - php: 7.1 + env: DB=PGSQL PHPUNIT_COVERAGE_TEST=1 + - php: 7.2 + env: DB=MYSQL PHPUNIT_TEST=1 before_script: - - composer self-update || true - - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support - - php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss - - cd ~/builds/ss - - composer install + # Init PHP + - phpenv rehash + - phpenv config-rm xdebug.ini + + # Install composer dependencies + - composer validate + - composer require --no-update silverstripe/installer 4.0.x-dev + - if [[ $DB == PGSQL ]]; then composer require --no-update silverstripe/postgresql 2.0.x-dev; fi + - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile script: - - vendor/bin/phpunit externallinks/tests + - 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 + +after_success: + - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi diff --git a/.tx/config b/.tx/config deleted file mode 100644 index 360a807..0000000 --- a/.tx/config +++ /dev/null @@ -1,8 +0,0 @@ -[main] -host = https://www.transifex.com - -[silverstripe-externallinks.master] -file_filter = lang/.yml -source_file = lang/en.yml -source_lang = en -type = YML diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..230b4ed --- /dev/null +++ b/codecov.yml @@ -0,0 +1,2 @@ +comment: false + diff --git a/composer.json b/composer.json index 757ad21..4ccbd0d 100644 --- a/composer.json +++ b/composer.json @@ -1,28 +1,40 @@ { - "name": "silverstripe/externallinks", - "description": "Adds tracking of external broken links to the SilverStripe CMS", - "type": "silverstripe-vendormodule", - "keywords": ["silverstripe", "broken", "links", "href"], - "license": "BSD-3-Clause", - "authors": [ - { - "name": "Kirk Mayo", - "email": "kirk@silverstripe.com" - } - ], - "require": { - "silverstripe/recipe-cms": "^1.0" - }, - "require-dev": { - "hafriedlander/silverstripe-phockito": "*", - "phpunit/PHPUnit": "^3.7" - }, - "suggest": { - "silverstripe/queuedjobs": "Speeds up running the job for Content Editors fropm the report" - }, - "extra": { - "branch-alias": { - "dev-master": "2.0.x-dev" - } - } + "name": "silverstripe/externallinks", + "description": + "Adds tracking of broken external links to the SilverStripe CMS", + "type": "silverstripe-vendormodule", + "keywords": ["silverstripe", "broken", "links", "href"], + "license": "BSD-3-Clause", + "authors": [ + { + "name": "Kirk Mayo", + "email": "kirk@silverstripe.com" + } + ], + "require": { + "silverstripe/recipe-cms": "^1.0" + }, + "require-dev": { + "hafriedlander/silverstripe-phockito": "*", + "phpunit/PHPUnit": "^5.7", + "squizlabs/php_codesniffer": "^3.0" + }, + "suggest": { + "silverstripe/queuedjobs": + "Provides a more efficient method of generating/updating the report" + }, + "autoload": { + "psr-4": { + "SilverStripe\\ExternalLinks\\": "src/", + "SilverStripe\\ExternalLinks\\Tests\\": "tests/" + } + }, + "extra": { + "expose": ["javascript"], + "branch-alias": { + "dev-master": "2.0.x-dev" + } + }, + "minimum-stability": "dev", + "prefer-stable": true } diff --git a/license.md b/license.md index 9445c8e..8794670 100644 --- a/license.md +++ b/license.md @@ -1,4 +1,4 @@ -Copyright (c) 2016, SilverStripe Limited +Copyright (c) 2017, SilverStripe Limited All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..1b984f8 --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,9 @@ + + + CodeSniffer ruleset for SilverStripe coding conventions. + + + + + + diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..b543ea6 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,13 @@ + + + tests/ + + + + src/ + + tests/ + + + + diff --git a/code/controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinks.php similarity index 100% rename from code/controllers/CMSExternalLinks.php rename to src/Controllers/CMSExternalLinks.php diff --git a/code/jobs/CheckExternalLinksJob.php b/src/Jobs/CheckExternalLinksJob.php similarity index 100% rename from code/jobs/CheckExternalLinksJob.php rename to src/Jobs/CheckExternalLinksJob.php diff --git a/code/model/BrokenExternalLink.php b/src/Model/BrokenExternalLink.php similarity index 100% rename from code/model/BrokenExternalLink.php rename to src/Model/BrokenExternalLink.php diff --git a/code/model/BrokenExternalPageTrack.php b/src/Model/BrokenExternalPageTrack.php similarity index 100% rename from code/model/BrokenExternalPageTrack.php rename to src/Model/BrokenExternalPageTrack.php diff --git a/code/model/BrokenExternalPageTrackStatus.php b/src/Model/BrokenExternalPageTrackStatus.php similarity index 100% rename from code/model/BrokenExternalPageTrackStatus.php rename to src/Model/BrokenExternalPageTrackStatus.php diff --git a/code/reports/BrokenExternalLinksReport.php b/src/Reports/BrokenExternalLinksReport.php similarity index 100% rename from code/reports/BrokenExternalLinksReport.php rename to src/Reports/BrokenExternalLinksReport.php diff --git a/code/tasks/CheckExternalLinksTask.php b/src/Tasks/CheckExternalLinksTask.php similarity index 100% rename from code/tasks/CheckExternalLinksTask.php rename to src/Tasks/CheckExternalLinksTask.php diff --git a/code/tasks/CurlLinkChecker.php b/src/Tasks/CurlLinkChecker.php similarity index 100% rename from code/tasks/CurlLinkChecker.php rename to src/Tasks/CurlLinkChecker.php diff --git a/code/tasks/LinkChecker.php b/src/Tasks/LinkChecker.php similarity index 100% rename from code/tasks/LinkChecker.php rename to src/Tasks/LinkChecker.php From e6ddf0fe4788d038395702c7404fd6d5549d88eb Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Wed, 22 Nov 2017 14:26:15 +1300 Subject: [PATCH 03/15] Add namespaces via `upgrade-code add-namespace` tool Running this command for each directory in the `src` folder we generate a valid `.upgrade.yml` which can be used by other modules that may use this one as a dependency to upgrade themselves without issue. --- .upgrade.yml | 10 ++++++++++ src/Controllers/CMSExternalLinks.php | 9 +++++++++ src/Jobs/CheckExternalLinksJob.php | 7 +++++++ src/Model/BrokenExternalLink.php | 8 ++++++++ src/Model/BrokenExternalPageTrack.php | 6 ++++++ src/Model/BrokenExternalPageTrackStatus.php | 6 ++++++ src/Reports/BrokenExternalLinksReport.php | 10 ++++++++++ src/Tasks/CheckExternalLinksTask.php | 13 +++++++++++++ src/Tasks/CurlLinkChecker.php | 5 +++++ src/Tasks/LinkChecker.php | 5 +++++ 10 files changed, 79 insertions(+) create mode 100644 .upgrade.yml diff --git a/.upgrade.yml b/.upgrade.yml new file mode 100644 index 0000000..ce4f759 --- /dev/null +++ b/.upgrade.yml @@ -0,0 +1,10 @@ +mappings: + CMSExternalLinks_Controller: SilverStripe\ExternalLinks\Controllers\CMSExternalLinks_Controller + CheckExternalLinksJob: SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob + BrokenExternalLink: SilverStripe\ExternalLinks\Model\BrokenExternalLink + BrokenExternalPageTrack: SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack + BrokenExternalPageTrackStatus: SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus + BrokenExternalLinksReport: SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport + CheckExternalLinksTask: SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask + CurlLinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker + LinkChecker: SilverStripe\ExternalLinks\Tasks\LinkChecker diff --git a/src/Controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinks.php index 2b0efb6..9b59eaf 100644 --- a/src/Controllers/CMSExternalLinks.php +++ b/src/Controllers/CMSExternalLinks.php @@ -1,5 +1,14 @@ Date: Thu, 23 Nov 2017 12:56:44 +1300 Subject: [PATCH 04/15] Run the upgrader & linting tools An initial (untested) run at a proper upgrade. Progress commit. --- src/Controllers/CMSExternalLinks.php | 96 +++--- src/Jobs/CheckExternalLinksJob.php | 52 +-- src/Model/BrokenExternalLink.php | 122 +++---- src/Model/BrokenExternalPageTrack.php | 46 +-- src/Model/BrokenExternalPageTrackStatus.php | 220 ++++++------ src/Reports/BrokenExternalLinksReport.php | 153 +++++---- src/Tasks/CheckExternalLinksTask.php | 361 +++++++++++--------- src/Tasks/CurlLinkChecker.php | 84 ++--- src/Tasks/LinkChecker.php | 22 +- tests/ExternalLinksTest.php | 249 +++++++------- tests/ExternalLinksTestPage.php | 2 + 11 files changed, 750 insertions(+), 657 deletions(-) diff --git a/src/Controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinks.php index 9b59eaf..a725ae9 100644 --- a/src/Controllers/CMSExternalLinks.php +++ b/src/Controllers/CMSExternalLinks.php @@ -2,62 +2,68 @@ namespace SilverStripe\ExternalLinks\Controllers; -use Controller; -use HTTP; -use BrokenExternalPageTrackStatus; -use CheckExternalLinksJob; -use CheckExternalLinksTask; +use SilverStripe\Control\HTTP; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob; +use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; +use SilverStripe\Control\Controller; +class CMSExternalLinks_Controller extends Controller +{ -class CMSExternalLinks_Controller extends Controller { + private static $allowed_actions = array('getJobStatus', 'start'); - 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 */ - public function getJobStatus() { - // Set headers - HTTP::set_cache_age(0); - HTTP::add_cache_headers($this->response); - $this->response - ->addHeader('Content-Type', 'application/json') - ->addHeader('Content-Encoding', 'UTF-8') - ->addHeader('X-Content-Type-Options', 'nosniff'); + public function getJobStatus() + { + // Set headers + HTTP::set_cache_age(0); + HTTP::add_cache_headers($this->response); + $this->response + ->addHeader('Content-Type', 'application/json') + ->addHeader('Content-Encoding', 'UTF-8') + ->addHeader('X-Content-Type-Options', 'nosniff'); - // Format status - $track = BrokenExternalPageTrackStatus::get_latest(); - if($track) return json_encode(array( - 'TrackID' => $track->ID, - 'Status' => $track->Status, - 'Completed' => $track->getCompletedPages(), - 'Total' => $track->getTotalPages() - )); - } + // Format status + $track = BrokenExternalPageTrackStatus::get_latest(); + if ($track) { + return json_encode(array( + 'TrackID' => $track->ID, + 'Status' => $track->Status, + 'Completed' => $track->getCompletedPages(), + 'Total' => $track->getTotalPages() + )); + } + } - /* + /* * Starts a broken external link check */ - public function start() { - // return if the a job is already running - $status = BrokenExternalPageTrackStatus::get_latest(); - if ($status && $status->Status == 'Running') return; + public function start() + { + // return if the a job is already running + $status = BrokenExternalPageTrackStatus::get_latest(); + if ($status && $status->Status == 'Running') { + return; + } - // Create a new job - if (class_exists('QueuedJobService')) { - // Force the creation of a new run - BrokenExternalPageTrackStatus::create_status(); - $checkLinks = new CheckExternalLinksJob(); - singleton('QueuedJobService')->queueJob($checkLinks); - } else { - //TODO this hangs as it waits for the connection to be released - // should return back and continue processing - // http://us3.php.net/manual/en/features.connection-handling.php - $task = CheckExternalLinksTask::create(); - $task->runLinksCheck(); - } - } + // Create a new job + if (class_exists('QueuedJobService')) { + // Force the creation of a new run + BrokenExternalPageTrackStatus::create_status(); + $checkLinks = new CheckExternalLinksJob(); + singleton('QueuedJobService')->queueJob($checkLinks); + } else { + //TODO this hangs as it waits for the connection to be released + // should return back and continue processing + // http://us3.php.net/manual/en/features.connection-handling.php + $task = CheckExternalLinksTask::create(); + $task->runLinksCheck(); + } + } } diff --git a/src/Jobs/CheckExternalLinksJob.php b/src/Jobs/CheckExternalLinksJob.php index 48dd723..662d81d 100644 --- a/src/Jobs/CheckExternalLinksJob.php +++ b/src/Jobs/CheckExternalLinksJob.php @@ -4,38 +4,44 @@ namespace SilverStripe\ExternalLinks\Jobs; use AbstractQueuedJob; use QueuedJob; -use CheckExternalLinksTask; +use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; -if(!class_exists('AbstractQueuedJob')) return; +if (!class_exists('AbstractQueuedJob')) { + return; +} /** * A Job for running a external link check for published pages * */ -class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob { +class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob +{ - public function getTitle() { - return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); - } + public function getTitle() + { + return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); + } - public function getJobType() { - return QueuedJob::QUEUED; - } + public function getJobType() + { + return QueuedJob::QUEUED; + } - public function getSignature() { - return md5(get_class($this)); - } - - /** - * Check an individual page - */ - public function process() { - $task = CheckExternalLinksTask::create(); - $track = $task->runLinksCheck(1); - $this->currentStep = $track->CompletedPages; - $this->totalSteps = $track->TotalPages; - $this->isComplete = $track->Status === 'Completed'; - } + public function getSignature() + { + return md5(get_class($this)); + } + /** + * Check an individual page + */ + public function process() + { + $task = CheckExternalLinksTask::create(); + $track = $task->runLinksCheck(1); + $this->currentStep = $track->CompletedPages; + $this->totalSteps = $track->TotalPages; + $this->isComplete = $track->Status === 'Completed'; + } } diff --git a/src/Model/BrokenExternalLink.php b/src/Model/BrokenExternalLink.php index 93fa065..cb62dad 100644 --- a/src/Model/BrokenExternalLink.php +++ b/src/Model/BrokenExternalLink.php @@ -2,11 +2,13 @@ namespace SilverStripe\ExternalLinks\Model; -use DataObject; -use Member; -use Permission; -use Config; - +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\Security\Member; +use SilverStripe\Security\Permission; +use SilverStripe\Core\Config\Config; +use SilverStripe\Control\HTTPResponse; +use SilverStripe\ORM\DataObject; /** * Represents a single link checked for a single run that is broken @@ -14,66 +16,68 @@ use Config; * @method BrokenExternalPageTrack Track() * @method BrokenExternalPageTrackStatus Status() */ -class BrokenExternalLink extends DataObject { +class BrokenExternalLink extends DataObject +{ - private static $db = array( - 'Link' => 'Varchar(2083)', // 2083 is the maximum length of a URL in Internet Explorer. - 'HTTPCode' =>'Int' - ); + private static $db = array( + 'Link' => 'Varchar(2083)', // 2083 is the maximum length of a URL in Internet Explorer. + 'HTTPCode' =>'Int' + ); - private static $has_one = array( - 'Track' => 'BrokenExternalPageTrack', - 'Status' => 'BrokenExternalPageTrackStatus' - ); + private static $has_one = array( + 'Track' => BrokenExternalPageTrack::class, + 'Status' => BrokenExternalPageTrackStatus::class + ); - private static $summary_fields = array( - 'Created' => 'Checked', - 'Link' => 'External Link', - 'HTTPCodeDescription' => 'HTTP Error Code', - 'Page.Title' => 'Page link is on' - ); + private static $summary_fields = array( + 'Created' => 'Checked', + 'Link' => 'External Link', + 'HTTPCodeDescription' => 'HTTP Error Code', + 'Page.Title' => 'Page link is on' + ); - private static $searchable_fields = array( - 'HTTPCode' => array('title' => 'HTTP Code') - ); + private static $searchable_fields = array( + 'HTTPCode' => array('title' => 'HTTP Code') + ); - /** - * @return SiteTree - */ - public function Page() { - return $this->Track()->Page(); - } + /** + * @return SiteTree + */ + public function Page() + { + return $this->Track()->Page(); + } - public function canEdit($member = false) { - return false; - } + public function canEdit($member = false) + { + return false; + } - public function canView($member = false) { - $member = $member ? $member : Member::currentUser(); - $codes = array('content-authors', 'administrators'); - return Permission::checkMember($member, $codes); - } + public function canView($member = false) + { + $member = $member ? $member : Member::currentUser(); + $codes = array('content-authors', 'administrators'); + return Permission::checkMember($member, $codes); + } - /** - * Retrieve a human readable description of a response code - * - * @return string - */ - public function getHTTPCodeDescription() { - $code = $this->HTTPCode; - if(empty($code)) { - // Assume that $code = 0 means there was no response - $description = _t('BrokenExternalLink.NOTAVAILABLE', 'Server Not Available'); - } elseif( - ($descriptions = Config::inst()->get('SS_HTTPResponse', 'status_codes')) - && isset($descriptions[$code]) - ) { - $description = $descriptions[$code]; - } else { - $description = _t('BrokenExternalLink.UNKNOWNRESPONSE', 'Unknown Response Code'); - } - return sprintf("%d (%s)", $code, $description); - } + /** + * Retrieve a human readable description of a response code + * + * @return string + */ + public function getHTTPCodeDescription() + { + $code = $this->HTTPCode; + if (empty($code)) { + // Assume that $code = 0 means there was no response + $description = _t('BrokenExternalLink.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'); + } + return sprintf("%d (%s)", $code, $description); + } } - - diff --git a/src/Model/BrokenExternalPageTrack.php b/src/Model/BrokenExternalPageTrack.php index f22c971..7226f17 100644 --- a/src/Model/BrokenExternalPageTrack.php +++ b/src/Model/BrokenExternalPageTrack.php @@ -2,33 +2,37 @@ namespace SilverStripe\ExternalLinks\Model; -use DataObject; -use Versioned; - +use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\ExternalLinks\Model\BrokenExternalLink; +use SilverStripe\Versioned\Versioned; +use SilverStripe\ORM\DataObject; /** * Represents a track for a single page */ -class BrokenExternalPageTrack extends DataObject { +class BrokenExternalPageTrack extends DataObject +{ - private static $db = array( - 'Processed' => 'Boolean' - ); + private static $db = array( + 'Processed' => 'Boolean' + ); - private static $has_one = array( - 'Page' => 'SiteTree', - 'Status' => 'BrokenExternalPageTrackStatus' - ); + private static $has_one = array( + 'Page' => SiteTree::class, + 'Status' => BrokenExternalPageTrackStatus::class + ); - private static $has_many = array( - 'BrokenLinks' => 'BrokenExternalLink' - ); + private static $has_many = array( + 'BrokenLinks' => BrokenExternalLink::class + ); - /** - * @return SiteTree - */ - public function Page() { - return Versioned::get_by_stage('SiteTree', 'Stage') - ->byID($this->PageID); - } + /** + * @return SiteTree + */ + public function Page() + { + return Versioned::get_by_stage(SiteTree::class, 'Stage') + ->byID($this->PageID); + } } diff --git a/src/Model/BrokenExternalPageTrackStatus.php b/src/Model/BrokenExternalPageTrackStatus.php index 23bcb60..38d6aae 100644 --- a/src/Model/BrokenExternalPageTrackStatus.php +++ b/src/Model/BrokenExternalPageTrackStatus.php @@ -2,9 +2,11 @@ namespace SilverStripe\ExternalLinks\Model; -use DataObject; -use Versioned; - +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; +use SilverStripe\ExternalLinks\Model\BrokenExternalLink; +use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Versioned\Versioned; +use SilverStripe\ORM\DataObject; /** * Represents the status of a track run @@ -14,121 +16,133 @@ use Versioned; * @property int $TotalPages Get total pages count * @property int $CompletedPages Get completed pages count */ -class BrokenExternalPageTrackStatus extends DataObject { +class BrokenExternalPageTrackStatus extends DataObject +{ - private static $db = array( - 'Status' => 'Enum("Completed, Running", "Running")', - 'JobInfo' => 'Varchar(255)' - ); + private static $db = array( + 'Status' => 'Enum("Completed, Running", "Running")', + 'JobInfo' => 'Varchar(255)' + ); - private static $has_many = array( - 'TrackedPages' => 'BrokenExternalPageTrack', - 'BrokenLinks' => 'BrokenExternalLink' - ); + private static $has_many = array( + 'TrackedPages' => BrokenExternalPageTrack::class, + 'BrokenLinks' => BrokenExternalLink::class + ); - /** - * Get the latest track status - * - * @return self - */ - public static function get_latest() { - return self::get() - ->sort('ID', 'DESC') - ->first(); - } + /** + * Get the latest track status + * + * @return self + */ + public static function get_latest() + { + return self::get() + ->sort('ID', 'DESC') + ->first(); + } - /** - * Gets the list of Pages yet to be checked - * - * @return DataList - */ - public function getIncompletePageList() { - $pageIDs = $this - ->getIncompleteTracks() - ->column('PageID'); - if($pageIDs) return Versioned::get_by_stage('SiteTree', 'Stage') - ->byIDs($pageIDs); - } + /** + * Gets the list of Pages yet to be checked + * + * @return DataList + */ + public function getIncompletePageList() + { + $pageIDs = $this + ->getIncompleteTracks() + ->column('PageID'); + if ($pageIDs) { + return Versioned::get_by_stage(SiteTree::class, 'Stage') + ->byIDs($pageIDs); + } + } - /** - * Get the list of incomplete BrokenExternalPageTrack - * - * @return DataList - */ - public function getIncompleteTracks() { - return $this - ->TrackedPages() - ->filter('Processed', 0); - } + /** + * Get the list of incomplete BrokenExternalPageTrack + * + * @return DataList + */ + public function getIncompleteTracks() + { + return $this + ->TrackedPages() + ->filter('Processed', 0); + } - /** - * Get total pages count - */ - public function getTotalPages() { - return $this->TrackedPages()->count(); - } + /** + * Get total pages count + */ + public function getTotalPages() + { + return $this->TrackedPages()->count(); + } - /** - * Get completed pages count - */ - public function getCompletedPages() { - return $this - ->TrackedPages() - ->filter('Processed', 1) - ->count(); - } + /** + * Get completed pages count + */ + public function getCompletedPages() + { + return $this + ->TrackedPages() + ->filter('Processed', 1) + ->count(); + } - /** - * Returns the latest run, or otherwise creates a new one - * - * @return self - */ - public static function get_or_create() { - // Check the current status - $status = self::get_latest(); - if ($status && $status->Status == 'Running') { - $status->updateStatus(); - return $status; - } + /** + * Returns the latest run, or otherwise creates a new one + * + * @return self + */ + public static function get_or_create() + { + // Check the current status + $status = self::get_latest(); + if ($status && $status->Status == 'Running') { + $status->updateStatus(); + return $status; + } - return self::create_status(); - } + return self::create_status(); + } - /* + /* * Create and prepare a new status * * @return self */ - public static function create_status() { - // If the script is to be started create a new status - $status = self::create(); - $status->updateJobInfo('Creating new tracking object'); + public static function create_status() + { + // If the script is to be started create a new status + $status = self::create(); + $status->updateJobInfo('Creating new tracking object'); - // Setup all pages to test - $pageIDs = Versioned::get_by_stage('SiteTree', 'Stage') - ->column('ID'); - foreach ($pageIDs as $pageID) { - $trackPage = BrokenExternalPageTrack::create(); - $trackPage->PageID = $pageID; - $trackPage->StatusID = $status->ID; - $trackPage->write(); - } + // Setup all pages to test + $pageIDs = Versioned::get_by_stage(SiteTree::class, 'Stage') + ->column('ID'); + foreach ($pageIDs as $pageID) { + $trackPage = BrokenExternalPageTrack::create(); + $trackPage->PageID = $pageID; + $trackPage->StatusID = $status->ID; + $trackPage->write(); + } - return $status; - } + return $status; + } - public function updateJobInfo($message) { - $this->JobInfo = $message; - $this->write(); - } + public function updateJobInfo($message) + { + $this->JobInfo = $message; + $this->write(); + } - /** - * Self check status - */ - public function updateStatus() { - if ($this->CompletedPages == $this->TotalPages) { - $this->Status = 'Completed'; - $this->updateJobInfo('Setting to completed'); - } - } -} \ No newline at end of file + /** + * Self check status + */ + public function updateStatus() + { + if ($this->CompletedPages == $this->TotalPages) { + $this->Status = 'Completed'; + $this->updateJobInfo('Setting to completed'); + } + } +} diff --git a/src/Reports/BrokenExternalLinksReport.php b/src/Reports/BrokenExternalLinksReport.php index 3f35321..1d2a57e 100644 --- a/src/Reports/BrokenExternalLinksReport.php +++ b/src/Reports/BrokenExternalLinksReport.php @@ -2,13 +2,12 @@ namespace SilverStripe\ExternalLinks\Reports; -use SS_Report; -use Convert; -use BrokenExternalPageTrackStatus; -use ArrayList; -use Requirements; -use LiteralField; - +use SilverStripe\Core\Convert; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\ORM\ArrayList; +use SilverStripe\View\Requirements; +use SilverStripe\Forms\LiteralField; +use SilverStripe\Reports\Report; /** * Content side-report listing pages with external broken links @@ -16,78 +15,86 @@ use LiteralField; * @subpackage content */ -class BrokenExternalLinksReport extends SS_Report { +class BrokenExternalLinksReport extends Report +{ - /** - * Returns the report title - * - * @return string - */ - public function title() { - return _t('ExternalBrokenLinksReport.EXTERNALBROKENLINKS', "External broken links report"); - } + /** + * Returns the report title + * + * @return string + */ + public function title() + { + return _t('ExternalBrokenLinksReport.EXTERNALBROKENLINKS', "External broken links report"); + } - public function columns() { - return array( - "Created" => "Checked", - 'Link' => array( - 'title' => 'External Link', - 'formatting' => function($value, $item) { - return sprintf( - '%s', - Convert::raw2att($item->Link), - Convert::raw2xml($item->Link) - ); - } - ), - 'HTTPCodeDescription' => 'HTTP Error Code', - "Title" => array( - "title" => 'Page link is on', - 'formatting' => function($value, $item) { - $page = $item->Page(); - return sprintf( - '%s', - Convert::raw2att($page->CMSEditLink()), - Convert::raw2xml($page->Title) - ); - } - ) - ); - } + public function columns() + { + return array( + "Created" => "Checked", + 'Link' => array( + 'title' => 'External Link', + 'formatting' => function ($value, $item) { + return sprintf( + '%s', + Convert::raw2att($item->Link), + Convert::raw2xml($item->Link) + ); + } + ), + 'HTTPCodeDescription' => 'HTTP Error Code', + "Title" => array( + "title" => 'Page link is on', + 'formatting' => function ($value, $item) { + $page = $item->Page(); + return sprintf( + '%s', + Convert::raw2att($page->CMSEditLink()), + Convert::raw2xml($page->Title) + ); + } + ) + ); + } - /** - * Alias of columns(), to support the export to csv action - * in {@link GridFieldExportButton} generateExportFileData method. - * @return array - */ - public function getColumns() { - return $this->columns(); - } + /** + * Alias of columns(), to support the export to csv action + * in {@link GridFieldExportButton} generateExportFileData method. + * @return array + */ + public function getColumns() + { + return $this->columns(); + } - public function sourceRecords() { - $track = BrokenExternalPageTrackStatus::get_latest(); - if ($track) return $track->BrokenLinks(); - return new ArrayList(); - } + public function sourceRecords() + { + $track = BrokenExternalPageTrackStatus::get_latest(); + if ($track) { + return $track->BrokenLinks(); + } + return new ArrayList(); + } - public function getCMSFields() { - Requirements::javascript('externallinks/javascript/BrokenExternalLinksReport.js'); - $fields = parent::getCMSFields(); + public function getCMSFields() + { + Requirements::javascript('externallinks/javascript/BrokenExternalLinksReport.js'); + $fields = parent::getCMSFields(); - $reportResultSpan = '

'; - $reportResult = new LiteralField('ResultTitle', $reportResultSpan); - $fields->push($reportResult); + $reportResultSpan = '

'; + $reportResult = new LiteralField('ResultTitle', $reportResultSpan); + $fields->push($reportResult); - $button = ''; - $runReportButton = new LiteralField( - 'runReport', - sprintf( - $button, - _t('ExternalBrokenLinksReport.RUNREPORT', 'Create new report') - ) - ); - $fields->push($runReportButton); + $button = ''; + $runReportButton = new LiteralField( + 'runReport', + sprintf( + $button, + _t('ExternalBrokenLinksReport.RUNREPORT', 'Create new report') + ) + ); + $fields->push($runReportButton); - return $fields; - } + return $fields; + } } diff --git a/src/Tasks/CheckExternalLinksTask.php b/src/Tasks/CheckExternalLinksTask.php index 2285793..617f12f 100644 --- a/src/Tasks/CheckExternalLinksTask.php +++ b/src/Tasks/CheckExternalLinksTask.php @@ -2,200 +2,229 @@ namespace SilverStripe\ExternalLinks\Tasks; -use BuildTask; -use Debug; -use BrokenExternalPageTrack; use DOMNode; -use BrokenExternalLink; -use Config; -use BrokenExternalPageTrackStatus; -use Injector; -use DB; -class CheckExternalLinksTask extends BuildTask { - private static $dependencies = array( - 'LinkChecker' => '%$LinkChecker' - ); - /** - * @var bool - */ - protected $silent = false; - /** - * @var LinkChecker - */ - protected $linkChecker; +use SilverStripe\Dev\Debug; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; +use SilverStripe\ExternalLinks\Model\BrokenExternalLink; +use SilverStripe\Core\Config\Config; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ORM\DB; +use SilverStripe\Dev\BuildTask; - protected $title = 'Checking broken External links in the SiteTree'; +class CheckExternalLinksTask extends BuildTask +{ - protected $description = 'A task that records external broken links in the SiteTree'; + private static $dependencies = array( + 'LinkChecker' => '%$LinkChecker' + ); - protected $enabled = true; + /** + * @var bool + */ + protected $silent = false; - /** - * Log a message - * - * @param string $message - */ - protected function log($message) { - if(!$this->silent) Debug::message($message); - } + /** + * @var LinkChecker + */ + protected $linkChecker; - public function run($request) { - $this->runLinksCheck(); - } - /** - * Turn on or off message output - * - * @param bool $silent - */ - public function setSilent($silent) { - $this->silent = $silent; - } + protected $title = 'Checking broken External links in the SiteTree'; - /** - * @param LinkChecker $linkChecker - */ - public function setLinkChecker(LinkChecker $linkChecker) { - $this->linkChecker = $linkChecker; - } + protected $description = 'A task that records external broken links in the SiteTree'; - /** - * @return LinkChecker - */ - public function getLinkChecker() { - return $this->linkChecker; - } + protected $enabled = true; - /** - * Check the status of a single link on a page - * - * @param BrokenExternalPageTrack $pageTrack - * @param DOMNode $link - */ - protected function checkPageLink(BrokenExternalPageTrack $pageTrack, DOMNode $link) { - $class = $link->getAttribute('class'); - $href = $link->getAttribute('href'); - $markedBroken = preg_match('/\b(ss-broken)\b/', $class); + /** + * Log a message + * + * @param string $message + */ + protected function log($message) + { + if (!$this->silent) { + Debug::message($message); + } + } - // Check link - $httpCode = $this->linkChecker->checkLink($href); - if($httpCode === null) return; // Null link means uncheckable, such as an internal link + public function run($request) + { + $this->runLinksCheck(); + } + /** + * Turn on or off message output + * + * @param bool $silent + */ + public function setSilent($silent) + { + $this->silent = $silent; + } - // If this code is broken then mark as such - if($foundBroken = $this->isCodeBroken($httpCode)) { - // Create broken record - $brokenLink = new BrokenExternalLink(); - $brokenLink->Link = $href; - $brokenLink->HTTPCode = $httpCode; - $brokenLink->TrackID = $pageTrack->ID; - $brokenLink->StatusID = $pageTrack->StatusID; // Slight denormalisation here for performance reasons - $brokenLink->write(); - } + /** + * @param LinkChecker $linkChecker + */ + public function setLinkChecker(LinkChecker $linkChecker) + { + $this->linkChecker = $linkChecker; + } - // Check if we need to update CSS class, otherwise return - if($markedBroken == $foundBroken) return; - if($foundBroken) { - $class .= ' ss-broken'; - } else { - $class = preg_replace('/\s*\b(ss-broken)\b\s*/', ' ', $class); - } - $link->setAttribute('class', trim($class)); - } + /** + * @return LinkChecker + */ + public function getLinkChecker() + { + return $this->linkChecker; + } - /** - * Determine if the given HTTP code is "broken" - * - * @param int $httpCode - * @return bool True if this is a broken code - */ - protected function isCodeBroken($httpCode) { - // Null represents no request attempted - if($httpCode === null) return false; + /** + * Check the status of a single link on a page + * + * @param BrokenExternalPageTrack $pageTrack + * @param DOMNode $link + */ + protected function checkPageLink(BrokenExternalPageTrack $pageTrack, DOMNode $link) + { + $class = $link->getAttribute('class'); + $href = $link->getAttribute('href'); + $markedBroken = preg_match('/\b(ss-broken)\b/', $class); - // do we have any whitelisted codes - $ignoreCodes = Config::inst()->get('CheckExternalLinks', 'IgnoreCodes'); - if(is_array($ignoreCodes) && in_array($httpCode, $ignoreCodes)) return false; + // Check link + $httpCode = $this->linkChecker->checkLink($href); + if ($httpCode === null) { + return; // Null link means uncheckable, such as an internal link + } - // Check if code is outside valid range - return $httpCode < 200 || $httpCode > 302; - } + // If this code is broken then mark as such + if ($foundBroken = $this->isCodeBroken($httpCode)) { + // Create broken record + $brokenLink = new BrokenExternalLink(); + $brokenLink->Link = $href; + $brokenLink->HTTPCode = $httpCode; + $brokenLink->TrackID = $pageTrack->ID; + $brokenLink->StatusID = $pageTrack->StatusID; // Slight denormalisation here for performance reasons + $brokenLink->write(); + } - /** - * Runs the links checker and returns the track used - * - * @param int $limit Limit to number of pages to run, or null to run all - * @return BrokenExternalPageTrackStatus - */ - public function runLinksCheck($limit = null) { - // Check the current status - $status = BrokenExternalPageTrackStatus::get_or_create(); + // Check if we need to update CSS class, otherwise return + if ($markedBroken == $foundBroken) { + return; + } + if ($foundBroken) { + $class .= ' ss-broken'; + } else { + $class = preg_replace('/\s*\b(ss-broken)\b\s*/', ' ', $class); + } + $link->setAttribute('class', trim($class)); + } - // Calculate pages to run - $pageTracks = $status->getIncompleteTracks(); - if($limit) $pageTracks = $pageTracks->limit($limit); + /** + * Determine if the given HTTP code is "broken" + * + * @param int $httpCode + * @return bool True if this is a broken code + */ + protected function isCodeBroken($httpCode) + { + // Null represents no request attempted + if ($httpCode === null) { + return false; + } - // Check each page - foreach ($pageTracks as $pageTrack) { - // Flag as complete - $pageTrack->Processed = 1; - $pageTrack->write(); + // do we have any whitelisted codes + $ignoreCodes = Config::inst()->get('CheckExternalLinks', 'IgnoreCodes'); + if (is_array($ignoreCodes) && in_array($httpCode, $ignoreCodes)) { + return false; + } - // Check value of html area - $page = $pageTrack->Page(); - $this->log("Checking {$page->Title}"); - $htmlValue = Injector::inst()->create('HTMLValue', $page->Content); - if (!$htmlValue->isValid()) continue; + // Check if code is outside valid range + return $httpCode < 200 || $httpCode > 302; + } - // Check each link - $links = $htmlValue->getElementsByTagName('a'); - foreach($links as $link) { - $this->checkPageLink($pageTrack, $link); - } + /** + * Runs the links checker and returns the track used + * + * @param int $limit Limit to number of pages to run, or null to run all + * @return BrokenExternalPageTrackStatus + */ + public function runLinksCheck($limit = null) + { + // Check the current status + $status = BrokenExternalPageTrackStatus::get_or_create(); - // Update content of page based on link fixes / breakages - $htmlValue->saveHTML(); - $page->Content = $htmlValue->getContent(); - $page->write(); + // Calculate pages to run + $pageTracks = $status->getIncompleteTracks(); + if ($limit) { + $pageTracks = $pageTracks->limit($limit); + } - // Once all links have been created for this page update HasBrokenLinks - $count = $pageTrack->BrokenLinks()->count(); - $this->log("Found {$count} broken links"); - if($count) { - // Bypass the ORM as syncLinkTracking does not allow you to update HasBrokenLink to true - DB::query(sprintf( - 'UPDATE "SiteTree" SET "HasBrokenLink" = 1 WHERE "ID" = \'%d\'', - intval($pageTrack->ID) - )); - } - } + // Check each page + foreach ($pageTracks as $pageTrack) { + // Flag as complete + $pageTrack->Processed = 1; + $pageTrack->write(); - $status->updateJobInfo('Updating completed pages'); - $status->updateStatus(); - return $status; - } + // Check value of html area + $page = $pageTrack->Page(); + $this->log("Checking {$page->Title}"); + $htmlValue = Injector::inst()->create('HTMLValue', $page->Content); + if (!$htmlValue->isValid()) { + continue; + } - private function updateCompletedPages($trackID = 0) { - $noPages = BrokenExternalPageTrack::get() - ->filter(array( - 'TrackID' => $trackID, - 'Processed' => 1 - )) - ->count(); - $track = BrokenExternalPageTrackStatus::get_latest(); - $track->CompletedPages = $noPages; - $track->write(); - return $noPages; - } + // Check each link + $links = $htmlValue->getElementsByTagName('a'); + foreach ($links as $link) { + $this->checkPageLink($pageTrack, $link); + } - private function updateJobInfo($message) { - $track = BrokenExternalPageTrackStatus::get_latest(); - if($track) { - $track->JobInfo = $message; - $track->write(); - } - } + // Update content of page based on link fixes / breakages + $htmlValue->saveHTML(); + $page->Content = $htmlValue->getContent(); + $page->write(); + + // Once all links have been created for this page update HasBrokenLinks + $count = $pageTrack->BrokenLinks()->count(); + $this->log("Found {$count} broken links"); + if ($count) { + // Bypass the ORM as syncLinkTracking does not allow you to update HasBrokenLink to true + DB::query(sprintf( + 'UPDATE "SiteTree" SET "HasBrokenLink" = 1 WHERE "ID" = \'%d\'', + intval($pageTrack->ID) + )); + } + } + + $status->updateJobInfo('Updating completed pages'); + $status->updateStatus(); + return $status; + } + + private function updateCompletedPages($trackID = 0) + { + $noPages = BrokenExternalPageTrack::get() + ->filter(array( + 'TrackID' => $trackID, + 'Processed' => 1 + )) + ->count(); + $track = BrokenExternalPageTrackStatus::get_latest(); + $track->CompletedPages = $noPages; + $track->write(); + return $noPages; + } + + private function updateJobInfo($message) + { + $track = BrokenExternalPageTrackStatus::get_latest(); + if ($track) { + $track->JobInfo = $message; + $track->write(); + } + } } diff --git a/src/Tasks/CurlLinkChecker.php b/src/Tasks/CurlLinkChecker.php index 4d95506..d86d26b 100644 --- a/src/Tasks/CurlLinkChecker.php +++ b/src/Tasks/CurlLinkChecker.php @@ -4,51 +4,57 @@ namespace SilverStripe\ExternalLinks\Tasks; use SS_Cache; - /** * Check links using curl */ -class CurlLinkChecker implements LinkChecker { +class CurlLinkChecker implements LinkChecker +{ - /** - * Return cache - * - * @return Zend_Cache_Frontend - */ - protected function getCache() { - return SS_Cache::factory( - __CLASS__, - 'Output', - array('automatic_serialization' => true) - ); - } + /** + * Return cache + * + * @return Zend_Cache_Frontend + */ + protected function getCache() + { + return SS_Cache::factory( + __CLASS__, + 'Output', + array('automatic_serialization' => true) + ); + } - /** - * Determine the http status code for a given link - * - * @param string $href URL to check - * @return int HTTP status code, or null if not checkable (not a link) - */ - public function checkLink($href) { - // Skip non-external links - if(!preg_match('/^https?[^:]*:\/\//', $href)) return null; + /** + * Determine the http status code for a given link + * + * @param string $href URL to check + * @return int HTTP status code, or null if not checkable (not a link) + */ + public function checkLink($href) + { + // Skip non-external links + if (!preg_match('/^https?[^:]*:\/\//', $href)) { + return null; + } - // Check if we have a cached result - $cacheKey = md5($href); - $result = $this->getCache()->load($cacheKey); - if($result !== false) return $result; + // Check if we have a cached result + $cacheKey = md5($href); + $result = $this->getCache()->load($cacheKey); + if ($result !== false) { + return $result; + } - // No cached result so just request - $handle = curl_init($href); - curl_setopt($handle, CURLOPT_RETURNTRANSFER, TRUE); - curl_setopt($handle, CURLOPT_CONNECTTIMEOUT, 5); - curl_setopt($handle, CURLOPT_TIMEOUT, 10); - curl_exec($handle); - $httpCode = curl_getinfo($handle, CURLINFO_HTTP_CODE); - curl_close($handle); + // No cached result so just request + $handle = curl_init($href); + curl_setopt($handle, CURLOPT_RETURNTRANSFER, true); + curl_setopt($handle, CURLOPT_CONNECTTIMEOUT, 5); + curl_setopt($handle, CURLOPT_TIMEOUT, 10); + curl_exec($handle); + $httpCode = curl_getinfo($handle, CURLINFO_HTTP_CODE); + curl_close($handle); - // Cache result - $this->getCache()->save($httpCode, $cacheKey); - return $httpCode; - } + // Cache result + $this->getCache()->save($httpCode, $cacheKey); + return $httpCode; + } } diff --git a/src/Tasks/LinkChecker.php b/src/Tasks/LinkChecker.php index f273448..146458b 100644 --- a/src/Tasks/LinkChecker.php +++ b/src/Tasks/LinkChecker.php @@ -2,19 +2,17 @@ namespace SilverStripe\ExternalLinks\Tasks; - - - /** * Provides an interface for checking that a link is valid */ -interface LinkChecker { - - /** - * Determine the http status code for a given link - * - * @param string $href URL to check - * @return int HTTP status code, or null if not checkable (not a link) - */ - public function checkLink($href); +interface LinkChecker +{ + + /** + * Determine the http status code for a given link + * + * @param string $href URL to check + * @return int HTTP status code, or null if not checkable (not a link) + */ + public function checkLink($href); } diff --git a/tests/ExternalLinksTest.php b/tests/ExternalLinksTest.php index 6186829..8b68df1 100644 --- a/tests/ExternalLinksTest.php +++ b/tests/ExternalLinksTest.php @@ -1,148 +1,165 @@ array('Translatable') ); - public function setUpOnce() { - if (class_exists('Phockito')) { - Phockito::include_hamcrest(false); - } + public function setUpOnce() + { + if (class_exists('Phockito')) { + Phockito::include_hamcrest(false); + } - parent::setUpOnce(); - } + parent::setUpOnce(); + } - public function setUp() { - parent::setUp(); + public function setUp() + { + parent::setUp(); - // Check dependencies - if (!class_exists('Phockito')) { - $this->skipTest = true; - return $this->markTestSkipped("These tests need the Phockito module installed to run"); - } + // Check dependencies + if (!class_exists('Phockito')) { + $this->skipTest = true; + return $this->markTestSkipped("These tests need the Phockito module installed to run"); + } - // Mock link checker - $checker = Phockito::mock('LinkChecker'); - Phockito::when($checker) - ->checkLink('http://www.working.com') - ->return(200); + // Mock link checker + $checker = Phockito::mock(LinkChecker::class); + Phockito::when($checker) + ->checkLink('http://www.working.com') + ->return(200); - Phockito::when($checker) - ->checkLink('http://www.broken.com/url/thing') // 404 on working site - ->return(404); + Phockito::when($checker) + ->checkLink('http://www.broken.com/url/thing') // 404 on working site + ->return(404); - Phockito::when($checker) - ->checkLink('http://www.broken.com') // 403 on working site - ->return(403); + Phockito::when($checker) + ->checkLink('http://www.broken.com') // 403 on working site + ->return(403); - Phockito::when($checker) - ->checkLink('http://www.nodomain.com') // no ping - ->return(0); + Phockito::when($checker) + ->checkLink('http://www.nodomain.com') // no ping + ->return(0); - Phockito::when($checker) - ->checkLink('/internal/link') - ->return(null); + Phockito::when($checker) + ->checkLink('/internal/link') + ->return(null); - Phockito::when($checker) - ->checkLink('[sitetree_link,id=9999]') - ->return(null); + Phockito::when($checker) + ->checkLink('[sitetree_link,id=9999]') + ->return(null); - Phockito::when($checker) - ->checkLink('home') - ->return(null); + Phockito::when($checker) + ->checkLink('home') + ->return(null); - Phockito::when($checker) - ->checkLink('broken-internal') - ->return(null); + Phockito::when($checker) + ->checkLink('broken-internal') + ->return(null); - Phockito::when($checker) - ->checkLink('[sitetree_link,id=1]') - ->return(null); + Phockito::when($checker) + ->checkLink('[sitetree_link,id=1]') + ->return(null); - Phockito::when($checker) - ->checkLink(Hamcrest_Matchers::anything()) // anything else is 404 - ->return(404); + Phockito::when($checker) + ->checkLink(Hamcrest_Matchers::anything()) // anything else is 404 + ->return(404); - Injector::inst()->registerService($checker, 'LinkChecker'); - } + Injector::inst()->registerService($checker, LinkChecker::class); + } - public function testLinks() { - // Run link checker - $task = CheckExternalLinksTask::create(); - $task->setSilent(true); // Be quiet during the test! - $task->runLinksCheck(); + public function testLinks() + { + // Run link checker + $task = CheckExternalLinksTask::create(); + $task->setSilent(true); // Be quiet during the test! + $task->runLinksCheck(); - // Get all links checked - $status = BrokenExternalPageTrackStatus::get_latest(); - $this->assertEquals('Completed', $status->Status); - $this->assertEquals(5, $status->TotalPages); - $this->assertEquals(5, $status->CompletedPages); + // Get all links checked + $status = BrokenExternalPageTrackStatus::get_latest(); + $this->assertEquals('Completed', $status->Status); + $this->assertEquals(5, $status->TotalPages); + $this->assertEquals(5, $status->CompletedPages); - // Check all pages have had the correct HTML adjusted - for($i = 1; $i <= 5; $i++) { - $page = $this->objFromFixture('ExternalLinksTestPage', 'page'.$i); - $this->assertNotEmpty($page->Content); - $this->assertEquals( - $page->ExpectedContent, - $page->Content, - "Assert that the content of page{$i} has been updated" - ); - } + // Check all pages have had the correct HTML adjusted + for ($i = 1; $i <= 5; $i++) { + $page = $this->objFromFixture('ExternalLinksTestPage', 'page'.$i); + $this->assertNotEmpty($page->Content); + $this->assertEquals( + $page->ExpectedContent, + $page->Content, + "Assert that the content of page{$i} has been updated" + ); + } - // Check that the correct report of broken links is generated - $links = $status - ->BrokenLinks() - ->sort('Link'); + // Check that the correct report of broken links is generated + $links = $status + ->BrokenLinks() + ->sort('Link'); - $this->assertEquals(4, $links->count()); - $this->assertEquals( - array( - 'http://www.broken.com', - 'http://www.broken.com/url/thing', - 'http://www.broken.com/url/thing', - 'http://www.nodomain.com' - ), - array_values($links->map('ID', 'Link')->toArray()) - ); + $this->assertEquals(4, $links->count()); + $this->assertEquals( + array( + 'http://www.broken.com', + 'http://www.broken.com/url/thing', + 'http://www.broken.com/url/thing', + 'http://www.nodomain.com' + ), + array_values($links->map('ID', 'Link')->toArray()) + ); - // Check response codes are correct - $expected = array( - 'http://www.broken.com' => 403, - 'http://www.broken.com/url/thing' => 404, - 'http://www.nodomain.com' => 0 - ); - $actual = $links->map('Link', 'HTTPCode')->toArray(); - $this->assertEquals($expected, $actual); + // Check response codes are correct + $expected = array( + 'http://www.broken.com' => 403, + 'http://www.broken.com/url/thing' => 404, + 'http://www.nodomain.com' => 0 + ); + $actual = $links->map('Link', 'HTTPCode')->toArray(); + $this->assertEquals($expected, $actual); - // Check response descriptions are correct - i18n::set_locale('en_NZ'); - $expected = array( - 'http://www.broken.com' => '403 (Forbidden)', - 'http://www.broken.com/url/thing' => '404 (Not Found)', - 'http://www.nodomain.com' => '0 (Server Not Available)' - ); - $actual = $links->map('Link', 'HTTPCodeDescription')->toArray(); - $this->assertEquals($expected, $actual); - } + // Check response descriptions are correct + i18n::set_locale('en_NZ'); + $expected = array( + 'http://www.broken.com' => '403 (Forbidden)', + 'http://www.broken.com/url/thing' => '404 (Not Found)', + 'http://www.nodomain.com' => '0 (Server Not Available)' + ); + $actual = $links->map('Link', 'HTTPCodeDescription')->toArray(); + $this->assertEquals($expected, $actual); + } - /** - * Test that broken links appears in the reports list - */ - public function testReportExists() { - $reports = SS_Report::get_reports(); - $reportNames = array(); - foreach($reports as $report) { - $reportNames[] = $report->class; - } - $this->assertContains('BrokenExternalLinksReport',$reportNames, - 'BrokenExternalLinksReport is in reports list'); - } + /** + * Test that broken links appears in the reports list + */ + public function testReportExists() + { + $reports = Report::get_reports(); + $reportNames = array(); + foreach ($reports as $report) { + $reportNames[] = $report->class; + } + $this->assertContains( + BrokenExternalLinksReport::class, + $reportNames, + 'BrokenExternalLinksReport is in reports list' + ); + } } diff --git a/tests/ExternalLinksTestPage.php b/tests/ExternalLinksTestPage.php index 69fd768..d985be6 100644 --- a/tests/ExternalLinksTestPage.php +++ b/tests/ExternalLinksTestPage.php @@ -1,5 +1,7 @@ Date: Thu, 23 Nov 2017 13:19:00 +1300 Subject: [PATCH 05/15] FIX Linting errors for code cleanliness --- .upgrade.yml | 2 +- _config/routes.yml | 2 +- src/Controllers/CMSExternalLinks.php | 2 +- src/Jobs/CheckExternalLinksJob.php | 72 ++++++++++++++-------------- tests/ExternalLinksTest.php | 2 + tests/ExternalLinksTestPage.php | 2 + 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/.upgrade.yml b/.upgrade.yml index ce4f759..5bad3f1 100644 --- a/.upgrade.yml +++ b/.upgrade.yml @@ -1,5 +1,5 @@ mappings: - CMSExternalLinks_Controller: SilverStripe\ExternalLinks\Controllers\CMSExternalLinks_Controller + CMSExternalLinks_Controller: SilverStripe\ExternalLinks\Controllers\CMSExternalLinksController CheckExternalLinksJob: SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob BrokenExternalLink: SilverStripe\ExternalLinks\Model\BrokenExternalLink BrokenExternalPageTrack: SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack diff --git a/_config/routes.yml b/_config/routes.yml index 2284d3a..7320d40 100644 --- a/_config/routes.yml +++ b/_config/routes.yml @@ -4,4 +4,4 @@ After: framework/routes --- Director: rules: - 'admin/externallinks//$Action': 'CMSExternalLinks_Controller' + 'admin/externallinks//$Action': 'CMSExternalLinksController' diff --git a/src/Controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinks.php index a725ae9..595e4db 100644 --- a/src/Controllers/CMSExternalLinks.php +++ b/src/Controllers/CMSExternalLinks.php @@ -8,7 +8,7 @@ use SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob; use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; use SilverStripe\Control\Controller; -class CMSExternalLinks_Controller extends Controller +class CMSExternalLinksController extends Controller { private static $allowed_actions = array('getJobStatus', 'start'); diff --git a/src/Jobs/CheckExternalLinksJob.php b/src/Jobs/CheckExternalLinksJob.php index 662d81d..aac7589 100644 --- a/src/Jobs/CheckExternalLinksJob.php +++ b/src/Jobs/CheckExternalLinksJob.php @@ -2,46 +2,44 @@ namespace SilverStripe\ExternalLinks\Jobs; -use AbstractQueuedJob; -use QueuedJob; - -use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; - -if (!class_exists('AbstractQueuedJob')) { - return; -} - -/** - * A Job for running a external link check for published pages - * - */ -class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob -{ - - public function getTitle() - { - return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); - } - - public function getJobType() - { - return QueuedJob::QUEUED; - } - - public function getSignature() - { - return md5(get_class($this)); - } +if (class_exists('Symbiote\QueuedJobs\Services\AbstractQueuedJob')) { + use Symbiote\QueuedJobs\Services\AbstractQueuedJob; + use Symbiote\QueuedJobs\Services\QueuedJob; + use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; /** - * Check an individual page + * A Job for running a external link check for published pages + * */ - public function process() + class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob { - $task = CheckExternalLinksTask::create(); - $track = $task->runLinksCheck(1); - $this->currentStep = $track->CompletedPages; - $this->totalSteps = $track->TotalPages; - $this->isComplete = $track->Status === 'Completed'; + + public function getTitle() + { + return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); + } + + public function getJobType() + { + return QueuedJob::QUEUED; + } + + public function getSignature() + { + return md5(get_class($this)); + } + + /** + * Check an individual page + */ + public function process() + { + $task = CheckExternalLinksTask::create(); + $track = $task->runLinksCheck(1); + $this->currentStep = $track->CompletedPages; + $this->totalSteps = $track->TotalPages; + $this->isComplete = $track->Status === 'Completed'; + } } + } diff --git a/tests/ExternalLinksTest.php b/tests/ExternalLinksTest.php index 8b68df1..3394aed 100644 --- a/tests/ExternalLinksTest.php +++ b/tests/ExternalLinksTest.php @@ -1,5 +1,7 @@ Date: Thu, 23 Nov 2017 14:54:27 +1300 Subject: [PATCH 06/15] Update tests & left over fixes from upgrader tool --- phpcs.xml.dist | 1 + src/Jobs/CheckExternalLinksJob.php | 73 +++++++++++---------- src/Model/BrokenExternalLink.php | 1 + src/Model/BrokenExternalPageTrack.php | 1 + src/Model/BrokenExternalPageTrackStatus.php | 1 + tests/ExternalLinksTest.php | 14 ++-- tests/ExternalLinksTest.yml | 2 +- tests/ExternalLinksTestPage.php | 3 + 8 files changed, 51 insertions(+), 45 deletions(-) diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 1b984f8..f6e295b 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -5,5 +5,6 @@ + diff --git a/src/Jobs/CheckExternalLinksJob.php b/src/Jobs/CheckExternalLinksJob.php index aac7589..fb2bfc2 100644 --- a/src/Jobs/CheckExternalLinksJob.php +++ b/src/Jobs/CheckExternalLinksJob.php @@ -2,44 +2,45 @@ namespace SilverStripe\ExternalLinks\Jobs; -if (class_exists('Symbiote\QueuedJobs\Services\AbstractQueuedJob')) { - use Symbiote\QueuedJobs\Services\AbstractQueuedJob; - use Symbiote\QueuedJobs\Services\QueuedJob; - use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; +use Symbiote\QueuedJobs\Services\AbstractQueuedJob; +use Symbiote\QueuedJobs\Services\QueuedJob; +use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; - /** - * A Job for running a external link check for published pages - * - */ - class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob +if (!class_exists(AbstractQueuedJob::class)) { + return; +} + +/** + * A Job for running a external link check for published pages + * + */ +class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob +{ + + public function getTitle() { - - public function getTitle() - { - return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); - } - - public function getJobType() - { - return QueuedJob::QUEUED; - } - - public function getSignature() - { - return md5(get_class($this)); - } - - /** - * Check an individual page - */ - public function process() - { - $task = CheckExternalLinksTask::create(); - $track = $task->runLinksCheck(1); - $this->currentStep = $track->CompletedPages; - $this->totalSteps = $track->TotalPages; - $this->isComplete = $track->Status === 'Completed'; - } + return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links'); } + public function getJobType() + { + return QueuedJob::QUEUED; + } + + public function getSignature() + { + return md5(get_class($this)); + } + + /** + * Check an individual page + */ + public function process() + { + $task = CheckExternalLinksTask::create(); + $track = $task->runLinksCheck(1); + $this->currentStep = $track->CompletedPages; + $this->totalSteps = $track->TotalPages; + $this->isComplete = $track->Status === 'Completed'; + } } diff --git a/src/Model/BrokenExternalLink.php b/src/Model/BrokenExternalLink.php index cb62dad..50a3c39 100644 --- a/src/Model/BrokenExternalLink.php +++ b/src/Model/BrokenExternalLink.php @@ -18,6 +18,7 @@ use SilverStripe\ORM\DataObject; */ class BrokenExternalLink extends DataObject { + private static $table_name = 'BrokenExternalLink'; private static $db = array( 'Link' => 'Varchar(2083)', // 2083 is the maximum length of a URL in Internet Explorer. diff --git a/src/Model/BrokenExternalPageTrack.php b/src/Model/BrokenExternalPageTrack.php index 7226f17..d1b257f 100644 --- a/src/Model/BrokenExternalPageTrack.php +++ b/src/Model/BrokenExternalPageTrack.php @@ -13,6 +13,7 @@ use SilverStripe\ORM\DataObject; */ class BrokenExternalPageTrack extends DataObject { + private static $table_name = 'BrokenExternalPageTrack'; private static $db = array( 'Processed' => 'Boolean' diff --git a/src/Model/BrokenExternalPageTrackStatus.php b/src/Model/BrokenExternalPageTrackStatus.php index 38d6aae..b1ef680 100644 --- a/src/Model/BrokenExternalPageTrackStatus.php +++ b/src/Model/BrokenExternalPageTrackStatus.php @@ -18,6 +18,7 @@ use SilverStripe\ORM\DataObject; */ class BrokenExternalPageTrackStatus extends DataObject { + private static $table_name = 'BrokenExternalPageTrackStatus'; private static $db = array( 'Status' => 'Enum("Completed, Running", "Running")', diff --git a/tests/ExternalLinksTest.php b/tests/ExternalLinksTest.php index 3394aed..ad26e1a 100644 --- a/tests/ExternalLinksTest.php +++ b/tests/ExternalLinksTest.php @@ -10,23 +10,21 @@ use SilverStripe\i18n\i18n; use SilverStripe\Reports\Report; use SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ExternalLinks\Tests\ExternalLinksTestPage; +use Phockito; class ExternalLinksTest extends SapphireTest { protected static $fixture_file = 'ExternalLinksTest.yml'; - protected $extraDataObjects = array( - 'ExternalLinksTestPage' - ); - - protected $illegalExtensions = array( - 'SiteTree' => array('Translatable') + protected static $extra_dataobjects = array( + ExternalLinksTestPage::class ); public function setUpOnce() { - if (class_exists('Phockito')) { + if (class_exists(Phockito::class)) { Phockito::include_hamcrest(false); } @@ -38,7 +36,7 @@ class ExternalLinksTest extends SapphireTest parent::setUp(); // Check dependencies - if (!class_exists('Phockito')) { + if (!class_exists(Phockito::class)) { $this->skipTest = true; return $this->markTestSkipped("These tests need the Phockito module installed to run"); } diff --git a/tests/ExternalLinksTest.yml b/tests/ExternalLinksTest.yml index 7b026b2..59992d0 100644 --- a/tests/ExternalLinksTest.yml +++ b/tests/ExternalLinksTest.yml @@ -1,4 +1,4 @@ -ExternalLinksTestPage: +SilverStripe\ExternalLinks\Tests\ExternalLinksTestPage: # Tests mix of broken and working external links page1: Title: 'Page 1' diff --git a/tests/ExternalLinksTestPage.php b/tests/ExternalLinksTestPage.php index 8a0d460..a4ce0e5 100644 --- a/tests/ExternalLinksTestPage.php +++ b/tests/ExternalLinksTestPage.php @@ -3,9 +3,12 @@ namespace SilverStripe\ExternalLinks\Tests; use SilverStripe\Dev\TestOnly; +use Page; class ExternalLinksTestPage extends Page implements TestOnly { + private static $table_name = 'ExternalLinksTestPage'; + private static $db = array( 'ExpectedContent' => 'HTMLText' ); From 77eaa62efc96eed812e8ceeabdf99d8fcdc705de Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Thu, 23 Nov 2017 15:36:00 +1300 Subject: [PATCH 07/15] update readme to reflect v4 upgrade work --- README.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index f657928..0fa01dd 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # External links -[![Build Status](https://travis-ci.org/silverstripe/silverstripe-externallinks.svg?branch=master)](https://travis-ci.org/silverstripe/silverstripe-externallinks) +[![Build Status](http://img.shields.io/travis/silverstripe/silverstripe-externallinks.svg?style=flat)](https://travis-ci.org/silverstripe/silverstripe-externallinks) +[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/silverstripe/silverstripe-externallinks/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/silverstripe/silverstripe-externallinks/?branch=master) +[![codecov](https://codecov.io/gh/silverstripe/silverstripe-externallinks/branch/master/graph/badge.svg)](https://codecov.io/gh/silverstripe/silverstripe-externallinks) ## Introduction @@ -12,15 +14,15 @@ The external links module is a task and ModelAdmin to track and to report on bro ## Requirements - * SilverStripe 3.1 + +* SilverStripe ^4.0 + +**Note:** For a SilverStripe 3.x compatible version, please use [the 1.x release line](https://github.com/silverstripe/silverstripe-externallinks/tree/1.0). ## Features * Add external links to broken links reports * Add a task to track external broken links -See the [changelog](CHANGELOG.md) for version history. - ## Installation 1. If you have composer you can use `composer require silverstripe/externallinks:*`. Otherwise, @@ -63,20 +65,17 @@ broken links. ## Queued job ## -If you have the queuedjobs module installed you can set the task to be run every so ofter -Add the following yml config to config.yml in mysite/_config have the the task run once every day (86400 seconds) - - CheckExternalLinks: - Delay: 86400 +If you have the queuedjobs module installed you can set the task to be run every so often. ## Whitelisting codes ## If you want to ignore or whitelist certain http codes this can be setup via IgnoreCodes in the config.yml -file in mysite/_config +file in `mysite/_config` +```yml CheckExternalLinks: - Delay: 60 IgnoreCodes: - 401 - 403 - 501 +``` From 0374d66b3207bd412c3e1ac69558ec657db1c1f4 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 11:19:58 +1300 Subject: [PATCH 08/15] 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, From ed7d91becbfee181abbbe824a649de9e25e86f54 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 11:23:36 +1300 Subject: [PATCH 09/15] re-add transifex config as it was removed from the wrong branch. --- .tx/config | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .tx/config diff --git a/.tx/config b/.tx/config new file mode 100644 index 0000000..360a807 --- /dev/null +++ b/.tx/config @@ -0,0 +1,8 @@ +[main] +host = https://www.transifex.com + +[silverstripe-externallinks.master] +file_filter = lang/.yml +source_file = lang/en.yml +source_lang = en +type = YML From 98dac9b3148b8eeea243ccb2d47668b1d77d1167 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 15:14:16 +1300 Subject: [PATCH 10/15] Update unit tests to be rid of Phockito This odd dependency is causing inclusion issues to do with namespacing and the ability to work well with the framework (according to the dependency's readme). The usage of the tool in this case adds no value, as it's performing purely stub type work, rather than mock work. So instead we can use an actual stub (simply return test values depending on input) and have everything work well. After removing the Phockito dependency it was found that the tests that would skip when it isn't present were actually in a failing state. So they had to be repaired too, mostly through the use of fixing the poor code that was causing them to fail (as opposed to being bad tests). This is both configuration and proceedural work. --- _config/injector.yml | 2 +- _config/routes.yml | 8 +- ...nks.php => CMSExternalLinksController.php} | 29 +++---- src/Model/BrokenExternalLink.php | 15 ++-- src/Model/BrokenExternalPageTrackStatus.php | 8 +- src/Reports/BrokenExternalLinksReport.php | 27 ++++--- src/Tasks/CheckExternalLinksTask.php | 24 +++--- src/Tasks/CurlLinkChecker.php | 1 + tests/ExternalLinksTest.php | 75 +++---------------- tests/Stubs/PretendLinkChecker.php | 28 +++++++ 10 files changed, 97 insertions(+), 120 deletions(-) rename src/Controllers/{CMSExternalLinks.php => CMSExternalLinksController.php} (77%) create mode 100644 tests/Stubs/PretendLinkChecker.php diff --git a/_config/injector.yml b/_config/injector.yml index 45b3195..4d890b6 100644 --- a/_config/injector.yml +++ b/_config/injector.yml @@ -2,7 +2,7 @@ Name: externallinksdependencies --- SilverStripe\Core\Injector\Injector: - LinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker + SilverStripe\ExternalLinks\Tasks\LinkChecker: SilverStripe\ExternalLinks\Tasks\CurlLinkChecker Psr\SimpleCache\CacheInterface.CurlLinkChecker: factory: SilverStripe\Core\Cache\CacheFactory constructor: diff --git a/_config/routes.yml b/_config/routes.yml index 7320d40..2ac22a9 100644 --- a/_config/routes.yml +++ b/_config/routes.yml @@ -1,7 +1,7 @@ --- -Name: externallink -After: framework/routes +Name: externallinkroutes +Before: '#adminroutes' --- -Director: +SilverStripe\Control\Director: rules: - 'admin/externallinks//$Action': 'CMSExternalLinksController' + 'admin/externallinks//$Action': SilverStripe\ExternalLinks\Controllers\CMSExternalLinksController diff --git a/src/Controllers/CMSExternalLinks.php b/src/Controllers/CMSExternalLinksController.php similarity index 77% rename from src/Controllers/CMSExternalLinks.php rename to src/Controllers/CMSExternalLinksController.php index 2bafb3f..7f35df1 100644 --- a/src/Controllers/CMSExternalLinks.php +++ b/src/Controllers/CMSExternalLinksController.php @@ -12,13 +12,16 @@ use Symbiote\QueuedJobs\Services\QueuedJobService; class CMSExternalLinksController extends Controller { - private static $allowed_actions = array('getJobStatus', 'start'); + private static $allowed_actions = [ + 'getJobStatus', + 'start' + ]; /** - * Respond to Ajax requests for info on a running job - * - * @return string JSON string detailing status of the job - */ + * Respond to Ajax requests for info on a running job + * + * @return string JSON string detailing status of the job + */ public function getJobStatus() { // Set headers @@ -32,19 +35,19 @@ class CMSExternalLinksController extends Controller // Format status $track = BrokenExternalPageTrackStatus::get_latest(); if ($track) { - return json_encode(array( - 'TrackID' => $track->ID, - 'Status' => $track->Status, - 'Completed' => $track->getCompletedPages(), - 'Total' => $track->getTotalPages() - )); + return json_encode([ + 'TrackID' => $track->ID, + 'Status' => $track->Status, + 'Completed' => $track->getCompletedPages(), + 'Total' => $track->getTotalPages() + ]); } } /** - * Starts a broken external link check - */ + * Starts a broken external link check + */ public function start() { // return if the a job is already running diff --git a/src/Model/BrokenExternalLink.php b/src/Model/BrokenExternalLink.php index 5d39f09..0aa2037 100644 --- a/src/Model/BrokenExternalLink.php +++ b/src/Model/BrokenExternalLink.php @@ -69,16 +69,17 @@ class BrokenExternalLink extends DataObject public function getHTTPCodeDescription() { $code = $this->HTTPCode; - if (empty($code)) { + + try { + $response = HTTPResponse::create('', $code); // Assume that $code = 0 means there was no response - $description = _t(__CLASS__ . '.NOTAVAILABLE', 'Server Not Available'); - } elseif (($descriptions = Config::inst()->get(HTTPResponse::class, 'status_codes')) - && isset($descriptions[$code]) - ) { - $description = $descriptions[$code]; - } else { + $description = $code ? + $response->getStatusDescription() : + _t(__CLASS__ . '.NOTAVAILABLE', 'Server Not Available'); + } catch (InvalidArgumentException $e) { $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 0a159be..c34596e 100644 --- a/src/Model/BrokenExternalPageTrackStatus.php +++ b/src/Model/BrokenExternalPageTrackStatus.php @@ -111,10 +111,10 @@ class BrokenExternalPageTrackStatus extends DataObject } /** - * Create and prepare a new status - * - * @return BrokenExternalPageTrackStatus - */ + * Create and prepare a new status + * + * @return BrokenExternalPageTrackStatus + */ public static function create_status() { // If the script is to be started create a new status diff --git a/src/Reports/BrokenExternalLinksReport.php b/src/Reports/BrokenExternalLinksReport.php index 8b9ab90..fec065a 100644 --- a/src/Reports/BrokenExternalLinksReport.php +++ b/src/Reports/BrokenExternalLinksReport.php @@ -2,12 +2,13 @@ namespace SilverStripe\ExternalLinks\Reports; -use SilverStripe\Core\Convert; -use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; use SilverStripe\ORM\ArrayList; -use SilverStripe\View\Requirements; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\Core\Convert; +use SilverStripe\View\HTML; use SilverStripe\Forms\LiteralField; use SilverStripe\Reports\Report; +use SilverStripe\View\Requirements; /** * Content side-report listing pages with external broken links @@ -72,7 +73,7 @@ class BrokenExternalLinksReport extends Report if ($track) { return $track->BrokenLinks(); } - return new ArrayList(); + return ArrayList::create(); } public function getCMSFields() @@ -81,17 +82,19 @@ class BrokenExternalLinksReport extends Report $fields = parent::getCMSFields(); $reportResultSpan = '

'; - $reportResult = new LiteralField('ResultTitle', $reportResultSpan); + $reportResult = LiteralField::create('ResultTitle', $reportResultSpan); $fields->push($reportResult); - $button = ''; - $runReportButton = new LiteralField( - 'runReport', - sprintf( - $button, - _t(__CLASS__ . '.RUNREPORT', 'Create new report') - ) + $button = HTML::createTag( + 'button', + [ + 'id' => 'externalLinksReport', + 'type' => 'button', + 'class' => 'btn btn-primary' + ], + _t(__CLASS__ . '.RUNREPORT', 'Create new report') ); + $runReportButton = LiteralField::create('runReport', $button); $fields->push($runReportButton); return $fields; diff --git a/src/Tasks/CheckExternalLinksTask.php b/src/Tasks/CheckExternalLinksTask.php index a588e86..41078a8 100644 --- a/src/Tasks/CheckExternalLinksTask.php +++ b/src/Tasks/CheckExternalLinksTask.php @@ -2,27 +2,23 @@ namespace SilverStripe\ExternalLinks\Tasks; -use DOMNode; - - - - - -use SilverStripe\Dev\Debug; -use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; use SilverStripe\ExternalLinks\Model\BrokenExternalLink; -use SilverStripe\Core\Config\Config; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\ORM\DB; use SilverStripe\Dev\BuildTask; +use SilverStripe\Core\Config\Config; +use SilverStripe\ORM\DB; +use SilverStripe\Dev\Debug; +use DOMNode; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ExternalLinks\Tasks\LinkChecker; class CheckExternalLinksTask extends BuildTask { - private static $dependencies = array( - 'LinkChecker' => '%$LinkChecker' - ); + private static $dependencies = [ + 'LinkChecker' => '%$' . LinkChecker::class + ]; /** * @var bool diff --git a/src/Tasks/CurlLinkChecker.php b/src/Tasks/CurlLinkChecker.php index 2e396a8..0a11f3d 100644 --- a/src/Tasks/CurlLinkChecker.php +++ b/src/Tasks/CurlLinkChecker.php @@ -3,6 +3,7 @@ namespace SilverStripe\ExternalLinks\Tasks; use Psr\SimpleCache\CacheInterface; +use SilverStripe\Core\Injector\Injector; /** * Check links using curl diff --git a/tests/ExternalLinksTest.php b/tests/ExternalLinksTest.php index d4de5e5..e1c4bbd 100644 --- a/tests/ExternalLinksTest.php +++ b/tests/ExternalLinksTest.php @@ -2,16 +2,16 @@ namespace SilverStripe\ExternalLinks\Tests; -use SilverStripe\ExternalLinks\Tasks\LinkChecker; -use SilverStripe\Core\Injector\Injector; -use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; -use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; -use SilverStripe\i18n\i18n; -use SilverStripe\Reports\Report; use SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport; -use SilverStripe\Dev\SapphireTest; +use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; +use SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask; use SilverStripe\ExternalLinks\Tests\ExternalLinksTestPage; -use Phockito; +use SilverStripe\i18n\i18n; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ExternalLinks\Tasks\LinkChecker; +use SilverStripe\ExternalLinks\Tests\Stubs\PretendLinkChecker; +use SilverStripe\Reports\Report; +use SilverStripe\Dev\SapphireTest; class ExternalLinksTest extends SapphireTest { @@ -22,67 +22,12 @@ class ExternalLinksTest extends SapphireTest ExternalLinksTestPage::class ); - public function setUpOnce() - { - if (class_exists(Phockito::class)) { - Phockito::include_hamcrest(false); - } - - parent::setUpOnce(); - } - protected function setUp() { parent::setUp(); - // Check dependencies - if (!class_exists(Phockito::class)) { - $this->skipTest = true; - return $this->markTestSkipped("These tests need the Phockito module installed to run"); - } - - // Mock link checker - $checker = Phockito::mock(LinkChecker::class); - Phockito::when($checker) - ->checkLink('http://www.working.com') - ->return(200); - - Phockito::when($checker) - ->checkLink('http://www.broken.com/url/thing') // 404 on working site - ->return(404); - - Phockito::when($checker) - ->checkLink('http://www.broken.com') // 403 on working site - ->return(403); - - Phockito::when($checker) - ->checkLink('http://www.nodomain.com') // no ping - ->return(0); - - Phockito::when($checker) - ->checkLink('/internal/link') - ->return(null); - - Phockito::when($checker) - ->checkLink('[sitetree_link,id=9999]') - ->return(null); - - Phockito::when($checker) - ->checkLink('home') - ->return(null); - - Phockito::when($checker) - ->checkLink('broken-internal') - ->return(null); - - Phockito::when($checker) - ->checkLink('[sitetree_link,id=1]') - ->return(null); - - Phockito::when($checker) - ->checkLink(Hamcrest_Matchers::anything()) // anything else is 404 - ->return(404); - + // Stub link checker + $checker = new PretendLinkChecker; Injector::inst()->registerService($checker, LinkChecker::class); } diff --git a/tests/Stubs/PretendLinkChecker.php b/tests/Stubs/PretendLinkChecker.php new file mode 100644 index 0000000..8701422 --- /dev/null +++ b/tests/Stubs/PretendLinkChecker.php @@ -0,0 +1,28 @@ + Date: Mon, 27 Nov 2017 15:30:34 +1300 Subject: [PATCH 11/15] Update UPDATE query to use true table name, not assumed one --- src/Tasks/CheckExternalLinksTask.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Tasks/CheckExternalLinksTask.php b/src/Tasks/CheckExternalLinksTask.php index 41078a8..84a8551 100644 --- a/src/Tasks/CheckExternalLinksTask.php +++ b/src/Tasks/CheckExternalLinksTask.php @@ -7,11 +7,13 @@ use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack; use SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus; use SilverStripe\Dev\BuildTask; use SilverStripe\Core\Config\Config; +use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; use SilverStripe\Dev\Debug; use DOMNode; use SilverStripe\Core\Injector\Injector; use SilverStripe\ExternalLinks\Tasks\LinkChecker; +use SilverStripe\CMS\Model\SiteTree; class CheckExternalLinksTask extends BuildTask { @@ -188,9 +190,11 @@ class CheckExternalLinksTask extends BuildTask $count = $pageTrack->BrokenLinks()->count(); $this->log("Found {$count} broken links"); if ($count) { + $siteTreeTable = DataObject::getSchema()->tableName(SiteTree::class); // Bypass the ORM as syncLinkTracking does not allow you to update HasBrokenLink to true DB::query(sprintf( - 'UPDATE "SiteTree" SET "HasBrokenLink" = 1 WHERE "ID" = \'%d\'', + 'UPDATE "%s" SET "HasBrokenLink" = 1 WHERE "ID" = \'%d\'', + $siteTreeTable, intval($pageTrack->ID) )); } From bb7a1c3eda2d4892f92c1dd0d84e7d9ef11643ed Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 16:42:36 +1300 Subject: [PATCH 12/15] Update language files to contain new namespaced classnames --- lang/de.yml | 4 ++-- lang/en.yml | 10 +++++----- lang/eo.yml | 10 +++++----- lang/pl.yml | 10 +++++----- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lang/de.yml b/lang/de.yml index 51b16f6..d5e1236 100644 --- a/lang/de.yml +++ b/lang/de.yml @@ -1,8 +1,8 @@ de: - BrokenExternalLink: + SilverStripe\ExternalLinks\Model\BrokenExternalLink: NOTAVAILABLE: 'Server nicht verfügbar' PLURALNAME: 'Defekte externe Links' SINGULARNAME: 'Defekter externer Link' UNKNOWNRESPONSE: 'Unbekannter Antwortcode' - ExternalBrokenLinksReport: + SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport: RUNREPORT: 'Generiere neuen Bericht' diff --git a/lang/en.yml b/lang/en.yml index 27c4b0c..a4e26b9 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -1,17 +1,17 @@ en: - BrokenExternalLink: + SilverStripe\ExternalLinks\Model\BrokenExternalLink: NOTAVAILABLE: 'Server Not Available' PLURALNAME: 'Broken External Links' SINGULARNAME: 'Broken External Link' UNKNOWNRESPONSE: 'Unknown Response Code' - BrokenExternalPageTrack: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack: PLURALNAME: 'Broken External Page Tracks' SINGULARNAME: 'Broken External Page Track' - BrokenExternalPageTrackStatus: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus: PLURALNAME: 'Broken External Page Track Statuss' SINGULARNAME: 'Broken External Page Track Status' - CheckExternalLiksJob: + SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob: TITLE: 'Checking for external broken links' - ExternalBrokenLinksReport: + SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport: EXTERNALBROKENLINKS: 'External broken links report' RUNREPORT: 'Create new report' diff --git a/lang/eo.yml b/lang/eo.yml index 2b45343..944f007 100644 --- a/lang/eo.yml +++ b/lang/eo.yml @@ -1,17 +1,17 @@ eo: - BrokenExternalLink: + SilverStripe\ExternalLinks\Model\BrokenExternalLink: NOTAVAILABLE: 'Servilo estas neatingenbla' PLURALNAME: 'Rompitaj eksteraj ligiloj' SINGULARNAME: 'Rompita ekstera ligilo' UNKNOWNRESPONSE: 'Nekonata respondokodo' - BrokenExternalPageTrack: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack: PLURALNAME: 'Rompitaj eksteraj paĝaj trakoj' SINGULARNAME: 'Rompita ekstera paĝa trako' - BrokenExternalPageTrackStatus: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus: PLURALNAME: 'Stato de rompitaj eksteraj paĝaj trakoj' SINGULARNAME: 'Stato de rompita ekstera paĝa trako' - CheckExternalLiksJob: + SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob: TITLE: 'Kontrolas por eksteraj rompitaj ligiloj' - ExternalBrokenLinksReport: + SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport: EXTERNALBROKENLINKS: 'Raporto pri eksteraj rompitaj ligiloj' RUNREPORT: 'Krei novan raporton' diff --git a/lang/pl.yml b/lang/pl.yml index 795f089..f264650 100644 --- a/lang/pl.yml +++ b/lang/pl.yml @@ -1,17 +1,17 @@ pl: - BrokenExternalLink: + SilverStripe\ExternalLinks\Model\BrokenExternalLink: NOTAVAILABLE: 'Serwer niedostępny' PLURALNAME: 'Uszkodzone linki zewnętrzne' SINGULARNAME: 'Uszkodzony link zewnętrzny' UNKNOWNRESPONSE: 'Nieznany kod odpowiedzi' - BrokenExternalPageTrack: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrack: PLURALNAME: 'Wykrywania wadliwych stron zewnętrznych' SINGULARNAME: 'Wykrywanie wadliwych stron zewnętrznych' - BrokenExternalPageTrackStatus: + SilverStripe\ExternalLinks\Model\BrokenExternalPageTrackStatus: PLURALNAME: 'Statusy wykrywania wadliwych stron zewnętrznych' SINGULARNAME: 'Status wykrywania wadliwych stron zewnętrznych' - CheckExternalLiksJob: + SilverStripe\ExternalLinks\Jobs\CheckExternalLinksJob: TITLE: 'Wyszukiwanie uszkodzonych linków zewnętrznych' - ExternalBrokenLinksReport: + SilverStripe\ExternalLinks\Reports\BrokenExternalLinksReport: EXTERNALBROKENLINKS: 'Raport uszkodzonych linków zewnętrznych' RUNREPORT: 'Stwórz nowy raport' From 12df3947c2bc53177d98ba404af17a1521c6e63b Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 17:41:17 +1300 Subject: [PATCH 13/15] Update readme in light of ss4 compatibility enhancements --- README.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0fa01dd..5e2f450 100644 --- a/README.md +++ b/README.md @@ -25,10 +25,8 @@ The external links module is a task and ModelAdmin to track and to report on bro ## Installation - 1. If you have composer you can use `composer require silverstripe/externallinks:*`. Otherwise, - download the module from GitHub and extract to the 'externallinks' folder. Place this directory - in your sites root directory. This is the one with framework and cms in it. - 2. Run in your browser - `/dev/build` to rebuild the database. + 1. Require the module via composer: `composer require silverstripe/externallinks` + 2. Run `/dev/build` in your browser to rebuild the database. 3. Run the following task *http://path.to.silverstripe/dev/tasks/CheckExternalLinks* to check for broken external links @@ -73,7 +71,7 @@ If you want to ignore or whitelist certain http codes this can be setup via Igno file in `mysite/_config` ```yml - CheckExternalLinks: + SilverStripe\ExternalLinks\Tasks\CheckExternalLinksTask: IgnoreCodes: - 401 - 403 From 4675a539d5af0d72b953524b946b906a1554fbe8 Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 27 Nov 2017 18:02:26 +1300 Subject: [PATCH 14/15] remove phockito from composer dev reqs --- composer.json | 1 - 1 file changed, 1 deletion(-) diff --git a/composer.json b/composer.json index 4ccbd0d..09b46e4 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,6 @@ "silverstripe/recipe-cms": "^1.0" }, "require-dev": { - "hafriedlander/silverstripe-phockito": "*", "phpunit/PHPUnit": "^5.7", "squizlabs/php_codesniffer": "^3.0" }, From 4a8fb144653a7cebbf8900dd37d14ee619944170 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 27 Nov 2017 19:30:16 +1300 Subject: [PATCH 15/15] Update branch alias for 2.x-dev --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 09b46e4..4cee129 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "extra": { "expose": ["javascript"], "branch-alias": { - "dev-master": "2.0.x-dev" + "dev-master": "2.x-dev" } }, "minimum-stability": "dev",