From 0a276c6ec6ac0ed7b69557b0aac859fbdd692ef3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Jul 2014 20:04:02 +0200 Subject: [PATCH 1/4] [ticket/301] Throw Exception on incorrect call of phpbb_fetch_posts B3P-301 --- includes/functions.php | 5 ++--- language/en/portal.php | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/functions.php b/includes/functions.php index 3dc643ae..b09cfd5a 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -166,9 +166,8 @@ function phpbb_fetch_posts($module_id, $forum_from, $permissions, $number_of_pos break; default: - $topic_type = $str_where = $user_link = $post_link = ''; - $topic_order = 't.topic_time DESC'; - // maybe use trigger_error here, as this shouldn't happen + // Method was called with unsupported type + throw new \InvalidArgumentexception($user->lang('B3P_WRONG_METHOD_CALL', __FUNCTION__)); } if ($type == 'announcements' && $global_f < 1) diff --git a/language/en/portal.php b/language/en/portal.php index 74b10f48..b4a8e963 100644 --- a/language/en/portal.php +++ b/language/en/portal.php @@ -37,4 +37,5 @@ $lang = array_merge($lang, array( 'PORTAL' => 'Portal', 'VIEWING_PORTAL' => 'Portal page', 'BACK' => 'Back', + 'B3P_WRONG_METHOD_CALL' => 'Incorrect call to method %s', )); From 63be896ca4505ae4d6840ae6ab3ca96860ed3d85 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Jul 2014 20:05:08 +0200 Subject: [PATCH 2/4] [ticket/301] Cover InvalidArgumentException in phpbb_fetch_posts B3P-301 --- tests/unit/functions/fetch_news_test.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/unit/functions/fetch_news_test.php b/tests/unit/functions/fetch_news_test.php index 21949e14..826679d1 100644 --- a/tests/unit/functions/fetch_news_test.php +++ b/tests/unit/functions/fetch_news_test.php @@ -26,6 +26,7 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework $user->data['user_id'] = 2; $user->timezone = new \DateTimeZone('UTC'); $user->add_lang('common'); + $user->add_lang('../../ext/board3/portal/language/en/portal'); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists')); $cache->expects($this->any()) @@ -93,23 +94,31 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework 'attachments', 'forum_name', )), - array('announcements', array(), true), + array('announcements', array(), 5, true), + array('news', array(), 0), + array('foobar', array(), 5, false, '\InvalidArgumentException'), ); } /** * @dataProvider data_phpbb_fetch_news */ - public function test_phpbb_fetch_news($type, $expected_columns, $empty = false) + public function test_phpbb_fetch_news($type, $expected_columns, $number_of_posts = 5, $empty = false, $expected_exception = false) { $module_id = 5; $forum_from = ''; $permissions = false; - $number_of_posts = 5; $text_length = 150; $time = time(); + $start = 0; + $invert = false; - $fetch_posts = phpbb_fetch_posts($module_id, $forum_from, $permissions, $number_of_posts, $text_length, $time, $type); + if ($expected_exception) + { + $this->setExpectedException($expected_exception); + } + + $fetch_posts = phpbb_fetch_posts($module_id, $forum_from, $permissions, $number_of_posts, $text_length, $time, $type, $start, $invert); if (!$empty) { From a552a5cb8fde7223ae35d7bbfb4baa7b4ac3a033 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Jul 2014 20:49:49 +0200 Subject: [PATCH 3/4] [ticket/301] Add higher test coverage of phpbb_fetch_posts B3P-301 --- tests/unit/functions/fetch_news_test.php | 63 ++++++++++++++++++------ tests/unit/functions/fixtures/news.xml | 3 ++ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/tests/unit/functions/fetch_news_test.php b/tests/unit/functions/fetch_news_test.php index 826679d1..ed2c1018 100644 --- a/tests/unit/functions/fetch_news_test.php +++ b/tests/unit/functions/fetch_news_test.php @@ -13,6 +13,8 @@ require_once(dirname(__FILE__) . '/../../../../../../includes/functions.php'); class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework\database_test_case { + protected $default_main_columns = array('topic_count', 'global_id', 'topic_icons'); + public function setUp() { parent::setUp(); @@ -28,11 +30,15 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework $user->add_lang('common'); $user->add_lang('../../ext/board3/portal/language/en/portal'); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists')); + $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists', 'put')); $cache->expects($this->any()) ->method('obtain_word_list') ->with() ->will($this->returnValue(array())); + $cache->expects($this->any()) + ->method('get') + ->with($this->anything()) + ->will($this->returnValue(false)); require_once(dirname(__FILE__) . '/../../../../../../includes/functions_content.php'); } @@ -94,24 +100,28 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework 'attachments', 'forum_name', )), - array('announcements', array(), 5, true), - array('news', array(), 0), - array('foobar', array(), 5, false, '\InvalidArgumentException'), + array('announcements', array(), array('topic_icons', 'topic_count'), 5, ''), + array('news', array(), array(), 0), + array('foobar', array(), array(), 5, '', false, false, false, '\InvalidArgumentException'), + array('news', array(), array(), 5, '', true, true), + array('announcements', array(), array('topic_icons', 'topic_count'), 5, '3'), + array('announcements', array(), array(), 5, '1,2', true, true), + array('news', array(), array(), 5, '1,2', true, false, true), + array('announcements', array(), array(), 5, '', true, true), + array('announcements', array(), array(), 5, '1,2', true, true, true), ); } /** * @dataProvider data_phpbb_fetch_news */ - public function test_phpbb_fetch_news($type, $expected_columns, $number_of_posts = 5, $empty = false, $expected_exception = false) + public function test_phpbb_fetch_news($type, $expected_columns, $expected_main_columns = array(), $number_of_posts = 5, $forum_from = '', $empty = false, $permissions = false, + $invert = false, $expected_exception = false) { $module_id = 5; - $forum_from = ''; - $permissions = false; $text_length = 150; $time = time(); $start = 0; - $invert = false; if ($expected_exception) { @@ -122,13 +132,16 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework if (!$empty) { - $this->assertArrayHasKey('topic_count', $fetch_posts); - $this->assertArrayHasKey('global_id', $fetch_posts); - $this->assertArrayHasKey('topic_icons', $fetch_posts); + if (empty($expected_main_columns)) + { + $expected_main_columns = $this->default_main_columns; + } - unset($fetch_posts['topic_count']); - unset($fetch_posts['global_id']); - unset($fetch_posts['topic_icons']); + foreach ($expected_main_columns as $main_column) + { + $this->assertArrayHasKey($main_column, $fetch_posts); + unset($fetch_posts[$main_column]); + } foreach ($fetch_posts as $post) { @@ -141,7 +154,29 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework } else { + if (!empty($fetch_posts)) + { + var_export($fetch_posts); + } $this->assertEmpty($fetch_posts); } } + + public function test_cached_first_forum_id() + { + global $cache; + + $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists', 'put')); + $cache->expects($this->any()) + ->method('obtain_word_list') + ->with() + ->will($this->returnValue(array())); + $cache->expects($this->any()) + ->method('get') + ->with($this->anything()) + ->will($this->returnValue(array())); + + $fetch_posts = phpbb_fetch_posts(5, '', false, 5, 150, time(), 'announcements'); + $this->assertEmpty($fetch_posts); + } } diff --git a/tests/unit/functions/fixtures/news.xml b/tests/unit/functions/fixtures/news.xml index 8a7e0f1f..51a24acd 100644 --- a/tests/unit/functions/fixtures/news.xml +++ b/tests/unit/functions/fixtures/news.xml @@ -80,12 +80,14 @@ forum_parents forum_desc forum_rules + forum_type 1 0 + 1 2 @@ -93,6 +95,7 @@ + 1 From 2cdc2ee2113660226d7e81580e3eee32c773bbcf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Wed, 9 Jul 2014 22:17:46 +0200 Subject: [PATCH 4/4] [ticket/301] Extend phpbb_fetch_posts coverage to 100% B3P-301 --- tests/unit/functions/fetch_news_test.php | 91 +++++++++++++++++++++--- tests/unit/functions/fixtures/news.xml | 57 ++++++++++++++- 2 files changed, 135 insertions(+), 13 deletions(-) diff --git a/tests/unit/functions/fetch_news_test.php b/tests/unit/functions/fetch_news_test.php index ed2c1018..dc70be8f 100644 --- a/tests/unit/functions/fetch_news_test.php +++ b/tests/unit/functions/fetch_news_test.php @@ -10,6 +10,7 @@ require_once(dirname(__FILE__) . '/../../../includes/functions.php'); require_once(dirname(__FILE__) . '/../../../../../../includes/functions_acp.php'); require_once(dirname(__FILE__) . '/../../../../../../includes/functions.php'); +require_once(dirname(__FILE__) . '/../../../../../../includes/utf/utf_tools.php'); class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework\database_test_case { @@ -19,18 +20,15 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework { parent::setUp(); - global $auth, $cache, $phpbb_container, $phpbb_dispatcher, $user; + global $auth, $cache, $config, $phpbb_container, $phpbb_dispatcher, $template, $user; - $auth = new \phpbb\auth\auth(); - $phpbb_container = new \phpbb_mock_container_builder(); - $phpbb_container->set('board3.portal.modules_helper', new \board3\portal\includes\modules_helper($auth)); $user = new \phpbb\user(); $user->data['user_id'] = 2; $user->timezone = new \DateTimeZone('UTC'); $user->add_lang('common'); $user->add_lang('../../ext/board3/portal/language/en/portal'); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists', 'put')); + $cache = $this->getMock('\phpbb\cache\cache', array('obtain_word_list', 'get', 'sql_exists', 'put', 'obtain_attach_extensions')); $cache->expects($this->any()) ->method('obtain_word_list') ->with() @@ -40,6 +38,19 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework ->with($this->anything()) ->will($this->returnValue(false)); require_once(dirname(__FILE__) . '/../../../../../../includes/functions_content.php'); + $config = new \phpbb\config\config(array('allow_attachments' => 1)); + $auth = new \phpbb\auth\auth(); + $userdata = array( + 'user_id' => 2, + ); + $auth->acl($userdata); + // Pretend to allow downloads + $auth->acl[0][0] = true; + // Pretend to allow downloads in forum 1 + $auth->acl[1][0] = true; + $phpbb_container = new \phpbb_mock_container_builder(); + $phpbb_container->set('board3.portal.modules_helper', new \board3\portal\includes\modules_helper($auth)); + $template = $this->getMock('\phpbb\template', array('set_filenames', 'destroy_block_vars', 'assign_block_vars', 'assign_display')); } public function getDataSet() @@ -102,13 +113,62 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework )), array('announcements', array(), array('topic_icons', 'topic_count'), 5, ''), array('news', array(), array(), 0), - array('foobar', array(), array(), 5, '', false, false, false, '\InvalidArgumentException'), - array('news', array(), array(), 5, '', true, true), + array('foobar', array(), array(), 5, '', false, false, false, 150, '\InvalidArgumentException'), + array('news', array( + 'forum_id', + 'topic_id', + 'topic_last_post_time', + 'topic_replies', + 'topic_replies_real', + 'topic_type', + 'topic_status', + 'topic_posted', + 'attachment', + 'forum_name', + 'topic_title', + 'username', + 'username_full', + 'username_full_last', + 'user_type', + 'user_id', + 'topic_time', + 'post_text', + 'topic_views', + 'icon_id', + 'poll', + 'attachments', + 'forum_name', + ), array(), 5, '', false, true), array('announcements', array(), array('topic_icons', 'topic_count'), 5, '3'), - array('announcements', array(), array(), 5, '1,2', true, true), + array('announcements', array(), array('topic_icons', 'topic_count'), 5, '1,2', false, true), array('news', array(), array(), 5, '1,2', true, false, true), - array('announcements', array(), array(), 5, '', true, true), + array('announcements', array(), array('topic_icons', 'topic_count'), 5, '', false, true), array('announcements', array(), array(), 5, '1,2', true, true, true), + array('news', array( + 'forum_id', + 'topic_id', + 'topic_last_post_time', + 'topic_replies', + 'topic_replies_real', + 'topic_type', + 'topic_status', + 'topic_posted', + 'attachment', + 'forum_name', + 'topic_title', + 'username', + 'username_full', + 'username_full_last', + 'user_type', + 'user_id', + 'topic_time', + 'post_text', + 'topic_views', + 'icon_id', + 'poll', + 'attachments', + 'forum_name', + ), array(), 5, '', false, true, false, 5), ); } @@ -116,10 +176,9 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework * @dataProvider data_phpbb_fetch_news */ public function test_phpbb_fetch_news($type, $expected_columns, $expected_main_columns = array(), $number_of_posts = 5, $forum_from = '', $empty = false, $permissions = false, - $invert = false, $expected_exception = false) + $invert = false, $text_length = 150, $expected_exception = false) { $module_id = 5; - $text_length = 150; $time = time(); $start = 0; @@ -179,4 +238,14 @@ class phpbb_functions_fetch_news_test extends \board3\portal\tests\testframework $fetch_posts = phpbb_fetch_posts(5, '', false, 5, 150, time(), 'announcements'); $this->assertEmpty($fetch_posts); } + + public function test_no_allowed_forums() + { + global $auth; + + $auth = new \phpbb\auth\auth(); + + $fetch_posts = phpbb_fetch_posts(5, '2', true, 5, 150, time(), 'announcements'); + $this->assertSame(array(), $fetch_posts); + } } diff --git a/tests/unit/functions/fixtures/news.xml b/tests/unit/functions/fixtures/news.xml index 51a24acd..3688ac4c 100644 --- a/tests/unit/functions/fixtures/news.xml +++ b/tests/unit/functions/fixtures/news.xml @@ -1,5 +1,44 @@ +
+ auth_option_id + auth_option + is_global + is_local + founder_only + + 93 + u_download + 1 + 0 + 0 + + + 7 + f_download + 0 + 1 + 0 + +
+ + attach_id + post_msg_id + topic_id + poster_id + physical_filename + real_filename + attach_comment + + 1 + 1 + 1 + 2 + foobar + foobar + foobar + +
forum_idtopic_id @@ -25,14 +64,14 @@ 1 1 - 1 + 2 1404830663 1404830663 test post 0 1 - 1 + 2 0 0 2 @@ -64,6 +103,20 @@ 2 1404830663 foobar post + 1 + foobar + 1 + 1 + 1 + + + 1 + + + 2 + 2 + 1404830663 + foobar post 0 foobar 1