wikimedia/mediawiki-extensions-Echo
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/Echo synced 2024-11-23 23:44:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Turn push notification token list into a circular buffer.

Browse source 
At the moment we support a maximum of 10 tokens per user for subscribing
to push notifications, stored as a basic list that runs out when the
limit is reached.  There may, however, be some edge cases where an app
registers a token and then forgets to unregister it (and repeats this 10
times), after which time it will be unable to register any new token.

This changes the token list to behave more like a circular buffer, by
simply deleting the oldest token before inserting the new one. This way
an app could register a new token even in the rare case of forgetting
the previous ten.

Change-Id: I387de63460882e4e56d1aa6db1f78d73a0495208
This commit is contained in:
Dmitry Brant 2021-07-23 09:23:31 -04:00
parent b4ec1eda88
commit 6c5a88107c
4 changed files with 26 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
includes/Push/SubscriptionManager.php
View file

@ -5,7 +5,6 @@ namespace EchoPush;
use EchoAbstractMapper;
use IDatabase;
use MediaWiki\Storage\NameTableStore;
use OverflowException;
use Wikimedia\Rdbms\DBError;
class SubscriptionManager extends EchoAbstractMapper {
@ -47,14 +46,6 @@ class SubscriptionManager extends EchoAbstractMapper {
$this->maxSubscriptionsPerUser = $maxSubscriptionsPerUser;
}
/**
* Get the configured maximum number of stored subscriptions per user.
* @return int
*/
public function getMaxSubscriptionsPerUser(): int {
return $this->maxSubscriptionsPerUser;
}
/**
* Store push subscription information for a central user ID.
* @param string $provider Provider name string (validated by presence in the PARAM_TYPE array)
@ -62,11 +53,20 @@ class SubscriptionManager extends EchoAbstractMapper {
* @param int $centralId
* @param string|null $topic APNS topic string
* @return bool true if the subscription was created; false if the token already exists
* @throws OverflowException if the user already has >= the configured max subscriptions
*/
public function create( string $provider, string $token, int $centralId, ?string $topic = null ): bool {
if ( $this->userHasMaxAllowedSubscriptions( $centralId ) ) {
throw new OverflowException( 'Max subscriptions exceeded' );
$subscriptions = $this->getSubscriptionsForUser( $centralId );
if ( count( $subscriptions ) >= $this->maxSubscriptionsPerUser ) {
// If we exceed the number of subscriptions for this user, then delete the oldest subscription
// before inserting the new one, making it behave like a circular buffer.
// (Find the oldest subscription by iterating, since their order in the DB is not guaranteed.)
$oldest = $subscriptions[0];
foreach ( $subscriptions as $subscription ) {
if ( $subscription->getUpdated() < $oldest->getUpdated() ) {
$oldest = $subscription;
}
}
$this->delete( [ $oldest->getToken() ], $centralId );
}
$topicId = $topic ? $this->pushTopicStore->acquireId( $topic ) : null;
$this->dbw->insert(
@ -131,27 +131,4 @@ class SubscriptionManager extends EchoAbstractMapper {
return $this->dbw->affectedRows();
}
/**
* Get count of all registered subscriptions for a user (by central ID).
* @param int $centralId
* @return int
*/
private function getSubscriptionCountForUser( int $centralId ) {
return $this->dbr->selectRowCount(
'echo_push_subscription',
'eps_id',
[ 'eps_user' => $centralId ],
__METHOD__
);
}
/**
* Returns true if the central user has >= the configured maximum push subscriptions in the DB
* @param int $centralId
* @return bool
*/
private function userHasMaxAllowedSubscriptions( int $centralId ): bool {
return $this->getSubscriptionCountForUser( $centralId ) >= $this->maxSubscriptionsPerUser;
}
}

10
includes/api/Push/ApiEchoPushSubscriptionsCreate.php
View file

@ -7,7 +7,6 @@ use ApiMain;
use EchoPush\SubscriptionManager;
use EchoPush\Utils;
use EchoServices;
use OverflowException;
use Wikimedia\ParamValidator\ParamValidator;
class ApiEchoPushSubscriptionsCreate extends ApiBase {
@ -69,14 +68,7 @@ class ApiEchoPushSubscriptionsCreate extends ApiBase {
}
}
$userId = Utils::getPushUserId( $this->getUser() );
try {
$success = $this->subscriptionManager->create( $provider, $token, $userId, $topic );
} catch ( OverflowException $e ) {
$maxSubscriptionsPerUser = $this->subscriptionManager->getMaxSubscriptionsPerUser();
$this->dieWithError( [ 'apierror-echo-push-too-many-subscriptions',
$maxSubscriptionsPerUser ] );
}
$success = $this->subscriptionManager->create( $provider, $token, $userId, $topic );
if ( !$success ) {
$this->dieWithError( 'apierror-echo-push-token-exists' );
}

13
tests/phpunit/api/Push/ApiEchoPushSubscriptionsCreateTest.php
View file

@ -36,10 +36,17 @@ class ApiEchoPushSubscriptionsCreateTest extends ApiTestCase {
$result = $this->doApiRequestWithToken( $params, null, $this->user );
$this->assertEquals( 'Success', $result[0]['create']['result'] );
// After max subscriptions reached
// Make sure it's possible to register a new token even when limit is reached
$params['providertoken'] = 'DEF456';
$this->expectException( ApiUsageException::class );
$this->doApiRequestWithToken( $params, null, $this->user );
$result = $this->doApiRequestWithToken( $params, null, $this->user );
$this->assertEquals( 'Success', $result[0]['create']['result'] );
// Explicitly verify that the oldest token was removed
$subscriptionManager = EchoServices::getInstance()->getPushSubscriptionManager();
$subscriptions = $subscriptionManager->getSubscriptionsForUser( Utils::getPushUserId( $this->user ) );
foreach ( $subscriptions as $subscription ) {
$this->assertNotEquals( 'XYZ789', $subscription->getToken() );
}
}
public function testApiCreateSubscriptionTokenExists(): void {

9
tests/phpunit/integration/Push/SubscriptionManagerTest.php
View file

@ -26,22 +26,19 @@ class SubscriptionManagerTest extends MediaWikiIntegrationTestCase {
$subscriptionManager->create( 'test', 'ABC123', $centralId );
$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
$this->assertCount( 1, $subscriptions );
$this->assertTrue( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
$subscriptionManager->delete( [ 'ABC123' ], $centralId );
$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
$this->assertCount( 0, $subscriptions );
$this->assertFalse( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
$subscriptionManager->create( 'test', 'ABC123', $centralId );
$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
$this->assertCount( 1, $subscriptions );
$this->assertTrue( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
$subscriptionManager->delete( [ 'ABC123' ] );
$subscriptionManager->create( 'test', 'DEF456', $centralId );
$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
$this->assertCount( 0, $subscriptions );
$this->assertFalse( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
$this->assertCount( 1, $subscriptions );
$this->assertEquals( 'DEF456', $subscriptions[0]->getToken() );
}
}