Limit number of topic subscriptions per user

Per Manuel Arostegui in T263817#7033384. The limit is 5000.
(I picked it arbitrarily, there's no real rationale for it.)

Also log a warning when any user reaches half of the limit,
so that we might make a decision about changing this mechanism
before it starts affecting users. Maybe at that time we'll
have data to show that it's safe to remove the limit.

Bug: T263817
Change-Id: I18a8ee0ad7383759229c5721d5253fb591457d4d
This commit is contained in:
Bartosz Dziewoński 2021-04-27 22:21:50 +02:00
parent 9974268b13
commit 6a50d1203c
5 changed files with 54 additions and 3 deletions

View file

@ -5,6 +5,8 @@
"apierror-discussiontools-commentid-notfound": "Comment with the ID '$1' not found.", "apierror-discussiontools-commentid-notfound": "Comment with the ID '$1' not found.",
"apierror-discussiontools-commentname-ambiguous": "Multiple comments with the name '$1' found, use ID instead.", "apierror-discussiontools-commentname-ambiguous": "Multiple comments with the name '$1' found, use ID instead.",
"apierror-discussiontools-commentname-notfound": "Comment with the name '$1' not found.", "apierror-discussiontools-commentname-notfound": "Comment with the name '$1' not found.",
"apierror-discussiontools-subscription-failed-add": "Could not subscribe to this topic.",
"apierror-discussiontools-subscription-failed-remove": "Could not unsubscribe from this topic.",
"apihelp-discussiontools-param-oldid": "The revision number to use (defaults to latest revision).", "apihelp-discussiontools-param-oldid": "The revision number to use (defaults to latest revision).",
"apihelp-discussiontools-paramvalue-paction-transcludedfrom": "Return titles of pages from which each comment on the given page is transcluded.", "apihelp-discussiontools-paramvalue-paction-transcludedfrom": "Return titles of pages from which each comment on the given page is transcluded.",
"apihelp-discussiontools-summary": "Returns metadata required to initialize the discussion tools.", "apihelp-discussiontools-summary": "Returns metadata required to initialize the discussion tools.",

View file

@ -7,6 +7,8 @@
"apierror-discussiontools-commentid-notfound": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment ID", "apierror-discussiontools-commentid-notfound": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment ID",
"apierror-discussiontools-commentname-ambiguous": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment name", "apierror-discussiontools-commentname-ambiguous": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment name",
"apierror-discussiontools-commentname-notfound": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment name", "apierror-discussiontools-commentname-notfound": "{{doc-apierror}}\n\nParameters:\n* $1 - Comment name",
"apierror-discussiontools-subscription-failed-add": "{{doc-apierror}}",
"apierror-discussiontools-subscription-failed-remove": "{{doc-apierror}}",
"apihelp-discussiontools-param-oldid": "{{doc-apihelp-param|discussiontools|oldid}}", "apihelp-discussiontools-param-oldid": "{{doc-apihelp-param|discussiontools|oldid}}",
"apihelp-discussiontools-paramvalue-paction-transcludedfrom": "{{doc-apihelp-paramvalue|discussiontools|paction|transcludedfrom}}", "apihelp-discussiontools-paramvalue-paction-transcludedfrom": "{{doc-apihelp-paramvalue|discussiontools|paction|transcludedfrom}}",
"apihelp-discussiontools-summary": "{{doc-apihelp-summary|discussiontools}}", "apihelp-discussiontools-summary": "{{doc-apihelp-summary|discussiontools}}",

View file

@ -58,16 +58,22 @@ class ApiDiscussionToolsSubscribe extends ApiBase {
$subscribe = $params['subscribe']; $subscribe = $params['subscribe'];
if ( $subscribe ) { if ( $subscribe ) {
$this->subscriptionStore->addSubscriptionForUser( $success = $this->subscriptionStore->addSubscriptionForUser(
$user, $user,
$title, $title,
$commentName $commentName
); );
if ( !$success ) {
$this->dieWithError( 'apierror-discussiontools-subscription-failed-add', 'subscription-failed' );
}
} else { } else {
$this->subscriptionStore->removeSubscriptionForUser( $success = $this->subscriptionStore->removeSubscriptionForUser(
$user, $user,
$commentName $commentName
); );
if ( !$success ) {
$this->dieWithError( 'apierror-discussiontools-subscription-failed-remove', 'subscription-failed' );
}
} }
// TODO: Subscribe should be tri-state: // TODO: Subscribe should be tri-state:
// * subscribe (add row) // * subscribe (add row)
@ -77,7 +83,7 @@ class ApiDiscussionToolsSubscribe extends ApiBase {
$result = [ $result = [
'page' => $title, 'page' => $title,
'commentname' => $commentName, 'commentname' => $commentName,
'subscribe' => $subscribe 'subscribe' => $subscribe,
]; ];
$this->getResult()->addValue( null, $this->getModuleName(), $result ); $this->getResult()->addValue( null, $this->getModuleName(), $result );

View file

@ -3,6 +3,7 @@
namespace MediaWiki\Extension\DiscussionTools; namespace MediaWiki\Extension\DiscussionTools;
use MediaWiki\Linker\LinkTarget; use MediaWiki\Linker\LinkTarget;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\User\UserFactory; use MediaWiki\User\UserFactory;
use MediaWiki\User\UserIdentity; use MediaWiki\User\UserIdentity;
use ReadOnlyMode; use ReadOnlyMode;
@ -18,6 +19,11 @@ use Wikimedia\Rdbms\IResultWrapper;
// use Wikimedia\Timestamp\ConvertibleTimestamp; // use Wikimedia\Timestamp\ConvertibleTimestamp;
class SubscriptionStore { class SubscriptionStore {
/**
* Maximum number of subscriptions that we can store for each user.
*/
private const USER_SUBSCRIPTION_LIMIT = 5000;
/** @var ILBFactory */ /** @var ILBFactory */
private $lbFactory; private $lbFactory;
@ -196,6 +202,36 @@ class SubscriptionStore {
); );
} }
/**
* @param UserIdentity $user
* @return bool
*/
private function userExceedsSubscriptionLimit( UserIdentity $user ) : bool {
$logger = LoggerFactory::getInstance( 'DiscussionTools' );
// This is always queried before updating
$db = $this->getConnectionRef( DB_PRIMARY );
$rowCount = $db->selectRowCount(
'discussiontools_subscription',
'*',
[ 'sub_user' => $user->getId() ],
__METHOD__,
[ 'LIMIT' => self::USER_SUBSCRIPTION_LIMIT ]
);
if ( $rowCount >= self::USER_SUBSCRIPTION_LIMIT / 2 ) {
$logger->warning(
"User {user} has {rowCount} subscriptions, approaching the limit",
[
'user' => $user->getId(),
'rowCount' => $rowCount,
]
);
}
return $rowCount >= self::USER_SUBSCRIPTION_LIMIT;
}
/** /**
* @param UserIdentity $user * @param UserIdentity $user
* @param LinkTarget $target * @param LinkTarget $target
@ -214,6 +250,9 @@ class SubscriptionStore {
if ( !$user->isRegistered() ) { if ( !$user->isRegistered() ) {
return false; return false;
} }
if ( $this->userExceedsSubscriptionLimit( $user ) ) {
return false;
}
$dbw = $this->getConnectionRef( DB_PRIMARY ); $dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw->upsert( $dbw->upsert(
'discussiontools_subscription', 'discussiontools_subscription',

View file

@ -285,6 +285,8 @@ function initTopicSubscriptions( $container ) {
) )
} }
); );
}, function ( code, data ) {
mw.notify( api.getErrorMessage( data ), { type: 'error' } );
} ); } );
} ); } );
} }