Skip to content

Commit

Permalink
MDL-44366 improve glossary filter performance
Browse files Browse the repository at this point in the history
This should have when there are no glossaries in course or
when there are no linked entries in existing glossaries.
  • Loading branch information
skodak committed Apr 14, 2014
1 parent 86286bd commit 1e83288
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 24 deletions.
8 changes: 3 additions & 5 deletions filter/glossary/filter.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,12 @@ public function filter($text, array $options = array()) {
$coursectx = $this->context->get_course_context(false);
if (!$coursectx) {
// Only global glossaries will be linked.
$course = null;
$courseid = 0;
} else {
$course = get_course($coursectx->instanceid, false);
$courseid = (int)$course->id;
$courseid = $coursectx->instanceid;
}

if ($this->cachecourseid !== $courseid or $this->cacheuserid !== $USER->id) {
if ($this->cachecourseid != $courseid or $this->cacheuserid != $USER->id) {
// Invalidate the page cache.
$this->cacheconceptlist = null;
}
Expand All @@ -79,7 +77,7 @@ public function filter($text, array $options = array()) {
return filter_phrases($text, $this->cacheconceptlist);
}

list($glossaries, $allconcepts) = \mod_glossary\local\concept_cache::get_concepts($course);
list($glossaries, $allconcepts) = \mod_glossary\local\concept_cache::get_concepts($courseid);

if (!$allconcepts) {
$this->cacheuserid = $USER->id;
Expand Down
40 changes: 24 additions & 16 deletions mod/glossary/classes/local/concept_cache.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,33 +147,35 @@ protected static function fetch_concepts(array $glossaries) {

/**
* Get all linked concepts from course.
* @param \stdClass $course
* @param int $courseid
* @return array
*/
protected static function get_course_concepts($course) {
protected static function get_course_concepts($courseid) {
global $DB;

if (empty($course->id)) {
if (empty($courseid)) {
return array(array(), array());
}

$courseid = (int)$courseid;

$cache = \cache::make('mod_glossary', 'concepts');
$data = $cache->get((int)$course->id);
$data = $cache->get($courseid);
if (is_array($data)) {
list($glossaries, $allconcepts) = $data;

} else {
// Find all course glossaries.
$sql = "SELECT g.id, g.name
FROM {glossary} g
JOIN {course_modules} cm ON (cm.instance = g.id)
JOIN {modules} m ON (m.name = 'glossary' AND m.id = cm.module)
WHERE g.usedynalink = 1 AND g.course = :course AND cm.visible = 1 AND m.visible = 1
ORDER BY g.globalglossary, g.id";
$glossaries = $DB->get_records_sql_menu($sql, array('course' => $course->id));
FROM {glossary} g
JOIN {course_modules} cm ON (cm.instance = g.id)
JOIN {modules} m ON (m.name = 'glossary' AND m.id = cm.module)
WHERE g.usedynalink = 1 AND g.course = :course AND cm.visible = 1 AND m.visible = 1
ORDER BY g.globalglossary, g.id";
$glossaries = $DB->get_records_sql_menu($sql, array('course' => $courseid));
if (!$glossaries) {
$data = array(array(), array());
$cache->set((int)$course->id, $data);
$cache->set($courseid, $data);
return $data;
}
foreach ($glossaries as $id => $name) {
Expand All @@ -187,13 +189,19 @@ protected static function get_course_concepts($course) {
unset($glossaries[$gid]);
}
}
$cache->set((int)$course->id, array($glossaries, $allconcepts));
if (!$glossaries) {
// This means there are no interesting concepts in the existing glossaries.
$data = array(array(), array());
$cache->set($courseid, $data);
return $data;
}
$cache->set($courseid, array($glossaries, $allconcepts));
}

$concepts = $allconcepts;

// Verify access control to glossary instances.
$modinfo = get_fast_modinfo($course);
$modinfo = get_fast_modinfo($courseid);
$cminfos = $modinfo->get_instances_of('glossary');
foreach ($concepts as $modid => $unused) {
if (!isset($cminfos[$modid])) {
Expand Down Expand Up @@ -256,11 +264,11 @@ protected static function get_global_concepts() {

/**
* Get all concepts that should be linked in the given course.
* @param \stdClass $course
* @param int $courseid
* @return array with two elements - array of glossaries and concepts for each glossary
*/
public static function get_concepts($course) {
list($glossaries, $concepts) = self::get_course_concepts($course);
public static function get_concepts($courseid) {
list($glossaries, $concepts) = self::get_course_concepts($courseid);
list($globalglossaries, $globalconcepts) = self::get_global_concepts();

foreach ($globalconcepts as $gid => $cs) {
Expand Down
1 change: 1 addition & 0 deletions mod/glossary/lang/en/glossary.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
$string['author'] = 'author';
$string['authorview'] = 'Browse by Author';
$string['back'] = 'Back';
$string['cachedef_concepts'] = 'Concept linking';
$string['cantinsertcat'] = 'Can\'t insert category';
$string['cantinsertrec'] = 'Can\'t insert record';
$string['cantinsertrel'] = 'Can\'t insert relation category-entry';
Expand Down
6 changes: 3 additions & 3 deletions mod/glossary/tests/concept_cache_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function test_concept_fetching() {

\mod_glossary\local\concept_cache::reset_caches();

$concepts1 = \mod_glossary\local\concept_cache::get_concepts($course1);
$concepts1 = \mod_glossary\local\concept_cache::get_concepts($course1->id);
$this->assertCount(3, $concepts1[0]);
$this->arrayHasKey($concepts1[0], $glossary1a->id);
$this->arrayHasKey($concepts1[0], $glossary1b->id);
Expand Down Expand Up @@ -136,7 +136,7 @@ public function test_concept_fetching() {
}
}

$concepts3 = \mod_glossary\local\concept_cache::get_concepts($site);
$concepts3 = \mod_glossary\local\concept_cache::get_concepts($site->id);
$this->assertCount(1, $concepts3[0]);
$this->arrayHasKey($concepts3[0], $glossary3->id);
$this->assertCount(1, $concepts3[1]);
Expand All @@ -156,7 +156,7 @@ public function test_concept_fetching() {
}
}

$concepts2 = \mod_glossary\local\concept_cache::get_concepts($course2);
$concepts2 = \mod_glossary\local\concept_cache::get_concepts($course2->id);
$this->assertEquals($concepts3, $concepts2);
}
}

0 comments on commit 1e83288

Please sign in to comment.