Skip to content

Commit 26c09cc

Browse files
jenkins-botGerrit Code Review
jenkins-bot
authored and
Gerrit Code Review
committed
Merge "Search: Validate pagination query params for SimpleItemSearch"
2 parents 391eb4f + ccefd78 commit 26c09cc

File tree

6 files changed

+119
-21
lines changed

6 files changed

+119
-21
lines changed

‎repo/domains/search/specs/global/responses.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
"invalid-query-parameter": {
1818
"value": {
1919
"code": "invalid-query-parameter",
20-
"message": "Invalid query parameter: 'language'",
21-
"context": { "parameter": "language" }
20+
"message": "Invalid query parameter: '{query_parameter}'",
21+
"context": { "parameter": "{query_parameter}" }
2222
}
2323
}
2424
}

‎repo/domains/search/src/Application/UseCases/SimpleItemSearch/SimpleItemSearchValidator.php‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
class SimpleItemSearchValidator {
1313

1414
public const LANGUAGE_QUERY_PARAM = 'language';
15+
public const LIMIT_QUERY_PARAM = 'limit';
16+
public const OFFSET_QUERY_PARAM = 'offset';
17+
18+
private const MAX_LIMIT = 500;
1519

1620
private SearchLanguageValidator $languageValidator;
1721

@@ -30,14 +34,25 @@ public function validate( SimpleItemSearchRequest $request ): void {
3034
if ( $validationError ) {
3135
switch ( $validationError->getCode() ) {
3236
case SearchLanguageValidator::CODE_INVALID_LANGUAGE_CODE:
33-
throw new UseCaseError(
34-
UseCaseError::INVALID_QUERY_PARAMETER,
35-
"Invalid query parameter: '" . self::LANGUAGE_QUERY_PARAM . "'",
36-
[ UseCaseError::CONTEXT_PARAMETER => self::LANGUAGE_QUERY_PARAM ]
37-
);
37+
throw UseCaseError::invalidQueryParameter( self::LANGUAGE_QUERY_PARAM );
3838
default:
3939
throw new LogicException( 'unknown validation error code ' . $validationError->getCode() );
4040
}
4141
}
42+
43+
$this->validateLimitAndOffset( $request );
44+
}
45+
46+
private function validateLimitAndOffset( SimpleItemSearchRequest $request ): void {
47+
$limit = $request->getLimit();
48+
$offset = $request->getOffset();
49+
50+
if ( $limit < 0 || $limit > self::MAX_LIMIT ) {
51+
throw UseCaseError::invalidQueryParameter( self::LIMIT_QUERY_PARAM );
52+
}
53+
54+
if ( $offset < 0 ) {
55+
throw UseCaseError::invalidQueryParameter( self::OFFSET_QUERY_PARAM );
56+
}
4257
}
4358
}

‎repo/domains/search/src/Application/UseCases/UseCaseError.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ public function __construct( string $code, string $message, array $context = []
4646
}
4747
}
4848

49+
public static function invalidQueryParameter( string $parameter ): self {
50+
return new self(
51+
self::INVALID_QUERY_PARAMETER,
52+
"Invalid query parameter: '$parameter'",
53+
[ self::CONTEXT_PARAMETER => $parameter ]
54+
);
55+
}
56+
4957
public function getErrorCode(): string {
5058
return $this->errorCode;
5159
}

‎repo/domains/search/tests/mocha/api-testing/SimpleItemSearchTest.js‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ function newSearchRequest( language, searchTerm ) {
1818
.withQueryParam( 'q', searchTerm );
1919
}
2020

21+
function assertValidError( response, statusCode, responseBodyErrorCode, context ) {
22+
expect( response ).to.have.status( statusCode );
23+
assert.header( response, 'Content-Language', 'en' );
24+
assert.strictEqual( response.body.code, responseBodyErrorCode );
25+
assert.deepStrictEqual( response.body.context, context );
26+
}
27+
2128
describe( 'Simple item search', () => {
2229
let item1;
2330
let itemWithoutDescription;
@@ -193,11 +200,7 @@ describe( 'Simple item search', () => {
193200
.assertInvalidRequest()
194201
.makeRequest();
195202

196-
expect( response ).to.have.status( 400 );
197-
198-
assert.header( response, 'Content-Language', 'en' );
199-
assert.strictEqual( response.body.code, 'invalid-query-parameter' );
200-
assert.deepStrictEqual( response.body.context, { parameter: 'language' } );
203+
assertValidError( response, 400, 'invalid-query-parameter', { parameter: 'language' } );
201204
} );
202205

203206
it( 'User-Agent empty', async () => {
@@ -208,6 +211,29 @@ describe( 'Simple item search', () => {
208211
expect( response ).to.have.status( 400 );
209212
assert.strictEqual( response.body.code, 'missing-user-agent' );
210213
} );
211-
} );
212214

215+
Object.entries( {
216+
'invalid limit parameter - exceeds max limit 500': {
217+
parameter: 'limit',
218+
value: 501
219+
},
220+
'invalid limit parameter - negative limit': {
221+
parameter: 'limit',
222+
value: -1
223+
},
224+
'invalid offset parameter - negative offset': {
225+
parameter: 'offset',
226+
value: -2
227+
}
228+
} ).forEach( ( [ title, { parameter, value } ] ) => {
229+
it( title, async () => {
230+
231+
const response = await newSearchRequest( 'en', 'search term' )
232+
.withQueryParam( parameter, value )
233+
.makeRequest();
234+
235+
assertValidError( response, 400, 'invalid-query-parameter', { parameter } );
236+
} );
237+
} );
238+
} );
213239
} );

