Fix indexing of unpublished documents (#648)

* Fix indexing of unpublished shadow documents * Revert deprecation * Fix shadow publishing * Add tests * Update baseline
sulu · Dec 14, 2023 · 61ac120 · 61ac120
1 parent 766c235
commit 61ac120
Show file tree

Hide file tree

Showing 9 changed files with 198 additions and 98 deletions.
diff --git a/Document/Index/ArticleGhostIndexer.php b/Document/Index/ArticleGhostIndexer.php
@@ -87,7 +87,7 @@ public function index(ArticleDocument $document): void
         }
 
         $article = $this->createOrUpdateArticle($document, $document->getLocale());
-        $this->createOrUpdateShadows($document);
+        $this->updateShadows($document);
         $this->createOrUpdateGhosts($document);
         $this->dispatchIndexEvent($document, $article);
         $this->manager->persist($article);

diff --git a/Document/Index/ArticleIndexer.php b/Document/Index/ArticleIndexer.php
@@ -327,6 +327,15 @@ private function getBlockContentFieldsRecursive(array $blocks, ArticleDocument $
         return $contentFields;
     }
 
+    protected function findViewDocument(ArticleDocument $document, string $locale): ?ArticleViewDocumentInterface
+    {
+        $articleId = $this->getViewDocumentId($document->getUuid(), $locale);
+        /** @var ArticleViewDocumentInterface $article */
+        $article = $this->manager->find($this->documentFactory->getClass('article'), $articleId);
+
+        return $article;
+    }
+
     /**
      * Returns view-document from index or create a new one.
      */
