[ticket/13713] Rework batch size handling

PHPBB3-13713

phpBB/adm/style/acp_posting_buttons.html

@@ -13,7 +13,7 @@
 <!-- INCLUDEJS {T_ASSETS_PATH}/javascript/editor.js -->
 
 <!-- EVENT acp_posting_buttons_before -->
-<div id="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-mention-batch-size="{S_MENTION_BATCH_SIZE}" data-topic-id="{S_TOPIC_ID}" data-user-id="{S_USER_ID}"<!-- ENDIF -->>
+<div id="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-topic-id="{S_TOPIC_ID}" data-user-id="{S_USER_ID}"<!-- ENDIF -->>
 	<input type="button" class="button2" accesskey="b" name="addbbcode0" value=" B " style="font-weight:bold; width: 30px" onclick="bbstyle(0)" title="{L_BBCODE_B_HELP}" />
 	<input type="button" class="button2" accesskey="i" name="addbbcode2" value=" i " style="font-style:italic; width: 30px" onclick="bbstyle(2)" title="{L_BBCODE_I_HELP}" />
 	<input type="button" class="button2" accesskey="u" name="addbbcode4" value=" u " style="text-decoration: underline; width: 30px" onclick="bbstyle(4)" title="{L_BBCODE_U_HELP}" />

phpBB/assets/javascript/editor.js

