From 07524a0a18329fd50d6730a075712f4677efed38 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Tue, 19 Mar 2024 14:12:32 +0100 Subject: [PATCH 1/3] IBX-3957: Made NOP URL aliases not reusable and original --- .../UrlAlias/Gateway/DoctrineDatabase.php | 2 +- .../Legacy/Content/UrlAlias/Handler.php | 6 +-- .../Core/Repository/URLAliasServiceTest.php | 45 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/lib/Persistence/Legacy/Content/UrlAlias/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Content/UrlAlias/Gateway/DoctrineDatabase.php index 1b708c059b..88fa221b2f 100644 --- a/src/lib/Persistence/Legacy/Content/UrlAlias/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Content/UrlAlias/Gateway/DoctrineDatabase.php @@ -603,7 +603,7 @@ public function insertRow(array $values): int $values['is_original'] = 1; } if ($values['action'] === self::NOP_ACTION) { - $values['is_original'] = 0; + $values['is_original'] = 1; } $query = $this->connection->createQueryBuilder(); diff --git a/src/lib/Persistence/Legacy/Content/UrlAlias/Handler.php b/src/lib/Persistence/Legacy/Content/UrlAlias/Handler.php index b841ce4bac..a518739f7d 100644 --- a/src/lib/Persistence/Legacy/Content/UrlAlias/Handler.php +++ b/src/lib/Persistence/Legacy/Content/UrlAlias/Handler.php @@ -231,11 +231,9 @@ private function internalPublishUrlAliasForLocation( } // Row exists, check if it is reusable. There are 3 cases when this is possible: - // 1. NOP entry - // 2. existing location or custom alias entry - // 3. history entry + // 1. existing location or custom alias entry + // 2. history entry if ( - $row['action'] === Gateway::NOP_ACTION || $row['action'] === $action || (int)$row['is_original'] === 0 ) { diff --git a/tests/integration/Core/Repository/URLAliasServiceTest.php b/tests/integration/Core/Repository/URLAliasServiceTest.php index 5f1392bbf9..95bd132bf5 100644 --- a/tests/integration/Core/Repository/URLAliasServiceTest.php +++ b/tests/integration/Core/Repository/URLAliasServiceTest.php @@ -1487,6 +1487,51 @@ public function testOverrideHistoryUrlAliasAtTheSameLocation(): void self::assertFalse($newAlias->isHistory); } + public function testRenamingParentContentDoesntBreakChildAlias(): void + { + $repository = $this->getRepository(); + $urlAliasService = $repository->getURLAliasService(); + $contentService = $repository->getContentService(); + + $languageCode = 'eng-GB'; + + // 1. Create parent folder + $folder = $this->createFolder([$languageCode => 'a'], 2); + $folderLocationId = $folder->contentInfo->getMainLocationId(); + + // 2. Create child folder + $child = $this->createFolder([$languageCode => 'b'], $folderLocationId); + $childLocation = $child->getVersionInfo()->getContentInfo()->getMainLocation(); + $childLocationId = $childLocation->id; + + // 3. Create custom URL alias for child folder + $urlAliasService->createUrlAlias($childLocation, '/c/b', $languageCode); + $lookup = $urlAliasService->lookup('/c/b'); + + self::assertSame('/c/b', $lookup->path); + self::assertSame($childLocationId, $lookup->destination); + + // 4. Rename "A" to "C" + $folderDraft = $contentService->createContentDraft($folder->contentInfo); + $folderUpdateStruct = $contentService->newContentUpdateStruct(); + $folderUpdateStruct->setField('name', 'c'); + $renamedFolder = $contentService->updateContent($folderDraft->getVersionInfo(), $folderUpdateStruct); + $contentService->publishVersion($renamedFolder->getVersionInfo()); + + // Loading aliases shouldn't throw a `BadStateException` + $childLocationAliases = $urlAliasService->listLocationAliases($childLocation); + + self::assertCount(1, $childLocationAliases); + self::assertSame('/c/b', $childLocationAliases[0]->path); + + // Renamed content should have '/c2' path alias + $lookupRenamed = $urlAliasService->lookup('c2'); + $originalLookup = $urlAliasService->lookup('/c/b'); + + self::assertSame($childLocationId, $originalLookup->destination); + self::assertSame('/c2', $lookupRenamed->path); + } + /** * Lookup given URL and check if it is archived and points to the given Location Id. * From b6239abe4403d113d08c7dedaa39825ba0e7eb78 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Tue, 19 Mar 2024 14:33:42 +0100 Subject: [PATCH 2/3] IBX-3957: Fixed issues found by PHPStan --- tests/integration/Core/Repository/URLAliasServiceTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/Core/Repository/URLAliasServiceTest.php b/tests/integration/Core/Repository/URLAliasServiceTest.php index 95bd132bf5..a63854baab 100644 --- a/tests/integration/Core/Repository/URLAliasServiceTest.php +++ b/tests/integration/Core/Repository/URLAliasServiceTest.php @@ -1501,6 +1501,7 @@ public function testRenamingParentContentDoesntBreakChildAlias(): void // 2. Create child folder $child = $this->createFolder([$languageCode => 'b'], $folderLocationId); + /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Location $childLocation */ $childLocation = $child->getVersionInfo()->getContentInfo()->getMainLocation(); $childLocationId = $childLocation->id; @@ -1519,6 +1520,7 @@ public function testRenamingParentContentDoesntBreakChildAlias(): void $contentService->publishVersion($renamedFolder->getVersionInfo()); // Loading aliases shouldn't throw a `BadStateException` + /** @var array $childLocationAliases */ $childLocationAliases = $urlAliasService->listLocationAliases($childLocation); self::assertCount(1, $childLocationAliases); From a213b64798c826d137278655b53b32d693f5fce4 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Mon, 13 May 2024 11:04:30 +0200 Subject: [PATCH 3/3] IBX-3957: Applied review remarks --- .../Core/Repository/URLAliasServiceTest.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/integration/Core/Repository/URLAliasServiceTest.php b/tests/integration/Core/Repository/URLAliasServiceTest.php index a63854baab..595796090c 100644 --- a/tests/integration/Core/Repository/URLAliasServiceTest.php +++ b/tests/integration/Core/Repository/URLAliasServiceTest.php @@ -1501,8 +1501,10 @@ public function testRenamingParentContentDoesntBreakChildAlias(): void // 2. Create child folder $child = $this->createFolder([$languageCode => 'b'], $folderLocationId); - /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Location $childLocation */ $childLocation = $child->getVersionInfo()->getContentInfo()->getMainLocation(); + + self::assertInstanceOf(Location::class, $childLocation); + $childLocationId = $childLocation->id; // 3. Create custom URL alias for child folder @@ -1520,11 +1522,11 @@ public function testRenamingParentContentDoesntBreakChildAlias(): void $contentService->publishVersion($renamedFolder->getVersionInfo()); // Loading aliases shouldn't throw a `BadStateException` - /** @var array $childLocationAliases */ $childLocationAliases = $urlAliasService->listLocationAliases($childLocation); + $childLocationAliasesUnpacked = iterator_to_array($childLocationAliases); - self::assertCount(1, $childLocationAliases); - self::assertSame('/c/b', $childLocationAliases[0]->path); + self::assertCount(1, $childLocationAliasesUnpacked); + self::assertSame('/c/b', $childLocationAliasesUnpacked[0]->path); // Renamed content should have '/c2' path alias $lookupRenamed = $urlAliasService->lookup('c2');