Skip to content

Commit b6200d6

Browse files
marc1706marc1706
authored
Merge pull request phpbb#6800 from rxu/ticket/17491-master
[ticket/17491] Fix caching search results - master
2 parents b7db1b0 + 4b7d7c2 commit b6200d6

File tree

5 files changed

+188
-23
lines changed

5 files changed

+188
-23
lines changed

‎phpBB/phpbb/search/backend/base.php‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,16 @@ protected function obtain_ids(string $search_key, int &$result_count, array &$id
112112
}
113113
}
114114

115-
// If the sort direction differs from the direction in the cache, then reverse the ids array
115+
// If the sort direction differs from the direction in the cache, then recalculate array keys
116116
if ($reverse_ids)
117117
{
118-
$stored_ids = array_reverse($stored_ids);
118+
$keys = array_keys($stored_ids);
119+
array_walk($keys, function (&$value, $key) use ($result_count)
120+
{
121+
$value = ($value >= 0) ? $result_count - $value - 1 : $value;
122+
}
123+
);
124+
$stored_ids = array_combine($keys, $stored_ids);
119125
}
120126

121127
for ($i = $start, $n = $start + $per_page; ($i < $n) && ($i < $result_count); $i++)
@@ -166,6 +172,8 @@ protected function save_ids(string $search_key, string $keywords, array $author_
166172
}
167173

168174
$store_ids = array_slice($id_ary, 0, $length);
175+
$id_range = range($start, $start + $length - 1);
176+
$store_ids = array_combine($id_range, $store_ids);
169177

170178
// create a new resultset if there is none for this search_key yet
171179
// or add the ids to the existing resultset
@@ -200,29 +208,26 @@ protected function save_ids(string $search_key, string $keywords, array $author_
200208
$this->db->sql_query($sql);
201209

202210
$store = array(-1 => $result_count, -2 => $sort_dir);
203-
$id_range = range($start, $start + $length - 1);
204211
}
205212
else
206213
{
207214
// we use one set of results for both sort directions so we have to calculate the indizes
208-
// for the reversed array and we also have to reverse the ids themselves
215+
// for the reversed array
209216
if ($store[-2] != $sort_dir)
210217
{
211-
$store_ids = array_reverse($store_ids);
212-
$id_range = range($store[-1] - $start - $length, $store[-1] - $start - 1);
213-
}
214-
else
215-
{
216-
$id_range = range($start, $start + $length - 1);
218+
$keys = array_keys($store_ids);
219+
array_walk($keys, function (&$value, $key) use ($store) {
220+
$value = $store[-1] - $value - 1;
221+
});
222+
$store_ids = array_combine($keys, $store_ids);
217223
}
218224
}
219225

220-
$store_ids = array_combine($id_range, $store_ids);
221-
222226
// append the ids
223227
if (is_array($store_ids))
224228
{
225229
$store += $store_ids;
230+
ksort($store);
226231

227232
// if the cache is too big
228233
if (count($store) - 2 > 20 * $this->config['search_block_size'])

‎phpBB/phpbb/search/backend/fulltext_mysql.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,6 @@ public function keyword_search(string $type, string $fields, string $terms, arra
550550
}
551551
$this->db->sql_freeresult($result);
552552

553-
$id_ary = array_unique($id_ary);
554553
// if the total result count is not cached yet, retrieve it from the db
555554
if (!$result_count && count($id_ary))
556555
{
@@ -576,10 +575,10 @@ public function keyword_search(string $type, string $fields, string $terms, arra
576575
$id_ary[] = (int) $row[$field];
577576
}
578577
$this->db->sql_freeresult($result);
579-
580-
$id_ary = array_unique($id_ary);
581578
}
582579

580+
$id_ary = array_unique($id_ary);
581+
583582
// store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page
584583
$this->save_ids($search_key, implode(' ', $this->split_words), $author_ary, $result_count, $id_ary, $start, $sort_dir);
585584
$id_ary = array_slice($id_ary, 0, (int) $per_page);
@@ -758,6 +757,8 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
758757
// Build the query for really selecting the post_ids
759758
if ($type == 'posts')
760759
{
760+
// For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results
761+
$sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC');
761762
$sql = "SELECT $sql_select
762763
FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t ' : ' ') . "
763764
WHERE $sql_author
@@ -821,10 +822,10 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
821822
$id_ary[] = (int) $row[$field];
822823
}
823824
$this->db->sql_freeresult($result);
824-
825-
$id_ary = array_unique($id_ary);
826825
}
827826

