diff --git a/blocks/rss_client/block_rss_client.php b/blocks/rss_client/block_rss_client.php index 2838651cd43..68a13ac2320 100644 --- a/blocks/rss_client/block_rss_client.php +++ b/blocks/rss_client/block_rss_client.php @@ -23,6 +23,8 @@ */ 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. function init() { $this->title = get_string('pluginname', 'block_rss_client'); @@ -275,21 +277,35 @@ } /** - * cron - goes through all feeds and retrieves them with the cache - * duration set to 0 in order to force the retrieval of the item and - * refresh the cache + * cron - goes through all the feeds. If the feed has a skipuntil value + * that is less than the current time cron will attempt to retrieve it + * with the cache duration set to 0 in order to force the retrieval of + * the item and refresh the cache. * - * @return boolean true if all feeds were retrieved succesfully + * If a feed fails then the skipuntil time of that feed is set to be + * later than the next expected cron time. The amount of time will + * increase each time the fetch fails until the maximum is reached. + * + * If a feed that has been failing is successfully retrieved it will + * go back to being handled as though it had never failed. + * + * CRON should therefor process requests for permanently broken RSS + * feeds infrequently, and temporarily unavailable feeds will be tried + * less often until they become available again. + * + * @return boolean Always returns true */ function cron() { global $CFG, $DB; require_once($CFG->libdir.'/simplepie/moodle_simplepie.php'); + // Get the legacy cron time, strangely the cron property of block_base + // does not seem to get set. This means we must retrive it here. + $this->cron = $DB->get_field('block', 'cron', array('name' => 'rss_client')); + // We are going to measure execution times $starttime = microtime(); - - // And we have one initial $status - $status = true; + $starttimesec = time(); // Fetch all site feeds. $rs = $DB->get_recordset('block_rss_client'); @@ -297,6 +313,13 @@ mtrace(''); foreach ($rs as $rec) { mtrace(' ' . $rec->url . ' ', ''); + + // Skip feed if it failed recently. + if ($starttimesec < $rec->skipuntil) { + mtrace('skipping until ' . userdate($rec->skipuntil)); + 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); @@ -310,20 +333,49 @@ $feed->init(); if ($feed->error()) { - mtrace('Error: could not load/find the RSS feed'); - $status = false; + // Skip this feed (for an ever-increasing time if it keeps failing). + $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."); } else { mtrace ('ok'); + // It worked this time, so reset the skiptime. + if ($rec->skiptime > 0) { + $rec->skiptime = 0; + $rec->skipuntil = 0; + $DB->update_record('block_rss_client', $rec); + } + // Only increase the counter when a feed is sucesfully refreshed. + $counter ++; } - $counter ++; } $rs->close(); // Show times mtrace($counter . ' feeds refreshed (took ' . microtime_diff($starttime, microtime()) . ' seconds)'); - // And return $status - return $status; + return true; + } + + /** + * 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. + */ + protected 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; } } diff --git a/blocks/rss_client/db/install.xml b/blocks/rss_client/db/install.xml index f81d650cf62..7a7e9cb4b6c 100644 --- a/blocks/rss_client/db/install.xml +++ b/blocks/rss_client/db/install.xml @@ -1,5 +1,5 @@ - @@ -13,6 +13,8 @@ + + diff --git a/blocks/rss_client/db/upgrade.php b/blocks/rss_client/db/upgrade.php new file mode 100644 index 00000000000..26fa4b5ec66 --- /dev/null +++ b/blocks/rss_client/db/upgrade.php @@ -0,0 +1,54 @@ +. + +/** + * Database upgrades for the RSS block. + * + * @package block_rss_client + * @copyright 2014 Davo Smith + * @author Neill Magill + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL + */ +defined('MOODLE_INTERNAL') || die(); + +/** + * Upgrade the block_rss_client database. + * + * @param int $oldversion The version number of the plugin that was installed. + * @return boolean + */ +function xmldb_block_rss_client_upgrade($oldversion) { + global $DB; + $dbman = $DB->get_manager(); + + if ($oldversion < 2015071700) { + // Support for skipping RSS feeds for a while when they fail. + $table = new xmldb_table('block_rss_client'); + // How many seconds we are currently ignoring this RSS feed for (due to an error). + $field = new xmldb_field('skiptime', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0', 'url'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + // When to next update this RSS feed. + $field = new xmldb_field('skipuntil', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0', 'skiptime'); + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + upgrade_block_savepoint(true, 2015071700, 'rss_client'); + } + + return true; +} diff --git a/blocks/rss_client/tests/cron_test.php b/blocks/rss_client/tests/cron_test.php new file mode 100644 index 00000000000..d6958ac45c1 --- /dev/null +++ b/blocks/rss_client/tests/cron_test.php @@ -0,0 +1,139 @@ +. + +/** + * PHPunit tests for rss client cron. + * + * @package block_rss_client + * @copyright 2015 University of Nottingham + * @author Neill Magill + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +defined('MOODLE_INTERNAL') || die(); +require_once(dirname(dirname(__DIR__)) . '/moodleblock.class.php'); +require_once(dirname(__DIR__) . '/block_rss_client.php'); + +/** + * Class for the PHPunit tests for rss client cron. + * + * @package block_rss_client + * @copyright 2015 Universit of Nottingham + * @author Neill Magill + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class block_rss_client_cron_testcase extends advanced_testcase { + /** + * Test that when a record has a skipuntil time that is greater + * than the current time the attempt is skipped. + */ + public function test_skip() { + global $DB; + $this->resetAfterTest(); + // Create a RSS feed record with a skip until time set to the future. + $record = (object) array( + '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, + ); + $DB->insert_record('block_rss_client', $record); + + $block = new block_rss_client(); + ob_start(); + $block->cron(); + $cronoutput = ob_get_clean(); + $this->assertContains('skipping until ' . userdate($record->skipuntil), $cronoutput); + $this->assertContains('0 feeds refreshed (took ', $cronoutput); + } + + /** + * Test that when a feed has an error the skip time is increaed correctly. + */ + public function test_error() { + global $DB; + $this->resetAfterTest(); + $time = time(); + // A record that has failed before. + $record = (object) array( + '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, + ); + $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); + + // 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); + + // Run the cron. + $block = new block_rss_client(); + ob_start(); + $block->cron(); + $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 = 330; // Assumes that the cron time in the version file is 300. + $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); + } +} \ No newline at end of file diff --git a/blocks/rss_client/version.php b/blocks/rss_client/version.php index d1979d34e0c..2e2cea99651 100644 --- a/blocks/rss_client/version.php +++ b/blocks/rss_client/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2015051100; // The current plugin version (Date: YYYYMMDDXX) +$plugin->version = 2015071700; // The current plugin version (Date: YYYYMMDDXX) $plugin->requires = 2015050500; // Requires this Moodle version $plugin->component = 'block_rss_client'; // Full name of the plugin (used for diagnostics) $plugin->cron = 300; // Set min time between cron executions to 300 secs (5 mins)