diff --git a/lib/ajax/ajaxlib.php b/lib/ajax/ajaxlib.php index c57e6c04014..c9f21f85344 100644 --- a/lib/ajax/ajaxlib.php +++ b/lib/ajax/ajaxlib.php @@ -67,3 +67,40 @@ function ajaxenabled(array $browsers = null) { return false; } } + +/** + * Starts capturing output whilst processing an AJAX request. + * + * This should be used in combination with ajax_check_captured_output to + * report any captured output to the user. + * + * @retrun Boolean Returns true on success or false on failure. + */ +function ajax_capture_output() { + // Start capturing output in case of broken plugins. + return ob_start(); +} + +/** + * Check captured output for content. If the site has a debug level of + * debugdeveloper set, and the content is non-empty, then throw a coding + * exception which can be captured by the Y.IO request and displayed to the + * user. + * + * @return Any output that was captured. + */ +function ajax_check_captured_output() { + global $CFG; + + // Retrieve the output - there should be none. + $output = ob_get_contents(); + ob_end_clean(); + + if ($CFG->debugdeveloper && !empty($output)) { + // Only throw an error if the site is in debugdeveloper. + throw new coding_exception('Unexpected output whilst processing AJAX request. ' . + 'This could be caused by trailing whitespace. Output received: ' . + var_export($output, true)); + } + return $output; +} diff --git a/lib/ajax/getsiteadminbranch.php b/lib/ajax/getsiteadminbranch.php index 9f7d9405787..459d1c388a5 100644 --- a/lib/ajax/getsiteadminbranch.php +++ b/lib/ajax/getsiteadminbranch.php @@ -40,7 +40,7 @@ if ($branchtype !== navigation_node::TYPE_SITE_ADMIN) { } // Start capturing output in case of broken plugins. -ob_start(); +ajax_capture_output(); $PAGE->set_context(context_system::instance()); $PAGE->set_url('/lib/ajax/getsiteadminbranch.php', array('type'=>$branchtype)); @@ -51,12 +51,5 @@ $sitenavigation = new settings_navigation_ajax($PAGE); $converter = new navigation_json(); $branch = $sitenavigation->get('root'); -$output = ob_get_contents(); -ob_end_clean(); -if ($CFG->debugdeveloper && !empty($output)) { - throw new coding_exception('Unexpected output whilst building the administration tree. ' . - 'This could be caused by trailing whitespace. Output received: ' . - var_export($output, true)); -} else { - echo $converter->convert($branch); -} +ajax_check_captured_output(); +echo $converter->convert($branch); diff --git a/lib/tests/ajaxlib_test.php b/lib/tests/ajaxlib_test.php new file mode 100644 index 00000000000..9e80697b586 --- /dev/null +++ b/lib/tests/ajaxlib_test.php @@ -0,0 +1,113 @@ +. + +/** + * Code quality unit tests that are fast enough to run each time. + * + * @package core + * @category phpunit + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + +defined('MOODLE_INTERNAL') || die(); + +class core_ajaxlib_testcase extends advanced_testcase { + + protected function helper_test_clean_output() { + $this->resetAfterTest(); + + $result = ajax_capture_output(); + + // ob_start should normally return without issue. + $this->assertTrue($result); + + $result = ajax_check_captured_output(); + $this->assertEmpty($result); + } + + protected function helper_test_dirty_output($expectexception = false) { + $this->resetAfterTest(); + + // Keep track of the content we will output. + $content = "Some example content"; + + $result = ajax_capture_output(); + + // ob_start should normally return without issue. + $this->assertTrue($result); + + // Fill the output buffer. + echo $content; + + if ($expectexception) { + $this->setExpectedException('coding_exception'); + ajax_check_captured_output(); + } else { + $result = ajax_check_captured_output(); + $this->assertEquals($result, $content); + } + } + + public function test_output_capture_normal_debug_none() { + // In normal conditions, and with DEBUG_NONE set, we should not receive any output or throw any exceptions. + set_debugging(DEBUG_NONE); + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debug_normal() { + // In normal conditions, and with DEBUG_NORMAL set, we should not receive any output or throw any exceptions. + set_debugging(DEBUG_NORMAL); + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debug_all() { + // In normal conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions. + set_debugging(DEBUG_ALL); + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debugdeveloper() { + // In normal conditions, and with DEBUG_DEVELOPER set, we should not receive any output or throw any exceptions. + set_debugging(DEBUG_DEVELOPER); + $this->helper_test_clean_output(); + } + + public function test_output_capture_error_debug_none() { + // With DEBUG_NONE set, we should not throw any exception, but the output will be returned. + set_debugging(DEBUG_NONE); + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debug_normal() { + // With DEBUG_NORMAL set, we should not throw any exception, but the output will be returned. + set_debugging(DEBUG_NORMAL); + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debug_all() { + // In error conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions. + set_debugging(DEBUG_ALL); + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debugdeveloper() { + // With DEBUG_DEVELOPER set, we should throw an exception. + set_debugging(DEBUG_DEVELOPER); + $this->helper_test_dirty_output(true); + } + +} diff --git a/repository/filepicker.js b/repository/filepicker.js index 898b8db37a0..8de36a5ec5d 100644 --- a/repository/filepicker.js +++ b/repository/filepicker.js @@ -582,27 +582,23 @@ M.core_filepicker.init = function(Y, options) { method: 'POST', on: { complete: function(id,o,p) { - if (!o) { - // TODO - alert('IO FATAL'); - return; - } var data = null; try { data = Y.JSON.parse(o.responseText); } catch(e) { - scope.print_msg(M.str.repository.invalidjson, 'error'); - scope.display_error(M.str.repository.invalidjson+'
'+stripHTML(o.responseText)+'
', 'invalidjson') - return; + if (o && o.status && o.status > 0) { + Y.use('moodle-core-notification-exception', function() { + return new M.core.exception(e); + }); + return; + } } // error checking if (data && data.error) { - scope.print_msg(data.error, 'error'); - if (args.onerror) { - args.onerror(id,data,p); - } else { - this.fpnode.one('.fp-content').setContent(''); - } + Y.use('moodle-core-notification-ajaxException', function () { + return new M.core.ajaxException(data); + }); + this.fpnode.one('.fp-content').setContent(''); return; } else { if (data.msg) { diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php index bd2c762681a..fe4316d61a0 100644 --- a/repository/repository_ajax.php +++ b/repository/repository_ajax.php @@ -74,6 +74,8 @@ $repooptions = array( 'ajax' => true, 'mimetypes' => $accepted_types ); + +ajax_capture_output(); $repo = repository::get_repository_by_id($repo_id, $contextid, $repooptions); // Check permissions @@ -97,6 +99,7 @@ switch ($action) { if ($repo->check_login()) { $listing = repository::prepare_listing($repo->get_listing($req_path, $page)); $listing['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($listing); break; } else { @@ -105,23 +108,27 @@ switch ($action) { case 'login': $listing = $repo->print_login(); $listing['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($listing); break; case 'logout': $logout = $repo->logout(); $logout['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($logout); break; case 'searchform': $search_form['repo_id'] = $repo_id; $search_form['form'] = $repo->print_search(); $search_form['allowcaching'] = true; + ajax_check_captured_output(); echo json_encode($search_form); break; case 'search': $search_result = repository::prepare_listing($repo->search($search_text, (int)$page)); $search_result['repo_id'] = $repo_id; $search_result['issearchresult'] = true; + ajax_check_captured_output(); echo json_encode($search_result); break; case 'download': @@ -160,6 +167,7 @@ switch ($action) { $info['file'] = $saveas_filename; $info['type'] = 'link'; $info['url'] = $link; + ajax_check_captured_output(); echo json_encode($info); die; } else { @@ -251,6 +259,7 @@ switch ($action) { // You can cache reository file in this callback // or complete other tasks. $repo->cache_file_by_reference($reference, $storedfile); + ajax_check_captured_output(); echo json_encode($event); die; } else if ($repo->has_moodle_files()) { @@ -261,6 +270,7 @@ switch ($action) { // {@link repository::copy_to_area()}. $fileinfo = $repo->copy_to_area($reference, $record, $maxbytes, $areamaxbytes); + ajax_check_captured_output(); echo json_encode($fileinfo); die; } else { @@ -286,12 +296,14 @@ switch ($action) { $info['e'] = get_string('error', 'moodle'); } } + ajax_check_captured_output(); echo json_encode($info); die; } break; case 'upload': $result = $repo->upload($saveas_filename, $maxbytes); + ajax_check_captured_output(); echo json_encode($result); break; @@ -304,6 +316,7 @@ switch ($action) { $newfilename = required_param('newfilename', PARAM_FILE); $info = repository::overwrite_existing_draftfile($itemid, $filepath, $filename, $newfilepath, $newfilename); + ajax_check_captured_output(); echo json_encode($info); break; @@ -311,6 +324,7 @@ switch ($action) { // delete tmp file $newfilepath = required_param('newfilepath', PARAM_PATH); $newfilename = required_param('newfilename', PARAM_FILE); + ajax_check_captured_output(); echo json_encode(repository::delete_tempfile_from_draft($itemid, $newfilepath, $newfilename)); break;