MDL-55356 core_search: New area API get_document_recordset
authorsam marshall <s.marshall@open.ac.uk>
Wed, 6 Sep 2017 14:37:14 +0000 (15:37 +0100)
committersam marshall <s.marshall@open.ac.uk>
Wed, 11 Oct 2017 16:17:08 +0000 (17:17 +0100)
The search area API now includes a new function get_document_recordset
which should be implemented in preference to the older
get_recordset_by_timestamp. (It's also possible to implement both in
plugin search areas which need to work against older Moodle versions.)

Existing search areas without the new function will continue to work as
before (obviously without the new functionality).

search/classes/base.php
search/classes/base_activity.php
search/classes/base_block.php
search/classes/base_mod.php
search/tests/base_activity_test.php
search/tests/base_block_test.php
search/upgrade.txt

index 915555d..3ba3297 100644 (file)
@@ -246,10 +246,53 @@ abstract class base {
      * - Order the returned data by time modified in ascending order, as \core_search::manager will need to store the modified time
      *   of the last indexed document.
      *
+     * Since Moodle 3.4, subclasses should instead implement get_document_recordset, which has
+     * an additional context parameter. This function continues to work for implementations which
+     * haven't been updated, or where the context parameter is not required.
+     *
      * @param int $modifiedfrom
-     * @return moodle_recordset
+     * @return \moodle_recordset
+     */
+    public function get_recordset_by_timestamp($modifiedfrom = 0) {
+        $result = $this->get_document_recordset($modifiedfrom);
+        if ($result === false) {
+            throw new \coding_exception(
+                    'Search area must implement get_document_recordset or get_recordset_by_timestamp');
+        }
+        return $result;
+    }
+
+    /**
+     * Returns a recordset containing all items from this area, optionally within the given context,
+     * and including only items modifed from (>=) the specified time. The recordset must be ordered
+     * in ascending order of modified time.
+     *
+     * Each record can include any data self::get_document might need. It must include an 'id'
+     * field,a unique identifier (in this area's scope) of a document to index in the search engine.
+     * If the indexed content field can contain embedded files, the 'id' value should match the
+     * filearea itemid.
+     *
+     * The return value can be a recordset, null (if this area does not provide any results in the
+     * given context and there is no need to do a database query to find out), or false (if this
+     * facility is not currently supported by this search area).
+     *
+     * If this function returns false, then:
+     * - If indexing the entire system (no context restriction) the search indexer will try
+     *   get_recordset_by_timestamp instead
+     * - If trying to index a context (e.g. when restoring a course), the search indexer will not
+     *   index this area, so that restored content may not be indexed.
+     *
+     * The default implementation returns false, indicating that this facility is not supported and
+     * the older get_recordset_by_timestamp function should be used.
+     *
+     * @param int $modifiedfrom Return only records modified after this date
+     * @param \context|null $context Context (null means no context restriction)
+     * @return \moodle_recordset|null|false Recordset / null if no results / false if not supported
+     * @since Moodle 3.4
      */
-    abstract public function get_recordset_by_timestamp($modifiedfrom = 0);
+    public function get_document_recordset($modifiedfrom = 0, \context $context = null) {
+        return false;
+    }
 
     /**
      * Returns the document related with the provided record.
@@ -341,4 +384,91 @@ abstract class base {
      * @return \moodle_url
      */
     abstract public function get_context_url(\core_search\document $doc);
+
+    /**
+     * Helper function that gets SQL useful for restricting a search query given a passed-in
+     * context, for data stored at course level.
+     *
+     * The SQL returned will be zero or more JOIN statements, surrounded by whitespace, which act
+     * as restrictions on the query based on the rows in a module table.
+     *
+     * You can pass in a null or system context, which will both return an empty string and no
+     * params.
+     *
+     * Returns an array with two nulls if there can be no results for a course within this context.
+     *
+     * If named parameters are used, these will be named gclcrs0, gclcrs1, etc. The table aliases
+     * used in SQL also all begin with gclcrs, to avoid conflicts.
+     *
+     * @param \context|null $context Context to restrict the query
+     * @param string $coursetable Name of alias for course table e.g. 'c'
+     * @param int $paramtype Type of SQL parameters to use (default question mark)
+     * @return array Array with SQL and parameters; both null if no need to query
+     * @throws \coding_exception If called with invalid params
+     */
+    protected function get_course_level_context_restriction_sql(\context $context = null,
+            $coursetable, $paramtype = SQL_PARAMS_QM) {
+        global $DB;
+
+        if (!$context) {
+            return ['', []];
+        }
+
+        switch ($paramtype) {
+            case SQL_PARAMS_QM:
+                $param1 = '?';
+                $param2 = '?';
+                $key1 = 0;
+                $key2 = 1;
+                break;
+            case SQL_PARAMS_NAMED:
+                $param1 = ':gclcrs0';
+                $param2 = ':gclcrs1';
+                $key1 = 'gclcrs0';
+                $key2 = 'gclcrs1';
+                break;
+            default:
+                throw new \coding_exception('Unexpected $paramtype: ' . $paramtype);
+        }
+
+        $params = [];
+        switch ($context->contextlevel) {
+            case CONTEXT_SYSTEM:
+                $sql = '';
+                break;
+
+            case CONTEXT_COURSECAT:
+                // Find all courses within the specified category or any sub-category.
+                $pathmatch = $DB->sql_like('gclcrscc2.path',
+                        $DB->sql_concat('gclcrscc1.path', $param2));
+                $sql = " JOIN {course_categories} gclcrscc1 ON gclcrscc1.id = $param1
+                         JOIN {course_categories} gclcrscc2 ON gclcrscc2.id = $coursetable.category
+                              AND (gclcrscc2.id = gclcrscc1.id OR $pathmatch) ";
+                $params[$key1] = $context->instanceid;
+                // Note: This param is a bit annoying as it obviously never changes, but sql_like
+                // throws a debug warning if you pass it anything with quotes in, so it has to be
+                // a bound parameter.
+                $params[$key2] = '/%';
+                break;
+
+            case CONTEXT_COURSE:
+                // We just join again against the same course entry and confirm that it has the
+                // same id as the context.
+                $sql = " JOIN {course} gclcrsc ON gclcrsc.id = $coursetable.id
+                              AND gclcrsc.id = $param1";
+                $params[$key1] = $context->instanceid;
+                break;
+
+            case CONTEXT_BLOCK:
+            case CONTEXT_MODULE:
+            case CONTEXT_USER:
+                // Context cannot contain any courses.
+                return [null, null];
+
+            default:
+                throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel);
+        }
+
+        return [$sql, $params];
+    }
 }