@@ -388,19 +388,19 @@ function getCaretPosition(txtarea) {
 	function Mentions() {
 		let $mentionDataContainer = $('[data-mention-url]:first');
 		let mentionURL = $mentionDataContainer.data('mentionUrl');
-		let mentionBatchSize = $mentionDataContainer.data('mentionBatchSize');
 		let mentionNamesLimit = $mentionDataContainer.data('mentionNamesLimit');
 		let mentionTopicId = $mentionDataContainer.data('topicId');
 		let mentionUserId = $mentionDataContainer.data('userId');
 		let queryInProgress = null;
 		let cachedNames = [];
+		let cachedAll = [];
 		let cachedSearchKey = 'name';
 
 		function defaultAvatar(type) {
 			return (type === 'group') ? '<svg class="mention-media-avatar" xmlns="http://www.w3.org/2000/svg" viewbox="0 0 24 24"><path fill-rule="evenodd" d="M16 11c1.66 0 2.99-1.34 2.99-3S17.66 5 16 5c-1.66 0-3 1.34-3 3s1.34 3 3 3zm-8 0c1.66 0 2.99-1.34 2.99-3S9.66 5 8 5C6.34 5 5 6.34 5 8s1.34 3 3 3zm0 2c-2.33 0-7 1.17-7 3.5V19h14v-2.5c0-2.33-4.67-3.5-7-3.5zm8 0c-.29 0-.62.02-.97.05 1.16.84 1.97 1.97 1.97 3.45V19h6v-2.5c0-2.33-4.67-3.5-7-3.5z"/></svg>' : '<svg class="mention-media-avatar" xmlns="http://www.w3.org/2000/svg" viewbox="0 0 24 24"><path fill-rule="evenodd" d="M12,19.2C9.5,19.2 7.29,17.92 6,16C6.03,14 10,12.9 12,12.9C14,12.9 17.97,14 18,16C16.71,17.92 14.5,19.2 12,19.2M12,5A3,3 0 0,1 15,8A3,3 0 0,1 12,11A3,3 0 0,1 9,8A3,3 0 0,1 12,5M12,2A10,10 0 0,0 2,12A10,10 0 0,0 12,22A10,10 0 0,0 22,12C22,6.47 17.5,2 12,2Z"/></svg>';
 		}
 
-		function getCachedNames(query) {
+		function getCachedKeyword(query) {
 			if (!cachedNames) {
 				return null;
 			}
@@ -410,11 +410,11 @@ function getCaretPosition(txtarea) {
 			for (i = query.length; i > 0; i--) {
 				let startStr = query.substr(0, i);
 				if (cachedNames[startStr]) {
-					return cachedNames[startStr];
+					return startStr;
 				}
 			}
 
-			return cachedNames[''];
+			return '';
 		}
 
 		function getMatchedNames(query, items, searchKey) {
@@ -442,7 +442,8 @@ function getCaretPosition(txtarea) {
 				return;
 			}
 
-			let cachedNamesForQuery = getCachedNames(query);
+			let cachedKeyword = getCachedKeyword(query),
+				cachedNamesForQuery = (cachedKeyword) ? cachedNames[cachedKeyword] : null;
 
 			/*
 			* Use cached values when we can:
@@ -453,7 +454,7 @@ function getCaretPosition(txtarea) {
 			*/
 			if (cachedNamesForQuery &&
 				(getMatchedNames(query, cachedNamesForQuery, cachedSearchKey).length >= mentionNamesLimit ||
-					cachedNamesForQuery.length < mentionBatchSize)) {
+					cachedAll[cachedKeyword])) {
 				callback(cachedNamesForQuery);
 				return;
 			}
@@ -462,7 +463,8 @@ function getCaretPosition(txtarea) {
 
 			let params = {keyword: query, topic_id: mentionTopicId, _referer: location.href};
 			$.getJSON(mentionURL, params, function(data) {
-				cachedNames[query] = data;
+				cachedNames[query] = data.names;
+				cachedAll[query] = data.all;
 				callback(data);
 			}).always(function() {
 				queryInProgress = null;

phpBB/config/default/container/services_mention.yml

@@ -20,6 +20,7 @@ services:
         abstract: true
         arguments:
             - '@dbal.conn'
+            - '@config'
             - '@group_helper'
             - '@user'
             - '@auth'

phpBB/includes/acp/acp_board.php

@@ -223,6 +223,7 @@ class acp_board
 						'legend3'				=> 'MENTIONS',
 						'allow_mentions'		=> array('lang' => 'ALLOW_MENTIONS',	'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => false),
 						'mention_names_limit'	=> array('lang' => 'MENTION_NAMES_LIMIT',	'validate' => 'int:1:9999',		'type' => 'number:1:9999', 'explain' => false),
+						'mention_batch_size'	=> array('lang' => 'MENTION_BATCH_SIZE',	'validate' => 'int:1:9999',		'type' => 'number:1:9999', 'explain' => true),
 
 						'legend4'				=> 'ACP_SUBMIT_CHANGES',
 					)

phpBB/includes/functions.php

@@ -3947,7 +3947,6 @@ function page_header($page_title = '', $display_online_list = false, $item_id =
 		'U_FEED'				=> $controller_helper->route('phpbb_feed_index'),
 
 		'S_ALLOW_MENTIONS'		=> ($config['allow_mentions'] && $auth->acl_get('u_mention') && (empty($forum_id) || $auth->acl_get('f_mention', $forum_id))) ? true : false,
-		'S_MENTION_BATCH_SIZE'	=> 100, // TODO: do not hardcode the value
 		'S_MENTION_NAMES_LIMIT'	=> $config['mention_names_limit'],
 		'U_MENTION_URL'			=> $controller_helper->route('phpbb_mention_controller'),
 

phpBB/includes/functions_acp.php

@@ -90,7 +90,6 @@ function adm_page_header($page_title)
 		'U_INDEX'				=> append_sid("{$phpbb_root_path}index.$phpEx"),
 
 		'S_ALLOW_MENTIONS'		=> ($config['allow_mentions'] && $auth->acl_get('u_mention')) ? true : false,
-		'S_MENTION_BATCH_SIZE'	=> 100, // TODO: do not hardcode the value
 		'S_MENTION_NAMES_LIMIT'	=> $config['mention_names_limit'],
 		'U_MENTION_URL'			=> $controller_helper->route('phpbb_mention_controller'),
 

phpBB/install/schemas/schema_data.sql

@@ -235,6 +235,7 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('max_sig_img_height
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('max_sig_img_width', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('max_sig_smilies', '0');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('max_sig_urls', '5');
+INSERT INTO phpbb_config (config_name, config_value) VALUES ('mention_batch_size', '50');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('mention_names_limit', '10');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('mime_triggers', 'body|head|html|img|plaintext|a href|pre|script|table|title');
 INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_name_chars', '3');

phpBB/language/en/acp/board.php

@@ -189,6 +189,8 @@ $lang = array_merge($lang, array(
 	'MAX_POST_URLS'					=> 'Maximum links per post',
 	'MAX_POST_URLS_EXPLAIN'			=> 'Maximum number of URLs in a post. Set to 0 for unlimited links.',
 	'MENTIONS'						=> 'Mentions',
+	'MENTION_BATCH_SIZE'			=> 'Maximum number of names fetched from each source of names for a single request',
+	'MENTION_BATCH_SIZE_EXPLAIN'	=> 'Examples of sources: friends, topic repliers, group members etc.',
 	'MENTION_NAMES_LIMIT'			=> 'Maximum number of names in dropdown list',
 	'MIN_CHAR_LIMIT'				=> 'Minimum characters per post/message',
 	'MIN_CHAR_LIMIT_EXPLAIN'		=> 'The minimum number of characters the user need to enter within a post/private message. The minimum for this setting is 1.',

phpBB/phpbb/db/migration/data/v330/add_mention_settings.php

@@ -19,6 +19,7 @@ class add_mention_settings extends \phpbb\db\migration\migration
 	{
 		return array(
 			array('config.add', array('allow_mentions', true)),
+			array('config.add', array('mention_batch_size', 50)),
 			array('config.add', array('mention_names_limit', 10)),
 
 			// Set up user permissions

phpBB/phpbb/mention/controller/mention.php

@@ -52,12 +52,16 @@ class mention
 		$keyword = $this->request->variable('keyword', '', true);
 		$topic_id = $this->request->variable('topic_id', 0);
 		$names = [];
+		$hasNamesRemaining = false;
 
 		foreach ($this->mention_sources as $source)
 		{
-			$source->get($names, $keyword, $topic_id);
+			$hasNamesRemaining = !$source->get($names, $keyword, $topic_id) || $hasNamesRemaining;
 		}
 
-		return new JsonResponse(array_values($names));
+		return new JsonResponse([
+			'names' => array_values($names),
+			'all' => !$hasNamesRemaining,
+		]);
 	}
 }

phpBB/phpbb/mention/source/base_group.php

@@ -18,6 +18,9 @@ abstract class base_group implements source_interface
 	/** @var \phpbb\db\driver\driver_interface */
 	protected $db;
 
+	/** @var \phpbb\config\config */
+	protected $config;
+
 	/** @var \phpbb\group\helper */
 	protected $helper;
 
@@ -39,9 +42,10 @@ abstract class base_group implements source_interface
 	/**
 	 * Constructor
 	 */
-	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\group\helper $helper, \phpbb\user $user, \phpbb\auth\auth $auth, $phpbb_root_path, $phpEx)
+	public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, \phpbb\group\helper $helper, \phpbb\user $user, \phpbb\auth\auth $auth, $phpbb_root_path, $phpEx)
 	{
 		$this->db = $db;
+		$this->config = $config;
 		$this->helper = $helper;
 		$this->user = $user;
 		$this->auth = $auth;
@@ -138,8 +142,15 @@ abstract class base_group implements source_interface
 		$matches = preg_grep('/^' . preg_quote($keyword) . '.*/i', $groups['names']);
 		$group_ids = array_intersect($group_ids, array_flip($matches));
 
+		$i = 0;
 		foreach ($group_ids as $group_id)
 		{
+			if ($i >= $this->config['mention_batch_size'])
+			{
+				// Do not exceed the names limit
+				return false;
+			}
+
 			$group_rank = phpbb_get_user_rank($groups[$group_id], false);
 			array_push($names, [
 				'name'		=> $groups[$group_id]['group_name'],
@@ -152,6 +163,10 @@ abstract class base_group implements source_interface
 				'rank'		=> (isset($group_rank['title'])) ? $group_rank['title'] : '',
 				'priority'	=> $this->get_priority($groups[$group_id]),
 			]);
-		}
+
+			$i++;
+		}
+
+		return true;
 	}
 }

phpBB/phpbb/mention/source/base_user.php

@@ -15,9 +15,6 @@ namespace phpbb\mention\source;
 
 abstract class base_user implements source_interface
 {
-	/** @var int */
-	const NAMES_BATCH_SIZE = 100;
-
 	/** @var \phpbb\db\driver\driver_interface */
 	protected $db;
 
@@ -73,6 +70,7 @@ abstract class base_user implements source_interface
 	 */
 	public function get(array &$names, $keyword, $topic_id)
 	{
+		$fetched_all = false;
 		$keyword = utf8_clean_string($keyword);
 
 		// Do not query all possible users (just a moderate amount), cache results for 5 minutes
@@ -81,12 +79,13 @@ abstract class base_user implements source_interface
 		$i = 0;
 		$users = [];
 		$user_ids = [];
-		while ($i < self::NAMES_BATCH_SIZE)
+		while ($i < $this->config['mention_batch_size'])
 		{
 			$row = $this->db->sql_fetchrow($result);
 
 			if (!$row)
 			{
+				$fetched_all = true;
 				break;
 			}
 
@@ -100,6 +99,24 @@ abstract class base_user implements source_interface
 			$user_ids[] = $row['user_id'];
 		}
 
+		// Determine whether all usernames were fetched in current batch
+		if (!$fetched_all)
+		{
+			$fetched_all = true;
+
+			while ($row = $this->db->sql_fetchrow($result))
+			{
+				if (!empty($keyword) && strpos($row['username_clean'], $keyword) !== 0)
+				{
+					continue;
+				}
+
+				// At least one username hasn't been fetched - exit loop
+				$fetched_all = false;
+				break;
+			}
+		}
+
 		$this->db->sql_freeresult($result);
 
 		// Load all user data with a single SQL query, needed for ranks and avatars
@@ -120,5 +137,7 @@ abstract class base_user implements source_interface
 				'priority'	=> $this->get_priority($user),
 			]);
 		}
+
+		return $fetched_all;
 	}
 }

phpBB/phpbb/mention/source/source_interface.php

@@ -22,6 +22,7 @@ interface source_interface
 	 * @param array  $names    Array of already fetched data with names
 	 * @param string $keyword  Search string
 	 * @param int    $topic_id Current topic ID
+	 * @return bool Whether there are no more satisfying names left
 	 */
 	public function get(array &$names, $keyword, $topic_id);
 

phpBB/styles/prosilver/template/posting_buttons.html

@@ -39,7 +39,7 @@
 </div>
 
 <!-- EVENT posting_editor_buttons_before -->
-<div id="format-buttons" class="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-mention-batch-size="{S_MENTION_BATCH_SIZE}" data-topic-id="{S_TOPIC_ID}" data-user-id="{S_USER_ID}"<!-- ENDIF -->>
+<div id="format-buttons" class="format-buttons"<!-- IF S_ALLOW_MENTIONS --> data-mention-url="{U_MENTION_URL}" data-mention-names-limit="{S_MENTION_NAMES_LIMIT}" data-topic-id="{S_TOPIC_ID}" data-user-id="{S_USER_ID}"<!-- ENDIF -->>
 	<button type="button" class="button button-icon-only bbcode-b" accesskey="b" name="addbbcode0" value=" B " onclick="bbstyle(0)" title="{L_BBCODE_B_HELP}">
 		{{ Icon('iconify', 'mdi:format-bold', '', true, 'c-button-icon') }}
 	</button>

tests/mention/controller_test.php

@@ -55,6 +55,7 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 		// Config
 		$config = new \phpbb\config\config(array(
 			'allow_mentions'      => true,
+			'mention_batch_size'  => 8,
 			'mention_names_limit' => 3,
 		));
 
@@ -143,11 +144,11 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 		/**
 		 * NOTE:
 		 * 1) in production comparison with 'myself' is being done in JS
-		 * 2) mention_names_limit does not limit the number of returned items
+		 * 2) team members of hidden groups can also be mentioned (because they are shown on teampage)
-		 * 3) team members of hidden groups can also be mentioned (because they are shown on teampage)
 		 */
 		return [
 			['', 0, [
+				'names' => [
 					[
 						'name'     => 'friend',
 						'type'     => 'u',
@@ -291,28 +292,6 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 0,
 					],
-				[
-					'name'     => 'test2',
-					'type'     => 'u',
-					'id'       => 10,
-					'avatar'   => [
-						'type' => 'user',
-						'img'  => '',
-					],
-					'rank'     => '',
-					'priority' => 0,
-				],
-				[
-					'name'     => 'test3',
-					'type'     => 'u',
-					'id'       => 11,
-					'avatar'   => [
-						'type' => 'user',
-						'img'  => '',
-					],
-					'rank'     => '',
-					'priority' => 0,
-				],
 					[
 						'name'     => 'Group we are a member of',
 						'type'     => 'g',
@@ -324,8 +303,11 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 1,
 					],
+				],
+				'all'   => false,
 			]],
 			['', 1, [
+				'names' => [
 					[
 						'name'     => 'friend',
 						'type'     => 'u',
@@ -491,28 +473,6 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 0,
 					],
-				[
-					'name'     => 'test2',
-					'type'     => 'u',
-					'id'       => 10,
-					'avatar'   => [
-						'type' => 'user',
-						'img'  => '',
-					],
-					'rank'     => '',
-					'priority' => 0,
-				],
-				[
-					'name'     => 'test3',
-					'type'     => 'u',
-					'id'       => 11,
-					'avatar'   => [
-						'type' => 'user',
-						'img'  => '',
-					],
-					'rank'     => '',
-					'priority' => 0,
-				],
 					[
 						'name'     => 'Group we are a member of',
 						'type'     => 'g',
@@ -524,8 +484,11 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 1,
 					],
+				],
+				'all'   => false,
 			]],
 			['t', 1, [
+				'names' => [
 					[
 						'name'     => 'team_member_hidden',
 						'type'     => 'u',
@@ -614,8 +577,11 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 0,
 					],
+				],
+				'all'   => true,
 			]],
 			['test', 1, [
+				'names' => [
 					[
 						'name'     => 'test',
 						'type'     => 'u',
@@ -660,8 +626,11 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 						'rank'     => '',
 						'priority' => 0,
 					],
+				],
+				'all'   => true,
 			]],
-			['test1', 1, [[
+			['test1', 1, [
+				'names' => [[
 					'name'     => 'test1',
 					'type'     => 'u',
 					'id'       => 9,
@@ -671,7 +640,9 @@ class phpbb_mention_controller_test extends phpbb_database_test_case
 					],
 					'rank'     => '',
 					'priority' => 0,
-			]]],
+				]],
+				'all'   => true,
+			]],
 		];
 	}