From f0a0f3c2bda95cf23b9d870c7fd819e21d727b76 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Thu, 20 Mar 2014 12:11:30 +0800 Subject: [PATCH 1/4] MDL-44596 logging: Improve management interfaces for reports and stores This patch methods to the manager that uses callbacks from reports to determine how reports and stores are compatible with each other. This information is then displayed in the management pages. --- admin/reports.php | 18 ++++- admin/tool/log/classes/log/manager.php | 81 +++++++++++++++++++ .../tool/log/classes/setting_managestores.php | 19 +++-- admin/tool/log/lang/en/tool_log.php | 1 + lang/en/admin.php | 3 + lib/classes/log/dummy_manager.php | 4 + lib/classes/log/manager.php | 10 +++ 7 files changed, 128 insertions(+), 8 deletions(-) diff --git a/admin/reports.php b/admin/reports.php index 46c244fb002..b02ba0d1542 100644 --- a/admin/reports.php +++ b/admin/reports.php @@ -41,8 +41,9 @@ echo $OUTPUT->heading(get_string('reports')); $struninstall = get_string('uninstallplugin', 'core_admin'); $table = new flexible_table('reportplugins_administration_table'); -$table->define_columns(array('name', 'version', 'uninstall')); -$table->define_headers(array(get_string('plugin'), get_string('version'), $struninstall)); +$table->define_columns(array('name', 'logstoressupported', 'version', 'uninstall')); +$table->define_headers(array(get_string('plugin'), get_string('logstoressupported', 'admin'), get_string('version'), + $struninstall)); $table->define_baseurl($PAGE->url); $table->set_attribute('id', 'reportplugins'); $table->set_attribute('class', 'admintable generaltable'); @@ -71,12 +72,23 @@ foreach ($installed as $config) { } } +$logmanager = get_log_manager(); + foreach ($plugins as $plugin => $name) { $uninstall = ''; if ($uninstallurl = core_plugin_manager::instance()->get_uninstall_url('report_'.$plugin, 'manage')) { $uninstall = html_writer::link($uninstallurl, $struninstall); } + $stores = $logmanager->get_supported_logstores('report_' . $plugin); + if ($stores === false) { + $supportedstores = get_string('logstorenotrequired', 'admin'); + } else if (!empty($stores)) { + $supportedstores = implode(', ', $stores); + } else { + $supportedstores = get_string('nosupportedlogstore', 'admin');; + } + if (!isset($versions[$plugin])) { if (file_exists("$CFG->dirroot/report/$plugin/version.php")) { // not installed yet @@ -96,7 +108,7 @@ foreach ($plugins as $plugin => $name) { } } - $table->add_data(array($name, $version, $uninstall)); + $table->add_data(array($name, $supportedstores, $version, $uninstall)); } $table->print_html(); diff --git a/admin/tool/log/classes/log/manager.php b/admin/tool/log/classes/log/manager.php index 4bffed0d3ca..b113bad287f 100644 --- a/admin/tool/log/classes/log/manager.php +++ b/admin/tool/log/classes/log/manager.php @@ -110,6 +110,87 @@ class manager implements \core\log\manager { return $return; } + /** + * Get a list of reports that support the given store instance. + * + * @param string $logstore Name of the store. + * + * @return array List of supported reports + */ + public function get_supported_reports($logstore) { + + $allstores = self::get_store_plugins(); + if (empty($allstores[$logstore])) { + // Store doesn't exist. + return array(); + } + + $reports = \core_component::get_plugin_list('report'); + $enabled = $this->stores; + + if (empty($enabled[$logstore])) { + // Store is not enabled, init an instance. + $classname = '\\' . $logstore . '\log\store'; + $instance = new $classname($this); + } else { + $instance = $enabled[$logstore]; + } + + $return = array(); + foreach ($reports as $report => $fulldir) { + $file = $fulldir . '/lib.php'; + if (file_exists($file)) { + require_once($file); + $function = 'report_' . $report . '_supports_logstore'; + if (function_exists($function)) { + if ($function($instance)) { + $return[$report] = get_string('pluginname', 'report_' . $report); + } + } + + } + } + + return $return; + } + + /** + * For a given report, returns a list of log stores that are supported. + * + * @param string $component component. + * + * @return false|array list of logstores that support the given report. It returns false if the given $component doesn't + * require logstores. + */ + public function get_supported_logstores($component) { + + $allstores = self::get_store_plugins(); + $enabled = $this->stores; + + $function = $component . '_supports_logstore'; + $file = \core_component::get_component_directory($component) . '/lib.php'; + + if (!file_exists($file)) { + // The report doesn't define the callback, most probably it doesn't need log stores. + return false; + } + + require_once($file); + if (!function_exists($function)) { + // The report doesn't define the callback, most probably it doesn't need log stores. + return false; + } + + $return = array(); + foreach ($allstores as $store => $logclass) { + $instance = empty($enabled[$store]) ? new $logclass($this) : $enabled[$store]; + if ($function($instance)) { + $return[$store] = get_string('pluginname', $store); + } + } + return $return; + } + /** * Intended for store management, do not use from reports. * diff --git a/admin/tool/log/classes/setting_managestores.php b/admin/tool/log/classes/setting_managestores.php index bb82a82560b..4161a5770cd 100644 --- a/admin/tool/log/classes/setting_managestores.php +++ b/admin/tool/log/classes/setting_managestores.php @@ -109,8 +109,8 @@ class tool_log_setting_managestores extends admin_setting { $strversion = get_string('version'); $pluginmanager = core_plugin_manager::instance(); - - $available = \tool_log\log\manager::get_store_plugins(); + $logmanager = new \tool_log\log\manager(); + $available = $logmanager->get_store_plugins(); $enabled = get_config('tool_log', 'enabled_stores'); if (!$enabled) { $enabled = array(); @@ -132,8 +132,10 @@ class tool_log_setting_managestores extends admin_setting { $return .= $OUTPUT->box_start('generalbox loggingui'); $table = new html_table(); - $table->head = array(get_string('name'), $strversion, $strenable, $strup . '/' . $strdown, $strsettings, $struninstall); - $table->colclasses = array('leftalign', 'centeralign', 'centeralign', 'centeralign', 'centeralign', 'centeralign'); + $table->head = array(get_string('name'), get_string('reportssupported', 'tool_log'), $strversion, $strenable, + $strup . '/' . $strdown, $strsettings, $struninstall); + $table->colclasses = array('leftalign', 'centeralign', 'centeralign', 'centeralign', 'centeralign', 'centeralign', + 'centeralign'); $table->id = 'logstoreplugins'; $table->attributes['class'] = 'admintable generaltable'; $table->data = array(); @@ -156,6 +158,13 @@ class tool_log_setting_managestores extends admin_setting { $name = $store; } + $reports = $logmanager->get_supported_reports($store); + if (!empty($reports)) { + $supportedreports = implode(', ', $reports); + } else { + $supportedreports = '-'; + } + // Hide/show links. if (isset($enabled[$store])) { $aurl = new moodle_url($url, array('action' => 'disable', 'store' => $store)); @@ -220,7 +229,7 @@ class tool_log_setting_managestores extends admin_setting { } // Add a row to the table. - $table->data[] = array($icon . $displayname, $version, $hideshow, $updown, $settings, $uninstall); + $table->data[] = array($icon . $displayname, $supportedreports, $version, $hideshow, $updown, $settings, $uninstall); $printed[$store] = true; } diff --git a/admin/tool/log/lang/en/tool_log.php b/admin/tool/log/lang/en/tool_log.php index 7301031633b..ef20f859e76 100644 --- a/admin/tool/log/lang/en/tool_log.php +++ b/admin/tool/log/lang/en/tool_log.php @@ -26,4 +26,5 @@ $string['actlogshdr'] = 'Available log stores'; $string['configlogplugins'] = 'Please enable all required plugins and arrange then in appropriate order.'; $string['logging'] = 'Logging'; $string['managelogging'] = 'Manage log stores'; +$string['reportssupported'] = 'Reports supported'; $string['pluginname'] = 'Log store manager'; diff --git a/lang/en/admin.php b/lang/en/admin.php index 86fa99f4f4b..8221742a3c8 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -651,6 +651,8 @@ $string['loginpageautofocus_help'] = 'Enabling this option improves usability of $string['loginpasswordautocomplete'] = 'Prevent password autocompletion on login form'; $string['loginpasswordautocomplete_help'] = 'Having this off will let users save their account password in their browser. Switching this setting on will result in your site no longer following XHTML strict validation rules.'; $string['loglifetime'] = 'Keep logs for'; +$string['logstorenotrequired'] = 'Log store not required'; +$string['logstoressupported'] = 'Log stores that support this report'; $string['longtimewarning'] = 'Please note that this process can take a long time.'; $string['maintenancemode'] = 'In maintenance mode'; $string['maintenancemodeisscheduled'] = 'This site will be switched to maintenance mode in {$a->min} mins {$a->sec} secs'; @@ -747,6 +749,7 @@ $string['nohttpsformobilewarning'] = 'It is recommended to enable HTTPS with a v $string['nomissingstrings'] = 'No missing strings'; $string['nonewsettings'] = 'No new settings were added during this upgrade.'; $string['nonexistentbookmark'] = 'The bookmark you requested does not exist.'; +$string['nosupportedlogstore'] = 'No supported logstore found'; $string['maxtimelimit'] = 'Maximum time limit'; $string['maxtimelimit_desc'] = 'To restrict the maximum PHP execution time that Moodle will allow without any output being displayed, enter a value in seconds here. 0 means that Moodle default restrictions are used. If you have a front-end server with its own time limit, set this value lower to receive PHP errors in logs. Does not apply to CLI scripts.'; $string['noresults'] = 'No results found.'; diff --git a/lib/classes/log/dummy_manager.php b/lib/classes/log/dummy_manager.php index 818e9ba2fea..5b4fc99f2a0 100644 --- a/lib/classes/log/dummy_manager.php +++ b/lib/classes/log/dummy_manager.php @@ -34,4 +34,8 @@ class dummy_manager implements manager { public function dispose() { } + + public function get_supported_logstores($component) { + return array(); + } } diff --git a/lib/classes/log/manager.php b/lib/classes/log/manager.php index b7b0849ea20..b6f39133c12 100644 --- a/lib/classes/log/manager.php +++ b/lib/classes/log/manager.php @@ -49,4 +49,14 @@ interface manager { * @return void */ public function dispose(); + + /** + * For a given report, returns a list of log stores that are supported. + * + * @param string $component component. + * + * @return false|array list of logstores that support the given report. It returns false if the given $component doesn't + * require logstores. + */ + public function get_supported_logstores($component); } From 8ead8802fbdf44098f86c6fbed8a1ce7959b77ae Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Thu, 20 Mar 2014 12:12:25 +0800 Subject: [PATCH 2/4] MDL-44596 logging: Add callbacks to all reports that support log stores --- report/log/lib.php | 14 ++++++ report/log/tests/lib_test.php | 57 +++++++++++++++++++++++++ report/loglive/lib.php | 14 ++++++ report/loglive/tests/lib_test.php | 57 +++++++++++++++++++++++++ report/outline/lib.php | 14 ++++++ report/outline/tests/lib_test.php | 56 ++++++++++++++++++++++++ report/participation/lib.php | 14 ++++++ report/participation/tests/lib_test.php | 56 ++++++++++++++++++++++++ report/stats/lib.php | 14 ++++++ report/stats/tests/lib_test.php | 56 ++++++++++++++++++++++++ 10 files changed, 352 insertions(+) create mode 100644 report/log/tests/lib_test.php create mode 100644 report/loglive/tests/lib_test.php create mode 100644 report/outline/tests/lib_test.php create mode 100644 report/participation/tests/lib_test.php create mode 100644 report/stats/tests/lib_test.php diff --git a/report/log/lib.php b/report/log/lib.php index c45bf1c8454..867da4dc4d9 100644 --- a/report/log/lib.php +++ b/report/log/lib.php @@ -40,6 +40,20 @@ function report_log_extend_navigation_course($navigation, $course, $context) { } } +/** + * Callback to verify if the given instance of store is supported by this report or not. + * + * @param string $instance store instance. + * + * @return bool returns true if the store is supported by the report, false otherwise. + */ +function report_log_supports_logstore($instance) { + if ($instance instanceof \core\log\sql_select_reader) { + return true; + } + return false; +} + /** * This function extends the course navigation with the report items * diff --git a/report/log/tests/lib_test.php b/report/log/tests/lib_test.php new file mode 100644 index 00000000000..0b605f61deb --- /dev/null +++ b/report/log/tests/lib_test.php @@ -0,0 +1,57 @@ +. + +/** + * Tests for report library functions. + * + * @package report_log + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class report_log_events_testcase. + * + * @package report_log + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class report_log_lib_testcase extends advanced_testcase { + + /** + * Test report_log_supports_logstore. + */ + public function test_report_log_supports_logstore() { + $logmanager = get_log_manager(); + $allstores = \core_component::get_plugin_list_with_class('logstore', 'log\store'); + + $supportedstores = array( + 'logstore_database' => '\logstore_database\log\store', + 'logstore_legacy' => '\logstore_legacy\log\store', + 'logstore_standard' => '\logstore_standard\log\store' + ); + + // Make sure all supported stores are installed. + $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); + $stores = $logmanager->get_supported_logstores('log'); + $stores = array_keys($stores); + foreach ($expectedstores as $expectedstore) { + $this->assertContains($expectedstore, $stores); + } + } +} diff --git a/report/loglive/lib.php b/report/loglive/lib.php index be4edf243d3..c0d3237bdac 100644 --- a/report/loglive/lib.php +++ b/report/loglive/lib.php @@ -43,3 +43,17 @@ function report_loglive_extend_navigation_course($navigation, $course, $context) $navigation->add(get_string('pluginname', 'report_loglive'), $action, navigation_node::TYPE_SETTING, null, null, new pix_icon('i/report', '')); } } + +/** + * Callback to verify if the given instance of store is supported by this report or not. + * + * @param string $instance store instance. + * + * @return bool returns true if the store is supported by the report, false otherwise. + */ +function report_loglive_supports_logstore($instance) { + if ($instance instanceof \core\log\sql_select_reader) { + return true; + } + return false; +} diff --git a/report/loglive/tests/lib_test.php b/report/loglive/tests/lib_test.php new file mode 100644 index 00000000000..5461902407b --- /dev/null +++ b/report/loglive/tests/lib_test.php @@ -0,0 +1,57 @@ +. + +/** + * Tests for report library functions. + * + * @package report_loglive + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class report_loglive_lib_testcase + * + * @package report_loglive + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class report_loglive_lib_testcase extends advanced_testcase { + + /** + * Test report_log_supports_logstore. + */ + public function test_report_participation_supports_logstore() { + $logmanager = get_log_manager(); + $allstores = \core_component::get_plugin_list_with_class('logstore', 'log\store'); + + $supportedstores = array( + 'logstore_database' => '\logstore_legacy\log\database', + 'logstore_legacy' => '\logstore_legacy\log\store', + 'logstore_standard' => '\logstore_standard\log\store' + ); + + // Make sure all supported stores are installed. + $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); + $stores = $logmanager->get_supported_logstores('loglive'); + $stores = array_keys($stores); + foreach ($expectedstores as $expectedstore) { + $this->assertContains($expectedstore, $stores); + } + } +} diff --git a/report/outline/lib.php b/report/outline/lib.php index 11b27fe6ff7..16a300708a4 100644 --- a/report/outline/lib.php +++ b/report/outline/lib.php @@ -105,3 +105,17 @@ function report_outline_page_type_list($pagetype, $parentcontext, $currentcontex ); return $array; } + +/** + * Callback to verify if the given instance of store is supported by this report or not. + * + * @param string $instance store instance. + * + * @return bool returns true if the store is supported by the report, false otherwise. + */ +function report_outline_supports_logstore($instance) { + if ($instance instanceof \core\log\sql_internal_reader || $instance instanceof \logstore_legacy\log\store) { + return true; + } + return false; +} diff --git a/report/outline/tests/lib_test.php b/report/outline/tests/lib_test.php new file mode 100644 index 00000000000..95593fb984d --- /dev/null +++ b/report/outline/tests/lib_test.php @@ -0,0 +1,56 @@ +. + +/** + * Tests for report library functions. + * + * @package report_outline + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class report_outline_lib_testcase + * + * @package report_outline + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class report_outline_lib_testcase extends advanced_testcase { + + /** + * Test report_log_supports_logstore. + */ + public function test_report_participation_supports_logstore() { + $logmanager = get_log_manager(); + $allstores = \core_component::get_plugin_list_with_class('logstore', 'log\store'); + + $supportedstores = array( + 'logstore_legacy' => '\logstore_legacy\log\store', + 'logstore_standard' => '\logstore_standard\log\store' + ); + + // Make sure all supported stores are installed. + $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); + $stores = $logmanager->get_supported_logstores('outline'); + $stores = array_keys($stores); + foreach ($expectedstores as $expectedstore) { + $this->assertContains($expectedstore, $stores); + } + } +} diff --git a/report/participation/lib.php b/report/participation/lib.php index bf6be747f27..008395e449c 100644 --- a/report/participation/lib.php +++ b/report/participation/lib.php @@ -55,4 +55,18 @@ function report_participation_page_type_list($pagetype, $parentcontext, $current 'report-participation-index' => get_string('page-report-participation-index', 'report_participation'), ); return $array; +} + +/** + * Callback to verify if the given instance of store is supported by this report or not. + * + * @param string $instance store instance. + * + * @return bool returns true if the store is supported by the report, false otherwise. + */ +function report_participation_supports_logstore($instance) { + if ($instance instanceof \core\log\sql_internal_reader || $instance instanceof \logstore_legacy\log\store) { + return true; + } + return false; } \ No newline at end of file diff --git a/report/participation/tests/lib_test.php b/report/participation/tests/lib_test.php new file mode 100644 index 00000000000..3f2d05be46a --- /dev/null +++ b/report/participation/tests/lib_test.php @@ -0,0 +1,56 @@ +. + +/** + * Tests for report library functions. + * + * @package report_participation + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class report_participation_lib_testcase + * + * @package report_participation + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class report_participation_lib_testcase extends advanced_testcase { + + /** + * Test report_log_supports_logstore. + */ + public function test_report_participation_supports_logstore() { + $logmanager = get_log_manager(); + $allstores = \core_component::get_plugin_list_with_class('logstore', 'log\store'); + + $supportedstores = array( + 'logstore_legacy' => '\logstore_legacy\log\store', + 'logstore_standard' => '\logstore_standard\log\store' + ); + + // Make sure all supported stores are installed. + $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); + $stores = $logmanager->get_supported_logstores('participation'); + $stores = array_keys($stores); + foreach ($expectedstores as $expectedstore) { + $this->assertContains($expectedstore, $stores); + } + } +} diff --git a/report/stats/lib.php b/report/stats/lib.php index 1c191a453c1..e3232a5434c 100644 --- a/report/stats/lib.php +++ b/report/stats/lib.php @@ -112,4 +112,18 @@ function report_stats_page_type_list($pagetype, $parentcontext, $currentcontext) 'report-stats-user' => get_string('page-report-stats-user', 'report_stats') ); return $array; +} + +/** + * Callback to verify if the given instance of store is supported by this report or not. + * + * @param string $instance store instance. + * + * @return bool returns true if the store is supported by the report, false otherwise. + */ +function report_stats_supports_logstore($instance) { + if ($instance instanceof \core\log\sql_internal_reader || $instance instanceof \logstore_legacy\log\store) { + return true; + } + return false; } \ No newline at end of file diff --git a/report/stats/tests/lib_test.php b/report/stats/tests/lib_test.php new file mode 100644 index 00000000000..6d6630f17f7 --- /dev/null +++ b/report/stats/tests/lib_test.php @@ -0,0 +1,56 @@ +. + +/** + * Tests for report library functions. + * + * @package report_stats + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Class report_stats_lib_testcase + * + * @package report_stats + * @copyright 2014 onwards Ankit agarwal + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later. + */ +class report_stats_lib_testcase extends advanced_testcase { + + /** + * Test report_log_supports_logstore. + */ + public function test_report_participation_supports_logstore() { + $logmanager = get_log_manager(); + $allstores = \core_component::get_plugin_list_with_class('logstore', 'log\store'); + + $supportedstores = array( + 'logstore_legacy' => '\logstore_legacy\log\store', + 'logstore_standard' => '\logstore_standard\log\store' + ); + + // Make sure all supported stores are installed. + $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); + $stores = $logmanager->get_supported_logstores('stats'); + $stores = array_keys($stores); + foreach ($expectedstores as $expectedstore) { + $this->assertContains($expectedstore, $stores); + } + } +} From 96c8d59dea83701253c86dafd8713fdaceaa9a2b Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Thu, 20 Mar 2014 12:12:50 +0800 Subject: [PATCH 3/4] MDL-44596 logging: Add some tests to log stores to test the new support methods --- .../log/store/database/tests/store_test.php | 21 ++++++++++++++++ .../log/store/legacy/tests/store_test.php | 24 +++++++++++++++++++ .../log/store/standard/tests/store_test.php | 24 +++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/admin/tool/log/store/database/tests/store_test.php b/admin/tool/log/store/database/tests/store_test.php index e71e58e0f67..05633efb663 100644 --- a/admin/tool/log/store/database/tests/store_test.php +++ b/admin/tool/log/store/database/tests/store_test.php @@ -224,4 +224,25 @@ class logstore_database_store_testcase extends advanced_testcase { set_config('enabled_stores', '', 'tool_log'); get_log_manager(true); } + + /** + * Test logmanager::get_supported_reports returns all reports that require this store. + */ + public function test_get_supported_reports() { + $logmanager = get_log_manager(); + $allreports = \core_component::get_plugin_list('report'); + + $supportedreports = array( + 'log' => '/report/log', + 'loglive' => '/report/loglive' + ); + + // Make sure all supported reports are installed. + $expectedreports = array_keys(array_intersect_key($allreports, $supportedreports)); + $reports = $logmanager->get_supported_reports('logstore_database'); + $reports = array_keys($reports); + foreach ($expectedreports as $expectedreport) { + $this->assertContains($expectedreport, $reports); + } + } } diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index 89479dcf6b8..00629bfbeb9 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -249,4 +249,28 @@ class logstore_legacy_store_testcase extends advanced_testcase { \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params, $sort); $this->assertSame('ip DESC', $sort); } + + /* + * Test logmanager::get_supported_reports returns all reports that require this store. + */ + public function test_get_supported_reports() { + $logmanager = get_log_manager(); + $allreports = \core_component::get_plugin_list('report'); + + $supportedreports = array( + 'log' => '/report/log', + 'loglive' => '/report/loglive', + 'outline' => '/report/outline', + 'participation' => '/report/participation', + 'stats' => '/report/stats' + ); + + // Make sure all supported reports are installed. + $expectedreports = array_keys(array_intersect_key($allreports, $supportedreports)); + $reports = $logmanager->get_supported_reports('logstore_legacy'); + $reports = array_keys($reports); + foreach ($expectedreports as $expectedreport) { + $this->assertContains($expectedreport, $reports); + } + } } diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index 78a4c900f70..7244b687f75 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -195,4 +195,28 @@ class logstore_standard_store_testcase extends advanced_testcase { set_config('enabled_stores', '', 'tool_log'); get_log_manager(true); } + + /** + * Test logmanager::get_supported_reports returns all reports that require this store. + */ + public function test_get_supported_reports() { + $logmanager = get_log_manager(); + $allreports = \core_component::get_plugin_list('report'); + + $supportedreports = array( + 'log' => '/report/log', + 'loglive' => '/report/loglive', + 'outline' => '/report/outline', + 'participation' => '/report/participation', + 'stats' => '/report/stats' + ); + + // Make sure all supported reports are installed. + $expectedreports = array_keys(array_intersect_key($allreports, $supportedreports)); + $reports = $logmanager->get_supported_reports('logstore_standard'); + $reports = array_keys($reports); + foreach ($expectedreports as $expectedreport) { + $this->assertContains($expectedreport, $reports); + } + } } From 16e59c6af8f7d013d8db50999fb6a7690cd63132 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Tue, 25 Mar 2014 18:09:37 +0800 Subject: [PATCH 4/4] MDL-44596 logging: Refecator code to use component_callback() and get_plugin_list_with_function() --- admin/tool/log/classes/log/manager.php | 26 ++++--------------- .../log/store/database/tests/store_test.php | 4 +-- .../log/store/legacy/tests/store_test.php | 10 +++---- .../log/store/standard/tests/store_test.php | 10 +++---- report/log/tests/lib_test.php | 2 +- report/loglive/tests/lib_test.php | 2 +- report/outline/tests/lib_test.php | 2 +- report/participation/tests/lib_test.php | 2 +- report/stats/tests/lib_test.php | 2 +- report/upgrade.txt | 2 ++ 10 files changed, 24 insertions(+), 38 deletions(-) diff --git a/admin/tool/log/classes/log/manager.php b/admin/tool/log/classes/log/manager.php index b113bad287f..129389f2e1c 100644 --- a/admin/tool/log/classes/log/manager.php +++ b/admin/tool/log/classes/log/manager.php @@ -125,7 +125,7 @@ class manager implements \core\log\manager { return array(); } - $reports = \core_component::get_plugin_list('report'); + $reports = get_plugin_list_with_function('report', 'supports_logstore', 'lib.php'); $enabled = $this->stores; if (empty($enabled[$logstore])) { @@ -138,16 +138,8 @@ class manager implements \core\log\manager { $return = array(); foreach ($reports as $report => $fulldir) { - $file = $fulldir . '/lib.php'; - if (file_exists($file)) { - require_once($file); - $function = 'report_' . $report . '_supports_logstore'; - if (function_exists($function)) { - if ($function($instance)) { - $return[$report] = get_string('pluginname', 'report_' . $report); - } - } - + if (component_callback($report, 'supports_logstore', array($instance), false)) { + $return[$report] = get_string('pluginname', $report); } } @@ -167,16 +159,8 @@ class manager implements \core\log\manager { $allstores = self::get_store_plugins(); $enabled = $this->stores; - $function = $component . '_supports_logstore'; - $file = \core_component::get_component_directory($component) . '/lib.php'; - - if (!file_exists($file)) { - // The report doesn't define the callback, most probably it doesn't need log stores. - return false; - } - - require_once($file); - if (!function_exists($function)) { + $function = component_callback_exists($component, 'supports_logstore'); + if (!$function) { // The report doesn't define the callback, most probably it doesn't need log stores. return false; } diff --git a/admin/tool/log/store/database/tests/store_test.php b/admin/tool/log/store/database/tests/store_test.php index 05633efb663..71e65432581 100644 --- a/admin/tool/log/store/database/tests/store_test.php +++ b/admin/tool/log/store/database/tests/store_test.php @@ -233,8 +233,8 @@ class logstore_database_store_testcase extends advanced_testcase { $allreports = \core_component::get_plugin_list('report'); $supportedreports = array( - 'log' => '/report/log', - 'loglive' => '/report/loglive' + 'report_log' => '/report/log', + 'report_loglive' => '/report/loglive' ); // Make sure all supported reports are installed. diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index 00629bfbeb9..c78e013d1b8 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -258,11 +258,11 @@ class logstore_legacy_store_testcase extends advanced_testcase { $allreports = \core_component::get_plugin_list('report'); $supportedreports = array( - 'log' => '/report/log', - 'loglive' => '/report/loglive', - 'outline' => '/report/outline', - 'participation' => '/report/participation', - 'stats' => '/report/stats' + 'report_log' => '/report/log', + 'report_loglive' => '/report/loglive', + 'report_outline' => '/report/outline', + 'report_participation' => '/report/participation', + 'report_stats' => '/report/stats' ); // Make sure all supported reports are installed. diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index 7244b687f75..2d42b4cf202 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -204,11 +204,11 @@ class logstore_standard_store_testcase extends advanced_testcase { $allreports = \core_component::get_plugin_list('report'); $supportedreports = array( - 'log' => '/report/log', - 'loglive' => '/report/loglive', - 'outline' => '/report/outline', - 'participation' => '/report/participation', - 'stats' => '/report/stats' + 'report_log' => '/report/log', + 'report_loglive' => '/report/loglive', + 'report_outline' => '/report/outline', + 'report_participation' => '/report/participation', + 'report_stats' => '/report/stats' ); // Make sure all supported reports are installed. diff --git a/report/log/tests/lib_test.php b/report/log/tests/lib_test.php index 0b605f61deb..4c0996de99b 100644 --- a/report/log/tests/lib_test.php +++ b/report/log/tests/lib_test.php @@ -48,7 +48,7 @@ class report_log_lib_testcase extends advanced_testcase { // Make sure all supported stores are installed. $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); - $stores = $logmanager->get_supported_logstores('log'); + $stores = $logmanager->get_supported_logstores('report_log'); $stores = array_keys($stores); foreach ($expectedstores as $expectedstore) { $this->assertContains($expectedstore, $stores); diff --git a/report/loglive/tests/lib_test.php b/report/loglive/tests/lib_test.php index 5461902407b..ddc20f5139d 100644 --- a/report/loglive/tests/lib_test.php +++ b/report/loglive/tests/lib_test.php @@ -48,7 +48,7 @@ class report_loglive_lib_testcase extends advanced_testcase { // Make sure all supported stores are installed. $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); - $stores = $logmanager->get_supported_logstores('loglive'); + $stores = $logmanager->get_supported_logstores('report_loglive'); $stores = array_keys($stores); foreach ($expectedstores as $expectedstore) { $this->assertContains($expectedstore, $stores); diff --git a/report/outline/tests/lib_test.php b/report/outline/tests/lib_test.php index 95593fb984d..9f1f1438957 100644 --- a/report/outline/tests/lib_test.php +++ b/report/outline/tests/lib_test.php @@ -47,7 +47,7 @@ class report_outline_lib_testcase extends advanced_testcase { // Make sure all supported stores are installed. $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); - $stores = $logmanager->get_supported_logstores('outline'); + $stores = $logmanager->get_supported_logstores('report_outline'); $stores = array_keys($stores); foreach ($expectedstores as $expectedstore) { $this->assertContains($expectedstore, $stores); diff --git a/report/participation/tests/lib_test.php b/report/participation/tests/lib_test.php index 3f2d05be46a..89b637a3b1b 100644 --- a/report/participation/tests/lib_test.php +++ b/report/participation/tests/lib_test.php @@ -47,7 +47,7 @@ class report_participation_lib_testcase extends advanced_testcase { // Make sure all supported stores are installed. $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); - $stores = $logmanager->get_supported_logstores('participation'); + $stores = $logmanager->get_supported_logstores('report_participation'); $stores = array_keys($stores); foreach ($expectedstores as $expectedstore) { $this->assertContains($expectedstore, $stores); diff --git a/report/stats/tests/lib_test.php b/report/stats/tests/lib_test.php index 6d6630f17f7..a7eb5008609 100644 --- a/report/stats/tests/lib_test.php +++ b/report/stats/tests/lib_test.php @@ -47,7 +47,7 @@ class report_stats_lib_testcase extends advanced_testcase { // Make sure all supported stores are installed. $expectedstores = array_keys(array_intersect($allstores, $supportedstores)); - $stores = $logmanager->get_supported_logstores('stats'); + $stores = $logmanager->get_supported_logstores('report_stats'); $stores = array_keys($stores); foreach ($expectedstores as $expectedstore) { $this->assertContains($expectedstore, $stores); diff --git a/report/upgrade.txt b/report/upgrade.txt index eb434830fa8..e6f06a74bf1 100644 --- a/report/upgrade.txt +++ b/report/upgrade.txt @@ -4,6 +4,8 @@ information provided here is intended especially for developers. === 2.7 === * How to migrate reports accessing table 'log': http://docs.moodle.org/dev/Migrating_log_access_in_reports +* All reports that use logstores must implement a callback report_reportname_supports_logstore($storeinstance) in lib.php of the + report. Refer MDL-44596 for details. === 2.2 ===