[ticket/15540] Decouple create_index and delete_index from acp

PHPBB3-15540

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
+					try
-					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)))
+					{
+						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
+					try
-					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)))
 					{
+						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
 			"</script>\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;

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
+		$max_post_id = $this->get_max_post_id();
-			FROM ' . FORUMS_TABLE;
+		$forums_indexing_enabled = $this->forum_ids_with_indexing_enabled();
-		$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);
 
 		$starttime = microtime(true);
 		$row_count = 0;
 
-		$post_counter = &$acp_module->state[2];
+		while (still_on_time() && $post_counter <= $max_post_id)
-		while (still_on_time() && $post_counter <= $acp_module->max_post_id)
 		{
-			$sql = 'SELECT post_id, post_subject, post_text, poster_id, forum_id
+			$rows = $this->get_posts_between($post_counter + 1, $post_counter + self::BATCH_SIZE);
-				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);
 
-			$buffer = $this->db->sql_buffer_nested_transactions();
+			foreach ($rows as $row)
-
-			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)))
 			{
 				// 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 <= $max_post_id)
-		while (still_on_time() && $post_counter <= $acp_module->max_post_id)
 		{
-			$sql = 'SELECT post_id, poster_id, forum_id
+			$rows = $this->get_posts_between($post_counter + 1, $post_counter + self::BATCH_SIZE);
-				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);
-
 			$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
+		if ($post_counter <= $max_post_id)
-		$acp_module->save_state();
-
-		if ($post_counter <= $acp_module->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;
+	}
 }

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;
 	}
 
 	/**

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;
 	}
 
 	/**

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;
 	}
 
 	/**

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
+	 * {@inheritdoc}
-	*
+	 */
-	* @return string|bool error string is returned incase of errors otherwise false
+	public function create_index(int &$post_counter = null): ?array
-	*/
-	public function create_index($acp_module, $u_action)
 	{
-		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
+	 * {@inheritdoc}
-	*
-	* @return string|bool error string is returned incase of errors otherwise false
 	*/
-	public function delete_index($acp_module, $u_action)
+	public function delete_index(int &$post_counter = null): ?array
 	{
-		if (!$this->index_created())
+		if ($this->index_created()) {
-		{
+			$this->db_tools->sql_table_drop(SPHINX_TABLE);
-			return false;
 		}
 
-		$this->db_tools->sql_table_drop(SPHINX_TABLE);
+		return null;
-
-		return false;
 	}
 
 	/**
-	* Returns true if the sphinx table was created
+	 * {@inheritdoc}
-	*
-	* @return bool true if sphinx table was created
 	*/
 	public function index_created($allow_new_files = true)
 	{

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