Merge pull request #134 from marxjohnson/docoffset-310

Backport docoffset fix

Author: brendanheywood

classes/document.php

@@ -111,6 +111,13 @@ class document extends \core_search\document {
             ),
     );
 
+    /**
+     * @var mixed $config Search plugin configuration.
+     */
+    protected $config;
+
+    /** @var bool Is file indexing enabled? */
+    protected $fileindexing;
 
     /**
      * Constructor for document class.

classes/engine.php

@@ -830,10 +830,10 @@ public function execute_query($filters, $accessinfo, $limit = 0) {
                     $totalhits = $results->hits->total;
                 }
                 $docs = array_merge($docs, $this->compile_results($results, $limit));
-                $docoffest = count($docs);
+                $docoffest += count($results->hits->hits);
             }
 
-        } while ((count($docs) < $limit) && ($totalhits > \search_elastic\query::MAX_RESULTS));
+        } while ((count($docs) < $limit) && ($totalhits > \search_elastic\query::MAX_RESULTS) && ($docoffest < $totalhits));
 
         // TODO: handle negative cases and errors.
         return $docs;

classes/enrich/base/base_enrich.php

@@ -33,6 +33,11 @@
  */
 abstract class base_enrich {
 
+    /**
+     * @var mixed $config Search plugin configuration.
+     */
+    protected $config;
+
     /**
      * The constructor for the class, will be overwritten in most cases.
      *

tests/engine_test.php

@@ -32,6 +32,8 @@
 require_once($CFG->dirroot . '/search/engine/elastic/tests/fixtures/mock_search_area.php');
 require_once($CFG->dirroot . '/search/engine/elastic/tests/fixtures/testable_engine.php');
 
+use core_mocksearch\search\mock_boost_area;
+use core_mocksearch\search\mock_search_area;
 use \GuzzleHttp\Handler\MockHandler;
 use \GuzzleHttp\HandlerStack;
 use \GuzzleHttp\Psr7\Response;
@@ -60,6 +62,21 @@ class engine_test extends \advanced_testcase {
      */
     protected $engine = null;
 
+    /**
+     * @var string the Apache Lucene version of the attached Elasticsearch / OpenSearch service.
+     */
+    protected $luceneversion;
+
+    /**
+     * @var mock_search_area
+     */
+    protected $area;
+
+    /**
+     * @var mock_boost_area
+     */
+    protected $areaboost;
+
     public function setUp(): void {
         $this->resetAfterTest();
         set_config('enableglobalsearch', true);
@@ -1015,6 +1032,8 @@ public function test_search_wildcardstart_enabled() {
      * Test the wildcard search functionality when wildcardend is enabled.
      */
     public function test_search_wildcardend_enabled() {
+        $searchuser = $this->getDataGenerator()->create_user();
+        $this->setUser($searchuser);
         set_config('wildcardend', 1, 'search_elastic');
 
         // Construct the search object and add it to the engine.
@@ -1088,4 +1107,45 @@ public function test_broken_index() {
         $this->assertFalse($result);
     }
 
+    /** Test docoffset is incremented correctly for multiple pages of search results. */
+    public function test_execute_query_docoffset() {
+        $searchuser = $this->getDataGenerator()->create_user();
+        $this->setUser($searchuser);
+        // Generate 2000 indexed documents.
+        for ($i = 1, $j = 2000; $i <= $j; $i++) {
+            $rec = (object)['content' => 'test message ' . $i];
+            // Deny user access to all but 100 documents, spaced evenly throughout the index.
+            // The engine returns 1000 results at a time, so there should be 50 visible in the first set, and 50 in the second.
+            if ($i % 20 != 0) {
+                $rec->denyuserids = [$searchuser->id];
+            }
+            $area = $this->area;
+            $record = $this->generator->create_record($rec);
+            $doc = $area->get_document($record);
+            $this->engine->add_document($doc, false, $this->luceneversion);
+        }
+        // We need to wait for Elastic search to update its index
+        // this happens in near realtime, not immediately.
+        sleep(1);
+
+        $querydata = (object) [
+            'q' => '*',
+            'timestart' => 0,
+            'timeend' => 0,
+        ];
+
+        // Execute the search.
+        $results = $this->search->search($querydata);
+
+        $this->assertCount(100, $results);
+
+        // Confirm that each result is unique. This proves that we are not re-querying the same documents multiple times.
+        $seenresults = [];
+        foreach ($results as $result) {
+            $content = $result->get('content');
+            $this->assertFalse(in_array($content, $seenresults), "'{$content}' included multiple times in search results.");
+            $seenresults[] = $content;
+        }
+    }
+
 }

version.php

@@ -24,8 +24,8 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2023092000;
-$plugin->release   = '4.1 Build (2023012400)'; // Build same as version.
+$plugin->version   = 2023092001;
+$plugin->release   = '4.1 Build (2023092001)'; // Build same as version.
 $plugin->requires  = 2016052304;
 $plugin->component = 'search_elastic';
 $plugin->maturity  = MATURITY_STABLE;