From 0629f859398ff4457c722480b934cfa6c45ab945 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 28 Jan 2025 22:05:42 +0100 Subject: [PATCH 1/4] feat(Api): add v2 OCS Api to get table/view rows - also changes Api Route definition to attribute Signed-off-by: Arthur Schiwon --- lib/Controller/RowOCSController.php | 153 +++++++- lib/Db/Row2Mapper.php | 3 +- lib/Db/RowQuery.php | 76 ++++ lib/Model/FilterInput.php | 48 +++ lib/Service/RowService.php | 66 +++- openapi.json | 331 ++++++++++++++++++ src/types/openapi/openapi.ts | 133 ++++++- tests/integration/features/RowOCS.feature | 213 +++++++++++ .../features/bootstrap/FeatureContext.php | 80 ++++- 9 files changed, 1093 insertions(+), 10 deletions(-) create mode 100644 lib/Db/RowQuery.php create mode 100644 lib/Model/FilterInput.php create mode 100644 tests/integration/features/RowOCS.feature diff --git a/lib/Controller/RowOCSController.php b/lib/Controller/RowOCSController.php index 5e7a161f12..a1922e0dca 100644 --- a/lib/Controller/RowOCSController.php +++ b/lib/Controller/RowOCSController.php @@ -1,5 +1,7 @@ $data An array containing the column identifiers and their values - * @return DataResponse|DataResponse + * @param string|array $data An array containing the column + * identifiers and their values + * @return DataResponse|DataResponse * * 200: Row returned * 400: Invalid request parameters @@ -105,6 +117,87 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat } } + /** + * [api v2] get a number of rows from a table or view + * + * When reading from views, the specified filter is added to each existing + * filter group. + * + * The filter definitions provided are all AND-connected. + * + * Sort orders on the other hand do overwrite the view's default sort order. + * Only when `null` is passed the default sort order will be used. + * + * @param 'tables'|'views' $nodeCollection Indicates whether to get rows + * from a table or view + * @psalm-param int<0,max> $nodeId The ID of the table or view + * @psalm-param ?int<1,500> $limit Number of rows to return between 1 and 500, fetches all by default (optional) + * @psalm-param ?int<0,max> $offset Offset of the rows to be returned (optional) + * @param ?string $filter Additional row filter as JSON-encoded filter groups (optional) + * @param list|null $sort Custom sort order (optional) + * @return DataResponse, + * array{}>|DataResponse + * + * 200: Rows returned + * 400: Invalid request parameters + * 403: No permissions + * 404: Not found + * 500: Internal error + */ + #[NoAdminRequired] + #[RequirePermission(permission: Application::PERMISSION_READ, typeParam: 'nodeCollection')] + #[ApiRoute( + verb: 'GET', + url: '/api/2/{nodeCollection}/{nodeId}/rows', + requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)'] + )] + public function getRows(string $nodeCollection, int $nodeId, ?int $limit, ?int $offset, mixed $filter = null, ?array $sort = null): DataResponse { + $queryData = new RowQuery( + nodeType: $nodeCollection === 'tables' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, + nodeId: $nodeId, + ); + + try { + if (($limit !== null && ($limit <= 0 || $limit > 500)) + || ($offset !== null && $offset < 0) + ) { + throw new InvalidArgumentException('Offset or limit parameter is out of bounds'); + } + + $filterInput = FilterInput::fromRequestValue($filter); + $filterGroups = $filterInput->filter ?: null; + if ($filterGroups) { + foreach ($filterGroups as $filterGroup) { + foreach ($filterGroup as $singleFilter) { + $this->assertFilterValue($singleFilter); + } + } + } + if ($sort) { + foreach ($sort as $singleSortRule) { + $this->assertSortValue($singleSortRule); + } + } + $queryData->setLimit($limit) + ->setOffset($offset) + ->setFilter($filterGroups) + ->setSort($sort) + ->setUserId($this->userId); + + $rows = $this->rowService->findAllByQuery($queryData); + return new DataResponse($this->rowService->formatRows($rows)); + } catch (PermissionError $e) { + return $this->handlePermissionError($e); + } catch (InternalError|Exception $e) { + return $this->handleError($e); + } catch (DoesNotExistException $e) { + return $this->handleNotFoundError(new NotFoundError($e->getMessage(), $e->getCode(), $e)); + } catch (MultipleObjectsReturnedException|InvalidArgumentException $e) { + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); + } + } + /** * [api v2] Update a row in a table or a view * @@ -198,4 +291,58 @@ public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): Data return $this->handleError($e); } } + + /** + * @param array{columnId: int, operator: 'begins-with'|'ends-with'|'contains'|'does-not-contain'|'is-equal'|'is-not-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal'|'is-empty',value: string|int|float} $filter + */ + protected function assertFilterValue(array $filter): void { + if (!isset($filter['columnId'], $filter['operator'], $filter['value']) + || count($filter) !== 3 + ) { + throw new InvalidArgumentException('Invalid filter supplied'); + } + $maxDigits = strlen((string)PHP_INT_MAX); + if (!is_numeric($filter['columnId']) + || (int)$filter['columnId'] < -5 + || !preg_match('/^\d{0,' . $maxDigits . '}$/', (string)$filter['columnId']) + ) { + throw new InvalidArgumentException(sprintf('Invalid column id supplied: %d', $filter['columnId'])); + } + if (!in_array($filter['operator'], [ + 'begins-with', + 'ends-with', + 'contains', + 'does-not-contain', + 'is-equal', + 'is-not-equal', + 'is-greater-than', + 'is-greater-than-or-equal', + 'is-lower-than', + 'is-lower-than-or-equal', + 'is-empty', + ], true)) { + throw new InvalidArgumentException('Invalid filter operator supplied'); + } + } + + /** + * @param array{columnId: int, mode: 'ASC'|'DESC'} $sort + */ + protected function assertSortValue(array $sort): void { + if (!isset($sort['columnId'], $sort['mode']) + || count($sort) !== 2 + ) { + throw new InvalidArgumentException('Invalid sort data supplied'); + } + $maxDigits = strlen((string)PHP_INT_MAX); + if (!is_numeric($sort['columnId']) + || (int)$sort['columnId'] < -5 + || !preg_match('/^\d{0,' . $maxDigits . '}$/', (string)$sort['columnId']) + ) { + throw new InvalidArgumentException('Invalid column id supplied'); + } + if ($sort['mode'] !== 'DESC' && $sort['mode'] !== 'ASC') { + throw new InvalidArgumentException('Invalid sort mode supplied'); + } + } } diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index a2979afbda..6e0d08ff3f 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -153,7 +153,7 @@ private function getWantedRowIds(string $userId, int $tableId, ?array $filter = throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), ); } - return array_map(fn (array $item) => $item['id'], $result->fetchAllAssociative()); + return array_map(static fn (array $item) => $item['id'], $result->fetchAllAssociative()); } /** @@ -173,7 +173,6 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset); - // Get rows without SQL sorting $rows = $this->getRows($wantedRowIdsArray, $showColumnIds); // Sort rows in PHP to preserve the order from getWantedRowIds diff --git a/lib/Db/RowQuery.php b/lib/Db/RowQuery.php new file mode 100644 index 0000000000..93b25024b9 --- /dev/null +++ b/lib/Db/RowQuery.php @@ -0,0 +1,76 @@ +nodeType; + } + + public function getNodeId(): int { + return $this->nodeId; + } + + public function getUserId(): ?string { + return $this->userId; + } + + public function setUserId(?string $userId): self { + $this->userId = $userId; + return $this; + } + + public function getLimit(): ?int { + return $this->limit; + } + + public function setLimit(?int $limit): self { + $this->limit = $limit; + return $this; + } + + public function getOffset(): ?int { + return $this->offset; + } + + public function setOffset(?int $offset): self { + $this->offset = $offset; + return $this; + } + + public function getFilter(): ?array { + return $this->filter; + } + + public function setFilter(?array $filter): self { + $this->filter = $filter; + return $this; + } + + public function getSort(): ?array { + return $this->sort; + } + + public function setSort(?array $sort): self { + $this->sort = $sort; + return $this; + } +} diff --git a/lib/Model/FilterInput.php b/lib/Model/FilterInput.php new file mode 100644 index 0000000000..56b9824542 --- /dev/null +++ b/lib/Model/FilterInput.php @@ -0,0 +1,48 @@ +> $filter + */ + private function __construct( + public readonly array $filter, + ) { + } + + public static function fromRequestValue(mixed $value): self { + if ($value === null || $value === '') { + return new self([]); + } + + if (is_string($value)) { + try { + $value = json_decode($value, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException $e) { + throw new InvalidArgumentException('Invalid filter supplied', 0, $e); + } + } + + if ($value === null) { + return new self([]); + } + + if (!is_array($value)) { + throw new InvalidArgumentException('Invalid filter supplied'); + } + + return new self($value); + } +} diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 95baa53324..bd0d9a571a 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -8,11 +8,12 @@ namespace OCA\Tables\Service; use OCA\Tables\Activity\ActivityManager; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\Column; use OCA\Tables\Db\ColumnMapper; use OCA\Tables\Db\Row2; use OCA\Tables\Db\Row2Mapper; -use OCA\Tables\Db\Table; +use OCA\Tables\Db\RowQuery; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; @@ -85,6 +86,66 @@ public function formatRowsForPublicShare(array $rows): array { }, $rows); } + /** + * @throws MultipleObjectsReturnedException + * @throws DoesNotExistException + * @throws Exception + * @throws InternalError + * @return Row2[] + */ + public function findAllByQuery(RowQuery $rowQuery): array { + $tableId = $rowQuery->getNodeId(); + $columns = null; + + if ($rowQuery->getNodeType() === Application::NODE_TYPE_VIEW) { + $view = $this->viewMapper->find($rowQuery->getNodeId()); + $tableId = $view->getTableId(); + $columns = $this->columnMapper->findAll($view->getColumnsArray()); + + $userId = $this->resolveFilterUserId($rowQuery->getUserId() ?? $this->userId ?? '', $view); + $rowQuery->setUserId($userId); + + if ($rowQuery->getFilter() !== null) { + $baseFilterGroups = $view->getFilterArray(); + if (empty($baseFilterGroups)) { + $baseFilterGroups = $rowQuery->getFilter(); + } else { + $additionalFilterRules = array_merge(...$rowQuery->getFilter()); + foreach ($baseFilterGroups as &$baseFilterGroup) { + array_push($baseFilterGroup, ...$additionalFilterRules); + } + unset($baseFilterGroup); + } + $rowQuery->setFilter($baseFilterGroups); + } else { + $rowQuery->setFilter($view->getFilterArray()); + } + + if ($rowQuery->getSort() === null) { + $rowQuery->setSort($view->getSortArray()); + } + } elseif ($rowQuery->getSort() === null) { + $table = $this->tableMapper->find($tableId); + $rowQuery->setSort($table->getSortArray() ?: null); + } + + $tableColumns = $this->columnMapper->findAllByTable($tableId); + $columns ??= $tableColumns; + $showColumnIds = array_map(static fn (Column $column): int => $column->getId(), $columns); + + $rows = $this->row2Mapper->findAll( + $showColumnIds, + $tableId, + $rowQuery->getLimit(), + $rowQuery->getOffset(), + $rowQuery->getFilter(), + $rowQuery->getSort(), + $rowQuery->getUserId() ?? $this->userId ?? '', + ); + $this->attachAliasPayloads($rows, $columns); + return $rows; + } + /** * @param int $tableId * @param string $userId @@ -852,6 +913,9 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { * We do not check if you are allowed to remove this data. That has to be * done before! Why? Mostly this check will have be run before and we can * pass this here due to performance reasons. + * We do not check if you are allowed to remove this data. That has to be + * done before! Why? Mostly this check will have be run before and we can + * pass this here due to performance reasons. * * @param Column $column * @throws InternalError diff --git a/openapi.json b/openapi.json index 412df8cf42..02e9527832 100644 --- a/openapi.json +++ b/openapi.json @@ -16031,6 +16031,337 @@ } } } + }, + "get": { + "operationId": "rowocs-get-rows", + "summary": "[api v2] get a number of rows from a table or view", + "description": "When reading from views, the specified filter is added to each existing filter group.\nThe filter definitions provided are all AND-connected.\nSort orders on the other hand do overwrite the view's default sort order. Only when `null` is passed the default sort order will be used.", + "tags": [ + "rowocs" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "nodeCollection", + "in": "path", + "description": "Indicates whether to get rows from a table or view", + "required": true, + "schema": { + "type": "string", + "enum": [ + "tables", + "views" + ], + "pattern": "^(tables|views)$" + } + }, + { + "name": "nodeId", + "in": "path", + "description": "The ID of the table or view", + "required": true, + "schema": { + "type": "integer", + "format": "int64", + "minimum": 0 + } + }, + { + "name": "limit", + "in": "query", + "description": "Number of rows to return between 1 and 500, fetches all by default (optional)", + "schema": { + "type": "integer", + "format": "int64", + "nullable": true, + "minimum": 1, + "maximum": 500 + } + }, + { + "name": "offset", + "in": "query", + "description": "Offset of the rows to be returned (optional)", + "schema": { + "type": "integer", + "format": "int64", + "nullable": true, + "minimum": 0 + } + }, + { + "name": "filter", + "in": "query", + "description": "Additional row filter as JSON-encoded filter groups (optional)", + "schema": { + "type": "string", + "nullable": true, + "default": null + } + }, + { + "name": "sort[]", + "in": "query", + "description": "Custom sort order (optional)", + "schema": { + "type": "array", + "nullable": true, + "default": null, + "items": { + "type": "object", + "required": [ + "columnId", + "mode" + ], + "properties": { + "columnId": { + "type": "integer", + "format": "int64" + }, + "mode": { + "type": "string", + "enum": [ + "ASC", + "DESC" + ] + } + } + } + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Rows returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Row" + } + } + } + } + } + } + } + } + }, + "403": { + "description": "No permissions", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "400": { + "description": "Invalid request parameters", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "404": { + "description": "Not found", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "500": { + "description": "Internal error", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "message" + ], + "properties": { + "message": { + "type": "string" + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } } }, "/ocs/v2.php/apps/tables/api/2/{nodeCollection}/{nodeId}/rows/{rowId}": { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 665abe8276..f10c7d23df 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1024,7 +1024,13 @@ export type paths = { readonly path?: never; readonly cookie?: never; }; - readonly get?: never; + /** + * [api v2] get a number of rows from a table or view + * @description When reading from views, the specified filter is added to each existing filter group. + * The filter definitions provided are all AND-connected. + * Sort orders on the other hand do overwrite the view's default sort order. Only when `null` is passed the default sort order will be used. + */ + readonly get: operations["rowocs-get-rows"]; readonly put?: never; /** [api v2] Create a new row in a table or a view */ readonly post: operations["rowocs-create-row"]; @@ -8490,6 +8496,131 @@ export interface operations { }; }; }; + readonly "rowocs-get-rows": { + readonly parameters: { + readonly query?: { + /** @description Number of rows to return between 1 and 500, fetches all by default (optional) */ + readonly limit?: number | null; + /** @description Offset of the rows to be returned (optional) */ + readonly offset?: number | null; + /** @description Additional row filter as JSON-encoded filter groups (optional) */ + readonly filter?: string | null; + /** @description Custom sort order (optional) */ + readonly "sort[]"?: readonly { + /** Format: int64 */ + readonly columnId: number; + /** @enum {string} */ + readonly mode: "ASC" | "DESC"; + }[] | null; + }; + readonly header: { + /** @description Required to be true for the API request to pass */ + readonly "OCS-APIRequest": boolean; + }; + readonly path: { + /** @description Indicates whether to get rows from a table or view */ + readonly nodeCollection: "tables" | "views"; + /** @description The ID of the table or view */ + readonly nodeId: number; + }; + readonly cookie?: never; + }; + readonly requestBody?: never; + readonly responses: { + /** @description Rows returned */ + readonly 200: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: readonly components["schemas"]["Row"][]; + }; + }; + }; + }; + /** @description Invalid request parameters */ + readonly 400: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; + /** @description Current user is not logged in */ + readonly 401: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: unknown; + }; + }; + }; + }; + /** @description No permissions */ + readonly 403: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; + /** @description Not found */ + readonly 404: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; + /** @description Internal error */ + readonly 500: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: { + readonly message: string; + }; + }; + }; + }; + }; + }; + }; readonly "rowocs-create-row": { readonly parameters: { readonly query?: never; diff --git a/tests/integration/features/RowOCS.feature b/tests/integration/features/RowOCS.feature new file mode 100644 index 0000000000..302ee043ea --- /dev/null +++ b/tests/integration/features/RowOCS.feature @@ -0,0 +1,213 @@ +# SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: AGPL-3.0-or-later +Feature: RowOCS + Background: + Given user "participant1-v2" exists + Given user "participant2-v2" exists + Given user "participant3-v2" exists + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "one" exists with following properties + | type | text | + | subtype | line | + | mandatory | 0 | + And column "two" exists with following properties + | type | number | + | mandatory | 1 | + | numberDefault | 10 | + And column "three" exists with following properties + | type | selection | + | subtype | check | + | mandatory | 1 | + And column "four" exists with following properties + | type | datetime | + | subtype | date | + | mandatory | 0 | + And column "five" exists with following properties + | type | usergroup | + | mandatory | 1 | + | usergroupMultipleItems | true | + | usergroupSelectUsers | true | + | usergroupSelectGroups | false | + | usergroupSelectTeams | false | + And using "table" "t1" + And user "participant1-v2" creates row "r1" with following values: + | one | Row one | + | two | 1600 | + | three | true | + | four | 2025-01-01 | + | five | [{"id": "alice", "type": 0}] | + And user "participant1-v2" creates row "r2" with following values: + | one | Row two | + | two | 1604 | + | three | false | + | four | 2025-01-12 | + | five | [{"id": "bob", "type": 0},{"id": "clarence", "type": 0}] | + And user "participant1-v2" creates row "r3" with following values: + | one | Row three | + | two | 1628 | + | three | true | + | four | 2025-01-23 | + | five | [{"id": "dany", "type": 0}] | + And user "participant1-v2" creates row "r4" with following values: + | one | Row four | + | two | 1669 | + | three | false | + | four | 2025-02-03 | + | five | [{"id": "elias", "type": 0},{"id": "fran", "type": 0}] | + And user "participant1-v2" creates row "r5" with following values: + | one | Row five | + | two | 1711 | + | three | true | + | four | 2025-02-14 | + | five | [{"id": "george", "type": 0},{"id": "hannah", "type": 0},{"id": "ines", "type": 0}] | + And user "participant1-v2" creates row "r6" with following values: + | one | Row six | + | two | 1729 | + | three | true | + | four | 2025-02-25 | + | five | [{"id": "jamie", "type": 0}] | + And user "participant1-v2" creates row "r7" with following values: + | one | Row seven | + | two | 1794 | + | three | false | + | four | 2025-03-08 | + | five | [{"id": "kate", "type": 0},{"id": "lena", "type": 0}] | + And user "participant1-v2" creates row "r8" with following values: + | one | Row eight | + | two | 1827 | + | three | false | + | four | 2025-03-19 | + | five | [{"id": "moe", "type": 0}] | + And user "participant1-v2" creates row "r9" with following values: + | one | Row nine | + | two | 1924 | + | three | true | + | four | 2025-03-30 | + | five | [{"id": "nora", "type": 0}] | + And user "participant1-v2" creates row "r10" with following values: + | one | Row ten | + | two | 1994 | + | three | true | + | four | 2025-04-10 | + | five | [{"id": "otto", "type": 0},{"id": "pierre", "type": 0}] | + And user "participant1-v2" creates row "r11" with following values: + | one | Row eleven | + | two | 2006 | + | three | true | + | four | 2025-04-21 | + | five | [{"id": "quinn", "type": 0},{"id": "roberta", "type": 0}] | + And user "participant1-v2" creates row "r12" with following values: + | one | Row twelve | + | two | 2023 | + | three | false | + | four | 2025-05-05 | + | five | [{"id": "samir", "type": 0},{"id": "teresa", "type": 0}] | + And user "participant1-v2" creates row "r13" with following values: + | one | Row thirteen | + | two | 2061 | + | three | false | + | four | 2025-05-16 | + | five | [{"id": "udai", "type": 0},{"id": "vera", "type": 0},{"id": "xuan", "type": 0}] | + And user "participant1-v2" creates row "r14" with following values: + | one | Row fourteen | + | two | 2083 | + | three | false | + | four | 2025-05-27 | + | five | [{"id": "yvonne", "type": 0},{"id": "zara", "type": 0},{"id": "ahmad", "type": 0}] | + And user "participant1-v2" creates row "r15" with following values: + | one | Row fifteen | + | two | 2137 | + | three | true | + | four | 2025-06-07 | + | five | [{"id": "bertram", "type": 0}] | + And user "participant1-v2" shares table with user "participant2-v2" + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" shares view "v1" with "participant3-v2" + + @tables + Scenario: Get all rows from a table + Given as user "participant2-v2" + When the current user fetches all rows from "table" "t1" + Then the reported status is 200 + And 15 rows have been loaded + + @views + Scenario: Get all rows from a view + Given as user "participant3-v2" + When the current user fetches all rows from "view" "v1" + Then the reported status is 200 + And 15 rows have been loaded + + @tables @views + Scenario Outline: Get rows from a table or view With Offset + Given as user "" + When the current user fetches rows from "" "" with those parameters + | offset | | + Then the reported status is + And rows have been loaded + + Examples: + | user | type | alias | offset | responseCode | rowsReturned | + | participant2-v2 | table | t1 | -1 | 400 | 0 | + | participant3-v2 | view | v1 | -1 | 400 | 0 | + | participant2-v2 | table | t1 | 200 | 200 | 0 | + | participant3-v2 | view | v1 | 200 | 200 | 0 | + + @tables @views + Scenario Outline: Get rows from a table or view With Offset + Given as user "" + When the current user fetches rows from "" "" with those parameters + | offset | 5 | + Then the reported status is 200 + And 10 rows have been loaded + And rows "r1,r2,r3,r4,r5" are not included in the response + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Get rows from a table or view With Offset + Given as user "" + When the current user fetches rows from "" "" with those parameters + | limit | | + Then the reported status is + And rows have been loaded + + Examples: + | user | type | alias | limit | responseCode | rowsReturned | + | participant2-v2 | table | t1 | -1 | 400 | 0 | + | participant3-v2 | view | v1 | -1 | 400 | 0 | + | participant2-v2 | table | t1 | 0 | 400 | 0 | + | participant3-v2 | view | v1 | 0 | 400 | 0 | + | participant2-v2 | table | t1 | 555 | 400 | 0 | + | participant3-v2 | view | v1 | 555 | 400 | 0 | + + @tables @views + Scenario Outline: Get rows from a table or view With Offset + Given as user "" + When the current user fetches rows from "" "" with those parameters + | limit | 5 | + Then the reported status is 200 + And 5 rows have been loaded + And rows "r6,r7,r8,r9,r10,r11,r12,r13,r14,r15" are not included in the response + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views @current + Scenario Outline: Get rows from a table or view with a filter + Given as user "" + When the current user fetches rows from "" "" with those parameters + | filter | one,contains,t | + Then the reported status is 200 + And 8 rows have been loaded + And rows "r6,r7,r8,r9,r10,r11,r12,r13,r14,r15" are not included in the response + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 7278b1e12d..278441b330 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -65,8 +65,6 @@ class FeatureContext implements Context { // Store data from last request to perform assertions, id is used as a key private array $tableData = []; - private $importColumnData = null; - // use CommandLineTrait; private readonly CollectionManager $collectionManager; @@ -95,7 +93,6 @@ public function setUp() { * @AfterScenario */ public function cleanupUsers() { - $this->importColumnData = null; $this->collectionManager->cleanUp(); foreach ($this->createdUsers as $user) { $this->deleteUser($user); @@ -3300,4 +3297,81 @@ public function theLastCreatedRowHasTheFollowingDataByAlias(TableNode $table): v Assert::assertEquals($columnId, $dataByAlias[$alias]['columnId'], "columnId mismatch for alias '$alias'"); } } + + /** + * @When the current user fetches all rows from :nodeType :nodeId + */ + public function theCurrentUserFetchesAllRowsFromCollection(string $nodeType, string $nodeAlias): void { + $nodeId = $this->collectionManager->getByAlias($nodeType, $nodeAlias)['id']; + $this->sendOcsRequest('GET', sprintf('/apps/tables/api/2/%ss/%d/rows', $nodeType, $nodeId)); + } + + /** + * @When the current user fetches rows from :nodeType :nodeId with those parameters + */ + public function theCurrentUserFetchesRowsFromWithThoseParameters(string $nodeType, string $nodeAlias, TableNode $parameters): void { + $query = ''; + foreach ($parameters->getRows() as $row) { + if ($row[0] !== 'filter') { + $parameterName = $row[0]; + $parameterValue = $row[1]; + } else { + [$columnAlias, $operator, $value] = explode(',', $row[1]); + $columnId = $this->collectionManager->getByAlias('column', $columnAlias)['id']; + $filterArray = [ + [ + [ + 'columnId' => $columnId, + 'operator' => $operator, + 'value' => $value, + ], + ], + ]; + $parameterName = $row[0]; + $parameterValue = json_encode($filterArray); + } + $query .= $parameterName . '=' . urlencode($parameterValue) . '&'; + } + $nodeId = $this->collectionManager->getByAlias($nodeType, $nodeAlias)['id']; + $this->sendOcsRequest('GET', sprintf('/apps/tables/api/2/%ss/%d/rows?%s', $nodeType, $nodeId, $query)); + } + + /** + * @Given :numberOfRows rows have been loaded + */ + public function rowsHaveBeenLoaded(int $numberOfRows): void { + $responseData = $this->getDataFromResponse($this->response)['ocs']['data']; + unset($responseData['message']); + Assert::assertCount($numberOfRows, $responseData); + + $returnedIDs = []; + foreach ($responseData as $row) { + $returnedIDs[] = (int)$row['id']; + } + $this->collectionManager->register($returnedIDs, 'returnedRowIDs', 0); + } + + /** + * @Given rows :rowAliasList are not included in the response + */ + public function rowsAreNotIncludedInTheResponse(string $rowAliasList): void { + $unexpectedRowAliases = array_map('trim', explode(',', $rowAliasList)); + $returnedRowIds = $this->collectionManager->getById('returnedRowIDs', 0); + foreach ($unexpectedRowAliases as $unexpectedRowAlias) { + $row = $this->collectionManager->getByAlias('row', $unexpectedRowAlias); + Assert::assertNotContains($row['id'], $returnedRowIds); + } + } + + /** + * @Given rows :rowAliasList are included in the response + */ + public function rowsAreIncludedInTheResponse(string $rowAliasList): void { + $expectedRowAliases = array_map('trim', explode(',', $rowAliasList)); + $returnedRowIds = $this->collectionManager->getById('returnedRowIDs', 0); + foreach ($expectedRowAliases as $expectedRowAlias) { + $row = $this->collectionManager->getByAlias('row', $expectedRowAlias); + Assert::assertContains($row['id'], $returnedRowIds); + } + } } From c0f15c577aa27ecb741e13ba4e8ef08ff3ff99df Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Mon, 29 Jun 2026 08:07:35 +0100 Subject: [PATCH 2/4] fix: Add tests and composer fixes Signed-off-by: Enjeck C. --- lib/Activity/ActivityManager.php | 12 +- lib/Controller/RowOCSController.php | 138 ++++++++++++------ lib/Db/ContextMapper.php | 8 +- lib/Db/Row2Mapper.php | 5 +- lib/Db/RowQuery.php | 1 + .../Version2020Date20260513185340.php | 4 +- .../Version2202Date20260825184226.php | 4 +- lib/Service/ColumnService.php | 9 +- lib/Service/PermissionsService.php | 4 +- lib/Service/RelationService.php | 4 +- lib/Service/RowService.php | 85 ++++++----- lib/Service/TableService.php | 4 +- openapi.json | 38 ++--- src/types/openapi/openapi.ts | 22 ++- tests/integration/features/RowOCS.feature | 99 ++++++++++++- .../features/bootstrap/FeatureContext.php | 58 ++++++-- vendor-bin/rector/composer.lock | 24 +-- 17 files changed, 323 insertions(+), 196 deletions(-) diff --git a/lib/Activity/ActivityManager.php b/lib/Activity/ActivityManager.php index 452b98591d..1add9d4382 100644 --- a/lib/Activity/ActivityManager.php +++ b/lib/Activity/ActivityManager.php @@ -83,17 +83,15 @@ public function __construct( /** * @param Row2|Table|View|Column $object - * @param array|null|string $additionalParams + * @param array $additionalParams * @param string|null $author * * @psalm-param self::TABLES_OBJECT_* $objectType - * @psalm-param array|null|string $additionalParams + * @psalm-param array $additionalParams * @psalm-param string|null $author */ - public function triggerEvent(string $objectType, Row2|Table|View|Column $object, string $subject, array|string|null $additionalParams = [], array|string|null $author = null) { - if ($author === null) { - $author = $this->userId; - } + public function triggerEvent(string $objectType, Row2|Table|View|Column $object, string $subject, array $additionalParams = [], ?string $author = null) { + $author ??= $this->userId; try { $event = $this->createEvent($objectType, $object, $subject, $additionalParams, $author); @@ -154,7 +152,7 @@ public function triggerUpdateEvents(string $objectType, ChangeSet $changeSet, st * @psalm-param array $additionalParams * @psalm-param string|null $author */ - private function createEvent(string $objectType, Row2|Table|View|Column $object, string $subject, array $additionalParams = [], array|string|null $author = null) { + private function createEvent(string $objectType, Row2|Table|View|Column $object, string $subject, array $additionalParams = [], ?string $author = null) { if ($object instanceof Table) { $objectTitle = $object->getTitle(); $table = $object; diff --git a/lib/Controller/RowOCSController.php b/lib/Controller/RowOCSController.php index a1922e0dca..20b9ae0ea6 100644 --- a/lib/Controller/RowOCSController.php +++ b/lib/Controller/RowOCSController.php @@ -18,7 +18,6 @@ use OCA\Tables\Errors\PermissionError; use OCA\Tables\Helper\ConversionHelper; use OCA\Tables\Middleware\Attribute\RequirePermission; -use OCA\Tables\Model\FilterInput; use OCA\Tables\Model\RowDataInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\FederationService; @@ -118,26 +117,28 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat } /** - * [api v2] get a number of rows from a table or view + * [api v2] Get a number of rows from a table or view * - * When reading from views, the specified filter is added to each existing - * filter group. + * Both `filter` and `sort` are passed as JSON encoded strings. * - * The filter definitions provided are all AND-connected. + * The filter is a list of filter groups, each group being a list of single + * filter definitions. Definitions within a group are AND-connected, while + * the groups themselves are OR-connected. * - * Sort orders on the other hand do overwrite the view's default sort order. - * Only when `null` is passed the default sort order will be used. + * When reading from a view, the provided filter is added to each of the + * view's existing filter groups, so the view's base rules are always + * enforced. * - * @param 'tables'|'views' $nodeCollection Indicates whether to get rows - * from a table or view + * A provided sort order overrides the view's default sort order. The view's + * default sort order is only used when no sort order is provided. + * + * @param 'tables'|'views' $nodeCollection Indicates whether to read from a table or a view * @psalm-param int<0,max> $nodeId The ID of the table or view * @psalm-param ?int<1,500> $limit Number of rows to return between 1 and 500, fetches all by default (optional) * @psalm-param ?int<0,max> $offset Offset of the rows to be returned (optional) - * @param ?string $filter Additional row filter as JSON-encoded filter groups (optional) - * @param list|null $sort Custom sort order (optional) - * @return DataResponse, - * array{}>|DataResponse + * @param ?string $filter JSON encoded list of filter groups. Definitions within a group are AND-connected, groups are OR-connected, e.g. `[[{"columnId":1,"operator":"contains","value":"foo"}]]` (optional) + * @param ?string $sort JSON encoded list of sort rules, e.g. `[{"columnId":1,"mode":"ASC"}]` (optional) + * @return DataResponse, array{}>|DataResponse * * 200: Rows returned * 400: Invalid request parameters @@ -150,14 +151,9 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat #[ApiRoute( verb: 'GET', url: '/api/2/{nodeCollection}/{nodeId}/rows', - requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)'] + requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\\d+)'] )] - public function getRows(string $nodeCollection, int $nodeId, ?int $limit, ?int $offset, mixed $filter = null, ?array $sort = null): DataResponse { - $queryData = new RowQuery( - nodeType: $nodeCollection === 'tables' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, - nodeId: $nodeId, - ); - + public function getRows(string $nodeCollection, int $nodeId, ?int $limit = null, ?int $offset = null, ?string $filter = null, ?string $sort = null): DataResponse { try { if (($limit !== null && ($limit <= 0 || $limit > 500)) || ($offset !== null && $offset < 0) @@ -165,36 +161,28 @@ public function getRows(string $nodeCollection, int $nodeId, ?int $limit, ?int $ throw new InvalidArgumentException('Offset or limit parameter is out of bounds'); } - $filterInput = FilterInput::fromRequestValue($filter); - $filterGroups = $filterInput->filter ?: null; - if ($filterGroups) { - foreach ($filterGroups as $filterGroup) { - foreach ($filterGroup as $singleFilter) { - $this->assertFilterValue($singleFilter); - } - } - } - if ($sort) { - foreach ($sort as $singleSortRule) { - $this->assertSortValue($singleSortRule); - } - } + $queryData = new RowQuery( + nodeType: $nodeCollection === 'tables' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, + nodeId: $nodeId, + ); $queryData->setLimit($limit) ->setOffset($offset) - ->setFilter($filterGroups) - ->setSort($sort) + // the provided filter is set here; any filter defined on a view + // is merged in on the service level + ->setFilter($this->parseFilter($filter)) + ->setSort($this->parseSort($sort)) ->setUserId($this->userId); $rows = $this->rowService->findAllByQuery($queryData); return new DataResponse($this->rowService->formatRows($rows)); } catch (PermissionError $e) { return $this->handlePermissionError($e); - } catch (InternalError|Exception $e) { - return $this->handleError($e); } catch (DoesNotExistException $e) { return $this->handleNotFoundError(new NotFoundError($e->getMessage(), $e->getCode(), $e)); } catch (MultipleObjectsReturnedException|InvalidArgumentException $e) { return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); + } catch (InternalError|Exception $e) { + return $this->handleError($e); } } @@ -293,20 +281,70 @@ public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): Data } /** - * @param array{columnId: int, operator: 'begins-with'|'ends-with'|'contains'|'does-not-contain'|'is-equal'|'is-not-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal'|'is-empty',value: string|int|float} $filter + * Decode and validate the JSON encoded filter parameter. + * + * @return list>|null + * @throws InvalidArgumentException + */ + protected function parseFilter(?string $filter): ?array { + if ($filter === null || $filter === '') { + return null; + } + $filterInput = FilterInput::fromRequestValue($filter); + $decoded = $filterInput->filter; + if (!is_array($decoded)) { + throw new InvalidArgumentException('Invalid filter supplied'); + } + foreach ($decoded as $filterGroup) { + if (!is_array($filterGroup)) { + throw new InvalidArgumentException('Invalid filter supplied'); + } + foreach ($filterGroup as $singleFilter) { + $this->assertFilterValue($singleFilter); + } + } + return $decoded; + } + + /** + * Decode and validate the JSON encoded sort parameter. + * + * @return list|null + * @throws InvalidArgumentException + */ + protected function parseSort(?string $sort): ?array { + if ($sort === null || $sort === '') { + return null; + } + $decoded = json_decode($sort, true); + if (!is_array($decoded)) { + throw new InvalidArgumentException('Invalid sort data supplied'); + } + foreach ($decoded as $singleSortRule) { + $this->assertSortValue($singleSortRule); + } + return $decoded; + } + + /** + * @throws InvalidArgumentException */ - protected function assertFilterValue(array $filter): void { - if (!isset($filter['columnId'], $filter['operator'], $filter['value']) + protected function assertFilterValue(mixed $filter): void { + if (!is_array($filter) + || !isset($filter['columnId'], $filter['operator'], $filter['value']) || count($filter) !== 3 ) { throw new InvalidArgumentException('Invalid filter supplied'); } + // values higher than PHP_INT_MAX will be capped to PHP_INT_MAX on cast, + // checking it roughly is sufficient. + // the lower value boundary is the lowest meta column id in \OCA\Tables\Db\Column $maxDigits = strlen((string)PHP_INT_MAX); if (!is_numeric($filter['columnId']) || (int)$filter['columnId'] < -5 - || !preg_match('/^\d{0,' . $maxDigits . '}$/', (string)$filter['columnId']) + || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$filter['columnId']) ) { - throw new InvalidArgumentException(sprintf('Invalid column id supplied: %d', $filter['columnId'])); + throw new InvalidArgumentException(sprintf('Invalid column id supplied: %s', (string)$filter['columnId'])); } if (!in_array($filter['operator'], [ 'begins-with', @@ -326,18 +364,22 @@ protected function assertFilterValue(array $filter): void { } /** - * @param array{columnId: int, mode: 'ASC'|'DESC'} $sort + * @throws InvalidArgumentException */ - protected function assertSortValue(array $sort): void { - if (!isset($sort['columnId'], $sort['mode']) + protected function assertSortValue(mixed $sort): void { + if (!is_array($sort) + || !isset($sort['columnId'], $sort['mode']) || count($sort) !== 2 ) { throw new InvalidArgumentException('Invalid sort data supplied'); } + // values higher than PHP_INT_MAX will be capped to PHP_INT_MAX on cast, + // checking it roughly is sufficient. + // the lower value boundary is the lowest meta column id in \OCA\Tables\Db\Column $maxDigits = strlen((string)PHP_INT_MAX); if (!is_numeric($sort['columnId']) || (int)$sort['columnId'] < -5 - || !preg_match('/^\d{0,' . $maxDigits . '}$/', (string)$sort['columnId']) + || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$sort['columnId']) ) { throw new InvalidArgumentException('Invalid column id supplied'); } diff --git a/lib/Db/ContextMapper.php b/lib/Db/ContextMapper.php index 3b677afc64..0da199335e 100644 --- a/lib/Db/ContextMapper.php +++ b/lib/Db/ContextMapper.php @@ -103,9 +103,7 @@ protected function formatResultRows(array $rows, ?string $userId) { 'display_mode_default' => (int)$item['display_mode_default'], ]; if ($userId !== null) { - if ($item['display_mode'] === null) { - $item['display_mode'] = $item['display_mode_default']; - } + $item['display_mode'] ??= $item['display_mode_default']; $carry[$item['share_id']]['display_mode'] = (int)$item['display_mode']; } return $carry; @@ -130,9 +128,7 @@ protected function formatResultRows(array $rows, ?string $userId) { // empty Context return $carry; } - if (!isset($carry[$item['page_id']])) { - $carry[$item['page_id']] = ['content' => []]; - } + $carry[$item['page_id']] ??= ['content' => []]; $carry[$item['page_id']]['id'] = (int)$item['page_id']; $carry[$item['page_id']]['page_type'] = $item['page_type']; if ($item['node_rel_id'] !== null) { diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 6e0d08ff3f..93c10c0f6c 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -173,6 +173,7 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset); + // Get rows without SQL sorting $rows = $this->getRows($wantedRowIdsArray, $showColumnIds); // Sort rows in PHP to preserve the order from getWantedRowIds @@ -669,9 +670,7 @@ private function parseEntities(IResult $result, array $sleeves): array { $column = $this->columnMapper->find($rowData['column_id']); $columnType = $column->getType(); - if (!isset($cellMapperCache[$columnType])) { - $cellMapperCache[$columnType] = $this->getCellMapperFromType($columnType); - } + $cellMapperCache[$columnType] ??= $this->getCellMapperFromType($columnType); $value = $cellMapperCache[$columnType]->formatRowData($column, $rowData); $compositeKey = (string)$rowData['row_id'] . ',' . (string)$rowData['column_id']; if ($cellMapperCache[$columnType]->hasMultipleValues()) { diff --git a/lib/Db/RowQuery.php b/lib/Db/RowQuery.php index 93b25024b9..c307071b93 100644 --- a/lib/Db/RowQuery.php +++ b/lib/Db/RowQuery.php @@ -6,6 +6,7 @@ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ + namespace OCA\Tables\Db; class RowQuery { diff --git a/lib/Migration/Version2020Date20260513185340.php b/lib/Migration/Version2020Date20260513185340.php index 067989d001..6e91736462 100644 --- a/lib/Migration/Version2020Date20260513185340.php +++ b/lib/Migration/Version2020Date20260513185340.php @@ -74,9 +74,7 @@ private function applyColumnOptionsUpdateIfNecessary(IQueryBuilder $query, int $ } foreach ($selectionOptions as &$selectionOption) { - if (!isset($selectionOption['uuid'])) { - $selectionOption['uuid'] = Uuid::v7()->toRfc4122(); - } + $selectionOption['uuid'] ??= Uuid::v7()->toRfc4122(); } $updatedSelectionOptions = json_encode($selectionOptions); diff --git a/lib/Migration/Version2202Date20260825184226.php b/lib/Migration/Version2202Date20260825184226.php index f42aec3443..7cc99b0343 100644 --- a/lib/Migration/Version2202Date20260825184226.php +++ b/lib/Migration/Version2202Date20260825184226.php @@ -88,9 +88,7 @@ private function applyColumnOptionsUpdateIfNecessary(IQueryBuilder $query, int $ } foreach ($selectionOptions as &$selectionOption) { - if (!isset($selectionOption['uuid'])) { - $selectionOption['uuid'] = Uuid::v7()->toRfc4122(); - } + $selectionOption['uuid'] ??= Uuid::v7()->toRfc4122(); } unset($selectionOption); diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 52a84b03c1..47c88240e1 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -614,9 +614,7 @@ public function delete(int $id, bool $skipRowCleanup = false, ?string $userId = public function findOrCreateColumnsByTitleForTableAsArray(?int $tableId, ?int $viewId, array $titles, array $dataTypes, ?string $userId, bool $createUnknownColumns, int &$countCreatedColumns, int &$countMatchingColumns): array { $result = []; - if ($userId === null) { - $userId = $this->userId; - } + $userId ??= $this->userId; if ($viewId) { $allColumns = $this->findAllByView($viewId, $userId); } elseif ($tableId) { @@ -638,10 +636,7 @@ public function findOrCreateColumnsByTitleForTableAsArray(?int $tableId, ?int $v } $result[$i] = ''; } - // if there are no columns at all - if (!isset($result[$i])) { - $result[$i] = ''; - } + $result[$i] ??= ''; // if column was not found if ($result[$i] === '' && $createUnknownColumns && $dataTypes[$i]['type'] !== Column::TYPE_META_ID) { $description = $this->l->t('This column was automatically created by the import service.'); diff --git a/lib/Service/PermissionsService.php b/lib/Service/PermissionsService.php index d75479c0d0..dc68798e23 100644 --- a/lib/Service/PermissionsService.php +++ b/lib/Service/PermissionsService.php @@ -64,9 +64,7 @@ public function __construct( * @throws InternalError */ public function preCheckUserId(?string $userId = null, bool $canBeEmpty = true): string { - if ($userId === null) { - $userId = $this->userId; - } + $userId ??= $this->userId; if ($userId === null) { $e = new \Exception(); diff --git a/lib/Service/RelationService.php b/lib/Service/RelationService.php index 5928e13727..6d0ca9fade 100644 --- a/lib/Service/RelationService.php +++ b/lib/Service/RelationService.php @@ -104,9 +104,7 @@ private function groupColumnsByTarget(array $columns): array { } $target = sprintf('%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn']); - if (!isset($groups[$target])) { - $groups[$target] = []; - } + $groups[$target] ??= []; $groups[$target][] = $column; } diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index bd0d9a571a..8bcb1f1bda 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -14,6 +14,7 @@ use OCA\Tables\Db\Row2; use OCA\Tables\Db\Row2Mapper; use OCA\Tables\Db\RowQuery; +use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; @@ -87,46 +88,37 @@ public function formatRowsForPublicShare(array $rows): array { } /** - * @throws MultipleObjectsReturnedException + * Fetch rows for a table or view, applying the given filter and sort rules. + * + * When reading from a view, the provided filter is added to each of the + * view's filter groups so that the view's base rules are always enforced. + * A provided sort order overrides the view's default sort order; the view + * default is only used when no sort order is given. + * + * @return Row2[] * @throws DoesNotExistException - * @throws Exception + * @throws MultipleObjectsReturnedException * @throws InternalError - * @return Row2[] */ public function findAllByQuery(RowQuery $rowQuery): array { $tableId = $rowQuery->getNodeId(); $columns = null; + $userId = $rowQuery->getUserId() ?? $this->userId ?? ''; + $filter = $rowQuery->getFilter(); + $sort = $rowQuery->getSort(); if ($rowQuery->getNodeType() === Application::NODE_TYPE_VIEW) { $view = $this->viewMapper->find($rowQuery->getNodeId()); $tableId = $view->getTableId(); $columns = $this->columnMapper->findAll($view->getColumnsArray()); - $userId = $this->resolveFilterUserId($rowQuery->getUserId() ?? $this->userId ?? '', $view); - $rowQuery->setUserId($userId); - - if ($rowQuery->getFilter() !== null) { - $baseFilterGroups = $view->getFilterArray(); - if (empty($baseFilterGroups)) { - $baseFilterGroups = $rowQuery->getFilter(); - } else { - $additionalFilterRules = array_merge(...$rowQuery->getFilter()); - foreach ($baseFilterGroups as &$baseFilterGroup) { - array_push($baseFilterGroup, ...$additionalFilterRules); - } - unset($baseFilterGroup); - } - $rowQuery->setFilter($baseFilterGroups); - } else { - $rowQuery->setFilter($view->getFilterArray()); - } + $userId = $this->resolveFilterUserId($userId, $view); + $filter = $this->mergeFilterWithViewFilter($filter, $view->getFilterArray()); - if ($rowQuery->getSort() === null) { - $rowQuery->setSort($view->getSortArray()); - } - } elseif ($rowQuery->getSort() === null) { + $sort ??= $view->getSortArray(); + } elseif ($sort === null) { $table = $this->tableMapper->find($tableId); - $rowQuery->setSort($table->getSortArray() ?: null); + $sort = $table->getSortArray() ?: null; } $tableColumns = $this->columnMapper->findAllByTable($tableId); @@ -138,14 +130,42 @@ public function findAllByQuery(RowQuery $rowQuery): array { $tableId, $rowQuery->getLimit(), $rowQuery->getOffset(), - $rowQuery->getFilter(), - $rowQuery->getSort(), - $rowQuery->getUserId() ?? $this->userId ?? '', + $filter, + $sort, + $userId, ); $this->attachAliasPayloads($rows, $columns); return $rows; } + /** + * Combine a user supplied filter with a view's base filter. + * + * A filter is a list of OR-connected groups, each group being a list of + * AND-connected conditions. To enforce the view's rules while also applying + * the user's filter, the user's conditions are appended to every base + * group, resulting in (group AND userFilter) OR ... When the view has no + * base filter, the user's filter is used as-is. + * + * @param list>|null $filter + * @param list> $viewFilter + * @return list> + */ + private function mergeFilterWithViewFilter(?array $filter, array $viewFilter): array { + if ($filter === null || $filter === []) { + return $viewFilter; + } + if ($viewFilter === []) { + return $filter; + } + $userConditions = array_merge(...$filter); + $merged = []; + foreach ($viewFilter as $group) { + $merged[] = array_merge($group, $userConditions); + } + return $merged; + } + /** * @param int $tableId * @param string $userId @@ -722,9 +742,7 @@ public function updateSet( throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } } else { - if ($tableId === null) { - $tableId = $item->getTableId(); - } + $tableId ??= $item->getTableId(); if ($tableId !== $item->getTableId()) { $e = new \Exception('Row does not belong to table with id ' . $tableId); $this->logger->error($e->getMessage(), ['exception' => $e]); @@ -913,9 +931,6 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { * We do not check if you are allowed to remove this data. That has to be * done before! Why? Mostly this check will have be run before and we can * pass this here due to performance reasons. - * We do not check if you are allowed to remove this data. That has to be - * done before! Why? Mostly this check will have be run before and we can - * pass this here due to performance reasons. * * @param Column $column * @throws InternalError diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index acc7d24fca..5d5b704cbd 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -120,9 +120,7 @@ public function findAll(?string $userId = null, bool $skipTableEnhancement = fal // clean duplicates foreach ($sharedTables as $sharedTable) { - if (!isset($allTables[$sharedTable->getId()])) { - $allTables[$sharedTable->getId()] = $sharedTable; - } + $allTables[$sharedTable->getId()] ??= $sharedTable; } } diff --git a/openapi.json b/openapi.json index 02e9527832..64ccf69fe7 100644 --- a/openapi.json +++ b/openapi.json @@ -16034,8 +16034,8 @@ }, "get": { "operationId": "rowocs-get-rows", - "summary": "[api v2] get a number of rows from a table or view", - "description": "When reading from views, the specified filter is added to each existing filter group.\nThe filter definitions provided are all AND-connected.\nSort orders on the other hand do overwrite the view's default sort order. Only when `null` is passed the default sort order will be used.", + "summary": "[api v2] Get a number of rows from a table or view", + "description": "Both `filter` and `sort` are passed as JSON encoded strings.\nThe filter is a list of filter groups, each group being a list of single filter definitions. Definitions within a group are AND-connected, while the groups themselves are OR-connected.\nWhen reading from a view, the provided filter is added to each of the view's existing filter groups, so the view's base rules are always enforced.\nA provided sort order overrides the view's default sort order. The view's default sort order is only used when no sort order is provided.", "tags": [ "rowocs" ], @@ -16051,7 +16051,7 @@ { "name": "nodeCollection", "in": "path", - "description": "Indicates whether to get rows from a table or view", + "description": "Indicates whether to read from a table or a view", "required": true, "schema": { "type": "string", @@ -16081,6 +16081,7 @@ "type": "integer", "format": "int64", "nullable": true, + "default": null, "minimum": 1, "maximum": 500 } @@ -16093,13 +16094,14 @@ "type": "integer", "format": "int64", "nullable": true, + "default": null, "minimum": 0 } }, { "name": "filter", "in": "query", - "description": "Additional row filter as JSON-encoded filter groups (optional)", + "description": "JSON encoded list of filter groups. Definitions within a group are AND-connected, groups are OR-connected, e.g. `[[{\"columnId\":1,\"operator\":\"contains\",\"value\":\"foo\"}]]` (optional)", "schema": { "type": "string", "nullable": true, @@ -16107,33 +16109,13 @@ } }, { - "name": "sort[]", + "name": "sort", "in": "query", - "description": "Custom sort order (optional)", + "description": "JSON encoded list of sort rules, e.g. `[{\"columnId\":1,\"mode\":\"ASC\"}]` (optional)", "schema": { - "type": "array", + "type": "string", "nullable": true, - "default": null, - "items": { - "type": "object", - "required": [ - "columnId", - "mode" - ], - "properties": { - "columnId": { - "type": "integer", - "format": "int64" - }, - "mode": { - "type": "string", - "enum": [ - "ASC", - "DESC" - ] - } - } - } + "default": null } }, { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index f10c7d23df..c92bc84321 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1025,10 +1025,11 @@ export type paths = { readonly cookie?: never; }; /** - * [api v2] get a number of rows from a table or view - * @description When reading from views, the specified filter is added to each existing filter group. - * The filter definitions provided are all AND-connected. - * Sort orders on the other hand do overwrite the view's default sort order. Only when `null` is passed the default sort order will be used. + * [api v2] Get a number of rows from a table or view + * @description Both `filter` and `sort` are passed as JSON encoded strings. + * The filter is a list of filter groups, each group being a list of single filter definitions. Definitions within a group are AND-connected, while the groups themselves are OR-connected. + * When reading from a view, the provided filter is added to each of the view's existing filter groups, so the view's base rules are always enforced. + * A provided sort order overrides the view's default sort order. The view's default sort order is only used when no sort order is provided. */ readonly get: operations["rowocs-get-rows"]; readonly put?: never; @@ -8503,22 +8504,17 @@ export interface operations { readonly limit?: number | null; /** @description Offset of the rows to be returned (optional) */ readonly offset?: number | null; - /** @description Additional row filter as JSON-encoded filter groups (optional) */ + /** @description JSON encoded list of filter groups. Definitions within a group are AND-connected, groups are OR-connected, e.g. `[[{"columnId":1,"operator":"contains","value":"foo"}]]` (optional) */ readonly filter?: string | null; - /** @description Custom sort order (optional) */ - readonly "sort[]"?: readonly { - /** Format: int64 */ - readonly columnId: number; - /** @enum {string} */ - readonly mode: "ASC" | "DESC"; - }[] | null; + /** @description JSON encoded list of sort rules, e.g. `[{"columnId":1,"mode":"ASC"}]` (optional) */ + readonly sort?: string | null; }; readonly header: { /** @description Required to be true for the API request to pass */ readonly "OCS-APIRequest": boolean; }; readonly path: { - /** @description Indicates whether to get rows from a table or view */ + /** @description Indicates whether to read from a table or a view */ readonly nodeCollection: "tables" | "views"; /** @description The ID of the table or view */ readonly nodeId: number; diff --git a/tests/integration/features/RowOCS.feature b/tests/integration/features/RowOCS.feature index 302ee043ea..1a71c84d3d 100644 --- a/tests/integration/features/RowOCS.feature +++ b/tests/integration/features/RowOCS.feature @@ -139,7 +139,7 @@ Feature: RowOCS And 15 rows have been loaded @tables @views - Scenario Outline: Get rows from a table or view With Offset + Scenario Outline: Get rows from a table or view with an out-of-bounds offset Given as user "" When the current user fetches rows from "" "" with those parameters | offset | | @@ -154,7 +154,7 @@ Feature: RowOCS | participant3-v2 | view | v1 | 200 | 200 | 0 | @tables @views - Scenario Outline: Get rows from a table or view With Offset + Scenario Outline: Get rows from a table or view with an offset Given as user "" When the current user fetches rows from "" "" with those parameters | offset | 5 | @@ -168,7 +168,7 @@ Feature: RowOCS | participant3-v2 | view | v1 | @tables @views - Scenario Outline: Get rows from a table or view With Offset + Scenario Outline: Get rows from a table or view with an out-of-bounds limit Given as user "" When the current user fetches rows from "" "" with those parameters | limit | | @@ -185,7 +185,7 @@ Feature: RowOCS | participant3-v2 | view | v1 | 555 | 400 | 0 | @tables @views - Scenario Outline: Get rows from a table or view With Offset + Scenario Outline: Get rows from a table or view with a limit Given as user "" When the current user fetches rows from "" "" with those parameters | limit | 5 | @@ -198,16 +198,101 @@ Feature: RowOCS | participant2-v2 | table | t1 | | participant3-v2 | view | v1 | - @tables @views @current - Scenario Outline: Get rows from a table or view with a filter + @tables @views + Scenario Outline: Get rows from a table or view with a text filter Given as user "" When the current user fetches rows from "" "" with those parameters | filter | one,contains,t | Then the reported status is 200 And 8 rows have been loaded - And rows "r6,r7,r8,r9,r10,r11,r12,r13,r14,r15" are not included in the response + And rows "r2,r3,r8,r10,r12,r13,r14,r15" are included in the response + And rows "r1,r4,r5,r6,r7,r9,r11" are not included in the response + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Get rows from a table or view with a numeric filter + Given as user "" + When the current user fetches rows from "" "" with those parameters + | filter | two,is-greater-than,2000 | + Then the reported status is 200 + And 5 rows have been loaded + And rows "r11,r12,r13,r14,r15" are included in the response + And rows "r1,r2,r3,r4,r5,r6,r7,r8,r9,r10" are not included in the response + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Get rows from a table or view sorted descending + Given as user "" + When the current user fetches rows from "" "" with those parameters + | sort | two,DESC | + | limit | 3 | + Then the reported status is 200 + And 3 rows have been loaded + And the rows are returned in the order "r15,r14,r13" Examples: | user | type | alias | | participant2-v2 | table | t1 | | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Get rows from a table or view sorted ascending + Given as user "" + When the current user fetches rows from "" "" with those parameters + | sort | two,ASC | + | limit | 3 | + Then the reported status is 200 + And 3 rows have been loaded + And the rows are returned in the order "r1,r2,r3" + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Reject an invalid filter operator + Given as user "" + When the current user fetches rows from "" "" with those parameters + | filter | one,not-an-operator,t | + Then the reported status is 400 + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @tables @views + Scenario Outline: Reject an invalid sort mode + Given as user "" + When the current user fetches rows from "" "" with those parameters + | sort | two,SIDEWAYS | + Then the reported status is 400 + + Examples: + | user | type | alias | + | participant2-v2 | table | t1 | + | participant3-v2 | view | v1 | + + @views + Scenario: A view's base filter is combined with the request filter + Given user "participant1-v2" create view "v2" with emoji "🔎" for "t1" as "v2" + And user "participant1-v2" sets filter to view "v2" + | column | operator | value | + | two | is-greater-than | 1700 | + And user "participant1-v2" shares view "v2" with "participant3-v2" + And as user "participant3-v2" + When the current user fetches rows from "view" "v2" with those parameters + | filter | one,contains,t | + Then the reported status is 200 + And 6 rows have been loaded + And rows "r8,r10,r12,r13,r14,r15" are included in the response + And rows "r2,r3" are not included in the response diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 278441b330..cde7a6a42c 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -2140,9 +2140,7 @@ public function sendRequestFullUrl($verb, $fullUrl, $body = null, array $headers } protected function getUserCookieJar($user) { - if (!isset($this->cookieJars[$user])) { - $this->cookieJars[$user] = new CookieJar(); - } + $this->cookieJars[$user] ??= new CookieJar(); return $this->cookieJars[$user]; } @@ -3312,23 +3310,33 @@ public function theCurrentUserFetchesAllRowsFromCollection(string $nodeType, str public function theCurrentUserFetchesRowsFromWithThoseParameters(string $nodeType, string $nodeAlias, TableNode $parameters): void { $query = ''; foreach ($parameters->getRows() as $row) { - if ($row[0] !== 'filter') { - $parameterName = $row[0]; - $parameterValue = $row[1]; - } else { + $parameterName = $row[0]; + if ($parameterName === 'filter') { + // `,,` is turned into a single + // filter group holding a single filter definition [$columnAlias, $operator, $value] = explode(',', $row[1]); $columnId = $this->collectionManager->getByAlias('column', $columnAlias)['id']; - $filterArray = [ - [ - [ + $parameterValue = json_encode([ // all filter groups + [ // single filter group + [ // single filter definition 'columnId' => $columnId, 'operator' => $operator, 'value' => $value, ], ], - ]; - $parameterName = $row[0]; - $parameterValue = json_encode($filterArray); + ]); + } elseif ($parameterName === 'sort') { + // `,` is turned into a single sort rule + [$columnAlias, $mode] = explode(',', $row[1]); + $columnId = $this->collectionManager->getByAlias('column', $columnAlias)['id']; + $parameterValue = json_encode([ + [ + 'columnId' => $columnId, + 'mode' => $mode, + ], + ]); + } else { + $parameterValue = $row[1]; } $query .= $parameterName . '=' . urlencode($parameterValue) . '&'; } @@ -3355,7 +3363,7 @@ public function rowsHaveBeenLoaded(int $numberOfRows): void { * @Given rows :rowAliasList are not included in the response */ public function rowsAreNotIncludedInTheResponse(string $rowAliasList): void { - $unexpectedRowAliases = array_map('trim', explode(',', $rowAliasList)); + $unexpectedRowAliases = array_map(trim(...), explode(',', $rowAliasList)); $returnedRowIds = $this->collectionManager->getById('returnedRowIDs', 0); foreach ($unexpectedRowAliases as $unexpectedRowAlias) { $row = $this->collectionManager->getByAlias('row', $unexpectedRowAlias); @@ -3367,11 +3375,31 @@ public function rowsAreNotIncludedInTheResponse(string $rowAliasList): void { * @Given rows :rowAliasList are included in the response */ public function rowsAreIncludedInTheResponse(string $rowAliasList): void { - $expectedRowAliases = array_map('trim', explode(',', $rowAliasList)); + $expectedRowAliases = array_map(trim(...), explode(',', $rowAliasList)); $returnedRowIds = $this->collectionManager->getById('returnedRowIDs', 0); foreach ($expectedRowAliases as $expectedRowAlias) { $row = $this->collectionManager->getByAlias('row', $expectedRowAlias); Assert::assertContains($row['id'], $returnedRowIds); } } + + /** + * @Then the rows are returned in the order :rowAliasList + */ + public function theRowsAreReturnedInTheOrder(string $rowAliasList): void { + $expectedRowAliases = array_map(trim(...), explode(',', $rowAliasList)); + $responseData = $this->getDataFromResponse($this->response)['ocs']['data']; + // do not count the error message, if present + unset($responseData['message']); + $responseData = array_values($responseData); + Assert::assertCount(count($expectedRowAliases), $responseData); + foreach ($expectedRowAliases as $index => $expectedRowAlias) { + $expectedRow = $this->collectionManager->getByAlias('row', $expectedRowAlias); + Assert::assertSame( + $expectedRow['id'], + $responseData[$index]['id'], + sprintf('Row at position %d does not match the expected row "%s"', $index, $expectedRowAlias), + ); + } + } } diff --git a/vendor-bin/rector/composer.lock b/vendor-bin/rector/composer.lock index 9a772f14ea..12549c1ef4 100644 --- a/vendor-bin/rector/composer.lock +++ b/vendor-bin/rector/composer.lock @@ -9,16 +9,16 @@ "packages-dev": [ { "name": "nextcloud/ocp", - "version": "v34.0.1", + "version": "v34.0.3", "source": { "type": "git", "url": "https://github.com/nextcloud-deps/ocp.git", - "reference": "3f920a7f46bae0c55643579ce25b6644a09065ff" + "reference": "3fb764be792476e4dcf1593101d978fc1dc8ac9a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/3f920a7f46bae0c55643579ce25b6644a09065ff", - "reference": "3f920a7f46bae0c55643579ce25b6644a09065ff", + "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/3fb764be792476e4dcf1593101d978fc1dc8ac9a", + "reference": "3fb764be792476e4dcf1593101d978fc1dc8ac9a", "shasum": "" }, "require": { @@ -52,9 +52,9 @@ "description": "Composer package containing Nextcloud's public OCP API and the unstable NCU API", "support": { "issues": "https://github.com/nextcloud-deps/ocp/issues", - "source": "https://github.com/nextcloud-deps/ocp/tree/v34.0.1" + "source": "https://github.com/nextcloud-deps/ocp/tree/v34.0.3" }, - "time": "2026-06-18T02:35:58+00:00" + "time": "2026-08-07T02:03:36+00:00" }, { "name": "nextcloud/rector", @@ -493,16 +493,16 @@ }, { "name": "rector/rector", - "version": "2.6.3", + "version": "2.6.4", "source": { "type": "git", "url": "https://github.com/rectorphp/rector.git", - "reference": "7e46709996a4b3dc59e1d6ecbb6a38ace335bd58" + "reference": "6ff008471683591224951526247b7a2b86980ba9" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/rectorphp/rector/zipball/7e46709996a4b3dc59e1d6ecbb6a38ace335bd58", - "reference": "7e46709996a4b3dc59e1d6ecbb6a38ace335bd58", + "url": "https://api.github.com/repos/rectorphp/rector/zipball/6ff008471683591224951526247b7a2b86980ba9", + "reference": "6ff008471683591224951526247b7a2b86980ba9", "shasum": "" }, "require": { @@ -541,7 +541,7 @@ ], "support": { "issues": "https://github.com/rectorphp/rector/issues", - "source": "https://github.com/rectorphp/rector/tree/2.6.3" + "source": "https://github.com/rectorphp/rector/tree/2.6.4" }, "funding": [ { @@ -549,7 +549,7 @@ "type": "github" } ], - "time": "2026-08-18T22:01:18+00:00" + "time": "2026-08-27T09:20:34+00:00" }, { "name": "webmozart/assert", From 4aaf1187b910aa8a2069068276f1815f7172c2ef Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Sun, 30 Aug 2026 00:50:56 +0100 Subject: [PATCH 3/4] fix: refactor function Signed-off-by: Enjeck C. --- lib/Controller/RowOCSController.php | 86 ++--------------------------- lib/Model/FilterGroup.php | 14 +++++ lib/Model/FilterInput.php | 2 +- lib/Model/FilterSet.php | 3 + lib/Model/SortRuleSet.php | 14 +++++ lib/Service/ValueObject/Filter.php | 2 +- 6 files changed, 38 insertions(+), 83 deletions(-) diff --git a/lib/Controller/RowOCSController.php b/lib/Controller/RowOCSController.php index 20b9ae0ea6..8b2b9d5e2a 100644 --- a/lib/Controller/RowOCSController.php +++ b/lib/Controller/RowOCSController.php @@ -18,7 +18,10 @@ use OCA\Tables\Errors\PermissionError; use OCA\Tables\Helper\ConversionHelper; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\FilterInput; +use OCA\Tables\Model\FilterSet; use OCA\Tables\Model\RowDataInput; +use OCA\Tables\Model\SortRuleSet; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\FederationService; use OCA\Tables\Service\RowService; @@ -155,12 +158,6 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat )] public function getRows(string $nodeCollection, int $nodeId, ?int $limit = null, ?int $offset = null, ?string $filter = null, ?string $sort = null): DataResponse { try { - if (($limit !== null && ($limit <= 0 || $limit > 500)) - || ($offset !== null && $offset < 0) - ) { - throw new InvalidArgumentException('Offset or limit parameter is out of bounds'); - } - $queryData = new RowQuery( nodeType: $nodeCollection === 'tables' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, nodeId: $nodeId, @@ -295,15 +292,7 @@ protected function parseFilter(?string $filter): ?array { if (!is_array($decoded)) { throw new InvalidArgumentException('Invalid filter supplied'); } - foreach ($decoded as $filterGroup) { - if (!is_array($filterGroup)) { - throw new InvalidArgumentException('Invalid filter supplied'); - } - foreach ($filterGroup as $singleFilter) { - $this->assertFilterValue($singleFilter); - } - } - return $decoded; + return FilterSet::createFromInputArray($decoded)->jsonSerialize(); } /** @@ -320,71 +309,6 @@ protected function parseSort(?string $sort): ?array { if (!is_array($decoded)) { throw new InvalidArgumentException('Invalid sort data supplied'); } - foreach ($decoded as $singleSortRule) { - $this->assertSortValue($singleSortRule); - } - return $decoded; - } - - /** - * @throws InvalidArgumentException - */ - protected function assertFilterValue(mixed $filter): void { - if (!is_array($filter) - || !isset($filter['columnId'], $filter['operator'], $filter['value']) - || count($filter) !== 3 - ) { - throw new InvalidArgumentException('Invalid filter supplied'); - } - // values higher than PHP_INT_MAX will be capped to PHP_INT_MAX on cast, - // checking it roughly is sufficient. - // the lower value boundary is the lowest meta column id in \OCA\Tables\Db\Column - $maxDigits = strlen((string)PHP_INT_MAX); - if (!is_numeric($filter['columnId']) - || (int)$filter['columnId'] < -5 - || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$filter['columnId']) - ) { - throw new InvalidArgumentException(sprintf('Invalid column id supplied: %s', (string)$filter['columnId'])); - } - if (!in_array($filter['operator'], [ - 'begins-with', - 'ends-with', - 'contains', - 'does-not-contain', - 'is-equal', - 'is-not-equal', - 'is-greater-than', - 'is-greater-than-or-equal', - 'is-lower-than', - 'is-lower-than-or-equal', - 'is-empty', - ], true)) { - throw new InvalidArgumentException('Invalid filter operator supplied'); - } - } - - /** - * @throws InvalidArgumentException - */ - protected function assertSortValue(mixed $sort): void { - if (!is_array($sort) - || !isset($sort['columnId'], $sort['mode']) - || count($sort) !== 2 - ) { - throw new InvalidArgumentException('Invalid sort data supplied'); - } - // values higher than PHP_INT_MAX will be capped to PHP_INT_MAX on cast, - // checking it roughly is sufficient. - // the lower value boundary is the lowest meta column id in \OCA\Tables\Db\Column - $maxDigits = strlen((string)PHP_INT_MAX); - if (!is_numeric($sort['columnId']) - || (int)$sort['columnId'] < -5 - || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$sort['columnId']) - ) { - throw new InvalidArgumentException('Invalid column id supplied'); - } - if ($sort['mode'] !== 'DESC' && $sort['mode'] !== 'ASC') { - throw new InvalidArgumentException('Invalid sort mode supplied'); - } + return SortRuleSet::createFromInputArray($decoded)->jsonSerialize(); } } diff --git a/lib/Model/FilterGroup.php b/lib/Model/FilterGroup.php index f655ebbdc3..72f21e583e 100644 --- a/lib/Model/FilterGroup.php +++ b/lib/Model/FilterGroup.php @@ -36,6 +36,7 @@ public static function createFromInputArray(array $data): self { if (!isset($filterInput['columnId'], $filterInput['operator'], $filterInput['value'])) { throw new InvalidArgumentException('Required input fields are missing'); } + self::assertColumnIdInBounds($filterInput['columnId']); try { $filters[] = new Filter( (int)$filterInput['columnId'], @@ -49,6 +50,19 @@ public static function createFromInputArray(array $data): self { return new self($filters); } + /** + * @throws InvalidArgumentException + */ + private static function assertColumnIdInBounds(mixed $columnId): void { + $maxDigits = strlen((string)PHP_INT_MAX); + if (!is_numeric($columnId) + || (int)$columnId < -5 + || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$columnId) + ) { + throw new InvalidArgumentException(sprintf('Invalid column id supplied: %s', (string)$columnId)); + } + } + public function jsonSerialize(): array { return array_map(static fn (Filter $f) => $f->jsonSerialize(), $this->filters); } diff --git a/lib/Model/FilterInput.php b/lib/Model/FilterInput.php index 56b9824542..c1fb9220a5 100644 --- a/lib/Model/FilterInput.php +++ b/lib/Model/FilterInput.php @@ -22,7 +22,7 @@ private function __construct( ) { } - public static function fromRequestValue(mixed $value): self { + public static function fromRequestValue(string $value): self { if ($value === null || $value === '') { return new self([]); } diff --git a/lib/Model/FilterSet.php b/lib/Model/FilterSet.php index bf65c2fc6c..d4985e6c12 100644 --- a/lib/Model/FilterSet.php +++ b/lib/Model/FilterSet.php @@ -31,6 +31,9 @@ public function __construct( public static function createFromInputArray(array $data, array $columnsMap = []): self { $filterGroups = []; foreach ($data as $inputFilterGroup) { + if (!is_array($inputFilterGroup)) { + throw new InvalidArgumentException('Each filter group entry must be an array'); + } foreach ($inputFilterGroup as $j => $item) { if (isset($item['columnUuid']) && isset($columnsMap[$item['columnUuid']]) && $columnsMap[$item['columnUuid']] instanceof Column) { $inputFilterGroup[$j]['columnId'] = $columnsMap[$item['columnUuid']]->getId(); diff --git a/lib/Model/SortRuleSet.php b/lib/Model/SortRuleSet.php index f020f36725..9681fbaa32 100644 --- a/lib/Model/SortRuleSet.php +++ b/lib/Model/SortRuleSet.php @@ -52,6 +52,7 @@ public static function createFromInputArray(array $data, array $columnsMap = []) if (!isset($inputSortRule['columnId'], $inputSortRule['mode'])) { throw new InvalidArgumentException('Required sort parameters are missing'); } + self::assertColumnIdInBounds($inputSortRule['columnId']); $sortRules[] = new SortRule( columnId: (int)$inputSortRule['columnId'], @@ -61,6 +62,19 @@ public static function createFromInputArray(array $data, array $columnsMap = []) return new self($sortRules); } + /** + * @throws InvalidArgumentException + */ + private static function assertColumnIdInBounds(mixed $columnId): void { + $maxDigits = strlen((string)PHP_INT_MAX); + if (!is_numeric($columnId) + || (int)$columnId < -5 + || !preg_match('/^-?\\d{0,' . $maxDigits . '}$/', (string)$columnId) + ) { + throw new InvalidArgumentException(sprintf('Invalid column id supplied: %s', (string)$columnId)); + } + } + /** * @return list */ diff --git a/lib/Service/ValueObject/Filter.php b/lib/Service/ValueObject/Filter.php index 08c476241e..5e08f027f2 100644 --- a/lib/Service/ValueObject/Filter.php +++ b/lib/Service/ValueObject/Filter.php @@ -16,7 +16,7 @@ class Filter implements JsonSerializable { public function __construct( protected readonly int $columnId, protected readonly FilterOperator $operator, - protected readonly string $value, + protected readonly string|int|float $value, ) { } From b831dce1bf41788cb600a6085d69e41856161ad3 Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Sun, 30 Aug 2026 10:15:58 +0100 Subject: [PATCH 4/4] fix: offset check Signed-off-by: Enjeck C. --- lib/Controller/RowOCSController.php | 6 +++++ tests/integration/features/RowOCS.feature | 27 +++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lib/Controller/RowOCSController.php b/lib/Controller/RowOCSController.php index 8b2b9d5e2a..3e3a3a8763 100644 --- a/lib/Controller/RowOCSController.php +++ b/lib/Controller/RowOCSController.php @@ -158,6 +158,12 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat )] public function getRows(string $nodeCollection, int $nodeId, ?int $limit = null, ?int $offset = null, ?string $filter = null, ?string $sort = null): DataResponse { try { + if (($limit !== null && ($limit <= 0 || $limit > 500)) + || ($offset !== null && $offset < 0) + ) { + throw new InvalidArgumentException('Offset or limit parameter is out of bounds'); + } + $queryData = new RowQuery( nodeType: $nodeCollection === 'tables' ? Application::NODE_TYPE_TABLE : Application::NODE_TYPE_VIEW, nodeId: $nodeId, diff --git a/tests/integration/features/RowOCS.feature b/tests/integration/features/RowOCS.feature index 1a71c84d3d..fcf0f8ddc5 100644 --- a/tests/integration/features/RowOCS.feature +++ b/tests/integration/features/RowOCS.feature @@ -5,6 +5,33 @@ Feature: RowOCS Given user "participant1-v2" exists Given user "participant2-v2" exists Given user "participant3-v2" exists + Given user "alice" exists + Given user "bob" exists + Given user "clarence" exists + Given user "dany" exists + Given user "elias" exists + Given user "fran" exists + Given user "george" exists + Given user "hannah" exists + Given user "ines" exists + Given user "jamie" exists + Given user "kate" exists + Given user "lena" exists + Given user "moe" exists + Given user "nora" exists + Given user "otto" exists + Given user "pierre" exists + Given user "quinn" exists + Given user "roberta" exists + Given user "samir" exists + Given user "teresa" exists + Given user "udai" exists + Given user "vera" exists + Given user "xuan" exists + Given user "yvonne" exists + Given user "zara" exists + Given user "ahmad" exists + Given user "bertram" exists Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 And column "one" exists with following properties | type | text |