Merge "Create and enforce a config setting for max subscriptions per user"

ServiceWiring.php

@@ -43,7 +43,10 @@ return [
 			$database
 		);
 
-		return new SubscriptionManager( $dbw, $dbr, $centralIdLookup, $pushProviderStore );
+		$maxSubscriptionsPerUser = $echoConfig->get( 'EchoPushMaxSubscriptionsPerUser' );
+
+		return new SubscriptionManager( $dbw, $dbr, $centralIdLookup, $pushProviderStore,
+			$maxSubscriptionsPerUser );
 	}
 
 ];

extension.json

@@ -1013,6 +1013,10 @@
 		"EchoPushServiceBaseUrl": {
 			"value": false,
 			"description": "Request endpoint URL for the push notification service"
+		},
+		"EchoPushMaxSubscriptionsPerUser": {
+			"value": 0,
+			"description": "Maximum number of push subscriptions that may be stored in the DB at any given time for a single central user ID."
 		}
 	},
 	"attributes": {

i18n/api/en.json

@@ -79,5 +79,6 @@
 	"apiwarn-echo-deprecation-html": "<kbd>notformat=html</kbd> has been deprecated and will be removed soon. Use <kbd>notformat=special</kbd> instead.",
 	"apierror-echo-event-creation-failed": "Could not create Echo event",
 	"apierror-echo-push-token-exists": "The provided token already exists in the database.",
-	"apierror-echo-push-token-not-found": "The provided token was not found in the database."
+	"apierror-echo-push-token-not-found": "The provided token was not found in the database.",
+	"apierror-echo-push-too-many-subscriptions": "The current user has already registered the maximum allowed number of push subscriptions ($1)."
 }

i18n/api/qqq.json

@@ -81,5 +81,6 @@
 	"apiwarn-echo-deprecation-html": "{{doc-apierror}}",
 	"apierror-echo-event-creation-failed": "{{doc-apierror}}",
 	"apierror-echo-push-token-exists": "{{doc-apierror}}",
-	"apierror-echo-push-token-not-found": "{{doc-apierror}}"
+	"apierror-echo-push-token-not-found": "{{doc-apierror}}",
+	"apierror-echo-push-too-many-subscriptions": "{{doc-apierror}}"
 }

includes/Push/SubscriptionManager.php

@@ -6,6 +6,7 @@ use CentralIdLookup;
 use EchoAbstractMapper;
 use IDatabase;
 use MediaWiki\Storage\NameTableStore;
+use OverflowException;
 use User;
 use Wikimedia\Rdbms\DBError;
 
