Skip to content

Commit

Permalink
MDL-66326 Global search: Delete from index when courses are deleted
Browse files Browse the repository at this point in the history
Adds new API support within search engines for optional methods to
delete data for courses and contexts, and implements this for the
two core search plugins (simpledb and solr).

The new API is automatically called when courses or contexts are
deleted. When a whole course is deleted, it only sends the course
delete rather than sending 1,000 separate context deletions as
each activity/block is deleted.
  • Loading branch information
sammarshallou committed Aug 23, 2019
1 parent 9e4178a commit 7ba2a20
Show file tree
Hide file tree
Showing 13 changed files with 438 additions and 2 deletions.
3 changes: 3 additions & 0 deletions lib/accesslib.php
Original file line number Diff line number Diff line change
Expand Up @@ -5390,6 +5390,9 @@ public function delete() {
$DB->delete_records('context', array('id'=>$this->_id));
// purge static context cache if entry present
context::cache_remove($this);

// Inform search engine to delete data related to this context.
\core_search\manager::context_deleted($this);
}

// ====== context level related methods ======
Expand Down
10 changes: 10 additions & 0 deletions lib/moodlelib.php
Original file line number Diff line number Diff line change
Expand Up @@ -4309,6 +4309,9 @@ function delete_user(stdClass $user) {
// Delete all content associated with the user context, but not the context itself.
$usercontext->delete_content();

// Delete any search data.
\core_search\manager::context_deleted($usercontext);

// Any plugin that needs to cleanup should register this event.
// Trigger event.
$event = \core\event\user_deleted::create(
Expand Down Expand Up @@ -5061,6 +5064,10 @@ function delete_course($courseorid, $showfeedback = true) {
}
}

// Tell the search manager we are about to delete a course. This prevents us sending updates
// for each individual context being deleted.
\core_search\manager::course_deleting_start($courseid);

$handler = core_course\customfield\course_handler::create();
$handler->delete_instance($courseid);

Expand All @@ -5078,6 +5085,9 @@ function delete_course($courseorid, $showfeedback = true) {
format_base::reset_course_cache($courseid);
}

// Tell search that we have deleted the course so it can delete course data from the index.
\core_search\manager::course_deleting_finish($courseid);

// Trigger a course deleted event.
$event = \core\event\course_deleted::create(array(
'objectid' => $course->id,
Expand Down
41 changes: 41 additions & 0 deletions search/classes/engine.php
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,47 @@ public abstract function execute_query($filters, $accessinfo, $limit = 0);
*/
abstract function delete($areaid = null);

/**
* Deletes information related to a specific context id. This should be used when the context
* itself is deleted from Moodle.
*
* This only deletes information for the specified context - not for any child contexts.
*
* This function is optional; if not supported it will return false and the information will
* not be deleted from the search index.
*
* If an engine implements this function it should also implement delete_index_for_course;
* otherwise, nothing will be deleted when users delete an entire course at once.
*
* @param int $oldcontextid ID of context that has been deleted
* @return bool True if implemented
* @throws \core_search\engine_exception Engines may throw this exception for any problem
*/
public function delete_index_for_context(int $oldcontextid) {
return false;
}

/**
* Deletes information related to a specific course id. This should be used when the course
* itself is deleted from Moodle.
*
* This deletes all information relating to that course from the index, including all child
* contexts.
*
* This function is optional; if not supported it will return false and the information will
* not be deleted from the search index.
*
* If an engine implements this function then, ideally, it should also implement
* delete_index_for_context so that deletion of single activities/blocks also works.
*
* @param int $oldcourseid ID of course that has been deleted
* @return bool True if implemented
* @throws \core_search\engine_exception Engines may throw this exception for any problem
*/
public function delete_index_for_course(int $oldcourseid) {
return false;
}

/**
* Checks that the schema is the latest version. If the version stored in config does not match
* the current, this function will attempt to upgrade the schema.
Expand Down
77 changes: 77 additions & 0 deletions search/classes/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ class manager {
*/
protected static $instance = null;

/**
* @var array IDs (as keys) of course deletions in progress in this requuest, if any.
*/
protected static $coursedeleting = [];

/**
* @var \core_search\engine
*/
Expand Down Expand Up @@ -1789,4 +1794,76 @@ public static function clean_up_non_existing_area($areaid) {
$engine->delete($areaid);
}

/**
* Informs the search system that a context has been deleted.
*
* This will clear the data from the search index, where the search engine supports that.
*
* This function does not usually throw an exception (so as not to get in the way of the
* context deletion finishing).
*
* This is called for all types of context deletion.
*
* @param \context $context Context object that has just been deleted
*/
public static function context_deleted(\context $context) {
if (self::is_indexing_enabled()) {
try {
// Hold on, are we deleting a course? If so, and this context is part of the course,
// then don't bother to send a delete because we delete the whole course at once
// later.
if (!empty(self::$coursedeleting)) {
$coursecontext = $context->get_course_context(false);
if ($coursecontext && array_key_exists($coursecontext->instanceid, self::$coursedeleting)) {
// Skip further processing.
return;
}
}

$engine = self::instance()->get_engine();
$engine->delete_index_for_context($context->id);
} catch (\moodle_exception $e) {
debugging('Error deleting search index data for context ' . $context->id . ': ' . $e->getMessage());
}
}
}

/**
* Informs the search system that a course is about to be deleted.
*
* This prevents it from sending hundreds of 'delete context' updates for all the individual
* contexts that are deleted.
*
* If you call this, you must call course_deleting_finish().
*
* @param int $courseid Course id that is being deleted
*/
public static function course_deleting_start(int $courseid) {
self::$coursedeleting[$courseid] = true;
}

/**
* Informs the search engine that a course has now been deleted.
*
* This causes the search engine to actually delete the index for the whole course.
*
* @param int $courseid Course id that no longer exists
*/
public static function course_deleting_finish(int $courseid) {
if (!array_key_exists($courseid, self::$coursedeleting)) {
// Show a debug warning. It doesn't actually matter very much, as we will now delete
// the course data anyhow.
debugging('course_deleting_start not called before deletion of ' . $courseid, DEBUG_DEVELOPER);
}
unset(self::$coursedeleting[$courseid]);

if (self::is_indexing_enabled()) {
try {
$engine = self::instance()->get_engine();
$engine->delete_index_for_course($courseid);
} catch (\moodle_exception $e) {
debugging('Error deleting search index data for course ' . $courseid . ': ' . $e->getMessage());
}
}
}
}
34 changes: 34 additions & 0 deletions search/engine/simpledb/classes/engine.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,38 @@ protected function get_simple_query($q) {
);
return array($sql, $params);
}

/**
* Simpledb supports deleting the index for a context.
*
* @param int $oldcontextid Context that has been deleted
* @return bool True to indicate that any data was actually deleted
* @throws \core_search\engine_exception
*/
public function delete_index_for_context(int $oldcontextid) {
global $DB;
try {
$DB->delete_records('search_simpledb_index', ['contextid' => $oldcontextid]);
} catch (\dml_exception $e) {
throw new \core_search\engine_exception('dbupdatefailed');
}
return true;
}

/**
* Simpledb supports deleting the index for a course.
*
* @param int $oldcourseid
* @return bool True to indicate that any data was actually deleted
* @throws \core_search\engine_exception
*/
public function delete_index_for_course(int $oldcourseid) {
global $DB;
try {
$DB->delete_records('search_simpledb_index', ['courseid' => $oldcourseid]);
} catch (\dml_exception $e) {
throw new \core_search\engine_exception('dbupdatefailed');
}
return true;
}
}
73 changes: 71 additions & 2 deletions search/engine/simpledb/tests/engine_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ public function setUp() {

$this->engine = new \search_simpledb\engine();
$this->search = testable_core_search::instance($this->engine);
$areaid = \core_search\manager::generate_areaid('core_mocksearch', 'mock_search_area');
$this->search->add_search_area($areaid, new core_mocksearch\search\mock_search_area());

$this->generator = self::getDataGenerator()->get_plugin_generator('core_search');
$this->generator->setup();
Expand Down Expand Up @@ -105,6 +103,8 @@ public function tearDown() {
public function test_index() {
global $DB;

$this->add_mock_search_area();

$record = new \stdClass();
$record->timemodified = time() - 1;
$this->generator->create_record($record);
Expand All @@ -130,6 +130,8 @@ public function test_index() {
public function test_search() {
global $USER, $DB;

$this->add_mock_search_area();

$this->generator->create_record();
$record = new \stdClass();
$record->title = "Special title";
Expand Down Expand Up @@ -214,6 +216,8 @@ public function test_search() {
*/
public function test_delete() {

$this->add_mock_search_area();

$this->generator->create_record();
$this->generator->create_record();
$this->search->index();
Expand All @@ -237,6 +241,8 @@ public function test_delete() {
*/
public function test_alloweduserid() {

$this->add_mock_search_area();

$area = new core_mocksearch\search\mock_search_area();

$record = $this->generator->create_record();
Expand Down Expand Up @@ -309,6 +315,8 @@ public function test_alloweduserid() {

public function test_delete_by_id() {

$this->add_mock_search_area();

$this->generator->create_record();
$this->generator->create_record();
$this->search->index();
Expand All @@ -334,6 +342,67 @@ public function test_delete_by_id() {
$this->assertNotEquals($deleteid, $result->get('id'));
}

/**
* Tries out deleting data for a context or a course.
*
* @throws moodle_exception
*/
public function test_deleted_contexts_and_courses() {
// Create some courses and activities.
$generator = $this->getDataGenerator();
$course1 = $generator->create_course(['fullname' => 'C1', 'summary' => 'xyzzy']);
$course1page1 = $generator->create_module('page', ['course' => $course1, 'name' => 'C1P1', 'content' => 'xyzzy']);
$generator->create_module('page', ['course' => $course1, 'name' => 'C1P2', 'content' => 'xyzzy']);
$course2 = $generator->create_course(['fullname' => 'C2', 'summary' => 'xyzzy']);
$course2page = $generator->create_module('page', ['course' => $course2, 'name' => 'C2P', 'content' => 'xyzzy']);
$course2pagecontext = \context_module::instance($course2page->cmid);

$this->search->index();

// By default we have all data in the index.
$this->assert_raw_index_contents('xyzzy', ['C1', 'C1P1', 'C1P2', 'C2', 'C2P']);

// Say we delete the course2pagecontext...
$this->engine->delete_index_for_context($course2pagecontext->id);
$this->assert_raw_index_contents('xyzzy', ['C1', 'C1P1', 'C1P2', 'C2']);

// Now delete the second course...
$this->engine->delete_index_for_course($course2->id);
$this->assert_raw_index_contents('xyzzy', ['C1', 'C1P1', 'C1P2']);

// Finally let's delete using Moodle functions to check that works. Single context first.
course_delete_module($course1page1->cmid);
$this->assert_raw_index_contents('xyzzy', ['C1', 'C1P2']);
delete_course($course1, false);
$this->assert_raw_index_contents('xyzzy', []);
}

/**
* Check the contents of the index.
*
* @param string $searchword Word to search for within the content field
* @param string[] $expected Array of expected result titles, in alphabetical order
* @throws dml_exception
*/
protected function assert_raw_index_contents(string $searchword, array $expected) {
global $DB;
$results = $DB->get_records_select('search_simpledb_index',
$DB->sql_like('content', '?'), ['%' . $searchword . '%'], 'id, title');
$titles = array_map(function($x) {
return $x->title;
}, $results);
sort($titles);
$this->assertEquals($expected, $titles);
}

/**
* Adds a mock search area to the search system.
*/
protected function add_mock_search_area() {
$areaid = \core_search\manager::generate_areaid('core_mocksearch', 'mock_search_area');
$this->search->add_search_area($areaid, new core_mocksearch\search\mock_search_area());
}

/**
* Updates mssql fulltext index if necessary.
*
Expand Down
36 changes: 36 additions & 0 deletions search/engine/solr/classes/engine.php
Original file line number Diff line number Diff line change
Expand Up @@ -1445,4 +1445,40 @@ public function get_supported_orders(\context $context) {
public function supports_users() {
return true;
}

/**
* Solr supports deleting the index for a context.
*
* @param int $oldcontextid Context that has been deleted
* @return bool True to indicate that any data was actually deleted
* @throws \core_search\engine_exception
*/
public function delete_index_for_context(int $oldcontextid) {
$client = $this->get_search_client();
try {
$client->deleteByQuery('contextid:' . $oldcontextid);
$client->commit(true);
return true;
} catch (\Exception $e) {
throw new \core_search\engine_exception('error_solr', 'search_solr', '', $e->getMessage());
}
}

/**
* Solr supports deleting the index for a course.
*
* @param int $oldcourseid
* @return bool True to indicate that any data was actually deleted
* @throws \core_search\engine_exception
*/
public function delete_index_for_course(int $oldcourseid) {
$client = $this->get_search_client();
try {
$client->deleteByQuery('courseid:' . $oldcourseid);
$client->commit(true);
return true;
} catch (\Exception $e) {
throw new \core_search\engine_exception('error_solr', 'search_solr', '', $e->getMessage());
}
}
}
1 change: 1 addition & 0 deletions search/engine/solr/lang/en/search_solr.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
$string['connectionsettings'] = 'Connection settings';
$string['errorcreatingschema'] = 'Error creating the Solr schema: {$a}';
$string['errorvalidatingschema'] = 'Error validating Solr schema: field {$a->fieldname} does not exist. Please <a href="{$a->setupurl}">follow this link</a> to set up the required fields.';
$string['errorsolr'] = 'The Solr search engine reported an error: {$a}';
$string['extensionerror'] = 'The Apache Solr PHP extension is not installed. Please check the documentation.';
$string['fileindexing'] = 'Enable file indexing';
$string['fileindexing_help'] = 'If your Solr install supports it, this feature allows Moodle to send files to be indexed.<br/>
Expand Down
Loading

0 comments on commit 7ba2a20

Please sign in to comment.