From ed291843f2b00fca43180313c3b2d10bbb66f7cb Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 4 May 2021 21:23:21 +0200 Subject: [PATCH] [ticket/13713] Add type hints and clean up code PHPBB3-13713 --- phpBB/phpbb/mention/controller/mention.php | 17 +++++++-- phpBB/phpbb/mention/source/base_group.php | 37 +++++++++++++------ phpBB/phpbb/mention/source/base_user.php | 26 +++++++++---- phpBB/phpbb/mention/source/friend.php | 19 +++++----- phpBB/phpbb/mention/source/group.php | 11 +++--- .../phpbb/mention/source/source_interface.php | 4 +- phpBB/phpbb/mention/source/team.php | 5 +-- phpBB/phpbb/mention/source/topic.php | 32 +++++++--------- phpBB/phpbb/mention/source/user.php | 15 ++++---- phpBB/phpbb/mention/source/usergroup.php | 15 ++++---- 10 files changed, 102 insertions(+), 79 deletions(-) diff --git a/phpBB/phpbb/mention/controller/mention.php b/phpBB/phpbb/mention/controller/mention.php index eec3fbc5d1..10e21f5cfc 100644 --- a/phpBB/phpbb/mention/controller/mention.php +++ b/phpBB/phpbb/mention/controller/mention.php @@ -13,15 +13,17 @@ namespace phpbb\mention\controller; +use phpbb\di\service_collection; +use phpbb\request\request_interface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; class mention { - /** @var \phpbb\di\service_collection */ + /** @var service_collection */ protected $mention_sources; - /** @var \phpbb\request\request_interface */ + /** @var request_interface */ protected $request; /** @var string */ @@ -33,8 +35,12 @@ class mention /** * Constructor * + * @param array $mention_sources + * @param request_interface $request + * @param string $phpbb_root_path + * @param string $phpEx */ - public function __construct($mention_sources, \phpbb\request\request_interface $request, $phpbb_root_path, $phpEx) + public function __construct(array $mention_sources, request_interface $request, string $phpbb_root_path, string $phpEx) { $this->mention_sources = $mention_sources; $this->request = $request; @@ -42,6 +48,11 @@ class mention $this->php_ext = $phpEx; } + /** + * Handle requests to mention controller + * + * @return JsonResponse|RedirectResponse + */ public function handle() { if (!$this->request->is_ajax()) diff --git a/phpBB/phpbb/mention/source/base_group.php b/phpBB/phpbb/mention/source/base_group.php index 126c01fa58..58fca4d054 100644 --- a/phpBB/phpbb/mention/source/base_group.php +++ b/phpBB/phpbb/mention/source/base_group.php @@ -13,21 +13,26 @@ namespace phpbb\mention\source; +use phpbb\auth\auth; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\group\helper; + abstract class base_group implements source_interface { - /** @var \phpbb\db\driver\driver_interface */ + /** @var driver_interface */ protected $db; - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var \phpbb\group\helper */ + /** @var helper */ protected $helper; /** @var \phpbb\user */ protected $user; - /** @var \phpbb\auth\auth */ + /** @var auth */ protected $auth; /** @var string */ @@ -43,9 +48,17 @@ abstract class base_group implements source_interface protected $groups = null; /** - * Constructor + * base_group constructor. + * + * @param driver_interface $db + * @param config $config + * @param helper $helper + * @param \phpbb\user $user + * @param auth $auth + * @param string $phpbb_root_path + * @param string $phpEx */ - public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\group\helper $helper, \phpbb\user $user, \phpbb\auth\auth $auth, $phpbb_root_path, $phpEx) + public function __construct(driver_interface $db, config $config, helper $helper, \phpbb\user $user, auth $auth, string $phpbb_root_path, string $phpEx) { $this->db = $db; $this->config = $config; @@ -66,13 +79,13 @@ abstract class base_group implements source_interface * * @return array Array of groups' data */ - protected function get_groups() + protected function get_groups(): array { if (is_null($this->groups)) { $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'g.*, ug.user_id as ug_user_id', - 'FROM' => [ + 'SELECT' => 'g.*, ug.user_id as ug_user_id', + 'FROM' => [ GROUPS_TABLE => 'g', ], 'LEFT_JOIN' => [ @@ -112,12 +125,12 @@ abstract class base_group implements source_interface * @param int $topic_id Current topic ID * @return string Query ready for execution */ - abstract protected function query($keyword, $topic_id); + abstract protected function query(string $keyword, int $topic_id): string; /** * {@inheritdoc} */ - public function get_priority($row) + public function get_priority(array $row): int { // By default every result from the source increases the priority by a fixed value return 1; @@ -126,7 +139,7 @@ abstract class base_group implements source_interface /** * {@inheritdoc} */ - public function get(array &$names, $keyword, $topic_id) + public function get(array &$names, string $keyword, int $topic_id): bool { // Grab all group IDs and cache them if needed $result = $this->db->sql_query($this->query($keyword, $topic_id), $this->cache_ttl); diff --git a/phpBB/phpbb/mention/source/base_user.php b/phpBB/phpbb/mention/source/base_user.php index f0d01fa8e1..7e9b41d67d 100644 --- a/phpBB/phpbb/mention/source/base_user.php +++ b/phpBB/phpbb/mention/source/base_user.php @@ -13,15 +13,19 @@ namespace phpbb\mention\source; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\user_loader; + abstract class base_user implements source_interface { - /** @var \phpbb\db\driver\driver_interface */ + /** @var driver_interface */ protected $db; - /** @var \phpbb\config\config */ + /** @var config */ protected $config; - /** @var \phpbb\user_loader */ + /** @var user_loader */ protected $user_loader; /** @var string */ @@ -34,9 +38,15 @@ abstract class base_user implements source_interface protected $cache_ttl = false; /** - * Constructor + * base_user constructor. + * + * @param driver_interface $db + * @param config $config + * @param user_loader $user_loader + * @param string $phpbb_root_path + * @param string $phpEx */ - public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\user_loader $user_loader, $phpbb_root_path, $phpEx) + public function __construct(driver_interface $db, config $config, user_loader $user_loader, string $phpbb_root_path, string $phpEx) { $this->db = $db; $this->config = $config; @@ -57,12 +67,12 @@ abstract class base_user implements source_interface * @param int $topic_id Current topic ID * @return string Query ready for execution */ - abstract protected function query($keyword, $topic_id); + abstract protected function query(string $keyword, int $topic_id): string; /** * {@inheritdoc} */ - public function get_priority($row) + public function get_priority(array $row): int { // By default every result from the source increases the priority by a fixed value return 1; @@ -71,7 +81,7 @@ abstract class base_user implements source_interface /** * {@inheritdoc} */ - public function get(array &$names, $keyword, $topic_id) + public function get(array &$names, string $keyword, int $topic_id): bool { $fetched_all = false; $keyword = utf8_clean_string($keyword); diff --git a/phpBB/phpbb/mention/source/friend.php b/phpBB/phpbb/mention/source/friend.php index ca16a374b5..5c7a3f91ef 100644 --- a/phpBB/phpbb/mention/source/friend.php +++ b/phpBB/phpbb/mention/source/friend.php @@ -23,7 +23,7 @@ class friend extends base_user * * @param \phpbb\user $user */ - public function set_user(\phpbb\user $user) + public function set_user(\phpbb\user $user): void { $this->user = $user; } @@ -31,29 +31,28 @@ class friend extends base_user /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { /* * For optimization purposes all friends are returned regardless of the keyword * Names filtering is done on the frontend * Results will be cached on a per-user basis */ - $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'u.username_clean, u.user_id', - 'FROM' => [ + return $this->db->sql_build_query('SELECT', [ + 'SELECT' => 'u.username_clean, u.user_id', + 'FROM' => [ USERS_TABLE => 'u', ], 'LEFT_JOIN' => [ [ - 'FROM' => [ZEBRA_TABLE => 'z'], - 'ON' => 'u.user_id = z.zebra_id' + 'FROM' => [ZEBRA_TABLE => 'z'], + 'ON' => 'u.user_id = z.zebra_id' ] ], - 'WHERE' => 'z.friend = 1 AND z.user_id = ' . (int) $this->user->data['user_id'] . ' + 'WHERE' => 'z.friend = 1 AND z.user_id = ' . (int) $this->user->data['user_id'] . ' AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . ' AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()), - 'ORDER_BY' => 'u.user_lastvisit DESC' + 'ORDER_BY' => 'u.user_lastvisit DESC' ]); - return $query; } } diff --git a/phpBB/phpbb/mention/source/group.php b/phpBB/phpbb/mention/source/group.php index 2b13ca7be0..11a8e02e94 100644 --- a/phpBB/phpbb/mention/source/group.php +++ b/phpBB/phpbb/mention/source/group.php @@ -21,7 +21,7 @@ class group extends base_group /** * {@inheritdoc} */ - public function get_priority($row) + public function get_priority(array $row): int { /* * Presence in array with all names for this type should not increase the priority @@ -35,15 +35,14 @@ class group extends base_group /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { - $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'g.group_id', - 'FROM' => [ + return $this->db->sql_build_query('SELECT', [ + 'SELECT' => 'g.group_id', + 'FROM' => [ GROUPS_TABLE => 'g', ], 'ORDER_BY' => 'g.group_name', ]); - return $query; } } diff --git a/phpBB/phpbb/mention/source/source_interface.php b/phpBB/phpbb/mention/source/source_interface.php index b9e126324b..2fe45ef234 100644 --- a/phpBB/phpbb/mention/source/source_interface.php +++ b/phpBB/phpbb/mention/source/source_interface.php @@ -24,7 +24,7 @@ interface source_interface * @param int $topic_id Current topic ID * @return bool Whether there are no more satisfying names left */ - public function get(array &$names, $keyword, $topic_id); + public function get(array &$names, string $keyword, int $topic_id): bool; /** * Returns the priority of the currently selected name @@ -34,5 +34,5 @@ interface source_interface * @param array $row Array of fetched data for the name type (e.g. user row) * @return int Priority (defaults to 1) */ - public function get_priority($row); + public function get_priority(array $row): int; } diff --git a/phpBB/phpbb/mention/source/team.php b/phpBB/phpbb/mention/source/team.php index 14281a85bf..02fd8cefbb 100644 --- a/phpBB/phpbb/mention/source/team.php +++ b/phpBB/phpbb/mention/source/team.php @@ -21,7 +21,7 @@ class team extends base_user /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { /* * Select unique names of team members: each name should be selected only once @@ -31,7 +31,7 @@ class team extends base_user * Names filtering is done on the frontend * Results will be cached in a single file */ - $query = $this->db->sql_build_query('SELECT_DISTINCT', [ + return $this->db->sql_build_query('SELECT_DISTINCT', [ 'SELECT' => 'u.username_clean, u.user_id', 'FROM' => [ USERS_TABLE => 'u', @@ -42,6 +42,5 @@ class team extends base_user AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]), 'ORDER_BY' => 'u.username_clean' ]); - return $query; } } diff --git a/phpBB/phpbb/mention/source/topic.php b/phpBB/phpbb/mention/source/topic.php index 0c630aa22e..842d38c4ef 100644 --- a/phpBB/phpbb/mention/source/topic.php +++ b/phpBB/phpbb/mention/source/topic.php @@ -18,53 +18,47 @@ class topic extends base_user /** * {@inheritdoc} */ - public function get_priority($row) + public function get_priority(array $row): int { /* * Topic's open poster is probably the most mentionable user in the topic * so we give him a significant priority */ - if ($row['user_id'] === $row['topic_poster']) - { - return 5; - } - - return 1; + return $row['user_id'] === $row['topic_poster'] ? 5 : 1; } /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { /* * Select poster's username together with topic author's ID - * that will be later used for priotirisation + * that will be later used for prioritisation * * For optimization purposes all users are returned regardless of the keyword * Names filtering is done on the frontend * Results will be cached on a per-topic basis */ - $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'u.username_clean, u.user_id, t.topic_poster', - 'FROM' => [ + return $this->db->sql_build_query('SELECT', [ + 'SELECT' => 'u.username_clean, u.user_id, t.topic_poster', + 'FROM' => [ USERS_TABLE => 'u', ], 'LEFT_JOIN' => [ [ - 'FROM' => [POSTS_TABLE => 'p'], - 'ON' => 'u.user_id = p.poster_id' + 'FROM' => [POSTS_TABLE => 'p'], + 'ON' => 'u.user_id = p.poster_id' ], [ - 'FROM' => [TOPICS_TABLE => 't'], - 'ON' => 't.topic_id = p.topic_id' + 'FROM' => [TOPICS_TABLE => 't'], + 'ON' => 't.topic_id = p.topic_id' ], ], - 'WHERE' => 'p.topic_id = ' . (int) $topic_id . ' + 'WHERE' => 'p.topic_id = ' . (int) $topic_id . ' AND ' . $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . ' AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()), - 'ORDER_BY' => 'p.post_time DESC' + 'ORDER_BY' => 'p.post_time DESC' ]); - return $query; } } diff --git a/phpBB/phpbb/mention/source/user.php b/phpBB/phpbb/mention/source/user.php index a85d3d3be4..3189f32b83 100644 --- a/phpBB/phpbb/mention/source/user.php +++ b/phpBB/phpbb/mention/source/user.php @@ -18,7 +18,7 @@ class user extends base_user /** * {@inheritdoc} */ - public function get_priority($row) + public function get_priority(array $row): int { /* * Presence in array with all names for this type should not increase the priority @@ -32,17 +32,16 @@ class user extends base_user /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { - $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'u.username_clean, u.user_id', - 'FROM' => [ + return $this->db->sql_build_query('SELECT', [ + 'SELECT' => 'u.username_clean, u.user_id', + 'FROM' => [ USERS_TABLE => 'u', ], - 'WHERE' => $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . ' + 'WHERE' => $this->db->sql_in_set('u.user_type', [USER_NORMAL, USER_FOUNDER]) . ' AND u.username_clean ' . $this->db->sql_like_expression($keyword . $this->db->get_any_char()), - 'ORDER_BY' => 'u.user_lastvisit DESC' + 'ORDER_BY' => 'u.user_lastvisit DESC' ]); - return $query; } } diff --git a/phpBB/phpbb/mention/source/usergroup.php b/phpBB/phpbb/mention/source/usergroup.php index b3b3e71ded..de02cd76d6 100644 --- a/phpBB/phpbb/mention/source/usergroup.php +++ b/phpBB/phpbb/mention/source/usergroup.php @@ -18,22 +18,21 @@ class usergroup extends base_group /** * {@inheritdoc} */ - protected function query($keyword, $topic_id) + protected function query(string $keyword, int $topic_id): string { - $query = $this->db->sql_build_query('SELECT', [ - 'SELECT' => 'g.group_id', - 'FROM' => [ + return $this->db->sql_build_query('SELECT', [ + 'SELECT' => 'g.group_id', + 'FROM' => [ GROUPS_TABLE => 'g', ], 'LEFT_JOIN' => [ [ - 'FROM' => [USER_GROUP_TABLE => 'ug'], - 'ON' => 'g.group_id = ug.group_id' + 'FROM' => [USER_GROUP_TABLE => 'ug'], + 'ON' => 'g.group_id = ug.group_id' ] ], - 'WHERE' => 'ug.user_pending = 0 AND ug.user_id = ' . (int) $this->user->data['user_id'], + 'WHERE' => 'ug.user_pending = 0 AND ug.user_id = ' . (int) $this->user->data['user_id'], 'ORDER_BY' => 'g.group_name', ]); - return $query; } }