[ticket/17491] Consistently apply array_unique to search results

PHPBB-17491

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);
@@ -896,10 +895,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);

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);
@@ -1313,6 +1315,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);

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);
@@ -858,10 +856,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);

tests/functional/search/base.php

@@ -210,6 +210,158 @@ 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
+		foreach (range(2, $last_page) 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');