Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/III-4855 Enforce max limit on /labels endpoint #2007

Merged
merged 6 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion features/label/get.feature
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,24 @@ Feature: Test the UDB3 labels API
@bugfix # https://jira.uitdatabank.be/browse/III-5006
Scenario: Search labels with offset beyond result window with at least one result
When I send a GET request to "/labels/" with parameters:
| start | 999999999999999999999999999999 |
| start | 9223372036854775807 |
| query | special |
Then the response status should be "200"
And the response body should be valid JSON
And the JSON response at "member" should be:
"""
[]
"""

@bugfix # https://jira.publiq.be/browse/III-4855
Scenario: Search labels without offset and limit
When I send a GET request to "/labels/" with parameters:
| query | special |
Then the response status should be "200"
And the response body should be valid JSON
And the JSON response at "itemsPerPage" should be 30
And the JSON response at "totalItems" should be 4
And the JSON response at "member/0/name" should be "special_label"
And the JSON response at "member/1/name" should be "special_label*"
And the JSON response at "member/2/name" should be "special_label#"
And the JSON response at "member/3/name" should be "special-label"
8 changes: 6 additions & 2 deletions src/Http/Label/Query/QueryFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

class QueryFactory implements QueryFactoryInterface
{
public const MAX_LIMIT = 30;
public const QUERY = 'query';
public const START = 'start';
public const LIMIT = 'limit';
Expand All @@ -31,9 +32,12 @@ public function createFromRequest(ServerRequestInterface $request): Query

$userId = $this->userId ?: null;

$offset = (int) $queryParameters->get(self::START);
$offset = $queryParameters->getAsInt(self::START, 0);

$limit = (int) $queryParameters->get(self::LIMIT);
$limit = min(
$queryParameters->getAsInt(self::LIMIT, self::MAX_LIMIT),
self::MAX_LIMIT
);

$suggestion = filter_var($queryParameters->get(self::SUGGESTION), FILTER_VALIDATE_BOOLEAN);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

final class DBALReadRepository extends AbstractDBALRepository implements ReadRepositoryInterface
{
private const MAX_RESULTS = 30;

private string $labelRolesTableName;

private string $userRolesTableName;
Expand Down Expand Up @@ -124,8 +122,7 @@ public function search(Query $query): array
->setFirstResult($query->getOffset());
}

$queryBuilder
->setMaxResults($query->getLimit() ?? self::MAX_RESULTS);
$queryBuilder->setMaxResults($query->getLimit());

return $this->getResults($queryBuilder);
}
Expand Down
39 changes: 35 additions & 4 deletions tests/Http/Label/Query/QueryFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function it_can_get_query_from_request_no_start(): void
$expectedQuery = new Query(
self::QUERY_VALUE,
self::USER_ID_VALUE,
null,
0,
self::LIMIT_VALUE
);

Expand Down Expand Up @@ -105,7 +105,7 @@ public function it_can_get_query_from_request_no_limit(): void
self::QUERY_VALUE,
self::USER_ID_VALUE,
self::START_VALUE,
null
QueryFactory::MAX_LIMIT,
);

$this->assertEquals($expectedQuery, $query);
Expand All @@ -131,8 +131,8 @@ public function it_can_get_query_from_request_no_start_and_no_limit(): void
$expectedQuery = new Query(
self::QUERY_VALUE,
self::USER_ID_VALUE,
null,
null
0,
QueryFactory::MAX_LIMIT,
);

$this->assertEquals($expectedQuery, $query);
Expand Down Expand Up @@ -169,6 +169,37 @@ public function it_can_get_query_from_request_with_zero_start_and_zero_limit():
$this->assertEquals($expectedQuery, $query);
}

/**
* @test
*/
public function it_enforces_a_maximum_limit(): void
{
$request = (new Psr7RequestBuilder())
->withUriFromString(
sprintf(
'/?%s=%s&%s=%s&%s=%s',
QueryFactory::QUERY,
self::QUERY_VALUE,
QueryFactory::START,
0,
QueryFactory::LIMIT,
9223372036854775807,
)
)
->build('GET');

$query = $this->queryFactory->createFromRequest($request);

$expectedQuery = new Query(
self::QUERY_VALUE,
self::USER_ID_VALUE,
0,
30,
);

$this->assertEquals($expectedQuery, $query);
}

/**
* @test
*/
Expand Down
62 changes: 62 additions & 0 deletions tests/Http/Label/SearchLabelsRequestHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,66 @@ public function it_returns_an_empty_collection_when_no_labels_are_found(): void
$response,
);
}

/**
* @test https://jira.publiq.be/browse/III-4855
*/
public function it_uses_default_start_and_limit_when_not_provided(): void
{
$request = (new Psr7RequestBuilder())
->withUriFromString('labels')
->build('GET');

$this->labelRepository->expects($this->once())
->method('searchTotalLabels')
->with(new Query('', '123', 0, 30))
->willReturn(count($this->labels));

$this->labelRepository->expects($this->once())
->method('search')
->with(new Query('', '123', 0, 30))
->willReturn($this->labels);

$response = $this->searchLabelsRequestHandler->handle($request);

$this->assertJsonResponse(
new PagedCollectionResponse(
30,
2,
$this->labels
),
$response,
);
}

/**
* @test https://jira.publiq.be/browse/III-4855
*/
public function it_enforces_a_maximum_number_of_items_returned(): void
{
$request = (new Psr7RequestBuilder())
->withUriFromString('labels?limit=9223372036854775807')
->build('GET');

$this->labelRepository->expects($this->once())
->method('searchTotalLabels')
->with(new Query('', '123', 0, 30))
->willReturn(count($this->labels));

$this->labelRepository->expects($this->once())
->method('search')
->with(new Query('', '123', 0, 30))
->willReturn($this->labels);

$response = $this->searchLabelsRequestHandler->handle($request);

$this->assertJsonResponse(
new PagedCollectionResponse(
30,
2,
$this->labels
),
$response,
);
}
}