[ticket/12683] Improve exception handling

PHPBB3-12683

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.

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_SERVICE_NOT_FOUND'				=> 'Service for class "%1$s" not found in collection.',
-	'DI_MULTIPLE_SERVICE_DEFINITIONS'	=> 'More than one service definitions found for class "%s" in collection.',
+	'DI_MULTIPLE_SERVICE_DEFINITIONS'	=> 'More than one service definitions found for class "%1$s" in collection.',
 
 	'EDIT_POST'							=> 'Edit post',
 	'ELLIPSIS'							=>	'…',

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;
 		}

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;
 		}

phpBB/phpbb/di/exception/multiple_service_definitions_exception.php

@@ -1,4 +1,15 @@
 <?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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;
 

phpBB/phpbb/di/exception/service_not_found_exception.php

@@ -1,4 +1,15 @@
 <?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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;
 

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());
+	}
 }

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());
+	}
 }

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();
 	}
 }
+

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');
 	}

tests/mock/search_backend_mock.php

@@ -1,4 +1,15 @@
 <?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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;
 	}
 }
+

tests/mock/search_backend_mock_not_available.php

@@ -0,0 +1,25 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @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;
+	}
+}