diff --git a/lib/Analytics/AnalyticsDatasource.php b/lib/Analytics/AnalyticsDatasource.php index 8b2b9cf3eb..fa5f8bbb08 100644 --- a/lib/Analytics/AnalyticsDatasource.php +++ b/lib/Analytics/AnalyticsDatasource.php @@ -87,16 +87,9 @@ public function getTemplate(): array { // get all tables and subsequent views foreach ($tables as $table) { $tableString = $tableString . $table->getId() . '-' . $table->getTitle() . '/'; - // get all views per table - try { - $views = $this->viewService->findAll($this->tableService->find($table->getId()), $this->userId); - foreach ($views as $view) { - // concatenate the option-string. The format is tableId:viewId-title - $tableString = $tableString . $table->getId() . ':' . $view->getId() . '-' . $view->getTitle() . '/'; - } - } catch (PermissionError) { - // this is a shared table without shared views; - continue; + foreach ($table->getViews() ?? [] as $view) { + // concatenate the option-string. The format is tableId:viewId-title + $tableString = $tableString . $table->getId() . ':' . $view->getId() . '-' . $view->getTitle() . '/'; } } diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index 1e5984a4d9..e21384ffe2 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -296,11 +296,12 @@ public function deleteTable(int $tableId): DataResponse { #[NoAdminRequired] #[NoCSRFRequired] #[CORS] - #[RequirePermission(permission: Application::PERMISSION_READ, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] + #[RequirePermission(permission: Application::PERMISSION_MANAGE, type: Application::NODE_TYPE_TABLE, idParam: 'tableId')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function indexViews(int $tableId): DataResponse { try { - return new DataResponse($this->viewService->formatViews($this->viewService->findAll($this->tableService->find($tableId)))); + $table = $this->tableService->find($tableId); + return new DataResponse($this->viewService->formatViews($table->getViews() ?? [])); } catch (PermissionError $e) { $this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]); $message = ['message' => $e->getMessage()]; diff --git a/lib/Db/ColumnMapper.php b/lib/Db/ColumnMapper.php index ac9de1512c..ab43616577 100644 --- a/lib/Db/ColumnMapper.php +++ b/lib/Db/ColumnMapper.php @@ -18,6 +18,8 @@ /** @template-extends QBMapper */ class ColumnMapper extends QBMapper { + private const DB_CHUNK_SIZE = 1_000; + protected string $table = 'tables_columns'; private readonly CappedMemoryCache $cacheColumn; @@ -166,24 +168,31 @@ public function getColumnTypes(array $neededColumnIds): array { } /** - * @param int $tableId - * @return int + * @param int[] $tableIds + * @return array */ - public function countColumns(int $tableId): int { + public function countColumnsForTables(array $tableIds): array { + if (empty($tableIds)) { + return []; + } + $qb = $this->db->getQueryBuilder(); - $qb->select($qb->func()->count('*', 'counter')); - $qb->from($this->table); - $qb->where( - $qb->expr()->eq('table_id', $qb->createNamedParameter($tableId)) - ); + $qb->select('table_id', $qb->func()->count('*', 'counter')) + ->from($this->table) + ->where($qb->expr()->in('table_id', $qb->createParameter('tableIds'))) + ->groupBy('table_id'); - try { - $result = $this->findOneQuery($qb); - return (int)$result['counter']; - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - $this->logger->warning('Exception occurred: ' . $e->getMessage() . ' Returning 0.'); - return 0; + $counts = array_fill_keys($tableIds, 0); + foreach (array_chunk($tableIds, self::DB_CHUNK_SIZE - 1) as $tableIdsChunk) { + $qb->setParameter('tableIds', $tableIdsChunk, IQueryBuilder::PARAM_INT_ARRAY); + $result = $qb->executeQuery(); + while ($row = $result->fetchAssociative()) { + $counts[(int)$row['table_id']] = (int)$row['counter']; + } + $result->closeCursor(); } + + return $counts; } /** diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index aeb7c59c8c..814c4e348a 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -1015,8 +1015,13 @@ public function deleteAllForTable(int $tableId, array $columns): void { } } + /** + * @param int $tableId + * @return int + */ public function countRowsForTable(int $tableId): int { - return $this->rowSleeveMapper->countRows($tableId); + $counts = $this->rowSleeveMapper->countRowsByTableIds([$tableId]); + return $counts[$tableId] ?? 0; } /** diff --git a/lib/Db/RowSleeveMapper.php b/lib/Db/RowSleeveMapper.php index b627424ba5..2ad587345f 100644 --- a/lib/Db/RowSleeveMapper.php +++ b/lib/Db/RowSleeveMapper.php @@ -105,27 +105,6 @@ public function deleteAllForTable(int $tableId): int { return $qb->executeStatement(); } - /** - * @param int $tableId - * @return int - */ - public function countRows(int $tableId): int { - $qb = $this->db->getQueryBuilder(); - $qb->select($qb->func()->count('*', 'counter')); - $qb->from($this->table, 't1'); - $qb->where( - $qb->expr()->eq('table_id', $qb->createNamedParameter($tableId)) - ); - - try { - $result = $this->findOneQuery($qb); - return (int)$result['counter']; - } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { - $this->logger->warning('Exception occurred: ' . $e->getMessage() . ' Will return 0.'); - return 0; - } - } - /** * @param int[] $tableIds * @return array diff --git a/lib/Db/ShareMapper.php b/lib/Db/ShareMapper.php index c8a42db6bc..a536daad0e 100644 --- a/lib/Db/ShareMapper.php +++ b/lib/Db/ShareMapper.php @@ -19,6 +19,8 @@ /** @template-extends QBMapper */ class ShareMapper extends QBMapper { + private const DB_CHUNK_SIZE = 1_000; + protected string $table = 'tables_shares'; public function __construct( @@ -99,17 +101,18 @@ public function findAllSharesFor(string $nodeType, array $receivers, string $use return []; } + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->table) + ->where($qb->expr()->in('receiver', $qb->createParameter('receivers'))) + ->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId))) + ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType))) + ->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType))); + $chunks = []; // deduct extra parameters (sender, node type, receiver type) - foreach (array_chunk($receivers, 1000 - 3) as $receiversChunk) { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->table) - ->where($qb->expr()->in('receiver', $qb->createNamedParameter($receiversChunk, IQueryBuilder::PARAM_STR_ARRAY))) - ->andWhere($qb->expr()->neq('sender', $qb->createNamedParameter($userId))) - ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType))) - ->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter($receiverType))); - + foreach (array_chunk($receivers, self::DB_CHUNK_SIZE - 3) as $receiversChunk) { + $qb->setParameter('receivers', $receiversChunk, IQueryBuilder::PARAM_STR_ARRAY); $chunks[] = $this->findEntities($qb); } @@ -119,12 +122,11 @@ public function findAllSharesFor(string $nodeType, array $receivers, string $use /** * @param string $nodeType * @param int $nodeId - * @param string $sender * @param array $excluded receiver types to exclude from results * @return Share[] * @throws Exception */ - public function findAllSharesForNode(string $nodeType, int $nodeId, string $sender = '', array $excluded = []): array { + public function findAllSharesForNode(string $nodeType, int $nodeId, array $excluded = []): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->table) @@ -138,6 +140,71 @@ public function findAllSharesForNode(string $nodeType, int $nodeId, string $send return $this->findEntities($qb); } + /** + * @param string $nodeType + * @param int[] $nodeIds + * @param array $excluded receiver types to exclude from results + * @return Share[] + * @throws Exception + */ + public function findAllSharesForNodes(string $nodeType, array $nodeIds, array $excluded = []): array { + if (empty($nodeIds)) { + return []; + } + + $extraParams = 2; + if (!empty($excluded)) { + $extraParams++; + } + + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->table) + ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter($nodeType, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('node_id', $qb->createParameter('nodeIds'))); + + if (!empty($excluded)) { + $qb->andWhere($qb->expr()->notIn('receiver_type', $qb->createNamedParameter($excluded, IQueryBuilder::PARAM_STR_ARRAY))); + } + + $chunks = []; + foreach (array_chunk($nodeIds, self::DB_CHUNK_SIZE - $extraParams) as $nodeIdsChunk) { + $qb->setParameter('nodeIds', $nodeIdsChunk, IQueryBuilder::PARAM_INT_ARRAY); + $chunks[] = $this->findEntities($qb); + } + + return array_merge(...$chunks); + } + + /** + * @param int[] $tableIds + * @return int[] + */ + public function findTableIdsWithLinkShares(array $tableIds): array { + if (empty($tableIds)) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('node_id') + ->from($this->table) + ->andWhere($qb->expr()->eq('node_type', $qb->createNamedParameter('table', IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('receiver_type', $qb->createNamedParameter(ShareReceiverType::LINK, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->in('node_id', $qb->createParameter('tableIds'))); + + $linkTableIds = []; + foreach (array_chunk($tableIds, self::DB_CHUNK_SIZE - 2) as $tableIdsChunk) { + $qb->setParameter('tableIds', $tableIdsChunk, IQueryBuilder::PARAM_INT_ARRAY); + $result = $qb->executeQuery(); + while ($row = $result->fetchAssociative()) { + $linkTableIds[] = (int)$row['node_id']; + } + $result->closeCursor(); + } + + return $linkTableIds; + } + /** * @param string $nodeType * @param int $nodeId diff --git a/lib/Db/ViewMapper.php b/lib/Db/ViewMapper.php index f5bbca13e8..c554d3e071 100644 --- a/lib/Db/ViewMapper.php +++ b/lib/Db/ViewMapper.php @@ -73,13 +73,14 @@ public function findMany(array $ids): array { } $missing = array_keys($missing); - foreach (array_chunk($missing, 1000) as $missingChunk) { - $qb = $this->db->getQueryBuilder(); - $qb->select('v.*', 't.ownership') - ->from($this->table, 'v') - ->leftJoin('v', 'tables_tables', 't', 't.id = v.table_id') - ->where($qb->expr()->in('v.id', $qb->createNamedParameter($missingChunk, IQueryBuilder::PARAM_INT_ARRAY))); + $qb = $this->db->getQueryBuilder(); + $qb->select('v.*', 't.ownership') + ->from($this->table, 'v') + ->leftJoin('v', 'tables_tables', 't', 't.id = v.table_id') + ->where($qb->expr()->in('v.id', $qb->createParameter('viewIds'))); + foreach (array_chunk($missing, self::DB_CHUNK_SIZE) as $missingChunk) { + $qb->setParameter('viewIds', $missingChunk, IQueryBuilder::PARAM_INT_ARRAY); foreach ($this->findEntities($qb) as $entity) { $id = $entity->getId(); $this->cache[(string)$id] = $entity; @@ -127,6 +128,8 @@ public function insert(Entity $entity): View { } /** + * @deprecated Use {@link findAllByTableIds} instead + * * @return View[] * @throws Exception */ @@ -146,6 +149,33 @@ public function findAll(?int $tableId = null): array { return $this->findEntities($qb); } + /** + * @param int[] $tableIds + * @return View[] + * @throws Exception + */ + public function findAllByTableIds(array $tableIds): array { + if (empty($tableIds)) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $qb->select('v.*', 't.ownership') + ->from($this->table, 'v') + ->innerJoin('v', 'tables_tables', 't', 't.id = v.table_id') + ->where($qb->expr()->in('v.table_id', $qb->createParameter('tableIds'))) + ->addOrderBy('v.sidebar_order', 'ASC') + ->addOrderBy('v.id', 'ASC'); + + $chunks = []; + foreach (array_chunk($tableIds, self::DB_CHUNK_SIZE - 1) as $tableIdsChunk) { + $qb->setParameter('tableIds', $tableIdsChunk, IQueryBuilder::PARAM_INT_ARRAY); + $chunks[] = $this->findEntities($qb); + } + + return array_merge(...$chunks); + } + /** * @return View[] * @throws Exception diff --git a/lib/Helper/UserHelper.php b/lib/Helper/UserHelper.php index 1a8dc7934d..cfd21a71b6 100644 --- a/lib/Helper/UserHelper.php +++ b/lib/Helper/UserHelper.php @@ -31,6 +31,23 @@ public function getUserDisplayName(string $userId): string { } } + /** + * @param string[] $userIds + * @return array + */ + public function getUsersDisplayNames(array $userIds): array { + $displayNames = []; + foreach (array_unique($userIds) as $userId) { + if ($userId === '' || $userId === null) { + $displayNames[$userId] = $userId; + continue; + } + $displayName = $this->userManager->getDisplayName($userId); + $displayNames[$userId] = $displayName ?: $userId; + } + return $displayNames; + } + /** * @throws InternalError */ diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index a73e842961..f6bb059d93 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -785,16 +785,24 @@ public function findOrCreateColumnsByTitleForTableAsArray(?int $tableId, ?int $v } /** - * @param int $tableId - * @return int - * @throws PermissionError + * @param int[] $tableIds + * @param string|null $userId + * @return array */ - public function getColumnsCount(int $tableId): int { - if ($this->permissionsService->canReadColumnsByTableId($tableId)) { - return $this->mapper->countColumns($tableId); - } else { - throw new PermissionError('no read access for counting to table id = ' . $tableId); + public function getColumnsCountForTables(array $tableIds, ?string $userId = null): array { + $userId = $this->permissionsService->preCheckUserId($userId); + $readableTableIds = array_values(array_filter( + $tableIds, + fn (int $tableId): bool => $this->permissionsService->canReadColumnsByTableId($tableId, $userId), + )); + + $counts = $this->mapper->countColumnsForTables($readableTableIds); + + $result = []; + foreach ($tableIds as $tableId) { + $result[$tableId] = $counts[$tableId] ?? 0; } + return $result; } /** diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 3e98e7d4a2..13b291f1aa 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -845,28 +845,16 @@ public function deleteColumnDataFromRows(Column $column):void { $this->row2Mapper->deleteDataForColumn($column); } - /** - * @param int $tableId - * @return int - * - * @throws PermissionError - */ - public function getRowsCount(int $tableId): int { - if ($this->permissionsService->canReadRowsByElementId($tableId, 'table')) { - return $this->row2Mapper->countRowsForTable($tableId); - } else { - throw new PermissionError('no read access for counting to table id = ' . $tableId); - } - } - /** * @param int[] $tableIds + * @param string|null $userId * @return array */ - public function getRowsCountForTables(array $tableIds): array { + public function getRowsCountForTables(array $tableIds, ?string $userId = null): array { + $userId = $this->permissionsService->preCheckUserId($userId); $readableTableIds = array_values(array_filter( $tableIds, - fn (int $tableId): bool => $this->permissionsService->canReadRowsByElementId($tableId, 'table'), + fn (int $tableId): bool => $this->permissionsService->canReadRowsByElementId($tableId, 'table', $userId), )); $counts = $this->row2Mapper->countRowsForTables($readableTableIds); @@ -880,12 +868,13 @@ public function getRowsCountForTables(array $tableIds): array { /** * @param View $view - * @param string $userId + * @param string|null $userId * @return int * * @throws PermissionError */ - public function getViewRowsCount(View $view, string $userId): int { + public function getViewRowsCount(View $view, ?string $userId = null): int { + $userId = $this->permissionsService->preCheckUserId($userId); if ($this->permissionsService->canReadRowsByElementId($view->getId(), 'view', $userId)) { try { return $this->row2Mapper->countRowsForView($view, $userId); diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php index 1d5408c188..8d8a575c7d 100644 --- a/lib/Service/ShareService.php +++ b/lib/Service/ShareService.php @@ -82,12 +82,10 @@ public function __construct( * @throws InternalError * @return Share[] */ - public function findAll(string $nodeType, int $nodeId, ?string $userId = null, bool $enhanceShares = true): array { - $userId = $this->permissionsService->preCheckUserId($userId); - + public function findAll(string $nodeType, int $nodeId, bool $enhanceShares = true): array { try { $excluded = !$this->circleHelper->isCirclesEnabled() ? [ShareReceiverType::CIRCLE] : []; - $shares = $this->mapper->findAllSharesForNode($nodeType, $nodeId, $userId, $excluded); + $shares = $this->mapper->findAllSharesForNode($nodeType, $nodeId, $excluded); return $enhanceShares ? $this->addReceiverDisplayNames($shares) : $shares; } catch (Exception $e) { @@ -259,6 +257,47 @@ public function findTablesSharedWithMe(?string $userId = null): array { return $this->findElementsSharedWithMe('table', $userId); } + /** + * @param int[] $nodeIds + * @return array + * @throws InternalError + */ + public function countSharesForNodes(string $nodeType, array $nodeIds): array { + if (empty($nodeIds)) { + return []; + } + + try { + $excluded = !$this->circleHelper->isCirclesEnabled() ? [ShareReceiverType::CIRCLE] : []; + $shares = $this->mapper->findAllSharesForNodes($nodeType, $nodeIds, $excluded); + + $counts = array_fill_keys($nodeIds, 0); + foreach ($shares as $share) { + $nodeId = $share->getNodeId(); + $counts[$nodeId] = ($counts[$nodeId] ?? 0) + 1; + } + + return $counts; + } catch (Exception $e) { + $this->logger->error($e->getMessage()); + throw new InternalError($e->getMessage()); + } + } + + /** + * @param int[] $tableIds + * @return int[] + * @throws InternalError + */ + public function getTableIdsWithLinkShares(array $tableIds): array { + try { + return $this->mapper->findTableIdsWithLinkShares($tableIds); + } catch (Exception $e) { + $this->logger->error($e->getMessage()); + throw new InternalError($e->getMessage()); + } + } + /** * @throws InternalError */ @@ -868,7 +907,7 @@ public function deleteAllForView(View $view):void { public function deleteAllForContext(Context $context): void { try { $this->atomic(function () use ($context): void { - $shares = $this->mapper->findAllSharesForNode('context', $context->getId(), $this->userId); + $shares = $this->mapper->findAllSharesForNode('context', $context->getId()); foreach ($shares as $share) { $this->contextNavigationMapper->deleteByShareId($share->getId()); } @@ -884,7 +923,7 @@ public function deleteAllForContext(Context $context): void { * @return Share[] */ public function changeSenderForNode(string $nodeType, int $nodeId, string $newOwnerUserId, ?string $userId = null): array { - $sharesForTable = $this->findAll($nodeType, $nodeId, $userId, false); + $sharesForTable = $this->findAll($nodeType, $nodeId, false); $newShares = []; foreach ($sharesForTable as $share) { @@ -946,7 +985,7 @@ public function transferSharesForContext(int $contextId, string $newOwnerId, str */ public function findSharedWithUserIds(int $elementId, string $elementType): array { try { - $shares = $this->mapper->findAllSharesForNode($elementType, $elementId, ''); + $shares = $this->mapper->findAllSharesForNode($elementType, $elementId); $sharedWithUserIds = []; foreach ($shares as $share) { diff --git a/lib/Service/TableService.php b/lib/Service/TableService.php index 11d2ca1173..a3a23a4ee9 100644 --- a/lib/Service/TableService.php +++ b/lib/Service/TableService.php @@ -143,14 +143,13 @@ public function findAll(?string $userId = null, bool $skipTableEnhancement = fal // enhance table objects with additional data if (!$skipTableEnhancement) { - $rowCountsByTableId = $this->rowService->getRowsCountForTables(array_keys($allTables)); + /** @var string $userId */ + try { + $this->enhanceTables($allTables, $userId); + } catch (InternalError|PermissionError $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } foreach ($allTables as $table) { - /** @var string $userId */ - try { - $this->enhanceTable($table, $userId, $rowCountsByTableId); - } catch (InternalError|PermissionError $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - } // if the table is shared with me, there are no other shares // will avoid showing the shared icon in the FE nav if ($table->getIsShared()) { @@ -171,55 +170,120 @@ public function formatTables(array $tables): array { } /** - * add some basic values related to this table in context + * Adds some basic values related to a list of tables in context (rows count, + * columns count, shares count, etc.) * * $userId can be set or '' - * @param Table $table + * + * @param Table[] $tables * @param string $userId * @throws InternalError * @throws PermissionError */ - private function enhanceTable(Table $table, string $userId, ?array $rowCountsByTableId = null): void { - // add owner display name for UI - $this->addOwnerDisplayName($table); + private function enhanceTables(array $tables, string $userId): void { + if (empty($tables)) { + return; + } - // set hasShares if this table is shared by you (you share it with somebody else) - // (senseless if we have no user in context) + $tablesById = []; + foreach ($tables as $table) { + $tablesById[$table->getId()] = $table; + } + $tableIds = array_keys($tablesById); + + // add owner display names for UI + $ownerIds = array_values(array_filter( + array_unique(array_map(static fn (Table $table) => $table->getOwnership(), $tables)) + )); + $ownerDisplayNames = $this->userHelper->getUsersDisplayNames($ownerIds); + foreach ($tables as $table) { + $ownerId = $table->getOwnership() ?? ''; + $table->setOwnerDisplayName($ownerDisplayNames[$ownerId] ?? $ownerId); + } + + // add the rows and columns counts + $rowsCounts = []; + try { + $rowsCounts = $this->rowService->getRowsCountForTables($tableIds, $userId); + } catch (InternalError|PermissionError|OcpDbException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } + $columnsCounts = []; + try { + $columnsCounts = $this->columnService->getColumnsCountForTables($tableIds, $userId); + } catch (InternalError|PermissionError|OcpDbException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } + foreach ($tables as $table) { + $table->setRowsCount($rowsCounts[$table->getId()] ?? 0); + $table->setColumnsCount($columnsCounts[$table->getId()] ?? 0); + } + + // set hasShares / public isShared in one batch + $linkShareTableIds = []; + $sharesCount = []; + try { + if ($userId === '') { + $linkShareTableIds = $this->shareService->getTableIdsWithLinkShares($tableIds); + $linkShareTableIds = array_flip($linkShareTableIds); + } else { + $sharesCount = $this->shareService->countSharesForNodes('table', $tableIds); + } + } catch (InternalError|PermissionError|OcpDbException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } + foreach ($tables as $table) { + if ($userId === '') { + $table->setIsShared(isset($linkShareTableIds[$table->getId()])); + $table->setOnSharePermissions(new Permissions(read: true)); + } else { + $table->setHasShares(($sharesCount[$table->getId()] ?? 0) > 0); + } + } + + // set isShared and onSharePermissions (kept as is, per table) if ($userId !== '') { - try { - $shares = $this->shareService->findAll('table', $table->getId()); - $table->setHasShares(count($shares) !== 0); - } catch (InternalError) { + foreach ($tables as $table) { + try { + $this->setIsSharedState($table, $userId); + } catch (InternalError|PermissionError $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + $table->setIsShared(false); + $table->setOnSharePermissions(new Permissions()); + } } } - // add the rows count - if ($rowCountsByTableId !== null) { - $table->setRowsCount($rowCountsByTableId[$table->getId()] ?? 0); - } else { + // add the corresponding views if it is an own table, or you have table manage rights + $tablesToLoadViews = []; + foreach ($tables as $table) { + $table->setViews([]); try { - $table->setRowsCount($this->rowService->getRowsCount($table->getId())); - } catch (PermissionError) { - $table->setRowsCount(0); + if ($this->permissionsService->canManageTable($table, $userId)) { + $tablesToLoadViews[] = $table; + } + } catch (InternalError|PermissionError|OcpDbException $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); } } - // add the column count try { - $table->setColumnsCount($this->columnService->getColumnsCount($table->getId())); - } catch (PermissionError) { - $table->setColumnsCount(0); - } - - $this->setIsSharedState($table, $userId); - - if (!$table->getIsShared() || $table->getOnSharePermissions()->manage) { - // add the corresponding views if it is an own table, or you have table manage rights - $table->setViews($this->viewService->findAll($table)); + $viewsByTable = $this->viewService->findForTables($tablesToLoadViews, $userId, $rowsCounts); + foreach ($tablesToLoadViews as $table) { + $table->setViews($viewsByTable[$table->getId()] ?? []); + } + } catch (InternalError|PermissionError $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + foreach ($tablesToLoadViews as $table) { + $table->setViews([]); + } } - if ($this->favoritesService->isFavorite(Application::NODE_TYPE_TABLE, $table->getId())) { - $table->setFavorite(true); + // add favorites + foreach ($tables as $table) { + if ($this->favoritesService->isFavorite(Application::NODE_TYPE_TABLE, $table->getId())) { + $table->setFavorite(true); + } } } @@ -266,7 +330,7 @@ public function find(int $id, bool $skipTableEnhancement = false, ?string $userI } if (!$skipTableEnhancement) { - $this->enhanceTable($table, $userId); + $this->enhanceTables([$table], $userId); } return $table; @@ -335,11 +399,11 @@ public function create(string $title, string $template, ?string $emoji, ?string throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } } else { - $table = $this->addOwnerDisplayName($newTable); + $table = $newTable; } try { - $this->enhanceTable($table, $userId); + $this->enhanceTables([$table], $userId); } catch (InternalError|PermissionError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -571,7 +635,7 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip $this->federationService->notifyNodeUpdate($table, 'table'); try { - $this->enhanceTable($table, $userId); + $this->enhanceTables([$table], $userId); } catch (InternalError|PermissionError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(static::class . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -604,9 +668,7 @@ public function search(string $term, int $limit = 100, int $offset = 0, ?string /** @var string $userId */ $userId = $this->permissionsService->preCheckUserId($userId); $tables = $this->mapper->search($term, $userId, $limit, $offset); - foreach ($tables as &$table) { - $this->enhanceTable($table, $userId); - } + $this->enhanceTables($tables, $userId); return $tables; } catch (InternalError|PermissionError|OcpDbException) { return []; @@ -619,23 +681,15 @@ public function search(string $term, int $limit = 100, int $offset = 0, ?string * @throws InternalError */ public function getScheme(int $id, ?string $userId = null): TableScheme { - $table = $this->find($id, skipTableEnhancement: true); - $columns = $this->columnService->findAllByTable($id, null, $table); - $this->enhanceTable($table, $userId ?? ''); + $userId = $this->permissionsService->preCheckUserId($userId); + $table = $this->find($id, skipTableEnhancement: true, userId: $userId); + $columns = $this->columnService->findAllByTable($id, $userId, $table); + $this->enhanceTables([$table], $userId); return new TableScheme($table->getTitle(), $table->getEmoji(), $columns, $table->getViews() ?: [], $table->getDescription() ?: '', $this->appManager->getAppVersion('tables'), $table->getColumnOrderSettingsArray(), $table->getSortArray(), $table->getUuid()); } // PRIVATE FUNCTIONS --------------------------------------------------------------- - /** - * @param Table $table - * @return Table - */ - private function addOwnerDisplayName(Table $table): Table { - $table->setOwnerDisplayName($this->userHelper->getUserDisplayName($table->getOwnership())); - return $table; - } - /** * @param array $table * @param string $userId diff --git a/lib/Service/ViewService.php b/lib/Service/ViewService.php index 7ee532d32f..c4b5d0213a 100644 --- a/lib/Service/ViewService.php +++ b/lib/Service/ViewService.php @@ -71,31 +71,58 @@ public function __construct( /** * @param Table $table * @param string|null $userId + * @param int|null $tableRowsCount * @return array * @throws InternalError * @throws PermissionError */ - public function findAll(Table $table, ?string $userId = null): array { + public function findAll(Table $table, ?string $userId = null, ?int $tableRowsCount = null): array { + /** @var string $userId */ $userId = $this->permissionsService->preCheckUserId($userId); // $userId can be set or '' - try { - // security + $viewsByTable = $this->findForTables([$table], $userId, [$table->getId() => $tableRowsCount]); + return $viewsByTable[$table->getId()] ?? []; + } + + /** + * @param Table[] $tables + * @param string|null $userId + * @param array $tableRowsCounts + * @return array + * @throws InternalError + * @throws PermissionError + */ + public function findForTables(array $tables, ?string $userId = null, array $tableRowsCounts = []): array { + /** @var string $userId */ + $userId = $this->permissionsService->preCheckUserId($userId); // $userId can be set or '' + + if (empty($tables)) { + return []; + } + + // security + foreach ($tables as $table) { if (!$this->permissionsService->canManageTable($table, $userId)) { throw new PermissionError('PermissionError: can not read views for tableId ' . $table->getId()); } + } - $allViews = $this->mapper->findAll($table->getId()); - foreach ($allViews as $view) { - $this->enhanceView($view, $userId); - } - return $allViews; + $tableIds = array_map(static fn (Table $table) => $table->getId(), $tables); + + try { + $allViews = $this->mapper->findAllByTableIds($tableIds); } catch (\OCP\DB\Exception|InternalError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError($e->getMessage()); - } catch (PermissionError $e) { - $this->logger->debug('permission error during looking for views', ['exception' => $e]); - throw new PermissionError($e->getMessage()); } + + $this->enhanceViews($allViews, $userId, $tableRowsCounts); + + $viewsByTable = []; + foreach ($allViews as $view) { + $viewsByTable[$view->getTableId()][] = $view; + } + return $viewsByTable; } /** @@ -166,9 +193,7 @@ public function findSharedViewsWithMe(?string $userId = null): array { } } - foreach ($sharedViews as $view) { - $this->enhanceView($view, $userId); - } + $this->enhanceViews($sharedViews, $userId, []); return array_values($sharedViews); } @@ -522,34 +547,98 @@ public function deleteByObject(View $view, ?string $userId = null): View { * add some basic values related to this view in context * * $userId can be set or '' + * + * @param int|null $tableRowsCount */ - private function enhanceView(View $view, string $userId): void { - if ($view->isFederated()) { - $this->enhanceFederatedView($view, $userId); - return; - } - - // add owner display name for UI - $view->setOwnerDisplayName($this->userHelper->getUserDisplayName($view->getOwnership())); + private function enhanceView(View $view, string $userId, ?int $tableRowsCount = null): void { + $this->enhanceViews([$view], $userId, [$view->getTableId() => $tableRowsCount]); + } - $this->setIsSharedState($view, $userId); + /** + * add some basic values related to a list of views in context + * + * $userId can be set or '' + * + * @param View[] $views + * @param array $tableRowsCounts + */ + private function enhanceViews(array $views, string $userId, array $tableRowsCounts = []): void { + $localViews = []; + foreach ($views as $view) { + if ($view->isFederated()) { + $this->enhanceFederatedView($view, $userId); + } else { + $localViews[] = $view; + } + } + $views = $localViews; - if (!$this->permissionsService->canReadRowsByElement($view, 'view', $userId)) { + if (empty($views)) { return; } - // add the rows count - try { - $view->setRowsCount($this->rowService->getViewRowsCount($view, $userId)); - } catch (InternalError|PermissionError) { + + // add owner display names for UI in one batch + $ownerIds = array_values(array_filter( + array_unique(array_map(static fn (View $view) => $view->getOwnership(), $views)) + )); + $ownerDisplayNames = $this->userHelper->getUsersDisplayNames($ownerIds); + foreach ($views as $view) { + $ownerId = $view->getOwnership() ?? ''; + $view->setOwnerDisplayName($ownerDisplayNames[$ownerId] ?? $ownerId); } // Filter and sort details are withheld from share recipients by // View::jsonSerialize(). They must not be stripped from the entity here: // ViewMapper caches it for the whole request, so the row queries built // from it would lose the filter and return excluded rows. + $rowsCountCache = []; - if ($this->favoritesService->isFavorite(Application::NODE_TYPE_VIEW, $view->getId())) { - $view->setFavorite(true); + $sharesCounts = []; + if ($userId !== '') { + $ownedViewIds = []; + foreach ($views as $view) { + if ($userId === $view->getOwnership()) { + $ownedViewIds[] = $view->getId(); + } + } + + if (!empty($ownedViewIds)) { + try { + $sharesCounts = $this->shareService->countSharesForNodes('view', $ownedViewIds); + } catch (InternalError $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + } + } + } + + foreach ($views as $view) { + $this->setIsSharedState($view, $userId, $sharesCounts[$view->getId()] ?? null); + + if (!$this->permissionsService->canReadRowsByElement($view, 'view', $userId)) { + continue; + } + + // add the rows count + $tableRowsCount = $tableRowsCounts[$view->getTableId()] ?? null; + if ($tableRowsCount !== null && $view->getFilterArray() === []) { + $view->setRowsCount($tableRowsCount); + } else { + $cacheKey = $view->getTableId() . ':' . json_encode($view->getFilterArray()) . ':' . $userId; + if (isset($rowsCountCache[$cacheKey])) { + $view->setRowsCount($rowsCountCache[$cacheKey]); + } else { + try { + $count = $this->rowService->getViewRowsCount($view, $userId); + $view->setRowsCount($count); + $rowsCountCache[$cacheKey] = $count; + } catch (InternalError|PermissionError) { + } + } + } + + if ($this->favoritesService->isFavorite(Application::NODE_TYPE_VIEW, $view->getId())) { + $view->setFavorite(true); + } } } @@ -568,7 +657,7 @@ private function enhanceFederatedView(View $view, string $userId): void { )); } - private function setIsSharedState(View $view, string $userId): void { + private function setIsSharedState(View $view, string $userId, ?int $sharesCount = null): void { // set if this is a shared table with you (somebody else shared it with you) // (senseless if we have no user in context) if ($userId !== '') { @@ -608,10 +697,14 @@ private function setIsSharedState(View $view, string $userId): void { } else { // set hasShares if this table is shared by you (you share it with somebody else) // (senseless if we have no user in context) - try { - $allShares = $this->shareService->findAll('view', $view->getId()); - $view->setHasShares(count($allShares) !== 0); - } catch (InternalError) { + if ($sharesCount !== null) { + $view->setHasShares($sharesCount > 0); + } else { + try { + $counts = $this->shareService->countSharesForNodes('view', [$view->getId()]); + $view->setHasShares(($counts[$view->getId()] ?? 0) > 0); + } catch (InternalError) { + } } } } else { @@ -715,9 +808,7 @@ public function search(string $term, int $limit = 100, int $offset = 0, ?string /** @var string $userId */ $userId = $this->permissionsService->preCheckUserId($userId); $views = $this->mapper->search($term, $userId, $limit, $offset); - foreach ($views as $view) { - $this->enhanceView($view, $userId); - } + $this->enhanceViews($views, $userId, []); return $views; } catch (InternalError|\OCP\DB\Exception) { return []; diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index 1c40674eca..80a88ed626 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -937,6 +937,134 @@ Feature: APIv2 When user "participant4-v2" attempts to fetch all shares of table t1 Then the reported status is 404 + @api2 @sharing @tables + Scenario: A table manager sees all shares of a table, including indirect ones + Given group "table-managers" exists + And user "participant2-v2" is member of group "table-managers" + And table "Managed table" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And user "participant1-v2" shares "table" "t1" with "group" "table-managers" + And user "participant1-v2" sets permission "manage" to 1 + And user "participant1-v2" shares "table" "t1" with "user" "participant3-v2" + # participant2-v2 manages the table indirectly through the group share + And user "participant2-v2" has the following permissions against "table" "t1" + | manage | 1 | + # all shares are found, also the ones sent by the owner + When user "participant2-v2" fetches table info for table "t1" + Then user "participant2-v2" sees the following table attributes on table "t1" + | isShared | 1 | + | hasShares | 1 | + When user "participant2-v2" attempts to fetch all shares of table t1 + Then the reported status is "200" + And the response contains 2 shares + # a share created by the co-manager is still counted for the owner + When user "participant2-v2" shares "table" "t1" with "user" "participant4-v2" + Then the reported status is "200" + When user "participant1-v2" fetches table info for table "t1" + Then user "participant1-v2" sees the following table attributes on table "t1" + | hasShares | 1 | + When user "participant1-v2" attempts to fetch all shares of table t1 + Then the reported status is "200" + And the response contains 3 shares + + @api2 @tables @views + Scenario: Table response contains counters and embedded views + Given table "Stats table" with emoji "📊" exists for user "participant1-v2" as "t1" via v2 + And column "one" exists with following properties + | type | text | + | subtype | line | + | mandatory | 0 | + And column "two" exists with following properties + | type | number | + | mandatory | 0 | + And using table "t1" + And user "participant1-v2" creates row "r1" with following values: + | one | A | + | two | 1 | + And user "participant1-v2" creates row "r2" with following values: + | one | B | + | two | 2 | + And user "participant1-v2" create view "Filtered" with emoji "🔎" for "t1" as "v1" + And user "participant1-v2" sets filter to view "v1" + | column | operator | value | + | one | is-equal | A | + And user "participant1-v2" create view "All rows" with emoji "📋" for "t1" as "v2" + And user "participant1-v2" shares view "v2" with "participant3-v2" + When user "participant1-v2" fetches table info for table "t1" + Then user "participant1-v2" sees the following table attributes on table "t1" + | columnsCount | 2 | + | rowsCount | 2 | + | isShared | 0 | + | hasShares | 0 | + | ownerDisplayName | participant1-v2-displayname | + # views are embedded in sidebar order, each with its own rows count + And user "participant1-v2" sees the following views of table "t1" + | alias | rowsCount | hasShares | isShared | + | v1 | 1 | 0 | 0 | + | v2 | 2 | 1 | 0 | + + @api2 @sharing @views + Scenario: Views of a table can only be listed by owners and managers + Given table "View listing" with emoji "📋" exists for user "participant1-v2" as "t1" via v2 + And user "participant1-v2" create view "v1" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" create view "v2" with emoji "🦉" for "t1" as "v2" + And user "participant1-v2" shares "table" "t1" with "user" "participant2-v2" + And user "participant1-v2" shares "table" "t1" with "user" "participant3-v2" + And user "participant1-v2" sets permission "manage" to 1 + When user "participant1-v2" attempts to fetch views of table "t1" + Then the reported status is "200" + And the response contains 2 views + When user "participant3-v2" attempts to fetch views of table "t1" + Then the reported status is "200" + And the response contains 2 views + When user "participant2-v2" attempts to fetch views of table "t1" + Then the reported status is "403" + When user "participant4-v2" attempts to fetch views of table "t1" + Then the reported status is "404" + # the same rule applies to the views embedded in the table response: + # they are only loaded for the owner and managers + When user "participant3-v2" fetches table info for table "t1" + Then user "participant3-v2" sees the following views of table "t1" + | alias | + | v1 | + | v2 | + When user "participant2-v2" fetches table info for table "t1" + Then user "participant2-v2" sees the following table attributes on table "t1" + | isShared | 1 | + | hasShares | 1 | + And user "participant2-v2" sees the following views of table "t1" + | alias | + + @api2 @sharing @views + Scenario: A shared view hides its filter details for sharees without table manage permission + Given table "Filtered shared view" with emoji "🔎" exists for user "participant1-v2" as "t1" via v2 + And column "state" exists with following properties + | type | text | + | subtype | line | + | mandatory | 0 | + And using table "t1" + And user "participant1-v2" creates row "r1" with following values: + | state | open | + And user "participant1-v2" creates row "r2" with following values: + | state | completed | + And user "participant1-v2" create view "Open items" with emoji "🔔" for "t1" as "v1" + And user "participant1-v2" sets filter to view "v1" + | column | operator | value | + | state | is-equal | open | + And user "participant1-v2" shares view "v1" with "participant2-v2" + # the owner sees the real filter and the share count + When user "participant1-v2" fetches view info for view "v1" + Then user "participant1-v2" sees the following view attributes on view "v1" + | isShared | 0 | + | hasShares | 1 | + | rowsCount | 1 | + # the sharee gets the rows count, but the filter details are hidden + When user "participant2-v2" fetches view info for view "v1" + Then user "participant2-v2" sees the following view attributes on view "v1" + | isShared | 1 | + | hasShares | 0 | + | rowsCount | 1 | + | filter | [] | + @api2 @sharing @views Scenario: Create a shared view and check its permissions Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 4f6340767f..bed558c7e4 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -64,6 +64,8 @@ class FeatureContext implements Context { // Store data from last request to perform assertions, id is used as a key private array $tableData = []; + private array $viewData = []; + private array $sharedViewsData = []; private $importColumnData = null; @@ -1349,6 +1351,27 @@ public function attemptToFetchAllShares(string $user, string $element, string $a ); } + /** + * @When user :user attempts to fetch views of table :tableAlias + */ + public function attemptToFetchAllViews(string $user, string $tableAlias): void { + $this->setCurrentUser($user); + $tableId = $this->collectionManager->getByAlias('table', $tableAlias)['id']; + + $this->sendRequest( + 'GET', + sprintf('/apps/tables/api/1/tables/%d/views', $tableId) + ); + } + + /** + * @Then the response contains :count shares + * @Then the response contains :count views + */ + public function theResponseContainsShares(int $count): void { + Assert::assertCount($count, $this->getDataFromResponse($this->response)); + } + /** * @Then user :user has the following permissions */ @@ -2203,13 +2226,106 @@ public function userSeesTheFollowingTableAttributesOnTable($user, $tableName, Ta foreach ($table->getRows() as $row) { $attribute = $row[0]; $value = $row[1]; - if (in_array($attribute, ['archived', 'favorite'])) { + if (in_array($attribute, ['archived', 'favorite', 'hasShares', 'isShared'])) { $value = (bool)$value; } Assert::assertEquals($value, $this->tableData[$tableName][$attribute]); } } + /** + * @Then user :user sees the following views of table :tableName + * + * The first row of the table contains the attribute names to check, + * the first column is always the view alias. Views are compared in order. + */ + public function userSeesTheFollowingViewsOfTable(string $user, string $tableName, TableNode $table): void { + $views = $this->tableData[$tableName]['views'] ?? []; + $rows = $table->getRows(); + $headers = array_shift($rows); + + Assert::assertCount(count($rows), $views, 'View count does not match'); + foreach ($rows as $i => $row) { + $alias = $row[0]; + $viewId = $this->collectionManager->getByAlias('view', $alias)['id']; + Assert::assertEquals($viewId, $views[$i]['id'], 'Unexpected view at position ' . $i); + foreach (array_slice($row, 1) as $j => $expected) { + $attribute = $headers[$j + 1]; + $actual = $views[$i][$attribute]; + if (is_bool($actual)) { + $expected = (bool)$expected; + } elseif (is_array($actual)) { + $expected = json_decode((string)$expected, true); + } elseif (is_int($actual)) { + $expected = (int)$expected; + } + Assert::assertEquals($expected, $actual, sprintf('Attribute %s of view %s is not as expected', $attribute, $alias)); + } + } + } + + /** + * @When user :user fetches view info for view :viewAlias + */ + public function userFetchesViewInfo(string $user, string $viewAlias): void { + $this->setCurrentUser($user); + $viewId = $this->collectionManager->getByAlias('view', $viewAlias)['id']; + + $this->sendRequest( + 'GET', + '/apps/tables/api/1/views/' . $viewId, + ); + + $this->viewData[$viewAlias] = $this->getDataFromResponse($this->response); + } + + /** + * @Then user :user sees the following view attributes on view :viewAlias + */ + public function userSeesTheFollowingViewAttributesOnView(string $user, string $viewAlias, TableNode $table): void { + $this->assertViewAttributes($this->viewData[$viewAlias] ?? [], $table); + } + + /** + * @When user :user fetches the views shared with them + */ + public function userFetchesViewsSharedWithThem(string $user): void { + $this->setCurrentUser($user); + + $this->sendRequest('GET', '/apps/tables/view'); + + $this->sharedViewsData = $this->getDataFromResponse($this->response); + } + + /** + * @Then view :viewAlias in the shared views list has the following attributes + */ + public function sharedViewsListViewHasAttributes(string $viewAlias, TableNode $table): void { + $viewId = $this->collectionManager->getByAlias('view', $viewAlias)['id']; + $matches = array_values(array_filter( + $this->sharedViewsData, + static fn (array $view): bool => $view['id'] === $viewId + )); + Assert::assertCount(1, $matches, 'View not found in the shared views list'); + $this->assertViewAttributes($matches[0], $table); + } + + private function assertViewAttributes(array $view, TableNode $table): void { + foreach ($table->getRows() as $row) { + $attribute = $row[0]; + $expected = $row[1]; + $actual = $view[$attribute] ?? null; + if (is_bool($actual)) { + $expected = (bool)$expected; + } elseif (is_array($actual)) { + $expected = json_decode((string)$expected, true); + } elseif (is_int($actual)) { + $expected = (int)$expected; + } + Assert::assertEquals($expected, $actual, sprintf('Attribute %s is not as expected', $attribute)); + } + } + /** * @Given user :user adds the table :tableName to favorites */ diff --git a/tests/integration/features/federation.feature b/tests/integration/features/federation.feature index 38c8fb380d..8159a915ab 100644 --- a/tests/integration/features/federation.feature +++ b/tests/integration/features/federation.feature @@ -54,6 +54,32 @@ Feature: Federation And user "federation-receiver" sees the following rows in view "federated-v1" | from owner | + @federation + Scenario: A federated view keeps its federated attributes when enhanced + Given acting on server "LOCAL" + And user "federation-owner" create view "Federated view" with emoji "🌍" for "t1" as "v1" + And user "federation-owner" sets columnSettings "statement" to view "v1" + And user "federation-owner" shares "view" "v1" with "remote" "federation-receiver@http://localhost:8180" + And user "federation-owner" sets permission "create" to 1 + And acting on server "REMOTE" + And user "federation-receiver" has federated view "Federated view" as "federated-v1" + When user "federation-receiver" fetches the views shared with them + Then view "federated-v1" in the shared views list has the following attributes + | title | Federated view | + | isFederated | 1 | + | isShared | 1 | + | hasShares | 0 | + | rowsCount | 0 | + | onSharePermissions | {"read":true,"create":true,"update":true,"delete":false,"manage":false} | + When user "federation-receiver" fetches view info for view "federated-v1" + Then user "federation-receiver" sees the following view attributes on view "federated-v1" + | title | Federated view | + | isFederated | 1 | + | isShared | 1 | + | hasShares | 0 | + | rowsCount | 0 | + | onSharePermissions | {"read":true,"create":true,"update":true,"delete":false,"manage":false} | + @federation Scenario: Receiver creates a row in a federated view after the owner grants the permission Given acting on server "LOCAL" diff --git a/tests/unit/Service/ShareServiceTest.php b/tests/unit/Service/ShareServiceTest.php index 1ab87968e5..4dfb6e57bd 100644 --- a/tests/unit/Service/ShareServiceTest.php +++ b/tests/unit/Service/ShareServiceTest.php @@ -10,6 +10,8 @@ namespace OCA\Tables\Tests\Unit\Service; use OCA\Tables\AuditLog\Operation; +use OCA\Tables\Constants\ShareReceiverType; +use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextNavigationMapper; use OCA\Tables\Db\Share; use OCA\Tables\Db\ShareMapper; @@ -122,6 +124,41 @@ public function testCreateContextShareSetsAllPermissionsFalse(): void { $this->assertFalse($share->getPermissionManage()); } + public function testCountSharesForNodesCountsAllSendersAndIncludesZeros(): void { + $firstShare = new Share(); + $firstShare->setNodeId(1); + $firstShare->setSender('alice'); + $secondShare = new Share(); + $secondShare->setNodeId(1); + $secondShare->setSender('bob'); + $this->mapper->expects($this->exactly(2))->method('findAllSharesForNodes') + ->withConsecutive(['table', [1, 2], [ShareReceiverType::CIRCLE]], ['view', [1, 2], [ShareReceiverType::CIRCLE]]) + ->willReturn([$firstShare, $secondShare]); + + foreach (['table', 'view'] as $nodeType) { + $this->assertSame([1 => 2, 2 => 0], $this->shareService->countSharesForNodes($nodeType, [1, 2])); + } + } + + public function testCountSharesForNodesKeepsCirclesWhenEnabled(): void { + $this->circleHelper->method('isCirclesEnabled')->willReturn(true); + $this->mapper->expects($this->once())->method('findAllSharesForNodes')->with('view', [1], [])->willReturn([]); + + $this->assertSame([1 => 0], $this->shareService->countSharesForNodes('view', [1])); + } + + public function testCountSharesForNodesSkipsEmptyInput(): void { + $this->mapper->expects($this->never())->method('findAllSharesForNodes'); + + $this->assertSame([], $this->shareService->countSharesForNodes('table', [])); + } + + public function testGetTableIdsWithLinkSharesUsesRenamedMapperMethod(): void { + $this->mapper->expects($this->once())->method('findTableIdsWithLinkShares')->with([1, 2])->willReturn([2]); + + $this->assertSame([2], $this->shareService->getTableIdsWithLinkShares([1, 2])); + } + private function createTableShare(): Share { $share = new Share(); $share->setId(12); @@ -351,4 +388,38 @@ public function testDeleteForShareReviewPropagatesDbException(): void { $this->expectException(Exception::class); $this->shareService->deleteForShareReview(5); } + + public function testDeleteAllForContextCleansNavigationForAllSenders(): void { + $context = new Context(); + $context->setId(123); + + $share1 = new Share(); + $share1->setId(1); + $share1->setSender('userA'); + + $share2 = new Share(); + $share2->setId(2); + $share2->setSender('userB'); + + $deletedShareIds = []; + $this->mapper->expects($this->once()) + ->method('findAllSharesForNode') + ->with('context', 123) + ->willReturn([$share1, $share2]); + + $this->contextNavigationMapper->expects($this->exactly(2)) + ->method('deleteByShareId') + ->willReturnCallback(function (int $shareId) use (&$deletedShareIds): int { + $deletedShareIds[] = $shareId; + return 1; + }); + + $this->mapper->expects($this->once()) + ->method('deleteByNode') + ->with(123, 'context'); + + $this->shareService->deleteAllForContext($context); + + $this->assertEqualsCanonicalizing([1, 2], $deletedShareIds); + } } diff --git a/tests/unit/Service/TableServiceTest.php b/tests/unit/Service/TableServiceTest.php index 919661b377..09f72e9959 100644 --- a/tests/unit/Service/TableServiceTest.php +++ b/tests/unit/Service/TableServiceTest.php @@ -9,12 +9,218 @@ namespace OCA\Tables\Tests\Unit\Service; +use OCA\Tables\Activity\ActivityManager; use OCA\Tables\Db\Table; +use OCA\Tables\Db\TableMapper; +use OCA\Tables\Db\View; +use OCA\Tables\Errors\InternalError; +use OCA\Tables\Helper\UserHelper; +use OCA\Tables\Model\Permissions; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\TableScheme; +use OCA\Tables\Service\ColumnService; +use OCA\Tables\Service\ContextService; +use OCA\Tables\Service\FavoritesService; +use OCA\Tables\Service\FederationService; +use OCA\Tables\Service\PermissionsService; +use OCA\Tables\Service\RowService; +use OCA\Tables\Service\ShareService; +use OCA\Tables\Service\Support\AuditLogServiceInterface; +use OCA\Tables\Service\TableService; +use OCA\Tables\Service\TableTemplateService; +use OCA\Tables\Service\ViewService; +use OCP\App\IAppManager; +use OCP\DB\Exception; +use OCP\Defaults; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IDBConnection; +use OCP\IL10N; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; class TableServiceTest extends TestCase { + private PermissionsService&MockObject $permissionsService; + private TableMapper&MockObject $mapper; + private ColumnService&MockObject $columnService; + private RowService&MockObject $rowService; + private ShareService&MockObject $shareService; + private ViewService&MockObject $viewService; + private FavoritesService&MockObject $favoritesService; + private TableService $service; + + protected function setUp(): void { + parent::setUp(); + $this->permissionsService = $this->createMock(PermissionsService::class); + $this->permissionsService->method('preCheckUserId') + ->willReturnCallback(static fn (?string $userId = null): string => $userId ?? 'alice'); + $this->mapper = $this->createMock(TableMapper::class); + $this->columnService = $this->createMock(ColumnService::class); + $this->rowService = $this->createMock(RowService::class); + $this->shareService = $this->createMock(ShareService::class); + $this->viewService = $this->createMock(ViewService::class); + $this->favoritesService = $this->createMock(FavoritesService::class); + $this->service = new TableService( + $this->permissionsService, + $this->createMock(LoggerInterface::class), + 'alice', + $this->mapper, + $this->createMock(TableTemplateService::class), + $this->columnService, + $this->rowService, + $this->viewService, + $this->shareService, + $this->createMock(UserHelper::class), + $this->favoritesService, + $this->createMock(IEventDispatcher::class), + $this->createMock(ContextService::class), + $this->createMock(IAppManager::class), + $this->createMock(IDBConnection::class), + $this->createMock(IL10N::class), + $this->createMock(Defaults::class), + $this->createMock(ActivityManager::class), + $this->createMock(FederationService::class), + $this->createMock(AuditLogServiceInterface::class), + ); + } + + private function createTable(int $id): Table { + $table = new Table(); + $table->setId($id); + $table->setOwnership('alice'); + return $table; + } + + public function testGetSchemeUsesSessionUser(): void { + $table = $this->createTable(1); + $this->mapper->method('find')->with(1)->willReturn($table); + $this->permissionsService->method('canReadTable')->with($table, 'alice')->willReturn(true); + $this->permissionsService->method('canManageTable')->with($table, 'alice')->willReturn(true); + $this->columnService->expects($this->once())->method('findAllByTable')->with(1, 'alice', $table)->willReturn([]); + $this->rowService->expects($this->once())->method('getRowsCountForTables')->with([1], 'alice')->willReturn([1 => 7]); + $this->viewService->expects($this->once())->method('findForTables')->with([$table], 'alice', [1 => 7])->willReturn([]); + + $this->service->getScheme(1); + $this->assertSame(7, $table->getRowsCount()); + } + + public function testGetSchemeUsesExplicitUserForAccessChecks(): void { + $table = $this->createTable(1); + $table->setOwnership('bob'); + $this->mapper->method('find')->with(1)->willReturn($table); + $this->permissionsService->method('canReadTable')->with($table, 'bob')->willReturn(true); + $this->permissionsService->method('canManageTable')->with($table, 'bob')->willReturn(true); + $this->columnService->expects($this->once())->method('findAllByTable')->with(1, 'bob', $table)->willReturn([]); + $this->rowService->expects($this->once())->method('getRowsCountForTables')->with([1], 'bob')->willReturn([]); + + $this->service->getScheme(1, 'bob'); + } + + public function testGetSchemeDoesNotLoadViewsForReadOnlyRecipients(): void { + $table = $this->createTable(1); + $table->setOwnership('bob'); + $table->setViews([new View()]); + $this->mapper->method('find')->with(1)->willReturn($table); + $this->permissionsService->method('canReadTable')->willReturn(true); + $this->shareService->method('getSharedPermissionsIfSharedWithMe')->willReturn(new Permissions(read: true)); + $this->viewService->expects($this->once())->method('findForTables')->with([], 'alice', [])->willReturn([]); + + $this->assertSame([], $this->service->getScheme(1)->jsonSerialize()['views']); + $this->assertTrue($table->getIsShared()); + } + + public function testEnhancementContinuesAfterRowCountFailure(): void { + $this->assertEnhancementContinuesAfterFailure('rows'); + } + + public function testEnhancementContinuesAfterColumnCountFailure(): void { + $this->assertEnhancementContinuesAfterFailure('columns'); + } + + public function testEnhancementContinuesAfterShareCountFailure(): void { + $this->assertEnhancementContinuesAfterFailure('shares'); + } + + public function testEnhancementContinuesAfterViewLookupFailure(): void { + $this->assertEnhancementContinuesAfterFailure('views'); + } + + public function testViewPermissionFailureDoesNotStopOtherTables(): void { + $tables = [$this->createTable(1), $this->createTable(2)]; + $this->mapper->method('findAll')->willReturn($tables); + $this->permissionsService->method('canManageTable')->willReturnCallback(static function (Table $table): bool { + if ($table->getId() === 1) { + throw new InternalError('Table permissions unavailable'); + } + return true; + }); + $view = new View(); + $this->viewService->expects($this->once())->method('findForTables') + ->with([$tables[1]], 'alice', [])->willReturn([2 => [$view]]); + $this->favoritesService->expects($this->exactly(2))->method('isFavorite')->willReturn(true); + + $this->assertSame($tables, $this->service->findAll(skipSharedTables: true, createTutorial: false)); + $this->assertSame([], $tables[0]->getViews()); + $this->assertSame([$view], $tables[1]->getViews()); + $this->assertTrue($tables[0]->getFavorite()); + $this->assertTrue($tables[1]->getFavorite()); + } + + public function testPublicShareLookupFailureDoesNotStopEnhancement(): void { + $table = $this->createTable(1); + $this->mapper->method('find')->willReturn($table); + $this->permissionsService->method('canReadTable')->willReturn(true); + $this->rowService->method('getRowsCountForTables')->willReturn([1 => 7]); + $this->columnService->method('getColumnsCountForTables')->willReturn([1 => 3]); + $this->shareService->method('getTableIdsWithLinkShares')->willThrowException(new InternalError('Link shares unavailable')); + $this->favoritesService->method('isFavorite')->willReturn(true); + + $this->assertSame($table, $this->service->find(1, userId: '')); + $this->assertSame(7, $table->getRowsCount()); + $this->assertSame(3, $table->getColumnsCount()); + $this->assertTrue($table->getOnSharePermissions()->read); + $this->assertTrue($table->getFavorite()); + } + + private function assertEnhancementContinuesAfterFailure(string $failedStep): void { + $tables = [$this->createTable(1), $this->createTable(2)]; + $this->mapper->method('findAll')->willReturn($tables); + $this->permissionsService->method('canManageTable')->willReturn(true); + $this->favoritesService->expects($this->exactly(2))->method('isFavorite')->willReturn(true); + $rowCounts = $failedStep === 'rows' ? [] : [1 => 7, 2 => 9]; + if ($failedStep === 'rows') { + $this->rowService->method('getRowsCountForTables')->willThrowException($this->createMock(Exception::class)); + } else { + $this->rowService->method('getRowsCountForTables')->willReturn($rowCounts); + } + if ($failedStep === 'columns') { + $this->columnService->method('getColumnsCountForTables')->willThrowException($this->createMock(Exception::class)); + } else { + $this->columnService->method('getColumnsCountForTables')->willReturn([1 => 3, 2 => 4]); + } + if ($failedStep === 'shares') { + $this->shareService->method('countSharesForNodes')->willThrowException(new InternalError('Share count unavailable')); + } else { + $this->shareService->method('countSharesForNodes')->willReturn([1 => 1, 2 => 1]); + } + $view = new View(); + $viewLookup = $this->viewService->expects($this->once())->method('findForTables')->with($tables, 'alice', $rowCounts); + if ($failedStep === 'views') { + $viewLookup->willThrowException(new InternalError('Views unavailable')); + } else { + $viewLookup->willReturn([1 => [$view]]); + } + + $this->assertSame($tables, $this->service->findAll(skipSharedTables: true, createTutorial: false)); + foreach ($tables as $table) { + $this->assertTrue($table->getFavorite()); + $this->assertSame($rowCounts[$table->getId()] ?? 0, $table->getRowsCount()); + $this->assertSame($failedStep === 'columns' ? 0 : $table->getId() + 2, $table->getColumnsCount()); + $this->assertSame($failedStep !== 'shares', $table->getHasShares()); + } + $this->assertSame($failedStep === 'views' ? [] : [$view], $tables[0]->getViews()); + $this->assertSame([], $tables[1]->getViews()); + } public function testSortRoundTripSingleRule(): void { $input = [['columnId' => 3, 'mode' => 'ASC']];