NEW: Use DB row for job status and refactor the sql statements

kmayo-ss · Aug 1, 2014 · e9fe1a4 · e9fe1a4
1 parent 564514b
commit e9fe1a4
Show file tree

Hide file tree

Showing 9 changed files with 180 additions and 132 deletions.
diff --git a/code/controllers/CMSExternalLinks.php b/code/controllers/CMSExternalLinks.php
@@ -2,69 +2,44 @@
 
 class CMSExternalLinks_Controller extends Controller {
 
-	private static $allowed_actions = array('getJobStatus', 'clear', 'start');
+	private static $allowed_actions = array('getJobStatus', 'start');
 
 	/*
 	 * Respond to Ajax requests for info on a running job
-	 * also calls continueJob and clear depending on the status of the job
 	 *
 	 * @return string JSON string detailing status of the job
 	 */
 	public function getJobStatus() {
-		$trackID = Session::get('ExternalLinksTrackID');
-		if (!$trackID) return;
-		$noPages = Versioned::get_by_stage('SiteTree', 'Live')->count();
-		$result = BrokenExternalPageTrack::get()
-			->filter('TrackID', $trackID)
-			->exclude('PageID', 0);
-		$completedPages = count($result);
-
+		$track = CheckExternalLinks::getLatestTrack();
+		if (!$track || !$track->exists()) return null;
 		echo json_encode(array(
-			'TrackID' => $trackID,
-			'Completed' => $completedPages,
-			'Total' => $noPages
+			'TrackID' => $track->ID,
+			'Status' => $track->Status,
+			'Completed' => $track->CompletedPages,
+			'Total' => $track->TotalPages
 		));
-
-		if ($completedPages >= $noPages) {
-			$this->clear();
-		} else {
-			$this->continueJob();
-		}
 	}
 
-	/*
-	 * Clears the tracking id and any surplus entries for the BrokenExternalPageTrack model
-	 */
-	public function clear() {
-		// clear any old entries
-		$trackID = Session::get('ExternalLinksTrackID');
-		$oldEntries = BrokenExternalPageTrack::get()
-			->exclude('TrackID', $trackID);
-		foreach ($oldEntries as $entry) {
-			$entry->delete();
-		}
-		Session::clear('ExternalLinksTrackID');
-	}
 
 	/*
 	 * Starts a broken external link check
 	 */
 	public function start() {
-		$track = BrokenExternalPageTrack::create();
-		$track->write();
-		$track->TrackID = $track->ID;
-		$track->write();
-
-		Session::set('ExternalLinksTrackID', $track->ID);
-
-		$this->continueJob();
-	}
-
-	/*
-	 * Continues a broken external link check
-	 */
-	public function continueJob() {
-		$task = new CheckExternalLinks();
-		$task->run(null);
+		$status = checkExternalLinks::getLatestTrackStatus();
+		// return if the a job is already running
+		if ($status == 'Running') {
+			return;
+		}
+		if (class_exists('QueuedJobService')) {
+			$checkLinks = new CheckExternalLinksJob();
+			singleton('QueuedJobService')
+				->queueJob($checkLinks, date('Y-m-d H:i:s', time() + 1));
+		} 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 = new CheckExternalLinks();
+			$task->run();
+		}
 	}
 }
diff --git a/code/jobs/CheckExternalLinksJob.php b/code/jobs/CheckExternalLinksJob.php
@@ -8,12 +8,6 @@
  */
 class CheckExternalLinksJob extends AbstractQueuedJob implements QueuedJob {
 
-	public function __construct() {
-		$this->pagesToProcess = Versioned::get_by_stage('SiteTree', 'Live')->column();
-		$this->currentStep = 0;
-		$this->totalSteps = count($this->pagesToProcess);
-	}
-
 	public function getTitle() {
 		return _t('CheckExternalLiksJob.TITLE', 'Checking for external broken links');
 	}
@@ -26,47 +20,14 @@ public function getSignature() {
 		return md5(get_class($this));
 	}
 
-	public function setup() {
-		parent::setup();
-		$restart = $this->currentStep == 0;
-		if ($restart) {
-			$this->pagesToProcess = Versioned::get_by_stage('SiteTree', 'Live')->column();
-		}
-
-	}
-
 	/**
 	 * Check a individual page
 	 */
 	public function process() {
-		$remainingPages = $this->pagesToProcess;
-		if (!count($remainingPages)) {
-			$this->isComplete = true;
-			return;
-		}
-
-		// lets process our first item - note that we take it off the list of things left to do
-		$ID = array_shift($remainingPages);
-
-		// get the page
-		$page = Versioned::get_by_stage('SiteTree', 'Live', 'ID = '.$ID);
-
-		if (!$page || !$page->Count()) {
-			$this->addMessage("Page ID #$ID could not be found, skipping");
-		}
-
 		$task = new CheckExternalLinks();
-		$task->pageToProcess = $page;
 		$task->run();
-
-		// and now we store the new list of remaining children
-		$this->pagesToProcess = $remainingPages;
-		$this->currentStep++;
-
-		if (!count($remainingPages)) {
-			$this->isComplete = true;
-			return;
-		}
+		$this->isComplete = true;
+		return;
 	}
 
 }
