diff --git a/administrator/components/com_finder/src/Controller/IndexerController.php b/administrator/components/com_finder/src/Controller/IndexerController.php index 4291c4924ef4e..ae91c25c37d6f 100644 --- a/administrator/components/com_finder/src/Controller/IndexerController.php +++ b/administrator/components/com_finder/src/Controller/IndexerController.php @@ -264,6 +264,21 @@ public function optimize() } } + /** + * Method to get a valid HTTP error status for an exception code. Exceptions such as database + * errors carry codes that are not valid HTTP statuses, e.g. 0, which must not be sent as header. + * + * @param integer $code The exception code. + * + * @return integer The code if it is a HTTP error status, otherwise 500. + * + * @since __DEPLOY_VERSION__ + */ + public static function getHttpStatus(int $code): int + { + return $code >= 400 && $code < 600 ? $code : 500; + } + /** * Method to handle a send a \JSON response. The body parameter * can be an \Exception object for when an error has occurred or @@ -294,7 +309,7 @@ public static function sendResponse($data = null) // Informational log only } - $app->setHeader('status', $data->getCode()); + $app->setHeader('status', static::getHttpStatus((int) $data->getCode())); } // Create the response object. diff --git a/administrator/components/com_finder/src/Indexer/Taxonomy.php b/administrator/components/com_finder/src/Indexer/Taxonomy.php index 5091a21b92d4a..1d995196daff9 100644 --- a/administrator/components/com_finder/src/Indexer/Taxonomy.php +++ b/administrator/components/com_finder/src/Indexer/Taxonomy.php @@ -449,7 +449,7 @@ public static function removeOrphanNodes() break; } - $query2->clear('where')->whereIn($db->quoteName('id'), $nodes); + $query2->clear('where')->clear('bounded')->whereIn($db->quoteName('id'), $nodes); $db->setQuery($query2); $db->execute(); $affectedRows += $db->getAffectedRows(); diff --git a/tests/Unit/Component/Finder/Administrator/Controller/IndexerControllerTest.php b/tests/Unit/Component/Finder/Administrator/Controller/IndexerControllerTest.php new file mode 100644 index 0000000000000..79640a217b519 --- /dev/null +++ b/tests/Unit/Component/Finder/Administrator/Controller/IndexerControllerTest.php @@ -0,0 +1,63 @@ + + * @license GNU General Public License version 2 or later; see LICENSE.txt + */ + +namespace Joomla\Tests\Unit\Component\Finder\Administrator\Controller; + +use Joomla\Component\Finder\Administrator\Controller\IndexerController; +use Joomla\Tests\Unit\UnitTestCase; +use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; + +/** + * Test class for \Joomla\Component\Finder\Administrator\Controller\IndexerController + * + * @since __DEPLOY_VERSION__ + */ +#[CoversMethod(IndexerController::class, 'getHttpStatus')] +class IndexerControllerTest extends UnitTestCase +{ + /** + * Exception codes which are not HTTP error statuses, e.g. 0 for database errors, must not be + * sent as status header, as the web server rejects them. + * + * @param integer $code The exception code + * @param integer $expected The expected HTTP status + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + #[DataProvider('statusProvider')] + public function testGetHttpStatus(int $code, int $expected): void + { + $this->assertSame($expected, IndexerController::getHttpStatus($code)); + } + + /** + * @return array + * + * @since __DEPLOY_VERSION__ + */ + public static function statusProvider(): array + { + return [ + 'database error' => [0, 500], + 'driver error code' => [1045, 500], + 'success status' => [200, 500], + 'negative' => [-1, 500], + 'lowest client error' => [400, 400], + 'forbidden' => [403, 403], + 'not found' => [404, 404], + 'server error' => [500, 500], + 'highest server error' => [599, 599], + 'out of range' => [600, 500], + ]; + } +} diff --git a/tests/Unit/Component/Finder/Administrator/Indexer/TaxonomyTest.php b/tests/Unit/Component/Finder/Administrator/Indexer/TaxonomyTest.php new file mode 100644 index 0000000000000..d6219d1813580 --- /dev/null +++ b/tests/Unit/Component/Finder/Administrator/Indexer/TaxonomyTest.php @@ -0,0 +1,161 @@ + + * @license GNU General Public License version 2 or later; see LICENSE.txt + */ + +namespace Joomla\Tests\Unit\Component\Finder\Administrator\Indexer; + +use Joomla\CMS\Application\CMSApplicationInterface; +use Joomla\CMS\Factory; +use Joomla\Component\Finder\Administrator\Indexer\Taxonomy; +use Joomla\Database\DatabaseInterface; +use Joomla\Database\DatabaseQuery; +use Joomla\DI\Container; +use Joomla\Event\Dispatcher; +use Joomla\Tests\Unit\UnitTestCase; +use PHPUnit\Framework\Attributes\CoversMethod; + +/** + * Test class for \Joomla\Component\Finder\Administrator\Indexer\Taxonomy + * + * @since __DEPLOY_VERSION__ + */ +#[CoversMethod(Taxonomy::class, 'removeOrphanNodes')] +class TaxonomyTest extends UnitTestCase +{ + /** + * @var \Joomla\CMS\Application\CMSApplicationInterface|null + */ + private $previousApplication; + + /** + * @var \Joomla\DI\Container|null + */ + private $previousContainer; + + /** + * @return void + * + * @since __DEPLOY_VERSION__ + */ + protected function setUp(): void + { + $this->previousApplication = Factory::$application; + $this->previousContainer = Factory::$container; + } + + /** + * @return void + * + * @since __DEPLOY_VERSION__ + */ + protected function tearDown(): void + { + Factory::$application = $this->previousApplication; + Factory::$container = $this->previousContainer; + } + + /** + * Returns a real query object so that bound values and placeholders behave as in production. + * + * @param DatabaseInterface $db The database + * + * @return DatabaseQuery + * + * @since __DEPLOY_VERSION__ + */ + private function createQuery(DatabaseInterface $db): DatabaseQuery + { + return new class ($db) extends DatabaseQuery { + public function groupConcat($expression, $separator = ',') + { + return $expression; + } + + public function processLimit($query, $limit, $offset = 0) + { + return $query; + } + }; + } + + /** + * Orphan nodes chained over several levels are removed in several passes. Every pass has to + * execute a delete query whose bound values match its placeholders, otherwise the PDO driver + * fails with SQLSTATE HY093. + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testRemoveOrphanNodesBindsOnlyCurrentPassValues(): void + { + // Each pass finds the parents of the nodes deleted by the previous one + $passes = [[3, 4], [2], [1]]; + $executed = []; + + $db = $this->createStub(DatabaseInterface::class); + $db->method('quoteName')->willReturnArgument(0); + $db->method('getServerType')->willReturn('mysql'); + $db->method('getName')->willReturn('mysql'); + $db->method('getTableColumns')->willReturn(array_fill_keys(['id', 'parent_id', 'lft', 'rgt', 'level', 'path', 'title', 'alias', 'state', 'access'], 'int')); + $db->method('getAffectedRows')->willReturn(1); + $db->method('createQuery')->willReturnCallback(fn() => $this->createQuery($db)); + + $current = null; + $db->method('setQuery')->willReturnCallback( + function ($query) use ($db, &$current) { + $current = $query; + + return $db; + } + ); + $db->method('loadObjectList')->willReturn([]); + $db->method('loadColumn')->willReturnCallback( + function () use (&$current, &$passes) { + // Only the orphan lookup is answered, the nested set rebuild finds nothing + return str_contains((string) $current, 'finder_taxonomy_map') ? (array) array_shift($passes) : []; + } + ); + $db->method('execute')->willReturnCallback( + function () use (&$current, &$executed) { + if ($current instanceof DatabaseQuery && str_starts_with(trim((string) $current), 'DELETE')) { + $executed[] = [ + 'placeholders' => preg_match_all('/:\w+/', (string) $current), + 'bound' => \count($current->getBounded()), + ]; + } + + return true; + } + ); + + $app = $this->createStub(CMSApplicationInterface::class); + $app->method('get')->willReturn(1); + $app->method('getDispatcher')->willReturn(new Dispatcher()); + + $container = new Container(); + $container->set(DatabaseInterface::class, $db); + $container->alias('DatabaseDriver', DatabaseInterface::class); + + Factory::$application = $app; + Factory::$container = $container; + + Taxonomy::removeOrphanNodes(); + + $this->assertCount(3, $executed, 'One delete query is expected per pass'); + + foreach ($executed as $pass => $delete) { + $this->assertSame( + $delete['placeholders'], + $delete['bound'], + 'Pass ' . ($pass + 1) . ' must only bind the values of its own placeholders' + ); + } + } +}