From 55af4309f7a982ac0abf023c52c414298b6b37b1 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Tue, 25 Oct 2016 13:39:14 +0530 Subject: [PATCH 1/2] MDL-56409 messages: Do not fetch notifications if they are disabled --- message/output/popup/classes/api.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/message/output/popup/classes/api.php b/message/output/popup/classes/api.php index cc8e85e5316..66908154612 100644 --- a/message/output/popup/classes/api.php +++ b/message/output/popup/classes/api.php @@ -34,7 +34,7 @@ defined('MOODLE_INTERNAL') || die(); */ class api { /** - * Get popup notifications for the specified users. + * Get popup notifications for the specified users. Nothing is returned if notifications are disabled. * * @param int $useridto the user id who received the notification * @param string $sort the column name to order by including optionally direction @@ -61,6 +61,18 @@ class api { 'useridto2' => $useridto, ]; + // Is notification enabled ? + if ($useridto == $USER->id) { + $disabled = $USER->emailstop; + } else { + $user = \core_user::get_user($useridto, "emailstop", MUST_EXIST); + $disabled = $user->emailstop; + } + if ($disabled) { + // Notifications are disabled, no need to run giant queries. + return array(); + } + $sql = "SELECT * FROM ( SELECT concat('r', r.id) as uniqueid, r.id, r.useridfrom, r.useridto, r.subject, r.fullmessage, r.fullmessageformat, From 20ab51fdbc8e4a4a37439dda1c8b1b010a8801ed Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Thu, 27 Oct 2016 11:07:47 +0530 Subject: [PATCH 2/2] MDL-56409 messages: Save one db query per page load --- admin/message.php | 4 +- lang/en/cache.php | 1 + lib/db/caches.php | 9 +++ message/classes/api.php | 107 +++++++++++++++++++++++++++++++++++ message/lib.php | 23 +------- message/output/popup/lib.php | 4 +- message/tests/api_test.php | 86 ++++++++++++++++++++++++++++ version.php | 2 +- 8 files changed, 209 insertions(+), 27 deletions(-) diff --git a/admin/message.php b/admin/message.php index 52cb5608bbc..6924e61db30 100644 --- a/admin/message.php +++ b/admin/message.php @@ -41,7 +41,7 @@ if (!empty($disable) && confirm_sesskey()) { if (!$processor = $DB->get_record('message_processors', array('id'=>$disable))) { print_error('outputdoesnotexist', 'message'); } - $DB->set_field('message_processors', 'enabled', '0', array('id'=>$processor->id)); // Disable output + \core_message\api::update_processor_status($processor, 0); // Disable output. core_plugin_manager::reset_caches(); } @@ -49,7 +49,7 @@ if (!empty($enable) && confirm_sesskey()) { if (!$processor = $DB->get_record('message_processors', array('id'=>$enable))) { print_error('outputdoesnotexist', 'message'); } - $DB->set_field('message_processors', 'enabled', '1', array('id'=>$processor->id)); // Enable output + \core_message\api::update_processor_status($processor, 1); // Enable output. core_plugin_manager::reset_caches(); } diff --git a/lang/en/cache.php b/lang/en/cache.php index bf280d21a97..99e33102e7d 100644 --- a/lang/en/cache.php +++ b/lang/en/cache.php @@ -53,6 +53,7 @@ $string['cachedef_groupdata'] = 'Course group information'; $string['cachedef_htmlpurifier'] = 'HTML Purifier - cleaned content'; $string['cachedef_langmenu'] = 'List of available languages'; $string['cachedef_locking'] = 'Locking'; +$string['cachedef_message_processors_enabled'] = "Message processors enabled status"; $string['cachedef_navigation_expandcourse'] = 'Navigation expandable courses'; $string['cachedef_observers'] = 'Event observers'; $string['cachedef_plugin_functions'] = 'Plugins available callbacks'; diff --git a/lib/db/caches.php b/lib/db/caches.php index da6b531c747..b709f06f061 100644 --- a/lib/db/caches.php +++ b/lib/db/caches.php @@ -292,4 +292,13 @@ $definitions = array( 'resettagindexbuilder', ), ), + + // Caches message processors. + 'message_processors_enabled' => array( + 'mode' => cache_store::MODE_APPLICATION, + 'simplekeys' => true, + 'simpledata' => true, + 'staticacceleration' => true, + 'staticaccelerationsize' => 3 + ), ); diff --git a/message/classes/api.php b/message/classes/api.php index 4398fba2372..eabe7d7379b 100644 --- a/message/classes/api.php +++ b/message/classes/api.php @@ -673,4 +673,111 @@ class api { return false; } + + /** + * Get specified message processor, validate corresponding plugin existence and + * system configuration. + * + * @param string $name Name of the processor. + * @param bool $ready only return ready-to-use processors. + * @return mixed $processor if processor present else empty array. + * @since Moodle 3.2 + */ + public static function get_message_processor($name, $ready = false) { + global $DB, $CFG; + + $processor = $DB->get_record('message_processors', array('name' => $name)); + if (empty($processor)) { + // Processor not found, return. + return array(); + } + + $processor = self::get_processed_processor_object($processor); + if ($ready) { + if ($processor->enabled && $processor->configured) { + return $processor; + } else { + return array(); + } + } else { + return $processor; + } + } + + /** + * Returns weather a given processor is enabled or not. + * Note:- This doesn't check if the processor is configured or not. + * + * @param string $name Name of the processor + * @return bool + */ + public static function is_processor_enabled($name) { + + $cache = \cache::make('core', 'message_processors_enabled'); + $status = $cache->get($name); + + if ($status === false) { + $processor = self::get_message_processor($name); + if (!empty($processor)) { + $cache->set($name, $processor->enabled); + return $processor->enabled; + } else { + return false; + } + } + + return $status; + } + + /** + * Set status of a processor. + * + * @param \stdClass $processor processor record. + * @param 0|1 $enabled 0 or 1 to set the processor status. + * @return bool + * @since Moodle 3.2 + */ + public static function update_processor_status($processor, $enabled) { + global $DB; + $cache = \cache::make('core', 'message_processors_enabled'); + $cache->delete($processor->name); + return $DB->set_field('message_processors', 'enabled', $enabled, array('id' => $processor->id)); + } + + /** + * Given a processor object, loads information about it's settings and configurations. + * This is not a public api, instead use @see \core_message\api::get_message_processor() + * or @see \get_message_processors() + * + * @param \stdClass $processor processor object + * @return \stdClass processed processor object + * @since Moodle 3.2 + */ + public static function get_processed_processor_object(\stdClass $processor) { + global $CFG; + + $processorfile = $CFG->dirroot. '/message/output/'.$processor->name.'/message_output_'.$processor->name.'.php'; + if (is_readable($processorfile)) { + include_once($processorfile); + $processclass = 'message_output_' . $processor->name; + if (class_exists($processclass)) { + $pclass = new $processclass(); + $processor->object = $pclass; + $processor->configured = 0; + if ($pclass->is_system_configured()) { + $processor->configured = 1; + } + $processor->hassettings = 0; + if (is_readable($CFG->dirroot.'/message/output/'.$processor->name.'/settings.php')) { + $processor->hassettings = 1; + } + $processor->available = 1; + } else { + print_error('errorcallingprocessor', 'message'); + } + } else { + $processor->available = 0; + } + return $processor; + } } diff --git a/message/lib.php b/message/lib.php index 641a14e106b..34431beabbe 100644 --- a/message/lib.php +++ b/message/lib.php @@ -960,28 +960,7 @@ function get_message_processors($ready = false, $reset = false, $resetonly = fal // Get all processors, ensure the name column is the first so it will be the array key $processors = $DB->get_records('message_processors', null, 'name DESC', 'name, id, enabled'); foreach ($processors as &$processor){ - $processorfile = $CFG->dirroot. '/message/output/'.$processor->name.'/message_output_'.$processor->name.'.php'; - if (is_readable($processorfile)) { - include_once($processorfile); - $processclass = 'message_output_' . $processor->name; - if (class_exists($processclass)) { - $pclass = new $processclass(); - $processor->object = $pclass; - $processor->configured = 0; - if ($pclass->is_system_configured()) { - $processor->configured = 1; - } - $processor->hassettings = 0; - if (is_readable($CFG->dirroot.'/message/output/'.$processor->name.'/settings.php')) { - $processor->hassettings = 1; - } - $processor->available = 1; - } else { - print_error('errorcallingprocessor', 'message'); - } - } else { - $processor->available = 0; - } + $processor = \core_message\api::get_processed_processor_object($processor); } } if ($ready) { diff --git a/message/output/popup/lib.php b/message/output/popup/lib.php index 27d889ee14f..88758555131 100644 --- a/message/output/popup/lib.php +++ b/message/output/popup/lib.php @@ -53,8 +53,8 @@ function message_popup_render_navbar_output(\renderer_base $renderer) { } // Add the notifications popover. - $processor = $DB->get_record('message_processors', array('name' => 'popup')); - if ($processor && $processor->enabled) { + $enabled = \core_message\api::is_processor_enabled("popup"); + if ($enabled) { $context = [ 'userid' => $USER->id, 'urls' => [ diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 48c7826e1f2..67d898ed569 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -867,4 +867,90 @@ class core_message_api_testcase extends core_message_messagelib_testcase { // As the admin you should still be able to send messages to the user. $this->assertFalse(\core_message\api::is_user_blocked($user1)); } + + /* + * Tes get_message_processor api. + */ + public function test_get_message_processor() { + $processors = get_message_processors(); + if (empty($processors)) { + $this->markTestSkipped("No message processors found"); + } + + list($name, $processor) = each($processors); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals($processor->name, $testprocessor->name); + $this->assertEquals($processor->enabled, $testprocessor->enabled); + $this->assertEquals($processor->available, $testprocessor->available); + $this->assertEquals($processor->configured, $testprocessor->configured); + + // Disable processor and test. + \core_message\api::update_processor_status($testprocessor, 0); + $testprocessor = \core_message\api::get_message_processor($name, true); + $this->assertEmpty($testprocessor); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals($processor->name, $testprocessor->name); + $this->assertEquals(0, $testprocessor->enabled); + + // Enable again and test. + \core_message\api::update_processor_status($testprocessor, 1); + $testprocessor = \core_message\api::get_message_processor($name, true); + $this->assertEquals($processor->name, $testprocessor->name); + $this->assertEquals(1, $testprocessor->enabled); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals($processor->name, $testprocessor->name); + $this->assertEquals(1, $testprocessor->enabled); + } + + /** + * Test method update_processor_status. + */ + public function test_update_processor_status() { + $processors = get_message_processors(); + if (empty($processors)) { + $this->markTestSkipped("No message processors found"); + } + list($name, $testprocessor) = each($processors); + + // Enable. + \core_message\api::update_processor_status($testprocessor, 1); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals(1, $testprocessor->enabled); + + // Disable. + \core_message\api::update_processor_status($testprocessor, 0); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals(0, $testprocessor->enabled); + + // Enable again. + \core_message\api::update_processor_status($testprocessor, 1); + $testprocessor = \core_message\api::get_message_processor($name); + $this->assertEquals(1, $testprocessor->enabled); + } + + /** + * Test method is_user_enabled. + */ + public function is_user_enabled() { + $processors = get_message_processors(); + if (empty($processors)) { + $this->markTestSkipped("No message processors found"); + } + list($name, $testprocessor) = each($processors); + + // Enable. + \core_message\api::update_processor_status($testprocessor, 1); + $status = \core_message\api::is_processor_enabled($name); + $this->assertEquals(1, $status); + + // Disable. + \core_message\api::update_processor_status($testprocessor, 0); + $status = \core_message\api::is_processor_enabled($name); + $this->assertEquals(0, $status); + + // Enable again. + \core_message\api::update_processor_status($testprocessor, 1); + $status = \core_message\api::is_processor_enabled($name); + $this->assertEquals(1, $status); + } } diff --git a/version.php b/version.php index 84c1ef847e5..948a7d638d9 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2016110400.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2016110400.05; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.