Bugfix: Reindex job fix (CMS 6) (#60)

* Fix issue with ReindexJob

* Add missing use import

* Add PageFakeVersioned to extra_dataobjects

* Update to configurable sort and direction for fetching documents

* ISSUE-58: update CMS6 API use

* ISSUE-58: update docs

fixes #58 

---------

Co-authored-by: Bernie Hamlin <bernie@silverstripe.com>

Author: adunn49

docs/en/08_customising_more.md

@@ -68,6 +68,26 @@ happen as a result of a DataObject being ready to go into the index. It is invok
 
 This is an extension point on the IndexConfiguration class that allows updating what indexes a document is configured for
 
+## Customising the fetch sort order
+
+When reindexing, the `DataObjectFetcher` retrieves records in batches. By default, records are sorted by `ID` in ascending order to ensure consistent ordering across batches. You can customise the sort field and direction using configuration:
+
+```yaml
+SilverStripe\Forager\DataObject\DataObjectFetcher:
+  fetch_sort: 'LastEdited'
+  fetch_sort_direction: 'DESC'
+```
+
+| Option | Default | Description |
+|---|---|---|
+| `fetch_sort` | `ID` | The database column to sort by when fetching records for indexing |
+| `fetch_sort_direction` | `ASC` | The sort direction (`ASC` or `DESC`) |
+
+This can be useful when you want to prioritise recently edited content during a reindex, or when you need to sort by a specific field for consistency with an external system.
+
+> **Warning**
+> Avoid using non-deterministic sort fields such as the default `Sort` column (used by `SortOrder` or drag-and-drop ordering). These values are not guaranteed to be unique across records, which can lead to records being duplicated or skipped between batches during a reindex. Always use a column with unique, stable values (e.g. `ID`, `Created`, or `LastEdited`) to ensure reliable batch pagination.
+
 ## More information
 
 * [Adding a new search service](06_customising_add_search_service.md)

src/DataObject/DataObjectFetcher.php

@@ -23,6 +23,10 @@ class DataObjectFetcher implements DocumentFetcherInterface
 
     private int $offset = 0;
 
+    private static string $fetch_sort = 'ID';
+
+    private static string $fetch_sort_direction = 'ASC';
+
     public function __construct(string $class)
     {
         if (!is_subclass_of($class, DataObject::class)) {
@@ -74,7 +78,14 @@ public function incrementOffsetDown(): void
      */
     public function fetch(): array
     {
-        $list = $this->createDataList($this->getBatchSize(), $this->getOffset());
+        // get configurable sort options
+        $sortBy = static::config()->get('fetch_sort') ?? 'ID';
+        $sortDirection = static::config()->get('fetch_sort_direction') ?? 'ASC';
+
+        // sort (default by ID) to ensure consistent ordering across batches
+        $list = $this->createDataList($this->getBatchSize(), $this->getOffset())
+            ->sort($sortBy, $sortDirection);
+
         $docs = [];
 
         foreach ($list as $record) {

tests/DataObject/DataObjectFetcherTest.php

@@ -8,6 +8,7 @@
 use SilverStripe\Forager\Tests\Fake\DataObjectFake;
 use SilverStripe\Forager\Tests\Fake\DataObjectSubclassFake;
 use SilverStripe\Forager\Tests\Fake\DataObjectSubclassFakeShouldNotIndex;
+use SilverStripe\Forager\Tests\Fake\PageFakeVersioned;
 use SilverStripe\Forager\Tests\SearchServiceTestTrait;
 
 class DataObjectFetcherTest extends SapphireTest
@@ -23,6 +24,7 @@ class DataObjectFetcherTest extends SapphireTest
      */
     protected static $extra_dataobjects = [
         DataObjectFake::class,
+        PageFakeVersioned::class,
     ];
 
     public function testFetch(): void
@@ -63,6 +65,54 @@ public function testFetch(): void
         $this->assertCount(2, $documents);
     }
 
+    /**
+     * This tests that we fetch all documents when processed in batches.
+     */
+    public function testFetchBatch(): void
+    {
+        // create pages
+        $createPageCount = 100;
+
+        for ($i = 0; $i < $createPageCount; $i++) {
+            $dataobject = PageFakeVersioned::create();
+            $dataobject->Title = sprintf('FetchTestPage');
+            // added to verify that all pages are set regardless of the sort order
+            $dataobject->Sort = 1;
+            $dataobject->write();
+            $dataobject->publishSingle();
+        }
+
+        $batchSize = 10;
+        $fetcher = DataObjectFetcher::create(PageFakeVersioned::class);
+        $fetcher->setBatchSize($batchSize);
+        $totalDocuments = $fetcher->getTotalDocuments();
+
+        $fetchedDocumentCount = 0;
+        $fetchedDocumentIDs = [];
+
+        // keep fetching until we've fetched all documents, using the batch size and offset to get the next batch of
+        // documents each time
+        while ($fetchedDocumentCount < $totalDocuments) {
+            $fetcher->setOffset($fetchedDocumentCount);
+            $documents = $fetcher->fetch();
+
+            $fetchedDocumentCount += count($documents);
+
+            // collect all ids so that we can check everything has been fetched at the end of the test
+            $batchIDs = array_map(function (DataObjectDocument $document) {
+                return $document->getDataObject()->ID;
+            }, $documents);
+
+            $fetchedDocumentIDs = array_merge($fetchedDocumentIDs, array_values($batchIDs));
+        }
+
+        // only get unique ids so that we can check that all expected documents have been fetched
+        $fetchedDocumentIDs = array_unique($fetchedDocumentIDs);
+
+        // make sure we fetched all the documents
+        $this->assertCount($totalDocuments, $fetchedDocumentIDs);
+    }
+
     public function testTotalDocuments(): void
     {
         $fetcher = DataObjectFetcher::create(DataObjectFake::class);

tests/Fake/PageFakeVersioned.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace SilverStripe\Forager\Tests\Fake;
+
+use Page;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\Forager\Extensions\SearchServiceExtension;
+use SilverStripe\Versioned\Versioned;
+
+class PageFakeVersioned extends Page implements TestOnly
+{
+
+    private static string $table_name = 'PageFakeVersioned';
+
+    private static array $extensions = [
+        SearchServiceExtension::class,
+        Versioned::class,
+    ];
+
+}