From bf7936380a65c73a5f492b83c49ceb6ab5458fb8 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 29 Oct 2021 21:32:26 +0700 Subject: [PATCH 1/8] [ticket/16902] Improve search results count for MySQL PHPBB3-16902 --- phpBB/phpbb/search/fulltext_mysql.php | 13 ++++++------- phpBB/phpbb/search/fulltext_native.php | 23 +++-------------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_mysql.php b/phpBB/phpbb/search/fulltext_mysql.php index b493ccf326..b841aa5a96 100644 --- a/phpBB/phpbb/search/fulltext_mysql.php +++ b/phpBB/phpbb/search/fulltext_mysql.php @@ -568,8 +568,7 @@ class fulltext_mysql extends \phpbb\search\base ); extract($this->phpbb_dispatcher->trigger_event('core.search_mysql_keywords_main_query_before', compact($vars))); - $sql_select = (!$result_count) ? 'SQL_CALC_FOUND_ROWS ' : ''; - $sql_select = ($type == 'posts') ? $sql_select . 'p.post_id' : 'DISTINCT ' . $sql_select . 't.topic_id'; + $sql_select = ($type == 'posts') ? 'p.post_id' : 'DISTINCT t.topic_id'; $sql_from = ($join_topic) ? TOPICS_TABLE . ' t, ' : ''; $field = ($type == 'posts') ? 'post_id' : 'topic_id'; if (count($author_ary) && $author_name) @@ -614,7 +613,7 @@ class fulltext_mysql extends \phpbb\search\base // if the total result count is not cached yet, retrieve it from the db if (!$result_count && count($id_ary)) { - $sql_found_rows = 'SELECT FOUND_ROWS() as result_count'; + $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT($sql_select) as result_count", $sql); $result = $this->db->sql_query($sql_found_rows); $result_count = (int) $this->db->sql_fetchfield('result_count'); $this->db->sql_freeresult($result); @@ -828,12 +827,12 @@ class fulltext_mysql extends \phpbb\search\base extract($this->phpbb_dispatcher->trigger_event('core.search_mysql_author_query_before', compact($vars))); // If the cache was completely empty count the results - $calc_results = ($result_count) ? '' : 'SQL_CALC_FOUND_ROWS '; + $sql_select = ($type == 'posts') ? 'p.post_id' : 't.topic_id'; // Build the query for really selecting the post_ids if ($type == 'posts') { - $sql = "SELECT {$calc_results}p.post_id + $sql = "SELECT $sql_select FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t ' : ' ') . " WHERE $sql_author $sql_topic_id @@ -847,7 +846,7 @@ class fulltext_mysql extends \phpbb\search\base } else { - $sql = "SELECT {$calc_results}t.topic_id + $sql = "SELECT $sql_select FROM " . $sql_sort_table . TOPICS_TABLE . ' t, ' . POSTS_TABLE . " p WHERE $sql_author $sql_topic_id @@ -874,7 +873,7 @@ class fulltext_mysql extends \phpbb\search\base // retrieve the total result count if needed if (!$result_count) { - $sql_found_rows = 'SELECT FOUND_ROWS() as result_count'; + $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT($sql_select) as result_count", $sql); $result = $this->db->sql_query($sql_found_rows); $result_count = (int) $this->db->sql_fetchfield('result_count'); $this->db->sql_freeresult($result); diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index ee8a65610d..05eddf56ef 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -908,9 +908,6 @@ class fulltext_native extends \phpbb\search\base switch ($this->db->get_sql_layer()) { case 'mysqli': - - // 3.x does not support SQL_CALC_FOUND_ROWS - // $sql_array['SELECT'] = 'SQL_CALC_FOUND_ROWS ' . $sql_array['SELECT']; $is_mysql = true; break; @@ -968,13 +965,6 @@ class fulltext_native extends \phpbb\search\base ); } - // if using mysql and the total result count is not calculated yet, get it from the db - if (!$total_results && $is_mysql) - { - // Also count rows for the query as if there was not LIMIT. Add SQL_CALC_FOUND_ROWS to SQL - $sql_array['SELECT'] = 'SQL_CALC_FOUND_ROWS ' . $sql_array['SELECT']; - } - $sql_array['WHERE'] = implode(' AND ', $sql_where); $sql_array['GROUP_BY'] = ($group_by) ? (($type == 'posts') ? 'p.post_id' : 'p.topic_id') . ', ' . $sort_by_sql[$sort_key] : ''; $sql_array['ORDER_BY'] = $sql_sort; @@ -990,10 +980,10 @@ class fulltext_native extends \phpbb\search\base } $this->db->sql_freeresult($result); + // If using mysql and the total result count is not calculated yet, get it from the db if (!$total_results && $is_mysql) { - // Get the number of results as calculated by MySQL - $sql_count = 'SELECT FOUND_ROWS() as total_results'; + $sql_count = str_replace("SELECT {$sql_array['SELECT']}", "SELECT COUNT({$sql_array['SELECT']}) as total_results", $sql); $result = $this->db->sql_query($sql_count); $total_results = (int) $this->db->sql_fetchfield('total_results'); $this->db->sql_freeresult($result); @@ -1202,7 +1192,6 @@ class fulltext_native extends \phpbb\search\base switch ($this->db->get_sql_layer()) { case 'mysqli': -// $select = 'SQL_CALC_FOUND_ROWS ' . $select; $is_mysql = true; break; @@ -1295,13 +1284,7 @@ class fulltext_native extends \phpbb\search\base if (!$total_results && $is_mysql) { - // Count rows for the executed queries. Replace $select within $sql with SQL_CALC_FOUND_ROWS, and run it. - $sql_calc = str_replace('SELECT ' . $select, 'SELECT SQL_CALC_FOUND_ROWS ' . $select, $sql); - - $result = $this->db->sql_query($sql_calc); - $this->db->sql_freeresult($result); - - $sql_count = 'SELECT FOUND_ROWS() as total_results'; + $sql_count = str_replace("SELECT $select", "SELECT COUNT($select) as total_results", $sql); $result = $this->db->sql_query($sql_count); $total_results = (int) $this->db->sql_fetchfield('total_results'); $this->db->sql_freeresult($result); From e7c81cd1a6007b49e62a7179af547a58ef383d59 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 30 Oct 2021 13:07:36 +0700 Subject: [PATCH 2/8] [ticket/16902] Improve test, use DISTINCT for precise results count PHPBB3-16902 --- phpBB/phpbb/search/fulltext_mysql.php | 12 +++++++----- phpBB/phpbb/search/fulltext_native.php | 9 +++++---- tests/functional/search/base.php | 25 +++++++++++++++++++++---- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_mysql.php b/phpBB/phpbb/search/fulltext_mysql.php index b841aa5a96..c280f095ef 100644 --- a/phpBB/phpbb/search/fulltext_mysql.php +++ b/phpBB/phpbb/search/fulltext_mysql.php @@ -569,6 +569,7 @@ class fulltext_mysql extends \phpbb\search\base extract($this->phpbb_dispatcher->trigger_event('core.search_mysql_keywords_main_query_before', compact($vars))); $sql_select = ($type == 'posts') ? 'p.post_id' : 'DISTINCT t.topic_id'; + $sql_select .= $sort_by_sql[$sort_key] ? ", {$sort_by_sql[$sort_key]}" : ''; $sql_from = ($join_topic) ? TOPICS_TABLE . ' t, ' : ''; $field = ($type == 'posts') ? 'post_id' : 'topic_id'; if (count($author_ary) && $author_name) @@ -609,11 +610,10 @@ class fulltext_mysql extends \phpbb\search\base $this->db->sql_freeresult($result); $id_ary = array_unique($id_ary); - // if the total result count is not cached yet, retrieve it from the db if (!$result_count && count($id_ary)) { - $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT($sql_select) as result_count", $sql); + $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT(*) as result_count", $sql); $result = $this->db->sql_query($sql_found_rows); $result_count = (int) $this->db->sql_fetchfield('result_count'); $this->db->sql_freeresult($result); @@ -828,6 +828,7 @@ class fulltext_mysql extends \phpbb\search\base // If the cache was completely empty count the results $sql_select = ($type == 'posts') ? 'p.post_id' : 't.topic_id'; + $sql_select .= $sort_by_sql[$sort_key] ? ", {$sort_by_sql[$sort_key]}" : ''; // Build the query for really selecting the post_ids if ($type == 'posts') @@ -856,7 +857,7 @@ class fulltext_mysql extends \phpbb\search\base AND t.topic_id = p.topic_id $sql_sort_join $sql_time - GROUP BY t.topic_id + GROUP BY $sql_select ORDER BY $sql_sort"; $field = 'topic_id'; } @@ -873,9 +874,10 @@ class fulltext_mysql extends \phpbb\search\base // retrieve the total result count if needed if (!$result_count) { - $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT($sql_select) as result_count", $sql); + $sql_found_rows = str_replace("SELECT $sql_select", "SELECT COUNT(*) as result_count", $sql); $result = $this->db->sql_query($sql_found_rows); - $result_count = (int) $this->db->sql_fetchfield('result_count'); + $result_count = ($type == 'posts') ? (int) $this->db->sql_fetchfield('result_count') : count($this->db->sql_fetchrowset($result)); + $this->db->sql_freeresult($result); if (!$result_count) diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index 05eddf56ef..689d2a8f40 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -968,6 +968,7 @@ class fulltext_native extends \phpbb\search\base $sql_array['WHERE'] = implode(' AND ', $sql_where); $sql_array['GROUP_BY'] = ($group_by) ? (($type == 'posts') ? 'p.post_id' : 'p.topic_id') . ', ' . $sort_by_sql[$sort_key] : ''; $sql_array['ORDER_BY'] = $sql_sort; + $sql_array['SELECT'] .= $sort_by_sql[$sort_key] ? ", {$sort_by_sql[$sort_key]}" : ''; unset($sql_where, $sql_sort, $group_by); @@ -983,7 +984,7 @@ class fulltext_native extends \phpbb\search\base // If using mysql and the total result count is not calculated yet, get it from the db if (!$total_results && $is_mysql) { - $sql_count = str_replace("SELECT {$sql_array['SELECT']}", "SELECT COUNT({$sql_array['SELECT']}) as total_results", $sql); + $sql_count = str_replace("SELECT {$sql_array['SELECT']}", "SELECT COUNT(DISTINCT {$sql_array['SELECT']}) as total_results", $sql); $result = $this->db->sql_query($sql_count); $total_results = (int) $this->db->sql_fetchfield('total_results'); $this->db->sql_freeresult($result); @@ -1005,7 +1006,6 @@ class fulltext_native extends \phpbb\search\base $id_ary[] = (int) $row[(($type == 'posts') ? 'post_id' : 'topic_id')]; } $this->db->sql_freeresult($result); - } // store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page @@ -1137,6 +1137,7 @@ class fulltext_native extends \phpbb\search\base } $select = ($type == 'posts') ? 'p.post_id' : 't.topic_id'; + $select .= $sort_by_sql[$sort_key] ? ", {$sort_by_sql[$sort_key]}" : ''; $is_mysql = false; /** @@ -1284,9 +1285,9 @@ class fulltext_native extends \phpbb\search\base if (!$total_results && $is_mysql) { - $sql_count = str_replace("SELECT $select", "SELECT COUNT($select) as total_results", $sql); + $sql_count = str_replace("SELECT $select", "SELECT COUNT(*) as total_results", $sql); $result = $this->db->sql_query($sql_count); - $total_results = (int) $this->db->sql_fetchfield('total_results'); + $total_results = ($type == 'posts') ? (int) $this->db->sql_fetchfield('total_results') : count($this->db->sql_fetchrowset($result)); $this->db->sql_freeresult($result); if (!$total_results) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index ebe036dee0..004ee13c25 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -18,17 +18,31 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { protected function assert_search_found($keywords, $posts_found, $words_highlighted) { + $this->purge_cache(); $crawler = self::request('GET', 'search.php?keywords=' . $keywords); - $this->assertEquals($posts_found, $crawler->filter('.postbody')->count()); - $this->assertEquals($words_highlighted, $crawler->filter('.posthilit')->count()); + $this->assertEquals($posts_found, $crawler->filter('.postbody')->count(), $this->search_backend); + $this->assertEquals($words_highlighted, $crawler->filter('.posthilit')->count(), $this->search_backend); + $this->assertStringContainsString("Search found $posts_found match", $crawler->filter('.searchresults-title')->text(), $this->search_backend); + } + + protected function assert_search_found_topics($keywords, $topics_found) + { + $this->purge_cache(); + $crawler = self::request('GET', 'search.php?sr=topics&keywords=' . $keywords); + $html = ''; + foreach ($crawler as $domElement) { + $html .= $domElement->ownerDocument->saveHTML($domElement); + } + $this->assertEquals($topics_found, $crawler->filter('.row')->count(), $html); + $this->assertStringContainsString("Search found $topics_found match", $crawler->filter('.searchresults-title')->text(), $html); } protected function assert_search_not_found($keywords) { $crawler = self::request('GET', 'search.php?keywords=' . $keywords); - $this->assertEquals(0, $crawler->filter('.postbody')->count()); + $this->assertEquals(0, $crawler->filter('.postbody')->count(),$this->search_backend); $split_keywords_string = str_replace('+', ' ', $keywords); - $this->assertEquals($split_keywords_string, $crawler->filter('#keywords')->attr('value')); + $this->assertEquals($split_keywords_string, $crawler->filter('#keywords')->attr('value'), $this->search_backend); } public function test_search_backend() @@ -67,6 +81,9 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->logout(); $this->assert_search_found('phpbb3+installation', 1, 3); $this->assert_search_found('foosubject+barsearch', 1, 2); + $this->assert_search_found_topics('phpbb3+installation', 1); + $this->assert_search_found_topics('foosubject+barsearch', 1); + $this->assert_search_not_found('loremipsumdedo'); $this->assert_search_found('barsearch-testing', 1, 2); // test hyphen ignored $this->assert_search_found('barsearch+-+testing', 1, 2); // test hyphen wrapped with space ignored From 015c9313a7de8989e5425e8cc2b4520b785413c1 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 31 Oct 2021 18:50:35 +0700 Subject: [PATCH 3/8] [ticket/16902] Extend test PHPBB3-16902 --- tests/functional/search/base.php | 74 +++++++++--- .../phpbb_functional_test_case.php | 108 ++++++++++++++++++ 2 files changed, 166 insertions(+), 16 deletions(-) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index 004ee13c25..ed0acc12bd 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -16,25 +16,37 @@ */ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { - protected function assert_search_found($keywords, $posts_found, $words_highlighted) + protected function assert_search_found($keywords, $posts_found, $words_highlighted, $sort_key = '') { $this->purge_cache(); - $crawler = self::request('GET', 'search.php?keywords=' . $keywords); + $crawler = self::request('GET', 'search.php?keywords=' . $keywords . ($sort_key ? "&sk=$sort_key" : '')); $this->assertEquals($posts_found, $crawler->filter('.postbody')->count(), $this->search_backend); $this->assertEquals($words_highlighted, $crawler->filter('.posthilit')->count(), $this->search_backend); $this->assertStringContainsString("Search found $posts_found match", $crawler->filter('.searchresults-title')->text(), $this->search_backend); } - protected function assert_search_found_topics($keywords, $topics_found) + protected function assert_search_found_topics($keywords, $topics_found, $sort_key = '') { $this->purge_cache(); - $crawler = self::request('GET', 'search.php?sr=topics&keywords=' . $keywords); - $html = ''; - foreach ($crawler as $domElement) { - $html .= $domElement->ownerDocument->saveHTML($domElement); - } - $this->assertEquals($topics_found, $crawler->filter('.row')->count(), $html); - $this->assertStringContainsString("Search found $topics_found match", $crawler->filter('.searchresults-title')->text(), $html); + $crawler = self::request('GET', 'search.php?sr=topics&keywords=' . $keywords . ($sort_key ? "&sk=$sort_key" : '')); + $this->assertEquals($topics_found, $crawler->filter('.row')->count(), $this->search_backend); + $this->assertStringContainsString("Search found $topics_found match", $crawler->filter('.searchresults-title')->text(), $this->search_backend); + } + + protected function assert_search_posts_by_author($author, $posts_found, $sort_key = '') + { + $this->purge_cache(); + $crawler = self::request('GET', 'search.php?author=' . $author . ($sort_key ? "&sk=$sort_key" : '')); + $this->assertEquals($posts_found, $crawler->filter('.postbody')->count(), $this->search_backend); + $this->assertStringContainsString("Search found $posts_found match", $crawler->filter('.searchresults-title')->text(), $this->search_backend); + } + + protected function assert_search_topics_by_author($author, $topics_found, $sort_key = '') + { + $this->purge_cache(); + $crawler = self::request('GET', 'search.php?sr=topics&author=' . $author . ($sort_key ? "&sk=$sort_key" : '')); + $this->assertEquals($topics_found, $crawler->filter('.row')->count(), $this->search_backend); + $this->assertStringContainsString("Search found $topics_found match", $crawler->filter('.searchresults-title')->text(), $this->search_backend); } protected function assert_search_not_found($keywords) @@ -45,8 +57,27 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->assertEquals($split_keywords_string, $crawler->filter('#keywords')->attr('value'), $this->search_backend); } + protected function assert_search_for_author_not_found($author) + { + $this->add_lang('search'); + $crawler = self::request('GET', 'search.php?author=' . $author); + $this->assertContainsLang('NO_SEARCH_RESULTS', $crawler->text(), $this->search_backend); + } + public function test_search_backend() { + // Create a new standard user if needed, topic and post to test searh for author + if (!$this->user_exists('searchforauthoruser')) + { + $searchforauthoruser_id = $this->create_user('searchforauthoruser'); + } + $this->remove_user_group('NEWLY_REGISTERED', ['searchforauthoruser']); + $this->disable_flood_interval(); + $this->login('searchforauthoruser'); + $topic_by_author = $this->create_topic(2, 'Test Topic from searchforauthoruser', 'This is a test topic posted by searchforauthoruser to test searching by author.'); + $this->create_post(2, $topic_by_author['topic_id'], 'Re: Test Topic from searchforauthoruser', 'This is a test post posted by searchforauthoruser'); + $this->logout(); + $this->login(); $this->admin_login(); @@ -54,6 +85,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $post = $this->create_topic(2, 'Test Topic 1 foosubject', 'This is a test topic posted by the barsearch testing framework.'); + $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=settings&sid=' . $this->sid); $form = $crawler->selectButton('Submit')->form(); $values = $form->getValues(); @@ -72,6 +104,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case if ($crawler->filter('.errorbox')->count() > 0) { $this->delete_topic($post['topic_id']); + $this->delete_topic($topic_by_author['topic_id']); $this->markTestSkipped("Search backend is not supported/running"); } @@ -79,20 +112,29 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case } $this->logout(); - $this->assert_search_found('phpbb3+installation', 1, 3); - $this->assert_search_found('foosubject+barsearch', 1, 2); - $this->assert_search_found_topics('phpbb3+installation', 1); - $this->assert_search_found_topics('foosubject+barsearch', 1); + + foreach (['', 'a', 't', 'f', 'i', 's'] as $sort_key) + { + $this->assert_search_found('phpbb3+installation', 1, 3, $sort_key); + $this->assert_search_found('foosubject+barsearch', 1, 2, $sort_key); + $this->assert_search_found('barsearch-testing', 1, 2, $sort_key); // test hyphen ignored + $this->assert_search_found('barsearch+-+testing', 1, 2, $sort_key); // test hyphen wrapped with space ignored + $this->assert_search_found_topics('phpbb3+installation', 1, $sort_key); + $this->assert_search_found_topics('foosubject+barsearch', 1, $sort_key); + + $this->assert_search_posts_by_author('searchforauthoruser', 2, $sort_key); + $this->assert_search_topics_by_author('searchforauthoruser', 1, $sort_key); + } $this->assert_search_not_found('loremipsumdedo'); - $this->assert_search_found('barsearch-testing', 1, 2); // test hyphen ignored - $this->assert_search_found('barsearch+-+testing', 1, 2); // test hyphen wrapped with space ignored $this->assert_search_not_found('barsearch+-testing'); // test excluding keyword + $this->assert_search_for_author_not_found('authornotexists'); $this->login(); $this->admin_login(); $this->delete_search_index(); $this->delete_topic($post['topic_id']); + $this->delete_topic($topic_by_author['topic_id']); } protected function create_search_index($backend = null) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 824dc2811a..35a1338850 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1437,4 +1437,112 @@ class phpbb_functional_test_case extends phpbb_test_case return $file_form_data; } + + /** + * Get HTML of the crawler + * See https://symfony.com/doc/current/components/dom_crawler.html#component-dom-crawler-dumping + * + * @param Symfony\Component\DomCrawler\Crawler $crawler Crawler instance + * @param string $url Request URL + * + * @return array Hidden form fields array + */ + protected function dump_crawler($crawler) + { + if (!$crawler) + { + return; + } + + $html = ''; + foreach ($crawler as $domElement) + { + $html .= $domElement->ownerDocument->saveHTML($domElement); + } + + return $html; + } + + /** + * Get username of currently logged in user + * + * @return string|bool username if logged in, false otherwise + */ + protected function get_logged_in_user() + { + $username_logged_in = false; + $crawler = self::request('GET', 'index.php'); + $is_logged_in = strpos($crawler->filter('div[class="navbar"]')->text(), 'Login') === false; + if ($is_logged_in) + { + $username_logged_in = $crawler->filter('li[id="username_logged_in"] > div > a > span')->text(); + } + return $username_logged_in; + } + + /** + * Disable posting flood control + */ + protected function disable_flood_interval() + { + $relogin_back = false; + $logged_in_username = $this->get_logged_in_user(); + if ($logged_in_username && $logged_in_username !== 'admin') + { + $this->logout(); + $relogin_back = true; + } + + if (!$logged_in_username || $relogin_back) + { + $this->login(); + $this->admin_login(); + } + + $this->add_lang('acp/common'); + $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=post&sid=' . $this->sid); + $form = $crawler->selectButton('submit')->form([ + 'config[flood_interval]' => 0, + ]); + $crawler = self::submit($form); + $this->assertContainsLang('CONFIG_UPDATED', $crawler->text()); + + // Get logged out back or get logged in in user back if needed + if (!$logged_in_username) + { + $this->logout(); + } + + if ($relogin_back) + { + $this->logout(); + $this->login($logged_in_username); + } + } + + /** + * Check if a user exists by username(s) or user_id(s) + * + * @param array &$user_id_ary The user ids to check or empty if usernames used + * @param array &$username_ary The usernames to check or empty if user ids used + * + * @return bool Returns true if a user exists, false otherwise + */ + protected function user_exists($username, $user_id = null) + { + global $db; + + $db = $this->get_db(); + + if (!function_exists('utf_clean_string')) + { + require_once(__DIR__ . '/../../phpBB/includes/utf/utf_tools.php'); + } + if (!function_exists('user_get_id_name')) + { + require_once(__DIR__ . '/../../phpBB/includes/functions_user.php'); + } + + return user_get_id_name($user_id, $username) ? false : true; + } } From ba487a24dcc5e53686cd0a05825ae62ea9b133f8 Mon Sep 17 00:00:00 2001 From: rxu Date: Mon, 1 Nov 2021 00:51:45 +0700 Subject: [PATCH 4/8] [ticket/16902] Fix PosgreSQL author topics search results count PHPBB3-16902 --- phpBB/phpbb/search/fulltext_postgres.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index 7eba463f2f..a0bb6242ec 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -836,8 +836,9 @@ class fulltext_postgres extends \phpbb\search\base GROUP BY t.topic_id, $sort_by_sql[$sort_key]"; } - $this->db->sql_query($sql_count); - $result_count = (int) $this->db->sql_fetchfield('result_count'); + $result = $this->db->sql_query($sql_count); + $result_count = ($type == 'posts') ? (int) $this->db->sql_fetchfield('result_count') : count($this->db->sql_fetchrowset($result)); + $this->db->sql_freeresult($result); if (!$result_count) { From b602b57b02d040744cb5faf5d01ccf85adba0e28 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 26 Nov 2021 11:34:52 +0700 Subject: [PATCH 5/8] [ticket/16902] Add search index deleted assertion to test PHPBB3-16902 --- tests/functional/search/base.php | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index ed0acc12bd..94067573fb 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -52,7 +52,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case protected function assert_search_not_found($keywords) { $crawler = self::request('GET', 'search.php?keywords=' . $keywords); - $this->assertEquals(0, $crawler->filter('.postbody')->count(),$this->search_backend); + $this->assertEquals(0, $crawler->filter('.postbody')->count(), $this->search_backend); $split_keywords_string = str_replace('+', ' ', $keywords); $this->assertEquals($split_keywords_string, $crawler->filter('#keywords')->attr('value'), $this->search_backend); } @@ -66,6 +66,8 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case public function test_search_backend() { + $this->add_lang('common'); + // Create a new standard user if needed, topic and post to test searh for author if (!$this->user_exists('searchforauthoruser')) { @@ -85,9 +87,8 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $post = $this->create_topic(2, 'Test Topic 1 foosubject', 'This is a test topic posted by the barsearch testing framework.'); - $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=settings&sid=' . $this->sid); - $form = $crawler->selectButton('Submit')->form(); + $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); $values = $form->getValues(); if ($values["config[search_type]"] != $this->search_backend) @@ -96,7 +97,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $form->setValues($values); $crawler = self::submit($form); - $form = $crawler->selectButton('Yes')->form(); + $form = $crawler->selectButton($this->lang('YES'))->form(); $values = $form->getValues(); $crawler = self::submit($form); @@ -141,13 +142,13 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { $this->add_lang('acp/search'); $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=index&sid=' . $this->sid); - $form = $crawler->selectButton('Create index')->form(); + $form = $crawler->selectButton($this->lang('CREATE_INDEX'))->form(); $form_values = $form->getValues(); $form_values = array_merge($form_values, - array( + [ 'search_type' => ( ($backend === null) ? $this->search_backend : $backend ), 'action' => 'create', - ) + ] ); $form->setValues($form_values); $crawler = self::submit($form); @@ -158,16 +159,21 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { $this->add_lang('acp/search'); $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=index&sid=' . $this->sid); - $form = $crawler->selectButton('Delete index')->form(); + $form = $crawler->selectButton($this->lang('DELETE_INDEX'))->form(); $form_values = $form->getValues(); $form_values = array_merge($form_values, - array( + [ 'search_type' => $this->search_backend, 'action' => 'delete', - ) + ] ); $form->setValues($form_values); $crawler = self::submit($form); $this->assertContainsLang('SEARCH_INDEX_REMOVED', $crawler->text()); + + // Ensure search index has been actually removed + $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=index&sid=' . $this->sid); + $posts_indexed = (int) $crawler->filter('#acp_search_index_' . $this->search_backend . ' td')->eq(1)->text(); + $this->assertEquals(0, $posts_indexed); } } From 6846eeaa48383b20f240f7b8d039941b47dc3348 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 26 Nov 2021 12:42:50 +0700 Subject: [PATCH 6/8] [ticket/16902] Add search index created assertion to test PHPBB3-16902 --- tests/functional/search/base.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index 94067573fb..2b648ec635 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -141,18 +141,24 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case protected function create_search_index($backend = null) { $this->add_lang('acp/search'); + $search_type = $backend ?? $this->search_backend; $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=index&sid=' . $this->sid); $form = $crawler->selectButton($this->lang('CREATE_INDEX'))->form(); $form_values = $form->getValues(); $form_values = array_merge($form_values, [ - 'search_type' => ( ($backend === null) ? $this->search_backend : $backend ), + 'search_type' => $search_type, 'action' => 'create', ] ); $form->setValues($form_values); $crawler = self::submit($form); $this->assertContainsLang('SEARCH_INDEX_CREATED', $crawler->text()); + + // Ensure search index has been actually created + $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=index&sid=' . $this->sid); + $posts_indexed = (int) $crawler->filter('#acp_search_index_' . $search_type . ' td')->eq(1)->text(); + $this->assertTrue($posts_indexed > 0); } protected function delete_search_index() From 5e43f6195ced3e5534a8f12749b0a5851683668c Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 15 Jan 2022 10:37:45 +0700 Subject: [PATCH 7/8] [ticket/16902] Make flood interval control in tests consistent Also remove crawler dumping tool PHPBB3-16902 --- tests/functional/feed_test.php | 18 ----------- tests/functional/mcp/mcp_main_test.php | 10 ++---- tests/functional/search/base.php | 3 +- .../functional/visibility_disapprove_test.php | 13 -------- .../functional/visibility_reapprove_test.php | 13 -------- .../visibility_unapproved_posts_test.php | 13 -------- .../phpbb_functional_test_case.php | 31 ++----------------- 7 files changed, 8 insertions(+), 93 deletions(-) diff --git a/tests/functional/feed_test.php b/tests/functional/feed_test.php index c1eb78938d..5a603d3175 100644 --- a/tests/functional/feed_test.php +++ b/tests/functional/feed_test.php @@ -868,24 +868,6 @@ class phpbb_functional_feed_test extends phpbb_functional_test_case $this->set_flood_interval(15); } - protected function set_flood_interval($flood_interval) - { - $this->login(); - $this->admin_login(); - - $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post'); - - $form = $crawler->selectButton('Submit')->form(); - $values = $form->getValues(); - - $values['config[flood_interval]'] = $flood_interval; - $form->setValues($values); - $crawler = self::submit($form); - self::assertGreaterThan(0, $crawler->filter('.successbox')->count()); - - $this->logout(); - } - public function test_feeds_unapproved_topic_admin() { $this->load_ids(array( diff --git a/tests/functional/mcp/mcp_main_test.php b/tests/functional/mcp/mcp_main_test.php index a440972058..a056217f73 100644 --- a/tests/functional/mcp/mcp_main_test.php +++ b/tests/functional/mcp/mcp_main_test.php @@ -22,13 +22,7 @@ class phpbb_functional_mcp_main_test extends phpbb_functional_test_case $this->login(); $this->admin_login(); - // Disable flood interval to post >1 of topics - $crawler = self::request('GET', "adm/index.php?i=acp_board&mode=post&sid={$this->sid}"); - $form = $crawler->selectButton($this->lang('SUBMIT'))->form([ - 'config[flood_interval]' => 0, - ]); - $crawler = self::submit($form); - $this->assertContainsLang('CONFIG_UPDATED', $crawler->text()); + $this->set_flood_interval(0); // Create a forum to move topics around $forum_name = 'MCP Test #1'; @@ -54,6 +48,8 @@ class phpbb_functional_mcp_main_test extends phpbb_functional_test_case $crawler = self::request('GET', "viewtopic.php?t={$post[1]['topic_id']}&sid={$this->sid}"); $this->assertStringContainsString('Testing merge topics moderation actions from MCP/View forum page.', $crawler->filter('html')->text()); + $this->set_flood_interval(15); + return $post; } diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index 2b648ec635..b414e42f10 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -74,7 +74,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $searchforauthoruser_id = $this->create_user('searchforauthoruser'); } $this->remove_user_group('NEWLY_REGISTERED', ['searchforauthoruser']); - $this->disable_flood_interval(); + $this->set_flood_interval(0); $this->login('searchforauthoruser'); $topic_by_author = $this->create_topic(2, 'Test Topic from searchforauthoruser', 'This is a test topic posted by searchforauthoruser to test searching by author.'); $this->create_post(2, $topic_by_author['topic_id'], 'Re: Test Topic from searchforauthoruser', 'This is a test post posted by searchforauthoruser'); @@ -86,6 +86,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->create_search_index('\phpbb\search\fulltext_native'); $post = $this->create_topic(2, 'Test Topic 1 foosubject', 'This is a test topic posted by the barsearch testing framework.'); + $this->set_flood_interval(15); $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=settings&sid=' . $this->sid); $form = $crawler->selectButton($this->lang('SUBMIT'))->form(); diff --git a/tests/functional/visibility_disapprove_test.php b/tests/functional/visibility_disapprove_test.php index f26e5436ae..f853491414 100644 --- a/tests/functional/visibility_disapprove_test.php +++ b/tests/functional/visibility_disapprove_test.php @@ -255,19 +255,6 @@ class phpbb_functional_visibility_disapprove_test extends phpbb_functional_test_ $this->assertEquals($details, $data, "Forum {$forum_id} does not match expected {$additional_error_message}"); } - protected function set_flood_interval($flood_interval) - { - $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post'); - - $form = $crawler->selectButton('Submit')->form(); - $values = $form->getValues(); - - $values["config[flood_interval]"] = $flood_interval; - $form->setValues($values); - $crawler = self::submit($form); - $this->assertGreaterThan(0, $crawler->filter('.successbox')->count()); - } - protected function load_ids($data) { $this->db = $this->get_db(); diff --git a/tests/functional/visibility_reapprove_test.php b/tests/functional/visibility_reapprove_test.php index 1affa87cd9..c9da0942b4 100644 --- a/tests/functional/visibility_reapprove_test.php +++ b/tests/functional/visibility_reapprove_test.php @@ -351,19 +351,6 @@ class phpbb_functional_visibility_reapprove_test extends phpbb_functional_test_c $this->assertEquals($details, $data, "Forum {$forum_id} does not match expected {$additional_error_message}"); } - protected function set_flood_interval($flood_interval) - { - $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post'); - - $form = $crawler->selectButton('Submit')->form(); - $values = $form->getValues(); - - $values["config[flood_interval]"] = $flood_interval; - $form->setValues($values); - $crawler = self::submit($form); - $this->assertGreaterThan(0, $crawler->filter('.successbox')->count()); - } - protected function load_ids($data) { $this->db = $this->get_db(); diff --git a/tests/functional/visibility_unapproved_posts_test.php b/tests/functional/visibility_unapproved_posts_test.php index f44d5186c4..814da8d4e7 100644 --- a/tests/functional/visibility_unapproved_posts_test.php +++ b/tests/functional/visibility_unapproved_posts_test.php @@ -268,19 +268,6 @@ class phpbb_functional_visibility_unapproved_test extends phpbb_functional_test_ $this->assertEquals($details, $data, "Forum {$forum_id} does not match expected {$additional_error_message}"); } - protected function set_flood_interval($flood_interval) - { - $crawler = self::request('GET', "adm/index.php?sid={$this->sid}&i=acp_board&mode=post"); - - $form = $crawler->selectButton('Submit')->form(); - $values = $form->getValues(); - - $values['config[flood_interval]'] = $flood_interval; - $form->setValues($values); - $crawler = self::submit($form); - $this->assertGreaterThan(0, $crawler->filter('.successbox')->count()); - } - protected function load_ids($data) { $this->db = $this->get_db(); diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 35a1338850..d5d0623d99 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1438,31 +1438,6 @@ class phpbb_functional_test_case extends phpbb_test_case return $file_form_data; } - /** - * Get HTML of the crawler - * See https://symfony.com/doc/current/components/dom_crawler.html#component-dom-crawler-dumping - * - * @param Symfony\Component\DomCrawler\Crawler $crawler Crawler instance - * @param string $url Request URL - * - * @return array Hidden form fields array - */ - protected function dump_crawler($crawler) - { - if (!$crawler) - { - return; - } - - $html = ''; - foreach ($crawler as $domElement) - { - $html .= $domElement->ownerDocument->saveHTML($domElement); - } - - return $html; - } - /** * Get username of currently logged in user * @@ -1481,9 +1456,9 @@ class phpbb_functional_test_case extends phpbb_test_case } /** - * Disable posting flood control + * Posting flood control */ - protected function disable_flood_interval() + protected function set_flood_interval($flood_interval) { $relogin_back = false; $logged_in_username = $this->get_logged_in_user(); @@ -1502,7 +1477,7 @@ class phpbb_functional_test_case extends phpbb_test_case $this->add_lang('acp/common'); $crawler = self::request('GET', 'adm/index.php?i=acp_board&mode=post&sid=' . $this->sid); $form = $crawler->selectButton('submit')->form([ - 'config[flood_interval]' => 0, + 'config[flood_interval]' => $flood_interval, ]); $crawler = self::submit($form); $this->assertContainsLang('CONFIG_UPDATED', $crawler->text()); From a8c93ff66151acf72fb9c8b5251969d2020a385f Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 18 Jan 2022 08:06:24 +0700 Subject: [PATCH 8/8] [ticket/16902] Fix docblock PHPBB3-16902 --- tests/test_framework/phpbb_functional_test_case.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index d5d0623d99..ae509475c9 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1496,10 +1496,10 @@ class phpbb_functional_test_case extends phpbb_test_case } /** - * Check if a user exists by username(s) or user_id(s) + * Check if a user exists by username or user_id * - * @param array &$user_id_ary The user ids to check or empty if usernames used - * @param array &$username_ary The usernames to check or empty if user ids used + * @param string $username The username to check or empty if user_id is used + * @param int $user_id The user id to check or empty if username is used * * @return bool Returns true if a user exists, false otherwise */