From 1da1152baa804ef8b26e581e24f89299aeafea04 Mon Sep 17 00:00:00 2001 From: Nathan Mares Date: Thu, 7 Oct 2021 23:14:48 +0000 Subject: [PATCH] MDL-72723 logs: Add get_events_select_exists to the sql_reader interface - Updates to log stores and backup helper to improve performance when checking if a course has been modified. - This is a breaking change as it adds a new function on the sql_reader interface. Co-authored-by: Kevin Pham --- .../log/store/database/classes/log/store.php | 20 ++++++++++++++++ .../log/store/legacy/classes/log/store.php | 23 +++++++++++++++++++ .../log/store/standard/classes/log/store.php | 21 +++++++++++++++++ .../log/store/standard/tests/store_test.php | 4 ++++ .../util/helper/backup_cron_helper.class.php | 2 +- lib/classes/log/sql_reader.php | 10 ++++++++ lib/upgrade.txt | 6 +++++ 7 files changed, 85 insertions(+), 1 deletion(-) diff --git a/admin/tool/log/store/database/classes/log/store.php b/admin/tool/log/store/database/classes/log/store.php index 5c85e3d91d6..710ee4280a1 100644 --- a/admin/tool/log/store/database/classes/log/store.php +++ b/admin/tool/log/store/database/classes/log/store.php @@ -262,6 +262,26 @@ class store implements \tool_log\log\writer, \core\log\sql_reader { return $this->extdb->count_records_select($dbtable, $selectwhere, $params); } + /** + * Get whether events are present for the given select clause. + * + * @param string $selectwhere select conditions. + * @param array $params params. + * + * @return bool Whether events available for the given conditions + */ + public function get_events_select_exists(string $selectwhere, array $params): bool { + if (!$this->init()) { + return false; + } + + if (!$dbtable = $this->get_config('dbtable')) { + return false; + } + + return $this->extdb->record_exists_select($dbtable, $selectwhere, $params); + } + /** * Get a config value for the store. * diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php index aa4a6855226..9bd89e2424d 100644 --- a/admin/tool/log/store/legacy/classes/log/store.php +++ b/admin/tool/log/store/legacy/classes/log/store.php @@ -131,6 +131,29 @@ class store implements \tool_log\log\store, \core\log\sql_reader { return $events; } + /** + * Get whether events are present for the given select clause. + * @deprecated since Moodle 3.6 MDL-52953 - Please use supported log stores such as "standard" or "external" instead. + * + * @param string $selectwhere select conditions. + * @param array $params params. + * + * @return bool Whether events available for the given conditions + */ + public function get_events_select_exists(string $selectwhere, array $params): bool { + global $DB; + + // Replace the query with hardcoded mappings required for core. + list($selectwhere, $params) = self::replace_sql_legacy($selectwhere, $params); + + try { + return $DB->record_exists_select('log', $selectwhere, $params); + } catch (\moodle_exception $ex) { + debugging("error converting legacy event data " . $ex->getMessage() . $ex->debuginfo, DEBUG_DEVELOPER); + return false; + } + } + /** * Fetch records using given criteria returning a Traversable object. * @deprecated since Moodle 3.6 MDL-52953 - Please use supported log stores such as "standard" or "external" instead. diff --git a/admin/tool/log/store/standard/classes/log/store.php b/admin/tool/log/store/standard/classes/log/store.php index c296546d44a..b1d0602855b 100644 --- a/admin/tool/log/store/standard/classes/log/store.php +++ b/admin/tool/log/store/standard/classes/log/store.php @@ -139,11 +139,32 @@ class store implements \tool_log\log\writer, \core\log\sql_internal_table_reader return $event; } + /** + * Get number of events present for the given select clause. + * + * @param string $selectwhere select conditions. + * @param array $params params. + * + * @return int Number of events available for the given conditions + */ public function get_events_select_count($selectwhere, array $params) { global $DB; return $DB->count_records_select('logstore_standard_log', $selectwhere, $params); } + /** + * Get whether events are present for the given select clause. + * + * @param string $selectwhere select conditions. + * @param array $params params. + * + * @return bool Whether events available for the given conditions + */ + public function get_events_select_exists(string $selectwhere, array $params): bool { + global $DB; + return $DB->record_exists_select('logstore_standard_log', $selectwhere, $params); + } + public function get_internal_log_table_name() { return 'logstore_standard_log'; } diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index c0154c02829..f600942e4cb 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -80,6 +80,8 @@ class store_test extends \advanced_testcase { $logs = $DB->get_records('logstore_standard_log', array(), 'id ASC'); $this->assertCount(0, $logs); + $exists = $store->get_events_select_exists('', array(), 'timecreated ASC', 0, 0); + $this->assertFalse($exists); $this->setCurrentTimeStart(); @@ -147,6 +149,8 @@ class store_test extends \advanced_testcase { $this->assertSame(3, $store->get_events_select_count('', array())); $events = $store->get_events_select('', array(), 'timecreated ASC', 0, 0); // Is actually sorted by "timecreated ASC, id ASC". $this->assertCount(3, $events); + $exists = $store->get_events_select_exists('', array(), 'timecreated ASC', 0, 0); + $this->assertTrue($exists); $resev1 = array_shift($events); array_shift($events); $resev2 = array_shift($events); diff --git a/backup/util/helper/backup_cron_helper.class.php b/backup/util/helper/backup_cron_helper.class.php index d1f649cd751..f21631d182e 100644 --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -790,7 +790,7 @@ abstract class backup_cron_automated_helper { $where .= " and target <> 'course_backup'"; } - if ($reader->get_events_select_count($where, $params)) { + if ($reader->get_events_select_exists($where, $params)) { return true; } } diff --git a/lib/classes/log/sql_reader.php b/lib/classes/log/sql_reader.php index 92aa4528900..5364604157a 100644 --- a/lib/classes/log/sql_reader.php +++ b/lib/classes/log/sql_reader.php @@ -60,6 +60,16 @@ interface sql_reader extends reader { */ public function get_events_select_count($selectwhere, array $params); + /** + * Get whether events are present for the given select clause. + * + * @param string $selectwhere select conditions. + * @param array $params params. + * + * @return bool Whether events available for the given conditions + */ + public function get_events_select_exists(string $selectwhere, array $params): bool; + /** * Fetch records using the given criteria returning an traversable list of events. * diff --git a/lib/upgrade.txt b/lib/upgrade.txt index d805cda6c7f..a8ba56c7d06 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -1,6 +1,12 @@ This files describes API changes in core libraries and APIs, information provided here is intended especially for developers. +=== 4.1 === + +* For performance reasons, sql_reader interface has a new function get_events_select_exists() which determines whether + an event exists with the given criteria (see MDL-72723 for details). + - Breaking: 3rd party log readers implementing interface sql_reader will need to implement get_events_select_exists() + === 4.0 === * To better detect wrong floats (like, for example, unformatted, using local-dependent separators ones) a number of