index d6702b9..66ea4e1 100644 (file)
@@ -55,15 +55,23 @@ abstract class base_activity extends base_mod {
     protected static $levels = [CONTEXT_MODULE];
 
     /**
-     * Returns recordset containing required data for indexing activities.
+     * Returns recordset containing all activities within the given context.
      *
-     * @param int $modifiedfrom timestamp
-     * @return \moodle_recordset
+     * @param \context|null $context Context
+     * @param int $modifiedfrom Return only records modified after this date
+     * @return \moodle_recordset|null Recordset, or null if no possible activities in given context
      */
-    public function get_recordset_by_timestamp($modifiedfrom = 0) {
+    public function get_document_recordset($modifiedfrom = 0, \context $context = null) {
         global $DB;
-        return $DB->get_recordset_select($this->get_module_name(), static::MODIFIED_FIELD_NAME . ' >= ?', array($modifiedfrom),
-                static::MODIFIED_FIELD_NAME . ' ASC');
+        list ($contextjoin, $contextparams) = $this->get_context_restriction_sql(
+                $context, $this->get_module_name(), 'modtable');
+        if ($contextjoin === null) {
+            return null;
+        }
+        return $DB->get_recordset_sql('SELECT modtable.* FROM {' . $this->get_module_name() .
+                '} modtable ' . $contextjoin . ' WHERE modtable.' . static::MODIFIED_FIELD_NAME .
+                ' >= ? ORDER BY modtable.' . static::MODIFIED_FIELD_NAME . ' ASC',
+                array_merge($contextparams, [$modifiedfrom]));
     }
 
     /**
index 8579c07..52d9034 100644 (file)
@@ -90,7 +90,7 @@ abstract class base_block extends base {
     }
 
     /**
-     * Gets recordset of all records modified since given time.
+     * Gets recordset of all blocks of this type modified since given time within the given context.
      *
      * See base class for detailed requirements. This implementation includes the key fields
      * from block_instances.
@@ -102,24 +102,30 @@ abstract class base_block extends base {
      * then you can override get_indexing_restrictions; by default this excludes rows with empty
      * configdata.
      *
-     * @param int $modifiedfrom Modified from time (>= this)
+     * @param int $modifiedfrom Return only records modified after this date
+     * @param \context|null $context Context to find blocks within
+     * @return false|\moodle_recordset|null
      */
-    public function get_recordset_by_timestamp($modifiedfrom = 0) {
+    public function get_document_recordset($modifiedfrom = 0, \context $context = null) {
         global $DB;
+
+        // Get context restrictions.
+        list ($contextjoin, $contextparams) = $this->get_context_restriction_sql($context, 'bi');
+
+        // Get custom restrictions for block type.
         list ($restrictions, $restrictionparams) = $this->get_indexing_restrictions();
         if ($restrictions) {
             $restrictions = 'AND ' . $restrictions;
         }
 
-        // Query for all entries in block_instances for this type of block, which were modified
-        // since the given date. Also find the course or module where the block is located.
-        // (Although this query supports both module and course context, currently only two page
-        // types are supported, which will both be at course context. The module support is present
-        // in case of extension to other page types later.)
+        // Query for all entries in block_instances for this type of block, within the specified
+        // context. The query is based on the one from get_recordset_by_timestamp and applies the
+        // same restrictions.
         return $DB->get_recordset_sql("
                 SELECT bi.id, bi.timemodified, bi.timecreated, bi.configdata,
                        c.id AS courseid, x.id AS contextid
                   FROM {block_instances} bi
+                       $contextjoin
                   JOIN {context} x ON x.instanceid = bi.id AND x.contextlevel = ?
                   JOIN {context} parent ON parent.id = bi.parentcontextid
              LEFT JOIN {course_modules} cm ON cm.id = parent.instanceid AND parent.contextlevel = ?
@@ -131,8 +137,9 @@ abstract class base_block extends base {
                            OR bi.pagetypepattern IN ('site-index', 'course-*', '*')))
                        $restrictions
               ORDER BY bi.timemodified ASC",
-                array_merge([CONTEXT_BLOCK, CONTEXT_MODULE, CONTEXT_COURSE, $modifiedfrom,
-                $this->get_block_name(), CONTEXT_COURSE, 'course-view-%'], $restrictionparams));
+                array_merge($contextparams, [CONTEXT_BLOCK, CONTEXT_MODULE, CONTEXT_COURSE,
+                    $modifiedfrom, $this->get_block_name(), CONTEXT_COURSE, 'course-view-%'],
+                $restrictionparams));
     }
 
     public function get_doc_url(\core_search\document $doc) {
@@ -259,4 +266,81 @@ abstract class base_block extends base {
         \cache::make_from_params(\cache_store::MODE_REQUEST, 'core_search',
                 self::CACHE_INSTANCES, [], ['simplekeys' => true])->purge();
     }
+
+    /**
+     * Helper function that gets SQL useful for restricting a search query given a passed-in
+     * context.
+     *
+     * The SQL returned will be one or more JOIN statements, surrounded by whitespace, which act
+     * as restrictions on the query based on the rows in the block_instances table.
+     *
+     * We assume the block instances have already been restricted by blockname.
+     *
+     * Returns null if there can be no results for this block within this context.
+     *
+     * If named parameters are used, these will be named gcrs0, gcrs1, etc. The table aliases used
+     * in SQL also all begin with gcrs, to avoid conflicts.
+     *
+     * @param \context|null $context Context to restrict the query
+     * @param string $blocktable Alias of block_instances table
+     * @param int $paramtype Type of SQL parameters to use (default question mark)
+     * @return array Array with SQL and parameters
+     * @throws \coding_exception If called with invalid params
+     */
+    protected function get_context_restriction_sql(\context $context = null, $blocktable = 'bi',
+            $paramtype = SQL_PARAMS_QM) {
+        global $DB;
+
+        if (!$context) {
+            return ['', []];
+        }
+
+        switch ($paramtype) {
+            case SQL_PARAMS_QM:
+                $param1 = '?';
+                $param2 = '?';
+                $key1 = 0;
+                $key2 = 1;
+                break;
+            case SQL_PARAMS_NAMED:
+                $param1 = ':gcrs0';
+                $param2 = ':gcrs1';
+                $key1 = 'gcrs0';
+                $key2 = 'gcrs1';
+                break;
+            default:
+                throw new \coding_exception('Unexpected $paramtype: ' . $paramtype);
+        }
+
+        $params = [];
+        switch ($context->contextlevel) {
+            case CONTEXT_SYSTEM:
+                $sql = '';
+                break;
+
+            case CONTEXT_COURSECAT:
+            case CONTEXT_COURSE:
+            case CONTEXT_MODULE:
+            case CONTEXT_USER:
+                // Find all blocks whose parent is within the specified context.
+                $sql = " JOIN {context} gcrsx ON gcrsx.id = $blocktable.parentcontextid
+                              AND (gcrsx.id = $param1 OR " . $DB->sql_like('gcrsx.path', $param2) . ") ";
+                $params[$key1] = $context->id;
+                $params[$key2] = $context->path . '/%';
+                break;
+
+            case CONTEXT_BLOCK:
+                // Find only the specified block of this type. Since we are generating JOINs
+                // here, we do this by joining again to the block_instances table with the same ID.
+                $sql = " JOIN {block_instances} gcrsbi ON gcrsbi.id = $blocktable.id
+                              AND gcrsbi.id = $param1 ";
+                $params[$key1] = $context->instanceid;
+                break;
+
+            default:
+                throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel);
+        }
+
+        return [$sql, $params];
+    }
 }
