From 16220058d370ace0ab361d8f6d64bea1fc28d339 Mon Sep 17 00:00:00 2001 From: rubencm Date: Mon, 22 Mar 2021 21:54:36 +0100 Subject: [PATCH] [ticket/15540] Decouple create_index and delete_index from acp PHPBB3-15540 --- phpBB/includes/acp/acp_search.php | 56 ++++--- phpBB/phpbb/search/backend/base.php | 157 +++++++++++------- phpBB/phpbb/search/backend/fulltext_mysql.php | 13 +- .../phpbb/search/backend/fulltext_native.php | 4 +- .../search/backend/fulltext_postgres.php | 13 +- .../phpbb/search/backend/fulltext_sphinx.php | 35 ++-- .../backend/search_backend_interface.php | 13 +- 7 files changed, 166 insertions(+), 125 deletions(-) diff --git a/phpBB/includes/acp/acp_search.php b/phpBB/includes/acp/acp_search.php index 2c7e36d6e3..438d798b76 100644 --- a/phpBB/includes/acp/acp_search.php +++ b/phpBB/includes/acp/acp_search.php @@ -24,8 +24,6 @@ class acp_search var $u_action; var $state; var $search; - var $max_post_id; - var $batch_size = 100; function main($id, $mode) { @@ -220,7 +218,7 @@ class acp_search function index($id, $mode) { - global $db, $user, $template, $phpbb_log, $request; + global $user, $template, $phpbb_log, $request; global $config, $phpbb_admin_path, $phpEx, $phpbb_container; $action = $request->variable('action', ''); @@ -273,19 +271,28 @@ class acp_search $action = &$this->state[1]; - $this->max_post_id = $this->get_max_post_id(); - $this->save_state(); switch ($action) { case 'delete': - // pass a reference to myself so the $search object can make use of save_state() and attributes - if ($error = $this->search->delete_index($this, append_sid("{$phpbb_admin_path}index.$phpEx", "i=$id&mode=$mode&action=delete&hash=" . generate_link_hash('acp_search'), false))) + try + { + if ($status = $this->search->delete_index($this->state[2])) // Status is not null, so deleting is in progress.... + { + // save the current state + $this->save_state(); + + $u_action = append_sid("{$phpbb_admin_path}index.$phpEx", "i=$id&mode=$mode&action=delete&hash=" . generate_link_hash('acp_search'), false); + meta_refresh(1, $u_action); + trigger_error($user->lang('SEARCH_INDEX_DELETE_REDIRECT', (int) $status['row_count'], $status['post_counter']) . $user->lang('SEARCH_INDEX_DELETE_REDIRECT_RATE', $status['rows_per_second'])); + } + } + catch (Exception $e) { $this->state = array(''); $this->save_state(); - trigger_error($error . adm_back_link($this->u_action) . $this->close_popup_js(), E_USER_WARNING); + trigger_error($e->getMessage() . adm_back_link($this->u_action) . $this->close_popup_js(), E_USER_WARNING); } $this->search->tidy(); @@ -298,14 +305,28 @@ class acp_search break; case 'create': - // pass a reference to acp_search so the $search object can make use of save_state() and attributes - if ($error = $this->search->create_index($this, append_sid("{$phpbb_admin_path}index.$phpEx", "i=$id&mode=$mode&action=create&hash=" . generate_link_hash('acp_search'), false))) + try { + if ($status = $this->search->create_index($this->state[2])) // Status is not null, so indexing is in progress.... + { + // save the current state + $this->save_state(); + + $u_action = append_sid("{$phpbb_admin_path}index.$phpEx", "i=$id&mode=$mode&action=create&hash=" . generate_link_hash('acp_search'), false); + meta_refresh(1, $u_action); + trigger_error($user->lang('SEARCH_INDEX_CREATE_REDIRECT', (int) $status['row_count'], $status['post_counter']) . $user->lang('SEARCH_INDEX_CREATE_REDIRECT_RATE', $status['rows_per_second'])); + } + } + catch (Exception $e) + { + // Error executing create_index $this->state = array(''); $this->save_state(); - trigger_error($error . adm_back_link($this->u_action) . $this->close_popup_js(), E_USER_WARNING); + trigger_error($e->getMessage() . adm_back_link($this->u_action) . $this->close_popup_js(), E_USER_WARNING); } + // Indexing have finished + $this->search->tidy(); $this->state = array(''); @@ -421,19 +442,6 @@ class acp_search "\n"; } - function get_max_post_id() - { - global $db; - - $sql = 'SELECT MAX(post_id) as max_post_id - FROM '. POSTS_TABLE; - $result = $db->sql_query($sql); - $max_post_id = (int) $db->sql_fetchfield('max_post_id'); - $db->sql_freeresult($result); - - return $max_post_id; - } - function save_state($state = false) { global $config; diff --git a/phpBB/phpbb/search/backend/base.php b/phpBB/phpbb/search/backend/base.php index 4553ee0efb..172edeca4d 100644 --- a/phpBB/phpbb/search/backend/base.php +++ b/phpBB/phpbb/search/backend/base.php @@ -23,6 +23,9 @@ abstract class base implements search_backend_interface public const SEARCH_RESULT_IN_CACHE = 1; public const SEARCH_RESULT_INCOMPLETE = 2; + // Batch size for create_index and delete_index + private const BATCH_SIZE = 100; + /** * Retrieves cached search results * @@ -281,61 +284,31 @@ abstract class base implements search_backend_interface /** * {@inheritdoc} */ - public function create_index($acp_module, $u_action) + public function create_index(int &$post_counter = null): ?array { - $sql = 'SELECT forum_id, enable_indexing - FROM ' . FORUMS_TABLE; - $result = $this->db->sql_query($sql, 3600); - - while ($row = $this->db->sql_fetchrow($result)) - { - $forums[$row['forum_id']] = (bool) $row['enable_indexing']; - } - $this->db->sql_freeresult($result); + $max_post_id = $this->get_max_post_id(); + $forums_indexing_enabled = $this->forum_ids_with_indexing_enabled(); $starttime = microtime(true); $row_count = 0; - $post_counter = &$acp_module->state[2]; - while (still_on_time() && $post_counter <= $acp_module->max_post_id) + while (still_on_time() && $post_counter <= $max_post_id) { - $sql = 'SELECT post_id, post_subject, post_text, poster_id, forum_id - FROM ' . POSTS_TABLE . ' - WHERE post_id >= ' . (int) ($post_counter + 1) . ' - AND post_id <= ' . (int) ($post_counter + $acp_module->batch_size); - $result = $this->db->sql_query($sql); + $rows = $this->get_posts_between($post_counter + 1, $post_counter + self::BATCH_SIZE); - $buffer = $this->db->sql_buffer_nested_transactions(); - - if ($buffer) - { - $rows = $this->db->sql_fetchrowset($result); - $rows[] = false; // indicate end of array for while loop below - - $this->db->sql_freeresult($result); - } - - $i = 0; - while ($row = ($buffer ? $rows[$i++] : $this->db->sql_fetchrow($result))) + foreach ($rows as $row) { // Indexing enabled for this forum - if (isset($forums[$row['forum_id']]) && $forums[$row['forum_id']]) + if (in_array($row['forum_id'], $forums_indexing_enabled, true)) { $this->index('post', $row['post_id'], $row['post_text'], $row['post_subject'], $row['poster_id'], $row['forum_id']); } $row_count++; } - if (!$buffer) - { - $this->db->sql_freeresult($result); - } - $post_counter += $acp_module->batch_size; + $post_counter += self::BATCH_SIZE; } - // save the current state - $acp_module->save_state(); - // pretend the number of posts was as big as the number of ids we indexed so far // just an estimation as it includes deleted posts $num_posts = $this->config['num_posts']; @@ -343,39 +316,41 @@ abstract class base implements search_backend_interface $this->tidy(); $this->config['num_posts'] = $num_posts; - if ($post_counter <= $acp_module->max_post_id) + if ($post_counter <= $max_post_id) { $totaltime = microtime(true) - $starttime; $rows_per_second = $row_count / $totaltime; - meta_refresh(1, $u_action); - trigger_error($this->user->lang('SEARCH_INDEX_CREATE_REDIRECT', (int) $row_count, $post_counter) . $this->user->lang('SEARCH_INDEX_CREATE_REDIRECT_RATE', $rows_per_second)); + + return [ + 'row_count' => $row_count, + 'post_counter' => $post_counter, + 'max_post_id' => $max_post_id, + 'rows_per_second' => $rows_per_second, + ]; } + + return null; } /** * {@inheritdoc} */ - public function delete_index($acp_module, $u_action) + public function delete_index(int &$post_counter = null): ?array { + $max_post_id = $this->get_max_post_id(); + $starttime = microtime(true); $row_count = 0; - $post_counter = &$acp_module->state[2]; - while (still_on_time() && $post_counter <= $acp_module->max_post_id) + while (still_on_time() && $post_counter <= $max_post_id) { - $sql = 'SELECT post_id, poster_id, forum_id - FROM ' . POSTS_TABLE . ' - WHERE post_id >= ' . (int) ($post_counter + 1) . ' - AND post_id <= ' . (int) ($post_counter + $acp_module->batch_size); - $result = $this->db->sql_query($sql); - + $rows = $this->get_posts_between($post_counter + 1, $post_counter + self::BATCH_SIZE); $ids = $posters = $forum_ids = array(); - while ($row = $this->db->sql_fetchrow($result)) + foreach ($rows as $row) { $ids[] = $row['post_id']; $posters[] = $row['poster_id']; $forum_ids[] = $row['forum_id']; } - $result->sql_freeresult($result); $row_count += count($ids); if (count($ids)) @@ -383,18 +358,82 @@ abstract class base implements search_backend_interface $this->index_remove($ids, $posters, $forum_ids); } - $post_counter += $acp_module->batch_size; + $post_counter += self::BATCH_SIZE; } - // save the current state - $acp_module->save_state(); - - if ($post_counter <= $acp_module->max_post_id) + if ($post_counter <= $max_post_id) { $totaltime = microtime(true) - $starttime; $rows_per_second = $row_count / $totaltime; - meta_refresh(1, append_sid($u_action)); - trigger_error($this->user->lang('SEARCH_INDEX_DELETE_REDIRECT', (int) $row_count, $post_counter) . $this->user->lang('SEARCH_INDEX_DELETE_REDIRECT_RATE', $rows_per_second)); + + return [ + 'row_count' => $row_count, + 'post_counter' => $post_counter, + 'max_post_id' => $max_post_id, + 'rows_per_second' => $rows_per_second, + ]; } } + + /** + * Return the ids of the forums that have indexing enabled + * + * @return array + */ + protected function forum_ids_with_indexing_enabled(): array + { + $forums = []; + + $sql = 'SELECT forum_id, enable_indexing + FROM ' . FORUMS_TABLE; + $result = $this->db->sql_query($sql, 3600); + + while ($row = $this->db->sql_fetchrow($result)) + { + if ((bool) $row['enable_indexing']) + { + $forums[] = $row['forum_id']; + } + } + $this->db->sql_freeresult($result); + + return $forums; + } + + /** + * Get posts between 2 ids + * + * @param int $initial_id + * @param int $final_id + * @return \Generator + */ + protected function get_posts_between(int $initial_id, int $final_id): \Generator + { + $sql = 'SELECT post_id, post_subject, post_text, poster_id, forum_id + FROM ' . POSTS_TABLE . ' + WHERE post_id >= ' . $initial_id . ' + AND post_id <= ' . $final_id; + $result = $this->db->sql_query($sql); + + while($row = $this->db->sql_fetchrow($result)) + { + yield $row; + } + + $this->db->sql_freeresult($result); + } + + /** + * Get post with higher id + */ + protected function get_max_post_id(): int + { + $sql = 'SELECT MAX(post_id) as max_post_id + FROM '. POSTS_TABLE; + $result = $this->db->sql_query($sql); + $max_post_id = (int) $this->db->sql_fetchfield('max_post_id'); + $this->db->sql_freeresult($result); + + return $max_post_id; + } } diff --git a/phpBB/phpbb/search/backend/fulltext_mysql.php b/phpBB/phpbb/search/backend/fulltext_mysql.php index c42139138d..b35f392578 100644 --- a/phpBB/phpbb/search/backend/fulltext_mysql.php +++ b/phpBB/phpbb/search/backend/fulltext_mysql.php @@ -17,6 +17,7 @@ use phpbb\config\config; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher_interface; use phpbb\user; +use RuntimeException; /** * Fulltext search for MySQL @@ -920,12 +921,12 @@ class fulltext_mysql extends base implements search_backend_interface /** * {@inheritdoc} */ - public function create_index($acp_module, $u_action) + public function create_index(int &$post_counter = null): ?array { // Make sure we can actually use MySQL with fulltext indexes if ($error = $this->init()) { - return $error; + throw new RuntimeException($error); } if (empty($this->stats)) @@ -986,18 +987,18 @@ class fulltext_mysql extends base implements search_backend_interface $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); - return false; + return null; } /** * {@inheritdoc} */ - public function delete_index($acp_module, $u_action) + public function delete_index(int &$post_counter = null): ?array { // Make sure we can actually use MySQL with fulltext indexes if ($error = $this->init()) { - return $error; + throw new RuntimeException($error); } if (empty($this->stats)) @@ -1052,7 +1053,7 @@ class fulltext_mysql extends base implements search_backend_interface $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); - return false; + return null; } /** diff --git a/phpBB/phpbb/search/backend/fulltext_native.php b/phpBB/phpbb/search/backend/fulltext_native.php index ec2c9dcf1c..a49d4f59f8 100644 --- a/phpBB/phpbb/search/backend/fulltext_native.php +++ b/phpBB/phpbb/search/backend/fulltext_native.php @@ -1624,7 +1624,7 @@ class fulltext_native extends base implements search_backend_interface /** * {@inheritdoc} */ - public function delete_index($acp_module, $u_action) + public function delete_index(int &$post_counter = null): ?array { $sql_queries = []; @@ -1663,6 +1663,8 @@ class fulltext_native extends base implements search_backend_interface { $this->db->sql_query($sql_query); } + + return null; } /** diff --git a/phpBB/phpbb/search/backend/fulltext_postgres.php b/phpBB/phpbb/search/backend/fulltext_postgres.php index 8692683d96..863c2d43e6 100644 --- a/phpBB/phpbb/search/backend/fulltext_postgres.php +++ b/phpBB/phpbb/search/backend/fulltext_postgres.php @@ -17,6 +17,7 @@ use phpbb\config\config; use phpbb\db\driver\driver_interface; use phpbb\event\dispatcher_interface; use phpbb\user; +use RuntimeException; /** * Fulltext search for PostgreSQL @@ -875,12 +876,12 @@ class fulltext_postgres extends base implements search_backend_interface /** * {@inheritdoc} */ - public function create_index($acp_module, $u_action) + public function create_index(int &$post_counter = null): ?array { // Make sure we can actually use PostgreSQL with fulltext indexes if ($error = $this->init()) { - return $error; + throw new RuntimeException($error); } if (empty($this->stats)) @@ -928,18 +929,18 @@ class fulltext_postgres extends base implements search_backend_interface $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); - return false; + return null; } /** * {@inheritdoc} */ - public function delete_index($acp_module, $u_action) + public function delete_index(int &$post_counter = null): ?array { // Make sure we can actually use PostgreSQL with fulltext indexes if ($error = $this->init()) { - return $error; + throw new RuntimeException($error); } if (empty($this->stats)) @@ -987,7 +988,7 @@ class fulltext_postgres extends base implements search_backend_interface $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); - return false; + return null; } /** diff --git a/phpBB/phpbb/search/backend/fulltext_sphinx.php b/phpBB/phpbb/search/backend/fulltext_sphinx.php index 680577d5c1..cd8d2194e1 100644 --- a/phpBB/phpbb/search/backend/fulltext_sphinx.php +++ b/phpBB/phpbb/search/backend/fulltext_sphinx.php @@ -607,21 +607,19 @@ class fulltext_sphinx implements search_backend_interface } /** - * Nothing needs to be destroyed - */ + * Nothing needs to be destroyed + */ public function tidy() { $this->config->set('search_last_gc', time(), false); } /** - * Create sphinx table - * - * @return string|bool error string is returned incase of errors otherwise false - */ - public function create_index($acp_module, $u_action) + * {@inheritdoc} + */ + public function create_index(int &$post_counter = null): ?array { - if (!$this->index_created()) + if ($this->index_created()) { $table_data = array( 'COLUMNS' => array( @@ -643,30 +641,23 @@ class fulltext_sphinx implements search_backend_interface $this->db->sql_query($sql); } - return false; + return null; } /** - * Drop sphinx table - * - * @return string|bool error string is returned incase of errors otherwise false + * {@inheritdoc} */ - public function delete_index($acp_module, $u_action) + public function delete_index(int &$post_counter = null): ?array { - if (!$this->index_created()) - { - return false; + if ($this->index_created()) { + $this->db_tools->sql_table_drop(SPHINX_TABLE); } - $this->db_tools->sql_table_drop(SPHINX_TABLE); - - return false; + return null; } /** - * Returns true if the sphinx table was created - * - * @return bool true if sphinx table was created + * {@inheritdoc} */ public function index_created($allow_new_files = true) { diff --git a/phpBB/phpbb/search/backend/search_backend_interface.php b/phpBB/phpbb/search/backend/search_backend_interface.php index 21b8b4dff7..dc874e9460 100644 --- a/phpBB/phpbb/search/backend/search_backend_interface.php +++ b/phpBB/phpbb/search/backend/search_backend_interface.php @@ -15,9 +15,6 @@ namespace phpbb\search\backend; interface search_backend_interface { - // TODO: comprobar los pasos pr referencia - // TODO: eliminar acp_module de create index y delete index - /** * Returns the name of this search backend to be displayed to administrators * @@ -148,16 +145,18 @@ interface search_backend_interface /** * Create fulltext index * - * @return string|bool error string is returned incase of errors otherwise false + * @param int|null $post_counter + * @return array|null array with current status or null if finished */ - public function create_index($acp_module, $u_action); + public function create_index(int &$post_counter = null): ?array; /** * Drop fulltext index * - * @return string|bool error string is returned incase of errors otherwise false + * @param int|null $post_counter + * @return array|null array with current status or null if finished */ - public function delete_index($acp_module, $u_action); + public function delete_index(int &$post_counter = null): ?array; /** * Returns true if both FULLTEXT indexes exist