Merge pull request #68 from silverstripeltd/feature/explicit-dataobje…

…ct-for-subsite

Index configuration overrides DO subsite field if NULL value

.github/workflows/main.yml

@@ -8,6 +8,7 @@ jobs:
   ci:
     uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1
     with:
+      composer_require_extra: silverstripe/subsites:^2.6
       dynamic_matrix: false
       extra_jobs: |
         - php: 8.0

docs/en/configuration.md

@@ -27,7 +27,7 @@ on your service provider. For EnterpriseSearch, it should only contain lowercase
 and hyphens.
 
 * `includedClasses`: A list of content classes to index. These are just the _source_ of the
-content, so they have no contractual bind to the module. If they are dataobjects, they 
+content, so they have no contractual bind to the module. If they are dataobjects, they
 should have the `SearchServiceExtension` applied, however. This is discussed further below.
 
 * `SilverStripe\CMS\Model\SiteTree`: This class already has the necessary extension applied
@@ -81,8 +81,8 @@ SilverStripe\SearchService\Service\IndexConfiguration:
             property: 'Comments.Author.Name'
 ```
 
-For DataObject content, the dot syntax allows traversal of relationships. If the final 
-property in the notation is on a list, it will use the `->column()` function to derive 
+For DataObject content, the dot syntax allows traversal of relationships. If the final
+property in the notation is on a list, it will use the `->column()` function to derive
 the values as an array.
 
 This will roughly get indexed as a structure like this:
@@ -193,15 +193,15 @@ SilverStripe\Core\Injector\Injector:
     constructor:
       index_variant: '`MY_CUSTOM_VAR`'
 
-``` 
+```
 
 This is useful if you have multiple staging environments and you don't want to overcrowd
 your search instance with distinct indexes for each one.
 
 ## Full page indexing
 
 Page and DataObject content is eligible for full-page indexing of its content. This is
-predicated upon the object having a `Link()` method defined that can be rendered in a 
+predicated upon the object having a `Link()` method defined that can be rendered in a
 controller.
 
 The content is extracted using an XPath selector. By default, this is `//main`, but it
@@ -236,7 +236,7 @@ SilverStripe\SearchService\Service\IndexConfiguration:
             summary:
               property: Summary
     content-subsite4:
-      subsite_id: 4
+      subsite_id: 4 # or you can use environment variable such as 'NAME_OF_ENVIRONMENT_VARIABLE'
       includeClasses:
         Page:
           <<: *page_defaults
@@ -248,12 +248,15 @@ SilverStripe\SearchService\Service\IndexConfiguration:
 Note the syntax to reduce the need for copy-paste if you want to duplicate the
 same configuration across.
 
+__Additional note__:
+> In the sample above, if the data object (My\Other\Class) does not have a subsite ID,  then it will be included in the indexing as it is explicitly defined in the index configuration
+
 This is handled via `SubsiteIndexConfigurationExtension` - this logic could be
 replicated for other scenarios like languages if required.
 
 ## More information
 
 * [Usage](usage.md)
 * [Implementations](implementations.md)
-* [Customising and extending](customising.md) 
+* [Customising and extending](customising.md)
 * [Overview and Rationale](overview.md)

src/DataObject/DataObjectDocument.php

@@ -21,6 +21,7 @@
 use SilverStripe\SearchService\Exception\IndexConfigurationException;
 use SilverStripe\SearchService\Extensions\DBFieldExtension;
 use SilverStripe\SearchService\Extensions\SearchServiceExtension;
+use SilverStripe\SearchService\Interfaces\DataObjectDocumentInterface;
 use SilverStripe\SearchService\Interfaces\DependencyTracker;
 use SilverStripe\SearchService\Interfaces\DocumentAddHandler;
 use SilverStripe\SearchService\Interfaces\DocumentInterface;
@@ -43,7 +44,8 @@ class DataObjectDocument implements
     DependencyTracker,
     DocumentRemoveHandler,
     DocumentAddHandler,
