diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 1c1be04c88ded..1e0a61467e7cb 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -9,6 +9,7 @@ namespace OCA\Provisioning_API\Controller; +use OC\AppFramework\Http\PaginationTrait; use OC\Group\DisplayNameCache as GroupDisplayNameCache; use OCA\Provisioning_API\ResponseDefinitions; use OCA\Settings\Settings\Admin\Sharing; @@ -30,6 +31,7 @@ use OCP\IGroup; use OCP\IGroupManager; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -42,6 +44,7 @@ * @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions */ class GroupsController extends AUserDataOCSController { + use PaginationTrait; public function __construct( string $appName, @@ -56,6 +59,7 @@ public function __construct( IRootFolder $rootFolder, private LoggerInterface $logger, GroupDisplayNameCache $groupDisplayNameCache, + private readonly IURLGenerator $urlGenerator, ) { parent::__construct($appName, $request, @@ -77,7 +81,7 @@ public function __construct( * @param string $search Text to search for * @param ?int $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups - * @return DataResponse}, array{}> + * @return DataResponse}, array{Link?: string}> * * 200: Groups returned */ @@ -89,7 +93,10 @@ public function getGroups(string $search = '', ?int $limit = null, int $offset = return $group->getGID(); }, $groups); - return new DataResponse(['groups' => $groups]); + $headers = $this->buildOffsetNextPageLinkHeader($groups, [ + 'search' => $search, + ], $limit, $offset); + return new DataResponse(['groups' => $groups], headers: $headers); } /** @@ -98,7 +105,7 @@ public function getGroups(string $search = '', ?int $limit = null, int $offset = * @param string $search Text to search for * @param ?int $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups - * @return DataResponse}, array{}> + * @return DataResponse}, array{Link?: string}> * * 200: Groups details returned */ @@ -119,7 +126,10 @@ public function getGroupsDetails(string $search = '', ?int $limit = null, int $o ]; }, $groups); - return new DataResponse(['groups' => $groups]); + $headers = $this->buildOffsetNextPageLinkHeader($groups, [ + 'search' => $search, + ], $limit, $offset); + return new DataResponse(['groups' => $groups], headers: $headers); } /** @@ -191,7 +201,7 @@ public function getGroupUsers(string $groupId): DataResponse { * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups * - * @return DataResponse, groups: list}, array{}> + * @return DataResponse, groups: list}, array{Link?: string}> * @throws OCSException * * 200: Group users details returned @@ -234,10 +244,13 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int // continue if a users ceased to exist. } } + $headers = $this->buildOffsetNextPageLinkHeader($users, [ + 'search' => $search, + ], $limit, $offset); return new DataResponse([ 'users' => $usersDetails, 'groups' => $this->findGroupsWithDisplayname($usersDetails), - ]); + ], headers: $headers); } throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND); diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index f46084a98741e..65ac66a18549c 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -11,6 +11,7 @@ namespace OCA\Provisioning_API\Controller; use InvalidArgumentException; +use OC\AppFramework\Http\PaginationTrait; use OC\Authentication\Token\RemoteWipe; use OC\Group\DisplayNameCache as GroupDisplayNameCache; use OC\Group\Group; @@ -62,6 +63,7 @@ * @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions */ class UsersController extends AUserDataOCSController { + use PaginationTrait; private IL10N $l10n; @@ -111,7 +113,7 @@ public function __construct( * @param string $search Text to search for * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups - * @return DataResponse}, array{}> + * @return DataResponse}, array{Link?: string}> * * 200: Users returned */ @@ -142,9 +144,12 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset = /** @var list $users */ $users = array_keys($users); + $headers = $this->buildOffsetNextPageLinkHeader($users, [ + 'search' => $search, + ], $limit, $offset); return new DataResponse([ 'users' => $users - ]); + ], headers: $headers); } /** @@ -153,7 +158,7 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset = * @param string $search Text to search for * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups - * @return DataResponse, groups: list}, array{}> + * @return DataResponse, groups: list}, array{Link?: string}> * * 200: Users details returned */ @@ -204,10 +209,13 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of } } + $headers = $this->buildOffsetNextPageLinkHeader($users, [ + 'search' => $search, + ], $limit, $offset); return new DataResponse([ 'users' => $usersDetails, 'groups' => $this->findGroupsWithDisplayname($usersDetails), - ]); + ], headers: $headers); } /** @@ -216,7 +224,7 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of * @param string $search Text to search for * @param ?int $limit Limit the amount of users returned * @param int $offset Offset - * @return DataResponse}, array{}> + * @return DataResponse}, array{Link?: string}> * * 200: Disabled users details returned */ @@ -287,9 +295,12 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null, } } + $headers = $this->buildOffsetNextPageLinkHeader($users, [ + 'search' => $search, + ], $limit, $offset); return new DataResponse([ 'users' => $usersDetails - ]); + ], headers: $headers); } /** @@ -298,7 +309,7 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null, * @param string $search Text to search for * @param ?int $limit Limit the amount of users returned * @param int $offset Offset - * @return DataResponse}, array{}> + * @return DataResponse}, array{Link?: string}> * * 200: Users details returned based on last logged in information */ @@ -344,9 +355,12 @@ public function getLastLoggedInUsers( } } + $headers = $this->buildOffsetNextPageLinkHeader($users, [ + 'search' => $search, + ], $limit, $offset); return new DataResponse([ 'users' => $usersDetails - ]); + ], headers: $headers); } /** diff --git a/apps/provisioning_api/openapi-administration.json b/apps/provisioning_api/openapi-administration.json index 52d5325ba6ffc..1a16cc7b53021 100644 --- a/apps/provisioning_api/openapi-administration.json +++ b/apps/provisioning_api/openapi-administration.json @@ -1492,6 +1492,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/openapi-full.json b/apps/provisioning_api/openapi-full.json index ebba71a0259dc..4303f86cd63cd 100644 --- a/apps/provisioning_api/openapi-full.json +++ b/apps/provisioning_api/openapi-full.json @@ -1277,6 +1277,13 @@ "responses": { "200": { "description": "Groups returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1791,6 +1798,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3218,6 +3232,13 @@ "responses": { "200": { "description": "Groups details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3524,6 +3545,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3676,6 +3704,13 @@ "responses": { "200": { "description": "Users returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3996,6 +4031,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -4148,6 +4190,13 @@ "responses": { "200": { "description": "Disabled users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/openapi.json b/apps/provisioning_api/openapi.json index 4550e551a02e6..b47957bcfdf10 100644 --- a/apps/provisioning_api/openapi.json +++ b/apps/provisioning_api/openapi.json @@ -488,6 +488,13 @@ "responses": { "200": { "description": "Groups returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -618,6 +625,13 @@ "responses": { "200": { "description": "Groups details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -924,6 +938,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1187,6 +1208,13 @@ "responses": { "200": { "description": "Users returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1507,6 +1535,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1659,6 +1694,13 @@ "responses": { "200": { "description": "Disabled users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 72fc57421e59c..7fee2d13e3376 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -18,6 +18,7 @@ use OCP\IConfig; use OCP\IGroup; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -39,6 +40,7 @@ class GroupsControllerTest extends \Test\TestCase { protected LoggerInterface&MockObject $logger; protected GroupsController&MockObject $api; private GroupDisplayNameCache&MockObject $groupDisplayNameCache; + private IURLGenerator&MockObject $urlGenerator; private IRootFolder $rootFolder; @@ -56,6 +58,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->rootFolder = $this->createMock(IRootFolder::class); $this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->groupManager ->method('getSubAdmin') @@ -75,6 +78,7 @@ protected function setUp(): void { $this->rootFolder, $this->logger, $this->groupDisplayNameCache, + $this->urlGenerator, ]) ->onlyMethods(['fillStorageInfo']) ->getMock(); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 08355a5cb1804..f4a4e0a340ae2 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -350,6 +350,42 @@ public function testGetDisabledUsersAsAdmin(): void { $this->assertEquals($expected, $this->api->getDisabledUsersDetails('MyCustomSearch', 3)->getData()); } + public function testGetDisabledUsersDetailsSetsLinkHeaderWhenMoreResultsExist(): void { + $loggedInUser = $this->getMockBuilder(IUser::class) + ->disableOriginalConstructor() + ->getMock(); + $loggedInUser + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->method('isAdmin') + ->willReturn(true); + $this->userManager + ->method('getDisabledUsers') + ->with(2, 0, 'MyCustomSearch') + ->willReturn([ + $this->createUserMock('foo', false), + $this->createUserMock('bar', false), + ]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/apps/provisioning_api/api/v1/users/disabled?search=MyCustomSearch&limit=2'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/apps/provisioning_api/api/v1/users/disabled') + ->willReturn('https://cloud.example.com/ocs/v2.php/apps/provisioning_api/api/v1/users/disabled'); + + $response = $this->api->getDisabledUsersDetails('MyCustomSearch', 2, 0); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + public function testGetDisabledUsersAsSubAdmin(): void { $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() diff --git a/apps/sharing/lib/Controller/ApiV1Controller.php b/apps/sharing/lib/Controller/ApiV1Controller.php index d56a653c14793..ca137614e20e8 100644 --- a/apps/sharing/lib/Controller/ApiV1Controller.php +++ b/apps/sharing/lib/Controller/ApiV1Controller.php @@ -28,6 +28,7 @@ use NCU\Sharing\ShareState; use NCU\Sharing\Source\IShareSourceType; use NCU\Sharing\Source\ShareSource; +use OC\AppFramework\Http\PaginationTrait; use OCA\Sharing\ResponseDefinitions; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; @@ -54,6 +55,8 @@ * @psalm-import-type SharingPermissionPreset from ResponseDefinitions */ final class ApiV1Controller extends OCSController { + use PaginationTrait; + public ShareAccessContext $accessContext; public function __construct( @@ -80,7 +83,7 @@ public function __construct( * @param int<1, 100> $limit The maximum number of participants * @param non-negative-int $offset The offset of the participants * @param ?string $id If provided, recipients that are already part of the share will not be returned. - * @return DataResponse, array{}>|DataResponse + * @return DataResponse, array{Link?: string}>|DataResponse * * 200: Recipients returned * 400: Invalid recipient search parameters @@ -110,7 +113,14 @@ public function searchRecipients(?array $filterRecipientTypeClasses, string $que $forShare = ($id === null) ? null : $this->manager->getShare($this->accessContext, $id); $recipients = $this->manager->searchRecipients($this->accessContext, $filterRecipientTypeClasses, $query, $limit, $offset, $forShare); $this->dbConnection->commit(); - return new DataResponse(ShareRecipient::formatMultiple($this->registry, $this->l10nFactory, $this->urlGenerator, $this->userManager, $recipients)); + + $headers = $this->buildOffsetNextPageLinkHeader($recipients, [ + 'filterRecipientTypeClasses' => $filterRecipientTypeClasses, + 'query' => $query, + 'id' => $id, + ], $limit, $offset); + + return new DataResponse(ShareRecipient::formatMultiple($this->registry, $this->l10nFactory, $this->urlGenerator, $this->userManager, $recipients), headers: $headers); } catch (Exception $exception) { $this->dbConnection->rollBack(); throw $exception; diff --git a/apps/sharing/openapi.json b/apps/sharing/openapi.json index 98beb6864c1d6..b98f56fb52c27 100644 --- a/apps/sharing/openapi.json +++ b/apps/sharing/openapi.json @@ -684,6 +684,13 @@ "responses": { "200": { "description": "Recipients returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/user_status/lib/Controller/StatusesController.php b/apps/user_status/lib/Controller/StatusesController.php index b1d55774a1177..d9520afde6a0b 100644 --- a/apps/user_status/lib/Controller/StatusesController.php +++ b/apps/user_status/lib/Controller/StatusesController.php @@ -9,6 +9,7 @@ namespace OCA\UserStatus\Controller; +use OC\AppFramework\Http\PaginationTrait; use OCA\UserStatus\Db\UserStatus; use OCA\UserStatus\ResponseDefinitions; use OCA\UserStatus\Service\StatusService; @@ -20,6 +21,7 @@ use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\UserStatus\IUserStatus; /** @@ -27,6 +29,7 @@ * @psalm-import-type UserStatusPublic from ResponseDefinitions */ class StatusesController extends OCSController { + use PaginationTrait; /** * StatusesController constructor. @@ -39,6 +42,7 @@ public function __construct( string $appName, IRequest $request, private StatusService $service, + private IURLGenerator $urlGenerator, ) { parent::__construct($appName, $request); } @@ -48,18 +52,32 @@ public function __construct( * * @param int|null $limit Maximum number of statuses to find * @param non-negative-int|null $offset Offset for finding statuses - * @return DataResponse, array{}> + * @param non-negative-int|null $lastId Id of the last status returned by the previous page; + * when given, keyset pagination is used and $offset is ignored + * @return DataResponse, array{Link?: string}> + * + * @note Prefer $lastId over $offset: it does not require the database to scan and discard + * every preceding row on every call. * * 200: Statuses returned */ #[NoAdminRequired] #[ApiRoute(verb: 'GET', url: '/api/v1/statuses')] - public function findAll(?int $limit = null, ?int $offset = null): DataResponse { - $allStatuses = $this->service->findAll($limit, $offset); - + public function findAll(?int $limit = null, ?int $offset = null, ?int $lastId = null): DataResponse { + if ($lastId !== null) { + $allStatuses = $this->service->findAllAfterId($limit, $lastId); + } else { + $allStatuses = $this->service->findAll($limit, $offset); + } + if ($lastId !== null) { + $lastStatus = end($allStatuses); + $headers = $this->buildCursorNextPageLinkHeader($allStatuses, [], $limit, $lastStatus !== false ? $lastStatus->getId() : null); + } else { + $headers = $this->buildOffsetNextPageLinkHeader($allStatuses, [], $limit, $offset ?? 0); + } return new DataResponse(array_values(array_map(function ($userStatus) { return $this->formatStatus($userStatus); - }, $allStatuses))); + }, $allStatuses)), headers: $headers); } /** diff --git a/apps/user_status/lib/Db/UserStatusMapper.php b/apps/user_status/lib/Db/UserStatusMapper.php index ea3b76c8e564d..3c6bd683393e2 100644 --- a/apps/user_status/lib/Db/UserStatusMapper.php +++ b/apps/user_status/lib/Db/UserStatusMapper.php @@ -49,6 +49,34 @@ public function findAll(?int $limit = null, ?int $offset = null):array { return $this->findEntities($qb); } + /** + * Finds statuses one page at a time ordered by id, using keyset (seek) pagination + * instead of OFFSET/LIMIT, so each page costs the same regardless of how deep it is. + * + * @param int|null $limit + * @param int|null $lastId Id of the last status from the previous page, or null to fetch the first page + * @return UserStatus[] + */ + public function findAllAfterId(?int $limit = null, ?int $lastId = null): array { + $qb = $this->db->getQueryBuilder(); + $qb + ->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('is_backup', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))); + + if ($lastId !== null) { + $qb->andWhere($qb->expr()->gt('id', $qb->createNamedParameter($lastId, IQueryBuilder::PARAM_INT))); + } + + $qb->orderBy('id', 'ASC'); + + if ($limit !== null) { + $qb->setMaxResults($limit); + } + + return $this->findEntities($qb); + } + /** * @param int|null $limit * @param int|null $offset diff --git a/apps/user_status/lib/Service/StatusService.php b/apps/user_status/lib/Service/StatusService.php index 6c1805963c881..5975a68759ff4 100644 --- a/apps/user_status/lib/Service/StatusService.php +++ b/apps/user_status/lib/Service/StatusService.php @@ -97,6 +97,24 @@ public function findAll(?int $limit = null, ?int $offset = null): array { }, $this->mapper->findAll($limit, $offset)); } + /** + * @param int|null $limit + * @param int|null $lastId Id of the last status from the previous page, or null to fetch the first page + * @return UserStatus[] + */ + public function findAllAfterId(?int $limit = null, ?int $lastId = null): array { + // Return empty array if user enumeration is disabled or limited to groups + // TODO: find a solution that scales to get only users from common groups if user enumeration is limited to + // groups. See discussion at https://github.com/nextcloud/server/pull/27879#discussion_r729715936 + if (!$this->shareeEnumeration || $this->shareeEnumerationInGroupOnly || $this->shareeEnumerationPhone) { + return []; + } + + return array_map(function ($status) { + return $this->processStatus($status); + }, $this->mapper->findAllAfterId($limit, $lastId)); + } + /** * @param int|null $limit * @param int|null $offset diff --git a/apps/user_status/openapi.json b/apps/user_status/openapi.json index 720d695c3ddf7..8a023e44b0e46 100644 --- a/apps/user_status/openapi.json +++ b/apps/user_status/openapi.json @@ -472,6 +472,7 @@ "get": { "operationId": "statuses-find-all", "summary": "Find statuses of users", + "description": "Prefer $lastId over $offset: it does not require the database to scan and discard every preceding row on every call.", "tags": [ "statuses" ], @@ -507,6 +508,18 @@ "minimum": 0 } }, + { + "name": "lastId", + "in": "query", + "description": "Id of the last status returned by the previous page; when given, keyset pagination is used and $offset is ignored", + "schema": { + "type": "integer", + "format": "int64", + "nullable": true, + "default": null, + "minimum": 0 + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -521,6 +534,13 @@ "responses": { "200": { "description": "Statuses returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/user_status/tests/Unit/Controller/StatusesControllerTest.php b/apps/user_status/tests/Unit/Controller/StatusesControllerTest.php index e623486c82d93..6beee8569a96f 100644 --- a/apps/user_status/tests/Unit/Controller/StatusesControllerTest.php +++ b/apps/user_status/tests/Unit/Controller/StatusesControllerTest.php @@ -15,20 +15,24 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\IRequest; +use OCP\IURLGenerator; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class StatusesControllerTest extends TestCase { private StatusService&MockObject $service; + private IRequest&MockObject $request; + private IURLGenerator&MockObject $urlGenerator; private StatusesController $controller; protected function setUp(): void { parent::setUp(); - $request = $this->createMock(IRequest::class); + $this->request = $this->createMock(IRequest::class); $this->service = $this->createMock(StatusService::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); - $this->controller = new StatusesController('user_status', $request, $this->service); + $this->controller = new StatusesController('user_status', $this->request, $this->service, $this->urlGenerator); } public function testFindAll(): void { @@ -47,6 +51,49 @@ public function testFindAll(): void { 'message' => 'On vacation', 'clearAt' => 60000, ]], $response->getData()); + $this->assertArrayNotHasKey('Link', $response->getHeaders()); + } + + public function testFindAllWithLastId(): void { + $userStatus = $this->getUserStatus(); + + $this->service->expects($this->once()) + ->method('findAllAfterId') + ->with(20, 1336) + ->willReturn([$userStatus]); + $this->service->expects($this->never()) + ->method('findAll'); + + $response = $this->controller->findAll(20, null, 1336); + $this->assertEquals([[ + 'userId' => 'john.doe', + 'status' => 'offline', + 'icon' => '🏝', + 'message' => 'On vacation', + 'clearAt' => 60000, + ]], $response->getData()); + $this->assertArrayNotHasKey('Link', $response->getHeaders()); + } + + public function testFindAllWithLastIdHasMoreResults(): void { + $userStatus = $this->getUserStatus(); + + $this->service->expects($this->once()) + ->method('findAllAfterId') + ->with(1, 1336) + ->willReturn([$userStatus]); + + $this->request->method('getRequestUri') + ->willReturn('/ocs/v2.php/apps/user_status/api/v1/statuses?limit=1&lastId=1336'); + $this->urlGenerator->method('getAbsoluteURL') + ->with('/ocs/v2.php/apps/user_status/api/v1/statuses') + ->willReturn('https://cloud.example.com/ocs/v2.php/apps/user_status/api/v1/statuses'); + + $response = $this->controller->findAll(1, null, 1336); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); } public function testFind(): void { diff --git a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php index a697571bd5709..73466adad4f34 100644 --- a/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php +++ b/apps/user_status/tests/Unit/Db/UserStatusMapperTest.php @@ -48,6 +48,19 @@ public function testGetFindAll(): void { $this->assertEquals('user2', $offsetResults[0]->getUserId()); } + public function testGetFindAllAfterId(): void { + $this->insertSampleStatuses(); + + $firstPage = $this->mapper->findAllAfterId(2); + $this->assertCount(2, $firstPage); + $this->assertEquals('admin', $firstPage[0]->getUserId()); + $this->assertEquals('user1', $firstPage[1]->getUserId()); + + $secondPage = $this->mapper->findAllAfterId(2, $firstPage[1]->getId()); + $this->assertCount(1, $secondPage); + $this->assertEquals('user2', $secondPage[0]->getUserId()); + } + public function testFindAllExcludesBackups(): void { $this->insertSampleStatuses(); diff --git a/apps/user_status/tests/Unit/Service/StatusServiceTest.php b/apps/user_status/tests/Unit/Service/StatusServiceTest.php index 69128033631e0..bb802e11622b5 100644 --- a/apps/user_status/tests/Unit/Service/StatusServiceTest.php +++ b/apps/user_status/tests/Unit/Service/StatusServiceTest.php @@ -82,6 +82,21 @@ public function testFindAll(): void { ], $this->service->findAll(20, 50)); } + public function testFindAllAfterId(): void { + $status1 = $this->createMock(UserStatus::class); + $status2 = $this->createMock(UserStatus::class); + + $this->mapper->expects($this->once()) + ->method('findAllAfterId') + ->with(20, 1336) + ->willReturn([$status1, $status2]); + + $this->assertEquals([ + $status1, + $status2, + ], $this->service->findAllAfterId(20, 1336)); + } + public function testFindAllRecentStatusChanges(): void { $status1 = $this->createMock(UserStatus::class); $status2 = $this->createMock(UserStatus::class); diff --git a/core/Controller/AppPasswordController.php b/core/Controller/AppPasswordController.php index 5dcf9b8c4e72c..c7d94085294c8 100644 --- a/core/Controller/AppPasswordController.php +++ b/core/Controller/AppPasswordController.php @@ -171,9 +171,9 @@ public function rotateAppPassword(): DataResponse { /** * Confirm the user password * - * @param string $password The password of the user + * @param non-empty-string $password The password of the user * - * @return DataResponse|DataResponse, array{}> + * @return DataResponse|DataResponse, array{}> * * 200: Password confirmation succeeded * 403: Password confirmation failed @@ -200,7 +200,7 @@ public function confirmUserPassword(string $password): DataResponse { /** * Get app password with one-time password * - * @return DataResponse + * @return DataResponse * @throws OCSForbiddenException Creating app password is not allowed * * 200: App password returned diff --git a/core/Controller/AutoCompleteController.php b/core/Controller/AutoCompleteController.php index 786996c694fce..eb140b7b3587c 100644 --- a/core/Controller/AutoCompleteController.php +++ b/core/Controller/AutoCompleteController.php @@ -9,6 +9,7 @@ namespace OC\Core\Controller; +use OC\AppFramework\Http\PaginationTrait; use OC\Core\ResponseDefinitions; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; @@ -20,18 +21,22 @@ use OCP\Collaboration\Collaborators\ISearch; use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\Share\IShare; /** * @psalm-import-type CoreAutocompleteResult from ResponseDefinitions */ class AutoCompleteController extends OCSController { + use PaginationTrait; + public function __construct( string $appName, IRequest $request, private ISearch $collaboratorSearch, private IManager $autoCompleteManager, private IEventDispatcher $dispatcher, + private IURLGenerator $urlGenerator, ) { parent::__construct($appName, $request); } @@ -43,19 +48,20 @@ public function __construct( * @param string|null $itemType Type of the items to search for * @param string|null $itemId ID of the items to search for * @param string|null $sorter can be piped, top priority first, e.g.: "commenters|share-recipients" - * @param list $shareTypes Types of shares to search for - * @param int $limit Maximum number of results to return + * @param list $shareTypes Types of shares to search for + * @param positive-int $limit Maximum number of results to return + * @param non-negative-int $offset Offset for searching * - * @return DataResponse, array{}> + * @return DataResponse, array{Link?: string}> * * 200: Autocomplete results returned */ #[NoAdminRequired] #[ApiRoute(verb: 'GET', url: '/autocomplete/get', root: '/core')] - public function get(string $search, ?string $itemType, ?string $itemId, ?string $sorter = null, array $shareTypes = [IShare::TYPE_USER], int $limit = 10): DataResponse { + public function get(string $search, ?string $itemType, ?string $itemId, ?string $sorter = null, array $shareTypes = [IShare::TYPE_USER], int $limit = 10, int $offset = 0): DataResponse { // if enumeration/user listings are disabled, we'll receive an empty // result from search() – thus nothing else to do here. - [$results,] = $this->collaboratorSearch->search($search, $shareTypes, false, $limit, 0); + [$results, $hasMoreResults] = $this->collaboratorSearch->search($search, $shareTypes, false, $limit, $offset); $event = new AutoCompleteFilterEvent( $results, @@ -84,7 +90,17 @@ public function get(string $search, ?string $itemType, ?string $itemId, ?string // transform to expected format $results = $this->prepareResultArray($results); - return new DataResponse($results); + // $hasMoreResults comes from the search backend, not from the (differently shaped, + // possibly filtered) $results, so it is passed through as-is rather than re-derived. + $headers = $this->buildOffsetNextPageLinkHeader($hasMoreResults, [ + 'search' => $search, + 'itemType' => $itemType, + 'itemId' => $itemId, + 'sorter' => $sorter, + 'shareTypes' => $shareTypes, + ], $limit, $offset); + + return new DataResponse($results, headers: $headers); } /** diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index c232dd140fc37..2a9a68bc3e302 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -54,7 +54,7 @@ public function __construct( /** * Get the dark avatar * - * @param string $userId ID of the user + * @param non-empty-string $userId ID of the user * @param 64|512 $size Size of the avatar * @param bool $guestFallback Fallback to guest avatar if not found * @return FileDisplayResponse|JSONResponse, array{}>|Response @@ -104,7 +104,7 @@ public function getAvatarDark(string $userId, int $size, bool $guestFallback = f /** * Get the avatar * - * @param string $userId ID of the user + * @param non-empty-string $userId ID of the user * @param 64|512 $size Size of the avatar * @param bool $guestFallback Fallback to guest avatar if not found * @return FileDisplayResponse|JSONResponse, array{}>|Response @@ -151,6 +151,9 @@ public function getAvatar(string $userId, int $size, bool $guestFallback = false return $response; } + /** + * @param ?non-empty-string $path + */ #[NoAdminRequired] #[FrontpageRoute(verb: 'POST', url: '/avatar/')] public function postAvatar(?string $path = null): JSONResponse { diff --git a/core/Controller/PreviewController.php b/core/Controller/PreviewController.php index 310d7f85f74a1..8d79082975215 100644 --- a/core/Controller/PreviewController.php +++ b/core/Controller/PreviewController.php @@ -45,8 +45,8 @@ public function __construct( * Get a preview by file path * * @param string $file Path of the file - * @param int $x Width of the preview. A width of -1 will use the original image width. - * @param int $y Height of the preview. A height of -1 will use the original image height. + * @param int<-1, max> $x Width of the preview. A width of -1 will use the original image width. + * @param int<-1, max> $y Height of the preview. A height of -1 will use the original image height. * @param bool $a Preserve the aspect ratio * @param bool $forceIcon Force returning an icon * @param 'fill'|'cover' $mode How to crop the image @@ -88,9 +88,9 @@ public function getPreview( /** * Get a preview by file ID * - * @param int $fileId ID of the file - * @param int $x Width of the preview. A width of -1 will use the original image width. - * @param int $y Height of the preview. A height of -1 will use the original image height. + * @param positive-int $fileId ID of the file + * @param int<-1, max> $x Width of the preview. A width of -1 will use the original image width. + * @param int<-1, max> $y Height of the preview. A height of -1 will use the original image height. * @param bool $a Preserve the aspect ratio * @param bool $forceIcon Force returning an icon * @param 'fill'|'cover' $mode How to crop the image @@ -108,14 +108,14 @@ public function getPreview( #[FrontpageRoute(verb: 'GET', url: '/core/preview')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function getPreviewByFileId( - int $fileId = -1, + int $fileId, int $x = 32, int $y = 32, bool $a = false, bool $forceIcon = true, string $mode = 'fill', bool $mimeFallback = false) { - if ($fileId === -1 || $x === 0 || $y === 0) { + if ($x === 0 || $y === 0) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } diff --git a/core/openapi-full.json b/core/openapi-full.json index 32740488c7743..ebc5b8d0fcce2 100644 --- a/core/openapi-full.json +++ b/core/openapi-full.json @@ -1643,7 +1643,8 @@ "properties": { "password": { "type": "string", - "description": "The password of the user" + "description": "The password of the user", + "minLength": 1 } } } @@ -1691,7 +1692,8 @@ "properties": { "lastLogin": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 1 } } } @@ -1823,7 +1825,8 @@ ], "properties": { "apppassword": { - "type": "string" + "type": "string", + "minLength": 1 } } } @@ -1955,7 +1958,8 @@ "default": [], "items": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 0 } } }, @@ -1966,7 +1970,19 @@ "schema": { "type": "integer", "format": "int64", - "default": 10 + "default": 10, + "minimum": 1 + } + }, + { + "name": "offset", + "in": "query", + "description": "Offset for searching", + "schema": { + "type": "integer", + "format": "int64", + "default": 0, + "minimum": 0 } }, { @@ -1983,6 +1999,13 @@ "responses": { "200": { "description": "Autocomplete results returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -9247,7 +9270,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -9350,7 +9374,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -9977,7 +10002,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -9987,7 +10013,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -10118,10 +10145,11 @@ "name": "fileId", "in": "query", "description": "ID of the file", + "required": true, "schema": { "type": "integer", "format": "int64", - "default": -1 + "minimum": 1 } }, { @@ -10131,7 +10159,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -10141,7 +10170,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { diff --git a/core/openapi.json b/core/openapi.json index 0673f3f96c649..41906ac31ac93 100644 --- a/core/openapi.json +++ b/core/openapi.json @@ -1643,7 +1643,8 @@ "properties": { "password": { "type": "string", - "description": "The password of the user" + "description": "The password of the user", + "minLength": 1 } } } @@ -1691,7 +1692,8 @@ "properties": { "lastLogin": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 1 } } } @@ -1823,7 +1825,8 @@ ], "properties": { "apppassword": { - "type": "string" + "type": "string", + "minLength": 1 } } } @@ -1955,7 +1958,8 @@ "default": [], "items": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 0 } } }, @@ -1966,7 +1970,19 @@ "schema": { "type": "integer", "format": "int64", - "default": 10 + "default": 10, + "minimum": 1 + } + }, + { + "name": "offset", + "in": "query", + "description": "Offset for searching", + "schema": { + "type": "integer", + "format": "int64", + "default": 0, + "minimum": 0 } }, { @@ -1983,6 +1999,13 @@ "responses": { "200": { "description": "Autocomplete results returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -9247,7 +9270,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -9350,7 +9374,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -9977,7 +10002,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -9987,7 +10013,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -10118,10 +10145,11 @@ "name": "fileId", "in": "query", "description": "ID of the file", + "required": true, "schema": { "type": "integer", "format": "int64", - "default": -1 + "minimum": 1 } }, { @@ -10131,7 +10159,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -10141,7 +10170,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index be06cdd45b860..028e899d51056 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1207,6 +1207,7 @@ 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => $baseDir . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => $baseDir . '/lib/private/AppFramework/Http/Output.php', + 'OC\\AppFramework\\Http\\PaginationTrait' => $baseDir . '/lib/private/AppFramework/Http/PaginationTrait.php', 'OC\\AppFramework\\Http\\Request' => $baseDir . '/lib/private/AppFramework/Http/Request.php', 'OC\\AppFramework\\Http\\RequestId' => $baseDir . '/lib/private/AppFramework/Http/RequestId.php', 'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 73277b5502fe1..1e803efed5f4b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1248,6 +1248,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Http\\Attributes\\TwoFactorSetUpDoneRequired' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Attributes/TwoFactorSetUpDoneRequired.php', 'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php', 'OC\\AppFramework\\Http\\Output' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Output.php', + 'OC\\AppFramework\\Http\\PaginationTrait' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/PaginationTrait.php', 'OC\\AppFramework\\Http\\Request' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Request.php', 'OC\\AppFramework\\Http\\RequestId' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/RequestId.php', 'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php', diff --git a/lib/private/AppFramework/Http/PaginationTrait.php b/lib/private/AppFramework/Http/PaginationTrait.php new file mode 100644 index 0000000000000..e02154ef9a2b8 --- /dev/null +++ b/lib/private/AppFramework/Http/PaginationTrait.php @@ -0,0 +1,85 @@ +request` (already declared by + * OCP\AppFramework\Controller) and its own `$this->urlGenerator`; not declared here + * to avoid property composition conflicts with classes that mark theirs readonly. + * + * @property IRequest $request + * @property IURLGenerator $urlGenerator + */ +trait PaginationTrait { + /** + * @param array $items The page of results as returned for $limit + * @param non-negative-int|null $limit Requested page size, or null for "no limit" (never "more" in that case) + */ + protected function hasMoreResults(array $items, ?int $limit): bool { + return $limit !== null && $limit > 0 && count($items) >= $limit; + } + + /** + * @param array|bool $items The page of results, or an already-known "has more" bool + * when the caller has a more authoritative source than the + * item count (e.g. a search backend) + */ + private function resolveHasMoreResults(array|bool $items, ?int $limit): bool { + return is_bool($items) ? $items : $this->hasMoreResults($items, $limit); + } + + /** + * Builds the `headers` for a DataResponse: a `Link: ; rel="next"` header if + * there's a next page, or an empty array otherwise. + * + * @param array $params Query parameters for the next page, e.g. the incremented offset + * @return array{Link?: string} + */ + protected function buildOffsetNextPageLinkHeader(array|bool $items, array $params, ?int $limit, int $offset): array { + if (!$this->resolveHasMoreResults($items, $limit)) { + return []; + } + + $params = array_merge($params, [ + 'limit' => $limit, + 'offset' => $offset + $limit, + ]); + $path = (string)parse_url($this->request->getRequestUri(), PHP_URL_PATH); + $url = $this->urlGenerator->getAbsoluteURL($path) . '?' . http_build_query($params); + return ['Link' => '<' . $url . '>; rel="next"']; + } + + /** + * Same as buildOffsetNextPageLinkHeader(), but using keyset (seek) pagination + * instead of an offset. + * + * @param array $params Query parameters for the next page + * @param int|string|null $lastId Id of the last entity of the current page + * @return array{Link?: string} + */ + protected function buildCursorNextPageLinkHeader(array|bool $items, array $params, ?int $limit, int|string|null $lastId): array { + if ($lastId === null || !$this->resolveHasMoreResults($items, $limit)) { + return []; + } + + $params = array_merge($params, [ + 'limit' => $limit, + 'lastId' => $lastId, + ]); + $path = (string)parse_url($this->request->getRequestUri(), PHP_URL_PATH); + $url = $this->urlGenerator->getAbsoluteURL($path) . '?' . http_build_query($params); + return ['Link' => '<' . $url . '>; rel="next"']; + } +} diff --git a/lib/private/Security/SecureRandom.php b/lib/private/Security/SecureRandom.php index 8d38bd32df799..d823942e95bd3 100644 --- a/lib/private/Security/SecureRandom.php +++ b/lib/private/Security/SecureRandom.php @@ -14,21 +14,9 @@ /** * Class SecureRandom provides a wrapper around the random_int function to generate - * secure random strings. For PHP 7 the native CSPRNG is used, older versions do - * use a fallback. - * - * Usage: - * \OC::$server->get(ISecureRandom::class)->generate(10); - * @package OC\Security + * secure random strings. This use the native CSPRNG. */ class SecureRandom implements ISecureRandom { - /** - * Generate a secure random string of specified length. - * @param int $length The length of the generated string - * @param string $characters An optional list of characters to use if no character list is - * specified all valid base64 characters are used. - * @throws \LengthException if an invalid length is requested - */ #[\Override] public function generate( int $length, @@ -38,6 +26,8 @@ public function generate( throw new \LengthException('Invalid length specified: ' . $length . ' must be bigger than 0'); } - return (new Randomizer())->getBytesFromString($characters, $length); + /** @var non-empty-string $result */ + $result = (new Randomizer())->getBytesFromString($characters, $length); + return $result; } } diff --git a/lib/public/AppFramework/Http/DataResponse.php b/lib/public/AppFramework/Http/DataResponse.php index ea03ffc877a17..86ccaa7c55a99 100644 --- a/lib/public/AppFramework/Http/DataResponse.php +++ b/lib/public/AppFramework/Http/DataResponse.php @@ -42,7 +42,7 @@ public function __construct(mixed $data = [], int $statusCode = Http::STATUS_OK, * Sets values in the data json array * @psalm-suppress InvalidTemplateParam * @param T $data an array or object which will be transformed - * @return DataResponse Reference to this object + * @return $this Reference to this object * @since 8.0.0 */ public function setData($data) { diff --git a/lib/public/Security/ISecureRandom.php b/lib/public/Security/ISecureRandom.php index c2b769e2ba834..2a3c510de287f 100644 --- a/lib/public/Security/ISecureRandom.php +++ b/lib/public/Security/ISecureRandom.php @@ -62,7 +62,7 @@ interface ISecureRandom { * @param int $length The length of the generated string * @param string $characters An optional list of characters to use if no character list is * specified all valid base64 characters are used. - * @return string + * @return non-empty-string * @since 8.0.0 * @deprecated 35.0.0 Use {@see Randomizer::getBytesFromString()} available in PHP 8.3+ instead. */ diff --git a/openapi.json b/openapi.json index 51b027183b69f..bfb5ed2b96cc2 100644 --- a/openapi.json +++ b/openapi.json @@ -5928,7 +5928,8 @@ "properties": { "password": { "type": "string", - "description": "The password of the user" + "description": "The password of the user", + "minLength": 1 } } } @@ -5976,7 +5977,8 @@ "properties": { "lastLogin": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 1 } } } @@ -6108,7 +6110,8 @@ ], "properties": { "apppassword": { - "type": "string" + "type": "string", + "minLength": 1 } } } @@ -6240,7 +6243,8 @@ "default": [], "items": { "type": "integer", - "format": "int64" + "format": "int64", + "minimum": 0 } } }, @@ -6251,7 +6255,19 @@ "schema": { "type": "integer", "format": "int64", - "default": 10 + "default": 10, + "minimum": 1 + } + }, + { + "name": "offset", + "in": "query", + "description": "Offset for searching", + "schema": { + "type": "integer", + "format": "int64", + "default": 0, + "minimum": 0 } }, { @@ -6268,6 +6284,13 @@ "responses": { "200": { "description": "Autocomplete results returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -13579,7 +13602,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -13682,7 +13706,8 @@ "description": "ID of the user", "required": true, "schema": { - "type": "string" + "type": "string", + "minLength": 1 } }, { @@ -14309,7 +14334,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -14319,7 +14345,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -14450,10 +14477,11 @@ "name": "fileId", "in": "query", "description": "ID of the file", + "required": true, "schema": { "type": "integer", "format": "int64", - "default": -1 + "minimum": 1 } }, { @@ -14463,7 +14491,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -14473,7 +14502,8 @@ "schema": { "type": "integer", "format": "int64", - "default": 32 + "default": 32, + "minimum": -1 } }, { @@ -31764,6 +31794,13 @@ "responses": { "200": { "description": "Groups returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -32278,6 +32315,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -33705,6 +33749,13 @@ "responses": { "200": { "description": "Groups details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -34011,6 +34062,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -34163,6 +34221,13 @@ "responses": { "200": { "description": "Users returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -34483,6 +34548,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -34635,6 +34707,13 @@ "responses": { "200": { "description": "Disabled users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -37980,6 +38059,13 @@ "responses": { "200": { "description": "Recipients returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -43563,6 +43649,7 @@ "get": { "operationId": "user_status-statuses-find-all", "summary": "Find statuses of users", + "description": "Prefer $lastId over $offset: it does not require the database to scan and discard every preceding row on every call.", "tags": [ "user_status/statuses" ], @@ -43598,6 +43685,18 @@ "minimum": 0 } }, + { + "name": "lastId", + "in": "query", + "description": "Id of the last status returned by the previous page; when given, keyset pagination is used and $offset is ignored", + "schema": { + "type": "integer", + "format": "int64", + "nullable": true, + "default": null, + "minimum": 0 + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -43612,6 +43711,13 @@ "responses": { "200": { "description": "Statuses returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/tests/Core/Controller/AutoCompleteControllerTest.php b/tests/Core/Controller/AutoCompleteControllerTest.php index 5196c0e990878..dff48e357c57d 100644 --- a/tests/Core/Controller/AutoCompleteControllerTest.php +++ b/tests/Core/Controller/AutoCompleteControllerTest.php @@ -12,6 +12,7 @@ use OCP\Collaboration\Collaborators\ISearch; use OCP\EventDispatcher\IEventDispatcher; use OCP\IRequest; +use OCP\IURLGenerator; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -22,6 +23,10 @@ class AutoCompleteControllerTest extends TestCase { protected $autoCompleteManager; /** @var IEventDispatcher|MockObject */ protected $dispatcher; + /** @var IRequest|MockObject */ + protected $request; + /** @var IURLGenerator|MockObject */ + protected $urlGenerator; /** @var AutoCompleteController */ protected $controller; @@ -29,18 +34,19 @@ class AutoCompleteControllerTest extends TestCase { protected function setUp(): void { parent::setUp(); - /** @var IRequest $request */ - $request = $this->createMock(IRequest::class); + $this->request = $this->createMock(IRequest::class); $this->collaboratorSearch = $this->createMock(ISearch::class); $this->autoCompleteManager = $this->createMock(IManager::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->controller = new AutoCompleteController( 'core', - $request, + $this->request, $this->collaboratorSearch, $this->autoCompleteManager, - $this->dispatcher + $this->dispatcher, + $this->urlGenerator, ); } @@ -170,5 +176,33 @@ public function testGet(array $searchResults, array $expected, string $searchTer $list = $response->getData(); $this->assertEquals($expected, $list); // has better error output… $this->assertSame($expected, $list); + $this->assertArrayNotHasKey('Link', $response->getHeaders()); + } + + public function testGetSetsLinkHeaderWhenMoreResultsExist(): void { + $this->collaboratorSearch->expects($this->once()) + ->method('search') + ->with('bob', [0], false, 2, 0) + ->willReturn([[ + 'exact' => ['users' => [], 'robots' => []], + 'users' => [ + ['label' => 'Bob Y.', 'value' => ['shareWith' => 'bob']], + ['label' => 'Bobby R.', 'value' => ['shareWith' => 'bobby']], + ], + ], true]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/core/autocomplete/get?search=bob&limit=2'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/core/autocomplete/get') + ->willReturn('https://cloud.example.com/ocs/v2.php/core/autocomplete/get'); + + $response = $this->controller->get('bob', null, null, null, [0], 2, 0); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); } } diff --git a/tests/lib/AppFramework/Http/PaginationTraitTest.php b/tests/lib/AppFramework/Http/PaginationTraitTest.php new file mode 100644 index 0000000000000..a2dcaf2d21485 --- /dev/null +++ b/tests/lib/AppFramework/Http/PaginationTraitTest.php @@ -0,0 +1,124 @@ +request = $this->createMock(IRequest::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + + $this->subject = new class($this->request, $this->urlGenerator) { + use PaginationTrait; + + public function __construct( + protected IRequest $request, + private IURLGenerator $urlGenerator, + ) { + } + + public function hasMore(array $items, ?int $limit): bool { + return $this->hasMoreResults($items, $limit); + } + + public function nextLinkHeaders(array|bool $items, array $params, ?int $limit, int $offset): array { + return $this->buildOffsetNextPageLinkHeader($items, $params, $limit, $offset); + } + + public function cursorLinkHeaders(array|bool $items, array $params, ?int $limit, int|string|null $lastId): array { + return $this->buildCursorNextPageLinkHeader($items, $params, $limit, $lastId); + } + }; + } + + public function testNullLimitNeverReportsMore(): void { + $this->assertFalse($this->subject->hasMore(['a', 'b', 'c'], null)); + } + + public function testZeroLimitNeverReportsMore(): void { + $this->assertFalse($this->subject->hasMore([], 0)); + } + + public function testFewerResultsThanLimitReportsNoMore(): void { + $this->assertFalse($this->subject->hasMore(['a', 'b'], 5)); + } + + public function testExactlyLimitResultsReportsMore(): void { + $this->assertTrue($this->subject->hasMore(['a', 'b', 'c'], 3)); + } + + public function testEmptyResultWithPositiveLimitReportsNoMore(): void { + $this->assertFalse($this->subject->hasMore([], 5)); + } + + public function testAssociativeArrayIsCountedByEntries(): void { + $this->assertTrue($this->subject->hasMore(['uid1' => 'Alice', 'uid2' => 'Bob'], 2)); + } + + public function testNextLinkKeepsRequestPathAndAppliesGivenQuery(): void { + $this->request->method('getRequestUri')->willReturn('/ocs/v2.php/apps/provisioning_api/api/v1/groups?search=foo&limit=5&offset=0'); + + $this->urlGenerator->method('getAbsoluteURL') + ->with('/ocs/v2.php/apps/provisioning_api/api/v1/groups') + ->willReturn('https://cloud.example.com/ocs/v2.php/apps/provisioning_api/api/v1/groups'); + + $headers = $this->subject->nextLinkHeaders(['a', 'b', 'c', 'd', 'e'], ['search' => 'foo'], 5, 0); + + $this->assertSame([ + 'Link' => '; rel="next"', + ], $headers); + } + + public function testNextLinkReturnsEmptyHeadersWhenFewerItemsThanLimit(): void { + $this->assertSame([], $this->subject->nextLinkHeaders(['a', 'b'], ['search' => 'foo'], 5, 0)); + } + + public function testNextLinkAcceptsAPrecomputedBool(): void { + $this->assertSame([], $this->subject->nextLinkHeaders(false, ['search' => 'foo'], 5, 0)); + + $this->request->method('getRequestUri')->willReturn('/ocs/v2.php/core/autocomplete/get'); + $this->urlGenerator->method('getAbsoluteURL') + ->with('/ocs/v2.php/core/autocomplete/get') + ->willReturn('https://cloud.example.com/ocs/v2.php/core/autocomplete/get'); + + $this->assertArrayHasKey('Link', $this->subject->nextLinkHeaders(true, ['search' => 'foo'], 5, 0)); + } + + public function testCursorLinkKeepsRequestPathAndUsesLastId(): void { + $this->request->method('getRequestUri')->willReturn('/ocs/v2.php/apps/user_status/api/v1/statuses?limit=1&lastId=1336'); + + $this->urlGenerator->method('getAbsoluteURL') + ->with('/ocs/v2.php/apps/user_status/api/v1/statuses') + ->willReturn('https://cloud.example.com/ocs/v2.php/apps/user_status/api/v1/statuses'); + + $headers = $this->subject->cursorLinkHeaders(['a'], [], 1, 1337); + + $this->assertSame([ + 'Link' => '; rel="next"', + ], $headers); + } + + public function testCursorLinkReturnsEmptyHeadersWhenFewerItemsThanLimit(): void { + $this->assertSame([], $this->subject->cursorLinkHeaders(['a', 'b'], [], 5, 1337)); + } + + public function testCursorLinkReturnsEmptyHeadersWhenLastIdIsMissing(): void { + $this->assertSame([], $this->subject->cursorLinkHeaders(['a'], [], 1, null)); + } +}