MDL-63086 block_rss_client: Move skip time calculation to task
This commit is contained in:
@@ -30,9 +30,6 @@
|
||||
*/
|
||||
|
||||
class block_rss_client extends block_base {
|
||||
/** The maximum time in seconds that cron will wait between attempts to retry failing RSS feeds. */
|
||||
const CLIENT_MAX_SKIPTIME = 43200; // 60 * 60 * 12 seconds.
|
||||
|
||||
/** @var bool track whether any of the output feeds have recorded failures */
|
||||
private $hasfailedfeeds = false;
|
||||
|
||||
@@ -282,24 +279,4 @@
|
||||
return core_text::substr($title, 0, $max - 3) . '...';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates a new skip time for a record based on the current skip time.
|
||||
*
|
||||
* @param int $currentskip The curreent skip time of a record.
|
||||
* @return int A new skip time that should be set.
|
||||
*/
|
||||
public function calculate_skiptime($currentskip) {
|
||||
// The default time to skiptime.
|
||||
$newskiptime = $this->cron * 1.1;
|
||||
if ($currentskip > 0) {
|
||||
// Double the last time.
|
||||
$newskiptime = $currentskip * 2;
|
||||
}
|
||||
if ($newskiptime > self::CLIENT_MAX_SKIPTIME) {
|
||||
// Do not allow the skip time to increase indefinatly.
|
||||
$newskiptime = self::CLIENT_MAX_SKIPTIME;
|
||||
}
|
||||
return $newskiptime;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -37,6 +37,9 @@ defined('MOODLE_INTERNAL') || die();
|
||||
*/
|
||||
class refreshfeeds extends \core\task\scheduled_task {
|
||||
|
||||
/** The maximum time in seconds that cron will wait between attempts to retry failing RSS feeds. */
|
||||
const CLIENT_MAX_SKIPTIME = HOURSECS * 12;
|
||||
|
||||
/**
|
||||
* Name for this task.
|
||||
*
|
||||
@@ -65,7 +68,7 @@ class refreshfeeds extends \core\task\scheduled_task {
|
||||
*/
|
||||
public function execute() {
|
||||
global $CFG, $DB;
|
||||
require_once($CFG->libdir.'/simplepie/moodle_simplepie.php');
|
||||
require_once("{$CFG->libdir}/simplepie/moodle_simplepie.php");
|
||||
|
||||
// We are going to measure execution times.
|
||||
$starttime = microtime();
|
||||
@@ -84,22 +87,11 @@ class refreshfeeds extends \core\task\scheduled_task {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Fetch the rss feed, using standard simplepie caching
|
||||
// so feeds will be renewed only if cache has expired.
|
||||
\core_php_time_limit::raise(60);
|
||||
|
||||
$feed = new \moodle_simplepie();
|
||||
// Set timeout for longer than normal to be agressive at
|
||||
// fetching feeds if possible..
|
||||
$feed->set_timeout(40);
|
||||
$feed->set_cache_duration(0);
|
||||
$feed->set_feed_url($rec->url);
|
||||
$feed->init();
|
||||
$feed = $this->fetch_feed($rec->url);
|
||||
|
||||
if ($feed->error()) {
|
||||
// Skip this feed (for an ever-increasing time if it keeps failing).
|
||||
$block = new \block_rss_client();
|
||||
$rec->skiptime = $block->calculate_skiptime($rec->skiptime);
|
||||
$rec->skiptime = $this->calculate_skiptime($rec->skiptime);
|
||||
$rec->skipuntil = time() + $rec->skiptime;
|
||||
$DB->update_record('block_rss_client', $rec);
|
||||
mtrace("Error: could not load/find the RSS feed - skipping for {$rec->skiptime} seconds.");
|
||||
@@ -119,6 +111,41 @@ class refreshfeeds extends \core\task\scheduled_task {
|
||||
|
||||
// Show times.
|
||||
mtrace($counter . ' feeds refreshed (took ' . microtime_diff($starttime, microtime()) . ' seconds)');
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a feed for the specified URL.
|
||||
*
|
||||
* @param string $url The URL to fetch
|
||||
* @return \moodle_simplepie
|
||||
*/
|
||||
protected function fetch_feed(string $url) : \moodle_simplepie {
|
||||
// Fetch the rss feed, using standard simplepie caching so feeds will be renewed only if cache has expired.
|
||||
\core_php_time_limit::raise(60);
|
||||
|
||||
$feed = new \moodle_simplepie();
|
||||
|
||||
// Set timeout for longer than normal to be agressive at fetching feeds if possible..
|
||||
$feed->set_timeout(40);
|
||||
$feed->set_cache_duration(0);
|
||||
$feed->set_feed_url($url);
|
||||
$feed->init();
|
||||
|
||||
return $feed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculates a new skip time for a record based on the current skip time.
|
||||
*
|
||||
* @param int $currentskip The current skip time of a record.
|
||||
* @return int The newly calculated skip time.
|
||||
*/
|
||||
protected function calculate_skiptime(int $currentskip) : int {
|
||||
// If the feed has never failed, then the initial skiptime will be 0. We use a default of 5 minutes in this case.
|
||||
// If the feed has previously failed then we double that time.
|
||||
$newskiptime = max(MINSECS * 5, ($currentskip * 2));
|
||||
|
||||
// Max out at the CLIENT_MAX_SKIPTIME.
|
||||
return min($newskiptime, self::CLIENT_MAX_SKIPTIME);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -69,81 +69,73 @@ class block_rss_client_cron_testcase extends advanced_testcase {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that when a feed has an error the skip time is increased correctly.
|
||||
* Data provider for skip time tests.
|
||||
*
|
||||
* @return array
|
||||
*/
|
||||
public function test_error() {
|
||||
public function skip_time_increase_provider() : array {
|
||||
return [
|
||||
'Never failed' => [
|
||||
'skiptime' => 0,
|
||||
'skipuntil' => 0,
|
||||
'newvalue' => MINSECS * 5,
|
||||
],
|
||||
'Failed before' => [
|
||||
// This should just double the time.
|
||||
'skiptime' => 330,
|
||||
'skipuntil' => time(),
|
||||
'newvalue' => 660,
|
||||
],
|
||||
'Near max' => [
|
||||
'skiptime' => \block_rss_client\task\refreshfeeds::CLIENT_MAX_SKIPTIME - 5,
|
||||
'skipuntil' => time(),
|
||||
'newvalue' => \block_rss_client\task\refreshfeeds::CLIENT_MAX_SKIPTIME,
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that when a feed has an error the skip time is increased correctly.
|
||||
*
|
||||
* @dataProvider skip_time_increase_provider
|
||||
*/
|
||||
public function test_error($skiptime, $skipuntil, $newvalue) {
|
||||
global $DB, $CFG;
|
||||
$this->resetAfterTest();
|
||||
|
||||
require_once("{$CFG->libdir}/simplepie/moodle_simplepie.php");
|
||||
|
||||
$time = time();
|
||||
// A record that has failed before.
|
||||
$record = (object) array(
|
||||
$record = (object) [
|
||||
'userid' => 1,
|
||||
'title' => 'Skip test feed',
|
||||
'preferredtitle' => '',
|
||||
'description' => 'A feed to test the skip time.',
|
||||
'shared' => 0,
|
||||
'url' => 'http://example.com/rss',
|
||||
'skiptime' => 330,
|
||||
'skipuntil' => $time - 300,
|
||||
);
|
||||
'skiptime' => $skiptime,
|
||||
'skipuntil' => $skipuntil,
|
||||
];
|
||||
$record->id = $DB->insert_record('block_rss_client', $record);
|
||||
|
||||
// A record that has not failed before.
|
||||
$record2 = (object) array(
|
||||
'userid' => 1,
|
||||
'title' => 'Skip test feed',
|
||||
'preferredtitle' => '',
|
||||
'description' => 'A feed to test the skip time.',
|
||||
'shared' => 0,
|
||||
'url' => 'http://example.com/rss2',
|
||||
'skiptime' => 0,
|
||||
'skipuntil' => 0,
|
||||
);
|
||||
$record2->id = $DB->insert_record('block_rss_client', $record2);
|
||||
// Run the scheduled task and have it fail.
|
||||
$task = $this->getMockBuilder(\block_rss_client\task\refreshfeeds::class)
|
||||
->setMethods(['fetch_feed'])
|
||||
->getMock();
|
||||
|
||||
// A record that is near the maximum wait time.
|
||||
$record3 = (object) array(
|
||||
'userid' => 1,
|
||||
'title' => 'Skip test feed',
|
||||
'preferredtitle' => '',
|
||||
'description' => 'A feed to test the skip time.',
|
||||
'shared' => 0,
|
||||
'url' => 'http://example.com/rss3',
|
||||
'skiptime' => block_rss_client::CLIENT_MAX_SKIPTIME - 5,
|
||||
'skipuntil' => $time - 1,
|
||||
);
|
||||
$record3->id = $DB->insert_record('block_rss_client', $record3);
|
||||
$piemock = $this->getMockBuilder(\moodle_simplepie::class)
|
||||
->setMethods(['error'])
|
||||
->getMock();
|
||||
|
||||
// Run the scheduled task.
|
||||
$task = new \block_rss_client\task\refreshfeeds();
|
||||
ob_start();
|
||||
$piemock->method('error')
|
||||
->willReturn(true);
|
||||
|
||||
// Silence SimplePie php notices.
|
||||
$errorlevel = error_reporting($CFG->debug & ~E_USER_NOTICE);
|
||||
$task->method('fetch_feed')
|
||||
->willReturn($piemock);
|
||||
|
||||
// Run the cron and capture its output.
|
||||
$this->expectOutputRegex("/.*Error: could not load\/find the RSS feed - skipping for {$newvalue} seconds.*/");
|
||||
$task->execute();
|
||||
error_reporting($errorlevel);
|
||||
|
||||
$cronoutput = ob_get_clean();
|
||||
$skiptime1 = $record->skiptime * 2;
|
||||
$message1 = 'http://example.com/rss Error: could not load/find the RSS feed - skipping for ' . $skiptime1 . ' seconds.';
|
||||
$this->assertContains($message1, $cronoutput);
|
||||
$skiptime2 = 0;
|
||||
$message2 = 'http://example.com/rss2 Error: could not load/find the RSS feed - skipping for ' . $skiptime2 . ' seconds.';
|
||||
$this->assertContains($message2, $cronoutput);
|
||||
$skiptime3 = block_rss_client::CLIENT_MAX_SKIPTIME;
|
||||
$message3 = 'http://example.com/rss3 Error: could not load/find the RSS feed - skipping for ' . $skiptime3 . ' seconds.';
|
||||
$this->assertContains($message3, $cronoutput);
|
||||
$this->assertContains('0 feeds refreshed (took ', $cronoutput);
|
||||
|
||||
// Test that the records have been correctly updated.
|
||||
$newrecord = $DB->get_record('block_rss_client', array('id' => $record->id));
|
||||
$this->assertAttributeEquals($skiptime1, 'skiptime', $newrecord);
|
||||
$this->assertAttributeGreaterThanOrEqual($time + $skiptime1, 'skipuntil', $newrecord);
|
||||
$newrecord2 = $DB->get_record('block_rss_client', array('id' => $record2->id));
|
||||
$this->assertAttributeEquals($skiptime2, 'skiptime', $newrecord2);
|
||||
$this->assertAttributeGreaterThanOrEqual($time + $skiptime2, 'skipuntil', $newrecord2);
|
||||
$newrecord3 = $DB->get_record('block_rss_client', array('id' => $record3->id));
|
||||
$this->assertAttributeEquals($skiptime3, 'skiptime', $newrecord3);
|
||||
$this->assertAttributeGreaterThanOrEqual($time + $skiptime3, 'skipuntil', $newrecord3);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user