From 5963905825ed65a522fe94e380c6c179a461e437 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 6 Mar 2013 11:32:23 +0100 Subject: [PATCH 1/5] [ticket/11404] Return empty array of avatar data if $row is empty While creating a group in the acp, the group data ($group_row) is empty. Due to that array_combine in phpbb_avatar_manager::clean_row() will cause PHP Warnings. In addition to that the required indexes 'avatar', 'avatar_width', 'avatar_height', and 'avatar_type' won't be defined. This patch will solve that issue. PHPBB3-11404 --- phpBB/includes/avatar/manager.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/phpBB/includes/avatar/manager.php b/phpBB/includes/avatar/manager.php index 9c60436de8..f126d69300 100644 --- a/phpBB/includes/avatar/manager.php +++ b/phpBB/includes/avatar/manager.php @@ -177,6 +177,17 @@ class phpbb_avatar_manager $keys = array_keys($row); $values = array_values($row); + // Upon creation of a user/group $row might be empty + if (empty($keys)) + { + return array( + 'avatar' => '', + 'avatar_type' => '', + 'avatar_width' => '', + 'avatar_height' => '', + ); + } + $keys = array_map(array('phpbb_avatar_manager', 'strip_prefix'), $keys); return array_combine($keys, $values); From bb584627248cc95443ebda511fca51effea6d0af Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 7 Mar 2013 13:03:27 +0100 Subject: [PATCH 2/5] [ticket/11404] Use a default data row if $row is empty in clean_row() A statically defined $default_row will be used inside the phpbb_avatar_manager::clean_row() method if the $row passed to it is empty. PHPBB3-11404 --- phpBB/includes/avatar/manager.php | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/phpBB/includes/avatar/manager.php b/phpBB/includes/avatar/manager.php index f126d69300..58d994c3c0 100644 --- a/phpBB/includes/avatar/manager.php +++ b/phpBB/includes/avatar/manager.php @@ -45,6 +45,17 @@ class phpbb_avatar_manager */ protected $container; + /** + * Default avatar data row + * @var array + */ + static protected $default_row = array( + 'avatar' => '', + 'avatar_type' => '', + 'avatar_width' => '', + 'avatar_height' => '', + ); + /** * Construct an avatar manager object * @@ -174,20 +185,15 @@ class phpbb_avatar_manager */ static public function clean_row($row) { + // Upon creation of a user/group $row might be empty + if (empty($row)) + { + return self::$default_row; + } + $keys = array_keys($row); $values = array_values($row); - // Upon creation of a user/group $row might be empty - if (empty($keys)) - { - return array( - 'avatar' => '', - 'avatar_type' => '', - 'avatar_width' => '', - 'avatar_height' => '', - ); - } - $keys = array_map(array('phpbb_avatar_manager', 'strip_prefix'), $keys); return array_combine($keys, $values); From 2ec0dc5b34e88c737e0737e73a46e1edd1573f10 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 11 Mar 2013 10:32:08 +0100 Subject: [PATCH 3/5] [ticket/11404] Convert manager_test to UNIX line endings PHPBB3-11404 --- tests/avatar/manager_test.php | 180 +++++++++++++++++----------------- 1 file changed, 90 insertions(+), 90 deletions(-) diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index f01ea47c25..7ea3cc6a0f 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -1,90 +1,90 @@ -phpbb_container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $this->phpbb_container->expects($this->any()) - ->method('get') - ->with('avatar.driver.foobar')->will($this->returnValue('avatar.driver.foobar')); - - // Prepare dependencies for avatar manager and driver - $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, $phpbb_root_path, $phpEx, $cache)); - $this->avatar_foobar->expects($this->any()) - ->method('get_name') - ->will($this->returnValue('avatar.driver.foobar')); - $this->avatar_barfoo = $this->getMock('phpbb_avatar_driver_barfoo', array('get_name')); - $this->avatar_barfoo->expects($this->any()) - ->method('get_name') - ->will($this->returnValue('avatar.driver.barfoo')); - - $avatar_drivers = array($this->avatar_foobar, $this->avatar_barfoo); - - $config['allow_avatar_' . get_class($this->avatar_foobar)] = true; - $config['allow_avatar_' . get_class($this->avatar_barfoo)] = false; - - // Set up avatar manager - $this->manager = new phpbb_avatar_manager($config, $avatar_drivers, $this->phpbb_container); - } - - public function test_get_driver() - { - $driver = $this->manager->get_driver('avatar.driver.foobar', false); - $this->assertEquals('avatar.driver.foobar', $driver); - - $driver = $this->manager->get_driver('avatar.driver.foo_wrong', false); - $this->assertNull($driver); - - $driver = $this->manager->get_driver('avatar.driver.foobar'); - $this->assertEquals('avatar.driver.foobar', $driver); - - $driver = $this->manager->get_driver('avatar.driver.foo_wrong'); - $this->assertNull($driver); - } - - public function test_get_all_drivers() - { - $drivers = $this->manager->get_all_drivers(); - $this->assertArrayHasKey('avatar.driver.foobar', $drivers); - $this->assertArrayHasKey('avatar.driver.barfoo', $drivers); - $this->assertEquals('avatar.driver.foobar', $drivers['avatar.driver.foobar']); - $this->assertEquals('avatar.driver.barfoo', $drivers['avatar.driver.barfoo']); - } - - public function test_get_enabled_drivers() - { - $drivers = $this->manager->get_enabled_drivers(); - $this->assertArrayHasKey('avatar.driver.foobar', $drivers); - $this->assertArrayNotHasKey('avatar.driver.barfoo', $drivers); - $this->assertEquals('avatar.driver.foobar', $drivers['avatar.driver.foobar']); - } - - public function test_get_avatar_settings() - { - $avatar_settings = $this->manager->get_avatar_settings($this->avatar_foobar); - - $expected_settings = array( - 'allow_avatar_' . get_class($this->avatar_foobar) => array('lang' => 'ALLOW_' . strtoupper(get_class($this->avatar_foobar)), 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => false), - ); - - $this->assertEquals($expected_settings, $avatar_settings); - } -} +phpbb_container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $this->phpbb_container->expects($this->any()) + ->method('get') + ->with('avatar.driver.foobar')->will($this->returnValue('avatar.driver.foobar')); + + // Prepare dependencies for avatar manager and driver + $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, $phpbb_root_path, $phpEx, $cache)); + $this->avatar_foobar->expects($this->any()) + ->method('get_name') + ->will($this->returnValue('avatar.driver.foobar')); + $this->avatar_barfoo = $this->getMock('phpbb_avatar_driver_barfoo', array('get_name')); + $this->avatar_barfoo->expects($this->any()) + ->method('get_name') + ->will($this->returnValue('avatar.driver.barfoo')); + + $avatar_drivers = array($this->avatar_foobar, $this->avatar_barfoo); + + $config['allow_avatar_' . get_class($this->avatar_foobar)] = true; + $config['allow_avatar_' . get_class($this->avatar_barfoo)] = false; + + // Set up avatar manager + $this->manager = new phpbb_avatar_manager($config, $avatar_drivers, $this->phpbb_container); + } + + public function test_get_driver() + { + $driver = $this->manager->get_driver('avatar.driver.foobar', false); + $this->assertEquals('avatar.driver.foobar', $driver); + + $driver = $this->manager->get_driver('avatar.driver.foo_wrong', false); + $this->assertNull($driver); + + $driver = $this->manager->get_driver('avatar.driver.foobar'); + $this->assertEquals('avatar.driver.foobar', $driver); + + $driver = $this->manager->get_driver('avatar.driver.foo_wrong'); + $this->assertNull($driver); + } + + public function test_get_all_drivers() + { + $drivers = $this->manager->get_all_drivers(); + $this->assertArrayHasKey('avatar.driver.foobar', $drivers); + $this->assertArrayHasKey('avatar.driver.barfoo', $drivers); + $this->assertEquals('avatar.driver.foobar', $drivers['avatar.driver.foobar']); + $this->assertEquals('avatar.driver.barfoo', $drivers['avatar.driver.barfoo']); + } + + public function test_get_enabled_drivers() + { + $drivers = $this->manager->get_enabled_drivers(); + $this->assertArrayHasKey('avatar.driver.foobar', $drivers); + $this->assertArrayNotHasKey('avatar.driver.barfoo', $drivers); + $this->assertEquals('avatar.driver.foobar', $drivers['avatar.driver.foobar']); + } + + public function test_get_avatar_settings() + { + $avatar_settings = $this->manager->get_avatar_settings($this->avatar_foobar); + + $expected_settings = array( + 'allow_avatar_' . get_class($this->avatar_foobar) => array('lang' => 'ALLOW_' . strtoupper(get_class($this->avatar_foobar)), 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => false), + ); + + $this->assertEquals($expected_settings, $avatar_settings); + } +} From fb1984dadb2ef4ec679f3a66e1a98e75bbe38dec Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 11 Mar 2013 10:33:16 +0100 Subject: [PATCH 4/5] [ticket/11404] Add tests for phpbb_avatar_manager::clean_row() PHPBB3-11404 --- tests/avatar/manager_test.php | 71 +++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index 7ea3cc6a0f..ffcfe2ce11 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -87,4 +87,75 @@ class phpbb_avatar_manager_test extends PHPUnit_Framework_TestCase $this->assertEquals($expected_settings, $avatar_settings); } + + public function database_row_data() + { + return array( + array( + array( + 'user_avatar' => '', + 'user_avatar_type' => '', + 'user_avatar_width' => '', + 'user_avatar_height' => '', + ), + array( + 'avatar' => '', + 'avatar_type' => '', + 'avatar_width' => '', + 'avatar_height' => '', + ), + ), + array( + array( + 'group_avatar' => '', + 'group_avatar_type' => '', + 'group_avatar_width' => '', + 'group_avatar_height' => '', + ), + array( + 'avatar' => '', + 'avatar_type' => '', + 'avatar_width' => '', + 'avatar_height' => '', + ), + ), + array( + array(), + array( + 'avatar' => '', + 'avatar_type' => '', + 'avatar_width' => '', + 'avatar_height' => '', + ), + ), + array( + array( + 'foobar_avatar' => '', + 'foobar_avatar_type' => '', + 'foobar_avatar_width' => '', + 'foobar_avatar_height' => '', + ), + array( + 'foobar_avatar' => '', + 'foobar_avatar_type' => '', + 'foobar_avatar_width' => '', + 'foobar_avatar_height' => '', + ), + ), + ); + } + + /** + * @dataProvider database_row_data + */ + public function test_clean_row(array $input, array $output) + { + $cleaned_row = array(); + + $cleaned_row = phpbb_avatar_manager::clean_row($input); + foreach ($output as $key => $null) + { + $this->assertArrayHasKey($key, $cleaned_row); + } + } } From 4ad8fcbd58aeb72b659db6ebc18ff38b0725f006 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 11 Mar 2013 10:35:18 +0100 Subject: [PATCH 5/5] [ticket/11404] Remove version ID from manager_test.php file header PHPBB3-11404 --- tests/avatar/manager_test.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/avatar/manager_test.php b/tests/avatar/manager_test.php index ffcfe2ce11..cb895b521a 100644 --- a/tests/avatar/manager_test.php +++ b/tests/avatar/manager_test.php @@ -2,7 +2,6 @@ /** * * @package testing -* @version $Id$ * @copyright (c) 2012 phpBB Group * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 *