From 023d7a972dd5c279e0b0b24801f9e53e7865d39a Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Jan 2013 22:49:48 +0100 Subject: [PATCH] [feature/avatars] Remove $request property and pass as argument if needed Remove the $request property from the phpbb_avatar_driver class and rather pass it as function argument if it's needed in a function. Currently this is only the case for the class methods prepare_form() and process_form(). PHPBB3-10018 --- phpBB/includes/acp/acp_groups.php | 4 ++-- phpBB/includes/acp/acp_users.php | 4 ++-- phpBB/includes/avatar/driver/driver.php | 9 +-------- phpBB/includes/avatar/driver/gravatar.php | 14 +++++++------- phpBB/includes/avatar/driver/interface.php | 6 ++++-- phpBB/includes/avatar/driver/local.php | 10 +++++----- phpBB/includes/avatar/driver/remote.php | 14 +++++++------- phpBB/includes/avatar/driver/upload.php | 8 ++++---- phpBB/includes/ucp/ucp_groups.php | 4 ++-- phpBB/includes/ucp/ucp_profile.php | 4 ++-- tests/avatar/driver/foobar.php | 6 +++--- tests/avatar/manager_test.php | 4 ++-- 12 files changed, 41 insertions(+), 46 deletions(-) diff --git a/phpBB/includes/acp/acp_groups.php b/phpBB/includes/acp/acp_groups.php index d9452a902e..25e199ab32 100644 --- a/phpBB/includes/acp/acp_groups.php +++ b/phpBB/includes/acp/acp_groups.php @@ -338,7 +338,7 @@ class acp_groups if (in_array($driver_name, $avatar_drivers) && !$request->is_set_post('avatar_delete')) { $driver = $phpbb_avatar_manager->get_driver($driver_name); - $result = $driver->process_form($template, $avatar_data, $avatar_error); + $result = $driver->process_form($request, $template, $avatar_data, $avatar_error); if ($result && empty($avatar_error)) { @@ -532,7 +532,7 @@ class acp_groups 'avatar' => "acp_avatar_options_{$config_name}.html", )); - if ($driver->prepare_form($template, $avatar_data, $avatar_error)) + if ($driver->prepare_form($request, $template, $avatar_data, $avatar_error)) { $driver_name = $phpbb_avatar_manager->prepare_driver_name($current_driver); $driver_upper = strtoupper($driver_name); diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 122bbeb770..61b644e9f5 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -1751,7 +1751,7 @@ class acp_users if (in_array($driver_name, $avatar_drivers) && !$request->is_set_post('avatar_delete')) { $driver = $phpbb_avatar_manager->get_driver($driver_name); - $result = $driver->process_form($template, $avatar_data, $error); + $result = $driver->process_form($request, $template, $avatar_data, $error); if ($result && empty($error)) { @@ -1813,7 +1813,7 @@ class acp_users 'avatar' => "acp_avatar_options_{$config_name}.html", )); - if ($driver->prepare_form($template, $avatar_data, $error)) + if ($driver->prepare_form($request, $template, $avatar_data, $error)) { $driver_name = $phpbb_avatar_manager->prepare_driver_name($current_driver); $driver_upper = strtoupper($driver_name); diff --git a/phpBB/includes/avatar/driver/driver.php b/phpBB/includes/avatar/driver/driver.php index ab89cfbffe..d4f9139c18 100644 --- a/phpBB/includes/avatar/driver/driver.php +++ b/phpBB/includes/avatar/driver/driver.php @@ -33,12 +33,6 @@ abstract class phpbb_avatar_driver implements phpbb_avatar_driver_interface */ protected $config; - /** - * Request object - * @var phpbb_request - */ - protected $request; - /** * Current $phpbb_root_path * @var string @@ -66,10 +60,9 @@ abstract class phpbb_avatar_driver implements phpbb_avatar_driver_interface * @param string $php_ext PHP file extension * @param phpbb_cache_driver_interface $cache Cache driver */ - public function __construct(phpbb_config $config, phpbb_request $request, $phpbb_root_path, $php_ext, phpbb_cache_driver_interface $cache = null) + public function __construct(phpbb_config $config, $phpbb_root_path, $php_ext, phpbb_cache_driver_interface $cache = null) { $this->config = $config; - $this->request = $request; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; $this->cache = $cache; diff --git a/phpBB/includes/avatar/driver/gravatar.php b/phpBB/includes/avatar/driver/gravatar.php index c6bfb0d5c1..c574e23836 100644 --- a/phpBB/includes/avatar/driver/gravatar.php +++ b/phpBB/includes/avatar/driver/gravatar.php @@ -52,11 +52,11 @@ class phpbb_avatar_driver_gravatar extends phpbb_avatar_driver /** * @inheritdoc */ - public function prepare_form($template, $row, &$error) + public function prepare_form($request, $template, $row, &$error) { $template->assign_vars(array( - 'AVATAR_GRAVATAR_WIDTH' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('avatar_gravatar_width', 0), - 'AVATAR_GRAVATAR_HEIGHT' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('avatar_gravatar_width', 0), + 'AVATAR_GRAVATAR_WIDTH' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_width']) ? $row['avatar_width'] : $request->variable('avatar_gravatar_width', 0), + 'AVATAR_GRAVATAR_HEIGHT' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar_height']) ? $row['avatar_height'] : $request->variable('avatar_gravatar_width', 0), 'AVATAR_GRAVATAR_EMAIL' => (($row['avatar_type'] == $this->get_name() || $row['avatar_type'] == 'gravatar') && $row['avatar']) ? $row['avatar'] : '', )); @@ -66,11 +66,11 @@ class phpbb_avatar_driver_gravatar extends phpbb_avatar_driver /** * @inheritdoc */ - public function process_form($template, $row, &$error) + public function process_form($request, $template, $row, &$error) { - $row['avatar'] = $this->request->variable('avatar_gravatar_email', ''); - $row['avatar_width'] = $this->request->variable('avatar_gravatar_width', 0); - $row['avatar_height'] = $this->request->variable('avatar_gravatar_height', 0); + $row['avatar'] = $request->variable('avatar_gravatar_email', ''); + $row['avatar_width'] = $request->variable('avatar_gravatar_width', 0); + $row['avatar_height'] = $request->variable('avatar_gravatar_height', 0); if (!function_exists('validate_data')) { diff --git a/phpBB/includes/avatar/driver/interface.php b/phpBB/includes/avatar/driver/interface.php index e8f529f3c4..3c1db019f0 100644 --- a/phpBB/includes/avatar/driver/interface.php +++ b/phpBB/includes/avatar/driver/interface.php @@ -50,6 +50,7 @@ interface phpbb_avatar_driver_interface /** * Prepare form for changing the settings of this avatar * + * @param phpbb_request $request Request object * @param phpbb_template $template Template object * @param array $row User data or group data that has been cleaned with * phpbb_avatar_manager::clean_row @@ -60,7 +61,7 @@ interface phpbb_avatar_driver_interface * * @return bool True if form has been successfully prepared */ - public function prepare_form($template, $row, &$error); + public function prepare_form($request, $template, $row, &$error); /** * Prepare form for changing the acp settings of this avatar @@ -74,6 +75,7 @@ interface phpbb_avatar_driver_interface /** * Process form data * + * @param phpbb_request $request Request object * @param phpbb_template $template Template object * @param array $row User data or group data that has been cleaned with * phpbb_avatar_manager::clean_row @@ -85,7 +87,7 @@ interface phpbb_avatar_driver_interface * @return array Array containing the avatar data as follows: * ['avatar'], ['avatar_width'], ['avatar_height'] */ - public function process_form($template, $row, &$error); + public function process_form($request, $template, $row, &$error); /** * Delete avatar diff --git a/phpBB/includes/avatar/driver/local.php b/phpBB/includes/avatar/driver/local.php index bef5c6d427..e49b8dd07f 100644 --- a/phpBB/includes/avatar/driver/local.php +++ b/phpBB/includes/avatar/driver/local.php @@ -36,10 +36,10 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver /** * @inheritdoc */ - public function prepare_form($template, $row, &$error) + public function prepare_form($request, $template, $row, &$error) { $avatar_list = $this->get_avatar_list(); - $category = $this->request->variable('avatar_local_cat', ''); + $category = $request->variable('avatar_local_cat', ''); foreach ($avatar_list as $cat => $null) { @@ -114,12 +114,12 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver /** * @inheritdoc */ - public function process_form($template, $row, &$error) + public function process_form($request, $template, $row, &$error) { $avatar_list = $this->get_avatar_list(); - $category = $this->request->variable('avatar_local_cat', ''); + $category = $request->variable('avatar_local_cat', ''); - $file = $this->request->variable('avatar_local_file', ''); + $file = $request->variable('avatar_local_file', ''); if (empty($category) || empty($file)) { diff --git a/phpBB/includes/avatar/driver/remote.php b/phpBB/includes/avatar/driver/remote.php index 717b73eb0c..e8f182063e 100644 --- a/phpBB/includes/avatar/driver/remote.php +++ b/phpBB/includes/avatar/driver/remote.php @@ -36,11 +36,11 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver /** * @inheritdoc */ - public function prepare_form($template, $row, &$error) + public function prepare_form($request, $template, $row, &$error) { $template->assign_vars(array( - 'AVATAR_REMOTE_WIDTH' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_width']) ? $row['avatar_width'] : $this->request->variable('avatar_remote_width', 0), - 'AVATAR_REMOTE_HEIGHT' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_height']) ? $row['avatar_height'] : $this->request->variable('avatar_remote_width', 0), + 'AVATAR_REMOTE_WIDTH' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_width']) ? $row['avatar_width'] : $request->variable('avatar_remote_width', 0), + 'AVATAR_REMOTE_HEIGHT' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar_height']) ? $row['avatar_height'] : $request->variable('avatar_remote_width', 0), 'AVATAR_REMOTE_URL' => ((in_array($row['avatar_type'], array(AVATAR_REMOTE, $this->get_name(), 'remote'))) && $row['avatar']) ? $row['avatar'] : '', )); @@ -50,11 +50,11 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver /** * @inheritdoc */ - public function process_form($template, $row, &$error) + public function process_form($request, $template, $row, &$error) { - $url = $this->request->variable('avatar_remote_url', ''); - $width = $this->request->variable('avatar_remote_width', 0); - $height = $this->request->variable('avatar_remote_height', 0); + $url = $request->variable('avatar_remote_url', ''); + $width = $request->variable('avatar_remote_width', 0); + $height = $request->variable('avatar_remote_height', 0); if (!preg_match('#^(http|https|ftp)://#i', $url)) { diff --git a/phpBB/includes/avatar/driver/upload.php b/phpBB/includes/avatar/driver/upload.php index 1bb86cf158..6e6956bfde 100644 --- a/phpBB/includes/avatar/driver/upload.php +++ b/phpBB/includes/avatar/driver/upload.php @@ -36,7 +36,7 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver /** * @inheritdoc */ - public function prepare_form($template, $row, &$error) + public function prepare_form($request, $template, $row, &$error) { if (!$this->can_upload()) { @@ -54,7 +54,7 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver /** * @inheritdoc */ - public function process_form($template, $row, &$error) + public function process_form($request, $template, $row, &$error) { if (!$this->can_upload()) { @@ -68,8 +68,8 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver $upload = new fileupload('AVATAR_', array('jpg', 'jpeg', 'gif', 'png'), $this->config['avatar_filesize'], $this->config['avatar_min_width'], $this->config['avatar_min_height'], $this->config['avatar_max_width'], $this->config['avatar_max_height'], (isset($this->config['mime_triggers']) ? explode('|', $this->config['mime_triggers']) : false)); - $url = $this->request->variable('avatar_upload_url', ''); - $upload_file = $this->request->file('avatar_upload_file'); + $url = $request->variable('avatar_upload_url', ''); + $upload_file = $request->file('avatar_upload_file'); if (!empty($upload_file['name'])) { diff --git a/phpBB/includes/ucp/ucp_groups.php b/phpBB/includes/ucp/ucp_groups.php index 86c02b5bcc..69cab610fc 100644 --- a/phpBB/includes/ucp/ucp_groups.php +++ b/phpBB/includes/ucp/ucp_groups.php @@ -526,7 +526,7 @@ class ucp_groups if (in_array($driver_name, $avatar_drivers) && !$request->is_set_post('avatar_delete')) { $driver = $phpbb_avatar_manager->get_driver($driver_name); - $result = $driver->process_form($template, $avatar_data, $avatar_error); + $result = $driver->process_form($request, $template, $avatar_data, $avatar_error); if ($result && empty($avatar_error)) { @@ -657,7 +657,7 @@ class ucp_groups 'avatar' => $driver->get_template_name(), )); - if ($driver->prepare_form($template, $avatar_data, $avatar_error)) + if ($driver->prepare_form($request, $template, $avatar_data, $avatar_error)) { $driver_name = $phpbb_avatar_manager->prepare_driver_name($current_driver); $driver_upper = strtoupper($driver_name); diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index 36d59e7854..518ad9d917 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -576,7 +576,7 @@ class ucp_profile if (in_array($driver_name, $avatar_drivers) && !$request->is_set_post('avatar_delete')) { $driver = $phpbb_avatar_manager->get_driver($driver_name); - $result = $driver->process_form($template, $avatar_data, $error); + $result = $driver->process_form($request, $template, $avatar_data, $error); if ($result && empty($error)) { @@ -641,7 +641,7 @@ class ucp_profile 'avatar' => $driver->get_template_name(), )); - if ($driver->prepare_form($template, $avatar_data, $error)) + if ($driver->prepare_form($request, $template, $avatar_data, $error)) { $driver_name = $phpbb_avatar_manager->prepare_driver_name($current_driver); $driver_upper = strtoupper($driver_name); diff --git a/tests/avatar/driver/foobar.php b/tests/avatar/driver/foobar.php index fd4e452818..a68d0aa6c6 100644 --- a/tests/avatar/driver/foobar.php +++ b/tests/avatar/driver/foobar.php @@ -7,13 +7,13 @@ class phpbb_avatar_driver_foobar extends phpbb_avatar_driver return array(); } - public function prepare_form($template, $row, &$error) + public function prepare_form($request, $template, $row, &$error) { return false; } - public function process_form($template, $row, &$error) + public function process_form($request, $template, $row, &$error) { return false; } -} \ No newline at end of file +} diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index b910db59ee..fd7132ea82 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -26,12 +26,12 @@ class phpbb_avatar_manager_test extends PHPUnit_Framework_TestCase $config = new phpbb_config(array()); $request = $this->getMock('phpbb_request'); $cache = $this->getMock('phpbb_cache_driver_interface'); - $this->avatar_foobar = $this->getMock('phpbb_avatar_driver_foobar', array('get_name'), array($config, $request, $phpbb_root_path, $phpEx, $cache)); + $this->avatar_foobar = $this->getMock('phpbb_avatar_driver_foobar', array('get_name'), array($config, $phpbb_root_path, $phpEx, $cache)); $this->avatar_foobar->expects($this->any()) ->method('get_name') ->will($this->returnValue('avatar.driver.foobar')); $avatar_drivers = array($this->avatar_foobar); - $config['allow_avatar_' . get_class($this->avatar_foobar)] = true; + $config['allow_avatar_' . get_class($this->avatar_foobar)] = true; // Set up avatar manager $this->manager = new phpbb_avatar_manager($config, $avatar_drivers, $this->phpbb_container);