diff --git a/code/model/BrokenExternalLink.php b/code/model/BrokenExternalLink.php
@@ -32,9 +32,19 @@ function canView($member = false) {
 	}
 }
 
+class BrokenExternalPageTrackStatus extends DataObject {
+	private static $db = array(
+		'Status' => 'Enum("Completed, Running", "Running")',
+		'TotalPages' => 'Int',
+		'CompletedPages' => 'Int',
+		'JobInfo' => 'Varchar(255)'
+	);
+}
+
 class BrokenExternalPageTrack extends DataObject {
 	private static $db = array(
-		'TrackID' => 'Int'
+		'TrackID' => 'Int',
+		'Processed' => 'Boolean'
 	);
 
 	private static $has_one = array(

diff --git a/code/reports/BrokenExternalLinksReport.php b/code/reports/BrokenExternalLinksReport.php
@@ -68,21 +68,21 @@ public function sourceRecords() {
 	public function getCMSFields() {
 		Requirements::javascript('externallinks/javascript/BrokenExternalLinksReport.js');
 		$fields = parent::getCMSFields();
-		if (class_exists('AbstractQueuedJob')) {
-			$button = '<button id="externalLinksReport" type="button">%s</button>';
-			$runReportButton = new LiteralField(
-				'runReport',
-				sprintf(
-					$button,
-					_t('ExternalBrokenLinksReport.RUNREPORT', 'Create new report')
-				)
-			);
-			$fields->push($runReportButton);
 
-			$reportResultSpan = '</ br></ br><h3 id="ReportHolder"></h3>';
-			$reportResult = new LiteralField('ResultTitle', $reportResultSpan);
-			$fields->push($reportResult);
-		}
+		$reportResultSpan = '</ br></ br><h3 id="ReportHolder"></h3>';
+		$reportResult = new LiteralField('ResultTitle', $reportResultSpan);
+		$fields->push($reportResult);
+
+		$button = '<button id="externalLinksReport" type="button">%s</button>';
+		$runReportButton = new LiteralField(
+			'runReport',
+			sprintf(
+				$button,
+				_t('ExternalBrokenLinksReport.RUNREPORT', 'Create new report')
+			)
+		);
+		$fields->push($runReportButton);
+
 		return $fields;
 	}
 }
diff --git a/code/tasks/CheckExternalLinks.php b/code/tasks/CheckExternalLinks.php
@@ -12,23 +12,63 @@ class CheckExternalLinks extends BuildTask {
 	private $totalPages;
 
 	function run($request) {
-		$trackID = Session::get('ExternalLinksTrackID');
-		if (isset($this->pageToProcess)) {
-			$pages = $this->pageToProcess;
+		$track = CheckExternalLinks::getLatestTrack();
+
+		// if the script has already been started
+		if ($track && $track->Status == 'Running') {
+			$batch = BrokenExternalPageTrack::get()
+				->filter(array(
+					'TrackID' => $track->ID,
+					'Processed' => 0
+				))->limit(10)->column('PageID');
+			$pages = Versioned::get_by_stage('SiteTree', 'Live')
+				->filter('ID', $batch)
+				->limit(10);
+			$this->updateJobInfo('Fetching pages to check');
+			if ($track->CompletedPages == $track->TotalPages) {
+				$track->Status = 'Completed';
+				$track->write();
+				$this->updateJobInfo('Setting to completed');
+			}
+		// if the script is to be started
 		} else {
-			if ($trackID) {
-				$result = BrokenExternalPageTrack::get()
-					->filter('TrackID', $trackID);
-				$pages = Versioned::get_by_stage('SiteTree', 'Live')
-					->exclude('ID', $result->column('PageID'))
-					->limit(10);
-			} else {
-				$pages = Versioned::get_by_stage('SiteTree', 'Live');
+			$pages = Versioned::get_by_stage('SiteTree', 'Live')->column('ID');
+			$noPages = count($pages);
+
+			$track = BrokenExternalPageTrackStatus::create();
+			$track->TotalPages = $noPages;
+			$track->write();
+			$this->updateJobInfo('Creating new tracking object');
+
+			foreach ($pages as $page) {
+				$trackPage = BrokenExternalPageTrack::create();
+				$trackPage->PageID = $page;
+				$trackPage->TrackID = $track->ID;
+				$trackPage->write();
 			}
+
+			$batch = BrokenExternalPageTrack::get()
+				->filter(array(
+					'TrackID' => $track->ID
+				))->limit(10)->column('PageID');
+
+			$pages = Versioned::get_by_stage('SiteTree', 'Live')
+				->filter('ID', $batch);
 		}
+		$trackID = $track->ID;
 		foreach ($pages as $page) {
 			++$this->totalPages;
 
+			if ($track->ID) {
+				$trackPage = BrokenExternalPageTrack::get()
+					->filter(array(
+						'PageID' => $page->ID,
+						'TrackID' => $track->ID
+					))->first();
+				$trackPage->Processed = 1;
+				$trackPage->write();
+			}
+
 			$htmlValue = Injector::inst()->create('HTMLValue', $page->Content);
 			if (!$htmlValue->isValid()) {
 				continue;
@@ -100,12 +140,27 @@ function run($request) {
 				}
 			}
 			++$this->completedPages;
-			if ($trackID) {
-				$trackPage = new BrokenExternalPageTrack();
-				$trackPage->PageID = $page->ID;
-				$trackPage->TrackID = $trackID;
-				$trackPage->write();
+		}
+
+		// run this outside the foreach loop to stop it locking DB rows
+		$this->updateJobInfo('Updating completed pages');
+		$this->updateCompletedPages($trackID);
+
+		// do we need to carry on running the job
+		$track = $this->getLatestTrack();
+		if ($track->CompletedPages >= $track->TotalPages) {
+			$track->Status = 'Completed';
+			$track->write();
+
+			// clear any old previous data
+			$rows = BrokenExternalPageTrack::get()
+				->exclude('TrackID', $track->ID);
+			foreach ($rows as $row) {
+				$row->delete();
 			}
+		} else {
+			$this->updateJobInfo("Running next batch {$track->CompletedPages}/{$track->TotalPages}");
+			$this->run($request);
 		}
 
 		// run this again if queued jobs exists and is a valid int
@@ -115,6 +170,39 @@ function run($request) {
 			singleton('QueuedJobService')
 				->queueJob($checkLinks, date('Y-m-d H:i:s', time() + $queuedJob));
 		}
+	}
+
+	public static function getLatestTrack() {
+		$track = BrokenExternalPageTrackStatus::get()->sort('ID', 'DESC')->first();
+		if (!$track || !$track->exists()) return null;
+		return $track;
+	}
+
+	public static function getLatestTrackID() {
+		$track = CheckExternalLinks::getLatestTrack();
+		if (!$track || !$track->exists()) return null;
+		return $track->ID;
+	}
+
+	public static function getLatestTrackStatus() {
+		$track = CheckExternalLinks::getLatestTrack();
+		if (!$track || !$track->exists()) return null;
+		return $track->Status;
+	}
+
+	private function updateCompletedPages($trackID = 0) {
+		$noPages = BrokenExternalPageTrack::get()
+			->filter(array('TrackID' => $trackID, 'Processed' => 1))->count();
+		$track = $this->getLatestTrack($trackID);
+		$track->CompletedPages = $noPages;
+		$track->write();
+		return $noPages;
+	}
 
+	private function updateJobInfo($message) {
+		$track = CheckExternalLinks::getLatestTrack();
+		if (!$track || !$track->exists()) return null;
+		$track->JobInfo = $message;
+		$track->write();
 	}
 }
diff --git a/composer.json b/composer.json
@@ -13,5 +13,8 @@
 	{
 		"silverstripe/framework": ">=3.0",
 		"silverstripe/cms": ">=3.0"
+	},
+	"suggest": {
+		"silverstripe/queuedjobs": "Speeds up running the job for Content Editors fropm the report"
 	}
 }