diff --git a/admin/tool/log/store/database/classes/log/store.php b/admin/tool/log/store/database/classes/log/store.php index c394d0b8add..0bbffb2950f 100644 --- a/admin/tool/log/store/database/classes/log/store.php +++ b/admin/tool/log/store/database/classes/log/store.php @@ -25,7 +25,7 @@ namespace logstore_database\log; defined('MOODLE_INTERNAL') || die(); -class store implements \tool_log\log\writer, \core\log\sql_select_reader { +class store implements \tool_log\log\writer, \core\log\sql_reader { use \tool_log\helper\store, \tool_log\helper\reader, \tool_log\helper\buffered_writer { @@ -173,28 +173,71 @@ class store implements \tool_log\log\writer, \core\log\sql_select_reader { $records = $this->extdb->get_records_select($dbtable, $selectwhere, $params, $sort, '*', $limitfrom, $limitnum); foreach ($records as $data) { - $extra = array('origin' => $data->origin, 'realuserid' => $data->realuserid, 'ip' => $data->ip); - $data = (array)$data; - $id = $data['id']; - $data['other'] = unserialize($data['other']); - if ($data['other'] === false) { - $data['other'] = array(); - } - unset($data['origin']); - unset($data['ip']); - unset($data['realuserid']); - unset($data['id']); - - $event = \core\event\base::restore($data, $extra); - // Add event to list if it's valid. - if ($event) { - $events[$id] = $event; + if ($event = $this->get_log_event($data)) { + $events[$data->id] = $event; } } return $events; } + /** + * Fetch records using given criteria returning a Traversable object. + * + * Note that the traversable object contains a moodle_recordset, so + * remember that is important that you call close() once you finish + * using it. + * + * @param string $selectwhere + * @param array $params + * @param string $sort + * @param int $limitfrom + * @param int $limitnum + * @return \core\dml\recordset_walk|\core\event\base[] + */ + public function get_events_select_iterator($selectwhere, array $params, $sort, $limitfrom, $limitnum) { + if (!$this->init()) { + return array(); + } + + if (!$dbtable = $this->get_config('dbtable')) { + return array(); + } + + $sort = self::tweak_sort_by_id($sort); + + $recordset = $this->extdb->get_recordset_select($dbtable, $selectwhere, $params, $sort, '*', $limitfrom, $limitnum); + + return new \core\dml\recordset_walk($recordset, array($this, 'get_log_event')); + } + + /** + * Returns an event from the log data. + * + * @param stdClass $data Log data + * @return \core\event\base + */ + public function get_log_event($data) { + + $extra = array('origin' => $data->origin, 'ip' => $data->ip, 'realuserid' => $data->realuserid); + $data = (array)$data; + $id = $data['id']; + $data['other'] = unserialize($data['other']); + if ($data['other'] === false) { + $data['other'] = array(); + } + unset($data['origin']); + unset($data['ip']); + unset($data['realuserid']); + unset($data['id']); + + if (!$event = \core\event\base::restore($data, $extra)) { + return null; + } + + return $event; + } + /** * Get number of events present for the given select clause. * diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php index 846e4ab0f93..7aa205689f1 100644 --- a/admin/tool/log/store/legacy/classes/log/store.php +++ b/admin/tool/log/store/legacy/classes/log/store.php @@ -26,7 +26,7 @@ namespace logstore_legacy\log; defined('MOODLE_INTERNAL') || die(); -class store implements \tool_log\log\store, \core\log\sql_select_reader { +class store implements \tool_log\log\store, \core\log\sql_reader { use \tool_log\helper\store, \tool_log\helper\reader; @@ -47,7 +47,7 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { const CRUD_REGEX = "/(crud).*?(<>|=|!=).*?'(.*?)'/s"; /** - * This method contains mapping required for Moodle core to make legacy store compatible with other sql_select_reader based + * This method contains mapping required for Moodle core to make legacy store compatible with other sql_reader based * queries. * * @param string $selectwhere Select statment @@ -104,7 +104,7 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { $events = array(); foreach ($records as $data) { - $events[$data->id] = \logstore_legacy\event\legacy_logged::restore_legacy($data); + $events[$data->id] = $this->get_log_event($data); } $records->close(); @@ -112,6 +112,48 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { return $events; } + /** + * Fetch records using given criteria returning a Traversable object. + * + * Note that the traversable object contains a moodle_recordset, so + * remember that is important that you call close() once you finish + * using it. + * + * @param string $selectwhere + * @param array $params + * @param string $sort + * @param int $limitfrom + * @param int $limitnum + * @return \Traversable|\core\event\base[] + */ + public function get_events_select_iterator($selectwhere, array $params, $sort, $limitfrom, $limitnum) { + global $DB; + + $sort = self::tweak_sort_by_id($sort); + + // Replace the query with hardcoded mappings required for core. + list($selectwhere, $params, $sort) = self::replace_sql_legacy($selectwhere, $params, $sort); + + try { + $recordset = $DB->get_recordset_select('log', $selectwhere, $params, $sort, '*', $limitfrom, $limitnum); + } catch (\moodle_exception $ex) { + debugging("error converting legacy event data " . $ex->getMessage() . $ex->debuginfo, DEBUG_DEVELOPER); + return new \EmptyIterator; + } + + return new \core\dml\recordset_walk($recordset, array($this, 'get_log_event')); + } + + /** + * Returns an event from the log data. + * + * @param stdClass $data Log data + * @return \core\event\base + */ + public function get_log_event($data) { + return \logstore_legacy\event\legacy_logged::restore_legacy($data); + } + public function get_events_select_count($selectwhere, array $params) { global $DB; diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index e571a235991..7fb6353b64d 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -50,7 +50,7 @@ class logstore_legacy_store_testcase extends advanced_testcase { $this->assertEquals(array('logstore_legacy'), array_keys($stores)); $store = $stores['logstore_legacy']; $this->assertInstanceOf('logstore_legacy\log\store', $store); - $this->assertInstanceOf('core\log\sql_select_reader', $store); + $this->assertInstanceOf('core\log\sql_reader', $store); $this->assertTrue($store->is_logging()); $logs = $DB->get_records('log', array(), 'id ASC'); diff --git a/admin/tool/log/store/standard/classes/log/store.php b/admin/tool/log/store/standard/classes/log/store.php index 44c66dea4b4..fe93ff58fbc 100644 --- a/admin/tool/log/store/standard/classes/log/store.php +++ b/admin/tool/log/store/standard/classes/log/store.php @@ -26,7 +26,7 @@ namespace logstore_standard\log; defined('MOODLE_INTERNAL') || die(); -class store implements \tool_log\log\writer, \core\log\sql_internal_reader { +class store implements \tool_log\log\writer, \core\log\sql_internal_table_reader { use \tool_log\helper\store, \tool_log\helper\buffered_writer, \tool_log\helper\reader; @@ -75,22 +75,8 @@ class store implements \tool_log\log\writer, \core\log\sql_internal_reader { $records = $DB->get_recordset_select('logstore_standard_log', $selectwhere, $params, $sort, '*', $limitfrom, $limitnum); foreach ($records as $data) { - $extra = array('origin' => $data->origin, 'ip' => $data->ip, 'realuserid' => $data->realuserid); - $data = (array)$data; - $id = $data['id']; - $data['other'] = unserialize($data['other']); - if ($data['other'] === false) { - $data['other'] = array(); - } - unset($data['origin']); - unset($data['ip']); - unset($data['realuserid']); - unset($data['id']); - - $event = \core\event\base::restore($data, $extra); - // Add event to list if it's valid. - if ($event) { - $events[$id] = $event; + if ($event = $this->get_log_event($data)) { + $events[$data->id] = $event; } } @@ -99,6 +85,57 @@ class store implements \tool_log\log\writer, \core\log\sql_internal_reader { return $events; } + /** + * Fetch records using given criteria returning a Traversable object. + * + * Note that the traversable object contains a moodle_recordset, so + * remember that is important that you call close() once you finish + * using it. + * + * @param string $selectwhere + * @param array $params + * @param string $sort + * @param int $limitfrom + * @param int $limitnum + * @return \core\dml\recordset_walk|\core\event\base[] + */ + public function get_events_select_iterator($selectwhere, array $params, $sort, $limitfrom, $limitnum) { + global $DB; + + $sort = self::tweak_sort_by_id($sort); + + $recordset = $DB->get_recordset_select('logstore_standard_log', $selectwhere, $params, $sort, '*', $limitfrom, $limitnum); + + return new \core\dml\recordset_walk($recordset, array($this, 'get_log_event')); + } + + /** + * Returns an event from the log data. + * + * @param stdClass $data Log data + * @return \core\event\base + */ + public function get_log_event($data) { + + $extra = array('origin' => $data->origin, 'ip' => $data->ip, 'realuserid' => $data->realuserid); + $data = (array)$data; + $id = $data['id']; + $data['other'] = unserialize($data['other']); + if ($data['other'] === false) { + $data['other'] = array(); + } + unset($data['origin']); + unset($data['ip']); + unset($data['realuserid']); + unset($data['id']); + + if (!$event = \core\event\base::restore($data, $extra)) { + return null; + } + + return $event; + } + public function get_events_select_count($selectwhere, array $params) { global $DB; return $DB->count_records_select('logstore_standard_log', $selectwhere, $params); diff --git a/admin/tool/log/store/standard/tests/store_test.php b/admin/tool/log/store/standard/tests/store_test.php index 08d70d43069..0ff4f6a1df0 100644 --- a/admin/tool/log/store/standard/tests/store_test.php +++ b/admin/tool/log/store/standard/tests/store_test.php @@ -218,4 +218,59 @@ class logstore_standard_store_testcase extends advanced_testcase { $this->assertContains($expectedreport, $reports); } } + + /** + * Test sql_reader::get_events_select_iterator. + * @return void + */ + public function test_events_traversable() { + global $DB; + + $this->resetAfterTest(); + $this->preventResetByRollback(); + $this->setAdminUser(); + + set_config('enabled_stores', 'logstore_standard', 'tool_log'); + + $manager = get_log_manager(true); + $stores = $manager->get_readers(); + $store = $stores['logstore_standard']; + + $events = $store->get_events_select_iterator('', array(), '', 0, 0); + $this->assertFalse($events->valid()); + + // Here it should be already closed, but we should be allowed to + // over-close it without exception. + $events->close(); + + $user = $this->getDataGenerator()->create_user(); + for ($i = 0; $i < 1000; $i++) { + \core\event\user_created::create_from_userid($user->id)->trigger(); + } + $store->flush(); + + // Check some various sizes get the right number of elements. + $this->assertEquals(1, iterator_count($store->get_events_select_iterator('', array(), '', 0, 1))); + $this->assertEquals(2, iterator_count($store->get_events_select_iterator('', array(), '', 0, 2))); + + $iterator = $store->get_events_select_iterator('', array(), '', 0, 500); + $this->assertInstanceOf('\core\event\base', $iterator->current()); + $this->assertEquals(500, iterator_count($iterator)); + $iterator->close(); + + // Look for non-linear memory usage for the iterator version. + $mem = memory_get_usage(); + $events = $store->get_events_select('', array(), '', 0, 0); + $delta1 = memory_get_usage() - $mem; + $events = $store->get_events_select_iterator('', array(), '', 0, 0); + $delta2 = memory_get_usage() - $mem; + $this->assertInstanceOf('\Traversable', $events); + $events->close(); + + $this->assertLessThan($delta1 / 10, $delta2); + + set_config('enabled_stores', '', 'tool_log'); + get_log_manager(true); + } + } diff --git a/lib/tablelib.php b/lib/tablelib.php index 2e6960f7730..134671adc3b 100644 --- a/lib/tablelib.php +++ b/lib/tablelib.php @@ -1345,7 +1345,7 @@ class table_sql extends flexible_table { */ public $sql = NULL; /** - * @var array Data fetched from the db. + * @var array|\Traversable Data fetched from the db. */ public $rawdata = NULL; @@ -1374,14 +1374,27 @@ class table_sql extends flexible_table { * processing each col using either col_{columnname} method or other_cols * method or if other_cols returns NULL then put the data straight into the * table. + * + * @return void */ function build_table() { - if ($this->rawdata) { - foreach ($this->rawdata as $row) { - $formattedrow = $this->format_row($row); - $this->add_data_keyed($formattedrow, - $this->get_row_class($row)); - } + + if ($this->rawdata instanceof \Traversable && !$this->rawdata->valid()) { + return; + } + if (!$this->rawdata) { + return; + } + + foreach ($this->rawdata as $row) { + $formattedrow = $this->format_row($row); + $this->add_data_keyed($formattedrow, + $this->get_row_class($row)); + } + + if ($this->rawdata instanceof \core\dml\recordset_walk || + $this->rawdata instanceof moodle_recordset) { + $this->rawdata->close(); } } diff --git a/report/log/classes/table_log.php b/report/log/classes/table_log.php index 8df6db9285d..25ac3df0154 100644 --- a/report/log/classes/table_log.php +++ b/report/log/classes/table_log.php @@ -36,7 +36,11 @@ class report_log_table_log extends table_sql { /** @var array list of user fullnames shown in report */ private $userfullnames = array(); - /** @var array list of course short names shown in report */ + /** + * @deprecated since Moodle 2.9 MDL-48595 - please do not use this argument any more. + * @todo MDL-49291 This will be deleted in 3.1 + * @var array list of course short names shown in report. + */ private $courseshortnames = array(); /** @var array list of context name shown in report */ @@ -54,7 +58,7 @@ class report_log_table_log extends table_sql { * - int userid: user id * - int|string modid: Module id or "site_errors" to view site errors * - int groupid: Group id - * - \core\log\sql_select_reader logreader: reader from which data will be fetched. + * - \core\log\sql_reader logreader: reader from which data will be fetched. * - int edulevel: educational level. * - string action: view action * - int date: Date from which logs to be viewed. @@ -94,10 +98,15 @@ class report_log_table_log extends table_sql { /** * Generate the course column. * + * @deprecated since Moodle 2.9 MDL-48595 - please do not use this function any more. + * @todo MDL-49291 This will be deleted in 3.1 * @param stdClass $event event data. * @return string HTML for the course column. */ public function col_course($event) { + + debugging('col_course() is deprecated, there is no such column', DEBUG_DEVELOPER); + if (empty($event->courseid) || empty($this->courseshortnames[$event->courseid])) { return '-'; } else { @@ -105,6 +114,39 @@ class report_log_table_log extends table_sql { } } + /** + * Gets the user full name. + * + * This function is useful because, in the unlikely case that the user is + * not already loaded in $this->userfullnames it will fetch it from db. + * + * @since Moodle 2.9 + * @param int $userid + * @return string|false + */ + protected function get_user_fullname($userid) { + global $DB; + + if (!empty($this->userfullnames[$userid])) { + return $this->userfullnames[$userid]; + } + + // We already looked for the user and it does not exist. + if ($this->userfullnames[$userid] === false) { + return false; + } + + // If we reach that point new users logs have been generated since the last users db query. + list($usql, $uparams) = $DB->get_in_or_equal($userid); + $sql = "SELECT id," . get_all_user_name_fields(true) . " FROM {user} WHERE id " . $usql; + if (!$user = $DB->get_records_sql($sql, $uparams)) { + return false; + } + + $this->userfullnames[$userid] = fullname($user); + return $this->userfullnames[$userid]; + } + /** * Generate the time column. * @@ -129,25 +171,29 @@ class report_log_table_log extends table_sql { // Add username who did the action. if (!empty($logextra['realuserid'])) { $a = new stdClass(); - $params = array('id' => $logextra['realuserid']); - if ($event->courseid) { - $params['course'] = $event->courseid; + if (!$a->realusername = $this->get_user_fullname($logextra['realuserid'])) { + $a->realusername = '-'; + } + if (!$a->asusername = $this->get_user_fullname($event->userid)) { + $a->asusername = '-'; } - $a->realusername = $this->userfullnames[$logextra['realuserid']]; - $a->asusername = $this->userfullnames[$event->userid]; if (empty($this->download)) { + $params = array('id' => $logextra['realuserid']); + if ($event->courseid) { + $params['course'] = $event->courseid; + } $a->realusername = html_writer::link(new moodle_url('/user/view.php', $params), $a->realusername); $params['id'] = $event->userid; $a->asusername = html_writer::link(new moodle_url('/user/view.php', $params), $a->asusername); } $username = get_string('eventloggedas', 'report_log', $a); - } else if (!empty($event->userid) && !empty($this->userfullnames[$event->userid])) { - $params = array('id' => $event->userid); - if ($event->courseid) { - $params['course'] = $event->courseid; - } - $username = $this->userfullnames[$event->userid]; + + } else if (!empty($event->userid) && $username = $this->get_user_fullname($event->userid)) { if (empty($this->download)) { + $params = array('id' => $event->userid); + if ($event->courseid) { + $params['course'] = $event->courseid; + } $username = html_writer::link(new moodle_url('/user/view.php', $params), $username); } } else { @@ -164,13 +210,12 @@ class report_log_table_log extends table_sql { */ public function col_relatedfullnameuser($event) { // Add affected user. - if (!empty($event->relateduserid) && isset($this->userfullnames[$event->relateduserid])) { - $params = array('id' => $event->relateduserid); - if ($event->courseid) { - $params['course'] = $event->courseid; - } - $username = $this->userfullnames[$event->relateduserid]; + if (!empty($event->relateduserid) && $username = $this->get_user_fullname($event->relateduserid)) { if (empty($this->download)) { + $params = array('id' => $event->relateduserid); + if ($event->courseid) { + $params['course'] = $event->courseid; + } $username = html_writer::link(new moodle_url('/user/view.php', $params), $username); } } else { @@ -463,26 +508,43 @@ class report_log_table_log extends table_sql { $this->pageable(false); } - $this->rawdata = $this->filterparams->logreader->get_events_select($selector, $params, $this->filterparams->orderby, - $this->get_page_start(), $this->get_page_size()); + // Get the users and course data. + $this->rawdata = $this->filterparams->logreader->get_events_select_iterator($selector, $params, + $this->filterparams->orderby, $this->get_page_start(), $this->get_page_size()); + + // Update list of users which will be displayed on log page. + $this->update_users_used(); + + // Get the events. Same query than before; even if it is not likely, logs from new users + // may be added since last query so we will need to work around later to prevent problems. + // In almost most of the cases this will be better than having two opened recordsets. + $this->rawdata = $this->filterparams->logreader->get_events_select_iterator($selector, $params, + $this->filterparams->orderby, $this->get_page_start(), $this->get_page_size()); // Set initial bars. if ($useinitialsbar && !$this->is_downloading()) { $this->initialbars($total > $pagesize); } - // Update list of users and courses list which will be displayed on log page. - $this->update_users_and_courses_used(); } /** * Helper function to create list of course shortname and user fullname shown in log report. + * * This will update $this->userfullnames and $this->courseshortnames array with userfullname and courseshortname (with link), * which will be used to render logs in table. + * + * @deprecated since Moodle 2.9 MDL-48595 - please do not use this function any more. + * @todo MDL-49291 This will be deleted in 3.1 + * @see self::update_users_used() */ public function update_users_and_courses_used() { global $SITE, $DB; + debugging('update_users_and_courses_used() is deprecated, please use update_users_used() instead.', DEBUG_DEVELOPER); + + // We should not call self::update_users_used() as would have to iterate twice around the list of logs. + $this->userfullnames = array(); $this->courseshortnames = array($SITE->id => $SITE->shortname); $userids = array(); @@ -491,14 +553,14 @@ class report_log_table_log extends table_sql { // Get list of userids and courseids which will be shown in log report. foreach ($this->rawdata as $event) { $logextra = $event->get_logextra(); - if (!empty($event->userid) && !in_array($event->userid, $userids)) { - $userids[] = $event->userid; + if (!empty($event->userid) && empty($userids[$event->userid])) { + $userids[$event->userid] = $event->userid; } - if (!empty($logextra['realuserid']) && !in_array($logextra['realuserid'], $userids)) { - $userids[] = $logextra['realuserid']; + if (!empty($logextra['realuserid']) && empty($userids[$logextra['realuserid']])) { + $userids[$logextra['realuserid']] = $logextra['realuserid']; } - if (!empty($event->relateduserid) && !in_array($event->relateduserid, $userids)) { - $userids[] = $event->relateduserid; + if (!empty($event->relateduserid) && empty($userids[$event->relateduserid])) { + $userids[$event->relateduserid] = $event->relateduserid; } if (!empty($event->courseid) && ($event->courseid != $SITE->id) && !in_array($event->courseid, $courseids)) { @@ -506,6 +568,12 @@ class report_log_table_log extends table_sql { } } + // Closing it just in case, we can not rewind moodle recordsets anyway. + if ($this->rawdata instanceof \core\dml\recordset_walk || + $this->rawdata instanceof moodle_recordset) { + $this->rawdata->close(); + } + // Get user fullname and put that in return list. if (!empty($userids)) { list($usql, $uparams) = $DB->get_in_or_equal($userids); @@ -513,6 +581,13 @@ class report_log_table_log extends table_sql { $uparams); foreach ($users as $userid => $user) { $this->userfullnames[$userid] = fullname($user); + unset($userids[$userid]); + } + + // We fill the array with false values for the users that don't exist anymore + // in the database so we don't need to query the db again later. + foreach ($userids as $userid) { + $this->userfullnames[$userid] = false; } } @@ -537,4 +612,53 @@ class report_log_table_log extends table_sql { } } } + + /** + * Helper function to create list of user fullnames shown in log report. + * + * This will update $this->userfullnames array with userfullname, + * which will be used to render logs in table. + * + * @since Moodle 2.9 + * @return void + */ + protected function update_users_used() { + global $DB; + + $this->userfullnames = array(); + $userids = array(); + + // For each event cache full username. + // Get list of userids which will be shown in log report. + foreach ($this->rawdata as $event) { + $logextra = $event->get_logextra(); + if (!empty($event->userid) && empty($userids[$event->userid])) { + $userids[$event->userid] = $event->userid; + } + if (!empty($logextra['realuserid']) && empty($userids[$logextra['realuserid']])) { + $userids[$logextra['realuserid']] = $logextra['realuserid']; + } + if (!empty($event->relateduserid) && empty($userids[$event->relateduserid])) { + $userids[$event->relateduserid] = $event->relateduserid; + } + } + $this->rawdata->close(); + + // Get user fullname and put that in return list. + if (!empty($userids)) { + list($usql, $uparams) = $DB->get_in_or_equal($userids); + $users = $DB->get_records_sql("SELECT id," . get_all_user_name_fields(true) . " FROM {user} WHERE id " . $usql, + $uparams); + foreach ($users as $userid => $user) { + $this->userfullnames[$userid] = fullname($user); + unset($userids[$userid]); + } + + // We fill the array with false values for the users that don't exist anymore + // in the database so we don't need to query the db again later. + foreach ($userids as $userid) { + $this->userfullnames[$userid] = false; + } + } + } }