Merge "REST: Fix AbuseFilter error handling"

wikimedia · Sep 23, 2024 · 5957a07 · 5957a07
2 parents d70ac08 + 35d4d9a
commit 5957a07
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 10 deletions.
diff --git a/repo/rest-api/src/Domain/Services/Exceptions/AbuseFilterException.php b/repo/rest-api/src/Domain/Services/Exceptions/AbuseFilterException.php
@@ -10,11 +10,11 @@
  */
 class AbuseFilterException extends Exception {
 
-	private int $filterId;
+	private string $filterId;
 	private string $filterDescription;
 
 	public function __construct(
-		int $filterId,
+		string $filterId,
 		string $filterDescription,
 		string $message = '',
 		int $code = 0,
@@ -25,7 +25,7 @@ public function __construct(
 		$this->filterDescription = $filterDescription;
 	}
 
-	public function getFilterId(): int {
+	public function getFilterId(): string {
 		return $this->filterId;
 	}
 

diff --git a/repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php b/repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php
@@ -2,6 +2,7 @@
 
 namespace Wikibase\Repo\RestApi\Infrastructure\DataAccess;
 
+use IApiMessage;
 use LogicException;
 use MediaWiki\Context\IContextSource;
 use MediaWiki\Permissions\PermissionManager;
@@ -107,10 +108,12 @@ private function createOrUpdate(
 				throw new ResourceTooLargeException( $maxSizeInKiloBytes );
 			}
 
-			$abuseFilterError = $this->findErrorInStatus( $status, 'abusefilter' );
+			$abuseFilterError = $this->findAbuseFilterError( $status->getMessages() );
 			if ( $abuseFilterError ) {
-				[ $filterDescription, $filterId ] = $abuseFilterError->getParams();
-				throw new AbuseFilterException( (int)$filterId, $filterDescription );
+				throw new AbuseFilterException(
+					$abuseFilterError->getApiData()['abusefilter']['id'],
+					$abuseFilterError->getApiData()['abusefilter']['description']
+				);
 			}
 
 			if ( $this->isPreventedEdit( $status ) ) {
@@ -141,6 +144,17 @@ private function findErrorInStatus( Status $status, string $errorCode ): ?Messag
 		return null;
 	}
 
+	private function findAbuseFilterError( array $messages ): ?IApiMessage {
+		foreach ( $messages as $message ) {
+			if ( $message instanceof IApiMessage &&
+				in_array( $message->getApiCode(), [ 'abusefilter-warning', 'abusefilter-disallowed' ] ) ) {
+				return $message;
+			}
+		}
+
+		return null;
+	}
+
 	private function checkBotRightIfProvided( User $user, bool $isBot ): void {
 		// This is only a low-level safeguard and should be checked and handled properly before using this service.
 		if ( $isBot && !$this->permissionManager->userHasRight( $user, 'bot' ) ) {

diff --git a/repo/rest-api/tests/mocha/api-testing/AbuseFilterTest.js b/repo/rest-api/tests/mocha/api-testing/AbuseFilterTest.js
@@ -20,7 +20,7 @@ const config = require( 'api-testing/lib/config' )();
  *
  * @param {string} description
  * @param {string} rules
- * @return {Promise<number>} the filter ID
+ * @return {Promise<string>} the filter ID
  */
 async function createAbuseFilter( description, rules ) {
 	const rootClient = await action.root();
@@ -49,7 +49,7 @@ async function createAbuseFilter( description, rules ) {
 		wpFilterTags: ''
 	} );
 
-	return parseInt( new URL( createFilterResponse.headers.location ).searchParams.get( 'changedfilter' ) );
+	return new URL( createFilterResponse.headers.location ).searchParams.get( 'changedfilter' );
 }
 
 describe( 'Abuse Filter', () => {

diff --git a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php
@@ -230,10 +230,22 @@ public function testGivenResourceTooLarge_throwsCorrespondingException( EntityDo
 	 * @dataProvider provideEntity
 	 */
 	public function testGivenAbuseFilterMatch_throwsCorrespondingException( EntityDocument $entity ): void {
-		$filterId = 777;
+		$filterId = '777';
 		$filterDescription = 'bad word rejecting filter';
 
-		$errorStatus = EditEntityStatus::newFatal( 'abusefilter-disallowed', $filterDescription, $filterId );
+		$errorStatus = EditEntityStatus::newFatal(
+			\ApiMessage::create(
+				[ 'abusefilter-disallowed', $filterDescription, $filterId ],
+				'abusefilter-disallowed',
+				[
+					'abusefilter' => [
+						'id' => $filterId,
+						'description' => $filterDescription,
+						'actions' => 'disallow',
+					],
+				]
+			)
+		);
 
 		$editEntity = $this->createStub( EditEntity::class );
 		$editEntity->method( 'attemptSave' )->willReturn( $errorStatus );