-    DocumentMetaProvider
+    DocumentMetaProvider,
+    DataObjectDocumentInterface
 {
 
     use Injectable;

src/Extensions/Subsites/IndexConfigurationExtension.php

@@ -3,24 +3,52 @@
 namespace SilverStripe\SearchService\Extensions\Subsites;
 
 use SilverStripe\Core\Extension;
-use SilverStripe\SearchService\DataObject\DataObjectDocument;
+use SilverStripe\SearchService\Interfaces\DataObjectDocumentInterface;
 use SilverStripe\SearchService\Interfaces\DocumentInterface;
 
 class IndexConfigurationExtension extends Extension
 {
 
     public function updateIndexesForDocument(DocumentInterface $doc, array &$indexes): void
     {
-        $docSubsiteId = null;
+        // Skip if document object does not implement DataObject interface
+        if (!$doc instanceof DataObjectDocumentInterface) {
+            return;
+        }
+
+        $docSubsiteId = $doc->getDataObject()->SubsiteID ?? 0;
 
-        if ($doc instanceof DataObjectDocument) {
-            $docSubsiteId = $doc->getDataObject()->SubsiteID ?? null;
+        if ((int) $docSubsiteId === 0) {
+            $this->updateDocumentWithoutSubsite($doc, $indexes);
+        } else {
+            $this->updateDocumentWithSubsite($indexes, $docSubsiteId);
         }
+    }
 
+    /**
+     * DataObject does not have a defined SubsiteID. So if the developer explicitly defined the dataObject to be
+     * included in the Subsite Index configuration then allow the dataObject to be added in.
+     */
+    protected function updateDocumentWithoutSubsite(DocumentInterface $doc, array &$indexes): void
+    {
+        foreach ($indexes as $indexName => $data) {
+            // DataObject explicitly defined on Subsite index definition
+            $explicitClasses = $data['includeClasses'] ?? [];
+
+            if (!isset($explicitClasses[$doc->getDataObject()->ClassName])) {
+                unset($indexes[$indexName]);
+
+                break;
+            }
+        }
+    }
+
+    protected function updateDocumentWithSubsite(array &$indexes, int $docSubsiteId): void
+    {
         foreach ($indexes as $indexName => $data) {
             $subsiteId = $data['subsite_id'] ?? 'all';
 
-            if ($subsiteId !== 'all' && $docSubsiteId !== $subsiteId) {
+            if ($subsiteId !== 'all' && $docSubsiteId !== (int)$subsiteId) {
                 unset($indexes[$indexName]);
             }
         }

src/Extensions/Subsites/SearchAdminExtension.php

@@ -4,6 +4,7 @@
 
 use SilverStripe\Core\Extension;
 use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataQuery;
 use SilverStripe\Subsites\Model\Subsite;
 
@@ -20,7 +21,11 @@ public function updateQuery(DataQuery $query, array $data): void
             }
 
             Subsite::disable_subsite_filter(true);
-            $query->where(sprintf('SubsiteID = %s', $data['subsite_id']));
+
+            // If the DataObject has a Subsite relation, then apply a SubsiteID filter
+            if (DataObject::getSchema()->hasOneComponent(Subsite::class, 'Subsite')) {
+                $query->where(sprintf('SubsiteID IS NULL OR SubsiteID = %d', $data['subsite_id']));
+            }
         }
     }
 

src/Interfaces/DataObjectDocumentInterface.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace SilverStripe\SearchService\Interfaces;
+
+use SilverStripe\ORM\DataObject;
+
+/**
+ * The contract to indicate that the Elastic Document sources its data from Silverstripe DataObject class
+ */
+interface DataObjectDocumentInterface
+{
+
+    public function getDataObject(): DataObject;
+
+}

src/Service/IndexConfiguration.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\SearchService\Service;
 
 use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Environment;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\SearchService\Interfaces\DocumentInterface;
@@ -95,6 +96,11 @@ public function getIndexes(): array
     {
         $indexes = $this->config()->get('indexes');
 
+        // Convert environment variable defined in YML config to its value
+        array_walk($indexes, function (array &$configuration): void {
+            $configuration = $this->environmentVariableToValue($configuration);
+        });
+
         if (!$this->onlyIndexes) {
             return $indexes;
         }
@@ -267,4 +273,26 @@ public function getFieldsForIndex(string $index): array
         return $fields;
     }
 
+    /**
+     * For every configuration item if value is environment variable then convert it to its value
+     */
+    protected function environmentVariableToValue(array $configuration): array
+    {
+        foreach ($configuration as $name => $value) {
+            if (!is_string($value)) {
+                continue;
+            }
+
+            $environmentValue = Environment::getEnv($value);
+
+            if (!$environmentValue) {
+                continue;
+            }
+
+            $configuration[$name] = $environmentValue;
+        }
+
+        return $configuration;
+    }
+
 }

tests/DataObject/DataObjectDocumentTest.php

@@ -19,6 +19,7 @@
 use SilverStripe\SearchService\Tests\Fake\TagFake;
 use SilverStripe\SearchService\Tests\SearchServiceTest;
 use SilverStripe\Security\Member;
+use SilverStripe\Subsites\Model\Subsite;
 
 class DataObjectDocumentTest extends SearchServiceTest
 {
@@ -175,6 +176,105 @@ public function testShouldIndexChild(): void
         $this->assertTrue($docOne->shouldIndex());
     }
 
+    public function testSubsiteDataObjectShouldIndex(): void
+    {
+        $subsite2 = $this->objFromFixture(Subsite::class, 'subsite2');
+
+        $config = $this->mockConfig();
+
+        // Mocked indexes:
+        //  - index0: allows all data without subsite filter
+        //  - index1: for subsite2 and Page class and Data object that does not implement subsite
+        //  - index2: for subsite2 and Data object that does not implement subsite
+        $config->set(
+            'indexes',
+            [
+                'index0' => [
+                    'subsite_id' => 0,
+                    'includeClasses' => [
+                        Page::class => true,
+                    ],
+                ],
+                'index1' => [
+                    'subsite_id' => $subsite2->ID,
+                    'includeClasses' => [
+                        Page::class => true,
+                        DataObjectFake::class => true,
+                        DataObjectFakeVersioned::class => true,
+                    ],
+                ],
+                'index2' => [
+                    'subsite_id' => $subsite2->ID,
+                    'includeClasses' => [
+                        DataObjectFake::class => true,
+                    ],
+                ],
+            ]
+        );
+
+        // Ensure page that belongs to a subsite is published
+        $page = $this->objFromFixture(Page::class, 'page6');
+        $page->publishRecursive();
+        $page = $this->objFromFixture(Page::class, 'page6');
+
+        // Prepare page for index
+        $docOne = DataObjectDocument::create($page);
+
+        // Assert that the page can't be indexed because we don't have an index with matching subsite ID
+        $this->assertFalse($docOne->shouldIndex());
+
+        // Remove the subsite ID
+        $page->update(['SubsiteID' => null])->publishRecursive();
+        $page = $this->objFromFixture(Page::class, 'page6');
+
+        // Prepare page for reindex
+        $doc2 = DataObjectDocument::create($page);
+
+        // Assert that the subsite ID removed from the page can be indexed because we explicitly defined the ClassName
+        // in index0 configuration
+        $this->assertNull($page->SubsiteID);
+        $this->assertTrue($doc2->shouldIndex());
+
+        // Update page subsite ID with correct ID
+        $page->update(['SubsiteID' => $subsite2->ID])->publishRecursive();
+        $page = $this->objFromFixture(Page::class, 'page6');
+
+        // Prepare page for reindex
+        $doc3 = DataObjectDocument::create($page);
+
+        // Assert that the page can be indexed in index1
+        $this->assertEquals($subsite2->ID, $page->SubsiteID);
+        $this->assertTrue($doc3->shouldIndex());
+
+        // Get an object without subsite filter
+        $object1 = $this->objFromFixture(DataObjectFake::class, 'one');
+
+        // Prepare object for index
+        $doc4 = DataObjectDocument::create($object1);
+
+        // Assert that the object without subsite ID and that it can be indexed, because configuration allows it
+        $this->assertNull($object1->SubsiteID);
+        $this->assertTrue($doc4->shouldIndex());
+        $this->assertArrayHasKey('index1', $doc4->getIndexes());
+        $this->assertArrayHasKey('index2', $doc4->getIndexes());
+        $this->assertArrayNotHasKey('index0', $doc4->getIndexes());
+
+        // Get an object without subsite filter
+        $object2 = $this->objFromFixture(DataObjectFakeVersioned::class, 'one');
+        $object2->publishRecursive();
+        $object2 = $this->objFromFixture(DataObjectFakeVersioned::class, 'one');
+
+        // Prepare object for index
+        $doc5 = DataObjectDocument::create($object2);
+
+        // Assert that the object without subsite ID and that it can be indexed, because configuration allows it
+        $this->assertNull($object2->SubsiteID);
+        $this->assertTrue($doc5->shouldIndex());
+        $this->assertArrayHasKey('index1', $doc5->getIndexes());
+        $this->assertArrayNotHasKey('index2', $doc5->getIndexes());
+        $this->assertArrayNotHasKey('index0', $doc5->getIndexes());
+    }
+
     public function testMarkIndexed(): void
     {
         $dataobject = new DataObjectFake(['ShowInSearch' => true]);

tests/fixtures.yml

@@ -58,3 +58,9 @@ SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned:
   two:
     Title: Dataobject two Versioned
     ShowInSearch: 0
+
+SilverStripe\Subsites\Model\Subsite:
+  subsite1:
+    Title: 'Subsite 1'
+  subsite2:
+    Title: 'Subsite 2'

tests/pages.yml

@@ -17,3 +17,7 @@ Page:
     Title: Child Page 3
     Parent: =>Page.page4
     ShowInSearch: 1
+  page6:
+    Title: Subsite Page 1
+    Subsite: =>SilverStripe\Subsites\Model\Subsite.subsite1
+    ShowInSearch: 1