diff --git a/phpBB/phpbb/search/base.php b/phpBB/phpbb/search/base.php index e33dfc8f75..6d60d03986 100644 --- a/phpBB/phpbb/search/base.php +++ b/phpBB/phpbb/search/base.php @@ -76,10 +76,16 @@ class base } } - // If the sort direction differs from the direction in the cache, then reverse the ids array + // If the sort direction differs from the direction in the cache, then recalculate array keys if ($reverse_ids) { - $stored_ids = array_reverse($stored_ids); + $keys = array_keys($stored_ids); + array_walk($keys, function (&$value, $key) use ($result_count) + { + $value = ($value >= 0) ? $result_count - $value - 1 : $value; + } + ); + $stored_ids = array_combine($keys, $stored_ids); } for ($i = $start, $n = $start + $per_page; ($i < $n) && ($i < $result_count); $i++) @@ -130,6 +136,8 @@ class base } $store_ids = array_slice($id_ary, 0, $length); + $id_range = range($start, $start + $length - 1); + $store_ids = array_combine($id_range, $store_ids); // create a new resultset if there is none for this search_key yet // or add the ids to the existing resultset @@ -164,29 +172,26 @@ class base $db->sql_query($sql); $store = array(-1 => $result_count, -2 => $sort_dir); - $id_range = range($start, $start + $length - 1); } else { // we use one set of results for both sort directions so we have to calculate the indizes - // for the reversed array and we also have to reverse the ids themselves + // for the reversed array if ($store[-2] != $sort_dir) { - $store_ids = array_reverse($store_ids); - $id_range = range($store[-1] - $start - $length, $store[-1] - $start - 1); - } - else - { - $id_range = range($start, $start + $length - 1); + $keys = array_keys($store_ids); + array_walk($keys, function (&$value, $key) use ($store) { + $value = $store[-1] - $value - 1; + }); + $store_ids = array_combine($keys, $store_ids); } } - $store_ids = array_combine($id_range, $store_ids); - // append the ids if (is_array($store_ids)) { $store += $store_ids; + ksort($store); // if the cache is too big if (count($store) - 2 > 20 * $config['search_block_size']) diff --git a/phpBB/phpbb/search/fulltext_mysql.php b/phpBB/phpbb/search/fulltext_mysql.php index 577cf412f3..30fe6a87b3 100644 --- a/phpBB/phpbb/search/fulltext_mysql.php +++ b/phpBB/phpbb/search/fulltext_mysql.php @@ -609,7 +609,6 @@ 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)) { @@ -635,10 +634,10 @@ class fulltext_mysql extends \phpbb\search\base $id_ary[] = (int) $row[$field]; } $this->db->sql_freeresult($result); - - $id_ary = array_unique($id_ary); } + $id_ary = array_unique($id_ary); + // store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page $this->save_ids($search_key, implode(' ', $this->split_words), $author_ary, $result_count, $id_ary, $start, $sort_dir); $id_ary = array_slice($id_ary, 0, (int) $per_page); @@ -833,6 +832,8 @@ class fulltext_mysql extends \phpbb\search\base // Build the query for really selecting the post_ids if ($type == 'posts') { + // For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results + $sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC'); $sql = "SELECT $sql_select FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t ' : ' ') . " WHERE $sql_author @@ -896,10 +897,10 @@ class fulltext_mysql extends \phpbb\search\base $id_ary[] = (int) $row[$field]; } $this->db->sql_freeresult($result); - - $id_ary = array_unique($id_ary); } + $id_ary = array_unique($id_ary); + if (count($id_ary)) { $this->save_ids($search_key, '', $author_ary, $result_count, $id_ary, $start, $sort_dir); diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index a2d4dab98e..f15f58ab97 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -1012,6 +1012,8 @@ class fulltext_native extends \phpbb\search\base $this->db->sql_freeresult($result); } + $id_ary = array_unique($id_ary); + // store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page $this->save_ids($search_key, $this->search_query, $author_ary, $total_results, $id_ary, $start, $sort_dir); $id_ary = array_slice($id_ary, 0, (int) $per_page); @@ -1249,6 +1251,8 @@ class fulltext_native extends \phpbb\search\base // Build the query for really selecting the post_ids if ($type == 'posts') { + // For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results + $sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC'); $sql = "SELECT $select FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t' : '') . " WHERE $sql_author @@ -1313,6 +1317,8 @@ class fulltext_native extends \phpbb\search\base $this->db->sql_freeresult($result); } + $id_ary = array_unique($id_ary); + if (count($id_ary)) { $this->save_ids($search_key, '', $author_ary, $total_results, $id_ary, $start, $sort_dir); diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index 5a16b4eed3..18a6321e8e 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -545,8 +545,6 @@ class fulltext_postgres 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) { @@ -576,10 +574,10 @@ class fulltext_postgres extends \phpbb\search\base $id_ary[] = $row[$field]; } $this->db->sql_freeresult($result); - - $id_ary = array_unique($id_ary); } + $id_ary = array_unique($id_ary); + // store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page $this->save_ids($search_key, implode(' ', $this->split_words), $author_ary, $result_count, $id_ary, $start, $sort_dir); $id_ary = array_slice($id_ary, 0, (int) $per_page); @@ -766,6 +764,8 @@ class fulltext_postgres extends \phpbb\search\base // Build the query for really selecting the post_ids if ($type == 'posts') { + // For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results + $sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC'); $sql = "SELECT p.post_id FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t ' : ' ') . " WHERE $sql_author @@ -858,10 +858,10 @@ class fulltext_postgres extends \phpbb\search\base $id_ary[] = (int) $row[$field]; } $this->db->sql_freeresult($result); - - $id_ary = array_unique($id_ary); } + $id_ary = array_unique($id_ary); + if (count($id_ary)) { $this->save_ids($search_key, '', $author_ary, $result_count, $id_ary, $start, $sort_dir); diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index af5bd8066a..f8ce015cf1 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -221,6 +221,159 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->delete_topic($topic_multiple_results_count2['topic_id']); } + public function test_caching_search_results() + { + global $phpbb_root_path; + + // Sphinx search doesn't use phpBB search results caching + if (strpos($this->search_backend, 'fulltext_sphinx')) + { + $this->markTestSkipped("Sphinx search doesn't use phpBB search results caching"); + } + + $this->purge_cache(); + $this->login(); + $this->admin_login(); + + $crawler = self::request('GET', 'search.php?author_id=2&sr=posts'); + $posts_found_text = $crawler->filter('.searchresults-title')->text(); + + // Get total user's post count + preg_match('!(\d+)!', $posts_found_text, $matches); + $posts_count = (int) $matches[1]; + + $this->assertStringContainsString("Search found $posts_count matches", $posts_found_text, $this->search_backend); + + // Set this value to cache less results than total count + $sql = 'UPDATE ' . CONFIG_TABLE . ' + SET config_value = ' . floor($posts_count / 3) . " + WHERE config_name = '" . $this->db->sql_escape('search_block_size') . "'"; + $this->db->sql_query($sql); + + // Temporarily set posts_per_page to the value allowing to get several pages (4+) + $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post'); + $form = $crawler->selectButton('Submit')->form(); + $values = $form->getValues(); + $current_posts_per_page = $values['config[posts_per_page]']; + $values['config[posts_per_page]'] = floor($posts_count / 10); + $form->setValues($values); + $crawler = self::submit($form); + $this->assertEquals(1, $crawler->filter('.successbox')->count(), $this->search_backend); + + // Now actually test caching search results + $this->purge_cache(); + + // Default sort direction is 'd' (descending), browse the 1st page + $crawler = self::request('GET', 'search.php?author_id=2&sr=posts'); + $pagination = $crawler->filter('.pagination')->eq(0); + $posts_found_text = $pagination->text(); + + $this->assertStringContainsString("Search found $posts_count matches", $posts_found_text, $this->search_backend); + + // Filter all search result page links on the 1st page + $pagination = $pagination->filter('li > a')->reduce( + function ($node, $i) + { + return ($node->attr('class') == 'button'); + } + ); + + // Get last page number + $last_page = (int) $pagination->last()->text(); + + // Browse the last search page + $crawler = self::$client->click($pagination->selectLink($last_page)->link()); + $pagination = $crawler->filter('.pagination')->eq(0); + + // Filter all search result page links on the last page + $pagination = $pagination->filter('li > a')->reduce( + function ($node, $i) + { + return ($node->attr('class') == 'button'); + } + ); + + // Now change sort direction to ascending + $form = $crawler->selectButton('sort')->form(); + $values = $form->getValues(); + $values['sd'] = 'a'; + $form->setValues($values); + $crawler = self::submit($form); + + $pagination = $crawler->filter('.pagination')->eq(0); + + // Filter all search result page links on the 1st page with new sort direction + $pagination = $pagination->filter('li > a')->reduce( + function ($node, $i) + { + return ($node->attr('class') == 'button'); + } + ); + + // Browse the rest of search results pages with new sort direction + $pages = range(2, $last_page); + foreach ($pages as $page_number) + { + $crawler = self::$client->click($pagination->selectLink($page_number)->link()); + $pagination = $crawler->filter('.pagination')->eq(0); + $pagination = $pagination->filter('li > a')->reduce( + function ($node, $i) + { + return ($node->attr('class') == 'button'); + } + ); + } + + // Get search results cache varname + $finder = new \Symfony\Component\Finder\Finder(); + $finder + ->name('data_search_results_*.php') + ->files() + ->in($phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT); + $iterator = $finder->getIterator(); + $iterator->rewind(); + $cache_filename = $iterator->current(); + $cache_varname = substr($cache_filename->getBasename('.php'), 4); + + // Get cached post ids data + $cache = $this->get_cache_driver(); + $post_ids_cached = $cache->get($cache_varname); + + $cached_results_count = count($post_ids_cached) - 2; // Don't count '-1' and '-2' indexes + + $post_ids_cached_backup = $post_ids_cached; + + // Cached data still should have initial 'd' sort direction + $this->assertTrue($post_ids_cached[-2] === 'd', $this->search_backend); + + // Cached search results count should be equal to displayed on search results page + $this->assertEquals($posts_count, $post_ids_cached[-1], $this->search_backend); + + // Actual cached data array count should be equal to displayed on search results page too + $this->assertEquals($posts_count, $cached_results_count, $this->search_backend); + + // Cached data array shouldn't change after removing duplicates. That is, it shouldn't have any duplicates. + unset($post_ids_cached[-2], $post_ids_cached[-1]); + unset($post_ids_cached_backup[-2], $post_ids_cached_backup[-1]); + $post_ids_cached = array_unique($post_ids_cached); + $this->assertEquals($post_ids_cached_backup, $post_ids_cached, $this->search_backend); + + // Restore this value to default + $sql = 'UPDATE ' . CONFIG_TABLE . " + SET config_value = 250 + WHERE config_name = '" . $this->db->sql_escape('search_block_size') . "'"; + $this->db->sql_query($sql); + + // Restore posts_per_page value + $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[posts_per_page]'] = $current_posts_per_page; + $form->setValues($values); + $crawler = self::submit($form); + $this->assertEquals(1, $crawler->filter('.successbox')->count(), $this->search_backend); + } + protected function create_search_index($backend = null) { $this->add_lang('acp/search');