diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index 434e2137bb..0184e28da9 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -17,6 +17,7 @@ use OCA\Tables\Errors\PermissionError; use OCA\Tables\Helper\IconHelper; use OCA\Tables\Middleware\Attribute\RequirePermission; +use OCA\Tables\Model\MenuItemInput; use OCA\Tables\ResponseDefinitions; use OCA\Tables\Service\ColumnService; use OCA\Tables\Service\ContextService; @@ -101,6 +102,7 @@ public function show(int $contextId): DataResponse { * @param string $iconName Material design icon name of the context * @param string $description Descriptive text of the context * @psalm-param list $nodes optional nodes to be connected to this context + * @psalm-param ?list $menuItems optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page * * @return DataResponse|DataResponse * @@ -109,7 +111,7 @@ public function show(int $contextId): DataResponse { * 403: lacking permissions on a resource */ #[NoAdminRequired] - public function create(string $name, string $iconName, string $description = '', array $nodes = []): DataResponse { + public function create(string $name, string $iconName, string $description = '', array $nodes = [], ?array $menuItems = null): DataResponse { try { if (!$this->isValidIcon($iconName)) { return new DataResponse(['message' => 'Invalid icon name'], Http::STATUS_BAD_REQUEST); @@ -121,13 +123,16 @@ public function create(string $name, string $iconName, string $description = '', $this->sanitizeInputNodes($nodes), $this->userId, 0, + $menuItems !== null ? MenuItemInput::createListFromInputArray($menuItems, allowNewStartPage: true) : null, )->jsonSerialize()); } catch (Exception $e) { return $this->handleError($e); } catch (PermissionError $e) { return $this->handlePermissionError($e); + } catch (BadRequestError $e) { + return $this->handleBadRequestError($e); } catch (InvalidArgumentException $e) { - return $this->handleError(new InternalError($e->getMessage(), $e->getCode(), $e)); + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); } } @@ -139,6 +144,7 @@ public function create(string $name, string $iconName, string $description = '', * @param ?string $iconName provide this parameter to set a new icon * @param ?string $description provide this parameter to set a new description * @param ?array{id: int, type: int, permissions: int, order: int} $nodes provide this parameter to set a new list of nodes. + * @psalm-param ?list $menuItems provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target * * @return DataResponse|DataResponse * @@ -150,7 +156,7 @@ public function create(string $name, string $iconName, string $description = '', * @CanManageContext */ #[NoAdminRequired] - public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, ?array $nodes): DataResponse { + public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null): DataResponse { try { if ($iconName !== null && !$this->isValidIcon($iconName)) { return new DataResponse(['message' => 'Invalid icon name'], Http::STATUS_BAD_REQUEST); @@ -163,6 +169,7 @@ public function update(int $contextId, ?string $name, ?string $iconName, ?string $iconName, $description, $nodes, + $menuItems !== null ? MenuItemInput::createListFromInputArray($menuItems) : null, )->jsonSerialize()); } catch (Exception|MultipleObjectsReturnedException $e) { return $this->handleError($e); @@ -170,6 +177,10 @@ public function update(int $contextId, ?string $name, ?string $iconName, ?string return $this->handlePermissionError($e); } catch (DoesNotExistException $e) { return $this->handleNotFoundError(new NotFoundError($e->getMessage(), $e->getCode(), $e)); + } catch (BadRequestError $e) { + return $this->handleBadRequestError($e); + } catch (InvalidArgumentException $e) { + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); } } diff --git a/lib/Db/Context.php b/lib/Db/Context.php index 3fa14f56b5..d465403521 100644 --- a/lib/Db/Context.php +++ b/lib/Db/Context.php @@ -28,6 +28,8 @@ * @method setNodes(array $value): void * @method getPages(): array * @method setPages(array $value): void + * @method getMenuItems(): array + * @method setMenuItems(array $value): void */ class Context extends EntitySuper implements JsonSerializable { protected ?string $name = null; @@ -40,8 +42,9 @@ class Context extends EntitySuper implements JsonSerializable { protected ?array $sharing = null; protected ?array $nodes = null; protected ?array $pages = null; + protected ?array $menuItems = null; - protected const VIRTUAL_PROPERTIES = ['sharing', 'nodes', 'pages']; + protected const VIRTUAL_PROPERTIES = ['sharing', 'nodes', 'pages', 'menuItems']; public function __construct() { $this->addType('id', 'integer'); @@ -56,8 +59,12 @@ public function jsonSerialize(): array { 'iconName' => $this->getIcon(), 'description' => $this->getDescription(), 'owner' => $this->getOwnerId(), - 'ownerType' => $this->getOwnerType() + 'ownerType' => $this->getOwnerType(), ]; + // left out where the menus are not loaded, such as for the navigation bar + if (is_array($this->menuItems)) { + $data['menuItems'] = array_values($this->menuItems); + } // extended data if (is_array($this->sharing) || is_array($this->nodes) || is_array($this->pages)) { diff --git a/lib/Db/ContextMapper.php b/lib/Db/ContextMapper.php index b5dd82380b..181dfb1d07 100644 --- a/lib/Db/ContextMapper.php +++ b/lib/Db/ContextMapper.php @@ -27,6 +27,7 @@ public function __construct( IDBConnection $db, protected UserHelper $userHelper, protected GroupHelper $groupHelper, + protected MenuItemMapper $menuItemMapper, ) { parent::__construct($db, $this->table, Context::class); } @@ -81,7 +82,10 @@ protected function getFindContextBaseQuery(?string $userId): IQueryBuilder { return $qb; } - protected function formatResultRows(array $rows, ?string $userId) { + /** + * @param array>|null $menuItemsByContext the menus by context id, null when they are not loaded + */ + protected function formatResultRows(array $rows, ?string $userId, ?array $menuItemsByContext = null) { $formatted = [ 'id' => $rows[0]['id'], 'name' => $rows[0]['name'], @@ -150,6 +154,13 @@ protected function formatResultRows(array $rows, ?string $userId) { return $carry; }, []); + if ($menuItemsByContext !== null) { + $formatted['menu_items'] = array_map( + static fn (MenuItem $menuItem): array => $menuItem->jsonSerialize(), + $menuItemsByContext[(int)$rows[0]['id']] ?? [], + ); + } + return $this->mapRowToEntity($formatted); } @@ -163,26 +174,9 @@ public function findAll(?string $userId = null): array { $result = $qb->executeQuery(); $r = $this->withOtherPages($result->fetchAllAssociative()); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; - } - $contextIds = array_keys($contextIds); - unset($row); - - $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ((int)$row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); - } - - return $resultEntities; + return $this->formatResultSet($r, $userId, true); } + public function findForNavBar(string $userId): array { $qb = $this->getFindContextBaseQuery($userId); $groupIDs = $this->userHelper->getGroupIdsForUser($userId); @@ -213,24 +207,27 @@ public function findForNavBar(string $userId): array { $result = $qb->executeQuery(); $r = $result->fetchAllAssociative(); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; + // the navigation bar shows the applications only, not their menus + return $this->formatResultSet($r, $userId, false); + } + + /** + * Groups the joined rows per context, loading the menus of all contexts in one query. + * + * @return Context[] + * @throws Exception + */ + private function formatResultSet(array $rows, ?string $userId, bool $withMenuItems): array { + $rowsByContext = []; + foreach ($rows as $row) { + $rowsByContext[(int)$row['id']][] = $row; } - $contextIds = array_keys($contextIds); - unset($row); + $menuItemsByContext = $withMenuItems ? $this->menuItemMapper->findByContextIds(array_keys($rowsByContext)) : null; $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ((int)$row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); + foreach ($rowsByContext as $contextRows) { + $resultEntities[] = $this->formatResultRows($contextRows, $userId, $menuItemsByContext); } - return $resultEntities; } @@ -249,7 +246,7 @@ public function findById(int $contextId, ?string $userId = null): Context { throw new NotFoundError('Context does not exist'); } - return $this->formatResultRows($r, $userId); + return $this->formatResultRows($r, $userId, $this->menuItemMapper->findByContextIds([$contextId])); } /** @@ -265,25 +262,8 @@ public function findAllContainingNode(int $nodeType, int $nodeId, string $userId $result = $qb->executeQuery(); $r = $result->fetchAllAssociative(); - $contextIds = []; - foreach ($r as $row) { - $contextIds[$row['id']] = 1; - } - $contextIds = array_keys($contextIds); - unset($row); - - $resultEntities = []; - foreach ($contextIds as $contextId) { - $workArray = []; - foreach ($r as $row) { - if ($row['id'] === $contextId) { - $workArray[] = $row; - } - } - $resultEntities[] = $this->formatResultRows($workArray, $userId); - } - - return $resultEntities; + // permission checks read the nodes only, not the menus + return $this->formatResultSet($r, $userId, false); } /** diff --git a/lib/Db/ContextNodeRelation.php b/lib/Db/ContextNodeRelation.php index fbabd3e4da..3f8d7bda9e 100644 --- a/lib/Db/ContextNodeRelation.php +++ b/lib/Db/ContextNodeRelation.php @@ -35,6 +35,21 @@ public function __construct() { $this->addType('permissions', 'integer'); } + /** + * The relation as an application carries it among its nodes, in the shape the + * context mapper reads it. + * + * @return array{id: int, node_id: int, node_type: int, permissions: int} + */ + public function toContextNode(): array { + return [ + 'id' => (int)$this->getId(), + 'node_id' => (int)$this->getNodeId(), + 'node_type' => (int)$this->getNodeType(), + 'permissions' => (int)$this->getPermissions(), + ]; + } + public function jsonSerialize(): array { return [ 'id' => $this->getId(), diff --git a/lib/Db/MenuItem.php b/lib/Db/MenuItem.php new file mode 100644 index 0000000000..3dd92ab78c --- /dev/null +++ b/lib/Db/MenuItem.php @@ -0,0 +1,107 @@ +addType('id', 'integer'); + $this->addType('contextId', 'integer'); + $this->addType('targetId', 'integer'); + $this->addType('order', 'integer'); + $this->addType('parentId', 'integer'); + $this->addType('broken', 'boolean'); + } + + /** + * Whether a technical name has the form of the automatic `item_`. Such names are + * reserved, so a given name never collides with the name a later item gets. + */ + public static function isDefaultName(string $technicalName): bool { + return preg_match('/^' . self::DEFAULT_NAME_PREFIX . '[0-9]+$/', $technicalName) === 1; + } + + /** + * @return array{id: int, contextId: int, label: string, icon: string|null, description: string|null, targetType: 'view'|'table'|'url'|'page', targetId: int|null, url: string|null, technicalName: string, order: int, parentId: int|null, broken: bool} + */ + public function jsonSerialize(): array { + return [ + 'id' => $this->getId(), + 'contextId' => $this->getContextId(), + 'label' => $this->getLabel() ?? '', + 'icon' => $this->getIcon(), + 'description' => $this->getDescription(), + 'targetType' => match ($this->targetType) { + self::TARGET_VIEW => self::TARGET_VIEW, + self::TARGET_TABLE => self::TARGET_TABLE, + self::TARGET_PAGE => self::TARGET_PAGE, + default => self::TARGET_URL, + }, + 'targetId' => $this->getTargetId(), + 'url' => $this->getUrl(), + 'technicalName' => $this->technicalName ?? '', + 'order' => $this->getOrder() ?? 0, + 'parentId' => $this->getParentId(), + 'broken' => $this->broken, + ]; + } +} diff --git a/lib/Db/MenuItemMapper.php b/lib/Db/MenuItemMapper.php new file mode 100644 index 0000000000..704e9c4722 --- /dev/null +++ b/lib/Db/MenuItemMapper.php @@ -0,0 +1,112 @@ + */ +class MenuItemMapper extends QBMapper { + use TTransactional; + + private const DB_CHUNK_SIZE = 1_000; + + protected string $table = 'tables_contexts_menu_item'; + + public function __construct(IDBConnection $db) { + parent::__construct($db, $this->table, MenuItem::class); + } + + /** + * Inserts a menu item under the default technical name `item_`. + * + * @throws Exception + */ + public function insertWithDefaultName(MenuItem $menuItem): MenuItem { + return $this->atomic(function () use ($menuItem): MenuItem { + $menuItem = $this->insert($menuItem); + $menuItem->setTechnicalName(MenuItem::DEFAULT_NAME_PREFIX . $menuItem->getId()); + return $this->update($menuItem); + }, $this->db); + } + + /** + * @return MenuItem[] ordered by their position in the menu + * @throws Exception + */ + public function findByContextId(int $contextId): array { + return $this->findByContextIds([$contextId])[$contextId] ?? []; + } + + /** + * The menu items of the given contexts, each list ordered by position. An item whose + * view or table no longer exists, or whose page is gone or belongs to another context, + * is marked broken. + * + * @param int[] $contextIds + * @return array> keyed by context id + * @throws Exception + */ + public function findByContextIds(array $contextIds): array { + if ($contextIds === []) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $brokenExpression = sprintf( + 'CASE WHEN (m.target_type = %s AND v.id IS NULL) OR (m.target_type = %s AND t.id IS NULL) OR (m.target_type = %s AND p.id IS NULL) THEN 1 ELSE 0 END', + $qb->createNamedParameter(MenuItem::TARGET_VIEW), + $qb->createNamedParameter(MenuItem::TARGET_TABLE), + $qb->createNamedParameter(MenuItem::TARGET_PAGE), + ); + $qb->select('m.*') + ->selectAlias($qb->createFunction($brokenExpression), 'broken') + ->from($this->table, 'm') + ->leftJoin('m', 'tables_views', 'v', $qb->expr()->andX( + $qb->expr()->eq('v.id', 'm.target_id'), + $qb->expr()->eq('m.target_type', $qb->createNamedParameter(MenuItem::TARGET_VIEW)), + )) + ->leftJoin('m', 'tables_tables', 't', $qb->expr()->andX( + $qb->expr()->eq('t.id', 'm.target_id'), + $qb->expr()->eq('m.target_type', $qb->createNamedParameter(MenuItem::TARGET_TABLE)), + )) + ->leftJoin('m', 'tables_contexts_page', 'p', $qb->expr()->andX( + $qb->expr()->eq('p.id', 'm.target_id'), + $qb->expr()->eq('m.target_type', $qb->createNamedParameter(MenuItem::TARGET_PAGE)), + $qb->expr()->eq('p.context_id', 'm.context_id'), + )) + ->where($qb->expr()->in('m.context_id', $qb->createParameter('contextIds'))) + ->orderBy('m.context_id', 'ASC') + ->addOrderBy('m.order', 'ASC') + ->addOrderBy('m.id', 'ASC'); + + $byContext = []; + foreach (array_chunk(array_values(array_unique($contextIds)), self::DB_CHUNK_SIZE) as $chunk) { + $qb->setParameter('contextIds', $chunk, IQueryBuilder::PARAM_INT_ARRAY); + foreach ($this->findEntities($qb) as $menuItem) { + $byContext[$menuItem->getContextId()][] = $menuItem; + } + } + return $byContext; + } + + /** + * @throws Exception + */ + public function deleteAllByContextId(int $contextId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->table) + ->where($qb->expr()->eq('context_id', $qb->createNamedParameter($contextId, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } +} diff --git a/lib/Db/PageContent.php b/lib/Db/PageContent.php index 966f48fc53..518aacfcce 100644 --- a/lib/Db/PageContent.php +++ b/lib/Db/PageContent.php @@ -30,6 +30,19 @@ public function __construct() { $this->addType('order', 'integer'); } + /** + * The content as a page of an application carries it, in the shape the context + * mapper reads it. + * + * @return array{order: int, node_rel_id: int} + */ + public function toPageContent(): array { + return [ + 'order' => (int)$this->getOrder(), + 'node_rel_id' => (int)$this->getNodeRelId(), + ]; + } + public function jsonSerialize(): array { return [ 'id' => $this->getId(), diff --git a/lib/Migration/Version2500Date20261022010000.php b/lib/Migration/Version2500Date20261022010000.php new file mode 100644 index 0000000000..ff118dc93a --- /dev/null +++ b/lib/Migration/Version2500Date20261022010000.php @@ -0,0 +1,53 @@ +hasTable(self::TABLE)) { + return null; + } + + $table = $schema->createTable(self::TABLE); + $table->addColumn('id', Types::INTEGER, ['autoincrement' => true, 'notnull' => true]); + $table->addColumn('context_id', Types::INTEGER, ['notnull' => true]); + $table->addColumn('label', Types::STRING, ['notnull' => true, 'length' => 200]); + $table->addColumn('icon', Types::STRING, ['notnull' => false, 'length' => 64]); + $table->addColumn('description', Types::TEXT, ['notnull' => false]); + $table->addColumn('target_type', Types::STRING, ['notnull' => true, 'length' => 16]); + $table->addColumn('target_id', Types::INTEGER, ['notnull' => false]); + $table->addColumn('url', Types::STRING, ['notnull' => false, 'length' => 2000]); + // Nullable so an item can be inserted first and named `item_` after. + $table->addColumn('technical_name', Types::STRING, ['notnull' => false, 'length' => 200]); + $table->addColumn('order', Types::INTEGER, ['notnull' => true, 'default' => 0]); + $table->addColumn('parent_id', Types::INTEGER, ['notnull' => false]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['context_id', 'technical_name'], 'tables_ctx_menu_name_uq'); + + return $schema; + } +} diff --git a/lib/Model/MenuItemInput.php b/lib/Model/MenuItemInput.php new file mode 100644 index 0000000000..21a4c00719 --- /dev/null +++ b/lib/Model/MenuItemInput.php @@ -0,0 +1,178 @@ +label) === '' || mb_strlen($this->label) > self::MAX_LABEL_LENGTH) { + throw new InvalidArgumentException('A menu item needs a label of at most ' . self::MAX_LABEL_LENGTH . ' characters.'); + } + if ($this->icon !== null && !IconHelper::isValid($this->icon)) { + throw new InvalidArgumentException('A menu item has an unknown icon: ' . $this->icon); + } + if ($this->description !== null && mb_strlen($this->description) > self::MAX_DESCRIPTION_LENGTH) { + throw new InvalidArgumentException('A menu item description has at most ' . self::MAX_DESCRIPTION_LENGTH . ' characters.'); + } + if (!in_array($this->targetType, MenuItem::TARGET_TYPES, true)) { + throw new InvalidArgumentException('Unexpected menu item target type, expected one of: ' . implode(', ', MenuItem::TARGET_TYPES) . '.'); + } + if ($this->targetType === MenuItem::TARGET_URL) { + self::assertUrlValid($this->url); + } elseif (!$this->pointsAtNewStartPage() && ($this->targetId === null || $this->targetId <= 0)) { + throw new InvalidArgumentException('A menu item pointing at a ' . $this->targetType . ' needs its id.'); + } + if ($this->technicalName !== null && !TechnicalNameValidator::isValid($this->technicalName)) { + throw new InvalidArgumentException('A technical name has 1 to ' . TechnicalNameValidator::MAX_LENGTH . ' lower case letters, digits and underscores and starts with a letter.'); + } + } + + /** + * @param bool $allowNewStartPage accept a page item without an id, meaning the start page + * of an application that is being created + * @throws InvalidArgumentException + */ + public static function createFromInputArray(mixed $data, bool $allowNewStartPage = false): self { + if (!is_array($data)) { + throw new InvalidArgumentException('A menu item must be an object.'); + } + if (!is_string($data['label'] ?? null)) { + throw new InvalidArgumentException('A menu item needs a label.'); + } + $targetType = $data['targetType'] ?? MenuItem::TARGET_URL; + if (!is_string($targetType)) { + throw new InvalidArgumentException('The target type of a menu item must be a string.'); + } + $isUrl = $targetType === MenuItem::TARGET_URL; + $targetId = $data['targetId'] ?? null; + $isNewStartPage = $allowNewStartPage && $targetType === MenuItem::TARGET_PAGE && $targetId === null; + if (!$isUrl && !$isNewStartPage && !is_int($targetId)) { + throw new InvalidArgumentException('A menu item pointing at a ' . $targetType . ' needs its id.'); + } + foreach (['icon', 'description', 'url', 'technicalName'] as $optional) { + if (isset($data[$optional]) && !is_string($data[$optional])) { + throw new InvalidArgumentException('The menu item property "' . $optional . '" must be a string.'); + } + } + + return new self( + label: trim($data['label']), + icon: self::emptyToNull($data['icon'] ?? null), + description: self::emptyToNull($data['description'] ?? null), + targetType: $targetType, + targetId: $isUrl ? null : $targetId, + url: $isUrl ? trim((string)($data['url'] ?? '')) : null, + technicalName: self::emptyToNull($data['technicalName'] ?? null), + allowNewStartPage: $allowNewStartPage, + ); + } + + /** + * @param bool $allowNewStartPage see createFromInputArray() + * @return list + * @throws InvalidArgumentException + */ + public static function createListFromInputArray(mixed $data, bool $allowNewStartPage = false): array { + if (!is_array($data) || !array_is_list($data)) { + throw new InvalidArgumentException('The menu items must be a list.'); + } + return array_map(static fn (mixed $item): self => self::createFromInputArray($item, $allowNewStartPage), $data); + } + + /** + * True for a page item without an id on the menu of an application that is being created. + */ + public function pointsAtNewStartPage(): bool { + return $this->allowNewStartPage && $this->targetType === MenuItem::TARGET_PAGE && $this->targetId === null; + } + + /** + * This item pointing at the given page. + * + * @throws InvalidArgumentException + */ + public function withPageId(int $pageId): self { + return new self($this->label, $this->icon, $this->description, MenuItem::TARGET_PAGE, $pageId, null, $this->technicalName); + } + + /** + * @throws InvalidArgumentException + */ + private static function assertUrlValid(?string $url): void { + $scheme = $url === null ? null : parse_url($url, PHP_URL_SCHEME); + // parse_url accepts spaces and control characters anywhere in a link + if ($url === null || mb_strlen($url) > self::MAX_URL_LENGTH || preg_match('/[\s\p{Z}\p{Cc}]/u', $url) !== 0 + || !is_string($scheme) || !in_array(strtolower($scheme), self::URL_SCHEMES, true) + || parse_url($url, PHP_URL_HOST) === null) { + throw new InvalidArgumentException('A menu item link must be an http or https address of at most ' . self::MAX_URL_LENGTH . ' characters.'); + } + } + + private static function emptyToNull(?string $value): ?string { + if ($value === null) { + return null; + } + $value = trim($value); + return $value === '' ? null : $value; + } + + public function getLabel(): string { + return $this->label; + } + + public function getIcon(): ?string { + return $this->icon; + } + + public function getDescription(): ?string { + return $this->description; + } + + public function getTargetType(): string { + return $this->targetType; + } + + public function getTargetId(): ?int { + return $this->targetId; + } + + public function getUrl(): ?string { + return $this->url; + } + + public function getTechnicalName(): ?string { + return $this->technicalName; + } +} diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index d750e9a969..82e5cb5eb7 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -234,6 +234,21 @@ * errors_count: int, * } * + * @psalm-type TablesMenuItem = array{ + * id: int, + * contextId: int, + * label: string, + * icon: string|null, + * description: string|null, + * targetType: 'view'|'table'|'url'|'page', + * targetId: int|null, + * url: string|null, + * technicalName: string, + * order: int, + * parentId: int|null, + * broken: bool, + * } + * * @psalm-type TablesContext = array{ * id: int, * name: string, @@ -241,6 +256,7 @@ * description: string, * owner: string, * ownerType: int, + * menuItems?: list, * } * * @psalm-type TablesWidgetValue = string|int|float|bool|null diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index ed01048c24..e4f6a8b4e6 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -15,6 +15,8 @@ use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItem; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; @@ -30,6 +32,7 @@ use OCA\Tables\Model\ColumnSettings; use OCA\Tables\Model\ContextScheme; use OCA\Tables\Model\FilterSet; +use OCA\Tables\Model\MenuItemInput; use OCA\Tables\Model\SortRuleSet; use OCA\Tables\Model\ViewSettings; use OCA\Tables\Service\Support\AuditLogServiceInterface; @@ -39,6 +42,7 @@ use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception; use OCP\IDBConnection; +use OCP\IL10N; use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IUserManager; @@ -66,6 +70,8 @@ public function __construct( private ViewMapper $viewMapper, private WidgetService $widgetService, private GridWidgetTypes $gridWidgetTypes, + private MenuItemMapper $menuItemMapper, + private IL10N $l10n, ) { } @@ -85,7 +91,11 @@ public function findAll(?string $userId): array { $this->logger->warning($error); throw new InternalError($error); } - return $this->contextMapper->findAll($userId); + $contexts = $this->contextMapper->findAll($userId); + foreach ($contexts as $context) { + $this->markMenuForReader($context, $userId); + } + return $contexts; } public function findForNavigation(string $userId): array { @@ -130,14 +140,58 @@ public function findById(int $id, ?string $userId): Context { throw new InternalError($error); } - return $this->contextMapper->findById($id, $userId); + $context = $this->contextMapper->findById($id, $userId); + $this->markMenuForReader($context, $userId); + return $context; } /** + * Marks a menu item as broken for a reader who cannot open its table or view, the + * way an item whose target is gone is broken for everyone. A table or view of the + * application itself is open to everyone the application is shared with. + */ + private function markMenuForReader(Context $context, ?string $userId): void { + $menuItems = $context->getMenuItems(); + if ($userId === null || !is_array($menuItems)) { + return; + } + $readableNodes = []; + foreach ($context->getNodes() ?? [] as $node) { + if (((int)$node['permissions'] & Application::PERMISSION_READ) !== 0) { + $readableNodes[sprintf('t%di%d', $node['node_type'], $node['node_id'])] = true; + } + } + $changed = false; + foreach ($menuItems as $index => $item) { + $nodeType = match ($item['targetType'] ?? null) { + MenuItem::TARGET_VIEW => Application::NODE_TYPE_VIEW, + MenuItem::TARGET_TABLE => Application::NODE_TYPE_TABLE, + default => null, + }; + $nodeId = $item['targetId'] ?? null; + if ($nodeType === null || !is_int($nodeId) || ($item['broken'] ?? false) || isset($readableNodes[sprintf('t%di%d', $nodeType, $nodeId)])) { + continue; + } + if (!$this->permissionsService->canAccessNodeById($nodeType, $nodeId, $userId)) { + $menuItems[$index]['broken'] = true; + $changed = true; + } + } + if ($changed) { + $context->setMenuItems($menuItems); + } + } + + /** + * Creates an application with its start page. Without a menu it gets a "Dashboard" + * item opening the start page; a given menu, even an empty one, is stored as it is, + * where a page item without an id opens the start page. + * * @psalm-param list $nodes + * @param list|null $menuItems * @throws Exception|PermissionError|InvalidArgumentException */ - public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType): Context { + public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType, ?array $menuItems = null): Context { $context = new Context(); $context->setName(trim($name)); $context->setIcon(trim($iconName)); @@ -145,7 +199,7 @@ public function create(string $name, string $iconName, string $description, arra $context->setOwnerId($ownerId); $context->setOwnerType($ownerType); - $this->atomic(function () use ($context, $nodes): void { + $this->atomic(function () use ($context, $nodes, $menuItems, $ownerId): void { $this->contextMapper->insert($context); if (!empty($nodes)) { @@ -153,6 +207,14 @@ public function create(string $name, string $iconName, string $description, arra $this->insertNodesFromArray($context, $nodes); } $this->insertPage($context); + $startPageId = self::startPageId($context->getPages() ?? []); + $menu = $menuItems ?? []; + if ($startPageId !== null) { + $menu = $menuItems === null + ? [$this->dashboardItem($startPageId)] + : array_map(static fn (MenuItemInput $item): MenuItemInput => $item->pointsAtNewStartPage() ? $item->withPageId($startPageId) : $item, $menuItems); + } + $this->replaceMenu($context, $menu, $ownerId); }, $this->dbc); $this->auditLogService->log(Operation::ContextCreated, 'Tables application with ID %s was created for user %s', [ 'contextId' => $context->getId(), @@ -160,6 +222,313 @@ public function create(string $name, string $iconName, string $description, arra 'nodes' => array_map(static fn (array $node): array => ['nodeType' => (int)$node['type'], 'nodeId' => (int)$node['id']], $nodes), ]); + $this->shapeAddedForResponse($context, [], [], true); + return $context; + } + + /** + * Puts the nodes and start page contents a save added into the shape the application + * endpoints have always returned for them: a node as its relation and a content entry + * as its row. While saving, every lookup uses the shape of the context mapper. + * + * @param array $knownNodeIds relation ids the application had before the save + * @param array $knownContentIds content ids the application had before the save + * @param bool $isNew a new application lists its nodes and leaves the page id out of its contents + */ + private function shapeAddedForResponse(Context $context, array $knownNodeIds, array $knownContentIds, bool $isNew): void { + $nodes = $context->getNodes(); + if (is_array($nodes)) { + foreach ($nodes as $relId => $node) { + if (!isset($knownNodeIds[$relId]) && isset($node['node_id'])) { + $nodes[$relId] = [ + 'id' => $node['id'], + 'contextId' => $context->getId(), + 'nodeId' => $node['node_id'], + 'nodeType' => $node['node_type'], + 'permissions' => $node['permissions'], + ]; + } + } + $context->setNodes($isNew ? array_values($nodes) : $nodes); + } + $pages = $context->getPages(); + if (is_array($pages)) { + foreach ($pages as $pageId => $page) { + foreach ($page['content'] ?? [] as $contentId => $content) { + if (isset($knownContentIds[$contentId]) || !isset($content['node_rel_id'])) { + continue; + } + $pages[$pageId]['content'][$contentId] = $isNew + ? ['id' => $contentId, 'nodeRelId' => $content['node_rel_id'], 'order' => $content['order']] + : ['id' => $contentId, 'pageId' => $pageId, 'nodeRelId' => $content['node_rel_id'], 'order' => $content['order']]; + } + } + $context->setPages($pages); + } + } + + /** + * The "Dashboard" item opening the start page, the menu of a new application created + * without one. + */ + private function dashboardItem(int $startPageId): MenuItemInput { + return new MenuItemInput( + label: $this->l10n->t('Dashboard'), + icon: null, + description: null, + targetType: MenuItem::TARGET_PAGE, + targetId: $startPageId, + url: null, + technicalName: Page::START_PAGE_NAME, + ); + } + + /** + * @param array $pages + */ + private static function startPageId(array $pages): ?int { + foreach ($pages as $pageId => $page) { + if (is_array($page) && ($page['page_type'] ?? $page['pageType'] ?? null) === Page::TYPE_STARTPAGE) { + return (int)$pageId; + } + } + return null; + } + + /** + * Replaces the menu of an application and adds the views and tables it opens as nodes, + * so everyone the application is shared with can open them. Removing an item leaves + * its view or table in the application. A page item must open a page of this + * application; a stored one whose page is gone is kept and read as broken. + * + * @param list $menuItems + * @throws Exception|PermissionError|InvalidArgumentException + */ + protected function replaceMenu(Context $context, array $menuItems, ?string $userId): void { + if (count($menuItems) > MenuItem::MAX_ITEMS_PER_APPLICATION) { + throw new InvalidArgumentException('An application menu holds at most ' . MenuItem::MAX_ITEMS_PER_APPLICATION . ' items.'); + } + $storedMenu = array_values(array_filter($context->getMenuItems() ?? [], is_array(...))); + $pageIds = array_map(intval(...), array_keys($context->getPages() ?? [])); + foreach ($storedMenu as $storedItem) { + if (($storedItem['targetType'] ?? null) === MenuItem::TARGET_PAGE && is_int($storedItem['targetId'] ?? null)) { + $pageIds[] = $storedItem['targetId']; + } + } + foreach ($menuItems as $item) { + if ($item->getTargetType() === MenuItem::TARGET_PAGE && !in_array($item->getTargetId(), $pageIds, true)) { + throw new InvalidArgumentException('A menu item can only open a page of its own application.'); + } + } + + $keptNames = $this->keptTechnicalNames($menuItems, $storedMenu); + $this->addMenuTargetsAsNodes($context, $menuItems, $storedMenu, $userId); + + $this->menuItemMapper->deleteAllByContextId($context->getId()); + $this->insertMenuItems($context->getId(), $menuItems, $keptNames); + + $context->setMenuItems(array_map( + static fn (MenuItem $menuItem): array => $menuItem->jsonSerialize(), + $this->menuItemMapper->findByContextId($context->getId()), + )); + } + + /** + * Inserts the items of a menu in their order. An item with a kept technical name is + * saved under it and refused when another save took it; every other item is named + * `item_`. + * + * @param list $menuItems + * @param array $keptNames technical names by position + * @throws Exception|InvalidArgumentException + */ + private function insertMenuItems(int $contextId, array $menuItems, array $keptNames): void { + foreach ($menuItems as $position => $item) { + $menuItem = new MenuItem(); + $menuItem->setContextId($contextId); + $menuItem->setLabel($item->getLabel()); + $menuItem->setIcon($item->getIcon()); + $menuItem->setDescription($item->getDescription()); + $menuItem->setTargetType($item->getTargetType()); + $menuItem->setTargetId($item->getTargetId()); + $menuItem->setUrl($item->getUrl()); + $menuItem->setOrder(($position + 1) * 10); + + if (!isset($keptNames[$position])) { + $this->menuItemMapper->insertWithDefaultName($menuItem); + continue; + } + $menuItem->setTechnicalName($keptNames[$position]); + try { + $this->menuItemMapper->insert($menuItem); + } catch (Exception $e) { + if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + throw new InvalidArgumentException('The technical name ' . $keptNames[$position] . ' is already used by a menu item of this application.'); + } + } + } + + /** + * The technical names items keep, by position: the one given, else the stored name + * of an item with the same target, so renaming or moving an item keeps it. + * + * @param list $menuItems + * @param list> $storedMenu + * @return array + * @throws InvalidArgumentException when two items are given the same name or a given + * name has the automatic form and is not the item's stored one + */ + private function keptTechnicalNames(array $menuItems, array $storedMenu): array { + $storedNames = []; + foreach ($storedMenu as $storedItem) { + $targetType = $storedItem['targetType'] ?? null; + $technicalName = $storedItem['technicalName'] ?? null; + if (!is_string($targetType) || !is_string($technicalName) || $technicalName === '') { + continue; + } + $targetId = is_int($storedItem['targetId'] ?? null) ? $storedItem['targetId'] : null; + $url = is_string($storedItem['url'] ?? null) ? $storedItem['url'] : null; + $storedNames[self::menuTargetIdentity($targetType, $targetId, $url)][] = $technicalName; + } + + $names = []; + $taken = []; + foreach ($menuItems as $position => $item) { + $technicalName = $item->getTechnicalName(); + if ($technicalName === null) { + continue; + } + if (isset($taken[$technicalName])) { + throw new InvalidArgumentException('Two menu items share the technical name "' . $technicalName . '".'); + } + $identity = self::menuTargetIdentity($item->getTargetType(), $item->getTargetId(), $item->getUrl()); + if (MenuItem::isDefaultName($technicalName) && !in_array($technicalName, $storedNames[$identity] ?? [], true)) { + throw new InvalidArgumentException('The technical name ' . $technicalName . ' is reserved for automatic names.'); + } + $names[$position] = $technicalName; + $taken[$technicalName] = true; + } + + foreach ($menuItems as $position => $item) { + if (isset($names[$position])) { + continue; + } + foreach ($storedNames[self::menuTargetIdentity($item->getTargetType(), $item->getTargetId(), $item->getUrl())] ?? [] as $storedName) { + if (!isset($taken[$storedName])) { + $names[$position] = $storedName; + $taken[$storedName] = true; + break; + } + } + } + return $names; + } + + private static function menuTargetIdentity(string $targetType, ?int $targetId, ?string $url): string { + return $targetType . ':' . ($targetType === MenuItem::TARGET_URL ? (string)$url : (string)$targetId); + } + + /** + * The node a menu target is stored as, keyed so nodes and targets can be compared. + * + * @return array{string, array{int, int}}|null + */ + private static function menuTargetNode(mixed $targetType, mixed $targetId): ?array { + $nodeType = match ($targetType) { + MenuItem::TARGET_VIEW => Application::NODE_TYPE_VIEW, + MenuItem::TARGET_TABLE => Application::NODE_TYPE_TABLE, + default => null, + }; + if ($nodeType === null || !is_int($targetId)) { + return null; + } + return [sprintf('t%di%d', $nodeType, $targetId), [$nodeType, $targetId]]; + } + + /** + * Adds the views and tables a menu opens to the application and its start page. A + * new target must exist and be manageable by the user, and every unusable one gets + * the same answer, so a save reveals nothing about the views and tables of others. A + * target that was in the stored menu before and is gone is kept. + * + * @param list $menuItems + * @param list> $storedMenu + * @throws Exception|PermissionError + */ + private function addMenuTargetsAsNodes(Context $context, array $menuItems, array $storedMenu, ?string $userId): void { + $missing = []; + foreach ($menuItems as $item) { + $target = self::menuTargetNode($item->getTargetType(), $item->getTargetId()); + if ($target !== null) { + $missing[$target[0]] = $target[1]; + } + } + $storedTargets = []; + foreach ($storedMenu as $storedItem) { + $target = self::menuTargetNode($storedItem['targetType'] ?? null, $storedItem['targetId'] ?? null); + if ($target !== null) { + $storedTargets[$target[0]] = true; + } + } + $nodes = $context->getNodes() ?? []; + foreach ($nodes as $node) { + unset($missing[sprintf('t%di%d', $node['node_type'], $node['node_id'])]); + } + if ($missing === []) { + return; + } + + $targetIds = [Application::NODE_TYPE_VIEW => [], Application::NODE_TYPE_TABLE => []]; + foreach ($missing as [$nodeType, $nodeId]) { + $targetIds[$nodeType][] = $nodeId; + } + $existing = [ + Application::NODE_TYPE_VIEW => $this->viewMapper->findMany($targetIds[Application::NODE_TYPE_VIEW]), + Application::NODE_TYPE_TABLE => $this->tableMapper->findMany($targetIds[Application::NODE_TYPE_TABLE]), + ]; + + $pages = $context->getPages() ?? []; + $startPageId = self::startPageId($pages); + foreach ($missing as $key => [$nodeType, $nodeId]) { + $exists = isset($existing[$nodeType][$nodeId]); + if (!$exists && isset($storedTargets[$key])) { + continue; + } + if (!$exists || !$this->permissionsService->canManageNodeById($nodeType, $nodeId, $userId)) { + throw new PermissionError(sprintf('Owner cannot manage node %d (type %d)', $nodeId, $nodeType)); + } + $relation = $this->addNodeToContext($context, $nodeId, $nodeType, Application::PERMISSION_READ); + $nodes[$relation->getId()] = $relation->toContextNode(); + if ($startPageId !== null) { + $content = $this->addNodeRelToPage($relation, null, $startPageId); + $pages[$startPageId]['content'][$content->getId()] = $content->toPageContent(); + } + } + $context->setNodes($nodes); + $context->setPages($pages); + } + + /** + * @psalm-param list $nodes + * @throws Exception + * @throws DoesNotExistException + * @throws PermissionError|MultipleObjectsReturnedException + * @param list|null $menuItems replaces the menu when given + * @throws InvalidArgumentException + */ + public function update(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems = null): Context { + // A transaction that reads before it writes can lose its write lock to a concurrent + // request (SQLite answers "database is locked" at once); running it again succeeds. + $changes = []; + $context = $this->atomicRetry( + function () use ($contextId, $userId, $name, $iconName, $description, $nodes, $menuItems, &$changes): Context { + return $this->applyUpdate($contextId, $userId, $name, $iconName, $description, $nodes, $menuItems, $changes); + }, + $this->dbc, + ); + $this->auditLogService->log(Operation::ContextUpdated, 'Tables application with ID %s was updated', ['contextId' => $contextId] + $changes); return $context; } @@ -168,9 +537,16 @@ public function create(string $name, string $iconName, string $description, arra * @throws Exception * @throws DoesNotExistException * @throws PermissionError|MultipleObjectsReturnedException + * @param list|null $menuItems + * @param array>> $changes set to the nodes added, removed and updated */ - public function update(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes): Context { + private function applyUpdate(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems, array &$changes): Context { $context = $this->contextMapper->findById($contextId, $userId); + $knownNodeIds = array_fill_keys(array_keys($context->getNodes() ?? []), true); + $knownContentIds = []; + foreach ($context->getPages() ?? [] as $page) { + $knownContentIds += array_fill_keys(array_keys($page['content'] ?? []), true); + } if ($name !== null) { $context->setName(trim($name)); @@ -263,8 +639,8 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $node['order'] ?? 100, $userId ); - $currentNodes[$addedNode->getId()] = $addedNode->jsonSerialize(); - $currentPages[$updatedContent->getPageId()]['content'][$updatedContent->getId()] = $updatedContent->jsonSerialize(); + $currentNodes[$addedNode->getId()] = $addedNode->toContextNode(); + $currentPages[$updatedContent->getPageId()]['content'][$updatedContent->getId()] = $updatedContent->toPageContent(); } unset($nodesBeingAdded); @@ -324,13 +700,16 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $context->setNodes($currentNodes); $context->setPages($currentPages); } - $this->auditLogService->log(Operation::ContextUpdated, 'Tables application with ID %s was updated', [ - 'contextId' => $contextId, + if ($menuItems !== null) { + $this->replaceMenu($context, $menuItems, $userId); + } + $changes = [ 'addedNodes' => $addedNodes, 'removedNodes' => $removedNodes, 'updatedNodes' => $updatedNodes, - ]); + ]; + $this->shapeAddedForResponse($context, $knownNodeIds, $knownContentIds, false); return $context; } @@ -341,8 +720,12 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i public function delete(int $contextId, string $userId): Context { $context = $this->contextMapper->findById($contextId, $userId); - $this->atomic(function () use ($context): void { + // Deleting the application first takes the write lock before anything is read, so on + // SQLite a concurrent write fails this transaction at once and the retry runs it again. + $this->atomicRetry(function () use ($context): void { + $this->contextMapper->delete($context); $this->shareService->deleteAllForContext($context); + $this->menuItemMapper->deleteAllByContextId($context->getId()); $this->contextNodeRelMapper->deleteAllByContextId($context->getId()); $pageIds = $this->pageMapper->getPageIdsForContext($context->getId()); $this->widgetService->deleteByPageIds($pageIds); @@ -350,7 +733,6 @@ public function delete(int $contextId, string $userId): Context { $this->pageContentMapper->deleteByPageId($pageId); $this->pageMapper->deleteByPageId($pageId); } - $this->contextMapper->delete($context); }, $this->dbc); $this->auditLogService->log(Operation::ContextDeleted, 'Tables application with ID %s was deleted', [ 'contextId' => $context->getId(), @@ -572,9 +954,7 @@ protected function insertPage(Context $context): void { $this->pageContentMapper->insert($pageContent); - $addedPage['content'][$pageContent->getId()] = $pageContent->jsonSerialize(); - // the content is already embedded in the page - unset($addedPage['content'][$pageContent->getId()]['pageId']); + $addedPage['content'][$pageContent->getId()] = $pageContent->toPageContent(); } } @@ -598,7 +978,7 @@ protected function insertNodesFromArray(Context $context, array $nodes): void { throw new PermissionError(sprintf('Owner cannot manage node %d (type %d)', $nodeId, $nodeType)); } $contextNodeRel = $this->addNodeToContext($context, $nodeId, $nodeType, $node['permissions'] ?? Application::PERMISSION_READ); - $addedNodes[] = $contextNodeRel->jsonSerialize(); + $addedNodes[$contextNodeRel->getId()] = $contextNodeRel->toContextNode(); } catch (Exception $e) { $this->logger->warning('Could not add node {ntype}/{nid} to context {cid}, skipping.', [ 'app' => Application::APP_ID, diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php index 1d5408c188..7bbbd7f250 100644 --- a/lib/Service/ShareService.php +++ b/lib/Service/ShareService.php @@ -865,18 +865,19 @@ public function deleteAllForView(View $view):void { } } + /** + * A database error is passed on, so a transaction around the call fails as a whole. + * + * @throws Exception + */ public function deleteAllForContext(Context $context): void { - try { - $this->atomic(function () use ($context): void { - $shares = $this->mapper->findAllSharesForNode('context', $context->getId(), $this->userId); - foreach ($shares as $share) { - $this->contextNavigationMapper->deleteByShareId($share->getId()); - } - $this->mapper->deleteByNode($context->getId(), 'context'); - }, $this->dbc); - } catch (Exception) { - $this->logger->error('something went wrong while deleting shares for context: ' . $context->getId()); - } + $this->atomic(function () use ($context): void { + $shares = $this->mapper->findAllSharesForNode('context', $context->getId(), $this->userId); + foreach ($shares as $share) { + $this->contextNavigationMapper->deleteByShareId($share->getId()); + } + $this->mapper->deleteByNode($context->getId(), 'context'); + }, $this->dbc); } /** diff --git a/openapi.json b/openapi.json index b0e4a2c6cd..3165a3b0a1 100644 --- a/openapi.json +++ b/openapi.json @@ -313,6 +313,12 @@ "ownerType": { "type": "integer", "format": "int64" + }, + "menuItems": { + "type": "array", + "items": { + "$ref": "#/components/schemas/MenuItem" + } } } }, @@ -420,6 +426,77 @@ } } }, + "MenuItem": { + "type": "object", + "required": [ + "id", + "contextId", + "label", + "icon", + "description", + "targetType", + "targetId", + "url", + "technicalName", + "order", + "parentId", + "broken" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64" + }, + "contextId": { + "type": "integer", + "format": "int64" + }, + "label": { + "type": "string" + }, + "icon": { + "type": "string", + "nullable": true + }, + "description": { + "type": "string", + "nullable": true + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url", + "page" + ] + }, + "targetId": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "url": { + "type": "string", + "nullable": true + }, + "technicalName": { + "type": "string" + }, + "order": { + "type": "integer", + "format": "int64" + }, + "parentId": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "broken": { + "type": "boolean" + } + } + }, "NotifyConfig": { "type": "object", "required": [ @@ -12515,6 +12592,48 @@ } } } + }, + "menuItems": { + "type": "array", + "nullable": true, + "default": null, + "description": "optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page", + "items": { + "type": "object", + "required": [ + "label" + ], + "properties": { + "label": { + "type": "string" + }, + "icon": { + "type": "string" + }, + "description": { + "type": "string" + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url", + "page" + ] + }, + "targetId": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + }, + "technicalName": { + "type": "string" + } + } + } } } } @@ -12947,6 +13066,48 @@ "format": "int64" } } + }, + "menuItems": { + "type": "array", + "nullable": true, + "default": null, + "description": "provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target", + "items": { + "type": "object", + "required": [ + "label" + ], + "properties": { + "label": { + "type": "string" + }, + "icon": { + "type": "string" + }, + "description": { + "type": "string" + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url", + "page" + ] + }, + "targetId": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + }, + "technicalName": { + "type": "string" + } + } + } } } } diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 301ff9a878..e092878ad8 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1245,6 +1245,7 @@ export type components = { readonly owner: string; /** Format: int64 */ readonly ownerType: number; + readonly menuItems?: readonly components["schemas"]["MenuItem"][]; }; readonly ContextNavigation: { /** Format: int64 */ @@ -1279,6 +1280,26 @@ export type components = { readonly shareToken: string; readonly url: string; }; + readonly MenuItem: { + /** Format: int64 */ + readonly id: number; + /** Format: int64 */ + readonly contextId: number; + readonly label: string; + readonly icon: string | null; + readonly description: string | null; + /** @enum {string} */ + readonly targetType: "view" | "table" | "url" | "page"; + /** Format: int64 */ + readonly targetId: number | null; + readonly url: string | null; + readonly technicalName: string; + /** Format: int64 */ + readonly order: number; + /** Format: int64 */ + readonly parentId: number | null; + readonly broken: boolean; + }; readonly NotifyConfig: { readonly "notify-assigned": boolean; readonly "notify-column": boolean; @@ -7207,6 +7228,21 @@ export interface operations { /** Format: int64 */ readonly permissions?: number; }[]; + /** + * @description optional menu of the context, in order; the views and tables it opens become nodes of the context and a page item without a targetId opens the start page; without a menu the context gets one item opening the start page + * @default null + */ + readonly menuItems?: readonly { + readonly label: string; + readonly icon?: string; + readonly description?: string; + /** @enum {string} */ + readonly targetType?: "view" | "table" | "url" | "page"; + /** Format: int64 */ + readonly targetId?: number; + readonly url?: string; + readonly technicalName?: string; + }[] | null; }; }; }; @@ -7397,6 +7433,21 @@ export interface operations { /** Format: int64 */ readonly order: number; } | null; + /** + * @description provide this parameter to replace the menu; the views and tables it opens become nodes of the context, and an item without a technicalName keeps the one stored for the same target + * @default null + */ + readonly menuItems?: readonly { + readonly label: string; + readonly icon?: string; + readonly description?: string; + /** @enum {string} */ + readonly targetType?: "view" | "table" | "url" | "page"; + /** Format: int64 */ + readonly targetId?: number; + readonly url?: string; + readonly technicalName?: string; + }[] | null; }; }; }; diff --git a/tests/unit/Controller/ContextControllerTest.php b/tests/unit/Controller/ContextControllerTest.php new file mode 100644 index 0000000000..e89f008a30 --- /dev/null +++ b/tests/unit/Controller/ContextControllerTest.php @@ -0,0 +1,121 @@ +contextService = $this->createMock(ContextService::class); + $this->controller = new ContextController( + $this->createMock(IRequest::class), + $this->createMock(LoggerInterface::class), + $this->createMock(IL10N::class), + 'user-1', + $this->contextService, + $this->createMock(TableService::class), + $this->createMock(IDBConnection::class), + $this->createMock(ColumnService::class), + $this->createMock(ViewService::class), + ); + } + + public function testCreatePassesAPageItemWithoutAnIdOnAsTheNewStartPage(): void { + $context = new Context(); + $context->setMenuItems([]); + $this->contextService->expects($this->once())->method('create') + ->with('Intake', 'star', '', [], 'user-1', 0, $this->callback(static fn (array $menuItems): bool => count($menuItems) === 1 + && $menuItems[0] instanceof MenuItemInput + && $menuItems[0]->pointsAtNewStartPage() + && $menuItems[0]->getLabel() === 'Home')) + ->willReturn($context); + + $response = $this->controller->create('Intake', 'star', '', [], [['label' => 'Home', 'targetType' => 'page']]); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + } + + public function testUpdateRefusesAPageItemWithoutAnId(): void { + $this->contextService->expects($this->never())->method('update'); + + $response = $this->controller->update(7, null, null, null, null, [['label' => 'Home', 'targetType' => 'page']]); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + $this->assertStringContainsString('needs its id', $response->getData()['message']); + } + + public function testCreateWithoutAMenuLeavesItToTheService(): void { + $context = new Context(); + $context->setMenuItems([]); + $this->contextService->expects($this->once())->method('create') + ->with('Intake', 'star', '', [], 'user-1', 0, null) + ->willReturn($context); + + $this->assertSame(Http::STATUS_OK, $this->controller->create('Intake', 'star')->getStatus()); + } + + public static function invalidMenus(): array { + return [ + 'not a list' => [['home' => ['label' => 'Home', 'url' => 'https://example.org']], 'must be a list'], + 'javascript link' => [[['label' => 'X', 'url' => 'javascript:alert(1)']], 'http or https'], + 'unknown icon' => [[['label' => 'X', 'url' => 'https://example.org', 'icon' => 'no-such-icon']], 'unknown icon'], + ]; + } + + /** + * @dataProvider invalidMenus + */ + public function testAnInvalidMenuIsABadRequest(array $menuItems, string $message): void { + $this->contextService->expects($this->never())->method('create'); + $this->contextService->expects($this->never())->method('update'); + + foreach ([$this->controller->create('Intake', 'star', '', [], $menuItems), $this->controller->update(7, null, null, null, null, $menuItems)] as $response) { + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + $this->assertStringContainsString($message, $response->getData()['message']); + } + } + + public function testUpdatePassesTheMenu(): void { + $context = new Context(); + $context->setMenuItems([]); + $this->contextService->expects($this->once())->method('update') + ->with(7, 'user-1', null, null, null, null, $this->callback(static fn (array $menuItems): bool => count($menuItems) === 1 && $menuItems[0]->getTargetId() === 5)) + ->willReturn($context); + + $response = $this->controller->update(7, null, null, null, null, [['label' => 'Overview', 'targetType' => 'page', 'targetId' => 5]]); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + } + + public function testATakenTechnicalNameIsABadRequest(): void { + $this->contextService->method('update')->willThrowException(new \InvalidArgumentException('The technical name docs is already used by a menu item of this application.')); + + $response = $this->controller->update(7, null, null, null, null, [['label' => 'Docs', 'url' => 'https://example.org', 'technicalName' => 'docs']]); + + $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); + } +} diff --git a/tests/unit/Db/ContextMapperTest.php b/tests/unit/Db/ContextMapperTest.php index 4ac495ad35..4f524d7553 100644 --- a/tests/unit/Db/ContextMapperTest.php +++ b/tests/unit/Db/ContextMapperTest.php @@ -14,6 +14,8 @@ use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItem; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; @@ -31,6 +33,7 @@ class ContextMapperTest extends DatabaseTestCase { private ContextMapper $mapper; private ContextNodeRelationMapper $nodeRelMapper; private PageMapper $pageMapper; + private MenuItemMapper $menuItemMapper; private PageContentMapper $pageContentMapper; /** @var Context[] */ private array $contexts = []; @@ -39,7 +42,8 @@ protected function setUp(): void { parent::setUp(); $userHelper = $this->createMock(UserHelper::class); $userHelper->method('getGroupIdsForUser')->willReturn([]); - $this->mapper = new ContextMapper($this->connectionAdapter, $userHelper, $this->createMock(GroupHelper::class)); + $this->menuItemMapper = new MenuItemMapper($this->connectionAdapter); + $this->mapper = new ContextMapper($this->connectionAdapter, $userHelper, $this->createMock(GroupHelper::class), $this->menuItemMapper); $this->nodeRelMapper = new ContextNodeRelationMapper($this->connectionAdapter); $this->pageMapper = new PageMapper($this->connectionAdapter); $this->pageContentMapper = new PageContentMapper($this->connectionAdapter); @@ -52,6 +56,7 @@ protected function tearDown(): void { $this->pageMapper->deleteByPageId($pageId); } $this->nodeRelMapper->deleteAllByContextId($context->getId()); + $this->menuItemMapper->deleteAllByContextId($context->getId()); $this->mapper->delete($context); } parent::tearDown(); @@ -86,6 +91,28 @@ public function testGridPageHasItsFieldsAndNoStartPageContent(): void { $this->assertSame([$first->getId(), $second->getId()], array_keys($this->mapper->findById($context->getId(), self::OWNER)->getNodes())); } + public function testTheApplicationsContainingANodeAreFoundWithoutTheirMenus(): void { + $context = $this->createContext(); + $this->addNode($context, 11); + $other = $this->createContext(); + $this->addNode($other, 12); + $menuItem = new MenuItem(); + $menuItem->setContextId($context->getId()); + $menuItem->setLabel('Home'); + $menuItem->setTargetType(MenuItem::TARGET_URL); + $menuItem->setUrl('https://example.com'); + $menuItem->setTechnicalName('home'); + $this->menuItemMapper->insert($menuItem); + + $found = $this->mapper->findAllContainingNode(Application::NODE_TYPE_TABLE, 11, self::OWNER); + + $this->assertSame([$context->getId()], array_map(static fn (Context $context): int => $context->getId(), $found)); + $this->assertSame([11], array_column($found[0]->getNodes(), 'node_id')); + $this->assertNull($found[0]->getMenuItems()); + $this->assertArrayNotHasKey('menuItems', $found[0]->jsonSerialize()); + $this->assertSame(['home'], array_column($this->mapper->findById($context->getId(), self::OWNER)->getMenuItems(), 'technicalName')); + } + private function createContext(): Context { $context = new Context(); $context->setName('Mapper test'); diff --git a/tests/unit/Db/ContextTest.php b/tests/unit/Db/ContextTest.php new file mode 100644 index 0000000000..3b6b44af4d --- /dev/null +++ b/tests/unit/Db/ContextTest.php @@ -0,0 +1,28 @@ +assertArrayNotHasKey('menuItems', (new Context())->jsonSerialize()); + } + + public function testALoadedMenuIsCarriedEvenWhenEmpty(): void { + $context = new Context(); + $context->setMenuItems([]); + $this->assertSame([], $context->jsonSerialize()['menuItems']); + + $context->setMenuItems([3 => ['id' => 3]]); + $this->assertSame([['id' => 3]], $context->jsonSerialize()['menuItems']); + } +} diff --git a/tests/unit/Db/MenuItemMapperTest.php b/tests/unit/Db/MenuItemMapperTest.php new file mode 100644 index 0000000000..f8286eab77 --- /dev/null +++ b/tests/unit/Db/MenuItemMapperTest.php @@ -0,0 +1,227 @@ +cleanupTablesData(); + $this->menuItemMapper = new MenuItemMapper($this->connectionAdapter); + $this->contextMapper = new ContextMapper( + $this->connectionAdapter, + $this->createMock(UserHelper::class), + $this->createMock(GroupHelper::class), + $this->menuItemMapper, + ); + } + + public function testItemsWhoseViewOrTableIsGoneAreBroken(): void { + $contextId = $this->createContext('Intake'); + $tableId = $this->createTestTable()['id']; + $viewId = $this->createView($tableId); + $this->createMenuItem($contextId, 'live_view', MenuItem::TARGET_VIEW, $viewId); + $this->createMenuItem($contextId, 'gone_view', MenuItem::TARGET_VIEW, $viewId + 100); + $this->createMenuItem($contextId, 'live_table', MenuItem::TARGET_TABLE, $tableId); + $this->createMenuItem($contextId, 'gone_table', MenuItem::TARGET_TABLE, $tableId + 100); + // A link points at neither, so its target id never makes it broken + $this->createMenuItem($contextId, 'link', MenuItem::TARGET_URL, $viewId + 100); + + $broken = []; + foreach ($this->menuItemMapper->findByContextId($contextId) as $menuItem) { + $broken[$menuItem->getTechnicalName()] = $menuItem->isBroken(); + } + + $this->assertSame([ + 'live_view' => false, + 'gone_view' => true, + 'live_table' => false, + 'gone_table' => true, + 'link' => false, + ], $broken); + } + + public function testItemsWhosePageIsGoneOrForeignAreBroken(): void { + $contextId = $this->createContext('Intake'); + $otherContextId = $this->createContext('Reports'); + $pageId = $this->createPage($contextId); + $foreignPageId = $this->createPage($otherContextId); + $this->createMenuItem($contextId, 'live_page', MenuItem::TARGET_PAGE, $pageId); + $this->createMenuItem($contextId, 'gone_page', MenuItem::TARGET_PAGE, $foreignPageId + 100); + $this->createMenuItem($contextId, 'foreign_page', MenuItem::TARGET_PAGE, $foreignPageId); + // A view with the id of a live page is still judged by the views table + $this->createMenuItem($contextId, 'view_with_page_id', MenuItem::TARGET_VIEW, $pageId); + + $broken = []; + foreach ($this->menuItemMapper->findByContextId($contextId) as $menuItem) { + $broken[$menuItem->getTechnicalName()] = $menuItem->isBroken(); + } + + $this->assertSame([ + 'live_page' => false, + 'gone_page' => true, + 'foreign_page' => true, + 'view_with_page_id' => true, + ], $broken); + } + + public function testItemsAreOrderedAndGroupedPerApplication(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'second', MenuItem::TARGET_URL, null, 20); + $this->createMenuItem($reportsId, 'overview', MenuItem::TARGET_URL, null, 0); + $this->createMenuItem($intakeId, 'first', MenuItem::TARGET_URL, null, 10); + + $menus = $this->menuItemMapper->findByContextIds([$intakeId, $reportsId]); + + $this->assertSame([ + $intakeId => ['first', 'second'], + $reportsId => ['overview'], + ], array_map(self::technicalNames(...), $menus)); + $this->assertSame([], $this->menuItemMapper->findByContextIds([])); + } + + public function testFindByIdIncludesTheMenu(): void { + $contextId = $this->createContext('Intake'); + $this->createMenuItem($contextId, 'home', MenuItem::TARGET_URL, null); + + $menuItems = $this->contextMapper->findById($contextId)->getMenuItems(); + + $this->assertSame(['home'], array_column($menuItems, 'technicalName')); + } + + public function testFindAllIncludesTheMenus(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'home', MenuItem::TARGET_URL, null); + $this->createMenuItem($reportsId, 'overview', MenuItem::TARGET_URL, null); + + $menus = []; + foreach ($this->contextMapper->findAll() as $context) { + $menus[$context->getId()] = array_column($context->getMenuItems(), 'technicalName'); + } + ksort($menus); + + $this->assertSame([$intakeId => ['home'], $reportsId => ['overview']], $menus); + } + + public function testATechnicalNameIsUniqueWithinAnApplication(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'home', MenuItem::TARGET_URL, null); + $this->createMenuItem($reportsId, 'home', MenuItem::TARGET_URL, null); + + try { + $this->createMenuItem($intakeId, 'home', MenuItem::TARGET_URL, null); + $this->fail('The unique index accepted a second item named home'); + } catch (Exception $e) { + $this->assertSame(Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION, $e->getReason()); + } + } + + public function testAnItemWithoutAGivenNameIsNamedByItsId(): void { + $intakeId = $this->createContext('Intake'); + $menuItem = new MenuItem(); + $menuItem->setContextId($intakeId); + $menuItem->setLabel('Docs'); + $menuItem->setTargetType(MenuItem::TARGET_URL); + $menuItem->setUrl('https://example.org'); + $menuItem = $this->menuItemMapper->insertWithDefaultName($menuItem); + + $this->assertSame('item_' . $menuItem->getId(), $menuItem->getTechnicalName()); + $this->assertSame(['item_' . $menuItem->getId()], self::technicalNames($this->menuItemMapper->findByContextId($intakeId))); + } + + public function testOnlyTheMenuOfTheGivenApplicationIsDeleted(): void { + $intakeId = $this->createContext('Intake'); + $reportsId = $this->createContext('Reports'); + $this->createMenuItem($intakeId, 'home', MenuItem::TARGET_URL, null); + $this->createMenuItem($reportsId, 'overview', MenuItem::TARGET_URL, null); + + $this->menuItemMapper->deleteAllByContextId($intakeId); + + $this->assertSame([], $this->menuItemMapper->findByContextId($intakeId)); + $this->assertSame(['overview'], self::technicalNames($this->menuItemMapper->findByContextId($reportsId))); + } + + /** + * @param list $menuItems + * @return list + */ + private static function technicalNames(array $menuItems): array { + return array_map(static fn (MenuItem $menuItem): string => $menuItem->getTechnicalName(), $menuItems); + } + + private function createContext(string $name): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_contexts_context') + ->values([ + 'name' => $qb->createNamedParameter($name), + 'icon' => $qb->createNamedParameter('star'), + 'description' => $qb->createNamedParameter(''), + 'owner_id' => $qb->createNamedParameter('user1'), + 'owner_type' => $qb->createNamedParameter(0), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function createPage(int $contextId): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_contexts_page') + ->values([ + 'context_id' => $qb->createNamedParameter($contextId), + 'page_type' => $qb->createNamedParameter('startpage'), + 'technical_name' => $qb->createNamedParameter('dashboard'), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function createView(int $tableId): int { + $qb = $this->connection->getQueryBuilder(); + $qb->insert('tables_views') + ->values([ + 'table_id' => $qb->createNamedParameter($tableId), + 'title' => $qb->createNamedParameter('Open requests'), + 'description' => $qb->createNamedParameter(''), + 'created_by' => $qb->createNamedParameter('user1'), + 'created_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + 'last_edit_by' => $qb->createNamedParameter('user1'), + 'last_edit_at' => $qb->createNamedParameter(date('Y-m-d H:i:s')), + ]) + ->executeStatement(); + return (int)$this->connection->lastInsertId(); + } + + private function createMenuItem(int $contextId, string $technicalName, string $targetType, ?int $targetId, int $order = 0): void { + $menuItem = new MenuItem(); + $menuItem->setContextId($contextId); + $menuItem->setLabel($technicalName); + $menuItem->setTargetType($targetType); + $menuItem->setTargetId($targetId); + $menuItem->setTechnicalName($technicalName); + $menuItem->setOrder($order); + $this->menuItemMapper->insert($menuItem); + } +} diff --git a/tests/unit/Db/MenuItemTest.php b/tests/unit/Db/MenuItemTest.php new file mode 100644 index 0000000000..d58f198d1f --- /dev/null +++ b/tests/unit/Db/MenuItemTest.php @@ -0,0 +1,99 @@ +setContextId(3); + + $this->assertSame([ + 'id' => null, + 'contextId' => 3, + 'label' => '', + 'icon' => null, + 'description' => null, + 'targetType' => MenuItem::TARGET_URL, + 'targetId' => null, + 'url' => null, + 'technicalName' => '', + 'order' => 0, + 'parentId' => null, + 'broken' => false, + ], $item->jsonSerialize()); + } + + public function testJsonSerializeCarriesEveryField(): void { + $item = new MenuItem(); + $item->setId(9); + $item->setContextId(3); + $item->setLabel('Intake home'); + $item->setIcon('home'); + $item->setDescription('Where intake starts'); + $item->setTargetType(MenuItem::TARGET_VIEW); + $item->setTargetId(6); + $item->setTechnicalName('home'); + $item->setOrder(20); + $item->setParentId(4); + + $json = $item->jsonSerialize(); + + $this->assertSame(9, $json['id']); + $this->assertSame('Intake home', $json['label']); + $this->assertSame('home', $json['icon']); + $this->assertSame('Where intake starts', $json['description']); + $this->assertSame('view', $json['targetType']); + $this->assertSame(6, $json['targetId']); + $this->assertSame('home', $json['technicalName']); + $this->assertSame(20, $json['order']); + $this->assertSame(4, $json['parentId']); + } + + public static function targetTypeProvider(): array { + return [ + 'view' => [MenuItem::TARGET_VIEW], + 'table' => [MenuItem::TARGET_TABLE], + 'url' => [MenuItem::TARGET_URL], + 'page' => [MenuItem::TARGET_PAGE], + ]; + } + + /** + * @dataProvider targetTypeProvider + */ + public function testJsonSerializeKeepsEachTargetType(string $targetType): void { + $item = new MenuItem(); + $item->setTargetType($targetType); + + $this->assertSame($targetType, $item->jsonSerialize()['targetType']); + } + + public function testBrokenIsReportedButNeverStored(): void { + $item = new MenuItem(); + $item->setBroken(true); + + $this->assertTrue($item->jsonSerialize()['broken']); + $this->assertArrayNotHasKey('broken', $item->getUpdatedFields()); + } + + public function testTargetTypesAreTheFourKnownOnes(): void { + $this->assertSame(['view', 'table', 'url', 'page'], MenuItem::TARGET_TYPES); + } + + public function testOnlyTheAutomaticFormIsADefaultName(): void { + $this->assertTrue(MenuItem::isDefaultName('item_7')); + foreach (['item', 'item_', 'item_7a', 'my_item_7', 'page_7', 'docs'] as $name) { + $this->assertFalse(MenuItem::isDefaultName($name), $name); + } + } +} diff --git a/tests/unit/Model/MenuItemInputTest.php b/tests/unit/Model/MenuItemInputTest.php new file mode 100644 index 0000000000..8ce6a356ca --- /dev/null +++ b/tests/unit/Model/MenuItemInputTest.php @@ -0,0 +1,109 @@ + ' Intake home ', 'targetType' => 'view', 'targetId' => 6, 'technicalName' => 'home', 'url' => 'https://ignored.example', 'description' => ' Where intake starts '], + ['label' => 'Docs', 'targetType' => 'url', 'url' => 'https://example.org/docs', 'targetId' => 99, 'icon' => 'book'], + ['label' => 'Requests', 'targetType' => 'table', 'targetId' => 4, 'icon' => ''], + ]); + + $this->assertSame(['Intake home', null, 'view', 6, null, 'home', 'Where intake starts'], $this->toArray($items[0])); + $this->assertSame(['Docs', 'book', 'url', null, 'https://example.org/docs', null, null], $this->toArray($items[1])); + $this->assertSame(['Requests', null, 'table', 4, null, null, null], $this->toArray($items[2])); + } + + public function testATargetTypeOfUrlIsTheDefault(): void { + $item = MenuItemInput::createFromInputArray(['label' => 'Docs', 'url' => 'http://example.org']); + + $this->assertSame('url', $item->getTargetType()); + } + + public static function invalidItems(): array { + return [ + 'not an object' => ['not an item', 'must be an object'], + 'no label' => [['targetType' => 'url', 'url' => 'https://example.org'], 'needs a label'], + 'blank label' => [['label' => ' ', 'targetType' => 'url', 'url' => 'https://example.org'], 'needs a label'], + 'label too long' => [['label' => str_repeat('x', 201), 'url' => 'https://example.org'], 'needs a label'], + 'unknown target type' => [['label' => 'X', 'targetType' => 'kanban', 'targetId' => 1], 'Unexpected menu item target type'], + 'view without id' => [['label' => 'X', 'targetType' => 'view'], 'needs its id'], + 'page without id' => [['label' => 'X', 'targetType' => 'page'], 'needs its id'], + 'id as a string' => [['label' => 'X', 'targetType' => 'table', 'targetId' => '4'], 'needs its id'], + 'id of zero' => [['label' => 'X', 'targetType' => 'table', 'targetId' => 0], 'needs its id'], + 'javascript link' => [['label' => 'X', 'url' => 'javascript:alert(1)'], 'http or https'], + 'data link' => [['label' => 'X', 'url' => 'data:text/html,hi'], 'http or https'], + // carries a host, so only the scheme check stops it + 'javascript link with a host' => [['label' => 'X', 'url' => 'javascript://example.com/%0Aalert(1)'], 'http or https'], + 'link without host' => [['label' => 'X', 'url' => 'https://'], 'http or https'], + 'link with a space' => [['label' => 'X', 'url' => 'https://exa mple.com'], 'http or https'], + 'link with a tab' => [['label' => 'X', 'url' => "https://example.com/\tpath"], 'http or https'], + 'link with a line break' => [['label' => 'X', 'url' => "https://example.com/a\nb"], 'http or https'], + 'link with a control character' => [['label' => 'X', 'url' => "https://example.com/\x01"], 'http or https'], + 'link with a delete character' => [['label' => 'X', 'url' => "https://example.com/\x7f"], 'http or https'], + 'link with a non-breaking space' => [['label' => 'X', 'url' => "https://example.com/\u{00A0}x"], 'http or https'], + 'link that is not utf-8' => [['label' => 'X', 'url' => "https://example.com/\xff"], 'http or https'], + 'empty link' => [['label' => 'X', 'url' => ''], 'http or https'], + 'technical name with a hyphen' => [['label' => 'X', 'url' => 'https://example.org', 'technicalName' => 'my-page'], 'technical name'], + 'icon that is not a string' => [['label' => 'X', 'url' => 'https://example.org', 'icon' => 3], '"icon" must be a string'], + 'unknown icon' => [['label' => 'X', 'url' => 'https://example.org', 'icon' => 'no-such-icon'], 'unknown icon'], + 'description that is not a string' => [['label' => 'X', 'url' => 'https://example.org', 'description' => 3], '"description" must be a string'], + 'description too long' => [['label' => 'X', 'url' => 'https://example.org', 'description' => str_repeat('x', 2001)], 'description has at most'], + 'technical name with a capital' => [['label' => 'X', 'url' => 'https://example.org', 'technicalName' => 'Home'], 'technical name'], + 'technical name too long' => [['label' => 'X', 'url' => 'https://example.org', 'technicalName' => str_repeat('a', 201)], 'technical name'], + ]; + } + + /** + * @dataProvider invalidItems + */ + public function testRejectsInvalidInput(mixed $input, string $message): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + MenuItemInput::createFromInputArray($input); + } + + public function testAPageItemWithoutAnIdOpensTheNewStartPageOnlyWhenAllowed(): void { + [$startPage, $page] = MenuItemInput::createListFromInputArray([ + ['label' => 'Dashboard', 'targetType' => 'page', 'technicalName' => 'home'], + ['label' => 'Overview', 'targetType' => 'page', 'targetId' => 5], + ], allowNewStartPage: true); + + $this->assertTrue($startPage->pointsAtNewStartPage()); + $this->assertSame(['Dashboard', null, 'page', null, null, 'home', null], $this->toArray($startPage)); + $this->assertFalse($page->pointsAtNewStartPage()); + $this->assertSame(['Overview', null, 'page', 5, null, null, null], $this->toArray($page)); + + $resolved = $startPage->withPageId(3); + $this->assertFalse($resolved->pointsAtNewStartPage()); + $this->assertSame(['Dashboard', null, 'page', 3, null, 'home', null], $this->toArray($resolved)); + } + + public function testOnlyAPageItemMayLeaveOutItsIdWhenTheStartPageIsAllowed(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('needs its id'); + MenuItemInput::createFromInputArray(['label' => 'X', 'targetType' => 'view'], allowNewStartPage: true); + } + + public function testRejectsAMenuThatIsNotAList(): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('must be a list'); + MenuItemInput::createListFromInputArray(['first' => ['label' => 'X', 'url' => 'https://example.org']]); + } + + private function toArray(MenuItemInput $item): array { + return [$item->getLabel(), $item->getIcon(), $item->getTargetType(), $item->getTargetId(), $item->getUrl(), $item->getTechnicalName(), $item->getDescription()]; + } +} diff --git a/tests/unit/Service/ContextPagesRoundTripTest.php b/tests/unit/Service/ContextPagesRoundTripTest.php index b2951a5578..7a946043c4 100644 --- a/tests/unit/Service/ContextPagesRoundTripTest.php +++ b/tests/unit/Service/ContextPagesRoundTripTest.php @@ -14,6 +14,7 @@ use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContentMapper; use OCA\Tables\Db\PageMapper; @@ -24,6 +25,7 @@ use OCA\Tables\Db\WidgetSourceMapper; use OCA\Tables\Helper\GroupHelper; use OCA\Tables\Helper\UserHelper; +use OCA\Tables\Model\MenuItemInput; use OCA\Tables\Service\ContextService; use OCA\Tables\Service\GridWidgetTypes; use OCA\Tables\Service\PermissionsService; @@ -35,6 +37,7 @@ use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IUserManager; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; /** @@ -49,6 +52,8 @@ class ContextPagesRoundTripTest extends DatabaseTestCase { private ContextNodeRelationMapper $nodeRelMapper; private PageMapper $pageMapper; private WidgetService $widgetService; + private PermissionsService&MockObject $permissionsService; + private TableMapper&MockObject $tableMapper; /** @var int[] */ private array $contextIds = []; @@ -58,7 +63,7 @@ protected function setUp(): void { $l10n->method('t')->willReturnArgument(0); $userHelper = $this->createMock(UserHelper::class); $userHelper->method('getGroupIdsForUser')->willReturn([]); - $this->contextMapper = new ContextMapper($this->connectionAdapter, $userHelper, $this->createMock(GroupHelper::class)); + $this->contextMapper = new ContextMapper($this->connectionAdapter, $userHelper, $this->createMock(GroupHelper::class), new MenuItemMapper($this->connectionAdapter)); $this->nodeRelMapper = new ContextNodeRelationMapper($this->connectionAdapter); $this->pageMapper = new PageMapper($this->connectionAdapter); $this->widgetService = new WidgetService( @@ -72,7 +77,7 @@ protected function setUp(): void { $this->pageMapper, new PageContentMapper($this->connectionAdapter), $this->createMock(LoggerInterface::class), - $this->createMock(PermissionsService::class), + $this->permissionsService = $this->createMock(PermissionsService::class), $this->createMock(IUserManager::class), $this->createMock(AuditLogServiceInterface::class), $this->connectionAdapter, @@ -80,10 +85,12 @@ protected function setUp(): void { false, $this->createMock(INavigationManager::class), $this->createMock(IURLGenerator::class), - $this->createMock(TableMapper::class), + $this->tableMapper = $this->createMock(TableMapper::class), $this->createMock(ViewMapper::class), $this->widgetService, new GridWidgetTypes($l10n), + new MenuItemMapper($this->connectionAdapter), + $l10n, ); } @@ -196,4 +203,72 @@ private function createContextWithGridPage(): array { return [$context, $relation, $page->getId()]; } + + /** + * The frontend sends a table both as a node and as a menu target. + */ + public function testATableInBothTheNodesAndTheMenuIsAddedOnceOnCreateAndUpdate(): void { + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $this->tableMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => array_combine($ids, array_map(static fn (): Table => new Table(), $ids))); + $node = static fn (int $id): array => ['id' => $id, 'type' => Application::NODE_TYPE_TABLE, 'permissions' => Application::PERMISSION_READ]; + $item = static fn (int $id): array => ['label' => 'Table ' . $id, 'targetType' => 'table', 'targetId' => $id]; + + $created = $this->service->create('Nodes and menu', 'tables', '', [$node(990501)], self::OWNER, Application::OWNER_TYPE_USER, + MenuItemInput::createListFromInputArray([$item(990501)], allowNewStartPage: true)); + $this->contextIds[] = $created->getId(); + $this->assertSame([990501], $this->storedNodeIds($created->getId())); + + $this->service->update($created->getId(), self::OWNER, null, null, null, [$node(990501), $node(990502)], + MenuItemInput::createListFromInputArray([$item(990501), $item(990502)])); + $this->assertSame([990501, 990502], $this->storedNodeIds($created->getId())); + } + + /** + * The node ids of an application, once for every relation, and the start page holds + * each relation once. + * + * @return list + */ + private function storedNodeIds(int $contextId): array { + $context = $this->contextMapper->findById($contextId, self::OWNER); + $nodeIds = array_values(array_map(static fn (array $node): int => $node['node_id'], $context->getNodes())); + sort($nodeIds); + $startPage = array_values(array_filter($context->getPages(), static fn (array $page): bool => $page['page_type'] === Page::TYPE_STARTPAGE))[0]; + $this->assertCount(count($nodeIds), $startPage['content']); + return $nodeIds; + } + + /** + * The responses of creating and updating an application keep the shape they had + * before menus: a new application lists its nodes as relations and its start page + * contents by id, and an update returns a node it added as its relation. + */ + public function testCreateAndUpdateResponsesKeepTheirNodeShape(): void { + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $node = static fn (int $id, int $permissions): array => ['id' => $id, 'type' => Application::NODE_TYPE_TABLE, 'permissions' => $permissions]; + + $created = $this->service->create('Shape', 'tables', '', [$node(990601, 1), $node(990602, 3)], self::OWNER, Application::OWNER_TYPE_USER); + $this->contextIds[] = $id = $created->getId(); + $json = json_decode(json_encode($created), true); + [$first, $second] = $json['nodes']; + $this->assertSame(array_keys($json['nodes']), [0, 1]); + $this->assertSame(['id' => $first['id'], 'contextId' => $id, 'nodeId' => 990601, 'nodeType' => 0, 'permissions' => 1], $first); + $this->assertSame(['id' => $second['id'], 'contextId' => $id, 'nodeId' => 990602, 'nodeType' => 0, 'permissions' => 3], $second); + $startPage = array_values($json['pages'])[0]; + $this->assertSame(['id', 'contextId', 'pageType', 'technicalName', 'title', 'icon', 'description', 'content'], array_keys($startPage)); + $contentIds = array_keys($startPage['content']); + $this->assertSame([ + ['id' => $contentIds[0], 'nodeRelId' => $first['id'], 'order' => 10], + ['id' => $contentIds[1], 'nodeRelId' => $second['id'], 'order' => 20], + ], array_values($startPage['content'])); + + $updated = json_decode(json_encode($this->service->update($id, self::OWNER, null, null, null, [$node(990602, 1), $node(990603, 1)])), true); + $this->assertSame([$second['id']], array_values(array_diff(array_keys($updated['nodes']), [max(array_keys($updated['nodes']))]))); + $added = $updated['nodes'][max(array_keys($updated['nodes']))]; + $this->assertSame(['id', 'contextId', 'nodeId', 'nodeType', 'permissions'], array_keys($added)); + $this->assertSame(['id', 'node_id', 'node_type', 'permissions'], array_keys($updated['nodes'][$second['id']])); + $addedContent = array_values(array_filter(array_values($updated['pages'])[0]['content'], static fn (array $content): bool => isset($content['nodeRelId']))); + $this->assertSame(['id', 'pageId', 'nodeRelId', 'order'], array_keys($addedContent[0])); + $this->assertSame($added['id'], $addedContent[0]['nodeRelId']); + } } diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 9dd2b6f031..962ba1bbdf 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -9,20 +9,28 @@ namespace OCA\Tables\Service; +use InvalidArgumentException; +use OC\DB\Exceptions\DbalException; use OCA\Tables\AppInfo\Application; use OCA\Tables\AuditLog\Operation; use OCA\Tables\Db\Context; use OCA\Tables\Db\ContextMapper; use OCA\Tables\Db\ContextNodeRelation; use OCA\Tables\Db\ContextNodeRelationMapper; +use OCA\Tables\Db\MenuItem; +use OCA\Tables\Db\MenuItemMapper; use OCA\Tables\Db\Page; use OCA\Tables\Db\PageContent; use OCA\Tables\Db\PageContentMapper; use OCA\Tables\Db\PageMapper; use OCA\Tables\Db\Table; use OCA\Tables\Db\TableMapper; +use OCA\Tables\Db\View; use OCA\Tables\Db\ViewMapper; +use OCA\Tables\Errors\PermissionError; +use OCA\Tables\Model\MenuItemInput; use OCA\Tables\Service\Support\AuditLogServiceInterface; +use OCA\Tables\Tests\Unit\Database\DbErrorStub; use OCP\IDBConnection; use OCP\IL10N; use OCP\INavigationManager; @@ -41,7 +49,12 @@ final class ContextServiceTest extends TestCase { private MockObject $userManager; private MockObject $auditLogService; private MockObject $widgetService; + private MockObject $menuItemMapper; + private MockObject $permissionsService; + private MockObject $shareService; private MockObject $dbConnection; + private MockObject $tableMapper; + private MockObject $viewMapper; protected function setUp(): void { parent::setUp(); @@ -51,14 +64,19 @@ protected function setUp(): void { $this->pageMapper = $this->createMock(PageMapper::class); $this->pageContentMapper = $this->createMock(PageContentMapper::class); $logger = $this->createMock(LoggerInterface::class); - $permissionsService = $this->createMock(PermissionsService::class); + $this->permissionsService = $this->createMock(PermissionsService::class); + $this->tableMapper = $this->createMock(TableMapper::class); + $this->viewMapper = $this->createMock(ViewMapper::class); $this->userManager = $this->createMock(IUserManager::class); $this->auditLogService = $this->createMock(AuditLogServiceInterface::class); $this->dbConnection = $this->createMock(IDBConnection::class); $this->widgetService = $this->createMock(WidgetService::class); + $this->menuItemMapper = $this->createMock(MenuItemMapper::class); $l10n = $this->createMock(IL10N::class); $l10n->method('t')->willReturnArgument(0); - $shareService = $this->createMock(ShareService::class); + $translations = $this->createMock(IL10N::class); + $translations->method('t')->willReturnCallback(static fn (string $text): string => 'translated ' . $text); + $this->shareService = $this->createMock(ShareService::class); $navigationManager = $this->createMock(INavigationManager::class); $urlGenerator = $this->createMock(IURLGenerator::class); @@ -68,18 +86,20 @@ protected function setUp(): void { $this->pageMapper, $this->pageContentMapper, $logger, - $permissionsService, + $this->permissionsService, $this->userManager, $this->auditLogService, $this->dbConnection, - $shareService, + $this->shareService, false, $navigationManager, $urlGenerator, - $this->createMock(TableMapper::class), - $this->createMock(ViewMapper::class), + $this->tableMapper, + $this->viewMapper, $this->widgetService, new GridWidgetTypes($l10n), + $this->menuItemMapper, + $translations, ); } @@ -305,6 +325,35 @@ public function testDeleteRemovesTheWidgetsOfEveryPage(): void { $this->service->delete(7, 'user-1'); } + public function testDeleteRemovesTheApplicationBeforeWhatBelongsToIt(): void { + $context = new Context(); + $context->id = 7; + $this->contextMapper->method('findById')->willReturn($context); + $this->pageMapper->method('getPageIdsForContext')->willReturn([301]); + $calls = []; + $this->contextMapper->method('delete')->willReturnCallback(static function (Context $deleted) use (&$calls): Context { + $calls[] = 'application'; + return $deleted; + }); + $this->menuItemMapper->method('deleteAllByContextId')->with(7)->willReturnCallback(static function () use (&$calls): void { + $calls[] = 'menu'; + }); + $this->contextNodeRelMapper->method('deleteAllByContextId')->willReturnCallback(static function () use (&$calls): void { + $calls[] = 'nodes'; + }); + $this->widgetService->method('deleteByPageIds')->willReturnCallback(static function () use (&$calls): void { + $calls[] = 'widgets'; + }); + $this->pageMapper->method('deleteByPageId')->willReturnCallback(static function () use (&$calls): int { + $calls[] = 'page'; + return 1; + }); + + $this->service->delete(7, 'alice'); + + $this->assertSame(['application', 'menu', 'nodes', 'widgets', 'page'], $calls); + } + public function testDeletingANodeUnlinksItsWidgets(): void { $this->contextNodeRelMapper->method('getRelIdsForNode')->with(11, 0)->willReturn([101, 102]); @@ -471,6 +520,483 @@ public function testImportedPages(array $pages, array $expected): void { self::assertSame($expected, $inserted); } + private function contextWithMenu(int $id, array $menuItems): Context { + $context = new Context(); + $context->id = $id; + $context->setName('Intake portal'); + $context->setOwnerId('user-1'); + $context->setOwnerType(Application::OWNER_TYPE_USER); + $context->setNodes([]); + $context->setPages([]); + $context->setMenuItems($menuItems); + return $context; + } + + /** + * @param list $inserted collects the stored items + */ + private function expectMenuReplacement(int $contextId, array &$inserted): void { + $this->menuItemMapper->expects($this->once())->method('deleteAllByContextId')->with($contextId); + $this->menuItemMapper->method('insert')->willReturnCallback(static function (MenuItem $item) use (&$inserted): MenuItem { + $item->id = count($inserted) + 1; + $inserted[] = $item; + return $item; + }); + $this->menuItemMapper->method('insertWithDefaultName')->willReturnCallback(static function (MenuItem $item) use (&$inserted): MenuItem { + $item->id = count($inserted) + 1; + $item->setTechnicalName('item_' . $item->getId()); + $inserted[] = $item; + return $item; + }); + $this->menuItemMapper->method('findByContextId')->willReturnCallback(static function () use (&$inserted): array { + return $inserted; + }); + } + + private function expectExistingTargets(array $views, array $tables): void { + $this->viewMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => array_intersect_key($views, array_flip($ids))); + $this->tableMapper->method('findMany')->willReturnCallback(static fn (array $ids): array => array_intersect_key($tables, array_flip($ids))); + } + + private function expectNodeInsertion(array &$relations): void { + $this->contextNodeRelMapper->method('insert')->willReturnCallback(static function (ContextNodeRelation $relation) use (&$relations): ContextNodeRelation { + $relation->id = 200 + count($relations); + $relations[] = $relation; + return $relation; + }); + } + + public function testUpdateReplacesTheMenuInOrderAndNamesUnnamedItemsByTheirId(): void { + $context = $this->contextWithMenu(7, []); + $this->contextMapper->method('findById')->with(7, 'user-1')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $this->expectExistingTargets([6 => new View()], [4 => new Table()]); + $relations = []; + $this->expectNodeInsertion($relations); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $updated = $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Intake home', 'targetType' => 'view', 'targetId' => 6, 'technicalName' => 'home'], + ['label' => 'Welcome table!', 'targetType' => 'table', 'targetId' => 4], + ['label' => 'Docs', 'targetType' => 'url', 'url' => 'https://docs.nextcloud.com'], + ])); + + $this->assertSame([10, 20, 30], array_map(static fn (MenuItem $item) => $item->getOrder(), $inserted)); + $this->assertSame(['home', 'item_2', 'item_3'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + $this->assertSame([6, 4, null], array_map(static fn (MenuItem $item) => $item->getTargetId(), $inserted)); + $this->assertSame([null, null, 'https://docs.nextcloud.com'], array_map(static fn (MenuItem $item) => $item->getUrl(), $inserted)); + $this->assertCount(3, $updated->getMenuItems()); + $this->assertSame('Welcome table!', $updated->getMenuItems()[1]['label']); + } + + public function testMenuTargetsBecomeNodesSoRecipientsCanOpenThem(): void { + $context = $this->contextWithMenu(7, []); + $context->setNodes([101 => ['id' => 101, 'node_id' => 4, 'node_type' => Application::NODE_TYPE_TABLE, 'permissions' => 1]]); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->permissionsService->method('canManageNodeById')->willReturn(true); + $this->expectExistingTargets([6 => new View()], [4 => new Table()]); + $relations = []; + $this->expectNodeInsertion($relations); + $this->pageContentMapper->method('findByPageAndNodeRelation')->willReturn(null); + $this->pageContentMapper->method('insert')->willReturnCallback(static function (PageContent $content): PageContent { + $content->id = 501; + return $content; + }); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $updated = $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Home', 'targetType' => 'view', 'targetId' => 6], + ['label' => 'Requests', 'targetType' => 'table', 'targetId' => 4], + ])); + + // the table already is a node, only the view is added, with read permission + $this->assertCount(1, $relations); + $this->assertSame([6, Application::NODE_TYPE_VIEW, Application::PERMISSION_READ], [$relations[0]->getNodeId(), $relations[0]->getNodeType(), $relations[0]->getPermissions()]); + // a node added by a save comes back as its relation, as the update has always returned it + $this->assertSame(['id' => 200, 'contextId' => 7, 'nodeId' => 6, 'nodeType' => Application::NODE_TYPE_VIEW, 'permissions' => Application::PERMISSION_READ], $updated->getNodes()[200]); + $this->assertSame(200, $updated->getPages()[301]['content'][501]['nodeRelId']); + } + + public static function unusableNewTargets(): array { + return [ + 'exists but cannot be managed' => [[4 => new Table()]], + 'does not exist' => [[]], + ]; + } + + /** + * Both get the same answer, so a save reveals nothing about which ids exist. + * + * @dataProvider unusableNewTargets + */ + public function testANewMenuTargetThatCannotBeUsedIsRefusedTheSameWay(array $tables): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->expectExistingTargets([], $tables); + $this->permissionsService->method('canManageNodeById')->willReturn(false); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $this->expectException(PermissionError::class); + $this->expectExceptionMessage('Owner cannot manage node 4 (type 0)'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Requests', 'targetType' => 'table', 'targetId' => 4], + ])); + } + + public function testAStoredMenuTargetThatNoLongerExistsIsKeptWithoutANode(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [ + ['targetType' => 'view', 'targetId' => 99, 'url' => null, 'technicalName' => 'gone'], + ])); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->expectExistingTargets([], []); + $this->contextNodeRelMapper->expects($this->never())->method('insert'); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Gone', 'targetType' => 'view', 'targetId' => 99], + ])); + + $this->assertSame([99], array_map(static fn (MenuItem $item) => $item->getTargetId(), $inserted)); + } + + public function testAnItemSentWithoutANameKeepsTheStoredNameOfItsTarget(): void { + $context = $this->contextWithMenu(7, [ + ['targetType' => 'view', 'targetId' => 6, 'url' => null, 'technicalName' => 'home'], + ['targetType' => 'url', 'targetId' => null, 'url' => 'https://example.org/a', 'technicalName' => 'docs'], + ['targetType' => 'url', 'targetId' => null, 'url' => 'https://example.org/b', 'technicalName' => 'docs_2'], + ]); + $context->setNodes([101 => ['id' => 101, 'node_id' => 6, 'node_type' => Application::NODE_TYPE_VIEW, 'permissions' => 1]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + // the view is renamed, the two links swap places and a new link is added + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Start', 'targetType' => 'view', 'targetId' => 6], + ['label' => 'Docs', 'url' => 'https://example.org/b'], + ['label' => 'Docs', 'url' => 'https://example.org/a'], + ['label' => 'Docs', 'url' => 'https://example.org/c'], + ])); + + $this->assertSame(['home', 'docs_2', 'docs', 'item_4'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + } + + public function testUnnamedItemsAreNamedByTheirIdBesideGivenNames(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org/a'], + ['label' => 'Docs', 'url' => 'https://example.org/b'], + ['label' => 'Other', 'url' => 'https://example.org/c', 'technicalName' => 'docs_2'], + ])); + + $this->assertSame(['item_1', 'item_2', 'docs_2'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + } + + public function testTwoMenuItemsWithTheSameTechnicalNameAreRefused(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('share the technical name "home"'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'A', 'url' => 'https://example.org/a', 'technicalName' => 'home'], + ['label' => 'B', 'url' => 'https://example.org/b', 'technicalName' => 'home'], + ])); + } + + public function testAGivenNameOfTheAutomaticFormIsRefused(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [ + ['targetType' => 'url', 'targetId' => null, 'url' => 'https://example.org/a', 'technicalName' => 'item_3'], + ])); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The technical name item_3 is reserved for automatic names.'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'B', 'url' => 'https://example.org/b', 'technicalName' => 'item_3'], + ])); + } + + public function testAnItemSentBackWithItsStoredAutomaticNameKeepsIt(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [ + ['targetType' => 'url', 'targetId' => null, 'url' => 'https://example.org/a', 'technicalName' => 'item_3'], + ])); + $this->contextMapper->method('update')->willReturnArgument(0); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'A', 'url' => 'https://example.org/a', 'technicalName' => 'item_3'], + ])); + + $this->assertSame(['item_3'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + } + + public function testUpdateRunsAgainWhenTheDatabaseAsksForARetry(): void { + $locked = $this->createMock(DbalException::class); + $locked->method('isRetryable')->willReturn(true); + $this->contextMapper->expects($this->exactly(2))->method('findById') + ->willReturnOnConsecutiveCalls($this->throwException($locked), $this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + + $updated = $this->service->update(7, 'user-1', 'Renamed', null, null, null); + + $this->assertSame('Renamed', $updated->getName()); + } + + public function testUpdateLeavesTheMenuAloneWhenNoneIsGiven(): void { + $context = $this->contextWithMenu(7, [['id' => 1, 'label' => 'Keep me']]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $updated = $this->service->update(7, 'user-1', 'Renamed', null, null, null); + + $this->assertSame('Renamed', $updated->getName()); + $this->assertSame('Keep me', $updated->getMenuItems()[0]['label']); + } + + public function testCreateStoresTheMenuItems(): void { + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context): Context { + $context->id = 8; + return $context; + }); + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page): Page { + $page->id = 3; + return $page; + }); + $inserted = []; + $this->expectMenuReplacement(8, $inserted); + + $created = $this->service->create('Intake portal', 'heart', ' desc ', [], 'user-1', 0, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'targetType' => 'url', 'url' => 'https://example.org'], + ], allowNewStartPage: true)); + + $this->assertSame('desc', $created->getDescription()); + $this->assertCount(1, $created->getMenuItems()); + $this->assertSame(['item_1'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + } + + /** + * @param list $inserted collects the stored items + */ + private function expectCreation(int $contextId, int $startPageId, array &$inserted): void { + $this->contextMapper->method('insert')->willReturnCallback(static function (Context $context) use ($contextId): Context { + $context->id = $contextId; + return $context; + }); + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page) use ($startPageId): Page { + $page->id = $startPageId; + return $page; + }); + $this->expectMenuReplacement($contextId, $inserted); + } + + /** + * @param list $items + * @return list + */ + private static function describeMenu(array $items): array { + return array_map(static fn (MenuItem $item): array => [ + $item->getLabel(), $item->getTargetType(), $item->getTargetId(), $item->getTechnicalName(), $item->getIcon(), + ], $items); + } + + public function testCreateWithoutAMenuAddsADashboardItemForTheStartPage(): void { + $inserted = []; + $this->expectCreation(8, 3, $inserted); + + $created = $this->service->create('Intake portal', 'heart', '', [], 'user-1', 0); + + $this->assertSame([['translated Dashboard', 'page', 3, 'dashboard', null]], self::describeMenu($inserted)); + $this->assertSame(3, $created->getMenuItems()[0]['targetId']); + } + + public function testCreateWithAnEmptyMenuStoresNoItems(): void { + $inserted = []; + $this->expectCreation(8, 3, $inserted); + + $created = $this->service->create('Intake portal', 'heart', '', [], 'user-1', 0, []); + + $this->assertSame([], $inserted); + $this->assertSame([], $created->getMenuItems()); + } + + public function testCreateStoresAMenuWithoutAStartPageItemAsSubmitted(): void { + $inserted = []; + $this->expectCreation(8, 3, $inserted); + + $this->service->create('Intake portal', 'heart', '', [], 'user-1', 0, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org', 'technicalName' => 'dashboard'], + ['label' => 'Guide', 'url' => 'https://example.org/guide'], + ], allowNewStartPage: true)); + + $this->assertSame([ + ['Docs', 'url', null, 'dashboard', null], + ['Guide', 'url', null, 'item_2', null], + ], self::describeMenu($inserted)); + } + + public function testCreateResolvesAPageItemWithoutAnIdToTheStartPage(): void { + $inserted = []; + $this->expectCreation(8, 3, $inserted); + + $this->service->create('Intake portal', 'heart', '', [], 'user-1', 0, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org'], + ['label' => 'Overview', 'targetType' => 'page', 'icon' => 'home'], + ], allowNewStartPage: true)); + + $this->assertSame([ + ['Docs', 'url', null, 'item_1', null], + ['Overview', 'page', 3, 'item_2', 'home'], + ], self::describeMenu($inserted)); + } + + public function testAPageOfTheApplicationIsAcceptedAndAddsNoNode(): void { + $context = $this->contextWithMenu(7, []); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->contextNodeRelMapper->expects($this->never())->method('insert'); + $this->pageContentMapper->expects($this->never())->method('insert'); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $updated = $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Dashboard', 'targetType' => 'page', 'targetId' => 301], + ])); + + $this->assertSame([['Dashboard', 'page', 301, 'item_1', null]], self::describeMenu($inserted)); + $this->assertSame([], $updated->getNodes()); + } + + public function testAPageOfAnotherApplicationIsRefused(): void { + $context = $this->contextWithMenu(7, []); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('a page of its own application'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Elsewhere', 'targetType' => 'page', 'targetId' => 302], + ])); + } + + public function testAStoredPageItemWhosePageIsGoneIsKept(): void { + $context = $this->contextWithMenu(7, [ + ['targetType' => 'page', 'targetId' => 302, 'url' => null, 'technicalName' => 'overview', 'broken' => true], + ]); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->contextNodeRelMapper->expects($this->never())->method('insert'); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Overview', 'targetType' => 'page', 'targetId' => 302], + ['label' => 'Docs', 'url' => 'https://example.org'], + ])); + + $this->assertSame([['Overview', 'page', 302, 'overview', null], ['Docs', 'url', null, 'item_2', null]], self::describeMenu($inserted)); + } + + public static function newPageTargetsOutsideTheApplication(): array { + return [ + 'page of another application' => [302], + 'page id that does not exist' => [9999], + ]; + } + + /** + * @dataProvider newPageTargetsOutsideTheApplication + */ + public function testANewPageTargetOutsideTheApplicationIsRefusedEvenWithABrokenStoredOne(int $pageId): void { + // the stored item points at a page that is gone; that does not let other pages in + $context = $this->contextWithMenu(7, [ + ['targetType' => 'page', 'targetId' => 303, 'url' => null, 'technicalName' => 'gone', 'broken' => true], + ]); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('a page of its own application'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Gone', 'targetType' => 'page', 'targetId' => 303], + ['label' => 'Elsewhere', 'targetType' => 'page', 'targetId' => $pageId], + ])); + } + + public function testUpdateRefusesAPageItemWithoutAnId(): void { + $context = $this->contextWithMenu(7, []); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('a page of its own application'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Dashboard', 'targetType' => 'page'], + ], allowNewStartPage: true)); + } + + public function testUpdateAddsNoDashboardItem(): void { + $context = $this->contextWithMenu(7, []); + $context->setPages([301 => ['id' => 301, 'page_type' => Page::TYPE_STARTPAGE, 'content' => []]]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org'], + ])); + + $this->assertSame([['Docs', 'url', null, 'item_1', null]], self::describeMenu($inserted)); + } + + public function testAGivenMenuNameTakenByAConcurrentSaveIsRefused(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->once())->method('insert')->willThrowException(DbErrorStub::uniqueViolation()); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('already used'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org', 'technicalName' => 'docs'], + ])); + } + + public function testMenuItemsKeepTheirIconAndDescription(): void { + $this->contextMapper->method('findById')->willReturn($this->contextWithMenu(7, [])); + $this->contextMapper->method('update')->willReturnArgument(0); + $inserted = []; + $this->expectMenuReplacement(7, $inserted); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray([ + ['label' => 'Docs', 'url' => 'https://example.org', 'icon' => 'book', 'description' => 'How it works'], + ])); + + $this->assertSame([['book', 'How it works']], array_map(static fn (MenuItem $item): array => [$item->getIcon(), $item->getDescription()], $inserted)); + } + private function buildPageContent(int $id, int $pageId, int $nodeRelId, int $order): PageContent { $pageContent = new PageContent(); $pageContent->id = $id; @@ -597,4 +1123,79 @@ public function testAnImportedPageKeepsAtMostTheMaximumNumberOfWidgets(): void { self::assertCount(GridWidgetTypes::MAX_WIDGETS_PER_PAGE, $record['widgets']); } + + public function testAMenuItemIsBrokenForAReaderWhoCannotOpenItsTarget(): void { + $context = new Context(); + $context->id = 7; + $context->setNodes([101 => ['id' => 101, 'node_id' => 11, 'node_type' => 0, 'permissions' => 1]]); + $context->setMenuItems([ + ['id' => 1, 'targetType' => 'table', 'targetId' => 11, 'broken' => false], + ['id' => 2, 'targetType' => 'table', 'targetId' => 5, 'broken' => false], + ['id' => 3, 'targetType' => 'view', 'targetId' => 6, 'broken' => false], + ['id' => 4, 'targetType' => 'url', 'targetId' => null, 'broken' => false], + ['id' => 5, 'targetType' => 'table', 'targetId' => 9, 'broken' => true], + ]); + $this->contextMapper->method('findById')->willReturnCallback(static fn (): Context => clone $context); + $this->contextMapper->method('findAll')->willReturnCallback(static fn (): array => [clone $context]); + $this->permissionsService->method('canAccessNodeById')->willReturnCallback( + static fn (int $nodeType, int $nodeId, string $userId): bool => $userId === 'owner' || ($nodeType === 1 && $nodeId === 6), + ); + + self::assertSame([false, false, false, false, true], array_column($this->service->findById(7, 'owner')->getMenuItems(), 'broken')); + self::assertSame([false, true, false, false, true], array_column($this->service->findById(7, 'sharee')->getMenuItems(), 'broken')); + self::assertSame([false, true, false, false, true], array_column($this->service->findAll('sharee')[0]->getMenuItems(), 'broken')); + } + + public function testDeletingAnApplicationRetriesATransientDatabaseError(): void { + $context = new Context(); + $context->id = 7; + $this->contextMapper->method('findById')->willReturn($context); + $this->pageMapper->method('getPageIdsForContext')->willReturn([]); + $deadlock = \OC\DB\Exceptions\DbalException::wrap(new \Doctrine\DBAL\Exception\DeadlockException( + \Doctrine\DBAL\Driver\PDO\Exception::new(new \PDOException('deadlock')), + null, + )); + $attempts = 0; + $this->shareService->method('deleteAllForContext')->willReturnCallback(static function () use (&$attempts, $deadlock): void { + if (++$attempts === 1) { + throw $deadlock; + } + }); + $this->contextMapper->expects($this->exactly(2))->method('delete'); + $this->dbConnection->expects($this->once())->method('rollBack'); + + $this->service->delete(7, 'alice'); + + self::assertSame(2, $attempts); + } + + public function testAMenuOfMoreThanTheMaximumNumberOfItemsIsRefused(): void { + $context = new Context(); + $context->id = 7; + $context->setNodes([]); + $context->setPages([]); + $context->setMenuItems([]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->never())->method('deleteAllByContextId'); + $items = array_fill(0, MenuItem::MAX_ITEMS_PER_APPLICATION + 1, ['label' => 'Docs', 'url' => 'https://example.org']); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('at most 200 items'); + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray($items)); + } + + public function testAMenuOfTheMaximumNumberOfItemsIsSaved(): void { + $context = new Context(); + $context->id = 7; + $context->setNodes([]); + $context->setPages([]); + $context->setMenuItems([]); + $this->contextMapper->method('findById')->willReturn($context); + $this->contextMapper->method('update')->willReturnArgument(0); + $this->menuItemMapper->expects($this->exactly(MenuItem::MAX_ITEMS_PER_APPLICATION))->method('insertWithDefaultName'); + $items = array_fill(0, MenuItem::MAX_ITEMS_PER_APPLICATION, ['label' => 'Docs', 'url' => 'https://example.org']); + + $this->service->update(7, 'user-1', null, null, null, null, MenuItemInput::createListFromInputArray($items)); + } } diff --git a/tests/unit/Service/ShareServiceTest.php b/tests/unit/Service/ShareServiceTest.php index 1ab87968e5..a09f066b5c 100644 --- a/tests/unit/Service/ShareServiceTest.php +++ b/tests/unit/Service/ShareServiceTest.php @@ -351,4 +351,14 @@ public function testDeleteForShareReviewPropagatesDbException(): void { $this->expectException(Exception::class); $this->shareService->deleteForShareReview(5); } + + public function testDeletingTheSharesOfAContextPassesADatabaseErrorOn(): void { + $context = new \OCA\Tables\Db\Context(); + $context->id = 7; + $this->mapper->method('findAllSharesForNode')->willReturn([]); + $this->mapper->method('deleteByNode')->willThrowException(new Exception('locked')); + + $this->expectException(Exception::class); + $this->shareService->deleteAllForContext($context); + } }