From: poltawski <poltawski>
Date: Mon, 27 Jul 2009 14:18:01 +0000 (+0000)
Subject: lib/simplepie: Improve Integration MDL-7946, MDL-13932
X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=926987ab5e693a52e3ea90692812ac729df29ec2;p=moodle.git

lib/simplepie: Improve Integration MDL-7946, MDL-13932

Added more unit tests, improved style and fixed:
- Failed behaviour of redirects
- Incorrect behaviour on feed retrival failure
- Ensured rss cache clear for unit test setup
---

diff --git a/lib/simplepie/moodle_simplepie.php b/lib/simplepie/moodle_simplepie.php
index 1893d9a57e..89680aa8e7 100644
--- a/lib/simplepie/moodle_simplepie.php
+++ b/lib/simplepie/moodle_simplepie.php
@@ -21,7 +21,7 @@
  * @package    moodle
  * @subpackage lib
  * @author     Dan Poltawski <talktodan@gmail.com>
- * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  *
  * Customised version of SimplePie for Moodle
  */
@@ -39,26 +39,55 @@ require_once($CFG->libdir.'/simplepie/simplepie.inc');
  * This class extends the stock SimplePie class
  * in order to make sensible configuration choices,
  * such as using the Moodle cache directory and
- * curl functions/proxy config  for making http
- * requests.
+ * curl functions/proxy config for making http
+ * requests in line with moodle configuration.
+ *
+ * @copyright 2009 Dan Poltawski <talktodan@gmail.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @since     Moodle 2.0
  */
 class moodle_simplepie extends SimplePie
 {
-    function __construct($feed_url = null){
-        global $CFG;
+    /** 
+     * Constructor - creates an instance of the SimplePie class
+     * with Moodle defaults.
+     *
+     * @param string $feedurl optional URL of the feed 
+     */
+    function __construct($feedurl = null) {
 
         // Use the Moodle class for http requests
         $this->file_class = 'moodle_simplepie_file';
 
-        // Use sensible cache directory
-        $cachedir = $CFG->dataroot.'/cache/simplepie/';
-        if (!file_exists($cachedir)){
-            mkdir($cachedir, 0777, true);
-        }
+        $cachedir = moodle_simplepie::get_cache_directory();
+        check_dir_exists($cachedir, true, true);
 
-        parent::__construct($feed_url, $cachedir);
+        parent::__construct($feedurl, $cachedir);
         parent::set_output_encoding('UTF-8');
     }
+
+    /**
+     * Get path for feed cache directory
+     *
+     * @return string absolute path to cache directory
+     */
+    private static function get_cache_directory() {
+        global $CFG;
+
+        return $CFG->dataroot.'/cache/simplepie/';
+    }
+
+    /**
+     * Reset RSS cache 
+     *
+     * @return boolean success if cache clear or didn't exist
+     */
+    public static function reset_cache() {
+
+        $cachedir = moodle_simplepie::get_cache_directory();
+
+        return remove_dir($cachedir);
+    }
 }
 
 /**
@@ -77,25 +106,24 @@ class moodle_simplepie_file extends SimplePie_File
      * been modifed to add in use the Moodle curl class rather than php curl
      * functions.
      */
-    function moodle_simplepie_file($url, $timeout = 10, $redirects = 5, $headers = null, $useragent = null, $force_fsockopen = false)
-    {
+    function moodle_simplepie_file($url, $timeout = 10, $redirects = 5, $headers = null, $useragent = null, $force_fsockopen = false) {
         $this->url = $url;
         $this->method = SIMPLEPIE_FILE_SOURCE_REMOTE | SIMPLEPIE_FILE_SOURCE_CURL;
 
         $curl = new curl();
         $curl->setopt(array('CURLOPT_HEADER'=>true));
 
-        try{
-            $this->headers = $curl->get($url);
-        }catch(moodle_exception $e){
+        $this->headers = $curl->get($url);
+
+        if ($curl->error) {
             $this->error = 'cURL Error: '.$curl->error;
             $this->success = false;
             return false;
         }
 
-        $parser =& new SimplePie_HTTP_Parser($this->headers);
+        $parser = new SimplePie_HTTP_Parser($this->headers);
 
-        if ($parser->parse()){
+        if ($parser->parse()) {
             $this->headers = $parser->headers;
             $this->body = $parser->body;
             $this->status_code = $parser->status_code;
@@ -105,7 +133,7 @@ class moodle_simplepie_file extends SimplePie_File
             {
                 $this->redirects++;
                 $location = SimplePie_Misc::absolutize_url($this->headers['location'], $url);
-                return $this->SimplePie_File($location, $timeout, $redirects, $headers);
+                return $this->moodle_simplepie_file($location, $timeout, $redirects, $headers);
             }
         }
     }
diff --git a/lib/simpletest/testrss.php b/lib/simpletest/testrss.php
index 407b3099e9..e59d0fd198 100644
--- a/lib/simpletest/testrss.php
+++ b/lib/simpletest/testrss.php
@@ -5,9 +5,13 @@ if (!defined('MOODLE_INTERNAL')) {
 }
 
 /*
- * * rsstest.xml: One valid rss feed.
- *     md5:  8fd047914863bf9b3a4b1514ec51c32c
- *         size: 32188
+ * These tests rely on the rsstest.xml file on download.moodle.org,
+ * from eloys listing:
+ *   rsstest.xml: One valid rss feed.
+ *   md5:  8fd047914863bf9b3a4b1514ec51c32c
+ *   size: 32188
+ * 
+ * If networking/proxy configuration is wrong these tests will fail..
  */
 
 require_once($CFG->libdir.'/simplepie/moodle_simplepie.php');
@@ -15,10 +19,20 @@ require_once($CFG->libdir.'/simplepie/moodle_simplepie.php');
 class moodlesimplepie_test extends UnitTestCase {
 
     public static $includecoverage = array('lib/simplepie/moodle_simplepie.php');
-    var $testurl = 'http://download.moodle.org/unittest/rsstest.xml';
 
-    function test_getfeed(){
-        $feed = new moodle_simplepie($this->testurl);
+    # A url we know exists and is valid
+    const VALIDURL = 'http://download.moodle.org/unittest/rsstest.xml';
+    # A url which we know doesn't exist
+    const INVALIDURL = 'http://download.moodle.org/unittest/rsstest-which-doesnt-exist.xml';
+    # This tinyurl redirects to th rsstest.xml file
+    const REDIRECTURL = 'http://tinyurl.com/lvyslv';
+
+    function setUp() {
+        moodle_simplepie::reset_cache();
+    }
+
+    function test_getfeed() {
+        $feed = new moodle_simplepie(moodlesimplepie_test::VALIDURL);
 
         $this->assertIsA($feed, 'moodle_simplepie');
 
@@ -61,17 +75,48 @@ EOD;
         // TODO fix this so it uses $CFG by default
         $this->assertEqual($itemone->get_date('U'), 1196412453);
 
-
         // last item
         $this->assertTrue($feed->get_item(14));
         // Past last item
         $this->assertFalse($feed->get_item(15));
     }
 
-    function test_failfeed(){
-        $feed = new moodle_simplepie('http://111xxxxxxxxxxxxxmoodle.org/');
+    /*
+     * Test retrieving a url which doesn't exist
+     */
+    function test_failurl() {
+        $feed = new moodle_simplepie(moodlesimplepie_test::INVALIDURL);
 
         $this->assertTrue($feed->error());
     }
 
+    /*
+     * Test retrieving a url with broken proxy configuration
+     */
+    function test_failproxy() {
+        global $CFG;
+
+        $oldproxy = $CFG->proxyhost;
+        $CFG->proxyhost = 'xxxxxxxxxxxxxxx.moodle.org';
+
+        $feed = new moodle_simplepie(moodlesimplepie_test::VALIDURL);
+
+        $this->assertTrue($feed->error());
+        $this->assertFalse($feed->get_title());
+        $CFG->proxyhost = $oldproxy;
+    }
+
+    /*
+     * Test retrieving a url which sends a redirect to another valid feed
+     */
+    function test_redirect() {
+        global $CFG;
+
+        $feed = new moodle_simplepie(moodlesimplepie_test::REDIRECTURL);
+
+        $this->assertFalse($feed->error());
+        $this->assertEqual($feed->get_title(), 'Moodle News');
+        $this->assertEqual($feed->get_link(), 'http://moodle.org/mod/forum/view.php?f=1');
+    }
+
 }