‎repo/domains/search/tests/phpunit/Application/UseCases/SimpleItemSearch/SimpleItemSearchValidatorTest.php‎

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

33
namespace Wikibase\Repo\Tests\Domains\Search\Application\UseCases\SimpleItemSearch;
44

5+
use Generator;
56
use MediaWiki\MediaWikiServices;
67
use PHPUnit\Framework\TestCase;
78
use Wikibase\Repo\Domains\Search\Application\UseCases\SimpleItemSearch\SimpleItemSearchRequest;
@@ -24,18 +25,29 @@
2425
*/
2526
class SimpleItemSearchValidatorTest extends TestCase {
2627

28+
private const DEFAULT_LIMIT = 10;
29+
private const DEFAULT_OFFSET = 0;
30+
2731
/**
2832
* @doesNotPerformAssertions
2933
*/
3034
public function testValidate_passes(): void {
3135
$this->newUseCaseValidator()
32-
->validate( new SimpleItemSearchRequest( 'search term', 'en', 10, 0 ) );
36+
->validate( new SimpleItemSearchRequest( 'search term', 'en', self::DEFAULT_LIMIT, self::DEFAULT_OFFSET ) );
37+
}
38+
39+
/**
40+
* @doesNotPerformAssertions
41+
*/
42+
public function testValidateWithoutLimitAndOffsetParams_passe(): void {
43+
$this->newUseCaseValidator()
44+
->validate( new SimpleItemSearchRequest( 'search term', 'en', null, null ) );
3345
}
3446

3547
public function testGivenInvalidLanguageCode_throws(): void {
3648
try {
3749
$this->newUseCaseValidator()
38-
->validate( new SimpleItemSearchRequest( 'search term', 'xyz', 10, 0 ) );
50+
->validate( new SimpleItemSearchRequest( 'search term', 'xyz', self::DEFAULT_LIMIT, self::DEFAULT_OFFSET ) );
3951

4052
$this->fail( 'Expected exception was not thrown' );
4153
} catch ( UseCaseError $e ) {
@@ -48,6 +60,43 @@ public function testGivenInvalidLanguageCode_throws(): void {
4860
}
4961
}
5062

63+
/**
64+
* @dataProvider provideInvalidLimitAndOffset
65+
*/
66+
public function testGivenInvalidLimitAndOffset_throws(
67+
UseCaseError $expectedError,
68+
int $limit,
69+
int $offset
70+
): void {
71+
try {
72+
$this->newUseCaseValidator()
73+
->validate( new SimpleItemSearchRequest( 'search term', 'en', $limit, $offset ) );
74+
$this->fail( 'Expected exception was not thrown' );
75+
} catch ( UseCaseError $e ) {
76+
$this->assertEquals( $expectedError, $e );
77+
}
78+
}
79+
80+
public static function provideInvalidLimitAndOffset(): Generator {
81+
yield 'invalid limit - negative limit' => [
82+
UseCaseError::invalidQueryParameter( 'limit' ),
83+
-1,
84+
self::DEFAULT_OFFSET,
85+
];
86+
87+
yield 'invalid limit - limit exceeds max (500)' => [
88+
UseCaseError::invalidQueryParameter( 'limit' ),
89+
501,
90+
self::DEFAULT_OFFSET,
91+
];
92+
93+
yield 'invalid offset - negative offset' => [
94+
UseCaseError::invalidQueryParameter( 'offset' ),
95+
self::DEFAULT_LIMIT,
96+
-2,
97+
];
98+
}
99+
51100
private function newUseCaseValidator(): SimpleItemSearchValidator {
52101
return new SimpleItemSearchValidator( $this->newSearchLanguageValidator() );
53102
}

‎repo/rest-api/src/openapi.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33800,9 +33800,9 @@
3380033800
"invalid-query-parameter": {
3380133801
"value": {
3380233802
"code": "invalid-query-parameter",
33803-
"message": "Invalid query parameter: 'language'",
33803+
"message": "Invalid query parameter: '{query_parameter}'",
3380433804
"context": {
33805-
"parameter": "language"
33805+
"parameter": "{query_parameter}"
3380633806
}
3380733807
}
3380833808
}
@@ -34034,9 +34034,9 @@
3403434034
"invalid-query-parameter": {
3403534035
"value": {
3403634036
"code": "invalid-query-parameter",
34037-
"message": "Invalid query parameter: 'language'",
34037+
"message": "Invalid query parameter: '{query_parameter}'",
3403834038
"context": {
34039-
"parameter": "language"
34039+
"parameter": "{query_parameter}"
3404034040
}
3404134041
}
3404234042
}
@@ -43247,9 +43247,9 @@
4324743247
"invalid-query-parameter": {
4324843248
"value": {
4324943249
"code": "invalid-query-parameter",
43250-
"message": "Invalid query parameter: 'language'",
43250+
"message": "Invalid query parameter: '{query_parameter}'",
4325143251
"context": {
43252-
"parameter": "language"
43252+
"parameter": "{query_parameter}"
4325343253
}
4325443254
}
4325543255
}

0 commit comments

Comments
 (0)