From 32e3100e836af5390b60e42537d11f7769e52a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 22 Feb 2024 16:40:45 +0100 Subject: [PATCH 01/23] feat: Add backend for table archive and favorite flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/routes.php | 9 ++- lib/Controller/ApiFavoriteController.php | 80 +++++++++++++++++++ lib/Controller/ApiTablesController.php | 4 +- lib/Controller/TableController.php | 6 +- lib/Db/Table.php | 9 +++ .../Version000800Date20240222000000.php | 59 ++++++++++++++ lib/Service/FavoritesService.php | 80 +++++++++++++++++++ lib/Service/TableService.php | 16 +++- 8 files changed, 254 insertions(+), 9 deletions(-) create mode 100644 lib/Controller/ApiFavoriteController.php create mode 100644 lib/Migration/Version000800Date20240222000000.php create mode 100644 lib/Service/FavoritesService.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 186a7fef5..e58ee8e4e 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -17,7 +17,7 @@ // -> tables ['name' => 'api1#index', 'url' => '/api/1/tables', 'verb' => 'GET'], ['name' => 'api1#createTable', 'url' => '/api/1/tables', 'verb' => 'POST'], - ['name' => 'api1#updateTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'PUT'], + ['name' => 'api1#updateTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'PUT'], // needs archived ['name' => 'api1#getTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'GET'], ['name' => 'api1#deleteTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'DELETE'], // -> views @@ -61,7 +61,7 @@ ['name' => 'table#index', 'url' => '/table', 'verb' => 'GET'], ['name' => 'table#show', 'url' => '/table/{id}', 'verb' => 'GET'], ['name' => 'table#create', 'url' => '/table', 'verb' => 'POST'], - ['name' => 'table#update', 'url' => '/table/{id}', 'verb' => 'PUT'], + ['name' => 'table#update', 'url' => '/table/{id}', 'verb' => 'PUT'], // needs archived ['name' => 'table#destroy', 'url' => '/table/{id}', 'verb' => 'DELETE'], // view @@ -115,7 +115,7 @@ ['name' => 'ApiTables#index', 'url' => '/api/2/tables', 'verb' => 'GET'], ['name' => 'ApiTables#show', 'url' => '/api/2/tables/{id}', 'verb' => 'GET'], ['name' => 'ApiTables#create', 'url' => '/api/2/tables', 'verb' => 'POST'], - ['name' => 'ApiTables#update', 'url' => '/api/2/tables/{id}', 'verb' => 'PUT'], + ['name' => 'ApiTables#update', 'url' => '/api/2/tables/{id}', 'verb' => 'PUT'], // needs archived ['name' => 'ApiTables#destroy', 'url' => '/api/2/tables/{id}', 'verb' => 'DELETE'], ['name' => 'ApiTables#transfer', 'url' => '/api/2/tables/{id}/transfer', 'verb' => 'PUT'], @@ -125,5 +125,8 @@ ['name' => 'ApiColumns#createTextColumn', 'url' => '/api/2/columns/text', 'verb' => 'POST'], ['name' => 'ApiColumns#createSelectionColumn', 'url' => '/api/2/columns/selection', 'verb' => 'POST'], ['name' => 'ApiColumns#createDatetimeColumn', 'url' => '/api/2/columns/datetime', 'verb' => 'POST'], + + ['name' => 'ApiFavorite#create', 'url' => '/api/2/favorites/{nodeType}/{nodeId}', 'verb' => 'POST'], + ['name' => 'ApiFavorite#destroy', 'url' => '/api/2/favorites/{nodeType}/{nodeId}', 'verb' => 'DELETE'], ] ]; diff --git a/lib/Controller/ApiFavoriteController.php b/lib/Controller/ApiFavoriteController.php new file mode 100644 index 000000000..24b19f082 --- /dev/null +++ b/lib/Controller/ApiFavoriteController.php @@ -0,0 +1,80 @@ +service = $service; + } + + /** + * [api v2] Create a new table and return it + * + * @NoAdminRequired + * + * @param string $title Title of the table + * @param string|null $emoji Emoji for the table + * @param string $template Template to use if wanted + * + * @return DataResponse|DataResponse + * + * 200: Tables returned + */ + public function create(int $nodeType, int $nodeId): DataResponse { + try { + $this->service->addFavorite($nodeType, $nodeId); + return new DataResponse(['ok']); + } catch (InternalError|Exception $e) { + return $this->handleError($e); + } + } + + + /** + * [api v2] Delete a table + * + * @NoAdminRequired + * + * @param int $id Table ID + * @return DataResponse|DataResponse + * + * 200: Deleted table returned + * 403: No permissions + * 404: Not found + */ + public function destroy(int $nodeType, int $nodeId): DataResponse { + try { + $this->service->removeFavorite($nodeType, $nodeId); + return new DataResponse(['ok']); + } catch (PermissionError $e) { + return $this->handlePermissionError($e); + } catch (InternalError $e) { + return $this->handleError($e); + } catch (NotFoundError $e) { + return $this->handleNotFoundError($e); + } + } +} diff --git a/lib/Controller/ApiTablesController.php b/lib/Controller/ApiTablesController.php index 60db21cc9..9efe47b9f 100644 --- a/lib/Controller/ApiTablesController.php +++ b/lib/Controller/ApiTablesController.php @@ -106,9 +106,9 @@ public function create(string $title, ?string $emoji, string $template = 'custom * 403: No permissions * 404: Not found */ - public function update(int $id, string $title = null, string $emoji = null): DataResponse { + public function update(int $id, ?string $title = null, ?string $emoji = null, ?bool $archived = null): DataResponse { try { - return new DataResponse($this->service->update($id, $title, $emoji, $this->userId)->jsonSerialize()); + return new DataResponse($this->service->update($id, $title, $emoji, $archived, $this->userId)->jsonSerialize()); } catch (PermissionError $e) { return $this->handlePermissionError($e); } catch (InternalError $e) { diff --git a/lib/Controller/TableController.php b/lib/Controller/TableController.php index 44d5d1b46..65d4e7fd0 100644 --- a/lib/Controller/TableController.php +++ b/lib/Controller/TableController.php @@ -70,9 +70,9 @@ public function destroy(int $id): DataResponse { /** * @NoAdminRequired */ - public function update(int $id, string $title = null, string $emoji = null): DataResponse { - return $this->handleError(function () use ($id, $title, $emoji) { - return $this->service->update($id, $title, $emoji, $this->userId); + public function update(int $id, string $title = null, string $emoji = null, ?bool $archived = null): DataResponse { + return $this->handleError(function () use ($id, $title, $emoji, $archived) { + return $this->service->update($id, $title, $emoji, $archived, $this->userId); }); } } diff --git a/lib/Db/Table.php b/lib/Db/Table.php index 1807a73d6..242496fe7 100644 --- a/lib/Db/Table.php +++ b/lib/Db/Table.php @@ -16,6 +16,8 @@ * @method setTitle(string $title) * @method getEmoji(): string * @method setEmoji(string $emoji) + * @method getArchived(): bool + * @method setArchived(bool $archived) * @method getOwnership(): string * @method setOwnership(string $ownership) * @method getOwnerDisplayName(): string @@ -26,6 +28,8 @@ * @method setOnSharePermissions(array $onSharePermissions) * @method getHasShares(): bool * @method setHasShares(bool $hasShares) + * @method getFavorite(): bool + * @method setFavorite(bool $favorite) * @method getRowsCount(): int * @method setRowsCount(int $rowsCount) * @method getColumnsCount(): int @@ -52,10 +56,12 @@ class Table extends Entity implements JsonSerializable { protected ?string $createdAt = null; protected ?string $lastEditBy = null; protected ?string $lastEditAt = null; + protected bool $archived = false; protected ?bool $isShared = null; protected ?array $onSharePermissions = null; protected ?bool $hasShares = false; + protected ?bool $favorite = false; protected ?int $rowsCount = 0; protected ?int $columnsCount = 0; protected ?array $views = null; @@ -63,6 +69,7 @@ class Table extends Entity implements JsonSerializable { public function __construct() { $this->addType('id', 'integer'); + $this->addType('archived', 'boolean'); } /** @@ -79,7 +86,9 @@ public function jsonSerialize(): array { 'createdAt' => $this->createdAt ?: '', 'lastEditBy' => $this->lastEditBy ?: '', 'lastEditAt' => $this->lastEditAt ?: '', + 'archived' => $this->archived, 'isShared' => !!$this->isShared, + 'favorite' => $this->favorite, 'onSharePermissions' => $this->getSharePermissions(), 'hasShares' => !!$this->hasShares, 'rowsCount' => $this->rowsCount ?: 0, diff --git a/lib/Migration/Version000800Date20240222000000.php b/lib/Migration/Version000800Date20240222000000.php new file mode 100644 index 000000000..1605c2780 --- /dev/null +++ b/lib/Migration/Version000800Date20240222000000.php @@ -0,0 +1,59 @@ +hasTable('tables_tables')) { + $table = $schema->getTable('tables_tables'); + $table->addColumn('archived', Types::BOOLEAN, [ + 'default' => false, + 'notnull' => true, + ]); + } + + if (!$schema->hasTable('tables_favorites')) { + $table = $schema->createTable('tables_favorites'); + $table->addColumn('id', Types::BIGINT, [ + 'notnull' => true, + 'autoincrement' => true, + 'unsigned' => true, + ]); + $table->addColumn('node_type', Types::SMALLINT, [ + 'notnull' => true, + ]); + $table->addColumn('node_id', Types::BIGINT, [ + 'notnull' => true, + ]); + $table->addColumn('user_id', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->setPrimaryKey(['id']); + } + + return $schema; + } + +} diff --git a/lib/Service/FavoritesService.php b/lib/Service/FavoritesService.php new file mode 100644 index 000000000..4c8b1fc5e --- /dev/null +++ b/lib/Service/FavoritesService.php @@ -0,0 +1,80 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Tables\Service; + +use OCP\Cache\CappedMemoryCache; +use OCP\IDBConnection; + +class FavoritesService { + + private CappedMemoryCache $cache; + + public function __construct(private IDBConnection $connection, private ?string $userId) { + $this->cache = new CappedMemoryCache(); + } + + public function isFavorite(int $nodeType, int $id): bool { + if ($cached = $this->cache->get($this->userId . '_' . $nodeType . '_' . $id)) { + return $cached; + } + + // We still might run this multiple times + + $qb = $this->connection->getQueryBuilder(); + $qb->select('*') + ->from('tables_favorites') + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($this->userId))); + + $result = $qb->executeQuery(); + while ($row = $result->fetch()) { + $this->cache->set($this->userId . '_' . $row['node_type'] . '_' . $row['node_id'], true); + } + + return $this->cache->get($this->userId . '_' . $nodeType . '_' . $id) ?? false; + } + + public function addFavorite(int $nodeType, int $id): void { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_favorites') + ->values([ + 'user_id' => $qb->createNamedParameter($this->userId), + 'node_type' => $qb->createNamedParameter($nodeType), + 'node_id' => $qb->createNamedParameter($id), + ]); + $qb->executeStatement(); + $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, true); + } + + public function removeFavorite(int $nodeType, int $id): void { + $qb = $this->connection->getQueryBuilder(); + $qb->delete('tables_favorites') + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($this->userId))) + ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType))) + ->andWhere($qb->expr()->eq('node_id', $qb->createNamedParameter($id))); + $qb->executeStatement(); + $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, false); + } + +} diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index 988462a7a..e28fe5fcb 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -6,6 +6,7 @@ use DateTime; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; use OCA\Tables\Errors\InternalError; @@ -38,6 +39,9 @@ class TableService extends SuperService { protected UserHelper $userHelper; + protected FavoritesService $favoritesService; + + protected IL10N $l; public function __construct( @@ -51,6 +55,7 @@ public function __construct( ViewService $viewService, ShareService $shareService, UserHelper $userHelper, + FavoritesService $favoritesService, IL10N $l ) { parent::__construct($logger, $userId, $permissionsService); @@ -61,6 +66,7 @@ public function __construct( $this->viewService = $viewService; $this->shareService = $shareService; $this->userHelper = $userHelper; + $this->favoritesService = $favoritesService; $this->l = $l; } @@ -197,6 +203,11 @@ private function enhanceTable(Table $table, string $userId): void { $table->setViews($this->viewService->findAll($table)); } + if ($this->favoritesService->isFavorite(Application::NODE_TYPE_TABLE, $table->getId())) { + $table->setFavorite(true); + } + + } @@ -420,7 +431,7 @@ public function delete(int $id, ?string $userId = null): Table { * @throws NotFoundError * @throws PermissionError */ - public function update(int $id, ?string $title, ?string $emoji, ?string $userId = null): Table { + public function update(int $id, ?string $title, ?string $emoji, ?bool $archived = null, ?string $userId = null): Table { $userId = $this->permissionsService->preCheckUserId($userId); try { @@ -445,6 +456,9 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $userId if ($emoji !== null) { $table->setEmoji($emoji); } + if ($archived !== null) { + $table->setArchived($archived); + } $table->setLastEditBy($userId); $table->setLastEditAt($time->format('Y-m-d H:i:s')); try { From c2bb58b579a1ad19bb74fc4d138ec0e274cf9964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 22 Feb 2024 17:44:51 +0100 Subject: [PATCH 02/23] fix: Adapt reset of the update methods to use the new archived flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- appinfo/routes.php | 6 +++--- lib/Command/RenameTable.php | 12 ++++++++++-- lib/Controller/Api1Controller.php | 4 ++-- lib/ResponseDefinitions.php | 2 ++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index e58ee8e4e..5f733dfbc 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -17,7 +17,7 @@ // -> tables ['name' => 'api1#index', 'url' => '/api/1/tables', 'verb' => 'GET'], ['name' => 'api1#createTable', 'url' => '/api/1/tables', 'verb' => 'POST'], - ['name' => 'api1#updateTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'PUT'], // needs archived + ['name' => 'api1#updateTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'PUT'], ['name' => 'api1#getTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'GET'], ['name' => 'api1#deleteTable', 'url' => '/api/1/tables/{tableId}', 'verb' => 'DELETE'], // -> views @@ -61,7 +61,7 @@ ['name' => 'table#index', 'url' => '/table', 'verb' => 'GET'], ['name' => 'table#show', 'url' => '/table/{id}', 'verb' => 'GET'], ['name' => 'table#create', 'url' => '/table', 'verb' => 'POST'], - ['name' => 'table#update', 'url' => '/table/{id}', 'verb' => 'PUT'], // needs archived + ['name' => 'table#update', 'url' => '/table/{id}', 'verb' => 'PUT'], ['name' => 'table#destroy', 'url' => '/table/{id}', 'verb' => 'DELETE'], // view @@ -115,7 +115,7 @@ ['name' => 'ApiTables#index', 'url' => '/api/2/tables', 'verb' => 'GET'], ['name' => 'ApiTables#show', 'url' => '/api/2/tables/{id}', 'verb' => 'GET'], ['name' => 'ApiTables#create', 'url' => '/api/2/tables', 'verb' => 'POST'], - ['name' => 'ApiTables#update', 'url' => '/api/2/tables/{id}', 'verb' => 'PUT'], // needs archived + ['name' => 'ApiTables#update', 'url' => '/api/2/tables/{id}', 'verb' => 'PUT'], ['name' => 'ApiTables#destroy', 'url' => '/api/2/tables/{id}', 'verb' => 'DELETE'], ['name' => 'ApiTables#transfer', 'url' => '/api/2/tables/{id}/transfer', 'verb' => 'PUT'], diff --git a/lib/Command/RenameTable.php b/lib/Command/RenameTable.php index 6e8c4f41b..f2a3f57bf 100644 --- a/lib/Command/RenameTable.php +++ b/lib/Command/RenameTable.php @@ -44,7 +44,8 @@ public function __construct(TableService $tableService, LoggerInterface $logger) protected function configure(): void { $this - ->setName('tables:rename') + ->setName('tables:update') + ->setAliases('tables:rename') ->setDescription('Rename a table.') ->addArgument( 'ID', @@ -62,6 +63,12 @@ protected function configure(): void { InputOption::VALUE_OPTIONAL, 'New emoji.' ) + ->addOption( + 'archived', + 'a', + InputOption::VALUE_NONE, + 'Archived' + ) ; } @@ -74,9 +81,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $id = $input->getArgument('ID'); $title = $input->getArgument('title'); $emoji = $input->getOption('emoji'); + $archived = $input->getOption('archived'); try { - $table = $this->tableService->update($id, $title, $emoji, ''); + $table = $this->tableService->update($id, $title, $emoji, $archived, ''); $arr = $table->jsonSerialize(); unset($arr['hasShares']); diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index a9d9a8566..65b6654f8 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -178,9 +178,9 @@ public function getTable(int $tableId): DataResponse { * 403: No permissions * 404: Not found */ - public function updateTable(int $tableId, string $title = null, string $emoji = null): DataResponse { + public function updateTable(int $tableId, string $title = null, string $emoji = null, ?bool $archived = false): DataResponse { try { - return new DataResponse($this->tableService->update($tableId, $title, $emoji, $this->userId)->jsonSerialize()); + return new DataResponse($this->tableService->update($tableId, $title, $emoji, $archived, $this->userId)->jsonSerialize()); } catch (PermissionError $e) { $this->logger->warning('A permission error occurred: ' . $e->getMessage()); $message = ['message' => $e->getMessage()]; diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 42668bce5..beec25b77 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -44,6 +44,8 @@ * createdAt: string, * lastEditBy: string, * lastEditAt: string, + * archived: bool, + * favorite: bool, * isShared: bool, * onSharePermissions: ?array{ * read: bool, From f4d478bda01f78d519b2587e205967a4f82aa041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 22 Feb 2024 17:45:26 +0100 Subject: [PATCH 03/23] fix: Proper error handling and validation of passed in node type and node id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl tmp: revert to work around segfault Signed-off-by: Julius Härtl --- lib/Command/RenameTable.php | 2 +- lib/Controller/ApiFavoriteController.php | 34 ++++++------ lib/Service/FavoritesService.php | 68 ++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 21 deletions(-) diff --git a/lib/Command/RenameTable.php b/lib/Command/RenameTable.php index f2a3f57bf..3a41153b5 100644 --- a/lib/Command/RenameTable.php +++ b/lib/Command/RenameTable.php @@ -45,7 +45,7 @@ public function __construct(TableService $tableService, LoggerInterface $logger) protected function configure(): void { $this ->setName('tables:update') - ->setAliases('tables:rename') + ->setAliases(['tables:rename']) ->setDescription('Rename a table.') ->addArgument( 'ID', diff --git a/lib/Controller/ApiFavoriteController.php b/lib/Controller/ApiFavoriteController.php index 24b19f082..6c08cec6a 100644 --- a/lib/Controller/ApiFavoriteController.php +++ b/lib/Controller/ApiFavoriteController.php @@ -10,6 +10,7 @@ use OCA\Tables\Service\FavoritesService; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\DB\Exception as DBException; use OCP\IL10N; use OCP\IRequest; use Psr\Log\LoggerInterface; @@ -31,35 +32,38 @@ public function __construct( } /** - * [api v2] Create a new table and return it + * [api v2] Add a node (table or view) to user favorites * * @NoAdminRequired * - * @param string $title Title of the table - * @param string|null $emoji Emoji for the table - * @param string $template Template to use if wanted - * - * @return DataResponse|DataResponse + * @param int $nodeType + * @param int $nodeId + * @return DataResponse|DataResponse * * 200: Tables returned */ public function create(int $nodeType, int $nodeId): DataResponse { try { $this->service->addFavorite($nodeType, $nodeId); - return new DataResponse(['ok']); - } catch (InternalError|Exception $e) { + return new DataResponse(); + } catch (NotFoundError $e) { + return $this->handleNotFoundError($e); + } catch (PermissionError $e) { + return $this->handlePermissionError($e); + } catch (InternalError|DBException|Exception $e) { return $this->handleError($e); } } /** - * [api v2] Delete a table + * [api v2] Remove a node (table or view) to from favorites * * @NoAdminRequired * - * @param int $id Table ID - * @return DataResponse|DataResponse + * @param int $nodeType + * @param int $nodeId + * @return DataResponse|DataResponse * * 200: Deleted table returned * 403: No permissions @@ -68,13 +72,13 @@ public function create(int $nodeType, int $nodeId): DataResponse { public function destroy(int $nodeType, int $nodeId): DataResponse { try { $this->service->removeFavorite($nodeType, $nodeId); - return new DataResponse(['ok']); + return new DataResponse(); + } catch (NotFoundError $e) { + return $this->handleNotFoundError($e); } catch (PermissionError $e) { return $this->handlePermissionError($e); - } catch (InternalError $e) { + } catch (InternalError|DBException|Exception $e) { return $this->handleError($e); - } catch (NotFoundError $e) { - return $this->handleNotFoundError($e); } } } diff --git a/lib/Service/FavoritesService.php b/lib/Service/FavoritesService.php index 4c8b1fc5e..8a9a7b6da 100644 --- a/lib/Service/FavoritesService.php +++ b/lib/Service/FavoritesService.php @@ -24,24 +24,39 @@ namespace OCA\Tables\Service; +use OCA\Tables\AppInfo\Application; +use OCA\Tables\Errors\InternalError; +use OCA\Tables\Errors\NotFoundError; +use OCA\Tables\Errors\PermissionError; use OCP\Cache\CappedMemoryCache; +use OCP\DB\Exception; use OCP\IDBConnection; class FavoritesService { + private IDBConnection $connection; + private PermissionsService $permissionsService; + private ?string $userId; private CappedMemoryCache $cache; - public function __construct(private IDBConnection $connection, private ?string $userId) { + public function __construct( + IDBConnection $connection, + PermissionsService $permissionsService, + ?string $userId + ) { + $this->connection = $connection; + $this->permissionsService = $permissionsService; + $this->userId = $userId; $this->cache = new CappedMemoryCache(); } public function isFavorite(int $nodeType, int $id): bool { - if ($cached = $this->cache->get($this->userId . '_' . $nodeType . '_' . $id)) { + $cacheKey = $this->userId . '_' . $nodeType . '_' . $id; + if ($cached = $this->cache->get($cacheKey)) { return $cached; } - // We still might run this multiple times - + $this->cache->clear(); $qb = $this->connection->getQueryBuilder(); $qb->select('*') ->from('tables_favorites') @@ -52,10 +67,24 @@ public function isFavorite(int $nodeType, int $id): bool { $this->cache->set($this->userId . '_' . $row['node_type'] . '_' . $row['node_id'], true); } - return $this->cache->get($this->userId . '_' . $nodeType . '_' . $id) ?? false; + // Set the cache for not found matches still to avoid further queries + if (!$this->cache->get($cacheKey)) { + $this->cache->set($cacheKey, false); + } + + return $this->cache->get($cacheKey); } + /** + * @throws Exception + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError + */ public function addFavorite(int $nodeType, int $id): void { + $this->checkValidNodeType($nodeType); + $this->checkAccessToNode($nodeType, $id); + $qb = $this->connection->getQueryBuilder(); $qb->insert('tables_favorites') ->values([ @@ -67,7 +96,16 @@ public function addFavorite(int $nodeType, int $id): void { $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, true); } + /** + * @throws Exception + * @throws InternalError + * @throws NotFoundError + * @throws PermissionError + */ public function removeFavorite(int $nodeType, int $id): void { + $this->checkValidNodeType($nodeType); + $this->checkAccessToNode($nodeType, $id); + $qb = $this->connection->getQueryBuilder(); $qb->delete('tables_favorites') ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($this->userId))) @@ -77,4 +115,24 @@ public function removeFavorite(int $nodeType, int $id): void { $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, false); } + /** + * @throws InternalError + */ + private function checkValidNodeType(int $nodeType): void { + if (!in_array($nodeType, [Application::NODE_TYPE_TABLE, Application::NODE_TYPE_VIEW])) { + throw new InternalError('Invalid node type'); + } + } + + /** + * @throws PermissionError + */ + private function checkAccessToNode(int $nodeType, int $nodeId): void { + if ($this->permissionsService->canAccessNodeById($nodeType, $nodeId)) { + return; + } + + throw new PermissionError('Invalid node type and id'); + } + } From 40af3b72c9f7f90f24d61748cc3e51d0a18b3b31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 22 Feb 2024 17:45:48 +0100 Subject: [PATCH 04/23] chore: Make psalm output more readable for the main usage when we are only interested in errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b8d66398a..7d7694377 100644 --- a/composer.json +++ b/composer.json @@ -35,7 +35,7 @@ "lint": "find . -name \\*.php -not -path './vendor/*' -not -path './build/*' -print0 | xargs -0 -n1 php -l", "cs:check": "php-cs-fixer fix --dry-run --diff", "cs:fix": "php-cs-fixer fix", - "psalm": "./vendor/bin/psalm.phar --show-info=true --no-cache", + "psalm": "./vendor/bin/psalm.phar --show-info=false --no-cache", "psalm:update-baseline": "./vendor/bin/psalm.phar --update-baseline", "psalm:fix": "./vendor/bin/psalm.phar --no-cache --alter --issues=InvalidReturnType,InvalidNullableReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,InvalidFalsableReturnType", "psalm:fix:dry": "./vendor/bin/psalm.phar --no-cache --alter --issues=InvalidReturnType,InvalidNullableReturnType,MismatchingDocblockParamType,MismatchingDocblockReturnType,MissingParamType,InvalidFalsableReturnType --dry-run", From 988571eccba38a9ee95118c407a7b1d3e51ad6d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 22 Feb 2024 17:47:20 +0100 Subject: [PATCH 05/23] feat: Expose api feature flags thorugh capabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Capabilities.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Capabilities.php b/lib/Capabilities.php index 2d7573ad2..f80adc6b5 100644 --- a/lib/Capabilities.php +++ b/lib/Capabilities.php @@ -46,7 +46,7 @@ public function __construct(IAppManager $appManager, LoggerInterface $logger, IC /** * - * @return array{tables: array{enabled: bool, version: string, apiVersions: string[], column_types: string[]}} + * @return array{tables: array{enabled: bool, version: string, apiVersions: string[], features: string[], column_types: string[]}} * * @inheritDoc */ @@ -63,6 +63,10 @@ public function getCapabilities(): array { 'apiVersions' => [ '1.0' ], + 'features' => [ + 'favorite', + 'archive', + ], 'column_types' => [ 'text-line', $textColumnVariant, From 19d15740da47c069e802e7a3f2561a3c461a91ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 23 Feb 2024 09:27:14 +0100 Subject: [PATCH 06/23] tests: Add integration tests for archive and favorites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- tests/integration/features/APIv2.feature | 27 ++++ .../features/bootstrap/FeatureContext.php | 117 ++++++++++++++++-- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index 7afbabba7..1848ce77e 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -2,6 +2,7 @@ Feature: APIv2 Background: Given user "participant1-v2" exists Given user "participant2-v2" exists + Given user "participant3-v2" exists @api2 Scenario: Test initial setup @@ -15,11 +16,37 @@ Feature: APIv2 Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 Then user "participant1-v2" has the following tables via v2 | Table 1 via api v2 | + And user "participant1-v2" sees the following table attributes on table "t1" + | archived | 0 | Then user "participant1-v2" updates table "t1" set title "updated title" and emoji "⛵︎" via v2 Then user "participant1-v2" has the following tables via v2 | updated title | + Then user "participant1-v2" updates table "t1" set archived 1 via v2 + And user "participant1-v2" sees the following table attributes on table "t1" + | archived | 1 | + Then user "participant1-v2" updates table "t1" set archived 0 via v2 + And user "participant1-v2" sees the following table attributes on table "t1" + | archived | 0 | Then user "participant1-v2" deletes table "t1" via v2 + @api2 + Scenario: Favorite tables + Given table "Own table" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And user "participant1-v2" shares table with user "participant2-v2" + And user "participant1-v2" adds the table "t1" to favorites + And user "participant1-v2" sees the following table attributes on table "t1" + | favorite | 1 | + And user "participant2-v2" fetches table info for table "t1" + And user "participant2-v2" sees the following table attributes on table "t1" + | favorite | 0 | + When user "participant1-v2" removes the table "t1" from favorites + And user "participant1-v2" sees the following table attributes on table "t1" + | favorite | 0 | + When user "participant3-v2" adds the table "t1" to favorites + Then the last response should have a "403" status code + + + @api2 Scenario: Basic column actions Given table "Table 2" with emoji "👋" exists for user "participant1-v2" as "t2" via v2 diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 959cbc870..7a4757e24 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -70,6 +70,10 @@ class FeatureContext implements Context { private array $viewIds = []; private array $columnIds = []; + // Store data from last request to perform assertions, id is used as a key + private array $tableData = []; + private array $viewData = []; + // use CommandLineTrait; /** @@ -127,16 +131,30 @@ public function createTableV2(string $user, string $title, string $tableName, st Assert::assertEquals($newTable['emoji'], $emoji); Assert::assertEquals($newTable['ownership'], $user); + $tableToVerify = $this->userFetchesTableInfo($user, $tableName); + Assert::assertEquals(200, $this->response->getStatusCode()); + Assert::assertEquals($tableToVerify['title'], $title); + Assert::assertEquals($tableToVerify['emoji'], $emoji); + Assert::assertEquals($tableToVerify['ownership'], $user); + } + + /** + * @Given user :user fetches table info for table :tableName + */ + public function userFetchesTableInfo($user, $tableName) { + $this->setCurrentUser($user); + $tableId = $this->tableIds[$tableName]; + $this->sendOcsRequest( 'GET', - '/apps/tables/api/2/tables/'.$newTable['id'], + '/apps/tables/api/2/tables/'.$tableId, ); $tableToVerify = $this->getDataFromResponse($this->response)['ocs']['data']; - Assert::assertEquals(200, $this->response->getStatusCode()); - Assert::assertEquals($tableToVerify['title'], $title); - Assert::assertEquals($tableToVerify['emoji'], $emoji); - Assert::assertEquals($tableToVerify['ownership'], $user); + $this->tableData[$tableName] = $tableToVerify; + $this->tableId = $tableToVerify['id']; + + return $tableToVerify; } /** @@ -223,6 +241,7 @@ public function initialResourcesV2(string $user, TableNode $body = null): void { /** * @Then user :user updates table :tableName set title :title and emoji :emoji via v2 + * @Then user :user updates table :tableName set archived :archived via v2 * * @param string $user * @param string $title @@ -230,13 +249,26 @@ public function initialResourcesV2(string $user, TableNode $body = null): void { * @param string $tableName * @throws Exception */ - public function updateTableV2(string $user, string $title, ?string $emoji, string $tableName): void { + public function updateTableV2(string $user, string $tableName, string $title = null, ?string $emoji = null, ?bool $archived = null): void { $this->setCurrentUser($user); - $data = ['title' => $title]; + $this->sendOcsRequest( + 'GET', + '/apps/tables/api/2/tables/'.$this->tableIds[$tableName], + ); + + $previousData = $this->getDataFromResponse($this->response)['ocs']['data']; + + $data = []; + if ($title !== null) { + $data['title'] = $title; + } if ($emoji !== null) { $data['emoji'] = $emoji; } + if ($archived !== null) { + $data['archived'] = $archived; + } $this->sendOcsRequest( 'PUT', @@ -247,9 +279,10 @@ public function updateTableV2(string $user, string $title, ?string $emoji, strin $updatedTable = $this->getDataFromResponse($this->response)['ocs']['data']; Assert::assertEquals(200, $this->response->getStatusCode()); - Assert::assertEquals($updatedTable['title'], $title); - Assert::assertEquals($updatedTable['emoji'], $emoji); - Assert::assertEquals($updatedTable['ownership'], $user); + Assert::assertEquals($updatedTable['title'], $title ?? $previousData['title']); + Assert::assertEquals($updatedTable['emoji'], $emoji ?? $previousData['emoji']); + Assert::assertEquals($updatedTable['ownership'], $user ?? $previousData['ownership']); + Assert::assertEquals($updatedTable['archived'], $archived ?? $previousData['archived']); $this->sendOcsRequest( 'GET', @@ -258,9 +291,12 @@ public function updateTableV2(string $user, string $title, ?string $emoji, strin $tableToVerify = $this->getDataFromResponse($this->response)['ocs']['data']; Assert::assertEquals(200, $this->response->getStatusCode()); - Assert::assertEquals($tableToVerify['title'], $title); - Assert::assertEquals($tableToVerify['emoji'], $emoji); - Assert::assertEquals($tableToVerify['ownership'], $user); + Assert::assertEquals($tableToVerify['title'], $title ?? $previousData['title']); + Assert::assertEquals($tableToVerify['emoji'], $emoji ?? $previousData['emoji']); + Assert::assertEquals($tableToVerify['ownership'], $user ?? $previousData['ownership']); + Assert::assertEquals($tableToVerify['archived'], $archived ?? $previousData['archived']); + + $this->tableData[$tableName] = $tableToVerify; } /** @@ -1634,4 +1670,59 @@ protected function assertStatusCode(ResponseInterface $response, int $statusCode Assert::assertEquals($statusCode, $response->getStatusCode(), $message); } } + + /** + * @Given user :user sees the following table attributes on table :tableName + */ + public function userSeesTheFollowingTableAttributesOnTable($user, $tableName, TableNode $table) { + foreach ($table->getRows() as $row) { + $attribute = $row[0]; + $value = $row[1]; + if (in_array($attribute, ['archived', 'favorite'])) { + $value = (bool)$value; + } + Assert::assertEquals($value, $this->tableData[$tableName][$attribute]); + } + } + + /** + * @Given user :user adds the table :tableName to favorites + */ + public function userAddsTheTableToFavorites($user, $tableName) { + $this->setCurrentUser($user); + $nodeType = 0; + $tableId = $this->tableIds[$tableName]; + + $this->sendOcsRequest( + 'POST', + '/apps/tables/api/2/favorites/' . $nodeType. '/' . $tableId, + ); + if ($this->response->getStatusCode() === 200) { + $this->userFetchesTableInfo($user, $tableName); + } + } + + /** + * @Given user :user removes the table :tableName from favorites + */ + public function userRemovesTheTableFromFavorites($user, $tableName) { + $this->setCurrentUser($user); + $nodeType = 0; + $tableId = $this->tableIds[$tableName]; + + $this->sendOcsRequest( + 'DELETE', + '/apps/tables/api/2/favorites/' . $nodeType. '/' . $tableId, + ); + if ($this->response->getStatusCode() === 200) { + $this->userFetchesTableInfo($user, $tableName); + } + } + + /** + * @Then /^the last response should have a "([^"]*)" status code$/ + */ + public function theLastResponseShouldHaveAStatusCode(int $statusCode) { + Assert::assertEquals($statusCode, $this->response->getStatusCode()); + } } From 513b6607055408fc31ca24c71935462167f53341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Feb 2024 12:31:27 +0100 Subject: [PATCH 07/23] fix: Expose favorites of views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/View.php | 4 ++++ lib/ResponseDefinitions.php | 1 + lib/Service/ViewService.php | 9 +++++++++ 3 files changed, 14 insertions(+) diff --git a/lib/Db/View.php b/lib/Db/View.php index 3e72d6240..73684b55b 100644 --- a/lib/Db/View.php +++ b/lib/Db/View.php @@ -35,6 +35,8 @@ * @method setOnSharePermissions(array $onSharePermissions) * @method getHasShares(): bool * @method setHasShares(bool $hasShares) + * @method getFavorite(): bool + * @method setFavorite(bool $favorite) * @method getRowsCount(): int * @method setRowsCount(int $rowCount) * @method getOwnership(): string @@ -57,6 +59,7 @@ class View extends Entity implements JsonSerializable { protected ?bool $isShared = null; protected ?array $onSharePermissions = null; protected ?bool $hasShares = false; + protected bool $favorite = false; protected ?int $rowsCount = 0; protected ?string $ownership = null; protected ?string $ownerDisplayName = null; @@ -137,6 +140,7 @@ public function jsonSerialize(): array { 'columns' => $this->getColumnsArray(), 'sort' => $this->getSortArray(), 'isShared' => !!$this->isShared, + 'favorite' => $this->favorite, 'onSharePermissions' => $this->getSharePermissions(), 'hasShares' => !!$this->hasShares, 'rowsCount' => $this->rowsCount ?: 0, diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index beec25b77..172b760a0 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -23,6 +23,7 @@ * sort: list, * filter: list>, * isShared: bool, + * favorite: bool, * onSharePermissions: ?array{ * read: bool, * create: bool, diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 5dd6297fd..4593086a0 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -7,6 +7,7 @@ use DateTime; use Exception; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\Table; use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; @@ -34,6 +35,8 @@ class ViewService extends SuperService { protected UserHelper $userHelper; + protected FavoritesService $favoritesService; + protected IL10N $l; public function __construct( @@ -44,6 +47,7 @@ public function __construct( ShareService $shareService, RowService $rowService, UserHelper $userHelper, + FavoritesService $favoritesService, IL10N $l ) { parent::__construct($logger, $userId, $permissionsService); @@ -52,6 +56,7 @@ public function __construct( $this->shareService = $shareService; $this->rowService = $rowService; $this->userHelper = $userHelper; + $this->favoritesService = $favoritesService; } @@ -395,6 +400,10 @@ private function enhanceView(View $view, string $userId): void { } } } + + if ($this->favoritesService->isFavorite(Application::NODE_TYPE_VIEW, $view->getId())) { + $view->setFavorite(true); + } } /** From 3e6172831fb565f0d0dc376b4fe3cf54555ce7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Feb 2024 12:32:20 +0100 Subject: [PATCH 08/23] fix: Do not use userId as cache key prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Service/FavoritesService.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/Service/FavoritesService.php b/lib/Service/FavoritesService.php index 8a9a7b6da..d4737f4b4 100644 --- a/lib/Service/FavoritesService.php +++ b/lib/Service/FavoritesService.php @@ -47,11 +47,12 @@ public function __construct( $this->connection = $connection; $this->permissionsService = $permissionsService; $this->userId = $userId; + // The cache usage is currently not unique to the user id as only a memory cache is used $this->cache = new CappedMemoryCache(); } public function isFavorite(int $nodeType, int $id): bool { - $cacheKey = $this->userId . '_' . $nodeType . '_' . $id; + $cacheKey = $nodeType . '_' . $id; if ($cached = $this->cache->get($cacheKey)) { return $cached; } @@ -64,7 +65,7 @@ public function isFavorite(int $nodeType, int $id): bool { $result = $qb->executeQuery(); while ($row = $result->fetch()) { - $this->cache->set($this->userId . '_' . $row['node_type'] . '_' . $row['node_id'], true); + $this->cache->set($row['node_type'] . '_' . $row['node_id'], true); } // Set the cache for not found matches still to avoid further queries @@ -93,7 +94,7 @@ public function addFavorite(int $nodeType, int $id): void { 'node_id' => $qb->createNamedParameter($id), ]); $qb->executeStatement(); - $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, true); + $this->cache->set($nodeType . '_' . $id, true); } /** @@ -112,7 +113,7 @@ public function removeFavorite(int $nodeType, int $id): void { ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType))) ->andWhere($qb->expr()->eq('node_id', $qb->createNamedParameter($id))); $qb->executeStatement(); - $this->cache->set($this->userId . '_' . $nodeType . '_' . $id, false); + $this->cache->set($nodeType . '_' . $id, false); } /** From c816afed1c58834a24d0df535db4c4684a4c4229 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 26 Feb 2024 12:35:11 +0100 Subject: [PATCH 09/23] fix: Add index to query favorites by user id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Migration/Version000800Date20240222000000.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Migration/Version000800Date20240222000000.php b/lib/Migration/Version000800Date20240222000000.php index 1605c2780..9a8d8a4d0 100644 --- a/lib/Migration/Version000800Date20240222000000.php +++ b/lib/Migration/Version000800Date20240222000000.php @@ -51,6 +51,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'length' => 64, ]); $table->setPrimaryKey(['id']); + $table->addIndex(['user_id'], 'idx_tables_fav_uid'); } return $schema; From cc66f1264cc790d5efc0b82ef5010aaeb7519ec9 Mon Sep 17 00:00:00 2001 From: Elizabeth Danzberger Date: Mon, 26 Feb 2024 14:51:25 -0500 Subject: [PATCH 10/23] added archive table action button Signed-off-by: Elizabeth Danzberger --- .../navigation/partials/NavigationTableItem.vue | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/modules/navigation/partials/NavigationTableItem.vue b/src/modules/navigation/partials/NavigationTableItem.vue index bca3c05dd..10b2447be 100644 --- a/src/modules/navigation/partials/NavigationTableItem.vue +++ b/src/modules/navigation/partials/NavigationTableItem.vue @@ -65,6 +65,14 @@ + + {{ t('tables', 'Archive table') }} + + +