From 94e37fd563fa16076b0ddf1596f42edb2dd4582b Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Wed, 10 Jan 2024 15:06:04 +0000 Subject: [PATCH] =?UTF-8?q?Issue=20#2745755=20by=20vasi,=20Steven=20Jones,?= =?UTF-8?q?=20smustgrave,=20mvc,=20Wim=20Leers,=20james.williams,=20G?= =?UTF-8?q?=C3=A1bor=20Hojtsy:=20AliasStorage::preloadPathAlias()=20incorr?= =?UTF-8?q?ectly=20prioritizes=20und=20aliases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../path_alias/src/AliasRepository.php | 19 +- .../path_alias/tests/src/Kernel/AliasTest.php | 244 ++++++++++++++++++ 2 files changed, 258 insertions(+), 5 deletions(-) diff --git a/core/modules/path_alias/src/AliasRepository.php b/core/modules/path_alias/src/AliasRepository.php index e226a4f05d9b..21eb3daef0d6 100644 --- a/core/modules/path_alias/src/AliasRepository.php +++ b/core/modules/path_alias/src/AliasRepository.php @@ -45,12 +45,21 @@ public function preloadPathAlias($preloaded, $langcode) { $this->addLanguageFallback($select, $langcode); - // We order by ID ASC so that fetchAllKeyed() returns the most recently - // created alias for each source. Subsequent queries using fetchField() must - // use ID DESC to have the same effect. - $select->orderBy('base_table.id', 'ASC'); + $select->orderBy('base_table.id', 'DESC'); + + // We want the most recently created alias for each source, however that + // will be at the start of the result-set, so fetch everything and reverse + // it. Note that it would not be sufficient to reverse the ordering of the + // 'base_table.id' column, as that would not guarantee other conditions + // added to the query, such as those in ::addLanguageFallback, would be + // reversed. + $results = $select->execute()->fetchAll(\PDO::FETCH_ASSOC); + $aliases = []; + foreach (array_reverse($results) as $result) { + $aliases[$result['path']] = $result['alias']; + } - return $select->execute()->fetchAllKeyed(); + return $aliases; } /** diff --git a/core/modules/path_alias/tests/src/Kernel/AliasTest.php b/core/modules/path_alias/tests/src/Kernel/AliasTest.php index d78ad96f9684..c46419f94575 100644 --- a/core/modules/path_alias/tests/src/Kernel/AliasTest.php +++ b/core/modules/path_alias/tests/src/Kernel/AliasTest.php @@ -38,6 +38,250 @@ protected function setUp(): void { $this->installEntitySchema('path_alias'); } + /** + * @covers ::preloadPathAlias + */ + public function testPreloadPathAlias() { + $path_alias_repository = $this->container->get('path_alias.repository'); + + // Every interesting language combination: + // Just unspecified. + $this->createPathAlias('/und/src', '/und/alias', LanguageInterface::LANGCODE_NOT_SPECIFIED); + // Just a single language. + $this->createPathAlias('/en/src', '/en/alias', 'en'); + // A single language, plus unspecified. + $this->createPathAlias('/en-und/src', '/en-und/und', LanguageInterface::LANGCODE_NOT_SPECIFIED); + $this->createPathAlias('/en-und/src', '/en-und/en', 'en'); + // Multiple languages. + $this->createPathAlias('/en-xx-lolspeak/src', '/en-xx-lolspeak/en', 'en'); + $this->createPathAlias('/en-xx-lolspeak/src', '/en-xx-lolspeak/xx-lolspeak', 'xx-lolspeak'); + // A duplicate alias for the same path. This is later, so should be + // preferred. + $this->createPathAlias('/en-xx-lolspeak/src', '/en-xx-lolspeak/en-dup', 'en'); + // Multiple languages, plus unspecified. + $this->createPathAlias('/en-xx-lolspeak-und/src', '/en-xx-lolspeak-und/und', LanguageInterface::LANGCODE_NOT_SPECIFIED); + $this->createPathAlias('/en-xx-lolspeak-und/src', '/en-xx-lolspeak-und/en', 'en'); + $this->createPathAlias('/en-xx-lolspeak-und/src', '/en-xx-lolspeak-und/xx-lolspeak', 'xx-lolspeak'); + + // Queries for unspecified language aliases. + // Ask for an empty array, get all results. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-und/src' => '/en-und/und', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/und', + ], + $path_alias_repository->preloadPathAlias([], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + // Ask for nonexistent source. + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/nonexistent'], LanguageInterface::LANGCODE_NOT_SPECIFIED)); + // Ask for each saved source, individually. + $this->assertEquals( + ['/und/src' => '/und/alias'], + $path_alias_repository->preloadPathAlias(['/und/src'], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/en/src'], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + $this->assertEquals( + ['/en-und/src' => '/en-und/und'], + $path_alias_repository->preloadPathAlias(['/en-und/src'], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak/src'], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + $this->assertEquals( + ['/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/und'], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak-und/src'], LanguageInterface::LANGCODE_NOT_SPECIFIED) + ); + // Ask for multiple sources, all that are known. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-und/src' => '/en-und/und', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/und', + ], + $path_alias_repository->preloadPathAlias( + [ + '/nonexistent', + '/und/src', + '/en/src', + '/en-und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + LanguageInterface::LANGCODE_NOT_SPECIFIED + ) + ); + // Ask for multiple sources, just a subset. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/und', + ], + $path_alias_repository->preloadPathAlias( + [ + '/und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + LanguageInterface::LANGCODE_NOT_SPECIFIED + ) + ); + + // Queries for English aliases. + // Ask for an empty array, get all results. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en/src' => '/en/alias', + '/en-und/src' => '/en-und/en', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/en-dup', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/en', + ], + $path_alias_repository->preloadPathAlias([], 'en') + ); + // Ask for nonexistent source. + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/nonexistent'], 'en')); + // Ask for each saved source, individually. + $this->assertEquals( + ['/und/src' => '/und/alias'], + $path_alias_repository->preloadPathAlias(['/und/src'], 'en') + ); + $this->assertEquals( + ['/en/src' => '/en/alias'], + $path_alias_repository->preloadPathAlias(['/en/src'], 'en') + ); + $this->assertEquals( + ['/en-und/src' => '/en-und/en'], + $path_alias_repository->preloadPathAlias(['/en-und/src'], 'en') + ); + $this->assertEquals( + ['/en-xx-lolspeak/src' => '/en-xx-lolspeak/en-dup'], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak/src'], 'en') + ); + $this->assertEquals( + ['/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/en'], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak-und/src'], 'en') + ); + // Ask for multiple sources, all that are known. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en/src' => '/en/alias', + '/en-und/src' => '/en-und/en', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/en-dup', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/en', + ], + $path_alias_repository->preloadPathAlias( + [ + '/nonexistent', + '/und/src', + '/en/src', + '/en-und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + 'en' + ) + ); + // Ask for multiple sources, just a subset. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/en-dup', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/en', + ], + $path_alias_repository->preloadPathAlias( + [ + '/und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + 'en' + ) + ); + + // Queries for xx-lolspeak aliases. + // Ask for an empty array, get all results. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-und/src' => '/en-und/und', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/xx-lolspeak', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/xx-lolspeak', + ], + $path_alias_repository->preloadPathAlias([], 'xx-lolspeak') + ); + // Ask for nonexistent source. + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/nonexistent'], 'xx-lolspeak')); + // Ask for each saved source, individually. + $this->assertEquals( + ['/und/src' => '/und/alias'], + $path_alias_repository->preloadPathAlias(['/und/src'], 'xx-lolspeak') + ); + $this->assertEquals( + [], + $path_alias_repository->preloadPathAlias(['/en/src'], 'xx-lolspeak') + ); + $this->assertEquals( + ['/en-und/src' => '/en-und/und'], + $path_alias_repository->preloadPathAlias(['/en-und/src'], 'xx-lolspeak') + ); + $this->assertEquals( + ['/en-xx-lolspeak/src' => '/en-xx-lolspeak/xx-lolspeak'], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak/src'], 'xx-lolspeak') + ); + $this->assertEquals( + ['/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/xx-lolspeak'], + $path_alias_repository->preloadPathAlias(['/en-xx-lolspeak-und/src'], 'xx-lolspeak') + ); + // Ask for multiple sources, all that are known. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-und/src' => '/en-und/und', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/xx-lolspeak', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/xx-lolspeak', + ], + $path_alias_repository->preloadPathAlias( + [ + '/nonexistent', + '/und/src', + '/en/src', + '/en-und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + 'xx-lolspeak' + ) + ); + // Ask for multiple sources, just a subset. + $this->assertEquals( + [ + '/und/src' => '/und/alias', + '/en-xx-lolspeak/src' => '/en-xx-lolspeak/xx-lolspeak', + '/en-xx-lolspeak-und/src' => '/en-xx-lolspeak-und/xx-lolspeak', + ], + $path_alias_repository->preloadPathAlias( + [ + '/und/src', + '/en-xx-lolspeak/src', + '/en-xx-lolspeak-und/src', + ], + 'xx-lolspeak' + ) + ); + } + /** * @covers ::lookupBySystemPath */ -- GitLab