From 727c21733e7c27c308df0d144403961db6c787c6 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Fri, 30 May 2014 11:27:10 +1200 Subject: [PATCH 1/2] MDL-45724 cache: admin screen now shows store warnings Implemented a means by which cache store instance can communicate warnings due to things such as conflicting configurations etc. --- cache/admin.php | 25 ++++++++++++------- cache/classes/store.php | 12 +++++++++ cache/locallib.php | 3 ++- cache/renderer.php | 20 +++++++++++++++ .../memcache/lang/en/cachestore_memcache.php | 1 + cache/stores/memcache/lib.php | 24 ++++++++++++++++++ .../lang/en/cachestore_memcached.php | 1 + cache/stores/memcached/lib.php | 25 +++++++++++++++++++ 8 files changed, 101 insertions(+), 10 deletions(-) diff --git a/cache/admin.php b/cache/admin.php index c9c35db6adb..ba49777a54c 100644 --- a/cache/admin.php +++ b/cache/admin.php @@ -52,7 +52,7 @@ $locks = cache_administration_helper::get_lock_summaries(); $title = new lang_string('cacheadmin', 'cache'); $mform = null; -$notification = null; +$notifications = array(); $notifysuccess = true; if (!empty($action) && confirm_sesskey()) { @@ -110,10 +110,10 @@ if (!empty($action) && confirm_sesskey()) { if (!array_key_exists($store, $stores)) { $notifysuccess = false; - $notification = get_string('invalidstore', 'cache'); + $notifications[] = array(get_string('invalidstore', 'cache'), false); } else if ($stores[$store]['mappings'] > 0) { $notifysuccess = false; - $notification = get_string('deletestorehasmappings', 'cache'); + $notifications[] = array(get_string('deletestorehasmappings', 'cache'), false); } if ($notifysuccess) { @@ -250,10 +250,10 @@ if (!empty($action) && confirm_sesskey()) { $confirm = optional_param('confirm', false, PARAM_BOOL); if (!array_key_exists($lock, $locks)) { $notifysuccess = false; - $notification = get_string('invalidlock', 'cache'); + $notifications[] = array(get_string('invalidlock', 'cache'), false); } else if ($locks[$lock]['uses'] > 0) { $notifysuccess = false; - $notification = get_string('deletelockhasuses', 'cache'); + $notifications[] = array(get_string('deletelockhasuses', 'cache'), false); } if ($notifysuccess) { if (!$confirm) { @@ -280,6 +280,16 @@ if (!empty($action) && confirm_sesskey()) { } } +// Stores can add notices to the cache configuration screen for things like conflicting configurations etc. +// Here we check each cache to see if it has warnings. +foreach ($stores as $store) { + if (!empty($store['warnings']) && is_array($store['warnings'])) { + foreach ($store['warnings'] as $warning) { + $notifications[] = array($warning, false); + } + } +} + $PAGE->set_title($title); $PAGE->set_heading($SITE->fullname); /* @var core_cache_renderer $renderer */ @@ -287,10 +297,7 @@ $renderer = $PAGE->get_renderer('core_cache'); echo $renderer->header(); echo $renderer->heading($title); - -if (!is_null($notification)) { - echo $renderer->notification($notification, ($notifysuccess)?'notifysuccess' : 'notifyproblem'); -} +echo $renderer->notififications($notifications); if ($mform instanceof moodleform) { $mform->display(); diff --git a/cache/classes/store.php b/cache/classes/store.php index b5cfcc11d6c..1b3c5bbde6d 100644 --- a/cache/classes/store.php +++ b/cache/classes/store.php @@ -365,4 +365,16 @@ abstract class cache_store implements cache_store_interface { public static function initialise_unit_test_instance(cache_definition $definition) { return static::initialise_test_instance($definition); } + + /** + * Can be overridden to return any warnings this store instance should make to the admin. + * + * This should be used to notify things like configuration conflicts etc. + * The warnings returned here will be displayed on the cache configuration screen. + * + * @return string[] Returns an array of warnings (strings) + */ + public function get_warnings() { + return array(); + } } diff --git a/cache/locallib.php b/cache/locallib.php index 042b89127b5..58dcdd897a6 100644 --- a/cache/locallib.php +++ b/cache/locallib.php @@ -709,7 +709,8 @@ abstract class cache_administration_helper extends cache_helper { 'nativelocking' => ($store instanceof cache_is_lockable), 'keyawareness' => ($store instanceof cache_is_key_aware), 'searchable' => ($store instanceof cache_is_searchable) - ) + ), + 'warnings' => $store->get_warnings() ); if (empty($details['default'])) { $return[$name] = $record; diff --git a/cache/renderer.php b/cache/renderer.php index 685b8389953..dbf7e88a80e 100644 --- a/cache/renderer.php +++ b/cache/renderer.php @@ -372,4 +372,24 @@ class core_cache_renderer extends plugin_renderer_base { $html .= html_writer::end_tag('div'); return $html; } + + /** + * Renders an array of notifications for the cache configuration screen. + * + * @param array $notifications + * @return string + */ + public function notififications(array $notifications = array()) { + if (count($notifications) === 0) { + // There are no notifications to render. + return ''; + } + $html = html_writer::start_div('notifications'); + foreach ($notifications as $notification) { + list($message, $notifysuccess) = $notification; + $html .= $this->notification($message, ($notifysuccess) ? 'notifysuccess' : 'notifyproblem'); + } + $html .= html_writer::end_div(); + return $html; + } } \ No newline at end of file diff --git a/cache/stores/memcache/lang/en/cachestore_memcache.php b/cache/stores/memcache/lang/en/cachestore_memcache.php index 78d39471c0d..26f1c9ed974 100644 --- a/cache/stores/memcache/lang/en/cachestore_memcache.php +++ b/cache/stores/memcache/lang/en/cachestore_memcache.php @@ -64,6 +64,7 @@ For example: server.url.com ipaddress:port '; +$string['sessionhandlerconflict'] = 'Warning: A memcache instance ({$a}) has being configured to use the same memcached server as sessions. Purging all caches will lead to sessions also being purged.'; $string['testservers'] = 'Test servers'; $string['testservers_desc'] = 'The test servers get used for unit tests and for performance tests. It is entirely optional to set up test servers. Servers should be defined one per line and consist of a server address and optionally a port and weight. If no port is provided then the default port (11211) is used.'; \ No newline at end of file diff --git a/cache/stores/memcache/lib.php b/cache/stores/memcache/lib.php index 3a7da5f84a9..036da565668 100644 --- a/cache/stores/memcache/lib.php +++ b/cache/stores/memcache/lib.php @@ -573,4 +573,28 @@ class cachestore_memcache extends cache_store implements cache_is_configurable { public function my_name() { return $this->name; } + + /** + * Used to notify of configuration conflicts. + * + * The warnings returned here will be displayed on the cache configuration screen. + * + * @return string[] Returns an array of warnings (strings) + */ + public function get_warnings() { + global $CFG; + $warnings = array(); + if (isset($CFG->session_memcached_save_path) && count($this->servers)) { + $bits = explode(':', $CFG->session_memcached_save_path, 3); + $host = array_shift($bits); + $port = (count($bits)) ? array_shift($bits) : '11211'; + foreach ($this->servers as $server) { + if ($server[0] === $host && $server[1] === $port) { + $warnings[] = get_string('sessionhandlerconflict', 'cachestore_memcache', $this->my_name()); + break; + } + } + } + return $warnings; + } } diff --git a/cache/stores/memcached/lang/en/cachestore_memcached.php b/cache/stores/memcached/lang/en/cachestore_memcached.php index 5d35c1cd7a2..97fe5b6d7bc 100644 --- a/cache/stores/memcached/lang/en/cachestore_memcached.php +++ b/cache/stores/memcached/lang/en/cachestore_memcached.php @@ -78,6 +78,7 @@ For example: server.url.com ipaddress:port '; +$string['sessionhandlerconflict'] = 'Warning: A memcached instance ({$a}) has being configured to use the same memcached server as sessions. Purging all caches will lead to sessions also being purged.'; $string['testservers'] = 'Test servers'; $string['testservers_desc'] = 'The test servers get used for unit tests and for performance tests. It is entirely optional to set up test servers. Servers should be defined one per line and consist of a server address and optionally a port and weight. If no port is provided then the default port (11211) is used.'; diff --git a/cache/stores/memcached/lib.php b/cache/stores/memcached/lib.php index 284f3c86796..b379b263e45 100644 --- a/cache/stores/memcached/lib.php +++ b/cache/stores/memcached/lib.php @@ -668,4 +668,29 @@ class cachestore_memcached extends cache_store implements cache_is_configurable public function my_name() { return $this->name; } + + /** + * Used to notify of configuration conflicts. + * + * The warnings returned here will be displayed on the cache configuration screen. + * + * @return string[] Returns an array of warnings (strings) + */ + public function get_warnings() { + global $CFG; + $warnings = array(); + if (isset($CFG->session_memcached_save_path) && count($this->servers)) { + $bits = explode(':', $CFG->session_memcached_save_path, 3); + $host = array_shift($bits); + $port = (count($bits)) ? array_shift($bits) : '11211'; + + foreach ($this->servers as $server) { + if ((string)$server[0] === $host && (string)$server[1] === $port) { + $warnings[] = get_string('sessionhandlerconflict', 'cachestore_memcached', $this->my_name()); + break; + } + } + } + return $warnings; + } } From 915140c9b5b5f803234a78405c16e61e6da8b275 Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Fri, 30 May 2014 11:41:48 +1200 Subject: [PATCH 2/2] MDL-45724 cache: added cache API warnings to the admin notifications page --- admin/index.php | 10 +++++++--- admin/renderer.php | 17 ++++++++++++++++- cache/admin.php | 12 ++---------- cache/classes/helper.php | 24 ++++++++++++++++++++++++ cache/classes/store.php | 6 +++++- cache/renderer.php | 8 +++++++- 6 files changed, 61 insertions(+), 16 deletions(-) diff --git a/admin/index.php b/admin/index.php index aaa530a2fe3..34c0f95c570 100644 --- a/admin/index.php +++ b/admin/index.php @@ -589,10 +589,14 @@ $availableupdatesfetch = $updateschecker->get_last_timefetched(); $buggyiconvnomb = (!function_exists('mb_convert_encoding') and @iconv('UTF-8', 'UTF-8//IGNORE', '100'.chr(130).'€') !== '100€'); //check if the site is registered on Moodle.org $registered = $DB->count_records('registration_hubs', array('huburl' => HUB_MOODLEORGHUBURL, 'confirmed' => 1)); +// Check if there are any cache warnings. +$cachewarnings = cache_helper::warnings(); admin_externalpage_setup('adminnotifications'); +/* @var core_admin_renderer $output */ $output = $PAGE->get_renderer('core', 'admin'); -echo $output->admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, - $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, $buggyiconvnomb, - $registered); + +echo $output->admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, $cronoverdue, $dbproblems, + $maintenancemode, $availableupdates, $availableupdatesfetch, $buggyiconvnomb, + $registered, $cachewarnings); diff --git a/admin/renderer.php b/admin/renderer.php index 954b977171d..99109a21e4f 100644 --- a/admin/renderer.php +++ b/admin/renderer.php @@ -303,12 +303,13 @@ class core_admin_renderer extends plugin_renderer_base { * @param bool $buggyiconvnomb warn iconv problems * @param array|null $availableupdates array of \core\update\info objects or null * @param int|null $availableupdatesfetch timestamp of the most recent updates fetch or null (unknown) + * @param string[] $cachewarnings An array containing warnings from the Cache API. * * @return string HTML to output. */ public function admin_notifications_page($maturity, $insecuredataroot, $errorsdisplayed, $cronoverdue, $dbproblems, $maintenancemode, $availableupdates, $availableupdatesfetch, - $buggyiconvnomb, $registered) { + $buggyiconvnomb, $registered, array $cachewarnings = array()) { global $CFG; $output = ''; @@ -321,6 +322,7 @@ class core_admin_renderer extends plugin_renderer_base { $output .= $this->cron_overdue_warning($cronoverdue); $output .= $this->db_problems($dbproblems); $output .= $this->maintenance_mode_warning($maintenancemode); + $output .= $this->cache_warnings($cachewarnings); $output .= $this->registration_warning($registered); ////////////////////////////////////////////////////////////////////////////////////////////////// @@ -595,6 +597,19 @@ class core_admin_renderer extends plugin_renderer_base { return $this->warning($dbproblems); } + /** + * Renders cache warnings if there are any. + * + * @param string[] $cachewarnings + * @return string + */ + public function cache_warnings(array $cachewarnings) { + if (!count($cachewarnings)) { + return ''; + } + return join("\n", array_map(array($this, 'warning'), $cachewarnings)); + } + /** * Render an appropriate message if the site in in maintenance mode. * @param bool $maintenancemode diff --git a/cache/admin.php b/cache/admin.php index ba49777a54c..827b13719e3 100644 --- a/cache/admin.php +++ b/cache/admin.php @@ -280,15 +280,7 @@ if (!empty($action) && confirm_sesskey()) { } } -// Stores can add notices to the cache configuration screen for things like conflicting configurations etc. -// Here we check each cache to see if it has warnings. -foreach ($stores as $store) { - if (!empty($store['warnings']) && is_array($store['warnings'])) { - foreach ($store['warnings'] as $warning) { - $notifications[] = array($warning, false); - } - } -} +$notifications = array_merge($notifications, cache_helper::warnings($stores)); $PAGE->set_title($title); $PAGE->set_heading($SITE->fullname); @@ -297,7 +289,7 @@ $renderer = $PAGE->get_renderer('core_cache'); echo $renderer->header(); echo $renderer->heading($title); -echo $renderer->notififications($notifications); +echo $renderer->notifications($notifications); if ($mform instanceof moodleform) { $mform->display(); diff --git a/cache/classes/helper.php b/cache/classes/helper.php index 02bd243afcb..dd150b7c403 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -736,4 +736,28 @@ class cache_helper { } return $stores; } + + /** + * Returns an array of warnings from the cache API. + * + * The warning returned here are for things like conflicting store instance configurations etc. + * These get shown on the admin notifications page for example. + * + * @param array|null $stores An array of stores to get warnings for, or null for all. + * @return string[] + */ + public static function warnings(array $stores = null) { + global $CFG; + if ($stores === null) { + require_once($CFG->dirroot.'/cache/locallib.php'); + $stores = cache_administration_helper::get_store_instance_summaries(); + } + $warnings = array(); + foreach ($stores as $store) { + if (!empty($store['warnings'])) { + $warnings = array_merge($warnings, $store['warnings']); + } + } + return $warnings; + } } diff --git a/cache/classes/store.php b/cache/classes/store.php index 1b3c5bbde6d..464df843d3b 100644 --- a/cache/classes/store.php +++ b/cache/classes/store.php @@ -372,7 +372,11 @@ abstract class cache_store implements cache_store_interface { * This should be used to notify things like configuration conflicts etc. * The warnings returned here will be displayed on the cache configuration screen. * - * @return string[] Returns an array of warnings (strings) + * @return array[] Returns an array of arrays with the format: + * $notifications = array( + * array('This is a success message', true), + * array('This is a failure message', false), + * ); */ public function get_warnings() { return array(); diff --git a/cache/renderer.php b/cache/renderer.php index dbf7e88a80e..2b18d6b3fe5 100644 --- a/cache/renderer.php +++ b/cache/renderer.php @@ -376,10 +376,16 @@ class core_cache_renderer extends plugin_renderer_base { /** * Renders an array of notifications for the cache configuration screen. * + * Takes an array of notifications with the form: + * $notifications = array( + * array('This is a success message', true), + * array('This is a failure message', false), + * ); + * * @param array $notifications * @return string */ - public function notififications(array $notifications = array()) { + public function notifications(array $notifications = array()) { if (count($notifications) === 0) { // There are no notifications to render. return '';