diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4144045..635cb4f 100644 --- a/.github/workflows/main.yml +++ b/.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 diff --git a/docs/en/configuration.md b/docs/en/configuration.md index e6cf482..89d9a74 100644 --- 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,7 +193,7 @@ 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. @@ -201,7 +201,7 @@ 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,6 +248,9 @@ 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. @@ -255,5 +258,5 @@ replicated for other scenarios like languages if required. * [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/DataObject/DataObjectDocument.php b/src/DataObject/DataObjectDocument.php index 6d43004..31106aa 100644 --- a/src/DataObject/DataObjectDocument.php +++ b/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; diff --git a/src/Extensions/Subsites/IndexConfigurationExtension.php b/src/Extensions/Subsites/IndexConfigurationExtension.php index eeefa27..9da94b6 100644 --- a/src/Extensions/Subsites/IndexConfigurationExtension.php +++ b/src/Extensions/Subsites/IndexConfigurationExtension.php @@ -3,7 +3,7 @@ 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 @@ -11,16 +11,44 @@ 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]); } } diff --git a/src/Extensions/Subsites/SearchAdminExtension.php b/src/Extensions/Subsites/SearchAdminExtension.php index 7f342b6..b12ab3f 100644 --- a/src/Extensions/Subsites/SearchAdminExtension.php +++ b/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'])); + } } } diff --git a/src/Interfaces/DataObjectDocumentInterface.php b/src/Interfaces/DataObjectDocumentInterface.php new file mode 100644 index 0000000..f310e4d --- /dev/null +++ b/src/Interfaces/DataObjectDocumentInterface.php @@ -0,0 +1,15 @@ +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; + } + } diff --git a/tests/DataObject/DataObjectDocumentTest.php b/tests/DataObject/DataObjectDocumentTest.php index a297f88..8f9b5ed 100644 --- a/tests/DataObject/DataObjectDocumentTest.php +++ b/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]); diff --git a/tests/fixtures.yml b/tests/fixtures.yml index 976bd7c..4ee023f 100644 --- a/tests/fixtures.yml +++ b/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' diff --git a/tests/pages.yml b/tests/pages.yml index 96423ea..83999f5 100644 --- a/tests/pages.yml +++ b/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