Skip to content

Commit

Permalink
MDL-74586 rss: Make rss_get_userid_from_token() use table index
Browse files Browse the repository at this point in the history
Due to missing `AND k.script = 'rss'` condition, the query did not make
use of the existing script-value compound index defined for the table.
So it had to perform the full sequential scan for all rows when
searching for the token. This had serious performance issues on sites
with many users, especially in case on non-existing token / key.
  • Loading branch information
mudrd8mz committed Feb 28, 2023
1 parent 9ee4f8d commit fcfe8ce
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
13 changes: 9 additions & 4 deletions lib/rsslib.php
Expand Up @@ -413,10 +413,15 @@ function rss_geterrorxmlfile($errortype = 'rsserror') {
function rss_get_userid_from_token($token) {
global $DB;

$sql = 'SELECT u.id FROM {user} u
JOIN {user_private_key} k ON u.id = k.userid
WHERE u.deleted = 0 AND u.confirmed = 1
AND u.suspended = 0 AND k.value = ?';
$sql = "SELECT u.id
FROM {user} u
JOIN {user_private_key} k ON u.id = k.userid
WHERE u.deleted = 0
AND u.confirmed = 1
AND u.suspended = 0
AND k.script = 'rss'
AND k.value = ?";

return $DB->get_field_sql($sql, array($token), IGNORE_MISSING);
}

Expand Down
17 changes: 16 additions & 1 deletion lib/tests/rsslib_test.php
Expand Up @@ -20,7 +20,7 @@

global $CFG;
require_once($CFG->libdir.'/simplepie/moodle_simplepie.php');

require_once($CFG->libdir . '/rsslib.php');

/**
* These tests rely on the rsstest.xml file on download.moodle.org,
Expand Down Expand Up @@ -140,4 +140,19 @@ public function test_redirect() {
$this->assertSame('Moodle News', $feed->get_title());
$this->assertSame('http://moodle.org/mod/forum/view.php?f=1', $feed->get_link());
}

/**
* Test that we can get the right user ID based on the provided private key (token).
*
* @covers ::rss_get_userid_from_token
*/
public function test_rss_get_userid_from_token() {
global $USER;

$this->resetAfterTest();
$this->setGuestUser();

$key = rss_get_token($USER->id);
$this->assertSame(rss_get_userid_from_token($key), $USER->id);
}
}

0 comments on commit fcfe8ce

Please sign in to comment.