Skip to content

Commit 5957a07

Browse files
jenkins-botGerrit Code Review
authored andcommitted
Merge "REST: Fix AbuseFilter error handling"
2 parents d70ac08 + 35d4d9a commit 5957a07

File tree

4 files changed

+36
-10
lines changed

4 files changed

+36
-10
lines changed

repo/rest-api/src/Domain/Services/Exceptions/AbuseFilterException.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@
1010
*/
1111
class AbuseFilterException extends Exception {
1212

13-
private int $filterId;
13+
private string $filterId;
1414
private string $filterDescription;
1515

1616
public function __construct(
17-
int $filterId,
17+
string $filterId,
1818
string $filterDescription,
1919
string $message = '',
2020
int $code = 0,
@@ -25,7 +25,7 @@ public function __construct(
2525
$this->filterDescription = $filterDescription;
2626
}
2727

28-
public function getFilterId(): int {
28+
public function getFilterId(): string {
2929
return $this->filterId;
3030
}
3131

repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Wikibase\Repo\RestApi\Infrastructure\DataAccess;
44

5+
use IApiMessage;
56
use LogicException;
67
use MediaWiki\Context\IContextSource;
78
use MediaWiki\Permissions\PermissionManager;
@@ -107,10 +108,12 @@ private function createOrUpdate(
107108
throw new ResourceTooLargeException( $maxSizeInKiloBytes );
108109
}
109110

110-
$abuseFilterError = $this->findErrorInStatus( $status, 'abusefilter' );
111+
$abuseFilterError = $this->findAbuseFilterError( $status->getMessages() );
111112
if ( $abuseFilterError ) {
112-
[ $filterDescription, $filterId ] = $abuseFilterError->getParams();
113-
throw new AbuseFilterException( (int)$filterId, $filterDescription );
113+
throw new AbuseFilterException(
114+
$abuseFilterError->getApiData()['abusefilter']['id'],
115+
$abuseFilterError->getApiData()['abusefilter']['description']
116+
);
114117
}
115118

116119
if ( $this->isPreventedEdit( $status ) ) {
@@ -141,6 +144,17 @@ private function findErrorInStatus( Status $status, string $errorCode ): ?Messag
141144
return null;
142145
}
143146

147+
private function findAbuseFilterError( array $messages ): ?IApiMessage {
148+
foreach ( $messages as $message ) {
149+
if ( $message instanceof IApiMessage &&
150+
in_array( $message->getApiCode(), [ 'abusefilter-warning', 'abusefilter-disallowed' ] ) ) {
151+
return $message;
152+
}
153+
}
154+
155+
return null;
156+
}
157+
144158
private function checkBotRightIfProvided( User $user, bool $isBot ): void {
145159
// This is only a low-level safeguard and should be checked and handled properly before using this service.
146160
if ( $isBot && !$this->permissionManager->userHasRight( $user, 'bot' ) ) {

repo/rest-api/tests/mocha/api-testing/AbuseFilterTest.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const config = require( 'api-testing/lib/config' )();
2020
*
2121
* @param {string} description
2222
* @param {string} rules
23-
* @return {Promise<number>} the filter ID
23+
* @return {Promise<string>} the filter ID
2424
*/
2525
async function createAbuseFilter( description, rules ) {
2626
const rootClient = await action.root();
@@ -49,7 +49,7 @@ async function createAbuseFilter( description, rules ) {
4949
wpFilterTags: ''
5050
} );
5151

52-
return parseInt( new URL( createFilterResponse.headers.location ).searchParams.get( 'changedfilter' ) );
52+
return new URL( createFilterResponse.headers.location ).searchParams.get( 'changedfilter' );
5353
}
5454

5555
describe( 'Abuse Filter', () => {

repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,22 @@ public function testGivenResourceTooLarge_throwsCorrespondingException( EntityDo
230230
* @dataProvider provideEntity
231231
*/
232232
public function testGivenAbuseFilterMatch_throwsCorrespondingException( EntityDocument $entity ): void {
233-
$filterId = 777;
233+
$filterId = '777';
234234
$filterDescription = 'bad word rejecting filter';
235235

236-
$errorStatus = EditEntityStatus::newFatal( 'abusefilter-disallowed', $filterDescription, $filterId );
236+
$errorStatus = EditEntityStatus::newFatal(
237+
\ApiMessage::create(
238+
[ 'abusefilter-disallowed', $filterDescription, $filterId ],
239+
'abusefilter-disallowed',
240+
[
241+
'abusefilter' => [
242+
'id' => $filterId,
243+
'description' => $filterDescription,
244+
'actions' => 'disallow',
245+
],
246+
]
247+
)
248+
);
237249

238250
$editEntity = $this->createStub( EditEntity::class );
239251
$editEntity->method( 'attemptSave' )->willReturn( $errorStatus );

0 commit comments

Comments
 (0)