From f78f3135fcd4155e527fcd72ba1b77f6ec0a8ac7 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 29 Oct 2021 21:32:26 +0700 Subject: [PATCH 1/9] [ticket/16902] Improve search results count for MySQL PHPBB3-16902 --- phpBB/phpbb/search/backend/fulltext_mysql.php | 13 +++++------ .../phpbb/search/backend/fulltext_native.php | 23 +++---------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/phpBB/phpbb/search/backend/fulltext_mysql.php b/phpBB/phpbb/search/backend/fulltext_mysql.php index c720bdd7d5..1ca202f39e 100644 --- a/phpBB/phpbb/search/backend/fulltext_mysql.php +++ b/phpBB/phpbb/search/backend/fulltext_mysql.php @@ -508,8 +508,7 @@ class fulltext_mysql extends base implements search_backend_interface ); 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) @@ -554,7 +553,7 @@ class fulltext_mysql extends base implements search_backend_interface // 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); @@ -752,12 +751,12 @@ class fulltext_mysql extends base implements search_backend_interface 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 @@ -771,7 +770,7 @@ class fulltext_mysql extends base implements search_backend_interface } 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 @@ -798,7 +797,7 @@ class fulltext_mysql extends base implements search_backend_interface // 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/backend/fulltext_native.php b/phpBB/phpbb/search/backend/fulltext_native.php index 0853e3a348..ba4f77d4e9 100644 --- a/phpBB/phpbb/search/backend/fulltext_native.php +++ b/phpBB/phpbb/search/backend/fulltext_native.php @@ -879,9 +879,6 @@ class fulltext_native extends base implements search_backend_interface 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; @@ -939,13 +936,6 @@ class fulltext_native extends base implements search_backend_interface ); } - // 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; @@ -961,10 +951,10 @@ class fulltext_native extends base implements search_backend_interface } $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); @@ -1157,7 +1147,6 @@ class fulltext_native extends base implements search_backend_interface switch ($this->db->get_sql_layer()) { case 'mysqli': -// $select = 'SQL_CALC_FOUND_ROWS ' . $select; $is_mysql = true; break; @@ -1250,13 +1239,7 @@ class fulltext_native extends base implements search_backend_interface 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 5a69fc22b453b2f90159ae1d8d990371df1b1ee1 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 30 Oct 2021 13:07:36 +0700 Subject: [PATCH 2/9] [ticket/16902] Improve test, use DISTINCT for precise results count PHPBB3-16902 --- phpBB/phpbb/search/backend/fulltext_mysql.php | 12 +++++---- .../phpbb/search/backend/fulltext_native.php | 9 ++++--- tests/functional/search/base.php | 25 ++++++++++++++++--- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/phpBB/phpbb/search/backend/fulltext_mysql.php b/phpBB/phpbb/search/backend/fulltext_mysql.php index 1ca202f39e..e671ffbd78 100644 --- a/phpBB/phpbb/search/backend/fulltext_mysql.php +++ b/phpBB/phpbb/search/backend/fulltext_mysql.php @@ -509,6 +509,7 @@ class fulltext_mysql extends base implements search_backend_interface 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) @@ -549,11 +550,10 @@ class fulltext_mysql extends base implements search_backend_interface $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); @@ -752,6 +752,7 @@ class fulltext_mysql extends base implements search_backend_interface // 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') @@ -780,7 +781,7 @@ class fulltext_mysql extends base implements search_backend_interface 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'; } @@ -797,9 +798,10 @@ class fulltext_mysql extends base implements search_backend_interface // 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/backend/fulltext_native.php b/phpBB/phpbb/search/backend/fulltext_native.php index ba4f77d4e9..ed3e53ed89 100644 --- a/phpBB/phpbb/search/backend/fulltext_native.php +++ b/phpBB/phpbb/search/backend/fulltext_native.php @@ -939,6 +939,7 @@ class fulltext_native extends base implements search_backend_interface $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); @@ -954,7 +955,7 @@ class fulltext_native extends base implements search_backend_interface // 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); @@ -976,7 +977,6 @@ class fulltext_native extends base implements search_backend_interface $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 @@ -1092,6 +1092,7 @@ class fulltext_native extends base implements search_backend_interface } $select = ($type == 'posts') ? 'p.post_id' : 't.topic_id'; + $select .= $sort_by_sql[$sort_key] ? ", {$sort_by_sql[$sort_key]}" : ''; $is_mysql = false; /** @@ -1239,9 +1240,9 @@ class fulltext_native extends base implements search_backend_interface 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 5e3f185c39..841a9be710 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -20,17 +20,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() @@ -79,6 +93,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 97906a736fa97e5a81b2a00322b9a687d5d18651 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 31 Oct 2021 18:50:35 +0700 Subject: [PATCH 3/9] [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 841a9be710..cbaabfcdc4 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -18,25 +18,37 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { protected $search_backend; - 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) @@ -47,8 +59,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(); @@ -56,6 +87,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 { // Search backed is not supported because don't appear in the select $this->delete_topic($post['topic_id']); + $this->delete_topic($topic_by_author['topic_id']); $this->markTestSkipped("Search backend is not supported/running"); } @@ -91,20 +124,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 65931d0e97..74310cf460 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1446,4 +1446,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 343551f2ce65621bba97bc9cc1aa86d3337bd58d Mon Sep 17 00:00:00 2001 From: rxu Date: Mon, 1 Nov 2021 00:51:45 +0700 Subject: [PATCH 4/9] [ticket/16902] Fix PosgreSQL author topics search results count PHPBB3-16902 --- phpBB/phpbb/search/backend/fulltext_postgres.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/search/backend/fulltext_postgres.php b/phpBB/phpbb/search/backend/fulltext_postgres.php index 8a941eef64..535e298e13 100644 --- a/phpBB/phpbb/search/backend/fulltext_postgres.php +++ b/phpBB/phpbb/search/backend/fulltext_postgres.php @@ -752,8 +752,9 @@ class fulltext_postgres extends base implements search_backend_interface 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 f14bf0ab9a008e983c53b76f0bb97ab7f933bbc4 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 26 Nov 2021 11:34:52 +0700 Subject: [PATCH 5/9] [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 cbaabfcdc4..46ec81ab86 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -54,7 +54,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); } @@ -68,6 +68,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')) { @@ -87,9 +89,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) @@ -110,7 +111,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $crawler = self::submit($form); - $form = $crawler->selectButton('Yes')->form(); + $form = $crawler->selectButton($this->lang('YES'))->form(); $values = $form->getValues(); $crawler = self::submit($form); @@ -153,13 +154,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); @@ -170,16 +171,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 f7a240f3faa51eeae6f3c8a70025056b7f4db264 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 26 Nov 2021 12:42:50 +0700 Subject: [PATCH 6/9] [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 46ec81ab86..99a231e7cf 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -153,18 +153,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 54fd71ea2edccad3399a9d4c04a9fbb2c72316e1 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 15 Jan 2022 10:37:45 +0700 Subject: [PATCH 7/9] [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 | 33 +++---------------- 7 files changed, 9 insertions(+), 94 deletions(-) diff --git a/tests/functional/feed_test.php b/tests/functional/feed_test.php index a6cc47aa67..dcbc9166d7 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 99a231e7cf..d26af94b72 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -76,7 +76,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'); @@ -88,6 +88,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->create_search_index('phpbb\\search\\backend\\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 52161ff58a..51e5641788 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 74310cf460..38980f57f2 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -983,7 +983,7 @@ class phpbb_functional_test_case extends phpbb_test_case // Any output before the doc type means there was an error $content = self::get_content(); self::assertStringNotContainsString('[phpBB Debug]', $content); - self::assertStringStartsWith('ownerDocument->saveHTML($domElement); - } - - return $html; - } - /** * Get username of currently logged in user * @@ -1490,9 +1465,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(); @@ -1511,7 +1486,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 c75a9b772a67d24e65d6ebc68619e5b68e313928 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 15 Jan 2022 13:16:57 +0700 Subject: [PATCH 8/9] [ticket/16902] Fix PostgreSQL search TypeError PHPBB3-16902 --- phpBB/phpbb/search/backend/fulltext_postgres.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/search/backend/fulltext_postgres.php b/phpBB/phpbb/search/backend/fulltext_postgres.php index 535e298e13..d9a7c366da 100644 --- a/phpBB/phpbb/search/backend/fulltext_postgres.php +++ b/phpBB/phpbb/search/backend/fulltext_postgres.php @@ -65,7 +65,7 @@ class fulltext_postgres extends base implements search_backend_interface * Operators are prefixed in search query and common words excluded * @var string */ - protected $search_query; + protected $search_query = ''; /** * Contains common words. From 0a88251fe5a9ff7400d25d2fd5032a7040e075c8 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 18 Jan 2022 08:06:24 +0700 Subject: [PATCH 9/9] [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 38980f57f2..5ca43de839 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1505,10 +1505,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 */