827+
$id_ary = array_unique($id_ary);
828+
828829
if (count($id_ary))
829830
{
830831
$this->save_ids($search_key, '', $author_ary, $result_count, $id_ary, $start, $sort_dir);

‎phpBB/phpbb/search/backend/fulltext_native.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,8 @@ public function keyword_search(string $type, string $fields, string $terms, arra
10041004
$this->db->sql_freeresult($result);
10051005
}
10061006

1007+
$id_ary = array_unique($id_ary);
1008+
10071009
// store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page
10081010
$this->save_ids($search_key, $this->search_query, $author_ary, $total_results, $id_ary, $start, $sort_dir);
10091011
$id_ary = array_slice($id_ary, 0, (int) $per_page);
@@ -1225,6 +1227,8 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
12251227
// Build the query for really selecting the post_ids
12261228
if ($type == 'posts')
12271229
{
1230+
// For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results
1231+
$sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC');
12281232
$sql = "SELECT $select
12291233
FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t' : '') . "
12301234
WHERE $sql_author
@@ -1289,6 +1293,8 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
12891293
$this->db->sql_freeresult($result);
12901294
}
12911295

1296+
$id_ary = array_unique($id_ary);
1297+
12921298
if (count($id_ary))
12931299
{
12941300
$this->save_ids($search_key, '', $author_ary, $total_results, $id_ary, $start, $sort_dir);

‎phpBB/phpbb/search/backend/fulltext_postgres.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,8 +478,6 @@ public function keyword_search(string $type, string $fields, string $terms, arra
478478
}
479479
$this->db->sql_freeresult($result);
480480

481-
$id_ary = array_unique($id_ary);
482-
483481
// if the total result count is not cached yet, retrieve it from the db
484482
if (!$result_count)
485483
{
@@ -509,10 +507,10 @@ public function keyword_search(string $type, string $fields, string $terms, arra
509507
$id_ary[] = $row[$field];
510508
}
511509
$this->db->sql_freeresult($result);
512-
513-
$id_ary = array_unique($id_ary);
514510
}
515511

512+
$id_ary = array_unique($id_ary);
513+
516514
// store the ids, from start on then delete anything that isn't on the current page because we only need ids for one page
517515
$this->save_ids($search_key, implode(' ', $this->split_words), $author_ary, $result_count, $id_ary, $start, $sort_dir);
518516
$id_ary = array_slice($id_ary, 0, (int) $per_page);
@@ -683,6 +681,8 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
683681
// Build the query for really selecting the post_ids
684682
if ($type == 'posts')
685683
{
684+
// For sorting by non-unique columns, add unique sort key to avoid duplicated rows in results
685+
$sql_sort .= ', p.post_id' . (($sort_dir == 'a') ? ' ASC' : ' DESC');
686686
$sql = "SELECT p.post_id
687687
FROM " . $sql_sort_table . POSTS_TABLE . ' p' . (($firstpost_only) ? ', ' . TOPICS_TABLE . ' t ' : ' ') . "
688688
WHERE $sql_author
@@ -775,10 +775,10 @@ public function author_search(string $type, bool $firstpost_only, array $sort_by
775775
$id_ary[] = (int) $row[$field];
776776
}
777777
$this->db->sql_freeresult($result);
778-
779-
$id_ary = array_unique($id_ary);
780778
}
781779

780+
$id_ary = array_unique($id_ary);
781+
782782
if (count($id_ary))
783783
{
784784
$this->save_ids($search_key, '', $author_ary, $result_count, $id_ary, $start, $sort_dir);

‎tests/functional/search/base.php‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,159 @@ public function test_search_backend()
229229
$this->delete_topic($topic_multiple_results_count2['topic_id']);
230230
}
231231

232+
public function test_caching_search_results()
233+
{
234+
global $phpbb_root_path;
235+
236+
// Sphinx search doesn't use phpBB search results caching
237+
if (strpos($this->search_backend, 'fulltext_sphinx'))
238+
{
239+
$this->markTestSkipped("Sphinx search doesn't use phpBB search results caching");
240+
}
241+
242+
$this->purge_cache();
243+
$this->login();
244+
$this->admin_login();
245+
246+
$crawler = self::request('GET', 'search.php?author_id=2&sr=posts');
247+
$posts_found_text = $crawler->filter('.searchresults-title')->text();
248+
249+
// Get total user's post count
250+
preg_match('!(\d+)!', $posts_found_text, $matches);
251+
$posts_count = (int) $matches[1];
252+
253+
$this->assertStringContainsString("Search found $posts_count matches", $posts_found_text, $this->search_backend);
254+
255+
// Set this value to cache less results than total count
256+
$sql = 'UPDATE ' . CONFIG_TABLE . '
257+
SET config_value = ' . floor($posts_count / 3) . "
258+
WHERE config_name = '" . $this->db->sql_escape('search_block_size') . "'";
259+
$this->db->sql_query($sql);
260+
261+
// Temporarily set posts_per_page to the value allowing to get several pages (4+)
262+
$crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post');
263+
$form = $crawler->selectButton('Submit')->form();
264+
$values = $form->getValues();
265+
$current_posts_per_page = $values['config[posts_per_page]'];
266+
$values['config[posts_per_page]'] = floor($posts_count / 10);
267+
$form->setValues($values);
268+
$crawler = self::submit($form);
269+
$this->assertEquals(1, $crawler->filter('.successbox')->count(), $this->search_backend);
270+
271+
// Now actually test caching search results
272+
$this->purge_cache();
273+
274+
// Default sort direction is 'd' (descending), browse the 1st page
275+
$crawler = self::request('GET', 'search.php?author_id=2&sr=posts');
276+
$pagination = $crawler->filter('.pagination')->eq(0);
277+
$posts_found_text = $pagination->text();
278+
279+
$this->assertStringContainsString("Search found $posts_count matches", $posts_found_text, $this->search_backend);
280+
281+
// Filter all search result page links on the 1st page
282+
$pagination = $pagination->filter('li > a')->reduce(
283+
function ($node, $i)
284+
{
285+
return ($node->attr('class') == 'button');
286+
}
287+
);
288+
289+
// Get last page number
290+
$last_page = (int) $pagination->last()->text();
291+
292+
// Browse the last search page
293+
$crawler = self::$client->click($pagination->selectLink($last_page)->link());
294+
$pagination = $crawler->filter('.pagination')->eq(0);
295+
296+
// Filter all search result page links on the last page
297+
$pagination = $pagination->filter('li > a')->reduce(
298+
function ($node, $i)
299+
{
300+
return ($node->attr('class') == 'button');
301+
}
302+
);
303+
304+
// Now change sort direction to ascending
305+
$form = $crawler->selectButton('sort')->form();
306+
$values = $form->getValues();
307+
$values['sd'] = 'a';
308+
$form->setValues($values);
309+
$crawler = self::submit($form);
310+
311+
$pagination = $crawler->filter('.pagination')->eq(0);
312+
313+
// Filter all search result page links on the 1st page with new sort direction
314+
$pagination = $pagination->filter('li > a')->reduce(
315+
function ($node, $i)
316+
{
317+
return ($node->attr('class') == 'button');
318+
}
319+
);
320+
321+
// Browse the rest of search results pages with new sort direction
322+
$pages = range(2, $last_page);
323+
foreach ($pages as $page_number)
324+
{
325+
$crawler = self::$client->click($pagination->selectLink($page_number)->link());
326+
$pagination = $crawler->filter('.pagination')->eq(0);
327+
$pagination = $pagination->filter('li > a')->reduce(
328+
function ($node, $i)
329+
{
330+
return ($node->attr('class') == 'button');
331+
}
332+
);
333+
}
334+
335+
// Get search results cache varname
336+
$finder = new \Symfony\Component\Finder\Finder();
337+
$finder
338+
->name('data_search_results_*.php')
339+
->files()
340+
->in($phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT);
341+
$iterator = $finder->getIterator();
342+
$iterator->rewind();
343+
$cache_filename = $iterator->current();
344+
$cache_varname = substr($cache_filename->getBasename('.php'), 4);
345+
346+
// Get cached post ids data
347+
$cache = $this->get_cache_driver();
348+
$post_ids_cached = $cache->get($cache_varname);
349+
350+
$cached_results_count = count($post_ids_cached) - 2; // Don't count '-1' and '-2' indexes
351+
352+
$post_ids_cached_backup = $post_ids_cached;
353+
354+
// Cached data still should have initial 'd' sort direction
355+
$this->assertTrue($post_ids_cached[-2] === 'd', $this->search_backend);
356+
357+
// Cached search results count should be equal to displayed on search results page
358+
$this->assertEquals($posts_count, $post_ids_cached[-1], $this->search_backend);
359+
360+
// Actual cached data array count should be equal to displayed on search results page too
361+
$this->assertEquals($posts_count, $cached_results_count, $this->search_backend);
362+
363+
// Cached data array shouldn't change after removing duplicates. That is, it shouldn't have any duplicates.
364+
unset($post_ids_cached[-2], $post_ids_cached[-1]);
365+
unset($post_ids_cached_backup[-2], $post_ids_cached_backup[-1]);
366+
$post_ids_cached = array_unique($post_ids_cached);
367+
$this->assertEquals($post_ids_cached_backup, $post_ids_cached, $this->search_backend);
368+
369+
// Restore this value to default
370+
$sql = 'UPDATE ' . CONFIG_TABLE . "
371+
SET config_value = 250
372+
WHERE config_name = '" . $this->db->sql_escape('search_block_size') . "'";
373+
$this->db->sql_query($sql);
374+
375+
// Restore posts_per_page value
376+
$crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_board&mode=post');
377+
$form = $crawler->selectButton('Submit')->form();
378+
$values = $form->getValues();
379+
$values['config[posts_per_page]'] = $current_posts_per_page;
380+
$form->setValues($values);
381+
$crawler = self::submit($form);
382+
$this->assertEquals(1, $crawler->filter('.successbox')->count(), $this->search_backend);
383+
}
384+
232385
protected function create_search_index($backend = null)
233386
{
234387
$this->add_lang('acp/search');

0 commit comments

Comments
 (0)