From a866538b9b63562a1dc6fd80a6e7d1a7ad6b5c93 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 13 Oct 2014 16:32:48 +0100 Subject: [PATCH] MDL-47649 csvlib: must not leave files open I added a destructor to csv_import_reader ensure the file is closed. I also ensured that the gradeimport_csv unit tests did not hold onto a reference to the csv_import_reader object while the phpunit framework tries to tidy up moodledata. Finally, to stop CiBoT complaining, I fixed up the PHP4-style declarations in the CSV import class. --- grade/import/csv/tests/load_data_test.php | 4 ++ lib/csvlib.class.php | 51 ++++++++++++++--------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/grade/import/csv/tests/load_data_test.php b/grade/import/csv/tests/load_data_test.php index 790f889b814..1d57c7d2767 100644 --- a/grade/import/csv/tests/load_data_test.php +++ b/grade/import/csv/tests/load_data_test.php @@ -59,6 +59,10 @@ Bobby,Bunce,,"Moodle HQ","Rock on!",student5@mail.com,75.00,75.00'; /** @var array $columns The first row of the csv file. These are the columns of the import file.*/ protected $columns; + public function tearDown() { + $this->csvimport = null; + } + /** * Load up the above text through the csv import. * diff --git a/lib/csvlib.class.php b/lib/csvlib.class.php index 229d5680044..f159d5aaaf3 100644 --- a/lib/csvlib.class.php +++ b/lib/csvlib.class.php @@ -35,26 +35,31 @@ defined('MOODLE_INTERNAL') || die(); * @package moodlecore */ class csv_import_reader { + /** * @var int import identifier */ - var $_iid; + private $_iid; + /** * @var string which script imports? */ - var $_type; + private $_type; + /** * @var string|null Null if ok, error msg otherwise */ - var $_error; + private $_error; + /** * @var array cached columns */ - var $_columns; + private $_columns; + /** * @var object file handle used during import */ - var $_fp; + private $_fp; /** * Contructor @@ -62,24 +67,29 @@ class csv_import_reader { * @param int $iid import identifier * @param string $type which script imports? */ - function csv_import_reader($iid, $type) { + public function __construct($iid, $type) { $this->_iid = $iid; $this->_type = $type; } + /** + * Make sure the file is closed when this object is discarded. + */ + public function __destruct() { + $this->close(); + } + /** * Parse this content * - * @global object - * @global object - * @param string $content passed by ref for memory reasons, unset after return + * @param string $content the content to parse. * @param string $encoding content encoding * @param string $delimiter_name separator (comma, semicolon, colon, cfg) * @param string $column_validation name of function for columns validation, must have one param $columns * @param string $enclosure field wrapper. One character only. * @return bool false if error, count of data lines if ok; use get_error() to get error string */ - function load_csv_content(&$content, $encoding, $delimiter_name, $column_validation=null, $enclosure='"') { + public function load_csv_content($content, $encoding, $delimiter_name, $column_validation=null, $enclosure='"') { global $USER, $CFG; $this->close(); @@ -182,7 +192,7 @@ class csv_import_reader { * * @return array */ - function get_columns() { + public function get_columns() { if (isset($this->_columns)) { return $this->_columns; } @@ -210,7 +220,7 @@ class csv_import_reader { * @global object * @return bool Success */ - function init() { + public function init() { global $CFG, $USER; if (!empty($this->_fp)) { @@ -232,7 +242,7 @@ class csv_import_reader { * * @return mixed false, or an array of values */ - function next() { + public function next() { if (empty($this->_fp) or feof($this->_fp)) { return false; } @@ -248,7 +258,7 @@ class csv_import_reader { * * @return void */ - function close() { + public function close() { if (!empty($this->_fp)) { fclose($this->_fp); $this->_fp = null; @@ -260,7 +270,7 @@ class csv_import_reader { * * @return string error text of null if none */ - function get_error() { + public function get_error() { return $this->_error; } @@ -271,7 +281,7 @@ class csv_import_reader { * @global object * @param boolean $full true means do a full cleanup - all sessions for current user, false only the active iid */ - function cleanup($full=false) { + public function cleanup($full=false) { global $USER, $CFG; if ($full) { @@ -286,7 +296,7 @@ class csv_import_reader { * * @return array suitable for selection box */ - static function get_delimiter_list() { + public static function get_delimiter_list() { global $CFG; $delimiters = array('comma'=>',', 'semicolon'=>';', 'colon'=>':', 'tab'=>'\\t'); if (isset($CFG->CSV_DELIMITER) and strlen($CFG->CSV_DELIMITER) === 1 and !in_array($CFG->CSV_DELIMITER, $delimiters)) { @@ -301,7 +311,7 @@ class csv_import_reader { * @param string separator name * @return string delimiter char */ - static function get_delimiter($delimiter_name) { + public static function get_delimiter($delimiter_name) { global $CFG; switch ($delimiter_name) { case 'colon': return ':'; @@ -320,7 +330,7 @@ class csv_import_reader { * @param string separator name * @return string encoded delimiter char */ - static function get_encoded_delimiter($delimiter_name) { + public static function get_encoded_delimiter($delimiter_name) { global $CFG; if ($delimiter_name == 'cfg' and isset($CFG->CSV_ENCODE)) { return $CFG->CSV_ENCODE; @@ -336,7 +346,7 @@ class csv_import_reader { * @param string who imports? * @return int iid */ - static function get_new_iid($type) { + public static function get_new_iid($type) { global $USER; $filename = make_temp_directory('csvimport/'.$type.'/'.$USER->id); @@ -351,6 +361,7 @@ class csv_import_reader { } } + /** * Utitily class for exporting of CSV files. * @copyright 2012 Adrian Greeve