From 45775e0406c29ed2158e1717c5ba786a1dc4db98 Mon Sep 17 00:00:00 2001 From: Marco Hermo Date: Thu, 4 Apr 2024 16:06:26 +1300 Subject: [PATCH] MNT: Additional PHPUnit tests --- phpunit.xml.dist | 28 ++++--- src/DataObject/DataObjectBatchProcessor.php | 2 + src/Jobs/RemoveDataObjectJob.php | 8 +- src/Service/Indexer.php | 2 + tests/DataObject/DataObjectDocumentTest.php | 2 + tests/Fake/ImageFake.php | 8 ++ tests/Fake/PageFake.php | 31 ++++++++ tests/Jobs/RemoveDataObjectJobTest.php | 88 ++++++++------------- tests/pages.yml | 56 +++++++++++-- 9 files changed, 150 insertions(+), 75 deletions(-) create mode 100644 tests/Fake/PageFake.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 745ac44..956c8ef 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 c2d1399..db0b12e 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/Jobs/RemoveDataObjectJob.php b/src/Jobs/RemoveDataObjectJob.php index 5314339..8a1d825 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,7 +52,7 @@ 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) { diff --git a/src/Service/Indexer.php b/src/Service/Indexer.php index b46f3a3..9ea719e 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 980e83b..d674def 100644 --- a/tests/DataObject/DataObjectDocumentTest.php +++ b/tests/DataObject/DataObjectDocumentTest.php @@ -17,6 +17,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; @@ -45,6 +46,7 @@ class DataObjectDocumentTest extends SearchServiceTest DataObjectSubclassFake::class, DataObjectFakeVersioned::class, DataObjectFakePrivate::class, + PageFake::class, ]; public function testGetIdentifier(): void diff --git a/tests/Fake/ImageFake.php b/tests/Fake/ImageFake.php index f117a95..df1c697 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 0000000..6d27d24 --- /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 3f98d0a..8660c86 100644 --- a/tests/Jobs/RemoveDataObjectJobTest.php +++ b/tests/Jobs/RemoveDataObjectJobTest.php @@ -2,10 +2,10 @@ namespace SilverStripe\SearchService\Tests\Jobs; -use Page; 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; @@ -17,10 +17,7 @@ class RemoveDataObjectJobTest extends SearchServiceTest { - protected static $fixture_file = [ // @phpcs:ignore - '../fixtures.yml', - '../pages.yml', - ]; + protected static $fixture_file = '../fixtures.yml'; // @phpcs:ignore /** * @phpcsSuppress SlevomatCodingStandard.TypeHints.PropertyTypeHint.MissingNativeTypeHint @@ -57,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 @@ -66,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(); @@ -80,66 +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; - } - - $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); - } - - public function testUnpublishedParentPage(): void - { - $config = $this->mockConfig(); - - $config->set( - 'getSearchableClasses', - [ - Page::class, - ] - ); - $config->set( - 'getFieldsForClass', - [ - Page::class => [ - new Field('title'), - new Field('content'), - ], - ] - ); - - // Publish all pages in fixtures since the internal dependency checks looks for live version - for ($i=1; $i<=8; $i++) { - $this->objFromFixture(Page::class, 'page' . $i)->publishRecursive(); + // The document should be added to index + $this->assertTrue($document->shouldIndex()); } - // Queue up a job to remove a page with child pages are added as related documents - $pageOne = $this->objFromFixture(Page::class, 'page1'); - $pageDoc = DataObjectDocument::create($pageOne); - $job = RemoveDataObjectJob::create($pageDoc); - $job->setup(); - - // 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 Page 1', - 'Child Page 2', - 'Grandchild Page 1', - 'Grandchild Page 2', - ]; + $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); - $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) { - $resultTitles[] = $document->getDataObject()?->Title; + // The document should be removed from index + $this->assertFalse($document->shouldIndex()); } - - $this->assertEqualsCanonicalizing($expectedTitles, $resultTitles); } } diff --git a/tests/pages.yml b/tests/pages.yml index 616ff25..6f75cab 100644 --- a/tests/pages.yml +++ b/tests/pages.yml @@ -1,20 +1,42 @@ +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: @@ -22,10 +44,32 @@ Page: Subsite: =>SilverStripe\Subsites\Model\Subsite.subsite1 ShowInSearch: 1 page7: - Title: Grandchild Page 1 + Title: Grandchild of Parent Page 1 - A1 Parent: =>Page.page2 ShowInSearch: 1 page8: - Title: Grandchild Page 2 + 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 + Banner: =>SilverStripe\SearchService\Tests\Fake\ImageFake.five + 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 + Banner: =>SilverStripe\SearchService\Tests\Fake\ImageFake.five + 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 + Banner: =>SilverStripe\SearchService\Tests\Fake\ImageFake.five + Tags: =>SilverStripe\SearchService\Tests\Fake\TagFake.four,=>SilverStripe\SearchService\Tests\Fake\TagFake.six + ShowInSearch: 1