From f9f55eb0125b6263682981ab4c1be62b70d90882 Mon Sep 17 00:00:00 2001 From: Ruben Calvo Date: Sun, 11 Dec 2022 16:42:02 +0100 Subject: [PATCH] [ticket/12683] Improve exception handling PHPBB3-12683 --- phpBB/language/en/cli.php | 1 + phpBB/language/en/common.php | 4 +-- .../console/command/searchindex/create.php | 10 ++++++++ .../console/command/searchindex/delete.php | 10 ++++++++ ...multiple_service_definitions_exception.php | 11 ++++++++ .../exception/service_not_found_exception.php | 11 ++++++++ tests/console/searchindex/create_test.php | 12 +++++++++ tests/console/searchindex/delete_test.php | 12 +++++++++ .../phpbb_console_searchindex_base.php | 10 +++++++- tests/di/service_collection_test.php | 4 +-- tests/mock/search_backend_mock.php | 12 +++++++++ .../search_backend_mock_not_available.php | 25 +++++++++++++++++++ 12 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 tests/mock/search_backend_mock_not_available.php diff --git a/phpBB/language/en/cli.php b/phpBB/language/en/cli.php index 8ad12848ae..80240b9cf2 100644 --- a/phpBB/language/en/cli.php +++ b/phpBB/language/en/cli.php @@ -154,6 +154,7 @@ $lang = array_merge($lang, array( 'CLI_SEARCHINDEX_DELETE_FAILURE' => 'Error deleting search index', 'CLI_SEARCHINDEX_ACTION_IN_PROGRESS' => 'There is an action currently in progress. CLI doesn’t support incomplete index/delete actions, please solve it from the ACP.', 'CLI_SEARCHINDEX_ACTIVE_NOT_INDEXED' => 'Active search backend isn’t indexed', + 'CLI_SEARCHINDEX_BACKEND_NOT_AVAILABLE' => 'Search backend isn’t available.', // In all the case %1$s is the logical name of the file and %2$s the real name on the filesystem // eg: big_image.png (2_a51529ae7932008cf8454a95af84cacd) generated. diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index 8333755f87..8d71ca2952 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -215,8 +215,8 @@ $lang = array_merge($lang, array( 2 => 'Downloaded %d times', ), - 'DI_SERVICE_NOT_FOUND' => 'No service found for class "%s" in collection.', - 'DI_MULTIPLE_SERVICE_DEFINITIONS' => 'More than one service definitions found for class "%s" in collection.', + 'DI_SERVICE_NOT_FOUND' => 'Service for class "%1$s" not found in collection.', + 'DI_MULTIPLE_SERVICE_DEFINITIONS' => 'More than one service definitions found for class "%1$s" in collection.', 'EDIT_POST' => 'Edit post', 'ELLIPSIS' => '…', diff --git a/phpBB/phpbb/console/command/searchindex/create.php b/phpBB/phpbb/console/command/searchindex/create.php index 9d6c71a67b..9a1d54444e 100644 --- a/phpBB/phpbb/console/command/searchindex/create.php +++ b/phpBB/phpbb/console/command/searchindex/create.php @@ -62,6 +62,8 @@ class create extends command $this->search_backend_factory = $search_backend_factory; $this->state_helper = $state_helper; + $this->language->add_lang(array('acp/common', 'acp/search')); + parent::__construct($user); } @@ -118,6 +120,12 @@ class create extends command return symfony_command::FAILURE; } + if (!$search->is_available()) + { + $io->error($this->language->lang('CLI_SEARCHINDEX_BACKEND_NOT_AVAILABLE', $search_backend)); + return symfony_command::FAILURE; + } + try { $progress = $this->create_progress_bar($this->post_helper->get_max_post_id(), $io, $output, true); @@ -141,6 +149,8 @@ class create extends command } catch (\Exception $e) { + $this->state_helper->clear_state(); // Unexpected error, cancel action + $io->error($e->getMessage()); // Show also exception message like in acp $io->error($this->language->lang('CLI_SEARCHINDEX_CREATE_FAILURE', $name)); return symfony_command::FAILURE; } diff --git a/phpBB/phpbb/console/command/searchindex/delete.php b/phpBB/phpbb/console/command/searchindex/delete.php index 78e045b5c8..02c94cf082 100644 --- a/phpBB/phpbb/console/command/searchindex/delete.php +++ b/phpBB/phpbb/console/command/searchindex/delete.php @@ -62,6 +62,8 @@ class delete extends command $this->search_backend_factory = $search_backend_factory; $this->state_helper = $state_helper; + $this->language->add_lang(array('acp/common', 'acp/search')); + parent::__construct($user); } @@ -118,6 +120,12 @@ class delete extends command return symfony_command::FAILURE; } + if (!$search->is_available()) + { + $io->error($this->language->lang('CLI_SEARCHINDEX_BACKEND_NOT_AVAILABLE', $search_backend)); + return symfony_command::FAILURE; + } + try { $progress = $this->create_progress_bar($this->post_helper->get_max_post_id(), $io, $output, true); @@ -141,6 +149,8 @@ class delete extends command } catch (\Exception $e) { + $this->state_helper->clear_state(); // Unexpected error, cancel action + $io->error($e->getMessage()); // Show also exception message like in acp $io->error($this->language->lang('CLI_SEARCHINDEX_DELETE_FAILURE', $name)); return symfony_command::FAILURE; } diff --git a/phpBB/phpbb/di/exception/multiple_service_definitions_exception.php b/phpBB/phpbb/di/exception/multiple_service_definitions_exception.php index 66dc71489e..1c89b7a229 100644 --- a/phpBB/phpbb/di/exception/multiple_service_definitions_exception.php +++ b/phpBB/phpbb/di/exception/multiple_service_definitions_exception.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\di\exception; diff --git a/phpBB/phpbb/di/exception/service_not_found_exception.php b/phpBB/phpbb/di/exception/service_not_found_exception.php index 22f5c89ed4..06cb909d9c 100644 --- a/phpBB/phpbb/di/exception/service_not_found_exception.php +++ b/phpBB/phpbb/di/exception/service_not_found_exception.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ namespace phpbb\di\exception; diff --git a/tests/console/searchindex/create_test.php b/tests/console/searchindex/create_test.php index 88c42ab897..4e6a438f29 100644 --- a/tests/console/searchindex/create_test.php +++ b/tests/console/searchindex/create_test.php @@ -66,4 +66,16 @@ class phpbb_console_searchindex_create_test extends phpbb_console_searchindex_ba $this->config['search_indexing_state'] = []; } + + public function test_create_when_search_backend_not_available() + { + $command_tester = $this->get_command_tester(); + + $command_tester->execute([ + 'search-backend' => 'search_backend_mock_not_available', + ]); + + $this->assertEquals(Command::FAILURE, $command_tester->getStatusCode()); + $this->assertStringContainsString('CLI_SEARCHINDEX_BACKEND_NOT_AVAILABLE', $command_tester->getDisplay()); + } } diff --git a/tests/console/searchindex/delete_test.php b/tests/console/searchindex/delete_test.php index 5551231afa..1d920648ba 100644 --- a/tests/console/searchindex/delete_test.php +++ b/tests/console/searchindex/delete_test.php @@ -66,4 +66,16 @@ class phpbb_console_searchindex_delete_test extends phpbb_console_searchindex_ba $this->config['search_indexing_state'] = []; } + + public function test_delete_when_search_backend_not_available() + { + $command_tester = $this->get_command_tester(); + + $command_tester->execute([ + 'search-backend' => 'search_backend_mock_not_available', + ]); + + $this->assertEquals(Command::FAILURE, $command_tester->getStatusCode()); + $this->assertStringContainsString('CLI_SEARCHINDEX_BACKEND_NOT_AVAILABLE', $command_tester->getDisplay()); + } } diff --git a/tests/console/searchindex/phpbb_console_searchindex_base.php b/tests/console/searchindex/phpbb_console_searchindex_base.php index 58d850cc6e..209d12281a 100644 --- a/tests/console/searchindex/phpbb_console_searchindex_base.php +++ b/tests/console/searchindex/phpbb_console_searchindex_base.php @@ -21,6 +21,7 @@ use phpbb\search\state_helper; use phpbb\user; require_once __DIR__ . '/../../mock/search_backend_mock.php'; +require_once __DIR__ . '/../../mock/search_backend_mock_not_available.php'; class phpbb_console_searchindex_base extends phpbb_test_case { @@ -70,12 +71,18 @@ class phpbb_console_searchindex_base extends phpbb_test_case $this->user = $this->createMock('\phpbb\user'); $phpbb_container = new phpbb_mock_container_builder(); - $search_backend_mock = new search_backend_mock(); $this->search_backend_collection = new \phpbb\di\service_collection($phpbb_container); + + $search_backend_mock = new search_backend_mock(); $this->search_backend_collection->add('search_backend_mock'); $this->search_backend_collection->add_service_class('search_backend_mock', 'search_backend_mock'); $phpbb_container->set('search_backend_mock', $search_backend_mock); + $search_backend_mock_not_available = new search_backend_mock_not_available(); + $this->search_backend_collection->add('search_backend_mock_not_available'); + $this->search_backend_collection->add_service_class('search_backend_mock_not_available', 'search_backend_mock_not_available'); + $phpbb_container->set('search_backend_mock_not_available', $search_backend_mock_not_available); + $this->search_backend_factory = new search_backend_factory($this->config, $this->search_backend_collection); $this->state_helper = new state_helper($this->config, $this->search_backend_factory); @@ -83,3 +90,4 @@ class phpbb_console_searchindex_base extends phpbb_test_case parent::setUp(); } } + diff --git a/tests/di/service_collection_test.php b/tests/di/service_collection_test.php index fd0e13e48b..7fc3e94e41 100644 --- a/tests/di/service_collection_test.php +++ b/tests/di/service_collection_test.php @@ -57,7 +57,7 @@ class phpbb_service_collection_test extends \phpbb_test_case public function test_get_by_class_many_services_exception() { $this->expectException('RuntimeException'); - $this->expectExceptionMessage('More than one service definitions found for class "bar_class" in collection.'); + $this->expectExceptionMessage('DI_MULTIPLE_SERVICE_DEFINITIONS'); $this->service_collection->get_by_class('bar_class'); } @@ -65,7 +65,7 @@ class phpbb_service_collection_test extends \phpbb_test_case public function test_get_by_class_no_service_exception() { $this->expectException('RuntimeException'); - $this->expectExceptionMessage('No service found for class "baz_class" in collection.'); + $this->expectExceptionMessage('DI_SERVICE_NOT_FOUND'); $this->service_collection->get_by_class('baz_class'); } diff --git a/tests/mock/search_backend_mock.php b/tests/mock/search_backend_mock.php index 0f2db43eaf..84dd26385e 100644 --- a/tests/mock/search_backend_mock.php +++ b/tests/mock/search_backend_mock.php @@ -1,4 +1,15 @@ + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ use phpbb\search\backend\search_backend_interface; @@ -103,3 +114,4 @@ class search_backend_mock implements search_backend_interface return static::class; } } + diff --git a/tests/mock/search_backend_mock_not_available.php b/tests/mock/search_backend_mock_not_available.php new file mode 100644 index 0000000000..5b299d334f --- /dev/null +++ b/tests/mock/search_backend_mock_not_available.php @@ -0,0 +1,25 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class search_backend_mock_not_available extends search_backend_mock +{ + public function get_name(): string + { + return 'Mock unavailable search backend'; + } + + public function is_available(): bool + { + return false; + } +}