MNT: Additional PHPUnit tests

silverstripe · Apr 5, 2024 · 0f9e26c · 0f9e26c
1 parent 188f6bd
commit 0f9e26c
Show file tree

Hide file tree

Showing 10 changed files with 552 additions and 75 deletions.
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -1,13 +1,17 @@
-<phpunit bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true">
-    <testsuite name="Default">
-        <directory>tests/</directory>
-    </testsuite>
-    <filter>
-        <whitelist processUncoveredFilesFromWhitelist="true">
-            <directory suffix=".php">src/</directory>
-            <exclude>
-                <directory suffix=".php">tests/</directory>
-            </exclude>
-        </whitelist>
-    </filter>
+<?xml version="1.0"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
+  <coverage processUncoveredFiles="true">
+    <include>
+      <directory suffix=".php">src/</directory>
+    </include>
+    <exclude>
+      <directory suffix=".php">tests/</directory>
+    </exclude>
+  </coverage>
+  <testsuite name="Default">
+    <directory>tests/</directory>
+  </testsuite>
+  <php>
+    <get name="flush" value="1"/>
+  </php>
 </phpunit>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,31 @@
+<?php
+
+namespace SilverStripe\SearchService\Tests\Fake;
+
+use Page;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\SearchService\Extensions\SearchServiceExtension;
+
+class PageFake extends Page implements TestOnly
+{
+
+    private static array $many_many = [
+        'Tags' => 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
@@ -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);
     }
 
 }