@@ -23,23 +24,37 @@ class SubscriptionManager extends EchoAbstractMapper {
 	/** @var NameTableStore */
 	private $pushProviderStore;
 
+	/** @var int */
+	private $maxSubscriptionsPerUser;
+
 	/**
 	 * @param IDatabase $dbw primary DB connection (for writes)
 	 * @param IDatabase $dbr replica DB connection (for reads)
 	 * @param CentralIdLookup $centralIdLookup
 	 * @param NameTableStore $pushProviderStore
+	 * @param int $maxSubscriptionsPerUser
 	 */
 	public function __construct(
 		IDatabase $dbw,
 		IDatabase $dbr,
 		CentralIdLookup $centralIdLookup,
-		NameTableStore $pushProviderStore
+		NameTableStore $pushProviderStore,
+		int $maxSubscriptionsPerUser
 	) {
 		parent::__construct();
 		$this->dbw = $dbw;
 		$this->dbr = $dbr;
 		$this->centralIdLookup = $centralIdLookup;
 		$this->pushProviderStore = $pushProviderStore;
+		$this->maxSubscriptionsPerUser = $maxSubscriptionsPerUser;
+	}
+
+	/**
+	 * Get the configured maximum number of stored subscriptions per user.
+	 * @return int
+	 */
+	public function getMaxSubscriptionsPerUser(): int {
+		return $this->maxSubscriptionsPerUser;
 	}
 
 	/**
@@ -48,12 +63,17 @@ class SubscriptionManager extends EchoAbstractMapper {
 	 * @param string $provider Provider name string (validated by presence in the PARAM_TYPE array)
 	 * @param string $token Subscriber token provided by the push provider
 	 * @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( User $user, string $provider, string $token ): bool {
+		$centralId = $this->getCentralId( $user );
+		if ( $this->userHasMaxAllowedSubscriptions( $centralId ) ) {
+			throw new OverflowException( 'Max subscriptions exceeded' );
+		}
 		$this->dbw->insert(
 			'echo_push_subscription',
 			[
-				'eps_user' => $this->getCentralId( $user ),
+				'eps_user' => $centralId,
 				'eps_provider' => $this->pushProviderStore->acquireId( $provider ),
 				'eps_token' => $token,
 				'eps_token_sha256' => hash( 'sha256', $token ),
@@ -66,7 +86,7 @@ class SubscriptionManager extends EchoAbstractMapper {
 	}
 
 	/**
-	 * Get all registered subscriptions for a user (by central ID).
+	 * Get full data for all registered subscriptions for a user (by central ID).
 	 * @param int $centralId
 	 * @return array array of Subscription objects
 	 */
@@ -108,6 +128,29 @@ 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;
+	}
+
 	/**
 	 * Get the user's central ID.
 	 * @param User $user

includes/api/Push/ApiEchoPushSubscriptionsCreate.php

@@ -6,6 +6,7 @@ use ApiBase;
 use ApiMain;
 use EchoPush\SubscriptionManager;
 use EchoServices;
+use OverflowException;
 use Wikimedia\ParamValidator\ParamValidator;
 
 class ApiEchoPushSubscriptionsCreate extends ApiBase {
@@ -58,7 +59,13 @@ class ApiEchoPushSubscriptionsCreate extends ApiBase {
 	public function execute(): void {
 		$provider = $this->getParameter( 'provider' );
 		$token = $this->getParameter( 'providertoken' );
+		try {
 			$success = $this->subscriptionManager->create( $this->getUser(), $provider, $token );
+		} 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' );
 		}

tests/phpunit/api/Push/ApiEchoPushSubscriptionsCreateTest.php

@@ -13,7 +13,10 @@ class ApiEchoPushSubscriptionsCreateTest extends ApiTestCase {
 
 	public function setUp(): void {
 		parent::setUp();
-		$this->setMwGlobals( 'wgEchoEnablePush', true );
+		$this->setMwGlobals( [
+			'wgEchoEnablePush' => true,
+			'wgEchoPushMaxSubscriptionsPerUser' => 2
+		] );
 		$this->tablesUsed[] = 'echo_push_subscription';
 		$this->tablesUsed[] = 'echo_push_provider';
 		$this->user = $this->getTestUser()->getUser();
@@ -21,6 +24,7 @@ class ApiEchoPushSubscriptionsCreateTest extends ApiTestCase {
 	}
 
 	public function testApiCreateSubscription(): void {
+		// Before max subscriptions reached
 		$params = [
 			'action' => 'echopushsubscriptions',
 			'command' => 'create',
@@ -29,6 +33,11 @@ class ApiEchoPushSubscriptionsCreateTest extends ApiTestCase {
 		];
 		$result = $this->doApiRequestWithToken( $params, null, $this->user );
 		$this->assertEquals( 'Success', $result[0]['create']['result'] );
+
+		// After max subscriptions reached
+		$params['providertoken'] = 'DEF456';
+		$this->expectException( ApiUsageException::class );
+		$this->doApiRequestWithToken( $params, null, $this->user );
 	}
 
 	public function testApiCreateSubscriptionTokenExists(): void {

tests/phpunit/api/Push/ApiEchoPushSubscriptionsDeleteTest.php

@@ -13,7 +13,10 @@ class ApiEchoPushSubscriptionsDeleteTest extends ApiTestCase {
 
 	public function setUp(): void {
 		parent::setUp();
-		$this->setMwGlobals( 'wgEchoEnablePush', true );
+		$this->setMwGlobals( [
+			'wgEchoEnablePush' => true,
+			'wgEchoPushMaxSubscriptionsPerUser' => 1
+		] );
 		$this->tablesUsed[] = 'echo_push_subscription';
 		$this->tablesUsed[] = 'echo_push_provider';
 		$this->user = $this->getTestUser()->getUser();

tests/phpunit/integration/Push/SubscriptionManagerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+use Wikimedia\TestingAccessWrapper;
+
 /**
  * @group Database
  * @covers \EchoPush\SubscriptionManager
@@ -10,18 +12,25 @@ class SubscriptionManagerTest extends MediaWikiIntegrationTestCase {
 		parent::setUp();
 		$this->tablesUsed[] = 'echo_push_subscription';
 		$this->tablesUsed[] = 'echo_push_provider';
+		$this->setMwGlobals( 'wgEchoPushMaxSubscriptionsPerUser', 1 );
 	}
 
 	public function testManagePushSubscriptions(): void {
-		$subscriptionManager = EchoServices::getInstance()->getPushSubscriptionManager();
+		$subscriptionManagerBase = EchoServices::getInstance()->getPushSubscriptionManager();
+		$subscriptionManager = TestingAccessWrapper::newFromObject( $subscriptionManagerBase );
+
 		$user = $this->getTestUser()->getUser();
 		$centralId = CentralIdLookup::factory()->centralIdFromLocalUser( $user );
+
 		$subscriptionManager->create( $user, 'test', 'ABC123' );
 		$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
 		$this->assertCount( 1, $subscriptions );
+		$this->assertTrue( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
+
 		$subscriptionManager->delete( $user, 'ABC123' );
 		$subscriptions = $subscriptionManager->getSubscriptionsForUser( $centralId );
 		$this->assertCount( 0, $subscriptions );
+		$this->assertFalse( $subscriptionManager->userHasMaxAllowedSubscriptions( $centralId ) );
 	}
 
 }