FIX Index configuration overrides DO subsite field if NULL value

silverstripe · Sep 8, 2022 · 18af2bd · 18af2bd
1 parent b1ed1e9
commit 18af2bd
Show file tree

Hide file tree

Showing 6 changed files with 183 additions and 11 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -0,0 +1,35 @@
+language: php
+
+dist: trusty
+
+env:
+  global:
+    - COMPOSER_ROOT_VERSION=2.x-dev
+    - APP_SEARCH_API_KEY='test'
+    - APP_SEARCH_ENDPOINT='test'
+
+matrix:
+  include:
+    - php: 7.2
+      env: DB=MYSQL PHPUNIT_TEST=1 PHPCS_TEST=1
+    - php: 7.3
+      env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1
+
+before_script:
+  # Init PHP
+  - phpenv rehash
+  - phpenv config-rm xdebug.ini
+  - composer validate
+  - composer require silverstripe/recipe-cms 4.5.x-dev --no-update
+  - composer require silverstripe/subsites 2.4.x-dev --no-update
+  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi
+  - if [[ $PHPCS_TEST ]]; then composer global require squizlabs/php_codesniffer:^3 --prefer-dist --no-interaction --no-progress --no-suggest -o; fi
+  - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile
+
+script:
+  - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit tests/; fi
+  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi
+  - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs src/ tests/ ; fi
+
+after_success:
+  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi
diff --git a/docs/en/configuration.md b/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
@@ -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)
diff --git a/src/Extensions/Subsites/IndexConfigurationExtension.php b/src/Extensions/Subsites/IndexConfigurationExtension.php
@@ -17,11 +17,26 @@ public function updateIndexesForDocument(DocumentInterface $doc, array &$indexes
             $docSubsiteId = $doc->getDataObject()->SubsiteID ?? null;
         }
 
-        foreach ($indexes as $indexName => $data) {
-            $subsiteId = $data['subsite_id'] ?? 'all';
+        if ($docSubsiteId === null) {
+            // 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.
+            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;
+                }
+            }
+        } else {
+            foreach ($indexes as $indexName => $data) {
+                $subsiteId = $data['subsite_id'] ?? 'all';
 
-            if ($subsiteId !== 'all' && $docSubsiteId !== $subsiteId) {
-                unset($indexes[$indexName]);
+                if ($subsiteId !== 'all' && $docSubsiteId !== $subsiteId) {
+                    unset($indexes[$indexName]);
+                }
             }
         }
     }

diff --git a/src/Extensions/Subsites/SearchAdminExtension.php b/src/Extensions/Subsites/SearchAdminExtension.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\SearchService\Extensions\Subsites;
 
+use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Extension;
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataQuery;
@@ -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 (in_array(Subsite::class, Config::inst()->get($query->dataClass(), 'has_one'))) {
+                $query->where("SubsiteID IS NULL OR SubsiteID = {$data['subsite_id']}");
+            }
         }
     }
 

diff --git a/tests/DataObject/DataObjectDocumentTest.php b/tests/DataObject/DataObjectDocumentTest.php
@@ -15,6 +15,7 @@
 use SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned;
 use SilverStripe\SearchService\Tests\Fake\DataObjectSubclassFake;
 use SilverStripe\SearchService\Tests\Fake\ImageFake;
+use SilverStripe\SearchService\Tests\Fake\SubsiteDataObjectFake;
 use SilverStripe\SearchService\Tests\Fake\TagFake;
 use SilverStripe\SearchService\Tests\SearchServiceTest;
 use SilverStripe\Security\Member;
@@ -29,6 +30,7 @@ class DataObjectDocumentTest extends SearchServiceTest
      * @var array
      */
     protected static $extra_dataobjects = [
+        SubsiteDataObjectFake::class,
         DataObjectFake::class,
         TagFake::class,
         ImageFake::class,
@@ -95,6 +97,83 @@ public function testShouldIndex(): void
         $this->assertFalse($docThree->shouldIndex());
     }
 
+    public function testSubsiteDataObjectShouldIndex(): void
+    {
+        $config = $this->mockConfig();
+
+        $config->set(
+            'indexes',
+            [
+                'index0' => [
+                    'subsite_id' => 0,
+                    'includeClasses' => [
+                        SubsiteDataObjectFake::class => true
+                    ]
+                ],
+                'index1' => [
+                    'subsite_id' => 1,
+                    'includeClasses' => [
+                        DataObjectFake::class => true
+                    ]
+                ],
+            ]
+        );
+
+        $dataobject1 = new SubsiteDataObjectFake([
+            'ID' => 6,
+            'ShowInSearch' => true,
+            'SubsiteID' => 2,
+        ]);
+        $dataobject1->write();
+        $dataobject1->can_view = true;
+
+        $doc1 = DataObjectDocument::create($dataobject1);
+
+        // Incorrect SubsiteID for DO so should fail
+        $this->assertFalse($doc1->shouldIndex());
+
+        $dataobject2 = new SubsiteDataObjectFake([
+            'ID' => 7,
+            'ShowInSearch' => true,
+            'SubsiteID' => null,
+        ]);
+        $dataobject2->write();
+        $dataobject2->can_view = true;
+
+        $doc2 = DataObjectDocument::create($dataobject2);
+
+        // SubsiteID is NULL for DO but explicitly defined in config so should be true.
+        $this->assertNull($dataobject2->SubsiteID);
+        $this->assertTrue($doc2->shouldIndex());
+
+        $dataobject3 = new SubsiteDataObjectFake([
+            'ID' => 7,
+            'ShowInSearch' => true,
+            'SubsiteID' => 0,
+        ]);
+        $dataobject3->write();
+        $dataobject3->can_view = true;
+
+        $doc3 = DataObjectDocument::create($dataobject3);
+
+        // SubsiteID is correct on DO and as per config so should be true.
+        $this->assertEquals(0, $dataobject3->SubsiteID);
+        $this->assertTrue($doc3->shouldIndex());
+
+        $dataobject4 = new DataObjectFake([
+            'ID' => 8,
+            'ShowInSearch' => true,
+        ]);
+        $dataobject4->write();
+        $dataobject4->can_view = true;
+
+        $doc4 = DataObjectDocument::create($dataobject3);
+
+        // Check DataObjects which don't have a Subsite relation will still be indexed
+        $this->assertNull($dataobject4->SubsiteID);
+        $this->assertTrue($doc4->shouldIndex());
+    }
+
     public function testMarkIndexed(): void
     {
         $dataobject = new DataObjectFake(['ShowInSearch' => true]);

diff --git a/tests/Fake/SubsiteDataObjectFake.php b/tests/Fake/SubsiteDataObjectFake.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace SilverStripe\SearchService\Tests\Fake;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\SearchService\Extensions\SearchServiceExtension;
+
+class SubsiteDataObjectFake extends DataObject implements TestOnly
+{
+    private static $table_name = 'SubsiteDataObjectFake';
+
+    private static $extensions = [
+        SearchServiceExtension::class,
+    ];
+
+    public $can_view;
+
+    private static $has_one = array(
+        'Subsite' => 'Subsite'
+    );
+
+    private static $db = [
+        'ShowInSearch' => 'Boolean',
+    ];
+
+    public function canView($member = null)
+    {
+        if (is_callable($this->can_view)) {
+            $func = $this->can_view;
+            return $func();
+        }
+        return $this->can_view;
+    }
+}