index bf57047..ef38750 100644 (file)
@@ -86,4 +86,110 @@ abstract class base_mod extends base {
         throw new \dml_missing_record_exception($modulename);
     }
 
+    /**
+     * Helper function that gets SQL useful for restricting a search query given a passed-in
+     * context.
+     *
+     * The SQL returned will be zero or more JOIN statements, surrounded by whitespace, which act
+     * as restrictions on the query based on the rows in a module table.
+     *
+     * You can pass in a null or system context, which will both return an empty string and no
+     * params.
+     *
+     * Returns an array with two nulls if there can be no results for the activity within this
+     * context (e.g. it is a block context).
+     *
+     * If named parameters are used, these will be named gcrs0, gcrs1, etc. The table aliases used
+     * in SQL also all begin with gcrs, to avoid conflicts.
+     *
+     * @param \context|null $context Context to restrict the query
+     * @param string $modname Name of module e.g. 'forum'
+     * @param string $modtable Alias of table containing module id
+     * @param int $paramtype Type of SQL parameters to use (default question mark)
+     * @return array Array with SQL and parameters; both null if no need to query
+     * @throws \coding_exception If called with invalid params
+     */
+    protected function get_context_restriction_sql(\context $context = null, $modname, $modtable,
+            $paramtype = SQL_PARAMS_QM) {
+        global $DB;
+
+        if (!$context) {
+            return ['', []];
+        }
+
+        switch ($paramtype) {
+            case SQL_PARAMS_QM:
+                $param1 = '?';
+                $param2 = '?';
+                $param3 = '?';
+                $key1 = 0;
+                $key2 = 1;
+                $key3 = 2;
+                break;
+            case SQL_PARAMS_NAMED:
+                $param1 = ':gcrs0';
+                $param2 = ':gcrs1';
+                $param3 = ':gcrs2';
+                $key1 = 'gcrs0';
+                $key2 = 'gcrs1';
+                $key3 = 'gcrs2';
+                break;
+            default:
+                throw new \coding_exception('Unexpected $paramtype: ' . $paramtype);
+        }
+
+        $params = [];
+        switch ($context->contextlevel) {
+            case CONTEXT_SYSTEM:
+                $sql = '';
+                break;
+
+            case CONTEXT_COURSECAT:
+                // Find all activities of this type within the specified category or any
+                // sub-category.
+                $pathmatch = $DB->sql_like('gcrscc2.path', $DB->sql_concat('gcrscc1.path', $param3));
+                $sql = " JOIN {course_modules} gcrscm ON gcrscm.instance = $modtable.id
+                              AND gcrscm.module = (SELECT id FROM {modules} WHERE name = $param1)
+                         JOIN {course} gcrsc ON gcrsc.id = gcrscm.course
+                         JOIN {course_categories} gcrscc1 ON gcrscc1.id = $param2
+                         JOIN {course_categories} gcrscc2 ON gcrscc2.id = gcrsc.category AND
+                              (gcrscc2.id = gcrscc1.id OR $pathmatch) ";
+                $params[$key1] = $modname;
+                $params[$key2] = $context->instanceid;
+                // Note: This param is a bit annoying as it obviously never changes, but sql_like
+                // throws a debug warning if you pass it anything with quotes in, so it has to be
+                // a bound parameter.
+                $params[$key3] = '/%';
+                break;
+
+            case CONTEXT_COURSE:
+                // Find all activities of this type within the course.
+                $sql = " JOIN {course_modules} gcrscm ON gcrscm.instance = $modtable.id
+                              AND gcrscm.course = $param1
+                              AND gcrscm.module = (SELECT id FROM {modules} WHERE name = $param2) ";
+                $params[$key1] = $context->instanceid;
+                $params[$key2] = $modname;
+                break;
+
+            case CONTEXT_MODULE:
+                // Find only the specified activity of this type.
+                $sql = " JOIN {course_modules} gcrscm ON gcrscm.instance = $modtable.id
+                              AND gcrscm.id = $param1
+                              AND gcrscm.module = (SELECT id FROM {modules} WHERE name = $param2) ";
+                $params[$key1] = $context->instanceid;
+                $params[$key2] = $modname;
+                break;
+
+            case CONTEXT_BLOCK:
+            case CONTEXT_USER:
+                // These contexts cannot contain any activities, so return null.
+                return [null, null];
+
+            default:
+                throw new \coding_exception('Unexpected contextlevel: ' . $context->contextlevel);
+        }
+
+        return [$sql, $params];
+    }
+
 }
