diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index e87f8ca440..ac236b95ae 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -16,6 +16,7 @@ use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Errors\PermissionError; 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; @@ -100,6 +101,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 * * @return DataResponse|DataResponse * @@ -108,7 +110,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); @@ -120,13 +122,14 @@ public function create(string $name, string $iconName, string $description = '', $this->sanitizeInputNodes($nodes), $this->userId, 0, + $menuItems !== null ? MenuItemInput::createListFromInputArray($menuItems) : null, )->jsonSerialize()); } catch (Exception $e) { return $this->handleError($e); } catch (PermissionError $e) { return $this->handlePermissionError($e); } catch (InvalidArgumentException $e) { - return $this->handleError(new InternalError($e->getMessage(), $e->getCode(), $e)); + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); } } @@ -138,6 +141,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 * @@ -149,7 +153,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); @@ -162,6 +166,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); @@ -169,6 +174,8 @@ 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 (InvalidArgumentException $e) { + return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e)); } } diff --git a/lib/Model/MenuItemInput.php b/lib/Model/MenuItemInput.php new file mode 100644 index 0000000000..e8e80906f2 --- /dev/null +++ b/lib/Model/MenuItemInput.php @@ -0,0 +1,151 @@ +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 && mb_strlen($this->icon) > self::MAX_ICON_LENGTH) { + throw new InvalidArgumentException('A menu item icon name must not exceed ' . self::MAX_ICON_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->targetId === null || $this->targetId <= 0) { + throw new InvalidArgumentException('A menu item pointing at a ' . $this->targetType . ' needs its id.'); + } + if ($this->technicalName !== null) { + self::assertTechnicalNameValid($this->technicalName); + } + } + + /** + * @throws InvalidArgumentException + */ + public static function createFromInputArray(mixed $data): 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; + if (!$isUrl && !is_int($targetId)) { + throw new InvalidArgumentException('A menu item pointing at a ' . $targetType . ' needs its id.'); + } + foreach (['icon', '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), + targetType: $targetType, + targetId: $isUrl ? null : $targetId, + url: $isUrl ? trim((string)($data['url'] ?? '')) : null, + technicalName: self::emptyToNull($data['technicalName'] ?? null), + ); + } + + /** + * @return list + * @throws InvalidArgumentException + */ + public static function createListFromInputArray(mixed $data): array { + if (!is_array($data) || !array_is_list($data)) { + throw new InvalidArgumentException('The menu items must be a list.'); + } + return array_map(self::createFromInputArray(...), $data); + } + + /** + * @throws InvalidArgumentException + */ + public static function assertTechnicalNameValid(string $technicalName): void { + if (strlen($technicalName) > self::MAX_TECHNICAL_NAME_LENGTH || !preg_match(self::TECHNICAL_NAME_PATTERN, $technicalName)) { + throw new InvalidArgumentException('A technical name must start with a lowercase letter and contain only lowercase letters, numbers and underscores.'); + } + } + + /** + * @throws InvalidArgumentException + */ + private static function assertUrlValid(?string $url): void { + $scheme = $url === null ? null : parse_url($url, PHP_URL_SCHEME); + if ($url === null || mb_strlen($url) > self::MAX_URL_LENGTH + || !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 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/Service/ContextService.php b/lib/Service/ContextService.php index bf1ba0a906..33bc18e9a4 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -14,6 +14,7 @@ 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; @@ -29,6 +30,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 OCP\AppFramework\Db\DoesNotExistException; @@ -133,9 +135,10 @@ public function findById(int $id, ?string $userId): Context { /** * @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)); @@ -143,7 +146,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)) { @@ -151,18 +154,247 @@ public function create(string $name, string $iconName, string $description, arra $this->insertNodesFromArray($context, $nodes); } $this->insertPage($context); + $this->replaceMenu($context, $menuItems ?? [], $ownerId); }, $this->dbc); return $context; } + /** + * Replaces the menu of an application and adds the views and tables it points at + * as nodes, so everyone the application is shared with can open them. Removing an + * item leaves its view or table in the application: it stays a resource until it is + * removed as one. + * + * @param list $menuItems + * @throws Exception|PermissionError|InvalidArgumentException + */ + protected function replaceMenu(Context $context, array $menuItems, ?string $userId): void { + $storedMenu = array_values(array_filter($context->getMenuItems() ?? [], is_array(...))); + $this->addMenuTargetsAsNodes($context, $menuItems, $storedMenu, $userId); + + $this->menuItemMapper->deleteAllByContextId($context->getId()); + $technicalNames = $this->menuTechnicalNames($menuItems, $storedMenu); + foreach ($menuItems as $position => $item) { + $menuItem = new MenuItem(); + $menuItem->setContextId($context->getId()); + $menuItem->setLabel($item->getLabel()); + $menuItem->setIcon($item->getIcon()); + $menuItem->setTargetType($item->getTargetType()); + $menuItem->setTargetId($item->getTargetId()); + $menuItem->setUrl($item->getUrl()); + $menuItem->setTechnicalName($technicalNames[$position]); + $menuItem->setOrder(($position + 1) * 10); + $this->menuItemMapper->insert($menuItem); + } + + $context->setMenuItems(array_map( + static fn (MenuItem $menuItem): array => $menuItem->jsonSerialize(), + $this->menuItemMapper->findByContextId($context->getId()), + )); + } + + /** + * The technical name of every menu item, by position: the one given; else the stored name of + * an item with the same target, so a rename or a reorder keeps it; else one derived from the + * label, with a numeric suffix when this menu already uses it. + * + * @param list $menuItems + * @param list> $storedMenu + * @return array + * @throws InvalidArgumentException + */ + private function menuTechnicalNames(array $menuItems, array $storedMenu): array { + $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 . '".'); + } + $names[$position] = $technicalName; + $taken[$technicalName] = true; + } + $storedNames = $this->storedTechnicalNamesByTarget($storedMenu); + 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; + } + } + } + foreach ($menuItems as $position => $item) { + if (isset($names[$position])) { + continue; + } + $base = $this->technicalNameFromLabel($item->getLabel()); + $technicalName = $base; + for ($suffix = 2; isset($taken[$technicalName]); $suffix++) { + $technicalName = substr($base, 0, MenuItemInput::MAX_TECHNICAL_NAME_LENGTH - 10) . '_' . $suffix; + } + $names[$position] = $technicalName; + $taken[$technicalName] = true; + } + return $names; + } + + /** + * The technical names of the stored menu, grouped by target and in menu order. + * + * @param list> $storedMenu + * @return array> + */ + private function storedTechnicalNamesByTarget(array $storedMenu): array { + $names = []; + 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; + $names[self::menuTargetIdentity($targetType, $targetId, $url)][] = $technicalName; + } + 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]]; + } + + /** + * A new target must exist and be manageable by the user, and every unusable one gets the + * same answer, so the save reveals nothing about others' views and tables. A target that was + * in the stored menu before and no longer exists is kept; the menu marks it broken. + * + * @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['nodeType'], $node['node_id'] ?? $node['nodeId'])]); + } + 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 = null; + foreach ($pages as $pageId => $page) { + if (($page['page_type'] ?? $page['pageType'] ?? null) === Page::TYPE_STARTPAGE) { + $startPageId = (int)$pageId; + break; + } + } + + 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()] = [ + 'id' => $relation->getId(), + 'node_id' => $nodeId, + 'node_type' => $nodeType, + 'permissions' => Application::PERMISSION_READ, + ]; + if ($startPageId !== null) { + $content = $this->addNodeRelToPage($relation, null, $startPageId); + $pages[$startPageId]['content'][$content->getId()] = [ + 'order' => $content->getOrder(), + 'node_rel_id' => $relation->getId(), + ]; + } + } + $context->setNodes($nodes); + $context->setPages($pages); + } + + protected function technicalNameFromLabel(string $label): string { + $name = strtolower(trim(preg_replace('/[^a-z0-9]+/i', '_', $label) ?? '', '_')); + $name = preg_replace('/^[^a-z]+/', '', $name) ?? ''; + return substr($name === '' ? 'item' : $name, 0, MenuItemInput::MAX_TECHNICAL_NAME_LENGTH); + } + /** * @psalm-param list $nodes + * @param list|null $menuItems * @throws Exception * @throws DoesNotExistException - * @throws PermissionError|MultipleObjectsReturnedException + * @throws PermissionError|MultipleObjectsReturnedException|InvalidArgumentException */ - public function update(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes): Context { + 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 a write lock to a concurrent request + // (SQLite answers "database is locked" at once); running it again then succeeds + return $this->atomicRetry( + fn (): Context => $this->applyUpdate($contextId, $userId, $name, $iconName, $description, $nodes, $menuItems), + $this->dbc, + ); + } + + /** + * @psalm-param list $nodes + * @param list|null $menuItems + * @throws Exception + * @throws DoesNotExistException + * @throws PermissionError|MultipleObjectsReturnedException|InvalidArgumentException + */ + private function applyUpdate(int $contextId, string $userId, ?string $name, ?string $iconName, ?string $description, ?array $nodes, ?array $menuItems): Context { $context = $this->contextMapper->findById($contextId, $userId); if ($name !== null) { @@ -306,6 +538,9 @@ public function update(int $contextId, string $userId, ?string $name, ?string $i $context->setNodes($currentNodes); $context->setPages($currentPages); } + if ($menuItems !== null) { + $this->replaceMenu($context, $menuItems, $userId); + } return $context; } diff --git a/openapi.json b/openapi.json index a73b8f9bc5..7d9eb643a1 100644 --- a/openapi.json +++ b/openapi.json @@ -12076,6 +12076,45 @@ } } } + }, + "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", + "items": { + "type": "object", + "required": [ + "label", + "targetType" + ], + "properties": { + "label": { + "type": "string" + }, + "icon": { + "type": "string" + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url" + ] + }, + "targetId": { + "type": "integer", + "format": "int64" + }, + "url": { + "type": "string" + }, + "technicalName": { + "type": "string" + } + } + } } } } @@ -12508,6 +12547,45 @@ "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", + "targetType" + ], + "properties": { + "label": { + "type": "string" + }, + "icon": { + "type": "string" + }, + "targetType": { + "type": "string", + "enum": [ + "view", + "table", + "url" + ] + }, + "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 e82cdb0ab0..933664d25e 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -6919,6 +6919,20 @@ 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 + * @default null + */ + readonly menuItems?: readonly { + readonly label: string; + readonly icon?: string; + /** @enum {string} */ + readonly targetType: "view" | "table" | "url"; + /** Format: int64 */ + readonly targetId?: number; + readonly url?: string; + readonly technicalName?: string; + }[] | null; }; }; }; @@ -7109,6 +7123,20 @@ 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; + /** @enum {string} */ + readonly targetType: "view" | "table" | "url"; + /** Format: int64 */ + readonly targetId?: number; + readonly url?: string; + readonly technicalName?: string; + }[] | null; }; }; }; diff --git a/tests/unit/Model/MenuItemInputTest.php b/tests/unit/Model/MenuItemInputTest.php new file mode 100644 index 0000000000..fe7a667aac --- /dev/null +++ b/tests/unit/Model/MenuItemInputTest.php @@ -0,0 +1,74 @@ + ' Intake home ', 'targetType' => 'view', 'targetId' => 6, 'technicalName' => 'home', 'url' => 'https://ignored.example'], + ['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'], $this->toArray($items[0])); + $this->assertSame(['Docs', 'book', 'url', null, 'https://example.org/docs', null], $this->toArray($items[1])); + $this->assertSame(['Requests', null, 'table', 4, 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'], + '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'], + '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'], + ]; + } + + /** + * @dataProvider invalidItems + */ + public function testRejectsInvalidInput(mixed $input, string $message): void { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + MenuItemInput::createFromInputArray($input); + } + + 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()]; + } +} diff --git a/tests/unit/Service/ContextServiceTest.php b/tests/unit/Service/ContextServiceTest.php index 9369e5f9e7..520ac51f7d 100644 --- a/tests/unit/Service/ContextServiceTest.php +++ b/tests/unit/Service/ContextServiceTest.php @@ -9,16 +9,25 @@ namespace OCA\Tables\Service; +use InvalidArgumentException; +use OC\DB\Exceptions\DbalException; +use OCA\Tables\AppInfo\Application; 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 OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use OCP\INavigationManager; @@ -35,6 +44,9 @@ final class ContextServiceTest extends TestCase { private MockObject $pageMapper; private MockObject $pageContentMapper; private MockObject $menuItemMapper; + private MockObject $tableMapper; + private MockObject $viewMapper; + private MockObject $permissionsService; protected function setUp(): void { parent::setUp(); @@ -44,8 +56,10 @@ protected function setUp(): void { $this->pageMapper = $this->createMock(PageMapper::class); $this->pageContentMapper = $this->createMock(PageContentMapper::class); $this->menuItemMapper = $this->createMock(MenuItemMapper::class); + $this->tableMapper = $this->createMock(TableMapper::class); + $this->viewMapper = $this->createMock(ViewMapper::class); + $this->permissionsService = $this->createMock(PermissionsService::class); $logger = $this->createMock(LoggerInterface::class); - $permissionsService = $this->createMock(PermissionsService::class); $userManager = $this->createMock(IUserManager::class); $eventDispatcher = $this->createMock(IEventDispatcher::class); $dbConnection = $this->createMock(IDBConnection::class); @@ -59,7 +73,7 @@ protected function setUp(): void { $this->pageMapper, $this->pageContentMapper, $logger, - $permissionsService, + $this->permissionsService, $userManager, $eventDispatcher, $dbConnection, @@ -67,12 +81,245 @@ protected function setUp(): void { false, $navigationManager, $urlGenerator, - $this->createMock(TableMapper::class), - $this->createMock(ViewMapper::class), + $this->tableMapper, + $this->viewMapper, $this->menuItemMapper, ); } + 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->setId(count($inserted) + 1); + $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->setId(200 + count($relations)); + $relations[] = $relation; + return $relation; + }); + } + + public function testUpdateReplacesTheMenuInOrderAndDerivesTechnicalNamesFromLabels(): 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', 'welcome_table', 'docs'], 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->setId(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()]); + $this->assertSame(['id' => 200, 'node_id' => 6, 'node_type' => Application::NODE_TYPE_VIEW, 'permissions' => Application::PERMISSION_READ], $updated->getNodes()[200]); + $this->assertSame(200, $updated->getPages()[301]['content'][501]['node_rel_id']); + } + + 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', 'docs_3'], array_map(static fn (MenuItem $item) => $item->getTechnicalName(), $inserted)); + } + + public function testDerivedTechnicalNamesGetASuffixWhenTheMenuAlreadyUsesThem(): 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(['docs', 'docs_3', '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 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->setId(8); + return $context; + }); + $this->pageMapper->method('insert')->willReturnCallback(static function (Page $page): Page { + $page->setId(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'], + ])); + + $this->assertSame('desc', $created->getDescription()); + $this->assertCount(1, $created->getMenuItems()); + $this->assertSame('docs', $inserted[0]->getTechnicalName()); + } + public function testDeleteRemovesTheMenuOfTheApplication(): void { $context = new Context(); $context->id = 7;