@@ -335,9 +344,7 @@ protected function findOrCreateViewDocument(
         string $locale,
         string $localizationState
     ): ?ArticleViewDocumentInterface {
-        $articleId = $this->getViewDocumentId($document->getUuid(), $locale);
-        /** @var ArticleViewDocumentInterface $article */
-        $article = $this->manager->find($this->documentFactory->getClass('article'), $articleId);
+        $article = $this->findViewDocument($document, $locale);
 
         if ($article) {
             // Only index ghosts when the article isn't a ghost himself.
@@ -351,7 +358,7 @@ protected function findOrCreateViewDocument(
         }
 
         $article = $this->documentFactory->create('article');
-        $article->setId($articleId);
+        $article->setId($this->getViewDocumentId($document->getUuid(), $locale));
         $article->setUuid($document->getUuid());
         $article->setLocale($locale);
 
@@ -505,11 +512,12 @@ public function index(ArticleDocument $document): void
         $this->dispatchIndexEvent($document, $article);
         $this->manager->persist($article);
 
-        $this->createOrUpdateShadows($document);
+        $this->updateShadows($document);
     }
 
     protected function indexShadow(ArticleDocument $document): void
     {
+        /** @var ArticleDocument $shadowDocument */
         $shadowDocument = $this->documentManager->find(
             $document->getUuid(),
             $document->getOriginalLocale(),
@@ -519,12 +527,11 @@ protected function indexShadow(ArticleDocument $document): void
         );
 
         $article = $this->createOrUpdateArticle($shadowDocument, $document->getOriginalLocale(), LocalizationState::SHADOW);
-
         $this->dispatchIndexEvent($shadowDocument, $article);
         $this->manager->persist($article);
     }
 
-    protected function createOrUpdateShadows(ArticleDocument $document): void
+    protected function updateShadows(ArticleDocument $document): void
     {
         if ($document->isShadowLocaleEnabled()) {
             return;
@@ -534,6 +541,12 @@ protected function createOrUpdateShadows(ArticleDocument $document): void
             try {
                 /** @var ArticleDocument $shadowDocument */
                 $shadowDocument = $this->documentManager->find($document->getUuid(), $shadowLocale);
+
+                // update shadow only if original document exists
+                if (!$this->findViewDocument($shadowDocument, $document->getLocale())) {
+                    continue;
+                }
+
                 $this->indexShadow($shadowDocument);
             } catch (DocumentManagerException $documentManagerException) {
                 // @ignoreException

diff --git a/Document/Subscriber/ArticleSubscriber.php b/Document/Subscriber/ArticleSubscriber.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /*
  * This file is part of Sulu.
  *
@@ -89,7 +91,7 @@ public function __construct(
         IndexerInterface $liveIndexer,
         DocumentManagerInterface $documentManager,
         DocumentInspector $documentInspector,
-        PropertyEncoder $propertyEncoder
+        PropertyEncoder $propertyEncoder,
     ) {
         $this->indexer = $indexer;
         $this->liveIndexer = $liveIndexer;
@@ -297,7 +299,7 @@ private function setPageData(ArticleDocument $document, NodeInterface $node, str
         $document->setPages($pages);
         $node->setProperty(
             $this->propertyEncoder->localizedSystemName(self::PAGES_PROPERTY, $locale),
-            \json_encode($pages)
+            \json_encode($pages),
         );
     }
 
@@ -313,7 +315,7 @@ public function hydratePageData(HydrateEvent $event): void
 
         $pages = $event->getNode()->getPropertyValueWithDefault(
             $this->propertyEncoder->localizedSystemName(self::PAGES_PROPERTY, $document->getOriginalLocale()),
-            \json_encode([])
+            \json_encode([]),
         );
         $pages = \json_decode($pages, true);
 
@@ -331,7 +333,7 @@ private function loadPageDataForShadow(NodeInterface $node, ArticleDocument $doc
     {
         $pages = $node->getPropertyValueWithDefault(
             $this->propertyEncoder->localizedSystemName(self::PAGES_PROPERTY, $document->getLocale()),
-            \json_encode([])
+            \json_encode([]),
         );
         $pages = \json_decode($pages, true);
 
@@ -465,6 +467,11 @@ public function handleRemoveLocale(RemoveLocaleEvent $event)
             return;
         }
 
+        $ghostLocale = $this->documentInspector->getConcreteLocales($document)[0] ?? null;
+        if (null !== $ghostLocale) {
+            $document = $this->documentManager->find($document->getUuid(), $ghostLocale);
+        }
+
         $this->indexer->replaceWithGhostData($document, $event->getLocale());
         $this->indexer->flush();
     }
@@ -495,7 +502,7 @@ public function handleRemoveLocaleLive(RemoveLocaleEvent $event)
             return;
         }
 
-        $this->liveIndexer->replaceWithGhostData($document, $event->getLocale());
+        $this->liveIndexer->remove($document, $event->getLocale());
         $this->liveIndexer->flush();
     }
 
@@ -562,7 +569,7 @@ public function handleChildrenPersist(PersistEvent $event): void
                         'clear_missing_content' => false,
                         'auto_name' => false,
                         'auto_rename' => false,
-                    ]
+                    ],
                 );
             }
         }
@@ -582,7 +589,7 @@ public function handleMetadataLoad(MetadataLoadEvent $event): void
             [
                 'encoding' => 'system_localized',
                 'property' => 'suluPageTitle',
-            ]
+            ],
         );
     }
 }
diff --git a/Resources/translations/admin.de.json b/Resources/translations/admin.de.json
@@ -28,5 +28,6 @@
     "sulu_activity.description.articles.translation_copied": "{userFullName} hat die Sprachvariante für \"{context_sourceLocale}\" des Artikels \"{resourceTitle}\" nach \"{resourceLocale}\" kopiert",
     "sulu_activity.description.articles.translation_removed": "{userFullName} hat die Sprachvariante für \"{resourceLocale}\" des Artikels \"{resourceTitle}\" gelöscht",
     "sulu_activity.description.articles.translation_restored": "{userFullName} hat die Sprachvariante für \"{resourceLocale}\" des Artikels \"{resourceTitle}\" wiederhergestellt",
-    "sulu_activity.description.articles.version_restored": "{userFullName} hat die Version \"{context_version}\" des Artikels \"{resourceTitle}\" wiederhergestellt"
+    "sulu_activity.description.articles.version_restored": "{userFullName} hat die Version \"{context_version}\" des Artikels \"{resourceTitle}\" wiederhergestellt",
+    "sulu_activity.description.articles.route_removed": "{userFullName} hat die Route \"{resourceTitle}\" gelöscht."
 }
diff --git a/Resources/translations/admin.en.json b/Resources/translations/admin.en.json
@@ -28,5 +28,6 @@
     "sulu_activity.description.articles.translation_copied": "{userFullName} has copied the \"{context_sourceLocale}\" translation of the article \"{resourceTitle}\" into \"{resourceLocale}\"",
     "sulu_activity.description.articles.translation_removed": "{userFullName} has removed the \"{resourceLocale}\" translation of the article \"{resourceTitle}\"",
     "sulu_activity.description.articles.translation_restored": "{userFullName} has restored the \"{resourceLocale}\" translation of the article \"{resourceTitle}\"",
-    "sulu_activity.description.articles.version_restored": "{userFullName} has restored the version \"{context_version}\" of the article \"{resourceTitle}\""
+    "sulu_activity.description.articles.version_restored": "{userFullName} has restored the version \"{context_version}\" of the article \"{resourceTitle}\"",
+    "sulu_activity.description.articles.route_removed": "{userFullName} has removed the route \"{resourceTitle}\"."
 }
diff --git a/Tests/Application/config/webspaces/sulu.io.xml b/Tests/Application/config/webspaces/sulu.io.xml
@@ -9,6 +9,7 @@
     <localizations>
         <localization language="de" default="true"/>
         <localization language="en"/>
+        <localization language="fr"/>
     </localizations>
 
     <theme>default</theme>

diff --git a/Tests/Functional/Document/Index/ArticleIndexerTest.php b/Tests/Functional/Document/Index/ArticleIndexerTest.php
@@ -264,6 +264,91 @@ public function testIndexShadow()
         $this->assertEquals($contentData['article'], 'Test content - CHANGED!');
     }
 
+    public function testUnpublishedShadows(): void
+    {
+        $article = $this->createArticle(
+            [
+                'article' => 'Test content',
+            ],
+            'Test Article',
+            'default_with_route'
+        );
+        $secondLocale = 'de';
+        $thirdLocale = 'fr';
+
+        $this->updateArticle(
+            $article['id'],
+            $secondLocale,
+            [
+                'id' => $article['id'],
+                'article' => 'Test Inhalt',
+            ],
+            'Test Artikel Deutsch',
+            'default_with_route'
+        );
+        $this->updateArticle(
+            $article['id'],
+            $secondLocale,
+            [
+                'id' => $article['id'],
+                'shadowOn' => true,
+                'shadowBaseLanguage' => $this->locale,
+            ],
+            null,
+            null
+        );
+
+        $this->updateArticle(
+            $article['id'],
+            $thirdLocale,
+            [
+                'id' => $article['id'],
+                'article' => 'Test French Content',
+            ],
+            'Test Artikel French',
+            'default_with_route'
+        );
+        $this->updateArticle(
+            $article['id'],
+            $thirdLocale,
+            [
+                'id' => $article['id'],
+                'shadowOn' => true,
+                'shadowBaseLanguage' => $this->locale,
+            ],
+            null,
+            null
+        );
+
+        self::assertNotNull($this->findViewDocument($article['id'], $this->locale));
+        self::assertNotNull($this->findViewDocument($article['id'], $secondLocale));
+        self::assertNotNull($this->findViewDocument($article['id'], $thirdLocale));
+
+        $this->unpublishArticle($article['id'], $this->locale);
+        $this->unpublishArticle($article['id'], $secondLocale);
+        $this->unpublishArticle($article['id'], $thirdLocale);
+
+        self::assertNull($this->findViewDocument($article['id'], $this->locale));
+        self::assertNull($this->findViewDocument($article['id'], $secondLocale));
+        self::assertNull($this->findViewDocument($article['id'], $thirdLocale));
+
+        // publish the shadow
+        $this->updateArticle(
+            $article['id'],
+            $secondLocale,
+            [
+                'id' => $article['id'],
+            ],
+            null,
+            null
+        );
+
+        // only the DE shadow should be published
+        self::assertNull($this->findViewDocument($article['id'], $this->locale));
+        self::assertNotNull($this->findViewDocument($article['id'], $secondLocale));
+        self::assertNull($this->findViewDocument($article['id'], $thirdLocale));
+    }
+
     public function testIndexPageTreeRoute()
     {
         $page = $this->createPage();
@@ -542,6 +627,18 @@ private function updateArticle(
         return \json_decode($this->client->getResponse()->getContent(), true);
     }
 
+    private function unpublishArticle($uuid, $locale = null)
+    {
+        $this->client->jsonRequest(
+            'POST',
+            '/api/articles/' . $uuid . '?locale=' . ($locale ?: $this->locale) . '&action=unpublish'
+        );
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        return \json_decode($this->client->getResponse()->getContent(), true);
+    }
+
     /**
      * Create article page.
      *

diff --git a/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php b/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php
@@ -31,6 +31,7 @@
 use Sulu\Component\DocumentManager\Event\PublishEvent;
 use Sulu\Component\DocumentManager\Event\RemoveDraftEvent;
 use Sulu\Component\DocumentManager\Event\RemoveEvent;
+use Sulu\Component\DocumentManager\Event\RemoveLocaleEvent;
 use Sulu\Component\DocumentManager\Event\ReorderEvent;
 
 class ArticleSubscriberTest extends TestCase
@@ -654,4 +655,63 @@ public function testPersistPageDataOnReorder()
 
         $this->articleSubscriber->persistPageDataOnReorder($event->reveal());
     }
+
+    public function testHandleRemoveLocaleWithInvalidDocument()
+    {
+        $event = $this->prophesize(RemoveLocaleEvent::class);
+        $document = $this->prophesize(\stdClass::class); // Not an ArticleDocument
+        $event->getDocument()->willReturn($document->reveal());
+
+        // Ensure the indexer methods are not called
+        $this->indexer->replaceWithGhostData()->shouldNotBeCalled();
+        $this->indexer->flush()->shouldNotBeCalled();
+
+        // Call the method and make sure it doesn't throw exceptions
+        $this->articleSubscriber->handleRemoveLocale($event->reveal());
+    }
+
+    public function testHandleRemoveLocale()
+    {
+        $event = $this->prophesize(RemoveLocaleEvent::class);
+        $document = $this->prophesize(ArticleDocument::class);
+        $event->getDocument()->willReturn($document->reveal());
+        $event->getLocale()->willReturn('en');
+
+        // Ensure the indexer methods are called
+        $this->indexer->replaceWithGhostData($document->reveal(), 'en')->shouldBeCalled();
+        $this->indexer->flush()->shouldBeCalled();
+
+        // Call the method
+        $this->articleSubscriber->handleRemoveLocale($event->reveal());
+    }
+
+    public function testHandleRemoveLocaleLiveWithInvalidDocument()
+    {
+        $event = $this->prophesize(RemoveLocaleEvent::class);
+        $document = $this->prophesize(\stdClass::class); // Not an ArticleDocument
+        $event->getDocument()->willReturn($document->reveal());
+
+        // Ensure the liveIndexer methods are not called
+        $this->liveIndexer->remove()->shouldNotBeCalled();
+        $this->liveIndexer->flush()->shouldNotBeCalled();
+
+        // Call the method and make sure it doesn't throw exceptions
+        $this->articleSubscriber->handleRemoveLocaleLive($event->reveal());
+    }
+
+    public function testHandleRemoveLocaleLive()
+    {
+        // Create a mock RemoveLocaleEvent with an ArticleDocument
+        $event = $this->prophesize(RemoveLocaleEvent::class);
+        $document = $this->prophesize(ArticleDocument::class);
+        $event->getDocument()->willReturn($document->reveal());
+        $event->getLocale()->willReturn('en');
+
+        // Ensure the liveIndexer methods are called
+        $this->liveIndexer->remove($document->reveal(), 'en')->shouldBeCalled();
+        $this->liveIndexer->flush()->shouldBeCalled();
+
+        // Call the method
+        $this->articleSubscriber->handleRemoveLocaleLive($event->reveal());
+    }
 }