From 0bede9cdaa27d4c77e8af622f4dcb9d6f4d7b1f3 Mon Sep 17 00:00:00 2001 From: Marco Hermo Date: Fri, 12 Apr 2024 13:27:44 +1200 Subject: [PATCH] FIX Unpublising parent pages should include child pages (#89) * FIX Unpublising parent pages should include child pages fixes Issue [#88](https://github.com/silverstripe/silverstripe-search-service/issues/88) Uses `SiteTree::enforce_strict_hierarchy` config in getting the dependent documents for indexing child pages. * Bugfix `DataObjectDocument::getDependentDocuments` to get dependencies via `has_one` relationships --------- Co-authored-by: Bernard Hamlin <948122+blueo@users.noreply.github.com> --- _config/extensions.yml | 3 +- phpunit.xml.dist | 28 +- src/DataObject/DataObjectBatchProcessor.php | 2 + src/DataObject/DataObjectDocument.php | 10 +- src/Extensions/SiteTreeHierarchyExtension.php | 27 ++ src/Jobs/RemoveDataObjectJob.php | 43 +- src/Service/Indexer.php | 2 + tests/DataObject/DataObjectDocumentTest.php | 36 +- .../SiteTreeHierarchyExtensionTest.php | 47 +++ tests/Fake/ImageFake.php | 8 + tests/Fake/PageFake.php | 31 ++ tests/Jobs/RemoveDataObjectJobTest.php | 35 ++ tests/Jobs/RemoveRelatedDataObjectJobTest.php | 393 ++++++++++++++++++ tests/pages.yml | 58 ++- 14 files changed, 693 insertions(+), 30 deletions(-) create mode 100644 src/Extensions/SiteTreeHierarchyExtension.php create mode 100644 tests/Extensions/SiteTreeHierarchyExtensionTest.php create mode 100644 tests/Fake/PageFake.php create mode 100644 tests/Jobs/RemoveRelatedDataObjectJobTest.php diff --git a/_config/extensions.yml b/_config/extensions.yml index 7b0a15ef..a453b3ee 100644 --- a/_config/extensions.yml +++ b/_config/extensions.yml @@ -26,4 +26,5 @@ Only: --- SilverStripe\CMS\Model\SiteTree: extensions: - - SilverStripe\SearchService\Extensions\SearchServiceExtension + SearchServiceExtension: SilverStripe\SearchService\Extensions\SearchServiceExtension + SiteTreeHierarchyExtension: SilverStripe\SearchService\Extensions\SiteTreeHierarchyExtension diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 745ac442..956c8ef1 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,13 +1,17 @@ - - - tests/ - - - - src/ - - tests/ - - - + + + + + src/ + + + tests/ + + + + tests/ + + + + diff --git a/src/DataObject/DataObjectBatchProcessor.php b/src/DataObject/DataObjectBatchProcessor.php index c2d13994..db0b12e5 100644 --- a/src/DataObject/DataObjectBatchProcessor.php +++ b/src/DataObject/DataObjectBatchProcessor.php @@ -31,6 +31,8 @@ public function removeDocuments(array $documents): array $this->run($job); foreach ($documents as $doc) { + // Indexer::METHOD_ADD as default parameter make sure we check first its related documents + // and decide whether we should delete or update them automatically. $childJob = RemoveDataObjectJob::create($doc, $timestamp); $this->run($childJob); } diff --git a/src/DataObject/DataObjectDocument.php b/src/DataObject/DataObjectDocument.php index 31106aa6..3d99458f 100644 --- a/src/DataObject/DataObjectDocument.php +++ b/src/DataObject/DataObjectDocument.php @@ -371,6 +371,12 @@ public function getFieldValue(Field $field): mixed } /** + * Collects documents that depend on the current DataObject for indexing. + * It will inspect the search index configuration for anything using this object in a field or, if the + * current object is an instance of SiteTree, it will respect `enforce_strict_hierarchy` + * and add any child objects. + * + * @see [the dependency tracking docs](docs/en/usage.md#dependency-tracking) * @return DocumentInterface[] */ public function getDependentDocuments(): array @@ -440,7 +446,7 @@ public function getDependentDocuments(): array /** @var DataObjectDocument $candidateDocument */ foreach ($chunker->chunk() as $candidateDocument) { - $relatedObj = $candidateDocument->getFieldValue($field); + $relatedObj = $candidateDocument->getFieldDependency($field); // Singleton returned a dataobject, but this record did not. Rare, but possible. if (!$relatedObj instanceof $objectClass) { @@ -448,7 +454,7 @@ public function getDependentDocuments(): array } if ($relatedObj->ID === $ownedDataObject->ID) { - $docs[$document->getIdentifier()] = $document; + $docs[$candidateDocument->getIdentifier()] = $candidateDocument; } } } diff --git a/src/Extensions/SiteTreeHierarchyExtension.php b/src/Extensions/SiteTreeHierarchyExtension.php new file mode 100644 index 00000000..8e6402f3 --- /dev/null +++ b/src/Extensions/SiteTreeHierarchyExtension.php @@ -0,0 +1,27 @@ +get('enforce_strict_hierarchy')) { + return; + } + + $page = $this->getOwner(); + + foreach ($page->AllChildren() as $record) { + $document = DataObjectDocument::create($record); + $dependentDocs[$document->getIdentifier()] = $document; + $dependentDocs = array_merge($dependentDocs, $document->getDependentDocuments()); + } + } + +} diff --git a/src/Jobs/RemoveDataObjectJob.php b/src/Jobs/RemoveDataObjectJob.php index 58a7daee..75024aad 100644 --- a/src/Jobs/RemoveDataObjectJob.php +++ b/src/Jobs/RemoveDataObjectJob.php @@ -11,6 +11,10 @@ use SilverStripe\Versioned\Versioned; /** + * By virture of the default parameter, Index::METHOD_ADD, this does not remove the documents straight away. + * It checks first the status of the underlaying DataObjects and decides whether to remove or add them to the index. + * Then pass on to the parent's process() method to handle the job. + * * @property DataObjectDocument|null $document * @property int|null $timestamp */ @@ -19,6 +23,8 @@ class RemoveDataObjectJob extends IndexJob public function __construct(?DataObjectDocument $document = null, ?int $timestamp = null, ?int $batchSize = null) { + // Indexer::METHOD_ADD as default parameter make sure we check first its related documents + // whether we should delete or update them automatically. parent::__construct([], Indexer::METHOD_ADD, $batchSize); if ($document !== null) { @@ -46,26 +52,43 @@ public function getTitle(): string */ public function setup(): void { - // Set the documents in setup to ensure async + /** @var DBDatetime $datetime - set the documents in setup to ensure async */ $datetime = DBField::create_field('Datetime', $this->getTimestamp()); $archiveDate = $datetime->format($datetime->getISOFormat()); $documents = Versioned::withVersionedMode(function () use ($archiveDate) { Versioned::reading_archived_date($archiveDate); + $currentDocument = $this->getDocument(); // Go back in time to find out what the owners were before unpublish - $dependentDocs = $this->document->getDependentDocuments(); + $dependentDocs = $currentDocument->getDependentDocuments(); // refetch everything on the live stage Versioned::set_stage(Versioned::LIVE); - return array_map(function (DataObjectDocument $doc) { - return DataObjectDocument::create( - DataObject::get_by_id( - $doc->getSourceClass(), - $doc->getDataObject()->ID - ) - ); - }, $dependentDocs); + return array_reduce( + $dependentDocs, + function (array $carry, DataObjectDocument $doc) { + $record = DataObject::get_by_id($doc->getSourceClass(), $doc->getDataObject()->ID); + + // Since SiteTree::onBeforeDelete recursively deletes the child pages, + // they end up not found on a live environment which breaks DataObjectDocument::_constructor + if ($record) { + $document = DataObjectDocument::create($record); + $carry[$document->getIdentifier()] = $document; + + return $carry; + } + + // Taking into account that this queued job has a reference of existing child pages + // We need to make sure that we are able to send these pages to ElasticSearch etc. for removal + $oldRecord = $doc->getDataObject(); + $document = DataObjectDocument::create($oldRecord); + $carry[$document->getIdentifier()] = $document; + + return $carry; + }, + [] + ); }); $this->setDocuments($documents); diff --git a/src/Service/Indexer.php b/src/Service/Indexer.php index b46f3a36..9ea719e8 100644 --- a/src/Service/Indexer.php +++ b/src/Service/Indexer.php @@ -124,6 +124,8 @@ function (DocumentInterface $dependentDocument) use ($document) { ); if ($dependentDocs) { + // Indexer::METHOD_ADD as default parameter make sure we check first its related documents + // and decide whether we should delete or update them automatically. $child = Indexer::create($dependentDocs, self::METHOD_ADD, $this->getBatchSize()); while (!$child->finished()) { diff --git a/tests/DataObject/DataObjectDocumentTest.php b/tests/DataObject/DataObjectDocumentTest.php index 8f9b5edf..0d0a7291 100644 --- a/tests/DataObject/DataObjectDocumentTest.php +++ b/tests/DataObject/DataObjectDocumentTest.php @@ -16,6 +16,7 @@ use SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned; use SilverStripe\SearchService\Tests\Fake\DataObjectSubclassFake; use SilverStripe\SearchService\Tests\Fake\ImageFake; +use SilverStripe\SearchService\Tests\Fake\PageFake; use SilverStripe\SearchService\Tests\Fake\TagFake; use SilverStripe\SearchService\Tests\SearchServiceTest; use SilverStripe\Security\Member; @@ -44,6 +45,7 @@ class DataObjectDocumentTest extends SearchServiceTest DataObjectSubclassFake::class, DataObjectFakeVersioned::class, DataObjectFakePrivate::class, + PageFake::class, ]; public function testGetIdentifier(): void @@ -531,6 +533,7 @@ public function testGetDependentDocuments(): void $config->set('getSearchableClasses', [ DataObjectFake::class, ImageFake::class, + Page::class, ]); $config->set('getFieldsForClass', [ DataObjectFake::class => [ @@ -543,6 +546,10 @@ public function testGetDependentDocuments(): void ImageFake::class => [ new Field('tagtitles', 'Tags.Title'), ], + Page::class => [ + new Field('title'), + new Field('content'), + ], ]); $dataobject = $this->objFromFixture(TagFake::class, 'one'); @@ -576,6 +583,33 @@ public function testGetDependentDocuments(): void } $this->assertEqualsCanonicalizing($expectedDocuments, $resultDocuments); + + $pageOne = $this->objFromFixture(Page::class, 'page1'); + $pageDoc = DataObjectDocument::create($pageOne); + /** @var DataObjectDocument[] $dependentPages */ + $dependentPages = $pageDoc->getDependentDocuments(); + + // Grab all the expected pages + $pageTwo = $this->objFromFixture(Page::class, 'page2'); + $pageThree = $this->objFromFixture(Page::class, 'page3'); + $pageSeven = $this->objFromFixture(Page::class, 'page7'); + $pageEight = $this->objFromFixture(Page::class, 'page8'); + + $expectedPages = [ + sprintf('%s-%s', Page::class, $pageTwo->ID), + sprintf('%s-%s', Page::class, $pageThree->ID), + sprintf('%s-%s', Page::class, $pageSeven->ID), + sprintf('%s-%s', Page::class, $pageEight->ID), + ]; + + $resultPages = []; + + // Now let's check that each Document represents the Pages that we expected + foreach ($dependentPages as $document) { + $resultPages[] = sprintf('%s-%s', $document->getSourceClass(), $document->getDataObject()?->ID); + } + + $this->assertEqualsCanonicalizing($expectedPages, $resultPages); } public function testExtensionRequired(): void @@ -612,7 +646,7 @@ public function testDeletedDataObject(): void $id = $dataObject->ID; $doc = DataObjectDocument::create($dataObject)->setShouldFallbackToLatestVersion(true); - $dataObject->delete(); + $dataObject->doArchive(); /** @var DataObjectDocument $serialDoc */ $serialDoc = unserialize(serialize($doc)); diff --git a/tests/Extensions/SiteTreeHierarchyExtensionTest.php b/tests/Extensions/SiteTreeHierarchyExtensionTest.php new file mode 100644 index 00000000..8a5bc9a3 --- /dev/null +++ b/tests/Extensions/SiteTreeHierarchyExtensionTest.php @@ -0,0 +1,47 @@ +objFromFixture(Page::class, 'page1'); + $pageTwo = $this->objFromFixture(Page::class, 'page2'); + $pageThree = $this->objFromFixture(Page::class, 'page3'); + $pageSeven = $this->objFromFixture(Page::class, 'page7'); + $pageEight = $this->objFromFixture(Page::class, 'page8'); + + $extension = new SiteTreeHierarchyExtension(); + $extension->setOwner($pageOne); + + $parentDocument = DataObjectDocument::create($pageOne); + $identifierPrefix = preg_replace('/\d+$/', '', $parentDocument->getIdentifier()); + $childDocs = []; + $extension->updateSearchDependentDocuments($childDocs); + + $expectedIdentifiers = [ + $identifierPrefix . $pageTwo->ID, + $identifierPrefix . $pageThree->ID, + $identifierPrefix . $pageSeven->ID, + $identifierPrefix . $pageEight->ID, + ]; + + $resultIdentifiers = ArrayList::create($childDocs)->column('getIdentifier'); + + $this->assertEqualsCanonicalizing($expectedIdentifiers, $resultIdentifiers); + } + +} diff --git a/tests/Fake/ImageFake.php b/tests/Fake/ImageFake.php index f117a95a..df1c697e 100644 --- a/tests/Fake/ImageFake.php +++ b/tests/Fake/ImageFake.php @@ -10,11 +10,13 @@ class ImageFake extends DataObject implements TestOnly { private static array $db = [ + 'Title' => 'Varchar', 'URL' => 'Varchar', ]; private static array $has_one = [ 'Parent' => DataObjectFake::class, + 'PageFake' => PageFake::class, ]; private static array $many_many = [ @@ -27,4 +29,10 @@ class ImageFake extends DataObject implements TestOnly private static string $table_name = 'ImageFake'; + // For DataObjects that are not versioned, canView is needed and must return true + public function canView($member = null) // phpcs:ignore SlevomatCodingStandard.TypeHints + { + return true; + } + } diff --git a/tests/Fake/PageFake.php b/tests/Fake/PageFake.php new file mode 100644 index 00000000..6d27d24c --- /dev/null +++ b/tests/Fake/PageFake.php @@ -0,0 +1,31 @@ + TagFake::class, + ]; + + private static array $has_many = [ + 'Images' => ImageFake::class, + ]; + + private static array $owns = [ + 'Tags', + 'Images', + ]; + + private static string $table_name = 'PageFake'; + + private static array $extensions = [ + SearchServiceExtension::class, + ]; + +} diff --git a/tests/Jobs/RemoveDataObjectJobTest.php b/tests/Jobs/RemoveDataObjectJobTest.php index 0cd12b24..87b53056 100644 --- a/tests/Jobs/RemoveDataObjectJobTest.php +++ b/tests/Jobs/RemoveDataObjectJobTest.php @@ -5,6 +5,7 @@ use SilverStripe\SearchService\DataObject\DataObjectDocument; use SilverStripe\SearchService\Jobs\RemoveDataObjectJob; use SilverStripe\SearchService\Schema\Field; +use SilverStripe\SearchService\Service\Indexer; use SilverStripe\SearchService\Tests\Fake\DataObjectFake; use SilverStripe\SearchService\Tests\Fake\DataObjectFakePrivate; use SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned; @@ -53,6 +54,23 @@ public function testJob(): void ] ); + $index = [ + 'main' => [ + 'includeClasses' => [ + DataObjectFake::class => ['title' => true], + TagFake::class => ['title' => true], + ], + ], + ]; + + $config->set( + 'getIndexesForClassName', + [ + DataObjectFake::class => $index, + TagFake::class => $index, + ] + ); + // Select tag one from our fixture $tag = $this->objFromFixture(TagFake::class, 'one'); // Queue up a job to remove our Tag, the result should be that any related DataObject (DOs that have this Tag @@ -62,6 +80,9 @@ public function testJob(): void ); $job->setup(); + // Creating this job does not necessarily mean to delete documents from index + $this->assertEquals(Indexer::METHOD_ADD, $job->getMethod()); + // Grab what Documents the Job determined it needed to action /** @var DataObjectDocument[] $documents */ $documents = $job->getDocuments(); @@ -76,11 +97,25 @@ public function testJob(): void $resultTitles = []; + // This determines whether the document should be added or removed from from the index foreach ($documents as $document) { $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be added to index + $this->assertTrue($document->shouldIndex()); } $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); + + // Deleting related documents so that they will be removed from index as well + $this->objFromFixture(DataObjectFake::class, 'one')->delete(); + $this->objFromFixture(DataObjectFake::class, 'three')->delete(); + + // This determines whether the document should be added or removed from from the index + foreach ($documents as $document) { + // The document should be removed from index + $this->assertFalse($document->shouldIndex()); + } } } diff --git a/tests/Jobs/RemoveRelatedDataObjectJobTest.php b/tests/Jobs/RemoveRelatedDataObjectJobTest.php new file mode 100644 index 00000000..b33d6869 --- /dev/null +++ b/tests/Jobs/RemoveRelatedDataObjectJobTest.php @@ -0,0 +1,393 @@ +objFromFixture(Page::class, 'page' . $i)->publishRecursive(); + } + + $this->objFromFixture(PageFake::class, 'page9')->publishRecursive(); + $this->objFromFixture(PageFake::class, 'page10')->publishRecursive(); + $this->objFromFixture(PageFake::class, 'page11')->publishRecursive(); + $this->objFromFixture(TagFake::class, 'four')->publishRecursive(); + $this->objFromFixture(TagFake::class, 'five')->publishRecursive(); + $this->objFromFixture(TagFake::class, 'six')->publishRecursive(); + + $config = $this->mockConfig(); + + $config->set( + 'getSearchableClasses', + [ + DataObjectFake::class, + TagFake::class, + ImageFake::class, + PageFake::class, + Page::class, + ] + ); + + $config->set( + 'getFieldsForClass', + [ + DataObjectFake::class => [ + new Field('title'), + new Field('tagtitles', 'Tags.Title'), + ], + PageFake::class => [ + new Field('title'), + new Field('tagtitles', 'Tags.Title'), + new Field('images', 'Images.URL'), + ], + ImageFake::class => [ + new Field('title'), + new Field('url'), + new Field('tagtitles', 'Tags.Title'), + ], + ] + ); + + $index = [ + 'main' => [ + 'includeClasses' => [ + DataObjectFake::class => ['title' => true], + TagFake::class => ['title' => true], + ImageFake::class => ['title' => true], + PageFake::class => ['title' => true], + Page::class => ['title' => true], + ], + ], + ]; + + $config->set( + 'getIndexesForClassName', + [ + DataObjectFake::class => $index, + TagFake::class => $index, + ImageFake::class => $index, + PageFake::class => $index, + Page::class => $index, + ] + ); + } + + public function testUnpublishParentPage(): void + { + $childA = $this->objFromFixture(Page::class, 'page2'); + $childB = $this->objFromFixture(Page::class, 'page3'); + $grandChildA1 = $this->objFromFixture(Page::class, 'page7'); + $grandChildA2 = $this->objFromFixture(Page::class, 'page8'); + + $this->assertTrue($childA->isPublished()); + $this->assertTrue($childB->isPublished()); + $this->assertTrue($grandChildA1->isPublished()); + $this->assertTrue($grandChildA2->isPublished()); + + $pageOne = $this->objFromFixture(Page::class, 'page1'); + $pageOne->doUnpublish(); + + // Default behaviour when unpublishng the parent page + $this->assertTrue(SiteTree::config()->get('enforce_strict_hierarchy')); + $this->assertFalse($childA->isPublished()); + $this->assertFalse($childB->isPublished()); + $this->assertFalse($grandChildA1->isPublished()); + $this->assertFalse($grandChildA2->isPublished()); + + // Queue up a job to remove a page with child pages are added as related documents + $pageDoc = DataObjectDocument::create($pageOne); + $job = RemoveDataObjectJob::create($pageDoc); + $job->setup(); + + // Creating this job does not necessarily mean to delete documents from index + $this->assertEquals(Indexer::METHOD_ADD, $job->getMethod()); + + // Grab what Documents the Job determined it needed to action + /** @var DataObjectDocument[] $documents */ + $documents = $job->getDocuments(); + + // There should be two Pages with this Tag assigned + $this->assertCount(4, $documents); + + $expectedTitles = [ + 'Child of Parent Page 1 - A', + 'Child of Parent Page 1 - B', + 'Grandchild of Parent Page 1 - A1', + 'Grandchild of Parent Page 1 - A2', + ]; + + $resultTitles = []; + + // This determines whether the document should be added or removed from from the index + foreach ($documents as $document) { + $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be removed from index + $this->assertFalse($document->shouldIndex()); + } + + $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); + } + + public function testUnpublishRelatedTagsObject(): void + { + $tagFour = $this->objFromFixture(TagFake::class, 'four'); + $tagFour->doUnpublish(); + + // Queue up a job to remove our Tag, the result should be that any related DataObject (DOs that have this Tag + // assigned to them) are added as related Documents + $job = RemoveDataObjectJob::create( + DataObjectDocument::create($tagFour) + ); + $job->setup(); + + // Creating this job does not necessarily mean to delete documents from index + $this->assertEquals(Indexer::METHOD_ADD, $job->getMethod()); + + // Grab what Documents the Job determined it needed to action + /** @var DataObjectDocument[] $documents */ + $documents = $job->getDocuments(); + + // There should be two Pages with this Tag assigned + $this->assertCount(3, $documents); + + $expectedTitles = [ + 'Child of Parent Page 2 - B', + 'Great Grandchild of Parent Page 2 - B1 - One', + 'Image Fake Three', + ]; + + $pageChild = $this->objFromFixture(PageFake::class, 'page9'); + $grandChild = $this->objFromFixture(PageFake::class, 'page10'); + $greatGrandChild = $this->objFromFixture(PageFake::class, 'page11'); + $imageThree = $this->objFromFixture(ImageFake::class, 'three'); + $imageFour = $this->objFromFixture(ImageFake::class, 'four'); + + $resultTitles = []; + + // This determines whether the document should be added or removed from from the index + foreach ($documents as $document) { + $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be added to index + $this->assertTrue($document->shouldIndex()); + } + + $this->assertEqualsCanonicalizing( + $expectedTitles, + $resultTitles + ); + + $this->assertEqualsCanonicalizing( + $expectedTitles, + [ + $pageChild->Title, + $greatGrandChild->Title, + $imageThree->Title, + ], + ); + + Versioned::withVersionedMode(function () use ( + $tagFour, + $pageChild, + $greatGrandChild + ): void { + Versioned::set_stage(Versioned::LIVE); + + $this->assertNotContains($tagFour->ID, $pageChild->Tags()->column('ID')); + $this->assertNotContains($tagFour->ID, $greatGrandChild->Tags()->column('ID')); + }); + + + $tagFive = $this->objFromFixture(TagFake::class, 'five'); + $tagFive->doUnpublish(); + // Queue up a job to remove our Tag, the result should be that any related DataObject (DOs that have this Tag + // assigned to them) are added as related Documents + $job2 = RemoveDataObjectJob::create( + DataObjectDocument::create($tagFive) + ); + $job2->setup(); + + // Creating this job does not necessarily mean to delete documents from index + $this->assertEquals(Indexer::METHOD_ADD, $job->getMethod()); + + // Grab what Documents the Job determined it needed to action + /** @var DataObjectDocument[] $documents */ + $documents = $job2->getDocuments(); + + $resultTitles = []; + + foreach ($documents as $document) { + $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be added to index + $this->assertTrue($document->shouldIndex()); + } + + // There should be two Pages with this Tag assigned + $this->assertCount(4, $documents); + + $expectedTitles = [ + 'Child of Parent Page 2 - B', + 'Grandchild of Parent Page 2 - B1', + 'Image Fake Three', + 'Image Fake Four', + ]; + + $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); + $this->assertEqualsCanonicalizing( + $expectedTitles, + [ + $pageChild->Title, + $grandChild->Title, + $imageThree->Title, + $imageFour->Title, + ], + ); + + Versioned::withVersionedMode(function () use ( + $tagFive, + $pageChild, + $grandChild, + $imageThree, + $imageFour, + ): void { + Versioned::set_stage(Versioned::LIVE); + + $this->assertNotContains($tagFive->ID, $pageChild->Tags()->column('ID')); + $this->assertNotContains($tagFive->ID, $grandChild->Tags()->column('ID')); + $this->assertNotContains($tagFive->ID, $imageThree->Tags()->column('ID')); + $this->assertNotContains($tagFive->ID, $imageFour->Tags()->column('ID')); + }); + + // Then unpublish these related pages to include them to the documents to be removed + $pageChild->doUnpublish(); + $grandChild->doUnpublish(); + $imageThree->delete(); + $imageFour->delete(); + + /** @var DataObjectDocument[] $documents */ + $documents = $job2->getDocuments(); + $resultTitles = []; + + foreach ($documents as $document) { + $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be removed from index + $this->assertFalse($document->shouldIndex()); + } + + // There should be two Pages with this Tag assigned + $this->assertCount(4, $documents); + + $expectedTitles = [ + 'Child of Parent Page 2 - B', + 'Grandchild of Parent Page 2 - B1', + 'Image Fake Three', + 'Image Fake Four', + ]; + + $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); + $this->assertEqualsCanonicalizing( + $expectedTitles, + [ + $pageChild->Title, + $grandChild->Title, + $imageThree->Title, + $imageFour->Title, + ], + ); + } + + public function testUnpublishImageRelatedtoPages(): void + { + $greatGrandChild = $this->objFromFixture(PageFake::class, 'page11'); + + $this->assertTrue($greatGrandChild->isPublished()); + + $imageFive = $this->objFromFixture(ImageFake::class, 'five'); + // Queue up a job to remove our Image, the result should be that any related DataObject + $job = RemoveDataObjectJob::create( + DataObjectDocument::create($imageFive) + ); + $job->setup(); + + // Creating this job does not necessarily mean to delete documents from index + $this->assertEquals(Indexer::METHOD_ADD, $job->getMethod()); + + // Grab what Documents the Job determined it needed to action + /** @var DataObjectDocument[] $documents */ + $documents = $job->getDocuments(); + + // There should be two Pages with this Tag assigned + $this->assertCount(1, $documents); + + $expectedTitles = [ + 'Great Grandchild of Parent Page 2 - B1 - One', + ]; + + $resultTitles = []; + + // This determines whether the document should be added or removed from from the index + foreach ($documents as $document) { + $resultTitles[] = $document->getDataObject()?->Title; + + // The document should be added to index + $this->assertTrue($document->shouldIndex()); + } + + $this->assertEqualsCanonicalizing( + $expectedTitles, + $resultTitles + ); + + $this->assertEqualsCanonicalizing( + $expectedTitles, + [ + $greatGrandChild->Title, + ], + ); + } + +} diff --git a/tests/pages.yml b/tests/pages.yml index 83999f57..6380e596 100644 --- a/tests/pages.yml +++ b/tests/pages.yml @@ -1,23 +1,73 @@ +SilverStripe\SearchService\Tests\Fake\TagFake: + four: + Title: Tag four + five: + Title: Tag five + six: + Title: Tag six + +SilverStripe\SearchService\Tests\Fake\ImageFake: + three: + Title: Image Fake Three + URL: "/image-three/" + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.four,=>SilverStripe\SearchService\Tests\Fake\TagFake.five + four: + Title: Image Fake Four + URL: "/image-four/" + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.five,=>SilverStripe\SearchService\Tests\Fake\TagFake.six + five: + Title: Image Fake Five + URL: "/image-five/" + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.six + Page: page1: - Title: Parent Page + Title: Parent Page 1 ShowInSearch: 1 page2: - Title: Child Page 1 + Title: Child of Parent Page 1 - A Parent: =>Page.page1 ShowInSearch: 1 page3: - Title: Child Page 2 + Title: Child of Parent Page 1 - B Parent: =>Page.page1 ShowInSearch: 1 page4: Title: Parent Page 2 ShowInSearch: 1 page5: - Title: Child Page 3 + Title: Child of Parent Page 2 - A Parent: =>Page.page4 ShowInSearch: 1 page6: Title: Subsite Page 1 Subsite: =>SilverStripe\Subsites\Model\Subsite.subsite1 ShowInSearch: 1 + page7: + Title: Grandchild of Parent Page 1 - A1 + Parent: =>Page.page2 + ShowInSearch: 1 + page8: + Title: Grandchild of Parent Page 1 - A2 + Parent: =>Page.page2 + ShowInSearch: 1 + +SilverStripe\SearchService\Tests\Fake\PageFake: + page9: + Title: Child of Parent Page 2 - B + Parent: =>Page.page4 + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.four,=>SilverStripe\SearchService\Tests\Fake\TagFake.five + Images: =>SilverStripe\SearchService\Tests\Fake\ImageFake.three + ShowInSearch: 1 + page10: + Title: Grandchild of Parent Page 2 - B1 + Parent: =>SilverStripe\SearchService\Tests\Fake\PageFake.page9 + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.five,=>SilverStripe\SearchService\Tests\Fake\TagFake.six + Images: =>SilverStripe\SearchService\Tests\Fake\ImageFake.four + ShowInSearch: 1 + page11: + Title: Great Grandchild of Parent Page 2 - B1 - One + Parent: =>SilverStripe\SearchService\Tests\Fake\PageFake.page10 + Images: =>SilverStripe\SearchService\Tests\Fake\ImageFake.five + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.four,=>SilverStripe\SearchService\Tests\Fake\TagFake.six + ShowInSearch: 1