From 5a69fc22b453b2f90159ae1d8d990371df1b1ee1 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 30 Oct 2021 13:07:36 +0700 Subject: [PATCH] [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