diff --git a/lib/BackgroundJob/BackfillRowUuids.php b/lib/BackgroundJob/BackfillRowUuids.php new file mode 100644 index 0000000000..243a29aa3e --- /dev/null +++ b/lib/BackgroundJob/BackfillRowUuids.php @@ -0,0 +1,108 @@ +setInterval(60); + $this->setAllowParallelRuns(false); + } + + public function run(mixed $argument): void { + $deadline = $this->time->getTime() + self::TIME_BUDGET; + $cursor = $this->config->getValueInt(Application::APP_ID, self::CURSOR_KEY, 0, true); + $select = $this->connection->getQueryBuilder(); + $select->select('id')->from('tables_row_sleeves') + ->where($select->expr()->isNull('uuid')) + ->andWhere($select->expr()->gt('id', $select->createParameter('cursor'))) + ->orderBy('id', 'ASC')->setMaxResults(self::BATCH_SIZE); + $update = $this->connection->getQueryBuilder(); + $update->update('tables_row_sleeves')->set('uuid', $update->createParameter('uuid')) + ->where($update->expr()->eq('id', $update->createParameter('id'))) + ->andWhere($update->expr()->isNull('uuid')); + + try { + do { + $select->setParameter('cursor', $cursor, IQueryBuilder::PARAM_INT); + $result = $select->executeQuery(); + $ids = array_map('intval', $result->fetchFirstColumn()); + $result->closeCursor(); + if ($ids === []) { + if ($cursor === 0) { + $this->jobList->remove(self::class); + $this->config->deleteKey(Application::APP_ID, self::CURSOR_KEY); + return; + } + $cursor = 0; + continue; + } + + $cursor = $this->processBatch($ids, $update, $deadline); + if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_SQLITE && $this->time->getTime() < $deadline) { + // Yield SQLite's single writer between committed batches. + usleep(self::SQLITE_BATCH_PAUSE_MICROSECONDS); + } + } while ($this->time->getTime() < $deadline); + // A lagging cursor only repeats a NULL scan, so one write per run spares the shared config cache. + $this->config->setValueInt(Application::APP_ID, self::CURSOR_KEY, $cursor, true); + } catch (\Throwable $e) { + $this->logger->error('Row UUID backfill will retry on its next run.', ['exception' => $e]); + } + } + + /** @param non-empty-list $ids */ + private function processBatch(array $ids, IQueryBuilder $update, int $deadline): int { + for ($attempt = 0; ; $attempt++) { + $this->connection->beginTransaction(); + try { + foreach ($ids as $id) { + $update->setParameter('id', $id, IQueryBuilder::PARAM_INT); + $update->setParameter('uuid', Uuid::v7()->toRfc4122()); + $update->executeStatement(); + if ($this->time->getTime() >= $deadline) { + break; + } + } + $this->connection->commit(); + return $id; + } catch (\Throwable $e) { + $this->connection->rollBack(); + // Only uuid changes in this statement, so any unique violation is a UUID collision. + if (!$e instanceof Exception || $e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION || $attempt >= 3) { + throw $e; + } + } + } + } +} diff --git a/lib/Controller/RowOCSController.php b/lib/Controller/RowOCSController.php index 13ddd4e946..aa29343e11 100644 --- a/lib/Controller/RowOCSController.php +++ b/lib/Controller/RowOCSController.php @@ -15,6 +15,7 @@ use OCA\Tables\Helper\ConversionHelper; use OCA\Tables\Middleware\Attribute\RequirePermission; use OCA\Tables\Model\RowDataInput; +use OCA\Tables\Model\RowIdentifier; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\FederationService; use OCA\Tables\Service\RowService; @@ -87,11 +88,11 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat try { if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) { $table = $this->tableService->find($nodeId, true); - return new DataResponse($this->federationService->createRow($table, $data)); + return new DataResponse($this->federationService->createRow($table, $data) + ['uuid' => null]); } if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) { $view = $this->viewService->find($nodeId, false, $this->userId); - return new DataResponse($this->federationService->createRow($view, $data)); + return new DataResponse($this->federationService->createRow($view, $data) + ['uuid' => null]); } return new DataResponse($this->rowService->create($tableId, $viewId, $newRowData)->jsonSerialize()); } catch (BadRequestError $e) { @@ -105,13 +106,49 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat } } + /** + * [api v2] Read a local row in a table or view + * + * @param 'tables'|'views' $nodeCollection The targeted node collection + * @param int $nodeId The table or view identifier + * @param string $rowId A positive decimal row ID or a hyphenated UUIDv7 + * @return DataResponse|DataResponse + * + * 200: Row returned + * 400: Invalid identifier or unsupported federated operation + * 403: No permissions + * 404: Not found in the requested node + * 500: Internal error + */ + #[NoAdminRequired] + #[RequirePermission(permission: Application::PERMISSION_READ, typeParam: 'nodeCollection')] + #[ApiRoute(verb: 'GET', url: '/api/2/{nodeCollection}/{nodeId}/rows/{rowId}', requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)'])] + public function getRow(string $nodeCollection, int $nodeId, string $rowId): DataResponse { + $iNodeType = ConversionHelper::stringNodeType2Const($nodeCollection); + try { + $identifier = $this->parseRowIdentifier($rowId); + if ($this->federationService->isNodeFederated($nodeId, ConversionHelper::constNodeType2String($iNodeType))) { + throw new BadRequestError('Single-row reads are not supported on federated nodes.'); + } + return new DataResponse($this->rowService->findByIdentifier($identifier, $iNodeType, $nodeId, $this->userId)->jsonSerialize()); + } catch (BadRequestError $e) { + return $this->handleBadRequestError($e); + } catch (PermissionError $e) { + return $this->handlePermissionError($e); + } catch (NotFoundError $e) { + return $this->handleNotFoundError($e); + } catch (\Exception $e) { + return $this->handleError($e); + } + } + /** * [api v2] Update a row in a table or a view * * @param 'tables'|'views' $nodeCollection Indicates whether to update a row on a table or view * @param int $nodeId The identifier of the targeted table or view - * @param int $rowId The identifier of the row to update - * @param string|array $data An array containing the column identifiers and their values + * @param string $rowId A positive decimal row ID or a hyphenated UUIDv7 + * @param string|list $data A list of cells containing columnId and value * @return DataResponse|DataResponse * * 200: Row updated @@ -123,7 +160,7 @@ public function createRow(string $nodeCollection, int $nodeId, mixed $data): Dat #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_UPDATE, typeParam: 'nodeCollection')] #[ApiRoute(verb: 'PUT', url: '/api/2/{nodeCollection}/{nodeId}/rows/{rowId}', requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)'])] - public function updateRow(string $nodeCollection, int $nodeId, int $rowId, mixed $data): DataResponse { + public function updateRow(string $nodeCollection, int $nodeId, string $rowId, mixed $data): DataResponse { if (is_string($data)) { $data = json_decode($data, true); } @@ -138,15 +175,20 @@ public function updateRow(string $nodeCollection, int $nodeId, int $rowId, mixed $viewId = $nodeId; } try { + $identifier = $this->parseRowIdentifier($rowId); + $rowData = $this->parseRowData($data); if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) { + $federatedRowId = $this->requireNumericRowId($identifier); $table = $this->tableService->find($nodeId, true); - return new DataResponse($this->federationService->updateRow($table, $rowId, $data)); + return new DataResponse($this->federationService->updateRow($table, $federatedRowId, $data) + ['uuid' => null]); } if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) { + $federatedRowId = $this->requireNumericRowId($identifier); $view = $this->viewService->find($nodeId, false, $this->userId); - return new DataResponse($this->federationService->updateRow($view, $rowId, $data)); + return new DataResponse($this->federationService->updateRow($view, $federatedRowId, $data) + ['uuid' => null]); } - return new DataResponse($this->rowService->updateSet($rowId, $viewId, $data, $this->userId, $tableId)->jsonSerialize()); + $localRowId = $this->rowService->resolveRowId($identifier, $iNodeType, $nodeId, $this->userId); + return new DataResponse($this->rowService->updateRowData($localRowId, $viewId, $rowData, $this->userId, $tableId)->jsonSerialize()); } catch (BadRequestError $e) { return $this->handleBadRequestError($e); } catch (NotFoundError $e) { @@ -163,10 +205,11 @@ public function updateRow(string $nodeCollection, int $nodeId, int $rowId, mixed * * @param 'tables'|'views' $nodeCollection Indicates whether to delete a row on a table or view * @param int $nodeId The identifier of the targeted table or view - * @param int $rowId The identifier of the row to delete - * @return DataResponse|DataResponse + * @param string $rowId A positive decimal row ID or a hyphenated UUIDv7 + * @return DataResponse|DataResponse * * 200: Row deleted + * 400: Invalid identifier or unsupported federated operation * 403: No permissions * 404: Not found * 500: Internal error @@ -174,7 +217,7 @@ public function updateRow(string $nodeCollection, int $nodeId, int $rowId, mixed #[NoAdminRequired] #[RequirePermission(permission: Application::PERMISSION_DELETE, typeParam: 'nodeCollection')] #[ApiRoute(verb: 'DELETE', url: '/api/2/{nodeCollection}/{nodeId}/rows/{rowId}', requirements: ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)'])] - public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): DataResponse { + public function deleteRow(string $nodeCollection, int $nodeId, string $rowId): DataResponse { $iNodeType = ConversionHelper::stringNodeType2Const($nodeCollection); $tableId = $viewId = null; if ($iNodeType === Application::NODE_TYPE_TABLE) { @@ -183,15 +226,21 @@ public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): Data $viewId = $nodeId; } try { + $identifier = $this->parseRowIdentifier($rowId); if ($tableId !== null && $this->federationService->isNodeFederated($tableId, 'table')) { + $federatedRowId = $this->requireNumericRowId($identifier); $table = $this->tableService->find($nodeId, true); - return new DataResponse($this->federationService->deleteRow($table, $rowId)); + return new DataResponse($this->federationService->deleteRow($table, $federatedRowId) + ['uuid' => null]); } if ($viewId !== null && $this->federationService->isNodeFederated($viewId, 'view')) { + $federatedRowId = $this->requireNumericRowId($identifier); $view = $this->viewService->find($nodeId, false, $this->userId); - return new DataResponse($this->federationService->deleteRow($view, $rowId)); + return new DataResponse($this->federationService->deleteRow($view, $federatedRowId) + ['uuid' => null]); } - return new DataResponse($this->rowService->delete($rowId, $viewId, $this->userId, $tableId)->jsonSerialize()); + $localRowId = $this->rowService->resolveRowId($identifier, $iNodeType, $nodeId, $this->userId); + return new DataResponse($this->rowService->delete($localRowId, $viewId, $this->userId, $tableId)->jsonSerialize()); + } catch (BadRequestError $e) { + return $this->handleBadRequestError($e); } catch (NotFoundError $e) { return $this->handleNotFoundError($e); } catch (PermissionError $e) { @@ -200,4 +249,36 @@ public function deleteRow(string $nodeCollection, int $nodeId, int $rowId): Data return $this->handleError($e); } } + + /** + * @throws BadRequestError + */ + private function parseRowIdentifier(string $rowId): RowIdentifier { + try { + return RowIdentifier::fromString($rowId); + } catch (\InvalidArgumentException $e) { + throw new BadRequestError($e->getMessage(), 0, $e); + } + } + + /** + * @throws BadRequestError + */ + private function parseRowData(array $data): RowDataInput { + try { + return RowDataInput::createFromInputArray($data); + } catch (\InvalidArgumentException $e) { + throw new BadRequestError($e->getMessage(), 0, $e); + } + } + + /** + * @throws BadRequestError + */ + private function requireNumericRowId(RowIdentifier $identifier): int { + if (!is_int($identifier->value)) { + throw new BadRequestError('Federated rows do not support UUID addressing.'); + } + return $identifier->value; + } } diff --git a/lib/Db/Row2.php b/lib/Db/Row2.php index 0755bb325a..67f93164bb 100644 --- a/lib/Db/Row2.php +++ b/lib/Db/Row2.php @@ -17,6 +17,7 @@ */ class Row2 implements JsonSerializable { private ?int $id = null; + private ?string $uuid = null; private ?int $tableId = null; private ?string $createdBy = null; private ?string $createdAt = null; @@ -36,6 +37,14 @@ public function setId(int $id): void { $this->id = $id; } + public function getUuid(): ?string { + return $this->uuid; + } + + public function setUuid(?string $uuid): void { + $this->uuid = $uuid; + } + public function getTableId(): ?int { return $this->tableId; } @@ -165,6 +174,7 @@ public function filterDataByAliasByColumns(array $columns): array { public function jsonSerialize(): array { return [ 'id' => $this->id, + 'uuid' => $this->uuid, 'tableId' => $this->tableId, 'createdBy' => $this->createdBy, 'createdAt' => $this->createdAt, diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index aeb7c59c8c..43b39f7c96 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -14,6 +14,7 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Helper\ColumnsHelper; use OCA\Tables\Helper\UserHelper; +use OCA\Tables\Model\RowIdentifier; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; @@ -43,6 +44,10 @@ public function __construct( ) { } + public function importSleeve(RowSleeve $sleeve): RowSleeve { + return $this->rowSleeveMapper->insertImported($sleeve); + } + /** * @param Row2 $row * @return Row2 @@ -124,13 +129,19 @@ public function setUserId(string $userId): void { * @return int[] * @throws InternalError */ - private function getWantedRowIds(string $userId, int $tableId, ?array $filter = null, ?array $sort = null, ?int $limit = null, ?int $offset = null): array { + private function getWantedRowIds(string $userId, int $tableId, ?array $filter = null, ?array $sort = null, ?int $limit = null, ?int $offset = null, ?RowIdentifier $identifier = null): array { $qb = $this->db->getQueryBuilder(); $qb->select('sleeves.id') ->from('tables_row_sleeves', 'sleeves') ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId, IQueryBuilder::PARAM_INT))); + if ($identifier !== null) { + $column = $identifier->isUuid() ? 'sleeves.uuid' : 'sleeves.id'; + $type = $identifier->isUuid() ? IQueryBuilder::PARAM_STR : IQueryBuilder::PARAM_INT; + $qb->andWhere($qb->expr()->eq($column, $qb->createNamedParameter($identifier->value, $type))); + } + if ($filter) { $this->addFilterToQuery($qb, $filter, $userId); } @@ -171,6 +182,10 @@ private function getWantedRowIds(string $userId, int $tableId, ?array $filter = * @throws InternalError */ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, ?int $offset = null, ?array $filter = null, ?array $sort = null, ?string $userId = null): array { + if ($showColumnIds === []) { + return []; + } + try { $this->columnMapper->preloadColumns($showColumnIds, $filter, $sort); @@ -194,7 +209,7 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, * @throws InternalError */ private function getRows(array $rowIds, array $columnIds): array { - if (empty($rowIds) || empty($columnIds)) { + if (empty($rowIds)) { return []; } @@ -212,6 +227,10 @@ private function getRows(array $rowIds, array $columnIds): array { * @throws InternalError */ private function getRowsChunk(array $rowIds, array $columnIds): array { + if ($columnIds === []) { + return $this->parseEntities(null, $this->rowSleeveMapper->findMultiple($rowIds)); + } + $qb = $this->db->getQueryBuilder(); $qbSqlForColumnTypes = null; @@ -712,16 +731,17 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col } /** - * @param IResult $result + * @param IResult|null $result * @param RowSleeve[] $sleeves * @return Row2[] * @throws InternalError */ - private function parseEntities(IResult $result, array $sleeves): array { + private function parseEntities(?IResult $result, array $sleeves): array { $rows = []; foreach ($sleeves as $sleeve) { $rows[$sleeve->getId()] = new Row2(); $rows[$sleeve->getId()]->setId($sleeve->getId()); + $rows[$sleeve->getId()]->setUuid($sleeve->getUuid()); $rows[$sleeve->getId()]->setCreatedBy($sleeve->getCreatedBy()); $rows[$sleeve->getId()]->setCreatedAt($sleeve->getCreatedAt()); $rows[$sleeve->getId()]->setLastEditBy($sleeve->getLastEditBy()); @@ -734,7 +754,7 @@ private function parseEntities(IResult $result, array $sleeves): array { $keyToRowId = []; $cellMapperCache = []; - while ($rowData = $result->fetchAssociative()) { + while ($result !== null && ($rowData = $result->fetchAssociative())) { if (!isset($rowData['row_id'], $rows[$rowData['row_id']])) { break; } @@ -768,7 +788,14 @@ private function parseEntities(IResult $result, array $sleeves): array { * @throws InternalError */ public function isRowInViewPresent(int $rowId, View $view, string $userId): bool { - return in_array($rowId, $this->getWantedRowIds($userId, $view->getTableId(), $view->getFilterArray())); + $identifier = RowIdentifier::tryFromInt($rowId); + return $identifier !== null && $this->findScopedId($identifier, $view->getTableId(), $view->getFilterArray(), $userId) !== null; + } + + public function findScopedId(RowIdentifier $identifier, int $tableId, ?array $filter, string $userId): ?int { + $this->columnMapper->preloadColumns([], $filter, null); + $ids = $this->getWantedRowIds($userId, $tableId, $filter, null, 1, null, $identifier); + return $ids === [] ? null : (int)$ids[0]; } /** @@ -791,6 +818,8 @@ public function insert(Row2 $row, ?string $userId = null): Row2 { $row->setId($rowSleeve->getId()); } + $row->setUuid($rowSleeve->getUuid()); + // write all cells to its db-table foreach ($row->getData() as $cell) { $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy()); @@ -804,6 +833,7 @@ public function insert(Row2 $row, ?string $userId = null): Row2 { * @param string|null $userId * @return Row2 * @throws InternalError + * @throws NotFoundError */ public function update(Row2 $row, ?string $userId = null): Row2 { if ($userId) { @@ -820,7 +850,10 @@ public function update(Row2 $row, ?string $userId = null): Row2 { $sleeve = $this->rowSleeveMapper->find($row->getId()); $this->updateMetaData($sleeve); $this->rowSleeveMapper->update($sleeve); - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + } catch (DoesNotExistException $e) { + $this->logger->warning($e->getMessage(), ['exception' => $e]); + throw new NotFoundError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), 0, $e); + } catch (MultipleObjectsReturnedException|Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } diff --git a/lib/Db/RowSleeve.php b/lib/Db/RowSleeve.php index b909a3f44f..82f8b72a37 100644 --- a/lib/Db/RowSleeve.php +++ b/lib/Db/RowSleeve.php @@ -8,10 +8,13 @@ namespace OCA\Tables\Db; use JsonSerializable; +use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; +use OCA\Tables\Vendor\Symfony\Component\Uid\UuidV7; use OCP\AppFramework\Db\Entity; /** * @psalm-suppress PropertyNotSetInConstructor + * @method getUuid(): ?string * @method getTableId(): ?int * @method setTableId(int $columnId) * @method getCreatedBy(): string @@ -25,6 +28,7 @@ */ class RowSleeve extends Entity implements JsonSerializable { protected ?int $tableId = null; + protected ?string $uuid = null; protected ?string $createdBy = null; protected ?string $createdAt = null; protected ?string $lastEditBy = null; @@ -33,11 +37,31 @@ class RowSleeve extends Entity implements JsonSerializable { public function __construct() { $this->addType('id', 'integer'); $this->addType('tableId', 'integer'); + $this->addType('uuid', 'string'); + } + + /** + * Hydration uses Entity::setter() instead, so stored values load unchanged. + */ + public function setUuid(?string $uuid): void { + $uuid = $uuid !== null ? (new UuidV7($uuid))->toRfc4122() : null; + if ($this->uuid !== null && $uuid !== $this->uuid) { + throw new \InvalidArgumentException('Row UUIDs are immutable.'); + } + $this->uuid = $uuid; + $this->markFieldUpdated('uuid'); + } + + public function assignUuid(): void { + if ($this->uuid === null) { + $this->setUuid(Uuid::v7()->toRfc4122()); + } } public function jsonSerialize(): array { return [ 'id' => $this->id, + 'uuid' => $this->uuid, 'tableId' => $this->tableId, 'createdBy' => $this->createdBy, 'createdAt' => $this->createdAt, diff --git a/lib/Db/RowSleeveMapper.php b/lib/Db/RowSleeveMapper.php index b627424ba5..a0b64b1c07 100644 --- a/lib/Db/RowSleeveMapper.php +++ b/lib/Db/RowSleeveMapper.php @@ -7,7 +7,9 @@ namespace OCA\Tables\Db; +use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\Exception; @@ -28,6 +30,15 @@ public function __construct( parent::__construct($db, $this->table, RowSleeve::class); } + /** + * @param RowSleeve $entity + * @return RowSleeve + */ + public function insert(Entity $entity): Entity { + $entity->assignUuid(); + return parent::insert($entity); + } + /** * @throws MultipleObjectsReturnedException * @throws DoesNotExistException @@ -41,6 +52,50 @@ public function find(int $id): RowSleeve { return $this->findEntity($qb); } + public function findByUuid(string $uuid): RowSleeve { + $qb = $this->db->getQueryBuilder(); + $qb->select('*')->from($this->table) + ->where($qb->expr()->eq('uuid', $qb->createNamedParameter($uuid))); + return $this->findEntity($qb); + } + + /** + * Import creates a copy when the archived identity is already in use. + */ + public function insertImported(RowSleeve $sleeve): RowSleeve { + $tableId = $sleeve->getTableId(); + if ($tableId === null) { + throw new \InvalidArgumentException('An imported row requires a destination table.'); + } + $uuid = $sleeve->getUuid() ?? Uuid::v7()->toRfc4122(); + for ($attempt = 0; ; $attempt++) { + $candidate = new RowSleeve(); + $candidate->setTableId($tableId); + $candidate->setCreatedBy($sleeve->getCreatedBy()); + $candidate->setCreatedAt($sleeve->getCreatedAt()); + $candidate->setLastEditBy($sleeve->getLastEditBy()); + $candidate->setLastEditAt($sleeve->getLastEditAt()); + $candidate->setUuid($uuid); + $this->db->beginTransaction(); + try { + $this->insert($candidate); + $this->db->commit(); + return $candidate; + } catch (\Throwable $e) { + $this->db->rollBack(); + if (!$e instanceof Exception || $e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION || $attempt >= 3) { + throw $e; + } + try { + $this->findByUuid($uuid); + } catch (DoesNotExistException) { + throw $e; + } + $uuid = Uuid::v7()->toRfc4122(); + } + } + } + /** * @param int[] $ids * @return RowSleeve[] @@ -51,6 +106,7 @@ public function findMultiple(array $ids): array { $qb = $this->db->getQueryBuilder(); $qb->select( $sleeveAlias . '.id', + $sleeveAlias . '.uuid', $sleeveAlias . '.table_id', $sleeveAlias . '.created_by', $sleeveAlias . '.created_at', diff --git a/lib/Migration/Version2500Date20261103000000.php b/lib/Migration/Version2500Date20261103000000.php new file mode 100644 index 0000000000..e51f96f3ff --- /dev/null +++ b/lib/Migration/Version2500Date20261103000000.php @@ -0,0 +1,46 @@ +getTable('tables_row_sleeves'); + if (!$table->hasColumn('uuid')) { + $table->addColumn('uuid', Types::STRING, [ + 'notnull' => false, + 'default' => null, + 'length' => 36, + ]); + } + if (!$table->hasIndex('tables_row_uuid_uniq')) { + $table->addUniqueIndex(['uuid'], 'tables_row_uuid_uniq'); + } + return $schema; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $this->jobList->add(BackfillRowUuids::class); + } +} diff --git a/lib/Model/RowDataInput.php b/lib/Model/RowDataInput.php index 21f54cd2f9..b5d7a4fa7d 100644 --- a/lib/Model/RowDataInput.php +++ b/lib/Model/RowDataInput.php @@ -84,6 +84,33 @@ public function rewind(): void { reset($this->data); } + public static function createFromInputArray(array $cells): self { + if (!array_is_list($cells)) { + throw new \InvalidArgumentException('Row data must be a list of cells.'); + } + $input = new self(); + foreach ($cells as $cell) { + if (!is_array($cell) || !array_key_exists('columnId', $cell) || !array_key_exists('value', $cell)) { + throw new \InvalidArgumentException('Each cell requires columnId and value.'); + } + $columnId = $cell['columnId']; + if (is_string($columnId) && preg_match('/^-?[0-9]+$/D', $columnId)) { + $digits = ltrim(ltrim($columnId, '-'), '0'); + $canonical = ($digits !== '' && str_starts_with($columnId, '-') ? '-' : '') . ($digits === '' ? '0' : $digits); + $columnId = filter_var($canonical, FILTER_VALIDATE_INT); + } + if (!is_int($columnId)) { + throw new \InvalidArgumentException('Cell columnId must be an integer within the supported range.'); + } + $value = $cell['value']; + if ($value !== null && !is_scalar($value) && !is_array($value)) { + throw new \InvalidArgumentException('Cell values must be JSON values.'); + } + $input->add($columnId, is_bool($value) ? (int)$value : $value); + } + return $input; + } + public static function fromArray(array $data): self { $newRowData = new RowDataInput(); foreach ($data as $value) { diff --git a/lib/Model/RowIdentifier.php b/lib/Model/RowIdentifier.php new file mode 100644 index 0000000000..65ab6bcee0 --- /dev/null +++ b/lib/Model/RowIdentifier.php @@ -0,0 +1,48 @@ + strlen($maximum) + || (strlen($digits) === strlen($maximum) && strcmp($digits, $maximum) > 0)) { + throw new \InvalidArgumentException('Row ID must be a positive integer within the supported range.'); + } + return new self((int)$digits); + } + + try { + return new self((new UuidV7($value))->toRfc4122()); + } catch (\InvalidArgumentException $e) { + throw new \InvalidArgumentException('Row identifier must be a positive integer or UUIDv7.', 0, $e); + } + } + + public static function tryFromInt(int $rowId): ?self { + return $rowId > 0 && $rowId <= self::MAX_ROW_ID ? new self($rowId) : null; + } + + public function isUuid(): bool { + return is_string($this->value); + } +} diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index dd41a0bf4f..ad037ebfeb 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -87,6 +87,7 @@ * * @psalm-type TablesRow = array{ * id: int, + * uuid: ?string, * tableId: int, * createdBy: string, * createdAt: string, @@ -98,6 +99,7 @@ * * @psalm-type TablesPublicRow = array{ * id: int, + * uuid: ?string, * createdAt: string, * lastEditAt: string, * data: ?array{columnId: int, value: mixed}, diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index fbc0054832..58f2f58b3d 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -691,8 +691,7 @@ private function upsertRow(Row $row, array $columnBusinesses): void { } if ($rowId) { - $this->rowService->updateSet($rowId, $this->viewId, $data, $this->userId, $this->tableId); - $this->countUpdatedRows++; + $this->updateImportedRow($rowId, $data); } else { $this->rowService->create($this->tableId, $this->viewId, $data, $this->userId); $this->countInsertedRows++; @@ -712,6 +711,24 @@ private function upsertRow(Row $row, array $columnBusinesses): void { } } + /** + * An ID that matches no row in the destination fails that line, not the whole import. + * + * @param list $data + * @throws BadRequestError + * @throws InternalError + * @throws PermissionError + */ + private function updateImportedRow(int $rowId, array $data): void { + try { + $this->rowService->updateSet($rowId, $this->viewId, $data, $this->userId, $this->tableId); + $this->countUpdatedRows++; + } catch (NotFoundError $e) { + $this->logger->warning('Row to update during import was not found: ' . $rowId, ['exception' => $e]); + $this->countErrors++; + } + } + /** * @param null|string $value */ diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 3e98e7d4a2..fd79563fda 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -8,10 +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\RowSleeve; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\View; @@ -24,7 +26,9 @@ use OCA\Tables\Event\RowDeletedEvent; use OCA\Tables\Event\RowUpdatedEvent; use OCA\Tables\Helper\ColumnsHelper; +use OCA\Tables\Helper\ConversionHelper; use OCA\Tables\Model\RowDataInput; +use OCA\Tables\Model\RowIdentifier; use OCA\Tables\Notification\NotificationHelper; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; @@ -32,9 +36,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; -use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; -use OCP\IDBConnection; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; @@ -58,7 +60,6 @@ public function __construct( private readonly ColumnsHelper $columnsHelper, private readonly ActivityManager $activityManager, private readonly NotificationHelper $notificationHelper, - private readonly IDBConnection $connection, ) { parent::__construct($logger, $userId, $permissionsService); @@ -194,6 +195,56 @@ public function find(int $rowId): Row2 { return $row; } + /** + * Resolve a validated identifier to the stored row ID inside the requested table or view. + * + * @throws PermissionError + * @throws NotFoundError + * @throws InternalError + */ + public function resolveRowId(RowIdentifier $identifier, int $nodeType, int $nodeId, string $userId): int { + if (!$this->permissionsService->canReadRowsByElementId($nodeId, ConversionHelper::constNodeType2String($nodeType), $userId)) { + throw new PermissionError('Cannot read rows in the requested node.'); + } + try { + $view = $nodeType === Application::NODE_TYPE_VIEW ? $this->viewMapper->find($nodeId) : null; + } catch (DoesNotExistException $e) { + throw new NotFoundError('Row not found in the requested node.', 0, $e); + } catch (Exception|MultipleObjectsReturnedException $e) { + throw new InternalError('Cannot load the requested view.', 0, $e); + } + $rowId = $this->row2Mapper->findScopedId( + $identifier, + $view !== null ? $view->getTableId() : $nodeId, + $view?->getFilterArray(), + $view !== null ? $this->resolveFilterUserId($userId, $view) : $userId, + ); + if ($rowId === null) { + throw new NotFoundError('Row not found in the requested node.'); + } + return $rowId; + } + + /** + * @throws PermissionError + * @throws NotFoundError + * @throws InternalError + */ + public function findByIdentifier(RowIdentifier $identifier, int $nodeType, int $nodeId, string $userId): Row2 { + $rowId = $this->resolveRowId($identifier, $nodeType, $nodeId, $userId); + try { + $view = $nodeType === Application::NODE_TYPE_VIEW ? $this->viewMapper->find($nodeId) : null; + $columns = $view !== null ? $this->columnMapper->findAll($view->getColumnIds()) : $this->columnMapper->findAllByTable($nodeId); + $row = $this->row2Mapper->find($rowId, $columns); + $this->attachAliasPayload($row, $columns); + return $this->filterRowResult($view, $row); + } catch (DoesNotExistException $e) { + throw new NotFoundError('Row not found in the requested node.', 0, $e); + } catch (Exception|MultipleObjectsReturnedException $e) { + throw new InternalError('Cannot load the requested row.', 0, $e); + } + } + /** * @param int|null $tableId * @param int|null $viewId @@ -558,10 +609,10 @@ private function getRowById(int $rowId): Row2 { } $row = $this->row2Mapper->find($rowId, $this->columnMapper->findAllByTable($this->row2Mapper->getTableIdForRow($rowId))); $row->markAsLoaded(); - } catch (InternalError|DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { + } catch (InternalError|MultipleObjectsReturnedException|Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); - } catch (NotFoundError $e) { + } catch (NotFoundError|DoesNotExistException $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new NotFoundError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } @@ -584,10 +635,29 @@ private function getRowById(int $rowId): Row2 { * @throws PermissionError * @noinspection DuplicatedCode */ - public function updateSet( + public function updateSet(int $id, ?int $viewId, array $data, string $userId, ?int $tableId): Row2 { + return $this->updateRowData($id, $viewId, RowDataInput::fromArray($data), $userId, $tableId); + } + + /** + * Update multiple cells in a row + * + * @param int $id + * @param int|null $viewId + * @param RowDataInput $data + * @param string $userId + * @return Row2 + * + * @throws BadRequestError + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError + * @noinspection DuplicatedCode + */ + public function updateRowData( int $id, ?int $viewId, - array $data, + RowDataInput $data, string $userId, ?int $tableId, ): Row2 { @@ -634,10 +704,10 @@ public function updateSet( } // is row in view? - if (!$this->row2Mapper->isRowInViewPresent($id, $view, $userId)) { + if (!$this->row2Mapper->isRowInViewPresent($id, $view, $this->resolveFilterUserId($userId, $view))) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + throw new NotFoundError('Row not found in the requested view.'); } // fetch all needed columns @@ -652,7 +722,7 @@ public function updateSet( if ($tableId !== $item->getTableId()) { $e = new \Exception('Row does not belong to table with id ' . $tableId); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + throw new NotFoundError('Row not found in the requested node.'); } // security @@ -675,7 +745,6 @@ public function updateSet( } $previousData = $item->getData(); - $data = RowDataInput::fromArray($data); $data = $this->cleanupAndValidateData($data, $columns, $item->getTableId(), $viewId, $id); foreach ($data as $entry) { @@ -748,7 +817,7 @@ public function delete(int $id, ?int $viewId, string $userId, ?int $tableId = nu $this->logger->error($e->getMessage(), ['exception' => $e]); throw new NotFoundError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } - if (!$this->permissionsService->canDeleteRowsByViewId($viewId)) { + if (!$this->permissionsService->canDeleteRowsByViewId($viewId, $userId)) { $e = new \Exception('Update row is not allowed.'); $this->logger->error($e->getMessage(), ['exception' => $e]); throw new PermissionError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -759,16 +828,14 @@ public function delete(int $id, ?int $viewId, string $userId, ?int $tableId = nu $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } - if (!$this->row2Mapper->isRowInViewPresent($id, $view, $userId)) { - $e = new \Exception('Update row is not allowed.'); - $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + if (!$this->row2Mapper->isRowInViewPresent($id, $view, $this->resolveFilterUserId($userId, $view))) { + throw new NotFoundError('Row not found in the requested view.'); } } else { if ($tableId !== null && $tableId !== $item->getTableId()) { $e = new \Exception('Row does not belong to table with id ' . $tableId); $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + throw new NotFoundError('Row not found in the requested node.'); } // security @@ -922,6 +989,7 @@ private function filterRowResult(?View $view, Row2 $row): Row2 { return $row; } + $row = clone $row; $columnIds = $view->getColumnIds(); $row->filterDataByColumns($columnIds); $row->filterDataByAliasByColumns($columnIds); @@ -1003,23 +1071,27 @@ private function buildAliasByColumnId(array $columns): array { * @throws InternalError */ public function importRow(Table $table, array $row): int { - $qb = $this->connection->getQueryBuilder(); + $tableId = $table->getId(); + $owner = $table->getOwnership(); + if ($tableId === null || $owner === null) { + throw new \InvalidArgumentException('An imported row requires a stored destination table and owner.'); + } + + $sleeve = new RowSleeve(); + if (isset($row['uuid']) && !is_string($row['uuid'])) { + throw new \InvalidArgumentException('Archived row UUID must be a string or null.'); + } + $sleeve->setUuid($row['uuid'] ?? null); + $sleeve->setTableId($tableId); + $sleeve->setCreatedBy($owner); + $sleeve->setCreatedAt($row['createdAt']); + $sleeve->setLastEditBy($owner); + $sleeve->setLastEditAt($row['lastEditAt']); try { - $qb->insert('tables_row_sleeves') - ->values([ - 'table_id' => $qb->createNamedParameter($table->getId(), IQueryBuilder::PARAM_INT), - 'created_by' => $qb->createNamedParameter($table->getOwnership()), - 'created_at' => $qb->createNamedParameter($row['createdAt']), - 'last_edit_by' => $qb->createNamedParameter($table->getOwnership()), - 'last_edit_at' => $qb->createNamedParameter($row['lastEditAt']), - ]); - $qb->executeStatement(); - $id = $this->connection->lastInsertId('*PREFIX*tables_row_sleeves'); - - return $id; - } catch (\Exception $e) { + return $this->row2Mapper->importSleeve($sleeve)->getId(); + } catch (Exception $e) { $this->logger->error('userMigrationImport insert error: ' . $e->getMessage()); - throw new InternalError('userMigrationImport insert error: ' . $e->getMessage()); + throw new InternalError('userMigrationImport insert error: ' . $e->getMessage(), 0, $e); } } diff --git a/openapi.json b/openapi.json index 9fd3ae317c..1dd9b21758 100644 --- a/openapi.json +++ b/openapi.json @@ -657,6 +657,7 @@ "type": "object", "required": [ "id", + "uuid", "createdAt", "lastEditAt", "data", @@ -667,6 +668,10 @@ "type": "integer", "format": "int64" }, + "uuid": { + "type": "string", + "nullable": true + }, "createdAt": { "type": "string" }, @@ -715,6 +720,7 @@ "type": "object", "required": [ "id", + "uuid", "tableId", "createdBy", "createdAt", @@ -728,6 +734,10 @@ "type": "integer", "format": "int64" }, + "uuid": { + "type": "string", + "nullable": true + }, "tableId": { "type": "integer", "format": "int64" @@ -16442,6 +16452,278 @@ } }, "/ocs/v2.php/apps/tables/api/2/{nodeCollection}/{nodeId}/rows/{rowId}": { + "get": { + "operationId": "rowocs-get-row", + "summary": "[api v2] Read a local row in a table or view", + "tags": [ + "rowocs" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "nodeCollection", + "in": "path", + "description": "The targeted node collection", + "required": true, + "schema": { + "type": "string", + "enum": [ + "tables", + "views" + ], + "pattern": "^(tables|views)$" + } + }, + { + "name": "nodeId", + "in": "path", + "description": "The table or view identifier", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "rowId", + "in": "path", + "description": "A positive decimal row ID or a hyphenated UUIDv7", + "required": true, + "schema": { + "type": "string" + } + }, + { + "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": "Row returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "$ref": "#/components/schemas/Row" + } + } + } + } + } + } + } + }, + "400": { + "description": "Invalid identifier or unsupported federated operation", + "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" + } + } + } + } + } + } + } + } + } + }, + "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" + } + } + } + } + } + } + } + } + } + }, + "404": { + "description": "Not found in the requested node", + "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": {} + } + } + } + } + } + } + } + } + }, "put": { "operationId": "rowocs-update-row", "summary": "[api v2] Update a row in a table or a view", @@ -16467,15 +16749,35 @@ ], "properties": { "data": { - "description": "An array containing the column identifiers and their values", + "description": "A list of cells containing columnId and value", "oneOf": [ { "type": "string" }, { - "type": "object", - "additionalProperties": { - "type": "object" + "type": "array", + "items": { + "type": "object", + "required": [ + "columnId", + "value" + ], + "properties": { + "columnId": { + "oneOf": [ + { + "type": "integer", + "format": "int64" + }, + { + "type": "string" + } + ] + }, + "value": { + "type": "object" + } + } } } ] @@ -16513,11 +16815,10 @@ { "name": "rowId", "in": "path", - "description": "The identifier of the row to update", + "description": "A positive decimal row ID or a hyphenated UUIDv7", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -16786,11 +17087,10 @@ { "name": "rowId", "in": "path", - "description": "The identifier of the row to delete", + "description": "A positive decimal row ID or a hyphenated UUIDv7", "required": true, "schema": { - "type": "integer", - "format": "int64" + "type": "string" } }, { @@ -16873,6 +17173,44 @@ } } }, + "400": { + "description": "Invalid identifier or unsupported federated operation", + "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": { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 85437d7fca..4ef16c8725 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1081,7 +1081,8 @@ export type paths = { readonly path?: never; readonly cookie?: never; }; - readonly get?: never; + /** [api v2] Read a local row in a table or view */ + readonly get: operations["rowocs-get-row"]; /** [api v2] Update a row in a table or a view */ readonly put: operations["rowocs-update-row"]; readonly post?: never; @@ -1306,6 +1307,7 @@ export type components = { readonly PublicRow: { /** Format: int64 */ readonly id: number; + readonly uuid: string | null; readonly createdAt: string; readonly lastEditAt: string; readonly data: { @@ -1324,6 +1326,7 @@ export type components = { readonly Row: { /** Format: int64 */ readonly id: number; + readonly uuid: string | null; /** Format: int64 */ readonly tableId: number; readonly createdBy: string; @@ -8848,6 +8851,119 @@ export interface operations { }; }; }; + readonly "rowocs-get-row": { + readonly parameters: { + readonly query?: never; + readonly header: { + /** @description Required to be true for the API request to pass */ + readonly "OCS-APIRequest": boolean; + }; + readonly path: { + /** @description The targeted node collection */ + readonly nodeCollection: "tables" | "views"; + /** @description The table or view identifier */ + readonly nodeId: number; + /** @description A positive decimal row ID or a hyphenated UUIDv7 */ + readonly rowId: string; + }; + readonly cookie?: never; + }; + readonly requestBody?: never; + readonly responses: { + /** @description Row returned */ + readonly 200: { + headers: { + readonly [name: string]: unknown; + }; + content: { + readonly "application/json": { + readonly ocs: { + readonly meta: components["schemas"]["OCSMeta"]; + readonly data: components["schemas"]["Row"]; + }; + }; + }; + }; + /** @description Invalid identifier or unsupported federated operation */ + 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 in the requested node */ + 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-update-row": { readonly parameters: { readonly query?: never; @@ -8860,18 +8976,19 @@ export interface operations { readonly nodeCollection: "tables" | "views"; /** @description The identifier of the targeted table or view */ readonly nodeId: number; - /** @description The identifier of the row to update */ - readonly rowId: number; + /** @description A positive decimal row ID or a hyphenated UUIDv7 */ + readonly rowId: string; }; readonly cookie?: never; }; readonly requestBody: { readonly content: { readonly "application/json": { - /** @description An array containing the column identifiers and their values */ - readonly data: string | { - readonly [key: string]: Record; - }; + /** @description A list of cells containing columnId and value */ + readonly data: string | readonly { + readonly columnId: number | string; + readonly value: Record; + }[]; }; }; }; @@ -8982,8 +9099,8 @@ export interface operations { readonly nodeCollection: "tables" | "views"; /** @description The identifier of the targeted table or view */ readonly nodeId: number; - /** @description The identifier of the row to delete */ - readonly rowId: number; + /** @description A positive decimal row ID or a hyphenated UUIDv7 */ + readonly rowId: string; }; readonly cookie?: never; }; @@ -9003,6 +9120,22 @@ export interface operations { }; }; }; + /** @description Invalid identifier or unsupported federated operation */ + 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: { diff --git a/tests/unit/BackgroundJob/BackfillRowUuidsTest.php b/tests/unit/BackgroundJob/BackfillRowUuidsTest.php new file mode 100644 index 0000000000..0310b5f2d7 --- /dev/null +++ b/tests/unit/BackgroundJob/BackfillRowUuidsTest.php @@ -0,0 +1,221 @@ +connectionAdapter->beginTransaction(); + $delete = $this->connectionAdapter->getQueryBuilder(); + $delete->delete('tables_row_sleeves')->where($delete->expr()->gt('id', $delete->createNamedParameter(0)))->executeStatement(); + $this->settings = []; + $this->config = $this->createMock(IAppConfig::class); + $this->config->method('getValueInt')->willReturnCallback(fn ($app, $key, $default) => $this->settings[$key] ?? $default); + $this->config->method('setValueInt')->willReturnCallback(function ($app, $key, $value): bool { + $this->settings[$key] = $value; + return true; + }); + $this->config->method('deleteKey')->willReturnCallback(function ($app, $key): void { + unset($this->settings[$key]); + }); + $this->jobList = $this->createMock(IJobList::class); + $this->mapper = new RowSleeveMapper($this->connectionAdapter, new NullLogger()); + } + + protected function tearDown(): void { + $this->connectionAdapter->rollBack(); + parent::tearDown(); + } + + private function historicalRows(int $count): array { + $insert = $this->connectionAdapter->getQueryBuilder(); + $insert->insert('tables_row_sleeves')->values([ + 'table_id' => $insert->createNamedParameter(991), + 'created_by' => $insert->createNamedParameter('original-author'), + 'created_at' => $insert->createNamedParameter('2020-01-01 00:00:00'), + 'last_edit_by' => $insert->createNamedParameter('original-editor'), + 'last_edit_at' => $insert->createNamedParameter('2020-01-02 00:00:00'), + ]); + $ids = []; + for ($i = 0; $i < $count; $i++) { + $insert->executeStatement(); + $ids[] = $insert->getLastInsertId(); + } + return $ids; + } + + private function job(ITimeFactory $clock, ?LoggerInterface $logger = null): BackfillRowUuids { + return new BackfillRowUuids($clock, $this->connectionAdapter, $this->config, $this->jobList, $logger ?? new NullLogger()); + } + + public function testTimeBudgetCheckpointsProcessedPrefixAndResumePreservesMetadata(): void { + $ids = $this->historicalRows(3); + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturnOnConsecutiveCalls(0, 5, 5, 5); + $this->jobList->expects($this->once())->method('remove')->with(BackfillRowUuids::class); + $this->job($clock)->run(null); + $this->assertSame($ids[0], $this->settings['row_uuid_backfill_cursor']); + $uuid = $this->mapper->find($ids[0])->getUuid(); + $this->assertNotNull($uuid); + $this->assertNull($this->mapper->find($ids[1])->getUuid()); + + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturn(10); + $this->job($clock)->run(null); + $this->assertSame($uuid, $this->mapper->find($ids[0])->getUuid()); + foreach ($ids as $id) { + $row = $this->mapper->find($id); + $this->assertNotNull($row->getUuid()); + $this->assertSame('original-author', $row->getCreatedBy()); + $this->assertSame('original-editor', $row->getLastEditBy()); + $this->assertSame('2020-01-01 00:00:00', $row->getCreatedAt()); + $this->assertSame('2020-01-02 00:00:00', $row->getLastEditAt()); + } + $this->assertSame([], $this->settings); + } + + public function testFinalPassRevisitsNullsBelowCursor(): void { + $ids = $this->historicalRows(2); + $this->settings['row_uuid_backfill_cursor'] = $ids[1] + 100; + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturn(0); + $this->jobList->expects($this->once())->method('remove'); + $this->job($clock)->run(null); + $this->assertNotNull($this->mapper->find($ids[0])->getUuid()); + $this->assertNotNull($this->mapper->find($ids[1])->getUuid()); + } + + public function testCommittedWorkSurvivesCheckpointFailure(): void { + $ids = $this->historicalRows(1); + $config = $this->createMock(IAppConfig::class); + $config->method('getValueInt')->willReturn(0); + $config->method('setValueInt')->willThrowException(new \RuntimeException('Checkpoint unavailable')); + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('error'); + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturnOnConsecutiveCalls(0, 5, 5, 5); + $job = new BackfillRowUuids($clock, $this->connectionAdapter, $config, $this->jobList, $logger); + $job->run(null); + $uuid = $this->mapper->find($ids[0])->getUuid(); + $this->assertNotNull($uuid); + $resumeClock = $this->createMock(ITimeFactory::class); + $resumeClock->method('getTime')->willReturn(10); + $this->jobList->expects($this->once())->method('remove'); + $this->job($resumeClock)->run(null); + $this->assertSame($uuid, $this->mapper->find($ids[0])->getUuid()); + } + + public function testCheckpointIsWrittenLazilyOncePerExecution(): void { + $ids = $this->historicalRows(501); + $config = $this->createMock(IAppConfig::class); + $config->method('getValueInt')->with(Application::APP_ID, 'row_uuid_backfill_cursor', 0, true)->willReturn(0); + $config->expects($this->once())->method('setValueInt')->with(Application::APP_ID, 'row_uuid_backfill_cursor', $ids[500], true)->willReturn(true); + $calls = 0; + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturnCallback(function () use (&$calls): int { + return ++$calls > 503 ? 5 : 0; + }); + $job = new BackfillRowUuids($clock, $this->connectionAdapter, $config, $this->jobList, new NullLogger()); + $job->run(null); + $this->assertNotNull($this->mapper->find($ids[500])->getUuid()); + } + + public function testFailedBatchRollsBackItsPrefixBeforeRetrying(): void { + $ids = $this->historicalRows(2); + $actualUpdate = $this->connectionAdapter->getQueryBuilder(); + $update = $this->createMock(IQueryBuilder::class); + foreach (['update', 'set', 'where', 'andWhere', 'setParameter'] as $method) { + $update->method($method)->willReturnCallback(function (...$arguments) use ($actualUpdate, $update, $method) { + $actualUpdate->$method(...$arguments); + return $update; + }); + } + $update->method('expr')->willReturn($actualUpdate->expr()); + $update->method('createParameter')->willReturnCallback(fn ($name) => $actualUpdate->createParameter($name)); + $calls = 0; + $update->method('executeStatement')->willReturnCallback(function () use ($actualUpdate, &$calls): int { + if (++$calls === 2) { + throw new \RuntimeException('Database write interrupted'); + } + return $actualUpdate->executeStatement(); + }); + $connection = $this->createMock(IDBConnection::class); + $connection->method('getQueryBuilder')->willReturnOnConsecutiveCalls($this->connectionAdapter->getQueryBuilder(), $update); + $connection->expects($this->once())->method('beginTransaction')->willReturnCallback(fn () => $this->connectionAdapter->beginTransaction()); + $connection->expects($this->once())->method('rollBack')->willReturnCallback(fn () => $this->connectionAdapter->rollBack()); + $connection->expects($this->never())->method('commit'); + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturn(0); + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('error'); + $job = new BackfillRowUuids($clock, $connection, $this->config, $this->jobList, $logger); + $job->run(null); + $this->assertSame([], $this->settings); + $this->assertNull($this->mapper->find($ids[0])->getUuid()); + $this->assertNull($this->mapper->find($ids[1])->getUuid()); + $this->jobList->expects($this->once())->method('remove'); + $this->job($clock)->run(null); + $this->assertNotNull($this->mapper->find($ids[0])->getUuid()); + $this->assertNotNull($this->mapper->find($ids[1])->getUuid()); + } + + public function testInterleavedAssignmentAndDeletionAreNotOverwritten(): void { + $ids = $this->historicalRows(3); + $uuid = '019f90c5-8576-7f0b-9b14-9bf49b2826cf'; + $assign = $this->connectionAdapter->getQueryBuilder(); + $assign->update('tables_row_sleeves')->set('uuid', $assign->createNamedParameter($uuid)) + ->where($assign->expr()->eq('id', $assign->createNamedParameter($ids[1]))); + $delete = $this->connectionAdapter->getQueryBuilder(); + $delete->delete('tables_row_sleeves')->where($delete->expr()->eq('id', $delete->createNamedParameter($ids[2]))); + $clock = $this->createMock(ITimeFactory::class); + $calls = 0; + $clock->method('getTime')->willReturnCallback(function () use (&$calls, $assign, $delete): int { + if (++$calls === 2) { + $assign->executeStatement(); + $delete->executeStatement(); + } + return 0; + }); + $this->jobList->expects($this->once())->method('remove'); + $this->job($clock)->run(null); + $this->assertNotNull($this->mapper->find($ids[0])->getUuid()); + $this->assertSame($uuid, $this->mapper->find($ids[1])->getUuid()); + $this->expectException(\OCP\AppFramework\Db\DoesNotExistException::class); + $this->mapper->find($ids[2]); + } + + public function testMoreThanOneBatchCompletesWithUniqueIdentities(): void { + $ids = $this->historicalRows(501); + $clock = $this->createMock(ITimeFactory::class); + $clock->method('getTime')->willReturn(0); + $this->jobList->expects($this->once())->method('remove'); + $this->job($clock)->run(null); + $uuids = array_map(fn ($id) => $this->mapper->find($id)->getUuid(), $ids); + $this->assertNotContains(null, $uuids); + $this->assertCount(501, array_unique($uuids)); + } +} diff --git a/tests/unit/Controller/RowOCSControllerTest.php b/tests/unit/Controller/RowOCSControllerTest.php new file mode 100644 index 0000000000..85ede89ce1 --- /dev/null +++ b/tests/unit/Controller/RowOCSControllerTest.php @@ -0,0 +1,164 @@ +rows = $this->createMock(RowService::class); + $this->federation = $this->createMock(FederationService::class); + $this->tables = $this->createMock(TableService::class); + $this->views = $this->createMock(ViewService::class); + $l10n = $this->createMock(IL10N::class); + $l10n->method('t')->willReturnArgument(0); + $this->controller = new RowOCSController($this->createMock(IRequest::class), new NullLogger(), $l10n, 'alice', $this->rows, $this->tables, $this->views, $this->federation); + } + + private function row(): Row2 { + $row = new Row2(); + $row->setId(13); + $row->setTableId(5); + $row->setUuid(self::UUID); + return $row; + } + + /** @dataProvider localOperations */ + public function testLocalItemOperationsConvergeOnStoredIdentity(string $method, string $collection, string $identifier): void { + $row = $this->row(); + $nodeType = $collection === 'views' ? Application::NODE_TYPE_VIEW : Application::NODE_TYPE_TABLE; + $matchesIdentifier = $this->callback(fn (RowIdentifier $id) => $id->value === RowIdentifier::fromString($identifier)->value); + if ($method === 'getRow') { + $this->rows->expects($this->once())->method('findByIdentifier')->with($matchesIdentifier, $nodeType, 5, 'alice')->willReturn($row); + } else { + $this->rows->expects($this->never())->method('findByIdentifier'); + $this->rows->expects($this->once())->method('resolveRowId')->with($matchesIdentifier, $nodeType, 5, 'alice')->willReturn(13); + } + if ($method === 'updateRow') { + $this->rows->expects($this->once())->method('updateRowData')->with(13, $collection === 'views' ? 5 : null, $this->isInstanceOf(RowDataInput::class), 'alice', $collection === 'tables' ? 5 : null)->willReturn($row); + } elseif ($method === 'deleteRow') { + $this->rows->expects($this->once())->method('delete')->with(13, $collection === 'views' ? 5 : null, 'alice', $collection === 'tables' ? 5 : null)->willReturn($row); + } + $response = $method === 'updateRow' + ? $this->controller->updateRow($collection, 5, $identifier, []) + : $this->controller->$method($collection, 5, $identifier); + $this->assertSame(200, $response->getStatus()); + $this->assertSame(self::UUID, $response->getData()['uuid']); + $this->assertSame(13, $response->getData()['id']); + } + + public static function localOperations(): array { + $cases = []; + foreach (['getRow', 'updateRow', 'deleteRow'] as $method) { + foreach (['tables', 'views'] as $collection) { + foreach (['00013', strtoupper(self::UUID)] as $identifier) { + $cases[] = [$method, $collection, $identifier]; + } + } + } + return $cases; + } + + /** @dataProvider invalidOperations */ + public function testInvalidIdentifiersReturnBadRequestBeforeResolving(string $method): void { + $this->rows->expects($this->never())->method('findByIdentifier'); + $this->rows->expects($this->never())->method('resolveRowId'); + $this->federation->expects($this->never())->method('isNodeFederated'); + $response = $method === 'updateRow' ? $this->controller->updateRow('tables', 5, 'not-a-row', []) : $this->controller->$method('tables', 5, 'not-a-row'); + $this->assertSame(400, $response->getStatus()); + } + + public static function invalidOperations(): array { + return [['getRow'], ['updateRow'], ['deleteRow']]; + } + + public function testMalformedUpdateCellsReturnBadRequestWithoutLoadingRow(): void { + $this->rows->expects($this->never())->method('resolveRowId'); + $this->assertSame(400, $this->controller->updateRow('tables', 5, '13', ['column_1' => 'value'])->getStatus()); + $this->assertSame(400, $this->controller->updateRow('tables', 5, '13', [['columnId' => 1]])->getStatus()); + } + + /** @dataProvider federatedOperations */ + public function testFederatedBoundary(string $method, string $collection, bool $uuid): void { + $this->federation->method('isNodeFederated')->willReturn(true); + $this->rows->expects($this->never())->method('findByIdentifier'); + $this->rows->expects($this->never())->method('resolveRowId'); + $forward = !$uuid && $method !== 'getRow'; + if ($forward) { + $node = $collection === 'tables' ? new Table() : new View(); + ($collection === 'tables' ? $this->tables : $this->views)->method('find')->willReturn($node); + if ($method === 'updateRow') { + $this->federation->expects($this->once())->method($method)->with($node, 13, [])->willReturn(['id' => 13]); + } else { + $this->federation->expects($this->once())->method($method)->with($node, 13)->willReturn(['id' => 13]); + } + } else { + $this->federation->expects($this->never())->method('updateRow'); + $this->federation->expects($this->never())->method('deleteRow'); + } + $identifier = $uuid ? self::UUID : '13'; + $response = $method === 'updateRow' ? $this->controller->updateRow($collection, 5, $identifier, []) : $this->controller->$method($collection, 5, $identifier); + $this->assertSame($forward ? 200 : 400, $response->getStatus()); + if ($forward) { + $this->assertArrayHasKey('uuid', $response->getData()); + $this->assertNull($response->getData()['uuid']); + } + } + + public static function federatedOperations(): array { + $cases = []; + foreach (['getRow', 'updateRow', 'deleteRow'] as $method) { + foreach (['tables', 'views'] as $collection) { + foreach ([false, true] as $uuid) { + $cases[] = [$method, $collection, $uuid]; + } + } + } + return $cases; + } + + /** @dataProvider serviceFailures */ + public function testServiceErrorsMapToHttpStatus(\Exception $error, int $status): void { + $this->rows->method('findByIdentifier')->willThrowException($error); + $this->assertSame($status, $this->controller->getRow('tables', 5, self::UUID)->getStatus()); + } + + public static function serviceFailures(): array { + return [ + [new NotFoundError('Missing'), 404], + [new PermissionError('Denied'), 403], + [new \RuntimeException('Failed'), 500], + [new \InvalidArgumentException('Stored state is invalid'), 500], + ]; + } +} diff --git a/tests/unit/Database/DatabaseTestCase.php b/tests/unit/Database/DatabaseTestCase.php index 4519504a84..ff09e4bff3 100644 --- a/tests/unit/Database/DatabaseTestCase.php +++ b/tests/unit/Database/DatabaseTestCase.php @@ -15,6 +15,7 @@ use OC\DB\MigrationService; use OC\Migration\NullOutput; use OC\SystemConfig; +use OCA\Tables\Vendor\Symfony\Component\Uid\Uuid; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Server; use PHPUnit\Framework\TestCase; @@ -234,6 +235,7 @@ protected function createTestColumn(int $tableId, array $data = []) { protected function createTestRow(int $tableId, array $data = []) { $defaultData = [ 'table_id' => $tableId, + 'uuid' => Uuid::v7()->toRfc4122(), 'created_by' => 'user1', 'created_at' => date('Y-m-d H:i:s'), 'last_edit_by' => 'user1', diff --git a/tests/unit/Db/Row2MapperTest.php b/tests/unit/Db/Row2MapperTest.php index a5a82b25ab..18771eea21 100644 --- a/tests/unit/Db/Row2MapperTest.php +++ b/tests/unit/Db/Row2MapperTest.php @@ -10,6 +10,10 @@ namespace OCA\Tables\Tests\Unit\Db; use OCA\Tables\Db\Column; +use OCA\Tables\Db\Row2; +use OCA\Tables\Db\View; +use OCA\Tables\Errors\NotFoundError; +use OCA\Tables\Model\RowIdentifier; use OCA\Tables\Tests\Unit\Database\DatabaseTestCase; use PHPUnit\Framework\Attributes\DataProvider; @@ -28,6 +32,59 @@ protected function setUp(): void { $this->setupRealColumnMapper(self::$testTableId); } + public function testEmptyRowKeepsStoredUuidWithoutVisibleColumns(): void { + $this->connectionAdapter->beginTransaction(); + try { + $row = new Row2(); + $row->setTableId(self::$testTableId); + $created = $this->mapper->insert($row, 'alice'); + $this->assertNotNull($created->getUuid()); + $loaded = $this->mapper->find($created->getId(), []); + $this->assertSame($created->getUuid(), $loaded->jsonSerialize()['uuid']); + $this->assertSame([], $loaded->getData()); + $identifier = RowIdentifier::fromString($created->getUuid()); + $this->assertSame($created->getId(), $this->mapper->findScopedId($identifier, self::$testTableId, null, 'alice')); + $this->assertNull($this->mapper->findScopedId($identifier, self::$testTableId + 1, null, 'alice')); + } finally { + $this->connectionAdapter->rollBack(); + } + } + + public function testListingWithoutColumnsReturnsNoRows(): void { + $this->assertSame([], $this->mapper->findAll([], self::$testTableId, null, null, null, null, 'alice')); + } + + /** @dataProvider idsNoRowCanHave */ + public function testPresenceCheckIsFalseForIdsNoRowCanHave(int $rowId): void { + $view = new View(); + $view->setTableId(self::$testTableId); + $this->assertFalse($this->mapper->isRowInViewPresent($rowId, $view, 'alice')); + } + + public static function idsNoRowCanHave(): array { + return [[0], [-1], [2147483648]]; + } + + public function testUpdatingRemovedRowIsNotFound(): void { + $row = new Row2(); + $row->setId(2147483647); + $row->markAsLoaded(); + $row->insertOrUpdateCell(['columnId' => 1, 'value' => 'changed']); + $this->expectException(NotFoundError::class); + $this->mapper->update($row, 'alice'); + } + + public function testTargetLookupAppliesSavedFiltersAndTableScope(): void { + $firstId = self::$testRowIds[0]; + $identifier = RowIdentifier::fromString((string)$firstId); + $this->assertSame($firstId, $this->mapper->findScopedId($identifier, self::$testTableId, null, 'alice')); + $filter = [[['columnId' => Column::TYPE_META_ID, 'operator' => 'is-equal', 'value' => (string)($firstId + 1)]]]; + $this->assertNull($this->mapper->findScopedId($identifier, self::$testTableId, $filter, 'alice')); + $filter[0][0]['value'] = (string)$firstId; + $this->assertSame($firstId, $this->mapper->findScopedId($identifier, self::$testTableId, $filter, 'alice')); + $this->assertNull($this->mapper->findScopedId($identifier, self::$testTableId + 1, null, 'alice')); + } + /** * Data provider for sorting tests * diff --git a/tests/unit/Db/RowSleeveMapperTest.php b/tests/unit/Db/RowSleeveMapperTest.php new file mode 100644 index 0000000000..b035e3c4cd --- /dev/null +++ b/tests/unit/Db/RowSleeveMapperTest.php @@ -0,0 +1,98 @@ +mapper = new RowSleeveMapper($this->connectionAdapter, new NullLogger()); + $this->connectionAdapter->beginTransaction(); + } + + protected function tearDown(): void { + $this->connectionAdapter->rollBack(); + parent::tearDown(); + } + + private function sleeve(int $tableId = 1): RowSleeve { + $sleeve = new RowSleeve(); + $sleeve->setTableId($tableId); + $sleeve->setCreatedBy('alice'); + $sleeve->setLastEditBy('alice'); + $sleeve->setCreatedAt('2020-01-01 00:00:00'); + $sleeve->setLastEditAt('2020-01-02 00:00:00'); + return $sleeve; + } + + public function testEveryInsertionGetsStoredUuidAndProjectionIncludesIt(): void { + $first = $this->mapper->insert($this->sleeve()); + $second = $this->mapper->insert($this->sleeve(2)); + $this->assertNotNull($first->getUuid()); + $this->assertNotSame($first->getUuid(), $second->getUuid()); + $this->assertSame($first->getUuid(), $this->mapper->find($first->getId())->getUuid()); + $this->assertSame($first->getUuid(), $this->mapper->findMultiple([$first->getId()])[0]->getUuid()); + $this->assertSame($first->getId(), $this->mapper->findByUuid($first->getUuid())->getId()); + } + + public function testExplicitLegacyIdAlsoGetsUuid(): void { + $sleeve = $this->sleeve(); + $sleeve->setId(900001); + $this->mapper->insert($sleeve); + $this->assertNotNull($this->mapper->find(900001)->getUuid()); + } + + public function testImportPreservesUnusedIdentityAndCopiesCollisionsAcrossTables(): void { + $sleeve = $this->sleeve(); + $uuid = '019f90c5-8576-7f0b-9b14-9bf49b2826cf'; + $sleeve->setUuid($uuid); + $first = $this->mapper->insertImported($sleeve); + $this->assertSame($uuid, $first->getUuid()); + $sleeve->setTableId(2); + $copy = $this->mapper->insertImported($sleeve); + $this->assertNotSame($uuid, $copy->getUuid()); + $this->assertTrue($this->connectionAdapter->inTransaction()); + $this->assertSame($uuid, $this->mapper->find($first->getId())->getUuid()); + $this->assertSame('2020-01-01 00:00:00', $copy->getCreatedAt()); + $this->assertSame($copy->getUuid(), $this->mapper->find($copy->getId())->getUuid()); + } + + public function testUniqueIndexAppliesAcrossTables(): void { + $first = $this->mapper->insert($this->sleeve()); + $second = $this->sleeve(2); + $second->setUuid($first->getUuid()); + $this->expectException(Exception::class); + $this->mapper->insert($second); + } + + public function testMultipleHistoricalNullsRemainReadable(): void { + $insert = $this->connectionAdapter->getQueryBuilder(); + $insert->insert('tables_row_sleeves')->values([ + 'table_id' => $insert->createNamedParameter(1), + 'created_by' => $insert->createNamedParameter('alice'), + 'created_at' => $insert->createNamedParameter('2020-01-01 00:00:00'), + 'last_edit_by' => $insert->createNamedParameter('alice'), + 'last_edit_at' => $insert->createNamedParameter('2020-01-01 00:00:00'), + ]); + $insert->executeStatement(); + $first = $insert->getLastInsertId(); + $insert->executeStatement(); + $second = $insert->getLastInsertId(); + $this->assertNull($this->mapper->find($first)->getUuid()); + $this->assertNull($this->mapper->find($second)->getUuid()); + } +} diff --git a/tests/unit/Db/RowSleeveTest.php b/tests/unit/Db/RowSleeveTest.php new file mode 100644 index 0000000000..55ff23956c --- /dev/null +++ b/tests/unit/Db/RowSleeveTest.php @@ -0,0 +1,74 @@ + 17, 'uuid' => null]); + $this->assertNull($sleeve->getUuid()); + $this->assertNull($sleeve->jsonSerialize()['uuid']); + $this->assertSame([], $sleeve->getUpdatedFields()); + } + + public function testHydrationPreservesAssignedUuid(): void { + $sleeve = RowSleeve::fromRow(['id' => 17, 'uuid' => self::UUID]); + $this->assertSame(self::UUID, $sleeve->getUuid()); + $this->assertSame([], $sleeve->getUpdatedFields()); + } + + public function testHydrationDoesNotRejectStoredValues(): void { + $sleeve = RowSleeve::fromRow(['id' => 17, 'uuid' => 'not-a-uuid']); + $this->assertSame('not-a-uuid', $sleeve->getUuid()); + $this->assertSame([], $sleeve->getUpdatedFields()); + } + + public function testAssignmentIsExplicitAndIdempotent(): void { + $sleeve = new RowSleeve(); + $this->assertNull($sleeve->getUuid()); + $sleeve->assignUuid(); + $uuid = $sleeve->getUuid(); + $this->assertMatchesRegularExpression('/^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/D', $uuid); + $this->assertArrayHasKey('uuid', $sleeve->getUpdatedFields()); + $sleeve->assignUuid(); + $this->assertSame($uuid, $sleeve->jsonSerialize()['uuid']); + } + + public function testArchiveUuidIsNormalized(): void { + $sleeve = new RowSleeve(); + $sleeve->setUuid(strtoupper(self::UUID)); + $this->assertSame(self::UUID, $sleeve->getUuid()); + } + + /** @dataProvider immutableChanges */ + public function testAssignedIdentityCannotChange(?string $uuid): void { + $sleeve = RowSleeve::fromRow(['uuid' => self::UUID]); + $this->expectException(\InvalidArgumentException::class); + $sleeve->setUuid($uuid); + } + + public static function immutableChanges(): array { + return [[null], ['019f90c5-8576-7f0b-9b14-9bf49b2826c0']]; + } + + /** @dataProvider invalidUuids */ + public function testInvalidUuidFails(string $uuid): void { + $this->expectException(\InvalidArgumentException::class); + (new RowSleeve())->setUuid($uuid); + } + + public static function invalidUuids(): array { + return [[''], ['invalid'], ['019f90c5-8576-4f0b-9b14-9bf49b2826cf'], ['019f90c5-8576-7f0b-0b14-9bf49b2826cf']]; + } +} diff --git a/tests/unit/Model/RowDataInputTest.php b/tests/unit/Model/RowDataInputTest.php new file mode 100644 index 0000000000..82be41d714 --- /dev/null +++ b/tests/unit/Model/RowDataInputTest.php @@ -0,0 +1,46 @@ + '0012', 'value' => null], + ['columnId' => '-2', 'value' => 'alice'], + ['columnId' => 13, 'value' => true], + ]); + $this->assertSame([ + ['columnId' => 12, 'value' => null], + ['columnId' => -2, 'value' => 'alice'], + ['columnId' => 13, 'value' => 1], + ], iterator_to_array($input)); + } + + /** @dataProvider malformedCells */ + public function testMalformedCellsAreRejected(array $cells): void { + $this->expectException(\InvalidArgumentException::class); + RowDataInput::createFromInputArray($cells); + } + + public static function malformedCells(): array { + return [ + [['column_1' => 'value']], + [[['columnId' => 1]]], + [[['value' => 'value']]], + [[['columnId' => 'garbage', 'value' => null]]], + [[['columnId' => '92233720368547758070', 'value' => null]]], + [[['columnId' => 1.5, 'value' => null]]], + [[['columnId' => true, 'value' => null]]], + ]; + } +} diff --git a/tests/unit/Model/RowIdentifierTest.php b/tests/unit/Model/RowIdentifierTest.php new file mode 100644 index 0000000000..e5f79b98d9 --- /dev/null +++ b/tests/unit/Model/RowIdentifierTest.php @@ -0,0 +1,57 @@ +assertSame($expected, $identifier->value); + $this->assertSame(is_string($expected), $identifier->isUuid()); + } + + public static function validIdentifiers(): array { + return [ + ['1', 1], ['00123', 123], ['2147483647', 2147483647], + ['019F90C5-8576-7F0B-9B14-9BF49B2826CF', '019f90c5-8576-7f0b-9b14-9bf49b2826cf'], + ]; + } + + /** @dataProvider invalidIdentifiers */ + public function testInvalidIdentifier(string $input): void { + $this->expectException(\InvalidArgumentException::class); + RowIdentifier::fromString($input); + } + + public static function invalidIdentifiers(): array { + return array_map(static fn (string $input): array => [$input], [ + '', '0', '000', '-1', '+1', ' 1', '1 ', '1.0', '1e3', "1\n", '12', + '2147483648', (string)PHP_INT_MAX, (string)PHP_INT_MAX . '0', '019f90c5-8576-4f0b-9b14-9bf49b2826cf', + '019f90c5-8576-7f0b-0b14-9bf49b2826cf', '019f90c585767f0b9b149bf49b2826cf', + ]); + } + + public function testIntegerWithinColumnRangeBecomesIdentifier(): void { + $this->assertSame(13, RowIdentifier::tryFromInt(13)?->value); + $this->assertSame(2147483647, RowIdentifier::tryFromInt(2147483647)?->value); + } + + /** @dataProvider integersOutsideColumnRange */ + public function testIntegerOutsideColumnRangeHasNoIdentifier(int $rowId): void { + $this->assertNull(RowIdentifier::tryFromInt($rowId)); + } + + public static function integersOutsideColumnRange(): array { + return [[0], [-1], [2147483648], [PHP_INT_MAX]]; + } +} diff --git a/tests/unit/Service/ImportServiceRowUpdateTest.php b/tests/unit/Service/ImportServiceRowUpdateTest.php new file mode 100644 index 0000000000..8e86f58feb --- /dev/null +++ b/tests/unit/Service/ImportServiceRowUpdateTest.php @@ -0,0 +1,128 @@ +temporaryFile = sys_get_temp_dir() . '/' . uniqid('tables-import-', true) . '.csv'; + $this->rows = $this->createMock(RowService::class); + } + + protected function tearDown(): void { + if (file_exists($this->temporaryFile)) { + unlink($this->temporaryFile); + } + parent::tearDown(); + } + + private function column(int $id, string $title): Column { + $column = new Column(); + $column->setId($id); + $column->setTitle($title); + $column->setType(Column::TYPE_TEXT); + $column->setSubtype(Column::SUBTYPE_TEXT_LINE); + $column->setMandatory(false); + return $column; + } + + private function createService(): ImportService { + $file = $this->createMock(ISimpleFile::class); + $file->method('getExtension')->willReturn('csv'); + $file->method('getContent')->willReturn(self::CSV); + $folder = $this->createMock(ISimpleFolder::class); + $folder->method('getFile')->willReturn($file); + $appData = $this->createMock(IAppData::class); + $appData->method('getFolder')->willReturn($folder); + $appDataFactory = $this->createMock(IAppDataFactory::class); + $appDataFactory->method('get')->willReturn($appData); + $tempManager = $this->createMock(ITempManager::class); + $tempManager->method('getTemporaryFile')->willReturn($this->temporaryFile); + + $columns = $this->createMock(ColumnService::class); + $columns->method('findOrCreateColumnsByTitleForTableAsArray')->willReturn([ + $this->column(Column::TYPE_META_ID, 'ID'), + $this->column(7, 'Title'), + ]); + $business = $this->createMock(IColumnTypeBusiness::class); + $business->method('canBeParsedDisplayValue')->willReturn(true); + $business->method('parseDisplayValue')->willReturn('"parsed"'); + $columnsHelper = $this->createMock(ColumnsHelper::class); + $columnsHelper->method('getColumnBusinessObject')->willReturn($business); + + return new ImportService( + $this->createMock(PermissionsService::class), + new NullLogger(), + 'alice', + $this->createMock(IRootFolder::class), + $columns, + $this->rows, + $this->createMock(TableService::class), + $this->createMock(ViewService::class), + $this->createMock(IUserManager::class), + $columnsHelper, + $this->createMock(IJobList::class), + $tempManager, + $this->createMock(IFilenameValidator::class), + $appDataFactory, + $this->createMock(AuditLogServiceInterface::class), + ); + } + + public function testRowMissingFromDestinationFailsOnlyThatLine(): void { + $this->rows->expects($this->once())->method('updateSet') + ->with(999, null, [['columnId' => 7, 'value' => 'parsed']], 'alice', 5) + ->willThrowException(new NotFoundError('Row not found.')); + $this->rows->expects($this->once())->method('create'); + + $stats = $this->createService()->importV2('alice', ImportService::IMPORT_TYPE_APP_FILE, 'import.csv', 5, null); + + $this->assertSame(1, $stats->errorsCount); + $this->assertSame(0, $stats->updatedRowsCount); + $this->assertSame(1, $stats->insertedRowsCount); + } + + public function testMissingDestinationStillAbortsTheImport(): void { + $this->rows->method('updateSet'); + $this->rows->method('create')->willThrowException(new NotFoundError('Table not found.')); + + $this->expectException(NotFoundError::class); + + $this->createService()->importV2('alice', ImportService::IMPORT_TYPE_APP_FILE, 'import.csv', 5, null); + } +} diff --git a/tests/unit/Service/LegacyRowMapperTest.php b/tests/unit/Service/LegacyRowMapperTest.php index 7822d50556..4933feb744 100644 --- a/tests/unit/Service/LegacyRowMapperTest.php +++ b/tests/unit/Service/LegacyRowMapperTest.php @@ -139,7 +139,8 @@ public function testMigrateLegacyRow() { self::assertTrue($data === $data2); $row2Serialized = $row2->jsonSerialize(); - unset($row2Serialized['dataByAlias']); + $this->assertNull($row2Serialized['uuid']); + unset($row2Serialized['dataByAlias'], $row2Serialized['uuid']); self::assertTrue($legacyRow->jsonSerialize() === $row2Serialized); } } diff --git a/tests/unit/Service/RowServiceUuidTest.php b/tests/unit/Service/RowServiceUuidTest.php new file mode 100644 index 0000000000..5dd44ad5a4 --- /dev/null +++ b/tests/unit/Service/RowServiceUuidTest.php @@ -0,0 +1,175 @@ +permissions = $this->createMock(PermissionsService::class); + $this->rows = $this->createMock(Row2Mapper::class); + $this->columns = $this->createMock(ColumnMapper::class); + $this->views = $this->createMock(ViewMapper::class); + $this->service = new RowService(new NullLogger(), 'alice', $this->permissions, $this->columns, $this->views, + $this->createMock(TableMapper::class), $this->rows, $this->createMock(IEventDispatcher::class), + $this->createMock(ColumnsHelper::class), $this->createMock(ActivityManager::class), $this->createMock(NotificationHelper::class)); + } + + private function view(): View { + $view = new View(); + $view->setId(8); + $view->setTableId(5); + $view->setColumnsArray([['columnId' => 1, 'order' => 0]]); + $view->setFilterArray([[['columnId' => -1, 'operator' => 'is-equal', 'value' => '13']]]); + return $view; + } + + private function row(): Row2 { + $row = new Row2(); + $row->setId(13); + $row->setTableId(5); + $row->setUuid('019f90c5-8576-7f0b-9b14-9bf49b2826cf'); + $row->setData([['columnId' => 1, 'value' => 'visible'], ['columnId' => 2, 'value' => 'hidden']]); + return $row; + } + + public function testViewOnlyAccessUsesViewScopeAndProjectsBothCellRepresentations(): void { + $view = $this->view(); + $this->permissions->expects($this->once())->method('canReadRowsByElementId')->with(8, 'view', 'alice')->willReturn(true); + $this->views->method('find')->with(8)->willReturn($view); + $this->rows->expects($this->once())->method('findScopedId')->with($this->isInstanceOf(RowIdentifier::class), 5, $view->getFilterArray(), 'alice')->willReturn(13); + $column = new Column(); + $column->setId(1); + $column->setTechnicalName('visible'); + $this->columns->method('findAll')->with([1])->willReturn([$column]); + $this->columns->expects($this->never())->method('findAllByTable'); + $original = $this->row(); + $this->rows->method('find')->with(13, [$column])->willReturn($original); + $result = $this->service->findByIdentifier(RowIdentifier::fromString('13'), Application::NODE_TYPE_VIEW, 8, 'alice'); + $this->assertSame([['columnId' => 1, 'value' => 'visible']], $result->getData()); + $this->assertSame(['visible' => ['columnId' => 1, 'value' => 'visible']], $result->getDataByAlias()); + $this->assertCount(2, $original->getData()); + $this->assertNotSame($original, $result); + $this->assertSame($original->getUuid(), $result->getUuid()); + } + + public function testMissingRowReadPermissionIsDeniedBeforeLookup(): void { + $this->permissions->method('canReadRowsByElementId')->willReturn(false); + $this->rows->expects($this->never())->method('findScopedId'); + $this->expectException(PermissionError::class); + $this->service->findByIdentifier(RowIdentifier::fromString('13'), Application::NODE_TYPE_TABLE, 5, 'alice'); + } + + public function testResolvingRowIdDoesNotLoadTheRow(): void { + $view = $this->view(); + $this->permissions->expects($this->once())->method('canReadRowsByElementId')->with(8, 'view', 'alice')->willReturn(true); + $this->views->method('find')->with(8)->willReturn($view); + $this->rows->expects($this->once())->method('findScopedId')->with($this->isInstanceOf(RowIdentifier::class), 5, $view->getFilterArray(), 'alice')->willReturn(13); + $this->rows->expects($this->never())->method('find'); + $this->columns->expects($this->never())->method('findAll'); + $this->assertSame(13, $this->service->resolveRowId(RowIdentifier::fromString('13'), Application::NODE_TYPE_VIEW, 8, 'alice')); + } + + public function testScopeMismatchIsNotFoundWithoutLoadingCells(): void { + $this->permissions->method('canReadRowsByElementId')->willReturn(true); + $this->rows->method('findScopedId')->willReturn(null); + $this->rows->expects($this->never())->method('find'); + $this->expectException(NotFoundError::class); + $this->service->findByIdentifier(RowIdentifier::fromString('13'), Application::NODE_TYPE_TABLE, 5, 'alice'); + } + + /** @dataProvider archiveIdentities */ + public function testArchiveIdentityGoesThroughSleeveInsertionBoundary(mixed $uuid, ?string $expected, bool $includeUuid = true): void { + $table = new Table(); + $table->setId(5); + $table->setOwnership('alice'); + $this->rows->expects($this->once())->method('importSleeve')->willReturnCallback(function (RowSleeve $sleeve) use ($expected): RowSleeve { + $this->assertSame($expected, $sleeve->getUuid()); + $this->assertSame(5, $sleeve->getTableId()); + $this->assertSame('alice', $sleeve->getCreatedBy()); + $sleeve->setId(99); + $sleeve->assignUuid(); + return $sleeve; + }); + $archive = ['createdAt' => '2020-01-01 00:00:00', 'lastEditAt' => '2020-01-02 00:00:00']; + if ($includeUuid) { + $archive['uuid'] = $uuid; + } + $this->assertSame(99, $this->service->importRow($table, $archive)); + } + + public static function archiveIdentities(): array { + return [[null, null, false], [null, null], ['019F90C5-8576-7F0B-9B14-9BF49B2826CF', '019f90c5-8576-7f0b-9b14-9bf49b2826cf']]; + } + + /** @dataProvider invalidArchiveIdentities */ + public function testInvalidArchiveIdentityFailsBeforeWriting(mixed $uuid): void { + $this->rows->expects($this->never())->method('importSleeve'); + $this->expectException(\InvalidArgumentException::class); + $table = new Table(); + $table->setId(5); + $table->setOwnership('alice'); + $this->service->importRow($table, ['uuid' => $uuid]); + } + + public static function invalidArchiveIdentities(): array { + return [[3], [[]], [''], ['019f90c5-8576-4f0b-9b14-9bf49b2826cf']]; + } + + /** @dataProvider mutationMethods */ + public function testRowRemovedBeforeMutationIsNotFound(string $method): void { + $this->rows->method('getTableIdForRow')->willThrowException(new \OCP\AppFramework\Db\DoesNotExistException('Deleted row')); + $this->expectException(NotFoundError::class); + if ($method === 'updateSet') { + $this->service->updateSet(13, null, [], 'alice', 5); + } else { + $this->service->delete(13, null, 'alice', 5); + } + } + + public static function mutationMethods(): array { + return [['updateSet'], ['delete']]; + } + + public function testPublicFormatterKeepsUuidAndRemovesPrivateMetadata(): void { + $row = $this->row(); + $result = $this->service->formatRowsForPublicShare([$row])[0]; + $this->assertSame($row->getUuid(), $result['uuid']); + $this->assertArrayNotHasKey('tableId', $result); + $this->assertArrayNotHasKey('createdBy', $result); + $this->assertArrayNotHasKey('lastEditBy', $result); + } +}