From 02e3485b40226f552d3b887cb98c1ad0ce71df08 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Fri, 15 Dec 2023 08:33:21 +0100 Subject: [PATCH] index correct ghost content when deleting the locale of an article --- Document/Index/ArticleIndexer.php | 12 ++ Document/Subscriber/ArticleSubscriber.php | 11 ++ Tests/Application/.env | 2 +- .../Document/Index/ArticleIndexerTest.php | 166 ++++++++++++------ .../Subscriber/ArticleSubscriberTest.php | 33 +++- phpstan-baseline.neon | 7 +- 6 files changed, 176 insertions(+), 55 deletions(-) diff --git a/Document/Index/ArticleIndexer.php b/Document/Index/ArticleIndexer.php index 2fd4eecf2..f17e72ec7 100644 --- a/Document/Index/ArticleIndexer.php +++ b/Document/Index/ArticleIndexer.php @@ -13,6 +13,7 @@ use ONGR\ElasticsearchBundle\Collection\Collection; use ONGR\ElasticsearchBundle\Service\Manager; +use ONGR\ElasticsearchDSL\Query\Compound\BoolQuery; use ONGR\ElasticsearchDSL\Query\MatchAllQuery; use ONGR\ElasticsearchDSL\Query\TermLevel\TermQuery; use Sulu\Bundle\ArticleBundle\Document\ArticleDocument; @@ -453,6 +454,17 @@ public function replaceWithGhostData(ArticleDocument $document, string $locale): $article = $this->createOrUpdateArticle($document, $locale); $article->setLocalizationState(new LocalizationStateViewObject(LocalizationState::GHOST, $document->getOriginalLocale())); + $repository = $this->manager->getRepository($this->documentFactory->getClass('article')); + $search = $repository->createSearch(); + $search->addQuery(new TermQuery('localization_state.state', 'ghost'), BoolQuery::MUST); + $search->addQuery(new TermQuery('localization_state.locale', $locale), BoolQuery::MUST); + + /** @var array $searchResult */ + $searchResult = $repository->findArray($search); + foreach ($searchResult as $result) { + $this->replaceWithGhostData($document, $result['locale']); + } + $this->manager->persist($article); } diff --git a/Document/Subscriber/ArticleSubscriber.php b/Document/Subscriber/ArticleSubscriber.php index 1aec28abe..9aee06958 100644 --- a/Document/Subscriber/ArticleSubscriber.php +++ b/Document/Subscriber/ArticleSubscriber.php @@ -465,6 +465,17 @@ public function handleRemoveLocale(RemoveLocaleEvent $event) return; } + $concreteLocales = $this->documentInspector->getConcreteLocales($document); + $ghostLocale = $document->getOriginalLocale(); + if (!\in_array($ghostLocale, $concreteLocales, true)) { + $ghostLocale = $concreteLocales[0] ?? null; + } + + if (null !== $ghostLocale) { + /** @var ArticleDocument $document */ + $document = $this->documentManager->find($document->getUuid(), $ghostLocale); + } + $this->indexer->replaceWithGhostData($document, $event->getLocale()); $this->indexer->flush(); } diff --git a/Tests/Application/.env b/Tests/Application/.env index 382f50030..3a015e6c0 100644 --- a/Tests/Application/.env +++ b/Tests/Application/.env @@ -1,5 +1,5 @@ APP_ENV=test -DATABASE_URL=mysql://root:@127.0.0.1:3306/su_article_test?serverVersion=5.7 +DATABASE_URL=mysql://root:ChangeMe@127.0.0.1:3306/sulu_test?serverVersion=5.7 DATABASE_CHARSET=utf8mb4 DATABASE_COLLATE=utf8mb4_unicode_ci ELASTICSEARCH_HOST=127.0.0.1:9200 diff --git a/Tests/Functional/Document/Index/ArticleIndexerTest.php b/Tests/Functional/Document/Index/ArticleIndexerTest.php index 1f1a65a49..e5bc1eb22 100644 --- a/Tests/Functional/Document/Index/ArticleIndexerTest.php +++ b/Tests/Functional/Document/Index/ArticleIndexerTest.php @@ -20,6 +20,7 @@ use Sulu\Bundle\PageBundle\Document\PageDocument; use Sulu\Bundle\RouteBundle\Entity\RouteRepositoryInterface; use Sulu\Bundle\TestBundle\Testing\SuluTestCase; +use Sulu\Component\Content\Document\LocalizationState; use Sulu\Component\DocumentManager\DocumentManagerInterface; use Symfony\Bundle\FrameworkBundle\KernelBrowser; @@ -35,11 +36,21 @@ class ArticleIndexerTest extends SuluTestCase */ private $documentManager; + /** + * @var Manager + */ + private $liveManager; + /** * @var Manager */ private $manager; + /** + * @var ArticleIndexer + */ + private $liveIndexer; + /** * @var ArticleIndexer */ @@ -58,9 +69,12 @@ public function setUp(): void $this->initPhpcr(); $this->purgeDatabase(); - $this->manager = $this->getContainer()->get('es.manager.live'); + $this->liveManager = $this->getContainer()->get('es.manager.live'); + $this->manager = $this->getContainer()->get('es.manager.default'); $this->documentManager = $this->getContainer()->get('sulu_document_manager.document_manager'); - $this->indexer = $this->getContainer()->get('sulu_article.elastic_search.article_live_indexer'); + $this->liveIndexer = $this->getContainer()->get('sulu_article.elastic_search.article_live_indexer'); + $this->liveIndexer->clear(); + $this->indexer = $this->getContainer()->get('sulu_article.elastic_search.article_indexer'); $this->indexer->clear(); } @@ -90,14 +104,14 @@ public function testRemove() /** @var ArticleDocument $articleDocument */ $articleDocument = $this->documentManager->find($article['id']); - $this->indexer->remove($articleDocument); - $this->indexer->flush(); + $this->liveIndexer->remove($articleDocument); + $this->liveIndexer->flush(); - self::assertNull($this->findViewDocument($articleDocument->getUuid(), 'de')); - self::assertNull($this->findViewDocument($articleDocument->getUuid(), 'en')); + self::assertNull($this->findLiveViewDocument($articleDocument->getUuid(), 'de')); + self::assertNull($this->findLiveViewDocument($articleDocument->getUuid(), 'en')); } - public function testRemoveLocale() + public function testReplaceWithGhostData() { $article = $this->createArticle( [ @@ -123,29 +137,32 @@ public function testRemoveLocale() /** @var ArticleDocument $articleDocument */ $articleDocument = $this->documentManager->find($article['id']); - $this->indexer->remove($articleDocument, 'en'); + $this->indexer->replaceWithGhostData($articleDocument, 'de'); $this->indexer->flush(); - self::assertNotNull($this->findViewDocument($articleDocument->getUuid(), 'de')); - self::assertNull($this->findViewDocument($articleDocument->getUuid(), 'en')); + $documentDE = $this->findViewDocument($articleDocument->getUuid(), 'de'); + $documentEN = $this->findViewDocument($articleDocument->getUuid(), 'en'); + + $this->assertSame('Test Article', $documentDE->getTitle()); + $this->assertSame('ghost', $documentDE->getLocalizationState()->state); + $this->assertSame('en', $documentDE->getLocalizationState()->locale); + + $this->assertSame('Test Article', $documentEN->getTitle()); } - public function testReplaceWithGhostData() + public function testReplaceWithGhostDataUpdateExistingGhosts() { $article = $this->createArticle( [ 'article' => 'Test content', ], - 'Test Article', + 'Test Article English', 'default_with_route' ); - $secondLocale = 'de'; - - // now add second locale $this->updateArticle( $article['id'], - $secondLocale, + 'de', [ 'id' => $article['id'], 'article' => 'Test Inhalt', @@ -154,19 +171,50 @@ public function testReplaceWithGhostData() 'default_with_route' ); + $this->updateArticle( + $article['id'], + 'fr', + [ + 'id' => $article['id'], + 'article' => 'Test Inhalt', + ], + 'Test Artikel French', + 'default_with_route' + ); + + $documentEN = $this->findViewDocument($article['id'], 'en'); + $this->assertSame(LocalizationState::LOCALIZED, $documentEN->getLocalizationState()->state); + $documentDE = $this->findViewDocument($article['id'], 'de'); + $this->assertSame(LocalizationState::LOCALIZED, $documentDE->getLocalizationState()->state); + $documentFR = $this->findViewDocument($article['id'], 'fr'); + $this->assertSame(LocalizationState::LOCALIZED, $documentFR->getLocalizationState()->state); + /** @var ArticleDocument $articleDocument */ - $articleDocument = $this->documentManager->find($article['id']); - $this->indexer->replaceWithGhostData($articleDocument, 'de'); + $articleDocument = $this->documentManager->find($article['id'], 'en'); + $this->indexer->replaceWithGhostData($articleDocument, 'fr'); $this->indexer->flush(); - $documentDE = $this->findViewDocument($articleDocument->getUuid(), 'de'); - $documentEN = $this->findViewDocument($articleDocument->getUuid(), 'en'); + $documentEN = $this->findViewDocument($article['id'], 'en'); + $this->assertSame(LocalizationState::LOCALIZED, $documentEN->getLocalizationState()->state); + $documentDE = $this->findViewDocument($article['id'], 'de'); + $this->assertSame(LocalizationState::LOCALIZED, $documentDE->getLocalizationState()->state); + $documentFR = $this->findViewDocument($article['id'], 'fr'); + $this->assertSame(LocalizationState::GHOST, $documentFR->getLocalizationState()->state); + $this->assertSame('en', $documentFR->getLocalizationState()->locale); - $this->assertSame('Test Article', $documentDE->getTitle()); - $this->assertSame('ghost', $documentDE->getLocalizationState()->state); - $this->assertSame('en', $documentDE->getLocalizationState()->locale); + /** @var ArticleDocument $articleDocument */ + $articleDocument = $this->documentManager->find($article['id'], 'de'); + $this->indexer->replaceWithGhostData($articleDocument, 'en'); + $this->indexer->flush(); - $this->assertSame('Test Article', $documentEN->getTitle()); + $documentEN = $this->findViewDocument($article['id'], 'en'); + $this->assertSame(LocalizationState::GHOST, $documentEN->getLocalizationState()->state); + $this->assertSame('de', $documentEN->getLocalizationState()->locale); + $documentDE = $this->findViewDocument($article['id'], 'de'); + $this->assertSame(LocalizationState::LOCALIZED, $documentDE->getLocalizationState()->state); + $documentFR = $this->findViewDocument($article['id'], 'fr'); + $this->assertSame(LocalizationState::GHOST, $documentFR->getLocalizationState()->state); + $this->assertSame('de', $documentFR->getLocalizationState()->locale); } public function testIndexDefaultWithRoute() @@ -179,12 +227,12 @@ public function testIndexDefaultWithRoute() 'default_with_route' ); - $this->indexer = $this->getContainer()->get('sulu_article.elastic_search.article_live_indexer'); + $this->liveIndexer = $this->getContainer()->get('sulu_article.elastic_search.article_live_indexer'); $document = $this->documentManager->find($article['id'], $this->locale); - $this->indexer->index($document); + $this->liveIndexer->index($document); - $viewDocument = $this->findViewDocument($article['id']); + $viewDocument = $this->findLiveViewDocument($article['id']); $this->assertEquals($document->getUuid(), $viewDocument->getUuid()); $this->assertEquals('/articles/test-article', $viewDocument->getRoutePath()); $this->assertInstanceOf('\DateTime', $viewDocument->getPublished()); @@ -230,7 +278,7 @@ public function testIndexShadow() null ); - $viewDocument = $this->findViewDocument($article['id'], $secondLocale); + $viewDocument = $this->findLiveViewDocument($article['id'], $secondLocale); $this->assertEquals($article['id'], $viewDocument->getUuid()); $this->assertEquals('/articles/test-artikel-deutsch', $viewDocument->getRoutePath()); @@ -257,7 +305,7 @@ public function testIndexShadow() 'default_with_route' ); - $viewDocument = $this->findViewDocument($article['id'], $secondLocale); + $viewDocument = $this->findLiveViewDocument($article['id'], $secondLocale); $this->assertEquals('Test Article - CHANGED!', $viewDocument->getTitle()); $contentData = \json_decode($viewDocument->getContentData(), true); @@ -320,17 +368,17 @@ public function testUnpublishedShadows(): void null ); - self::assertNotNull($this->findViewDocument($article['id'], $this->locale)); - self::assertNotNull($this->findViewDocument($article['id'], $secondLocale)); - self::assertNotNull($this->findViewDocument($article['id'], $thirdLocale)); + self::assertNotNull($this->findLiveViewDocument($article['id'], $this->locale)); + self::assertNotNull($this->findLiveViewDocument($article['id'], $secondLocale)); + self::assertNotNull($this->findLiveViewDocument($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)); + self::assertNull($this->findLiveViewDocument($article['id'], $this->locale)); + self::assertNull($this->findLiveViewDocument($article['id'], $secondLocale)); + self::assertNull($this->findLiveViewDocument($article['id'], $thirdLocale)); // publish the shadow $this->updateArticle( @@ -344,9 +392,9 @@ public function testUnpublishedShadows(): void ); // 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)); + self::assertNull($this->findLiveViewDocument($article['id'], $this->locale)); + self::assertNotNull($this->findLiveViewDocument($article['id'], $secondLocale)); + self::assertNull($this->findLiveViewDocument($article['id'], $thirdLocale)); } public function testIndexPageTreeRoute() @@ -364,9 +412,9 @@ public function testIndexPageTreeRoute() ); $document = $this->documentManager->find($article['id'], $this->locale); - $this->indexer->index($document); + $this->liveIndexer->index($document); - $viewDocument = $this->findViewDocument($article['id']); + $viewDocument = $this->findLiveViewDocument($article['id']); $this->assertEquals($page->getUuid(), $viewDocument->getParentPageUuid()); } @@ -374,7 +422,7 @@ public function testSetUnpublished() { $article = $this->createArticle(); - $viewDocument = $this->indexer->setUnpublished($article['id'], $this->locale); + $viewDocument = $this->liveIndexer->setUnpublished($article['id'], $this->locale); $this->assertNull($viewDocument->getPublished()); $this->assertFalse($viewDocument->getPublishedState()); } @@ -411,10 +459,10 @@ public function testIndexTaggedProperties() $this->documentManager->clear(); $document = $this->documentManager->find($article['id'], $this->locale); - $this->indexer->index($document); - $this->indexer->flush(); + $this->liveIndexer->index($document); + $this->liveIndexer->flush(); - $viewDocument = $this->findViewDocument($article['id']); + $viewDocument = $this->findLiveViewDocument($article['id']); $contentFields = $viewDocument->getContentFields(); $this->assertSame($article['id'], $viewDocument->getUuid()); @@ -496,10 +544,10 @@ public function testIndexTaggedPropertiesBlocksInBlocks(): void $this->documentManager->clear(); $document = $this->documentManager->find($article['id'], $this->locale); - $this->indexer->index($document); - $this->indexer->flush(); + $this->liveIndexer->index($document); + $this->liveIndexer->flush(); - $viewDocument = $this->findViewDocument($article['id']); + $viewDocument = $this->findLiveViewDocument($article['id']); $contentFields = $viewDocument->getContentFields(); $this->assertEquals($article['id'], $viewDocument->getUuid()); @@ -532,10 +580,10 @@ public function testIndexContentData() $this->documentManager->clear(); $document = $this->documentManager->find($article['id'], $this->locale); - $this->indexer->index($document); - $this->indexer->flush(); + $this->liveIndexer->index($document); + $this->liveIndexer->flush(); - $viewDocument = $this->findViewDocument($article['id']); + $viewDocument = $this->findLiveViewDocument($article['id']); $this->assertEquals($article['id'], $viewDocument->getUuid()); $this->assertEquals($data, \json_decode($viewDocument->getContentData(), true)); @@ -694,7 +742,23 @@ private function createPage($title = 'Test Page', $resourceSegment = '/test-page } /** - * Find view-document. + * Find view-document in live index. + * + * @param string $uuid + * @param string $locale + * + * @return ArticleViewDocument + */ + private function findLiveViewDocument($uuid, $locale = null) + { + return $this->liveManager->find( + $this->getContainer()->getParameter('sulu_article.view_document.article.class'), + $uuid . '-' . ($locale ? $locale : $this->locale) + ); + } + + /** + * Find view-document in live index. * * @param string $uuid * @param string $locale diff --git a/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php b/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php index f6509564f..a35a8b86e 100644 --- a/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php +++ b/Tests/Unit/Document/Subscriber/ArticleSubscriberTest.php @@ -674,11 +674,40 @@ public function testHandleRemoveLocale() { $event = $this->prophesize(RemoveLocaleEvent::class); $document = $this->prophesize(ArticleDocument::class); + $document->getOriginalLocale()->willReturn('en'); + $document->getUuid()->willReturn($this->uuid); $event->getDocument()->willReturn($document->reveal()); - $event->getLocale()->willReturn('en'); + $event->getLocale()->willReturn('de'); + + $this->documentInspector->getConcreteLocales($document->reveal())->willReturn(['en', 'fr']); + + $originalDocument = $this->prophesize(ArticleDocument::class); + $this->documentManager->find($this->uuid, 'en')->willReturn($originalDocument->reveal()); + + // Ensure the indexer methods are called + $this->indexer->replaceWithGhostData($originalDocument->reveal(), 'de')->shouldBeCalled(); + $this->indexer->flush()->shouldBeCalled(); + + // Call the method + $this->articleSubscriber->handleRemoveLocale($event->reveal()); + } + + public function testHandleRemoveLocaleWithDefaultLocaleDoesNotExists() + { + $event = $this->prophesize(RemoveLocaleEvent::class); + $document = $this->prophesize(ArticleDocument::class); + $document->getOriginalLocale()->willReturn('en'); + $document->getUuid()->willReturn($this->uuid); + $event->getDocument()->willReturn($document->reveal()); + $event->getLocale()->willReturn('de'); + + $this->documentInspector->getConcreteLocales($document->reveal())->willReturn(['fr']); + + $originalDocument = $this->prophesize(ArticleDocument::class); + $this->documentManager->find($this->uuid, 'fr')->willReturn($originalDocument->reveal()); // Ensure the indexer methods are called - $this->indexer->replaceWithGhostData($document->reveal(), 'en')->shouldBeCalled(); + $this->indexer->replaceWithGhostData($originalDocument->reveal(), 'de')->shouldBeCalled(); $this->indexer->flush()->shouldBeCalled(); // Call the method diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7b70dbe0b..0768ecb6f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -10,6 +10,11 @@ parameters: count: 1 path: Admin/ArticleAdmin.php + - + message: "#^Method Sulu\\\\Bundle\\\\ArticleBundle\\\\Admin\\\\ArticleAdmin\\:\\:getSecurityContexts\\(\\) should return array\\\\>\\>\\> but returns array\\\\>\\>\\>\\.$#" + count: 1 + path: Admin/ArticleAdmin.php + - message: "#^Method Sulu\\\\Bundle\\\\ArticleBundle\\\\Admin\\\\ArticleAdmin\\:\\:getTypes\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -2147,7 +2152,7 @@ parameters: - message: "#^Cannot access offset 'pageNumber' on mixed\\.$#" - count: 2 + count: 1 path: Preview/ArticleObjectProvider.php -