Merge "Turn push notification token list into a circular buffer."

This commit is contained in:
jenkins-bot 2021-07-30 14:46:10 +00:00 committed by Gerrit Code Review
commit 5566f84378
4 changed files with 26 additions and 53 deletions

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

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 {
@ -68,14 +67,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 ] );
}
if ( !$success ) {
$this->dieWithError( 'apierror-echo-push-token-exists' );
}

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 {

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() );
}
}