index 1d255d2..7b40c21 100644 (file)
@@ -51,7 +51,14 @@ class search_base_activity_testcase extends advanced_testcase {
      */
     protected $engine = null;
 
+    /** @var context[] Array of test contexts */
+    protected $contexts;
+
+    /** @var stdClass[] Array of test forum objects */
+    protected $forums;
+
     public function setUp() {
+        global $DB;
         $this->resetAfterTest();
         set_config('enableglobalsearch', true);
 
@@ -60,6 +67,39 @@ class search_base_activity_testcase extends advanced_testcase {
 
         $this->generator = self::getDataGenerator()->get_plugin_generator('core_search');
         $this->generator->setup();
+
+        $this->setAdminUser();
+
+        // Create course and 2 forums.
+        $generator = $this->getDataGenerator();
+        $course = $generator->create_course();
+        $this->contexts['c1'] = \context_course::instance($course->id);
+        $this->forums[1] = $generator->create_module('forum', ['course' => $course->id, 'name' => 'Forum 1',
+                'intro' => '<p>Intro 1</p>', 'introformat' => FORMAT_HTML]);
+        $this->contexts['f1'] = \context_module::instance($this->forums[1]->cmid);
+        $this->forums[2] = $generator->create_module('forum', ['course' => $course->id, 'name' => 'Forum 2',
+                'intro' => '<p>Intro 2</p>', 'introformat' => FORMAT_HTML]);
+        $this->contexts['f2'] = \context_module::instance($this->forums[2]->cmid);
+
+        // Create another 2 courses (in same category and in a new category) with one forum each.
+        $this->contexts['cc1']  = \context_coursecat::instance($course->category);
+        $course2 = $generator->create_course();
+        $this->contexts['c2'] = \context_course::instance($course2->id);
+        $this->forums[3] = $generator->create_module('forum', ['course' => $course2->id, 'name' => 'Forum 3',
+                'intro' => '<p>Intro 3</p>', 'introformat' => FORMAT_HTML]);
+        $this->contexts['f3'] = \context_module::instance($this->forums[3]->cmid);
+        $cat2 = $generator->create_category();
+        $this->contexts['cc2'] = \context_coursecat::instance($cat2->id);
+        $course3 = $generator->create_course(['category' => $cat2->id]);
+        $this->contexts['c3'] = \context_course::instance($course3->id);
+        $this->forums[4] = $generator->create_module('forum', ['course' => $course3->id, 'name' => 'Forum 4',
+                'intro' => '<p>Intro 4</p>', 'introformat' => FORMAT_HTML]);
+        $this->contexts['f4'] = \context_module::instance($this->forums[4]->cmid);
+
+        // Hack about with the time modified values.
+        foreach ($this->forums as $index => $forum) {
+            $DB->set_field('forum', 'timemodified', $index, ['id' => $forum->id]);
+        }
     }
 
     public function tearDown() {
@@ -138,4 +178,160 @@ class search_base_activity_testcase extends advanced_testcase {
         $this->assertEquals(1, count($files));
         $this->assertEquals($content, $file->get_content());
     }
+
+    /**
+     * Tests getting the recordset.
+     */
+    public function test_get_document_recordset() {
+        global $USER, $DB;
+
+        // Get all the forums to index (no restriction).
+        $area = new mod_forum\search\activity();
+        $results = self::recordset_to_indexed_array($area->get_document_recordset());
+
+        // Should return all forums.
+        $this->assertCount(4, $results);
+
+        // Each result should basically have the contents of the forum table. We'll just check
+        // the key fields for the first one and then the other ones by id only.
+        $this->assertEquals($this->forums[1]->id, $results[0]->id);
+        $this->assertEquals(1, $results[0]->timemodified);
+        $this->assertEquals($this->forums[1]->course, $results[0]->course);
+        $this->assertEquals('Forum 1', $results[0]->name);
+        $this->assertEquals('<p>Intro 1</p>', $results[0]->intro);
+        $this->assertEquals(FORMAT_HTML, $results[0]->introformat);
+
+        $allids = self::records_to_ids($this->forums);
+        $this->assertEquals($allids, self::records_to_ids($results));
+
+        // Repeat with a time restriction.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(3));
+        $this->assertEquals([$this->forums[3]->id, $this->forums[4]->id],
+                self::records_to_ids($results));
+
+        // Now use context restrictions. First, the whole site (no change).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, context_system::instance()));
+        $this->assertEquals($allids, self::records_to_ids($results));
+
+        // Course 1 only.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $this->contexts['c1']));
+        $this->assertEquals([$this->forums[1]->id, $this->forums[2]->id],
+                self::records_to_ids($results));
+
+        // Course 2 only.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $this->contexts['c2']));
+        $this->assertEquals([$this->forums[3]->id], self::records_to_ids($results));
+
+        // Specific forum only.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $this->contexts['f4']));
+        $this->assertEquals([$this->forums[4]->id], self::records_to_ids($results));
+
+        // Category 1 context (courses 1 and 2).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $this->contexts['cc1']));
+        $this->assertEquals([$this->forums[1]->id, $this->forums[2]->id, $this->forums[3]->id],
+                self::records_to_ids($results));
+
+        // Category 2 context (course 3).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $this->contexts['cc2']));
+        $this->assertEquals([$this->forums[4]->id], self::records_to_ids($results));
+
+        // Combine context restriction (category 1) with timemodified.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                2, $this->contexts['cc1']));
+        $this->assertEquals([$this->forums[2]->id, $this->forums[3]->id],
+                self::records_to_ids($results));
+
+        // Find an arbitrary block on the system to get a block context.
+        $blockid = array_values($DB->get_records('block_instances', null, 'id', 'id', 0, 1))[0]->id;
+        $blockcontext = context_block::instance($blockid);
+
+        // Block context (cannot return anything, so always null).
+        $this->assertNull($area->get_document_recordset(0, $blockcontext));
+
+        // User context (cannot return anything, so always null).
+        $usercontext = context_user::instance($USER->id);
+        $this->assertNull($area->get_document_recordset(0, $usercontext));
+    }
+
+    /**
+     * Utility function to convert recordset to array for testing.
+     *
+     * @param moodle_recordset $rs Recordset to convert
+     * @return array Array indexed by number (0, 1, 2, ...)
+     */
+    protected static function recordset_to_indexed_array(moodle_recordset $rs) {
+        $results = [];
+        foreach ($rs as $rec) {
+            $results[] = $rec;
+        }
+        $rs->close();
+        return $results;
+    }
+
+    /**
+     * Utility function to convert records to array of IDs.
+     *
+     * @param array $recs Records which should have an 'id' field
+     * @return array Array of ids
+     */
+    protected static function records_to_ids(array $recs) {
+        $ids = [];
+        foreach ($recs as $rec) {
+            $ids[] = $rec->id;
+        }
+        return $ids;
+    }
+
+    /**
+     * Tests the get_doc_url function.
+     */
+    public function test_get_doc_url() {
+        $area = new mod_forum\search\activity();
+        $results = self::recordset_to_indexed_array($area->get_document_recordset());
+
+        for ($i = 0; $i < 4; $i++) {
+            $this->assertEquals(new moodle_url('/mod/forum/view.php',
+                    ['id' => $this->forums[$i + 1]->cmid]),
+                    $area->get_doc_url($area->get_document($results[$i])));
+        }
+    }
+
+    /**
+     * Tests the check_access function.
+     */
+    public function test_check_access() {
+        global $CFG;
+        require_once($CFG->dirroot . '/course/lib.php');
+
+        // Create a test user who can access courses 1 and 2 (everything except forum 4).
+        $generator = $this->getDataGenerator();
+        $user = $generator->create_user();
+        $generator->enrol_user($user->id, $this->forums[1]->course, 'student');
+        $generator->enrol_user($user->id, $this->forums[3]->course, 'student');
+        $this->setUser($user);
+
+        // Delete forum 2 and set forum 3 hidden.
+        course_delete_module($this->forums[2]->cmid);
+        set_coursemodule_visible($this->forums[3]->cmid, 0);
+
+        // Call check access on all the first three.
+        $area = new mod_forum\search\activity();
+        $this->assertEquals(\core_search\manager::ACCESS_GRANTED, $area->check_access(
+                $this->forums[1]->id));
+        $this->assertEquals(\core_search\manager::ACCESS_DELETED, $area->check_access(
+                $this->forums[2]->id));
+        $this->assertEquals(\core_search\manager::ACCESS_DENIED, $area->check_access(
+                $this->forums[3]->id));
+
+        // Note: Do not check forum 4 which is in a course the user can't access; this will return
+        // ACCESS_GRANTED, but it does not matter because the search engine will not have included
+        // that context in the list to search. (This is because the $cm->uservisible access flag
+        // is only valid if the user is known to be able to access the course.)
+    }
 }
index 1fc34a5..89f97ef 100644 (file)
@@ -46,10 +46,11 @@ class base_block_testcase extends advanced_testcase {
     /**
      * Tests getting the recordset.
      */
-    public function test_get_recordset_by_timestamp() {
-        global $DB;
+    public function test_get_document_recordset() {
+        global $DB, $USER;
 
         $this->resetAfterTest();
+        $this->setAdminUser();
 
         // Create course and activity module.
         $generator = $this->getDataGenerator();
@@ -58,6 +59,15 @@ class base_block_testcase extends advanced_testcase {
         $page = $generator->create_module('page', ['course' => $course->id]);
         $pagecontext = \context_module::instance($page->cmid);
 
+        // Create another 2 courses (in same category and in a new category).
+        $cat1context = \context_coursecat::instance($course->category);
+        $course2 = $generator->create_course();
+        $course2context = \context_course::instance($course2->id);
+        $cat2 = $generator->create_category();
+        $cat2context = \context_coursecat::instance($cat2->id);
+        $course3 = $generator->create_course(['category' => $cat2->id]);
+        $course3context = \context_course::instance($course3->id);
+
         // Add blocks by hacking table (because it's not a real block type).
 
         // 1. Block on course page.
@@ -113,17 +123,24 @@ class base_block_testcase extends advanced_testcase {
         $block7id = $DB->insert_record('block_instances', $instance);
         \context_block::instance($block7id);
 
+        // 8. Block on course 2.
+        $instance->parentcontextid = $course2context->id;
+        $instance->timemodified = 8;
+        $block8id = $DB->insert_record('block_instances', $instance);
+        \context_block::instance($block8id);
+
+        // 9. Block on course 3.
+        $instance->parentcontextid = $course3context->id;
+        $instance->timemodified = 9;
+        $block9id = $DB->insert_record('block_instances', $instance);
+        \context_block::instance($block9id);
+
         // Get all the blocks.
         $area = new block_mockblock\search\area();
-        $rs = $area->get_recordset_by_timestamp();
-        $results = [];
-        foreach ($rs as $rec) {
-            $results[] = $rec;
-        }
-        $rs->close();
+        $results = self::recordset_to_indexed_array($area->get_document_recordset());
 
-        // Only blocks 1, 3, 6, and 7 should be returned. Check all the fields for the first two.
-        $this->assertCount(4, $results);
+        // Only blocks 1, 3, 6, 7, 8, 9 should be returned. Check all the fields for the first two.
+        $this->assertCount(6, $results);
 
         $this->assertEquals($block1id, $results[0]->id);
         $this->assertEquals(1, $results[0]->timemodified);
@@ -142,20 +159,93 @@ class base_block_testcase extends advanced_testcase {
         // For the later ones, just check it got the right ones!
         $this->assertEquals($block6id, $results[2]->id);
         $this->assertEquals($block7id, $results[3]->id);
+        $this->assertEquals($block8id, $results[4]->id);
+        $this->assertEquals($block9id, $results[5]->id);
 
         // Repeat with a time restriction.
-        $rs = $area->get_recordset_by_timestamp(2);
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(2));
+
+        // Only block 3, 6, 7, 8, and 9 are returned.
+        $this->assertEquals([$block3id, $block6id, $block7id, $block8id, $block9id],
+                self::records_to_ids($results));
+
+        // Now use context restrictions. First, the whole site (no change).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, context_system::instance()));
+        $this->assertEquals([$block1id, $block3id, $block6id, $block7id, $block8id, $block9id],
+                self::records_to_ids($results));
+
+        // Course page only (leave out the one on site page and other courses).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $coursecontext));
+        $this->assertEquals([$block1id, $block6id, $block7id],
+                self::records_to_ids($results));
+
+        // Other course page only.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $course2context));
+        $this->assertEquals([$block8id], self::records_to_ids($results));
+
+        // Activity module only (no results).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $pagecontext));
+        $this->assertCount(0, $results);
+
+        // Specific block context.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $block3context));
+        $this->assertEquals([$block3id], self::records_to_ids($results));
+
+        // User context (no results).
+        $usercontext = context_user::instance($USER->id);
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $usercontext));
+        $this->assertCount(0, $results);
+
+        // Category 1 context (courses 1 and 2).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $cat1context));
+        $this->assertEquals([$block1id, $block6id, $block7id, $block8id],
+                self::records_to_ids($results));
+
+        // Category 2 context (course 3).
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                0, $cat2context));
+        $this->assertEquals([$block9id], self::records_to_ids($results));
+
+        // Combine context restriction (category 1) with timemodified.
+        $results = self::recordset_to_indexed_array($area->get_document_recordset(
+                7, $cat1context));
+        $this->assertEquals([$block7id, $block8id], self::records_to_ids($results));
+    }
+
+    /**
+     * Utility function to convert recordset to array for testing.
+     *
+     * @param moodle_recordset $rs Recordset to convert
+     * @return array Array indexed by number (0, 1, 2, ...)
+     */
+    protected static function recordset_to_indexed_array(moodle_recordset $rs) {
         $results = [];
         foreach ($rs as $rec) {
             $results[] = $rec;
         }
         $rs->close();
+        return $results;
+    }
 
-        // Only block 3, 6, and 7 are returned.
-        $this->assertCount(3, $results);
-        $this->assertEquals($block3id, $results[0]->id);
-        $this->assertEquals($block6id, $results[1]->id);
-        $this->assertEquals($block7id, $results[2]->id);
+    /**
+     * Utility function to convert records to array of IDs.
+     *
+     * @param array $recs Records which should have an 'id' field
+     * @return array Array of ids
+     */
+    protected static function records_to_ids(array $recs) {
+        $ids = [];
+        foreach ($recs as $rec) {
+            $ids[] = $rec->id;
+        }
+        return $ids;
     }
 
     /**
index 5a84158..ba4a4f8 100644 (file)
@@ -7,6 +7,13 @@ information provided here is intended especially for developers.
   this to work, search engine plugins will need to implement the 'stopat' parameter if they
   override the add_documents() function, and return an extra parameter from this function (see base
   class in engine.php). Unmodified plugins will still work, but without supporting time limits.
+* Search areas should now implement the get_document_recordset function instead of the old
+  get_recordset_by_timestamp API (implement both if the area should work in older Moodle versions
+  as well). The new function is the same as the old one, but has an additional context parameter.
+  There is a helper function get_context_restriction_sql to make this easy to implement; see code
+  in base_activity.php for an example of how to implement this in your search area. (The
+  change was required to make search work after restoring sites. It also allows more flexible
+  reindexing in other cases.)
 
 === 3.2 ===