From 7b544632f38f2b15339498ed0f57426c7737d73b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 19 Jun 2025 14:36:39 +0200 Subject: [PATCH 1/3] feat: add last modified information to response Signed-off-by: Arthur Schiwon --- lib/Controller/Api1Controller.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index 240891f302..d4a83180f1 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -1257,7 +1257,12 @@ public function indexTableRowsSimple(int $tableId, ?int $limit, ?int $offset): D #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function indexTableRows(int $tableId, ?int $limit, ?int $offset): DataResponse { try { - return new DataResponse($this->rowService->formatRows($this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset))); + $rows = $this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset); + $response = new DataResponse($this->rowService->formatRows($rows)); + $table = $this->tableService->find($tableId); + $lastModified = new \DateTime($table->getLastEditAt()); + $response->setLastModified($lastModified); + return $response; } catch (PermissionError $e) { $this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]); $message = ['message' => $e->getMessage()]; From 0dc898a19c0fe9a0e0ab7dabfe47903452b31f2c Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Sun, 30 Aug 2026 02:02:52 +0100 Subject: [PATCH 2/3] feat: implement last modified handling Signed-off-by: Enjeck C. --- lib/Controller/Api1Controller.php | 27 +++++++++++++++++++++++---- lib/Db/TableMapper.php | 23 +++++++++++++++++++++++ lib/Service/RowService.php | 5 +++++ openapi.json | 10 ++++++++++ src/types/openapi/openapi.ts | 11 ++++++++++- 5 files changed, 71 insertions(+), 5 deletions(-) diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index d4a83180f1..ed5d9aa5d7 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -42,6 +42,7 @@ use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; +use OCP\Constants; use OCP\IL10N; use OCP\IRequest; use Psr\Log\LoggerInterface; @@ -1244,9 +1245,10 @@ public function indexTableRowsSimple(int $tableId, ?int $limit, ?int $offset): D * @param int $tableId Table ID * @param int|null $limit Limit * @param int|null $offset Offset - * @return DataResponse, array{}>|DataResponse + * @return DataResponse, array{}>|DataResponse|DataResponse * * 200: Rows returned + * 304: Not modified * 403: No permissions * 404: Not found */ @@ -1257,16 +1259,33 @@ public function indexTableRowsSimple(int $tableId, ?int $limit, ?int $offset): D #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function indexTableRows(int $tableId, ?int $limit, ?int $offset): DataResponse { try { + $table = $this->tableService->find($tableId); + $lastEditAt = $table->getLastEditAt(); + $lastModified = $lastEditAt !== null && $lastEditAt !== '' ? new \DateTime($lastEditAt) : null; + + if ($lastModified !== null) { + $ifModifiedSince = $this->request->getHeader('IF_MODIFIED_SINCE'); + if ($ifModifiedSince !== '' && trim($ifModifiedSince) === $lastModified->format(Constants::DATE_RFC7231)) { + $response = new DataResponse([], Http::STATUS_NOT_MODIFIED); + $response->setLastModified($lastModified); + return $response; + } + } + $rows = $this->rowService->findAllByTable($tableId, $this->userId, $limit, $offset); $response = new DataResponse($this->rowService->formatRows($rows)); - $table = $this->tableService->find($tableId); - $lastModified = new \DateTime($table->getLastEditAt()); - $response->setLastModified($lastModified); + if ($lastModified !== null) { + $response->setLastModified($lastModified); + } return $response; } catch (PermissionError $e) { $this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]); $message = ['message' => $e->getMessage()]; return new DataResponse($message, Http::STATUS_FORBIDDEN); + } catch (NotFoundError $e) { + $this->logger->warning('A not found error occurred: ' . $e->getMessage(), ['exception' => $e]); + $message = ['message' => $e->getMessage()]; + return new DataResponse($message, Http::STATUS_NOT_FOUND); } catch (InternalError|Exception $e) { $this->logger->error('An internal error or exception occurred: ' . $e->getMessage(), ['exception' => $e]); $message = ['message' => $e->getMessage()]; diff --git a/lib/Db/TableMapper.php b/lib/Db/TableMapper.php index 5a9ad8d368..1c0e6a43d1 100644 --- a/lib/Db/TableMapper.php +++ b/lib/Db/TableMapper.php @@ -219,6 +219,29 @@ public function insert(Entity $entity): Table { return $entity; } + public function update(Entity $entity): Table { + /** @var Table $entity */ + $entity = parent::update($entity); + $this->cache[(string)$entity->getId()] = $entity; + return $entity; + } + + /** + * @throws Exception + */ + public function touch(int $id, ?string $userId = null, ?\DateTimeInterface $time = null): void { + $time = $time ?? new \DateTime(); + $qb = $this->db->getQueryBuilder(); + $qb->update($this->table) + ->set('last_edit_at', $qb->createNamedParameter($time->format('Y-m-d H:i:s'), IQueryBuilder::PARAM_STR)) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))); + if ($userId !== null && $userId !== '') { + $qb->set('last_edit_by', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)); + } + $qb->executeStatement(); + unset($this->cache[(string)$id]); + } + public function getDbConnection() { return $this->db; } diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 95baa53324..6278870d73 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -271,6 +271,7 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data, ?s $row2->setData($data); try { $insertedRow = $this->row2Mapper->insert($row2, $this->userId); + $this->tableMapper->touch($tableId, $this->userId); $this->attachAliasPayload($insertedRow, $columns); $this->eventDispatcher->dispatchTyped(new RowAddedEvent($insertedRow)); @@ -704,6 +705,7 @@ public function updateSet( } $updatedRow = $this->row2Mapper->update($item, $this->userId); + $this->tableMapper->touch($item->getTableId(), $this->userId); $this->attachAliasPayload($updatedRow, $columns); $this->eventDispatcher->dispatchTyped(new RowUpdatedEvent($updatedRow, $previousData)); @@ -802,6 +804,7 @@ public function delete(int $id, ?int $viewId, string $userId, ?int $tableId = nu try { $columns = $this->loadColumnsForData($item->getData() ?? []); $deletedRow = $this->row2Mapper->delete($item); + $this->tableMapper->touch($item->getTableId(), $userId); $this->attachAliasPayload($item, $columns); $event = new RowDeletedEvent($item, $item->getData()); @@ -843,6 +846,7 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { $columns = $this->columnMapper->findAllByTable($tableId); $this->row2Mapper->deleteAllForTable($tableId, $columns); + $this->tableMapper->touch($tableId, $userId ?? $this->userId); } /** @@ -858,6 +862,7 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { */ public function deleteColumnDataFromRows(Column $column):void { $this->row2Mapper->deleteDataForColumn($column); + $this->tableMapper->touch($column->getTableId(), $this->userId); } /** diff --git a/openapi.json b/openapi.json index 412df8cf42..4cc3ea1a08 100644 --- a/openapi.json +++ b/openapi.json @@ -5194,6 +5194,13 @@ "format": "int64", "nullable": true } + }, + { + "name": "if-modified-since", + "in": "header", + "schema": { + "type": "string" + } } ], "responses": { @@ -5210,6 +5217,9 @@ } } }, + "304": { + "description": "Not modified" + }, "403": { "description": "No permissions", "content": { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 665abe8276..4f1b8a5d7c 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -3707,7 +3707,9 @@ export interface operations { /** @description Offset */ readonly offset?: number | null; }; - readonly header?: never; + readonly header?: { + readonly "if-modified-since"?: string; + }; readonly path: { /** @description Table ID */ readonly tableId: number; @@ -3725,6 +3727,13 @@ export interface operations { readonly "application/json": readonly components["schemas"]["Row"][]; }; }; + /** @description Not modified */ + readonly 304: { + headers: { + readonly [name: string]: unknown; + }; + content?: never; + }; /** @description Current user is not logged in */ readonly 401: { headers: { From 54c87d94d1a27771159b8538826260bc5a947360 Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Sun, 30 Aug 2026 09:18:48 +0100 Subject: [PATCH 3/3] refactor: move functions Signed-off-by: Enjeck C. --- lib/Controller/Api1Controller.php | 17 +++++---- lib/Db/TableMapper.php | 9 +---- lib/Service/ColumnService.php | 15 ++++++++ lib/Service/RowService.php | 21 ++++++++--- openapi.json | 1 + src/types/openapi/openapi.ts | 1 + tests/unit/Db/TableMapperTest.php | 59 +++++++++++++++++++++++++++++++ 7 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 tests/unit/Db/TableMapperTest.php diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index ed5d9aa5d7..c9714902cb 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -40,6 +40,7 @@ use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\Attribute\RequestHeader; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\Constants; @@ -1257,18 +1258,22 @@ public function indexTableRowsSimple(int $tableId, ?int $limit, ?int $offset): D #[CORS] #[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] + #[RequestHeader(name: 'if-modified-since', description: 'Respond with 304 Not Modified if the table has not been edited since this date')] public function indexTableRows(int $tableId, ?int $limit, ?int $offset): DataResponse { try { - $table = $this->tableService->find($tableId); + $table = $this->tableService->find($tableId, true); $lastEditAt = $table->getLastEditAt(); $lastModified = $lastEditAt !== null && $lastEditAt !== '' ? new \DateTime($lastEditAt) : null; if ($lastModified !== null) { - $ifModifiedSince = $this->request->getHeader('IF_MODIFIED_SINCE'); - if ($ifModifiedSince !== '' && trim($ifModifiedSince) === $lastModified->format(Constants::DATE_RFC7231)) { - $response = new DataResponse([], Http::STATUS_NOT_MODIFIED); - $response->setLastModified($lastModified); - return $response; + $ifModifiedSince = trim($this->request->getHeader('if-modified-since')); + if ($ifModifiedSince !== '') { + $modifiedSince = \DateTime::createFromFormat(Constants::DATE_RFC7231, $ifModifiedSince); + if ($modifiedSince !== false && $lastModified->getTimestamp() <= $modifiedSince->getTimestamp()) { + $response = new DataResponse([], Http::STATUS_NOT_MODIFIED); + $response->setLastModified($lastModified); + return $response; + } } } diff --git a/lib/Db/TableMapper.php b/lib/Db/TableMapper.php index 1c0e6a43d1..47a463455a 100644 --- a/lib/Db/TableMapper.php +++ b/lib/Db/TableMapper.php @@ -219,18 +219,11 @@ public function insert(Entity $entity): Table { return $entity; } - public function update(Entity $entity): Table { - /** @var Table $entity */ - $entity = parent::update($entity); - $this->cache[(string)$entity->getId()] = $entity; - return $entity; - } - /** * @throws Exception */ public function touch(int $id, ?string $userId = null, ?\DateTimeInterface $time = null): void { - $time = $time ?? new \DateTime(); + $time ??= new \DateTime(); $qb = $this->db->getQueryBuilder(); $qb->update($this->table) ->set('last_edit_at', $qb->createNamedParameter($time->format('Y-m-d H:i:s'), IQueryBuilder::PARAM_STR)) diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 52a84b03c1..799dee41b9 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -345,6 +345,8 @@ public function create( author: $userId ?? $this->userId, ); + $this->touchTable($table->getId(), $userId ?? $this->userId); + return $this->enhanceColumn($entity); } @@ -442,6 +444,8 @@ public function update( author: $userId ?? $this->userId, ); + $this->touchTable($updatedColumn->getTableId(), $userId ?? $this->userId); + return $this->enhanceColumn($updatedColumn); } catch (\OCP\DB\Exception $e) { $this->handleColumnPersistDbException($e, static::class . ' - ' . __FUNCTION__); @@ -528,6 +532,17 @@ private function updateMetadata(Column $column, ?string $userId, bool $setCreate } } + /** + * Updates the table's last-modified metadata after a column change. + */ + private function touchTable(int $tableId, ?string $userId): void { + try { + $this->tableMapper->touch($tableId, $userId); + } catch (\OCP\DB\Exception $e) { + $this->logger->warning('Failed to update last modified timestamp for table ' . $tableId . ': ' . $e->getMessage(), ['exception' => $e]); + } + } + /** * @param int $id * @param bool $skipRowCleanup diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 6278870d73..043d31bfb8 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -271,7 +271,7 @@ public function create(?int $tableId, ?int $viewId, RowDataInput|array $data, ?s $row2->setData($data); try { $insertedRow = $this->row2Mapper->insert($row2, $this->userId); - $this->tableMapper->touch($tableId, $this->userId); + $this->touchTable($tableId, $this->userId); $this->attachAliasPayload($insertedRow, $columns); $this->eventDispatcher->dispatchTyped(new RowAddedEvent($insertedRow)); @@ -705,7 +705,7 @@ public function updateSet( } $updatedRow = $this->row2Mapper->update($item, $this->userId); - $this->tableMapper->touch($item->getTableId(), $this->userId); + $this->touchTable($item->getTableId(), $this->userId); $this->attachAliasPayload($updatedRow, $columns); $this->eventDispatcher->dispatchTyped(new RowUpdatedEvent($updatedRow, $previousData)); @@ -804,7 +804,7 @@ public function delete(int $id, ?int $viewId, string $userId, ?int $tableId = nu try { $columns = $this->loadColumnsForData($item->getData() ?? []); $deletedRow = $this->row2Mapper->delete($item); - $this->tableMapper->touch($item->getTableId(), $userId); + $this->touchTable($item->getTableId(), $userId); $this->attachAliasPayload($item, $columns); $event = new RowDeletedEvent($item, $item->getData()); @@ -846,7 +846,7 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { $columns = $this->columnMapper->findAllByTable($tableId); $this->row2Mapper->deleteAllForTable($tableId, $columns); - $this->tableMapper->touch($tableId, $userId ?? $this->userId); + $this->touchTable($tableId, $userId ?? $this->userId); } /** @@ -862,7 +862,18 @@ public function deleteAllByTable(int $tableId, ?string $userId = null): void { */ public function deleteColumnDataFromRows(Column $column):void { $this->row2Mapper->deleteDataForColumn($column); - $this->tableMapper->touch($column->getTableId(), $this->userId); + $this->touchTable($column->getTableId(), $this->userId); + } + + /** + * Updates the table's last-modified metadata. + */ + private function touchTable(int $tableId, ?string $userId): void { + try { + $this->tableMapper->touch($tableId, $userId); + } catch (Exception $e) { + $this->logger->warning('Failed to update last modified timestamp for table ' . $tableId . ': ' . $e->getMessage(), ['exception' => $e]); + } } /** diff --git a/openapi.json b/openapi.json index 4cc3ea1a08..4e3c199706 100644 --- a/openapi.json +++ b/openapi.json @@ -5198,6 +5198,7 @@ { "name": "if-modified-since", "in": "header", + "description": "Respond with 304 Not Modified if the table has not been edited since this date", "schema": { "type": "string" } diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 4f1b8a5d7c..3166463cb3 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -3708,6 +3708,7 @@ export interface operations { readonly offset?: number | null; }; readonly header?: { + /** @description Respond with 304 Not Modified if the table has not been edited since this date */ readonly "if-modified-since"?: string; }; readonly path: { diff --git a/tests/unit/Db/TableMapperTest.php b/tests/unit/Db/TableMapperTest.php new file mode 100644 index 0000000000..0df858ee8c --- /dev/null +++ b/tests/unit/Db/TableMapperTest.php @@ -0,0 +1,59 @@ +cleanupTablesData(); + $this->mapper = new TableMapper($this->connectionAdapter, $this->createMock(UserHelper::class)); + } + + protected function tearDown(): void { + $this->cleanupTablesData(); + parent::tearDown(); + } + + public function testTouchUpdatesLastEditAtAndBy(): void { + $table = $this->createTestTable(['last_edit_at' => '2020-01-01 00:00:00', 'last_edit_by' => 'user1']); + + $this->mapper->touch($table['id'], 'user2', new \DateTime('2030-05-01 12:00:00')); + + $updated = $this->mapper->find($table['id']); + $this->assertSame('2030-05-01 12:00:00', $updated->getLastEditAt()); + $this->assertSame('user2', $updated->getLastEditBy()); + } + + public function testTouchWithoutUserIdKeepsExistingLastEditBy(): void { + $table = $this->createTestTable(['last_edit_at' => '2020-01-01 00:00:00', 'last_edit_by' => 'user1']); + + $this->mapper->touch($table['id'], null, new \DateTime('2030-05-01 12:00:00')); + + $updated = $this->mapper->find($table['id']); + $this->assertSame('2030-05-01 12:00:00', $updated->getLastEditAt()); + $this->assertSame('user1', $updated->getLastEditBy()); + } + + public function testTouchInvalidatesMapperCache(): void { + $table = $this->createTestTable(['last_edit_at' => '2020-01-01 00:00:00']); + $this->mapper->find($table['id']); // warms the cache + + $this->mapper->touch($table['id'], 'user2', new \DateTime('2030-05-01 12:00:00')); + + $updated = $this->mapper->find($table['id']); + $this->assertSame('2030-05-01 12:00:00', $updated->getLastEditAt()); + } +}