From a5dff57999d015919d99066a774a41d9def2fd69 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 22:43:45 +0200 Subject: [PATCH 1/6] fix(security): bump phpcsstandards/phpcsutils to 1.2.3 for CVE-2026-65954 phpcsstandards/phpcsutils < 1.2.3 carries CVE-2026-65954 (arbitrary code execution, GHSA-r6hr-vr92-vv28, affected >=1.0.0-alpha1,<1.2.3). The advisory was published today, so composer audit turns red on a lock file that has not changed. The Security (composer) job in run 31516037194 passed at 17:25Z only because it ran before the advisory landed. Negative control before the bump, on this tree: composer audit --locked -> Found 1 security vulnerability advisory affecting 1 package phpcsstandards/phpcsutils / CVE-2026-65954 / exit 1 After 'composer update phpcsstandards/phpcsutils --no-install --no-scripts' (1.2.2 => 1.2.3, a lock-only change, 0 installs 0 removals): composer audit --locked -> No security vulnerability advisories found / exit 0 The bump is exercised rather than merely locked: phpcs runs green against the new library on PHP 8.4 -- 0 ERRORS AND 468 WARNINGS IN 211 FILES, exit 0 so no sniff regressed on the upgrade. --- composer.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/composer.lock b/composer.lock index 3ed64231..9fc97b2b 100644 --- a/composer.lock +++ b/composer.lock @@ -2597,16 +2597,16 @@ }, { "name": "phpcsstandards/phpcsutils", - "version": "1.2.2", + "version": "1.2.3", "source": { "type": "git", "url": "https://github.com/PHPCSStandards/PHPCSUtils.git", - "reference": "c216317e96c8b3f5932808f9b0f1f7a14e3bbf55" + "reference": "5f35d9408c54d7b529501f3c688b6eae562aea1f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/PHPCSStandards/PHPCSUtils/zipball/c216317e96c8b3f5932808f9b0f1f7a14e3bbf55", - "reference": "c216317e96c8b3f5932808f9b0f1f7a14e3bbf55", + "url": "https://api.github.com/repos/PHPCSStandards/PHPCSUtils/zipball/5f35d9408c54d7b529501f3c688b6eae562aea1f", + "reference": "5f35d9408c54d7b529501f3c688b6eae562aea1f", "shasum": "" }, "require": { @@ -2686,7 +2686,7 @@ "type": "thanks_dev" } ], - "time": "2025-12-08T14:27:58+00:00" + "time": "2026-07-27T10:28:41+00:00" }, { "name": "phpdocumentor/reflection-common", From 23127829db0567bbcdec42cf04e0606189c11b6a Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 22:55:20 +0200 Subject: [PATCH 2/6] refactor: retire the inert groupfolder-storage-backend capability (closes #87, gate-57 1 -> 0) gate-57 (orphaned-write-capability) reported exactly one finding on this repo: lib/Service/DashboardService.php:3189 method=writeDashboardContent rule=orphaned-write-capability class=DashboardService It is a true positive and the real scope is larger than one method. Task 5 of the original change -- the wiring that would have routed dashboard get/create/update/delete through the storage factory -- was ticked in tasks.md but never performed. Everything around it landed, which is why the capability looked complete from every angle except the one that mattered. Measured on this tree, every absence claim taken with a positive control (the same search resolves callers for getUserDashboards and ->findByUuid): DashboardService::writeDashboardContent() 0 callers DashboardService::readDashboardContent() 0 callers DashboardService::deleteDashboardContent() 0 callers DashboardContentStorageFactory::getStorage() 0 (only the three above) DashboardApiController::storageUnavailableResponse() 0 callers DashboardMapper::findAll() 1 -- the migration command So nothing ever populated oc_launchpad_dashboards.content, Dashboard:: jsonSerialize() emitted a content key that was always null, launchpad. content_storage was a no-op setting, and launchpad:storage:migrate-to-groupfolder migrated nothing. Retired rather than completed, per Ruben's decision on #87: finishing it needs an ADR-level ruling on whether the content blob or the WidgetPlacement rows are the source of truth for a dashboard's layout. The spec asserts the blob, the code has used the rows since long before this capability existed, and wiring the blob in as a second source of truth invites divergence. Nobody uses it, so retiring costs nothing and removes that risk permanently. Removed: lib/Service/DashboardContentStorage/ (interface, both backends, three exceptions), DashboardContentStorageFactory, the three DashboardService facade methods and the factory constructor parameter, storageUnavailableResponse(), DashboardMapper::findAll(), both CLI commands and their registrations, the $content/$locale properties with decodeContent() and their jsonSerialize() keys, and the five PHPUnit classes covering the removed tree. Version002009Date20260811000000 drops the content and locale columns. That loses no data: the only writer, DbContentStorage::write(), was reachable only through the factory, which was reachable only through the three orphaned facade methods, so the columns were NULL on every row of every install. The step is idempotent (hasColumn-guarded). Version002001Date20260603000000 and DashboardTableBuilder::addContentStorageColumns() are deliberately KEPT -- a shipped migration is part of the version ledger and deleting it would desynchronise instances that have already run it. Deliberately NOT removed, and tracked as Task 8 on #87: launchpad.content_storage, AdminSettingKey::CONTENT_STORAGE, SetupWizardService::{get,set}ContentStorage / hasGroupfolderApp, AdminController::setWizardStorage and setup-wizard step 2. Those belong to the setup-wizard capability, which WRITES the setting; this capability was the READER. The setting was already inert before this change, so removing the reader does not make it worse, and retiring the wizard step edits a different spec and deserves its own review. The spec is marked withdrawn rather than deleted, which also retires its 16 @e2e exclude markers -- their stated reasons cited DbContentStorageTest.php, a file this change deletes, so they would otherwise have become sixteen exemptions resting on a class that no longer exists. Verification, all on this tree: gate-57 over lib/Service/*.php 1 -> 0 planted true positive (postGate57Probe, a tracked write method with no caller) 0 -> 1, naming exactly the plant plant removed 1 -> 0 PHPUnit 1570 tests, 4032 assertions, 0 failures, 3 pre-existing skips phpcs 0 errors, 468 warnings, 211 files -- identical to the pre-change baseline check_spec_anchors.py over lib/ 0 dangling anchors gate-19 unchanged at 77 (base origin/beta) NOTE for anyone reproducing gate-57: its checker takes a FILE LIST, not a directory. `check_orphaned_write_capability.py .` prints nothing and exits 0 whatever the tree contains, which reads exactly like a clean repo. --- appinfo/info.xml | 4 - lib/Command/MigrateStorageToGroupFolder.php | 212 -------- lib/Command/ToggleStorageSetting.php | 156 ------ lib/Controller/DashboardApiController.php | 33 -- lib/Db/Dashboard.php | 56 --- lib/Db/DashboardMapper.php | 20 - .../Version002009Date20260811000000.php | 94 ++++ .../DashboardContentStorageException.php | 35 -- .../DashboardContentStorageInterface.php | 103 ---- .../DashboardNotFoundException.php | 33 -- .../DbContentStorage.php | 258 ---------- .../GroupFolderContentStorage.php | 400 --------------- .../GroupFoldersNotInstalledException.php | 47 -- .../DashboardContentStorageFactory.php | 79 --- lib/Service/DashboardService.php | 378 ++++++--------- .../tasks.md | 2 +- .../proposal.md | 52 ++ .../tasks.md | 18 + .../specs/groupfolder-storage-backend/spec.md | 454 ++++-------------- .../MigrateStorageToGroupFolderTest.php | 254 ---------- .../DashboardContentStorageExceptionTest.php | 89 ---- .../DbContentStorageTest.php | 249 ---------- .../GroupFolderContentStorageTest.php | 238 --------- .../DashboardContentStorageFactoryTest.php | 133 ----- 24 files changed, 393 insertions(+), 3004 deletions(-) delete mode 100644 lib/Command/MigrateStorageToGroupFolder.php delete mode 100644 lib/Command/ToggleStorageSetting.php create mode 100644 lib/Migration/Version002009Date20260811000000.php delete mode 100644 lib/Service/DashboardContentStorage/DashboardContentStorageException.php delete mode 100644 lib/Service/DashboardContentStorage/DashboardContentStorageInterface.php delete mode 100644 lib/Service/DashboardContentStorage/DashboardNotFoundException.php delete mode 100644 lib/Service/DashboardContentStorage/DbContentStorage.php delete mode 100644 lib/Service/DashboardContentStorage/GroupFolderContentStorage.php delete mode 100644 lib/Service/DashboardContentStorage/GroupFoldersNotInstalledException.php delete mode 100644 lib/Service/DashboardContentStorageFactory.php create mode 100644 openspec/changes/retire-groupfolder-storage-backend/proposal.md create mode 100644 openspec/changes/retire-groupfolder-storage-backend/tasks.md delete mode 100644 tests/Unit/Command/MigrateStorageToGroupFolderTest.php delete mode 100644 tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php delete mode 100644 tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php delete mode 100644 tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php delete mode 100644 tests/Unit/Service/DashboardContentStorageFactoryTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 04973454..fb06fe3f 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -157,10 +157,6 @@ Vrij en open source onder de EUPL-1.2-licentie. OCA\LaunchPad\Command\DemoShowcasesListCommand OCA\LaunchPad\Command\SetupCommand - - OCA\LaunchPad\Command\MigrateStorageToGroupFolder - - OCA\LaunchPad\Command\ToggleStorageSetting diff --git a/lib/Command/MigrateStorageToGroupFolder.php b/lib/Command/MigrateStorageToGroupFolder.php deleted file mode 100644 index 31406e27..00000000 --- a/lib/Command/MigrateStorageToGroupFolder.php +++ /dev/null @@ -1,212 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Command; - -use OCA\LaunchPad\Db\DashboardMapper; -use OCA\LaunchPad\Service\CommandService; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageException; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFolderContentStorage; -use OCP\IUserSession; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; - -/** - * One-time migration command from DB to GroupFolder storage (REQ-GFSB-008). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ -class MigrateStorageToGroupFolder extends CommandBase -{ - /** - * Constructor. - * - * @param CommandService $commandService Shared CLI helper. - * @param IUserSession $userSession Caller resolution. - * @param DashboardMapper $dashboardMapper Dashboard mapper. - * @param GroupFolderContentStorage $groupFolderStorage GroupFolder backend. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ - public function __construct( - CommandService $commandService, - IUserSession $userSession, - private readonly DashboardMapper $dashboardMapper, - private readonly GroupFolderContentStorage $groupFolderStorage, - ) { - parent::__construct( - commandService: $commandService, - userSession: $userSession - ); - }//end __construct() - - /** - * Wire command name, description, and options. - * - * @return void - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ - protected function configureCommand(): void - { - $this->setName(name: 'launchpad:storage:migrate-to-groupfolder') - ->setDescription(description: 'Migrate dashboard content from DB to GroupFolder (REQ-GFSB-008).') - ->setHelp( - help: implode( - separator: "\n", - array: [ - 'Copies all dashboard content blobs from the `content` column in', - '`launchpad_dashboards` to the configured GroupFolder backend.', - '', - 'The command is idempotent: dashboards already present in the', - 'GroupFolder are skipped. Re-run safely after partial failures.', - '', - 'After a successful migration, switch the active backend via:', - ' php occ launchpad:storage:toggle-backend groupfolder', - '', - 'Use --prune-source to remove DB content for successfully migrated', - 'dashboards (default: DB content is kept for rollback safety).', - '', - 'Run launchpad:storage:migrate-to-groupfolder --help for more details.', - ] - ) - ) - ->addOption( - name: 'prune-source', - shortcut: null, - mode: InputOption::VALUE_NONE, - description: 'Remove DB content for successfully migrated dashboards (REQ-GFSB-008 design D3).' - ); - }//end configureCommand() - - /** - * Execute the migration. - * - * @param InputInterface $input CLI input. - * @param OutputInterface $output CLI output. - * - * @return int Exit code (0 = success, 1 = partial failure). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ - protected function handle(InputInterface $input, OutputInterface $output): int - { - $pruneSource = (bool) $input->getOption(name: 'prune-source'); - - $dashboards = $this->dashboardMapper->findAll(); - $total = count($dashboards); - - if ($total === 0) { - $output->writeln(messages: 'No dashboards found — nothing to migrate.'); - return CommandService::EXIT_SUCCESS; - } - - $migrated = 0; - $skipped = 0; - $errors = 0; - - foreach ($dashboards as $i => $dashboard) { - $uuid = (string) $dashboard->getUuid(); - $current = (int) $i + 1; - - if ($uuid === '') { - $output->writeln( - messages: " [{$current}/{$total}] Dashboard id={$dashboard->getId()} has no UUID, skipping." - ); - $skipped++; - continue; - } - - // Idempotent: skip when content already exists in GroupFolder. - if ($this->groupFolderStorage->exists(uuid: $uuid) === true) { - $output->writeln( - messages: " [{$current}/{$total}] {$uuid} already migrated, skipping." - ); - $skipped++; - continue; - } - - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $rawContent = $dashboard->getContent(); - // phpcs:enable - - $contentArray = self::decodeContent(rawContent: $rawContent); - - try { - $this->groupFolderStorage->write(uuid: $uuid, content: $contentArray); - $migrated++; - $output->writeln(messages: " [{$current}/{$total}] Migrated {$uuid}."); - - // Optionally clear DB content after successful migration (design D3). - if ($pruneSource === true) { - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $dashboard->setContent(null); - // phpcs:enable - $this->dashboardMapper->update(entity: $dashboard); - } - } catch (DashboardContentStorageException $e) { - $errors++; - $output->writeln( - messages: " [{$current}/{$total}] ERROR for {$uuid}: ".$e->getMessage() - ); - }//end try - }//end foreach - - $output->writeln( - messages: "\nMigration complete: {$migrated}/{$total} migrated, {$skipped} skipped, {$errors} errors." - ); - - if ($errors > 0) { - return CommandService::EXIT_ERROR; - } - - return CommandService::EXIT_SUCCESS; - }//end handle() - - /** - * Decode a dashboard's stored content blob into an array. - * - * A missing, empty, or non-array-decoding blob yields an empty array - * so the GroupFolder write always receives a well-formed payload - * instead of failing on legacy or corrupt rows. - * - * @param string|null $rawContent The raw `content` column value. - * - * @return array The decoded content, or an empty array. - */ - private static function decodeContent(?string $rawContent): array - { - if ($rawContent === null || $rawContent === '') { - return []; - } - - $decoded = json_decode(json: $rawContent, associative: true); - if (is_array($decoded) === false) { - return []; - } - - return $decoded; - }//end decodeContent() -}//end class diff --git a/lib/Command/ToggleStorageSetting.php b/lib/Command/ToggleStorageSetting.php deleted file mode 100644 index 0156781e..00000000 --- a/lib/Command/ToggleStorageSetting.php +++ /dev/null @@ -1,156 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-10 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Command; - -use OCA\LaunchPad\Service\CommandService; -use OCA\LaunchPad\Service\SetupWizardService; -use OCP\IUserSession; -use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; - -/** - * Toggle the content storage backend via the CLI (REQ-GFSB-010). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-10 - */ -class ToggleStorageSetting extends CommandBase -{ - /** - * Constructor. - * - * @param CommandService $commandService Shared CLI helper. - * @param IUserSession $userSession Caller resolution. - * @param SetupWizardService $wizardService Admin setting writer. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-10 - */ - public function __construct( - CommandService $commandService, - IUserSession $userSession, - private readonly SetupWizardService $wizardService, - ) { - parent::__construct( - commandService: $commandService, - userSession: $userSession - ); - }//end __construct() - - /** - * Wire command name, description, and arguments. - * - * @return void - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-10 - */ - protected function configureCommand(): void - { - $this->setName(name: 'launchpad:storage:toggle-backend') - ->setDescription(description: 'Change the active content storage backend (db|groupfolder).') - ->setHelp( - help: implode( - separator: "\n", - array: [ - 'Changes the `launchpad.content_storage` admin setting.', - '', - 'Valid backends: db, groupfolder', - '', - ' db — Store dashboard content in the database (default).', - ' groupfolder — Store dashboard content in the "LaunchPad" GroupFolder.', - '', - 'WARNING: Switching back from groupfolder to db does NOT auto-copy', - 'GroupFolder data back to the database. Run the migration first or', - 'ensure DB content is intact (migration keeps DB copies by default).', - '', - 'Run php occ launchpad:storage:migrate-to-groupfolder before switching', - 'to groupfolder to ensure all existing dashboards are available.', - ] - ) - ) - ->addArgument( - name: 'backend', - mode: InputArgument::REQUIRED, - description: 'Target backend: "db" or "groupfolder".' - ); - }//end configureCommand() - - /** - * Execute the backend toggle. - * - * @param InputInterface $input CLI input. - * @param OutputInterface $output CLI output. - * - * @return int Exit code (0 = success, 1 = invalid argument). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-10 - */ - protected function handle(InputInterface $input, OutputInterface $output): int - { - $requested = (string) $input->getArgument(name: 'backend'); - $current = $this->wizardService->getContentStorage(); - - // Map CLI aliases to internal constants. - $targetMap = [ - 'db' => SetupWizardService::STORAGE_DATABASE, - 'database' => SetupWizardService::STORAGE_DATABASE, - 'groupfolder' => SetupWizardService::STORAGE_GROUPFOLDER, - ]; - - if (array_key_exists(key: $requested, array: $targetMap) === false) { - $output->writeln( - messages: 'Invalid backend: "'.$requested.'". Use "db" or "groupfolder".' - ); - return CommandService::EXIT_ERROR; - } - - $target = $targetMap[$requested]; - - if ($target === $current) { - $output->writeln(messages: 'Backend is already set to "'.$current.'". No change.'); - return CommandService::EXIT_SUCCESS; - } - - // Warn when switching back from groupfolder to DB. - if ($current === SetupWizardService::STORAGE_GROUPFOLDER - && $target === SetupWizardService::STORAGE_DATABASE - ) { - $output->writeln( - messages: implode( - separator: "\n", - array: [ - 'WARNING: Switching from groupfolder to db.', - 'GroupFolder content is NOT auto-copied back to the database.', - 'Ensure database content is intact before proceeding.', - ] - ) - ); - } - - $this->wizardService->setContentStorage(value: $target); - - $output->writeln(messages: 'Storage backend changed from "'.$current.'" to "'.$target.'".'); - return CommandService::EXIT_SUCCESS; - }//end handle() -}//end class diff --git a/lib/Controller/DashboardApiController.php b/lib/Controller/DashboardApiController.php index 8af60c55..a8cefc1d 100644 --- a/lib/Controller/DashboardApiController.php +++ b/lib/Controller/DashboardApiController.php @@ -29,7 +29,6 @@ use OCA\LaunchPad\Exception\PersonalDashboardsDisabledException; use OCA\LaunchPad\Exception\QuotaExceededException; use OCA\LaunchPad\Service\ActionAuthService; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageException; use OCA\LaunchPad\Service\AnalyticsService; use OCA\LaunchPad\Service\DashboardService; use OCA\LaunchPad\Service\DashboardTreeService; @@ -1954,36 +1953,4 @@ private function captureAutomaticSnapshot( ); } }//end captureAutomaticSnapshot() - - /** - * Build a HTTP 503 response for a storage backend failure (REQ-GFSB-007). - * - * The error key `dashboard_content_storage_unavailable` is the stable - * identifier callers MUST treat as a signal to surface an actionable - * message ("Run the migration command or check GroupFolder availability"). - * - * @param DashboardContentStorageException $e The caught exception. - * - * @return JSONResponse HTTP 503 with the standard error envelope. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-11 - */ - protected function storageUnavailableResponse( - DashboardContentStorageException $e - ): JSONResponse { - $this->logger->warning( - message: 'launchpad: dashboard content storage unavailable', - context: ['message' => $e->getMessage(), 'exception' => $e] - ); - - return new JSONResponse( - data: [ - 'error' => 'dashboard_content_storage_unavailable', - 'message' => 'The dashboard content storage backend is unavailable. ' - .'If you recently changed the backend, run: ' - .'php occ launchpad:storage:migrate-to-groupfolder', - ], - statusCode: Http::STATUS_SERVICE_UNAVAILABLE - ); - }//end storageUnavailableResponse() }//end class diff --git a/lib/Db/Dashboard.php b/lib/Db/Dashboard.php index 3446fa43..c1de00a1 100644 --- a/lib/Db/Dashboard.php +++ b/lib/Db/Dashboard.php @@ -83,10 +83,6 @@ * @method void setTemplateDescription(?string $templateDescription) * @method string|null getTemplatePreviewImage() * @method void setTemplatePreviewImage(?string $templatePreviewImage) - * @method string|null getContent() - * @method void setContent(?string $content) - * @method string|null getLocale() - * @method void setLocale(?string $locale) * * @SuppressWarnings(PHPMD.TooManyFields) Each field maps to a documented * column on `oc_launchpad_dashboards` @@ -591,30 +587,6 @@ class Dashboard extends Entity implements JsonSerializable */ protected ?string $templatePreviewImage = null; - /** - * The serialized dashboard content JSON (REQ-GFSB-002). - * - * Populated when the `db` storage backend is active; NULL when the - * `groupfolder` backend is active (content lives in the GroupFolder - * file system in that case). `jsonSerialize()` always surfaces this - * field (possibly null) so the API contract is stable regardless of - * the active backend. - * - * @var string|null - */ - protected ?string $content = null; - - /** - * Optional locale code associated with the dashboard content (REQ-GFSB-004). - * - * Used by the GroupFolder backend to route reads/writes to the - * locale-specific sub-path `LaunchPad//.json`. NULL - * means locale-neutral storage. - * - * @var string|null - */ - protected ?string $locale = null; - /** * Constructor * @@ -718,37 +690,9 @@ public function jsonSerialize(): array 'templateCategory' => $this->templateCategory, 'templateDescription' => $this->templateDescription, 'templatePreviewImage' => $this->templatePreviewImage, - // REQ-GFSB-002: content field; null when GroupFolder backend is active. - 'content' => $this->decodeContent(), - 'locale' => $this->locale, ]; }//end jsonSerialize() - /** - * Decode the raw `content` JSON string for API serialisation (REQ-GFSB-002). - * - * Returns null when the column is null or empty; returns the decoded array - * when content is valid JSON; returns null and degrades gracefully when - * the stored value is corrupt. - * - * @return array|null The decoded content, or null. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-6 - */ - private function decodeContent(): ?array - { - if ($this->content === null || $this->content === '') { - return null; - } - - $decoded = json_decode(json: $this->content, associative: true); - if (is_array($decoded) === true) { - return $decoded; - } - - return null; - }//end decodeContent() - /** * Serialize for a non-owner viewer (M5 — strips internal identity * fields from the public envelope). diff --git a/lib/Db/DashboardMapper.php b/lib/Db/DashboardMapper.php index e3dd2fd8..87946702 100644 --- a/lib/Db/DashboardMapper.php +++ b/lib/Db/DashboardMapper.php @@ -1128,26 +1128,6 @@ public function findOwnedByUserAndUuid( return Dashboard::fromRow($row); }//end findOwnedByUserAndUuid() - /** - * Return every dashboard row without filtering. - * - * Used by the storage-migration command (REQ-GFSB-008) to enumerate all - * dashboards regardless of type, owner, or publication state. - * - * @return Dashboard[] All dashboard entities. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-9 - */ - public function findAll(): array - { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from(from: $this->getTableName()) - ->orderBy(sort: 'id', order: 'ASC'); - - return $this->findEntities(query: $qb); - }//end findAll() - /** * Clear default flag on all admin templates. * diff --git a/lib/Migration/Version002009Date20260811000000.php b/lib/Migration/Version002009Date20260811000000.php new file mode 100644 index 00000000..fe83c61a --- /dev/null +++ b/lib/Migration/Version002009Date20260811000000.php @@ -0,0 +1,94 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @link https://conduction.nl + * + * @spec openspec/changes/retire-groupfolder-storage-backend/tasks.md#task-3 + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Drop the withdrawn content-storage columns from launchpad_dashboards. + * + * @spec openspec/changes/retire-groupfolder-storage-backend/tasks.md#task-3 + */ +class Version002009Date20260811000000 extends SimpleMigrationStep +{ + /** + * Alter the database schema to drop the content storage columns. + * + * Each column is dropped only when present, so the step is idempotent and + * safe on an install that never ran Version002001Date20260603000000. + * + * @param IOutput $output Migration output. + * @param Closure $schemaClosure Schema closure (provides ISchemaWrapper). + * @param array $options Migration options (unused). + * + * @return ISchemaWrapper|null The modified schema, or null when unchanged. + * + * @spec openspec/changes/retire-groupfolder-storage-backend/tasks.md#task-3 + */ + public function changeSchema( + IOutput $output, + Closure $schemaClosure, + array $options + ): ?ISchemaWrapper { + // @var ISchemaWrapper $schema. + $schema = $schemaClosure(); + + if ($schema->hasTable('launchpad_dashboards') === false) { + return null; + } + + $table = $schema->getTable('launchpad_dashboards'); + $changed = false; + + foreach (['content', 'locale'] as $column) { + if ($table->hasColumn($column) === true) { + $table->dropColumn($column); + $changed = true; + } + } + + if ($changed === false) { + return null; + } + + return $schema; + }//end changeSchema() +}//end class diff --git a/lib/Service/DashboardContentStorage/DashboardContentStorageException.php b/lib/Service/DashboardContentStorage/DashboardContentStorageException.php deleted file mode 100644 index a06b2e9e..00000000 --- a/lib/Service/DashboardContentStorage/DashboardContentStorageException.php +++ /dev/null @@ -1,35 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -use RuntimeException; - -/** - * Base exception for all dashboard content storage failures (REQ-GFSB-005). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ -class DashboardContentStorageException extends RuntimeException -{ -}//end class diff --git a/lib/Service/DashboardContentStorage/DashboardContentStorageInterface.php b/lib/Service/DashboardContentStorage/DashboardContentStorageInterface.php deleted file mode 100644 index 8fbd2901..00000000 --- a/lib/Service/DashboardContentStorage/DashboardContentStorageInterface.php +++ /dev/null @@ -1,103 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -/** - * Unified interface for dashboard content persistence (REQ-GFSB-001). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ -interface DashboardContentStorageInterface -{ - /** - * Read dashboard content by UUID. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code (e.g. 'nl'). When - * supplied the backend SHOULD attempt a - * locale-specific read first and fall back - * to the locale-neutral path. - * - * @return array The decoded content array. - * - * @throws DashboardNotFoundException When the UUID does not exist. - * @throws DashboardContentStorageException On any other storage failure. - * @throws GroupFoldersNotInstalledException When the GroupFolder app is absent. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - public function read(string $uuid, ?string $locale=null): array; - - /** - * Write (create or overwrite) dashboard content. - * - * The operation MUST be idempotent: calling write with identical content - * a second time MUST NOT raise an error. - * - * @param string $uuid The dashboard UUID. - * @param array $content The content to persist (JSON-serialisable). - * @param string|null $locale Optional locale code. When supplied the - * content is stored in a locale-specific path. - * - * @return void - * - * @throws DashboardContentStorageException On any storage failure. - * @throws GroupFoldersNotInstalledException When the GroupFolder app is absent. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - public function write(string $uuid, array $content, ?string $locale=null): void; - - /** - * Delete dashboard content by UUID. - * - * MUST NOT throw when the UUID does not exist (soft delete semantics). - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code. When supplied only the - * locale-specific file is removed; pass null to - * remove all locale variants. - * - * @return void - * - * @throws DashboardContentStorageException On any storage failure. - * @throws GroupFoldersNotInstalledException When the GroupFolder app is absent. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - public function delete(string $uuid, ?string $locale=null): void; - - /** - * Check whether content for the given UUID exists without throwing. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code. When supplied checks - * for the locale-specific variant. - * - * @return boolean True when the content exists; false otherwise. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - public function exists(string $uuid, ?string $locale=null): bool; -}//end interface diff --git a/lib/Service/DashboardContentStorage/DashboardNotFoundException.php b/lib/Service/DashboardContentStorage/DashboardNotFoundException.php deleted file mode 100644 index 571f5262..00000000 --- a/lib/Service/DashboardContentStorage/DashboardNotFoundException.php +++ /dev/null @@ -1,33 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -/** - * Thrown when a dashboard does not exist in the active storage backend. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ -class DashboardNotFoundException extends DashboardContentStorageException -{ -}//end class diff --git a/lib/Service/DashboardContentStorage/DbContentStorage.php b/lib/Service/DashboardContentStorage/DbContentStorage.php deleted file mode 100644 index f4e7b9bc..00000000 --- a/lib/Service/DashboardContentStorage/DbContentStorage.php +++ /dev/null @@ -1,258 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -use OCA\LaunchPad\Db\DashboardMapper; -use OCP\AppFramework\Db\DoesNotExistException; -use Psr\Log\LoggerInterface; - -/** - * Database-backed implementation of DashboardContentStorageInterface (REQ-GFSB-002). - * - * Content is stored as JSON in the `content` column of `oc_launchpad_dashboards`. - * The locale parameter is accepted for interface compatibility but is not used - * because the DB backend stores a single content blob per dashboard (not - * locale-separated). Locale-separated content is a GroupFolder-only concern. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ -class DbContentStorage implements DashboardContentStorageInterface -{ - /** - * Constructor. - * - * @param DashboardMapper $dashboardMapper Dashboard persistence mapper. - * @param LoggerInterface $logger PSR-3 logger. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - public function __construct( - private readonly DashboardMapper $dashboardMapper, - private readonly LoggerInterface $logger, - ) { - }//end __construct() - - /** - * Read dashboard content by UUID from the database. - * - * Finds the dashboard entity by UUID and returns `json_decode` of its - * `content` field. The `$locale` parameter is accepted for interface - * compatibility but ignored — DB storage is not locale-separated. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code (ignored by this backend). - * - * @return array The decoded content array, or an empty array when content - * is null or empty. - * - * @throws DashboardNotFoundException When no dashboard with the UUID exists. - * @throws DashboardContentStorageException On any other persistence failure. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - public function read(string $uuid, ?string $locale=null): array - { - try { - $dashboard = $this->dashboardMapper->findByUuid(uuid: $uuid); - } catch (DoesNotExistException $e) { - throw new DashboardNotFoundException( - message: 'Dashboard not found: '.$uuid, - previous: $e - ); - } catch (\Throwable $e) { - $this->logger->error( - message: 'DbContentStorage: failed to read dashboard content.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to read dashboard content: '.$e->getMessage(), - previous: $e - ); - }//end try - - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $raw = $dashboard->getContent(); - // phpcs:enable - - if ($raw === null || $raw === '') { - return []; - } - - $decoded = json_decode(json: $raw, associative: true); - if (is_array($decoded) === true) { - return $decoded; - } - - return []; - }//end read() - - /** - * Write dashboard content by UUID to the database. - * - * Finds the entity by UUID, JSON-encodes the supplied content array, - * sets it on the entity, and calls the mapper's `update()`. The - * `$locale` parameter is accepted for interface compatibility but ignored - * — DB storage is not locale-separated. - * - * @param string $uuid The dashboard UUID. - * @param array $content The content to persist (JSON-serialisable). - * @param string|null $locale Optional locale code (ignored by this backend). - * - * @return void - * - * @throws DashboardNotFoundException When no dashboard with the UUID exists. - * @throws DashboardContentStorageException On any other persistence failure. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - public function write(string $uuid, array $content, ?string $locale=null): void - { - try { - $dashboard = $this->dashboardMapper->findByUuid(uuid: $uuid); - } catch (DoesNotExistException $e) { - throw new DashboardNotFoundException( - message: 'Dashboard not found: '.$uuid, - previous: $e - ); - } catch (\Throwable $e) { - $this->logger->error( - message: 'DbContentStorage: failed to locate dashboard for write.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to locate dashboard for write: '.$e->getMessage(), - previous: $e - ); - }//end try - - try { - $encoded = json_encode(value: $content, flags: JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR); - - // Entity setters resolve via __call which uses $args[0]; named - // args would break the magic forwarding. - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $dashboard->setContent($encoded); - // phpcs:enable - - $this->dashboardMapper->update(entity: $dashboard); - } catch (\Throwable $e) { - $this->logger->error( - message: 'DbContentStorage: failed to write dashboard content.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to write dashboard content: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end write() - - /** - * Delete dashboard content by UUID (soft delete — sets content to null). - * - * Finds the entity, sets its `content` field to null, and calls the - * mapper's `update()`. Does NOT throw when the UUID does not exist, in - * line with the interface contract. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code (ignored by this backend). - * - * @return void - * - * @throws DashboardContentStorageException On any storage failure other - * than "not found". - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - public function delete(string $uuid, ?string $locale=null): void - { - try { - $dashboard = $this->dashboardMapper->findByUuid(uuid: $uuid); - } catch (DoesNotExistException) { - // Not found — soft-delete semantics require no-op here. - return; - } catch (\Throwable $e) { - $this->logger->error( - message: 'DbContentStorage: failed to locate dashboard for delete.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to locate dashboard for delete: '.$e->getMessage(), - previous: $e - ); - }//end try - - try { - // Entity setters resolve via __call — named args break magic forwarding. - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $dashboard->setContent(null); - // phpcs:enable - - $this->dashboardMapper->update(entity: $dashboard); - } catch (\Throwable $e) { - $this->logger->error( - message: 'DbContentStorage: failed to clear dashboard content.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to clear dashboard content: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end delete() - - /** - * Check whether content exists for the given UUID without throwing. - * - * Tries to find the dashboard by UUID and returns true when a row - * exists and has non-null/non-empty content. Returns false for any - * "not found" outcome without raising an exception. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code (ignored by this backend). - * - * @return boolean True when content exists; false otherwise. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-2 - */ - public function exists(string $uuid, ?string $locale=null): bool - { - try { - $dashboard = $this->dashboardMapper->findByUuid(uuid: $uuid); - } catch (DoesNotExistException) { - return false; - } catch (\Throwable $e) { - $this->logger->warning( - message: 'DbContentStorage: exists() check failed.', - context: ['uuid' => $uuid, 'exception' => $e] - ); - return false; - }//end try - - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $raw = $dashboard->getContent(); - // phpcs:enable - - return ($raw !== null && $raw !== ''); - }//end exists() -}//end class diff --git a/lib/Service/DashboardContentStorage/GroupFolderContentStorage.php b/lib/Service/DashboardContentStorage/GroupFolderContentStorage.php deleted file mode 100644 index af8305ef..00000000 --- a/lib/Service/DashboardContentStorage/GroupFolderContentStorage.php +++ /dev/null @@ -1,400 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -use OCP\App\IAppManager; -use OCP\Files\File; -use OCP\Files\Folder; -use OCP\Files\IRootFolder; -use OCP\Files\Node; -use OCP\Files\NotFoundException; -use Psr\Log\LoggerInterface; - -/** - * GroupFolder-backed implementation of DashboardContentStorageInterface (REQ-GFSB-003). - * - * Content is stored as JSON files in a dedicated GroupFolder named `LaunchPad`. - * Without a locale the file lives at `LaunchPad/.json`; with a locale it - * lives at `LaunchPad//.json`. Locale-specific reads fall back to - * the locale-neutral path when the locale-specific file is absent. - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) write() handles all locale/path/conflict branches atomically. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ -class GroupFolderContentStorage implements DashboardContentStorageInterface -{ - - /** - * The `groupfolders` app ID required by this backend. - * - * @var string - */ - private const GROUPFOLDERS_APP_ID = 'groupfolders'; - - /** - * The name of the top-level GroupFolder used by this backend. - * - * @var string - */ - private const FOLDER_NAME = 'LaunchPad'; - - /** - * Constructor. - * - * @param IRootFolder $rootFolder Nextcloud virtual file system root. - * @param IAppManager $appManager Used to verify the groupfolders app - * is installed. - * @param LoggerInterface $logger PSR-3 logger. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - public function __construct( - private readonly IRootFolder $rootFolder, - private readonly IAppManager $appManager, - private readonly LoggerInterface $logger, - ) { - }//end __construct() - - /** - * Resolve the storage path for a given UUID and optional locale. - * - * Returns `LaunchPad/.json` when no locale is provided, and - * `LaunchPad//.json` when one is. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale The optional locale code. - * - * @return string The resolved relative path within the root folder. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - private function resolvePath(string $uuid, ?string $locale): string - { - if ($locale === null || $locale === '') { - return self::FOLDER_NAME.'/'.$uuid.'.json'; - } - - return self::FOLDER_NAME.'/'.$locale.'/'.$uuid.'.json'; - }//end resolvePath() - - /** - * Ensure the `LaunchPad` GroupFolder exists, creating it when absent. - * - * Verifies that the `groupfolders` Nextcloud app is installed before any - * file-system operation. Creates the top-level folder via `newFolder()` - * when `get()` raises `NotFoundException`. - * - * @return Node The `LaunchPad` folder node. - * - * @throws GroupFoldersNotInstalledException When the groupfolders app is absent. - * @throws DashboardContentStorageException On any other file-system failure. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - * - * @psalm-suppress UnusedReturnValue - */ - private function ensureLaunchPadGroupFolder(): Node - { - if ($this->appManager->isInstalled(appId: self::GROUPFOLDERS_APP_ID) === false) { - throw new GroupFoldersNotInstalledException(); - } - - try { - return $this->rootFolder->get(path: '/'.self::FOLDER_NAME); - } catch (NotFoundException) { - // Folder does not exist yet — create it. - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to get LaunchPad folder.', - context: ['exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to get LaunchPad GroupFolder: '.$e->getMessage(), - previous: $e - ); - }//end try - - try { - return $this->rootFolder->newFolder(path: '/'.self::FOLDER_NAME); - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to create LaunchPad folder.', - context: ['exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to create LaunchPad GroupFolder: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end ensureLaunchPadGroupFolder() - - /** - * Read dashboard content from the GroupFolder. - * - * Resolves the file path, reads and JSON-decodes the content. When a locale - * is supplied and the locale-specific file does not exist, automatically - * falls back to the locale-neutral path before raising - * `DashboardNotFoundException`. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code. - * - * @return array The decoded content array. - * - * @throws DashboardNotFoundException When no file is found for the UUID. - * @throws GroupFoldersNotInstalledException When the groupfolders app is absent. - * @throws DashboardContentStorageException On any other file-system failure. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - public function read(string $uuid, ?string $locale=null): array - { - if ($this->appManager->isInstalled(appId: self::GROUPFOLDERS_APP_ID) === false) { - throw new GroupFoldersNotInstalledException(); - } - - // Attempt locale-specific path first, then fall back to neutral. - $paths = []; - if ($locale !== null && $locale !== '') { - $paths[] = $this->resolvePath(uuid: $uuid, locale: $locale); - } - - $paths[] = $this->resolvePath(uuid: $uuid, locale: null); - - foreach ($paths as $path) { - try { - $node = $this->rootFolder->get(path: '/'.$path); - } catch (NotFoundException) { - // Try the next candidate path. - continue; - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: error accessing file.', - context: ['path' => $path, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to access GroupFolder file: '.$e->getMessage(), - previous: $e - ); - }//end try - - if (($node instanceof File) === false) { - continue; - } - - try { - $raw = $node->getContent(); - $decoded = json_decode(json: $raw, associative: true); - if (is_array($decoded) === true) { - return $decoded; - } - - return []; - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to read file content.', - context: ['path' => $path, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to read GroupFolder file content: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end foreach - - throw new DashboardNotFoundException( - message: 'Dashboard content not found in GroupFolder: '.$uuid - ); - }//end read() - - /** - * Write dashboard content to the GroupFolder as a JSON file. - * - * Ensures the `LaunchPad` folder exists, creates any locale subdirectory - * when needed, then writes (or overwrites) the JSON file. - * - * @param string $uuid The dashboard UUID. - * @param array $content The content to persist (JSON-serialisable). - * @param string|null $locale Optional locale code. When supplied the file - * is written under `LaunchPad//`. - * - * @return void - * - * @throws GroupFoldersNotInstalledException When the groupfolders app is absent. - * @throws DashboardContentStorageException On any file-system failure. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - public function write(string $uuid, array $content, ?string $locale=null): void - { - $this->ensureLaunchPadGroupFolder(); - - $path = $this->resolvePath(uuid: $uuid, locale: $locale); - - // Ensure locale subdirectory exists when writing a locale-specific file. - if ($locale !== null && $locale !== '') { - $localeDirPath = '/'.self::FOLDER_NAME.'/'.$locale; - try { - $this->rootFolder->get(path: $localeDirPath); - } catch (NotFoundException) { - try { - $this->rootFolder->newFolder(path: $localeDirPath); - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to create locale subdirectory.', - context: ['localeDirPath' => $localeDirPath, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to create locale directory: '.$e->getMessage(), - previous: $e - ); - }//end try - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to check locale subdirectory.', - context: ['localeDirPath' => $localeDirPath, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to check locale directory: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end if - - try { - $encoded = json_encode( - value: $content, - flags: JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT | JSON_THROW_ON_ERROR - ); - - try { - // File exists — overwrite. - $existingNode = $this->rootFolder->get(path: '/'.$path); - if ($existingNode instanceof File) { - $existingNode->putContent(data: $encoded); - } - } catch (NotFoundException) { - // File does not exist — determine the parent folder and create it. - $parentPath = '/'.self::FOLDER_NAME; - if ($locale !== null && $locale !== '') { - $parentPath = '/'.self::FOLDER_NAME.'/'.$locale; - } - - $parentNode = $this->rootFolder->get(path: $parentPath); - if ($parentNode instanceof Folder) { - $parentNode->newFile(path: $uuid.'.json', content: $encoded); - } - }//end try - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to write content file.', - context: ['path' => $path, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to write GroupFolder content file: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end write() - - /** - * Delete the content file for the given UUID from the GroupFolder. - * - * Silently ignores the case where the file does not exist (soft-delete - * semantics required by the interface contract). - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code. When supplied only the - * locale-specific file is removed. Pass null to - * remove the locale-neutral file. - * - * @return void - * - * @throws GroupFoldersNotInstalledException When the groupfolders app is absent. - * @throws DashboardContentStorageException On any file-system failure other - * than "not found". - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - public function delete(string $uuid, ?string $locale=null): void - { - if ($this->appManager->isInstalled(appId: self::GROUPFOLDERS_APP_ID) === false) { - throw new GroupFoldersNotInstalledException(); - } - - $path = $this->resolvePath(uuid: $uuid, locale: $locale); - - try { - $file = $this->rootFolder->get(path: '/'.$path); - $file->delete(); - } catch (NotFoundException) { - // File does not exist — soft-delete requires a no-op here. - } catch (\Throwable $e) { - $this->logger->error( - message: 'GroupFolderContentStorage: failed to delete content file.', - context: ['path' => $path, 'exception' => $e] - ); - throw new DashboardContentStorageException( - message: 'Failed to delete GroupFolder content file: '.$e->getMessage(), - previous: $e - ); - }//end try - }//end delete() - - /** - * Check whether a content file for the given UUID exists. - * - * Returns false for any "not found" or error outcome without raising - * an exception, in line with the interface contract. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code. When supplied checks - * for the locale-specific variant. - * - * @return boolean True when the file exists; false otherwise. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-3 - */ - public function exists(string $uuid, ?string $locale=null): bool - { - if ($this->appManager->isInstalled(appId: self::GROUPFOLDERS_APP_ID) === false) { - return false; - } - - $path = $this->resolvePath(uuid: $uuid, locale: $locale); - - try { - $this->rootFolder->get(path: '/'.$path); - return true; - } catch (NotFoundException) { - return false; - } catch (\Throwable $e) { - $this->logger->warning( - message: 'GroupFolderContentStorage: exists() check failed.', - context: ['path' => $path, 'exception' => $e] - ); - return false; - }//end try - }//end exists() -}//end class diff --git a/lib/Service/DashboardContentStorage/GroupFoldersNotInstalledException.php b/lib/Service/DashboardContentStorage/GroupFoldersNotInstalledException.php deleted file mode 100644 index 577765ca..00000000 --- a/lib/Service/DashboardContentStorage/GroupFoldersNotInstalledException.php +++ /dev/null @@ -1,47 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service\DashboardContentStorage; - -/** - * Thrown when the `groupfolders` app is required but not installed (REQ-GFSB-005). - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-1 - */ -class GroupFoldersNotInstalledException extends DashboardContentStorageException -{ - /** - * Human-readable error message surfaced to admins. - * - * @var string - */ - public const MESSAGE = "The 'groupfolders' app is required but not installed. " - ."Please install it via the app store."; - - /** - * Constructor. - */ - public function __construct() - { - parent::__construct(message: self::MESSAGE); - }//end __construct() -}//end class diff --git a/lib/Service/DashboardContentStorageFactory.php b/lib/Service/DashboardContentStorageFactory.php deleted file mode 100644 index ae3a5176..00000000 --- a/lib/Service/DashboardContentStorageFactory.php +++ /dev/null @@ -1,79 +0,0 @@ - - * @copyright 2026 Conduction B.V. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * @link https://conduction.nl - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-4 - */ - -declare(strict_types=1); - -namespace OCA\LaunchPad\Service; - -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageInterface; -use OCA\LaunchPad\Service\DashboardContentStorage\DbContentStorage; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFolderContentStorage; - -/** - * Factory for the active DashboardContentStorage backend (REQ-GFSB-004). - * - * Reads the persisted admin setting via `SetupWizardService::getContentStorage()` - * and returns the matching implementation. The factory itself has no I/O — it - * is a pure selection layer that the DI container resolves once per request. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-4 - */ -class DashboardContentStorageFactory -{ - /** - * Constructor. - * - * @param DbContentStorage $dbStorage Database-backed implementation. - * @param GroupFolderContentStorage $groupFolderStorage GroupFolder-backed implementation. - * @param SetupWizardService $wizardService Source of the `content_storage` - * admin setting. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-4 - */ - public function __construct( - private readonly DbContentStorage $dbStorage, - private readonly GroupFolderContentStorage $groupFolderStorage, - private readonly SetupWizardService $wizardService, - ) { - }//end __construct() - - /** - * Return the active content storage backend. - * - * Reads the persisted backend choice from `SetupWizardService::getContentStorage()`. - * Returns the GroupFolder implementation when the value equals - * `SetupWizardService::STORAGE_GROUPFOLDER`; falls back to the DB - * implementation for any other value (including the default `'database'`). - * - * @return DashboardContentStorageInterface The active storage backend. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-4 - */ - public function getStorage(): DashboardContentStorageInterface - { - $backend = $this->wizardService->getContentStorage(); - - if ($backend === SetupWizardService::STORAGE_GROUPFOLDER) { - return $this->groupFolderStorage; - } - - return $this->dbStorage; - }//end getStorage() -}//end class diff --git a/lib/Service/DashboardService.php b/lib/Service/DashboardService.php index 0991bc2d..47bb0c69 100644 --- a/lib/Service/DashboardService.php +++ b/lib/Service/DashboardService.php @@ -32,8 +32,6 @@ use OCA\LaunchPad\Db\Dashboard; use OCA\LaunchPad\Db\DashboardLockMapper; use OCA\LaunchPad\Db\DashboardMapper; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageException; -use OCA\LaunchPad\Service\DashboardContentStorageFactory; use OCA\LaunchPad\Db\WidgetPlacement; use OCA\LaunchPad\Db\WidgetPlacementMapper; use OCA\LaunchPad\Event\DashboardDeletedEvent; @@ -145,160 +143,149 @@ class DashboardService /** * Constructor * - * @param DashboardMapper $dashboardMapper Dashboard mapper. - * @param WidgetPlacementMapper $placementMapper Widget placement mapper. - * @param AdminSettingMapper $settingMapper Admin setting mapper. - * @param TemplateService $templateService Template service. - * @param DashboardFactory $dashboardFactory Dashboard factory. - * @param DashboardResolver $dashResolver Dashboard resolver. - * @param DashboardTreeService $treeService Tree-aware - * validation - * / cascade - * walker - * (REQ-DASH-023..030). - * @param IGroupManager $groupManager Group manager (used for - * `isAdmin` only — - * group membership - * lookups go through the - * routing resolver per - * REQ-TMPL-013). - * @param AdminTemplateService $adminTemplateService Routing resolver - * — single - * source of truth - * for - * `IGroupManager::getUserGroupIds` - * (REQ-TMPL-013). - * @param IDBConnection $db DB connection (for the - * transactional default - * flip — - * REQ-DASH-015). - * @param IConfig $config Nextcloud per-user - * preference - * storage. - * @param IFactory $l10nFactory L10N factory used to - * build the "My copy - * of {name}" default - * fork name - * (REQ-DASH-020). - * @param LoggerInterface $logger PSR logger. - * @param DashboardTranslationService|null $translationService Optional - * translation - * service - * for the - * per-language - * content - * variants - * (REQ-DASH-038..044). - * Nullable - * so - * legacy - * test - * doubles - * constructed - * without - * it keep - * working. - * @param DashboardLockMapper|null $lockMapper Optional lock - * mapper. When - * provided the - * delete path - * cascades the - * row removal - * to the - * editing-lock - * table per - * REQ-LOCK-008. - * Nullable to - * keep the - * constructor - * backwards- - * compatible - * with existing - * unit tests. - * @param FooterService|null $footerService Optional - * per-dashboard - * footer - * sanitiser - * + - * resolver - * (REQ-FTR-006). - * Nullable - * for - * backwards- - * compat - * with - * existing - * test - * doubles. - * @param RoleFeaturePermissionService|null $roleFeaturePerm Role-default - * layout - * seeding - * (REQ-RFP-002). - * Nullable to - * keep legacy - * PHPUnit - * doubles - * working — - * the - * seedLayoutFromRoleDefaults - * call site - * below guards - * on null and - * degrades to - * the original - * no- op - * behaviour. - * @param IEventDispatcher|null $eventDispatcher Event - * dispatcher - * for - * DashboardDeletedEvent. - * Nullable - * for - * backwards- - * compat - * with - * existing - * test - * doubles. - * @param DashboardContentStorageFactory|null $contentStorageFactory Factory - * for the - * active - * content - * storage - * backend - * (REQ-GFSB-001). - * Nullable - * for - * backwards- - * compat - * with - * existing - * test - * doubles. - * @param PublicShareContext|null $publicShareContext Optional public-share - * bearer context; when - * present a share bearer - * cannot mutate dashboards - * (dashboard-public-share). - * @param QuotaService|null $quotaService Optional quota-enforcement - * service gating per-user - * dashboard creation - * (dashboard-quota-limits). - * @param IURLGenerator|null $urlGenerator URL generator used to - * build widget / app image - * paths and to resolve app - * image paths for default - * tile logos, so they render - * regardless of the app's - * install location. - * @param ILockingProvider|null $lockingProvider Optional cluster lock - * provider used to - * serialise the fork - * naming + insert - * sequence per user so - * concurrent forks cannot - * produce duplicate slugs - * (REQ-DASH-020). + * @param DashboardMapper $dashboardMapper Dashboard mapper. + * @param WidgetPlacementMapper $placementMapper Widget placement mapper. + * @param AdminSettingMapper $settingMapper Admin setting mapper. + * @param TemplateService $templateService Template service. + * @param DashboardFactory $dashboardFactory Dashboard factory. + * @param DashboardResolver $dashResolver Dashboard resolver. + * @param DashboardTreeService $treeService Tree-aware + * validation + * / cascade + * walker + * (REQ-DASH-023..030). + * @param IGroupManager $groupManager Group manager (used for + * `isAdmin` only — + * group membership + * lookups go through the + * routing resolver per + * REQ-TMPL-013). + * @param AdminTemplateService $adminTemplateService Routing resolver + * — single + * source of truth + * for + * `IGroupManager::getUserGroupIds` + * (REQ-TMPL-013). + * @param IDBConnection $db DB connection (for the + * transactional default + * flip — + * REQ-DASH-015). + * @param IConfig $config Nextcloud per-user + * preference + * storage. + * @param IFactory $l10nFactory L10N factory used to + * build the "My copy + * of {name}" default + * fork name + * (REQ-DASH-020). + * @param LoggerInterface $logger PSR logger. + * @param DashboardTranslationService|null $translationService Optional + * translation + * service + * for the + * per-language + * content + * variants + * (REQ-DASH-038..044). + * Nullable + * so + * legacy + * test + * doubles + * constructed + * without + * it keep + * working. + * @param DashboardLockMapper|null $lockMapper Optional lock + * mapper. When + * provided the + * delete path + * cascades the + * row removal + * to the + * editing-lock + * table per + * REQ-LOCK-008. + * Nullable to + * keep the + * constructor + * backwards- + * compatible + * with existing + * unit tests. + * @param FooterService|null $footerService Optional + * per-dashboard + * footer + * sanitiser + * + + * resolver + * (REQ-FTR-006). + * Nullable + * for + * backwards- + * compat + * with + * existing + * test + * doubles. + * @param RoleFeaturePermissionService|null $roleFeaturePerm Role-default + * layout + * seeding + * (REQ-RFP-002). + * Nullable to + * keep legacy + * PHPUnit + * doubles + * working — + * the + * seedLayoutFromRoleDefaults + * call site + * below guards + * on null and + * degrades to + * the original + * no- op + * behaviour. + * @param IEventDispatcher|null $eventDispatcher Event + * dispatcher + * for + * DashboardDeletedEvent. + * Nullable + * for + * backwards- + * compat + * with + * existing + * test + * doubles. + * @param PublicShareContext|null $publicShareContext Optional public-share + * bearer context; when + * present a share + * bearer cannot mutate + * dashboards + * (dashboard-public-share). + * @param QuotaService|null $quotaService Optional quota-enforcement + * service gating per-user + * dashboard creation + * (dashboard-quota-limits). + * @param IURLGenerator|null $urlGenerator URL generator used to + * build widget / app + * image paths and to + * resolve app image + * paths for default + * tile logos, so they + * render regardless of + * the app's install + * location. + * @param ILockingProvider|null $lockingProvider Optional cluster lock + * provider used to + * serialise the fork + * naming + insert + * sequence per user so + * concurrent forks + * cannot produce + * duplicate slugs + * (REQ-DASH-020). */ public function __construct( private readonly DashboardMapper $dashboardMapper, @@ -319,7 +306,6 @@ public function __construct( private readonly ?FooterService $footerService=null, private readonly ?RoleFeaturePermissionService $roleFeaturePerm=null, private readonly ?IEventDispatcher $eventDispatcher=null, - private readonly ?DashboardContentStorageFactory $contentStorageFactory=null, private readonly ?PublicShareContext $publicShareContext=null, private readonly ?QuotaService $quotaService=null, private readonly ?IURLGenerator $urlGenerator=null, @@ -3141,82 +3127,4 @@ private function parseFuturePublishAt(string $publishAt): string return $parsed->format(format: 'Y-m-d H:i:s'); }//end parseFuturePublishAt() - - /** - * Read dashboard content from the active storage backend (REQ-GFSB-001). - * - * Delegates to the factory-selected backend. When the factory is absent - * (e.g. in legacy test doubles) returns an empty array. - * - * @param string $uuid The dashboard UUID. - * @param string|null $locale Optional locale code for GroupFolder path routing. - * - * @return array The decoded content array, or an empty array when unavailable. - * - * @throws DashboardContentStorageException When the backend is unavailable - * or the operation fails. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-5 - */ - public function readDashboardContent(string $uuid, ?string $locale=null): array - { - if ($this->contentStorageFactory === null) { - return []; - } - - return $this->contentStorageFactory->getStorage()->read( - uuid: $uuid, - locale: $locale - ); - }//end readDashboardContent() - - /** - * Write dashboard content to the active storage backend (REQ-GFSB-001). - * - * When the factory is absent (e.g. in legacy test doubles) this is a no-op. - * - * @param string $uuid The dashboard UUID. - * @param array $content The content array to persist. - * @param string|null $locale Optional locale code for GroupFolder path routing. - * - * @return void - * - * @throws DashboardContentStorageException When the backend is unavailable - * or the write fails. - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-5 - */ - public function writeDashboardContent(string $uuid, array $content, ?string $locale=null): void - { - if ($this->contentStorageFactory === null) { - return; - } - - $this->contentStorageFactory->getStorage()->write( - uuid: $uuid, - content: $content, - locale: $locale - ); - }//end writeDashboardContent() - - /** - * Delete dashboard content from the active storage backend (REQ-GFSB-001). - * - * Called by the cascade delete path to clean up storage alongside the - * entity row. When the factory is absent this is a no-op. - * - * @param string $uuid The dashboard UUID. - * - * @return void - * - * @spec openspec/changes/groupfolder-storage-backend/tasks.md#task-5 - */ - public function deleteDashboardContent(string $uuid): void - { - if ($this->contentStorageFactory === null) { - return; - } - - $this->contentStorageFactory->getStorage()->delete(uuid: $uuid); - }//end deleteDashboardContent() }//end class diff --git a/openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md b/openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md index 3b0dde6f..cd296cc1 100644 --- a/openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md +++ b/openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md @@ -6,7 +6,7 @@ - [x] Task 2: Implement `DbContentStorage` against the existing `DashboardMapper` (reads/writes the entity `content` field, idempotent `write`, soft `delete`) - [x] Task 3: Implement `GroupFolderContentStorage` against `IRootFolder`/`IGroupManager`/`IAppManager` with `ensureLaunchPadGroupFolder()` bootstrap, `resolvePath()` (`LaunchPad//.json`), and 503-wrapping for all I/O failures - [x] Task 4: Implement `DashboardContentStorageFactory::getStorage()` reading the `launchpad.content_storage` admin setting (`db` default, `groupfolder` opt-in) -- [x] Task 5: Wire `DashboardContentStorageFactory` into `DashboardService` so `get/create/update/delete` route through the active backend; catch storage exceptions and rethrow with user-friendly messages +- [ ] Task 5: Wire `DashboardContentStorageFactory` into `DashboardService` so `get/create/update/delete` route through the active backend; catch storage exceptions and rethrow with user-friendly messages — **NEVER DONE; TICK CORRECTED 2026-08-11.** The three facade methods (`readDashboardContent`, `writeDashboardContent`, `deleteDashboardContent`) were written, but no `get/create/update/delete` path was ever routed through them: each had zero callers, which left the factory and both backends unreachable and the whole capability inert. Found by gate-57 (orphaned-write-capability) and recorded in launchpad#87. The capability is now **withdrawn** — see `openspec/specs/groupfolder-storage-backend/spec.md`. Tasks 1-4 and 6-14 did land, which is why this looked complete from every angle except the one that mattered - [x] Task 6: Update `lib/Db/Dashboard.php` to add the optional `locale` property and document the now-optional `content` column; keep `jsonSerialize()` returning `content` - [x] Task 7: Register `launchpad.content_storage` as an admin setting (enum `db|groupfolder`, default `db`) with GET/POST validation returning 400 on invalid values - [x] Task 8: Set restrictive ACL on the auto-created `LaunchPad` GroupFolder (admins full, all others denied; per-dashboard ACL stays in the API layer) and document the layout in code comments diff --git a/openspec/changes/retire-groupfolder-storage-backend/proposal.md b/openspec/changes/retire-groupfolder-storage-backend/proposal.md new file mode 100644 index 00000000..97c809a7 --- /dev/null +++ b/openspec/changes/retire-groupfolder-storage-backend/proposal.md @@ -0,0 +1,52 @@ +# Proposal — retire the groupfolder-storage-backend capability + +## Why + +`gate-57` (orphaned-write-capability) reports exactly one finding on launchpad: + +``` +lib/Service/DashboardService.php:3189 method=writeDashboardContent rule=orphaned-write-capability class=DashboardService +``` + +It is a true positive, and the real scope is larger than one method. Task 5 of +the original change — the wiring that would have routed dashboard +get/create/update/delete through the storage factory — was ticked in `tasks.md` +but never performed. Everything around it landed, so the capability looks +complete from every angle except the one that matters. + +Measured on `origin/development` at `e24334b6`, each claim taken with a positive +control (the same search resolves callers for `getUserDashboards` and +`findByUuid`): + +| symbol | callers outside its own definition | +|---|---| +| `DashboardService::writeDashboardContent()` | 0 | +| `DashboardService::readDashboardContent()` | 0 | +| `DashboardService::deleteDashboardContent()` | 0 | +| `DashboardContentStorageFactory::getStorage()` | 0 (only the three above) | +| `DashboardApiController::storageUnavailableResponse()` | 0 | +| `DashboardMapper::findAll()` | 1 — the migration command, itself retired here | + +Consequences that follow: nothing ever populated +`oc_launchpad_dashboards.content`, so `Dashboard::jsonSerialize()` emitted a +`content` key that was always `null`; `launchpad.content_storage` was a no-op +setting; and `launchpad:storage:migrate-to-groupfolder` migrated nothing. + +## What + +Retire the capability (path 2 of the two options in launchpad#87), rather than +complete it. Completing it needs an ADR-level decision about whether the +`content` blob or the `WidgetPlacement` rows are the source of truth for a +dashboard's layout — the spec asserts the blob, the code has used the rows +since long before this capability existed, and wiring the blob in as a second +source of truth invites divergence. Nobody is using the capability, so retiring +it costs nothing and removes that risk permanently. + +## Scope boundary + +The `launchpad.content_storage` setting and setup-wizard step 2 are **retained** +and tracked separately on launchpad#87. They belong to the `setup-wizard` +capability, which *writes* the setting; this capability was the *reader*. The +setting was already inert before this change, so removing the reader does not +make it worse, and removing the wizard step changes a different spec and +deserves its own review. diff --git a/openspec/changes/retire-groupfolder-storage-backend/tasks.md b/openspec/changes/retire-groupfolder-storage-backend/tasks.md new file mode 100644 index 00000000..1c9fceb8 --- /dev/null +++ b/openspec/changes/retire-groupfolder-storage-backend/tasks.md @@ -0,0 +1,18 @@ +# Tasks — retire-groupfolder-storage-backend + +## Tasks + +- [x] Task 1: Remove the storage tree — `lib/Service/DashboardContentStorage/` (interface, `DbContentStorage`, `GroupFolderContentStorage`, and the three exception classes) and `lib/Service/DashboardContentStorageFactory.php` +- [x] Task 2: Remove the orphaned service and controller surface — `DashboardService::{readDashboardContent,writeDashboardContent,deleteDashboardContent}` and the `?DashboardContentStorageFactory $contentStorageFactory` constructor parameter; `DashboardApiController::storageUnavailableResponse()`; `DashboardMapper::findAll()`, whose only caller was the retired migration command +- [x] Task 3: Remove the `content` and `locale` columns — drop the `$content`/`$locale` properties, `decodeContent()`, the `@method` docblocks and the `jsonSerialize()` keys from `lib/Db/Dashboard.php`, and drop both columns from `launchpad_dashboards` in `lib/Migration/Version002009Date20260811000000.php`. Leave `Version002001Date20260603000000` and `DashboardTableBuilder::addContentStorageColumns()` in place — a shipped migration is part of the version ledger and deleting it would desynchronise instances that have already run it +- [x] Task 4: Remove the two CLI commands — `lib/Command/MigrateStorageToGroupFolder.php` and `lib/Command/ToggleStorageSetting.php`, and their `` entries in `appinfo/info.xml` +- [x] Task 5: Remove the PHPUnit classes that covered the removed tree — `DashboardContentStorageFactoryTest`, `DashboardContentStorageExceptionTest`, `DbContentStorageTest`, `GroupFolderContentStorageTest`, `MigrateStorageToGroupFolderTest` +- [x] Task 6: Mark `openspec/specs/groupfolder-storage-backend/spec.md` **withdrawn**, recording what was removed, what was deliberately kept, and why. This also retires the spec's 16 `@e2e exclude` markers, whose stated reasons cited `tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php` — a test file this change deletes, which would have left sixteen exemptions resting on a class that no longer exists +- [x] Task 7: Correct the phantom `- [x]` on Task 5 of the archived change at `openspec/changes/archive/2026-06-14-groupfolder-storage-backend/tasks.md`, because the project record asserted a wiring that did not exist +- [ ] Task 8: **Follow-up, tracked on launchpad#87 — NOT in this change.** Decide whether setup-wizard step 2 (`launchpad.content_storage`, `AdminSettingKey::CONTENT_STORAGE`, `SetupWizardService::{getContentStorage,setContentStorage,hasGroupfolderApp}`, `AdminController::setWizardStorage` and its route) also retires. It writes a setting nothing now reads. Note that `tools/spec-annotations-allowlist.txt` carries entries for those three `SetupWizardService` methods and for `AdminController::setWizardStorage` + +## Verification + +- `gate-57` (orphaned-write-capability) over `lib/Service/*.php` must report **0** findings, down from 1. The checker takes a **file list**, not a directory — `check_orphaned_write_capability.py .` prints nothing and exits 0 regardless of the tree's state +- PHPUnit must stay green, with the five removed classes gone rather than skipped +- No `@spec` anchor may dangle: `check_spec_anchors.py ` must report 0 findings over `lib/` diff --git a/openspec/specs/groupfolder-storage-backend/spec.md b/openspec/specs/groupfolder-storage-backend/spec.md index 497c2cf0..7540bb8e 100644 --- a/openspec/specs/groupfolder-storage-backend/spec.md +++ b/openspec/specs/groupfolder-storage-backend/spec.md @@ -1,377 +1,93 @@ --- -status: done +status: withdrawn --- -# groupfolder-storage-backend Specification +# groupfolder-storage-backend Specification (WITHDRAWN) ## Purpose -Abstracts dashboard content storage behind a unified read/write/delete interface so operators can choose between the default database backend and an optional Nextcloud GroupFolder backend. The GroupFolder backend stores dashboards as human-readable JSON files in an auto-created, admin-restricted folder, fails closed without silent fallback, and ships a one-time migration command — all transparently to existing API clients. -## Requirements -### Requirement: REQ-GFSB-001 Storage Interface Abstraction - -The system MUST provide a unified `DashboardContentStorage` interface that abstracts the physical storage mechanism from the dashboard service layer. The interface MUST define three core operations: read, write, and delete. - -@e2e exclude these scenarios assert which PHP collaborator the service layer delegates to (`getStorage()->read/write/delete/exists`). Delegation is invisible from a browser by construction — REQ-GFSB-010 requires the HTTP response to be byte-identical whichever backend served it, so a UI assertion cannot distinguish a passing case from a failing one. Verified unit coverage: `tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php` (`testReadReturnsDecodedContent`, `testWriteCallsMapperUpdate`, `testDeleteCallsUpdate`, `testExistsReturnsFalseWhenNotFound`, `testExistsReturnsTrueWhenContentNotEmpty`). - -#### Scenario: Read dashboard content from active backend - -- GIVEN a dashboard with UUID "dash-001" exists in the active storage backend -- WHEN `DashboardService::getDashboard("dash-001")` is called -- THEN the system MUST delegate to `getStorage()->read("dash-001")` to fetch the content -- AND the content (widgets array, layout metadata) MUST be returned as a parsed PHP array -- AND the HTTP response MUST include the full content in the `content` field - -#### Scenario: Write dashboard content to active backend - -- GIVEN a user creates or updates a dashboard with new widget placements -- WHEN `DashboardService::createDashboard()` or `updateDashboard()` is called with a content array -- THEN the system MUST delegate to `getStorage()->write(uuid, $content)` to persist the content -- AND the operation MUST be idempotent (rewriting identical content produces no error) -- AND the HTTP response MUST return HTTP 201 (create) or HTTP 200 (update) - -#### Scenario: Delete dashboard content from active backend - -- GIVEN a user deletes a dashboard -- WHEN `DashboardService::deleteDashboard(uuid)` is called -- THEN the system MUST delegate to `getStorage()->delete(uuid)` to remove the content -- AND the dashboard entity MUST also be deleted from the database via the mapper -- AND the HTTP response MUST return HTTP 204 - -#### Scenario: Check existence without raising exception - -- GIVEN a dashboard UUID "unknown" does not exist in any backend -- WHEN `getStorage()->exists("unknown")` is called -- THEN the system MUST return `false` without raising an exception -- AND the caller can use this for optional-read patterns - -### Requirement: REQ-GFSB-002 Database Backend Default Behavior - -The database backend MUST implement the storage interface using the existing `oc_launchpad_dashboards` table, preserving all current behavior for operators who do not opt-in to GroupFolder storage. - -#### Scenario: Database backend is the default - -- GIVEN a fresh LaunchPad installation -- AND no `launchpad.content_storage` admin setting has been explicitly configured -- WHEN any dashboard operation occurs -- THEN the system MUST use `DbContentStorage` automatically -- AND dashboard content MUST be read from and written to the `content` field in `oc_launchpad_dashboards` -- AND no GroupFolder dependency is invoked - -@e2e exclude "which class the factory returned, and which one it did not touch" has no browser surface: REQ-GFSB-010 requires the HTTP response to be identical either way, so a UI assertion here would be one that cannot fail — the same green whether the default held or not. The Setup Wizard's storage step is not a usable surface for it either, and not merely because it is unexciting: `GET /api/admin/setup-wizard/state` returns only `{complete, currentRecommendedStep, stepStatuses}`, so the modal's `data.contentStorage` is always `undefined` and the radio falls back to "database" on every instance regardless of the persisted setting. Verified unit coverage: `tests/Unit/Service/DashboardContentStorageFactoryTest.php::testGetStorageDefaultsToDbForUnknownValue` and `::testGetStorageReturnsDbWhenSettingIsDatabase`. - -#### Scenario: Database backend reads existing dashboards - -- GIVEN the database contains a dashboard with UUID "dash-legacy" and content `{"widgets": [...]}` -- WHEN `DbContentStorage::read("dash-legacy")` is called -- THEN the system MUST fetch the dashboard entity via `DashboardMapper::findByUuid()` -- AND the `content` field MUST be JSON-decoded and returned as a PHP array -- AND the operation MUST not create a GroupFolder or depend on the `groupfolders` app - -@e2e exclude names the private collaborator (`DashboardMapper::findByUuid()`) and asserts a NEGATIVE — that no GroupFolder was created — neither of which a browser can see; a rendered dashboard looks the same whichever mapper call produced it. Verified unit coverage: `tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php::testReadReturnsDecodedContent` and `::testReadReturnsEmptyArrayWhenContentIsNull`. - -#### Scenario: Database backend handles missing dashboard gracefully - -- GIVEN a dashboard UUID "dash-missing" does not exist in the database -- WHEN `DbContentStorage::read("dash-missing")` is called -- THEN the system MUST throw `DashboardNotFoundException` (extending `DashboardContentStorageException`) -- AND the exception message MUST be descriptive and logged for debugging - -@e2e exclude asserts a PHP exception CLASS and its inheritance, plus a log line — the browser sees an HTTP error page, not a class name, so a UI assertion could not tell `DashboardNotFoundException` from any other failure. Verified unit coverage: `tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php::testReadThrowsDashboardNotFoundExceptionWhenUuidMissing` and `tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php::testDashboardNotFoundExceptionExtendsDashboardContentStorageException`. - -#### Scenario: Database backend writes and overwrites - -- GIVEN a dashboard entity exists in the database -- WHEN `DbContentStorage::write(uuid, {"widgets": [...], "new": true})` is called -- THEN the system MUST update the dashboard entity's `content` field -- AND call `DashboardMapper::update()` to persist the change -- AND rewriting with identical content MUST not raise an error - -@e2e exclude asserts which mapper method is called, which is invisible from a browser. Verified unit coverage: `tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php::testWriteCallsMapperUpdate`. The user-visible half of this — that a dashboard content write persists and survives a reload — is already proven end-to-end by `tests/e2e/widget-collision-placement.spec.ts` ("REQ-GRID-005: widget positions survive a page reload"), which CI runs. - -### Requirement: REQ-GFSB-003 GroupFolder Backend Auto-Creation and ACL - -The GroupFolder backend MUST automatically create a managed GroupFolder named "LaunchPad" on first use, with restrictive ACL rules ensuring only administrators have file-level access. - -@e2e exclude the `groupfolders` Nextcloud app is not installed on the Playwright CI fixture — `.github/workflows/code-quality.yml` provisions only `ConductionNL/openregister` through `additional-apps`, and neither seed script installs it — so no browser reaching that instance can exercise the GroupFolder backend at all. The server refuses the selection outright: `AdminController::setWizardStorage()` returns HTTP 400 "GroupFolder app is not installed." whenever `SetupWizardService::hasGroupfolderApp()` is false. Verified unit coverage of the auto-create path: `tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php::testWriteCreatesGroupFolderWhenMissing`. - -#### Scenario: GroupFolder is auto-created on first write - -- GIVEN the `launchpad.content_storage` setting is `'groupfolder'` -- AND no "LaunchPad" GroupFolder yet exists -- WHEN `GroupFolderContentStorage::write(uuid, {...})` is called for the first time -- THEN the system MUST create a GroupFolder named "LaunchPad" via the `groupfolders` app API -- AND the GroupFolder MUST be created with ACL rules: - - Administrators: read, write, delete - - All other users: no default access (dashboard permissions mediate visibility) -- AND the write operation MUST proceed to store the content in the newly created folder - -#### Scenario: Subsequent writes reuse existing GroupFolder - -- GIVEN the "LaunchPad" GroupFolder already exists with correct ACL rules -- WHEN `GroupFolderContentStorage::write(uuid, {...})` is called -- THEN the system MUST NOT attempt to create the GroupFolder again -- AND the write MUST proceed directly to persisting the content - -#### Scenario: GroupFolder creation is idempotent - -- GIVEN `ensureLaunchPadGroupFolder()` is called twice in rapid succession -- THEN the system MUST return the same GroupFolder ID both times -- AND no duplicate GroupFolder MUST be created -- AND the operation MUST be thread-safe (atomic read-or-create pattern) - -#### Scenario: ACL isolation from user dashboard permissions - -- GIVEN a GroupFolder with administrator-only file access -- AND a non-administrator user "alice" has `view_full` permission on a dashboard (from the `dashboards` capability) -- WHEN alice's browser calls `GET /api/dashboard/{uuid}` to read the dashboard content -- THEN the system MUST: - - Check alice's dashboard permission via `PermissionService` - - If authorized, fetch the content via `GroupFolderContentStorage` - - Never expose filesystem-level GroupFolder ACL to the user -- AND alice MUST NOT be able to directly access the GroupFolder via the Files API (file-level ACL is restrictive) - -### Requirement: REQ-GFSB-004 GroupFolder File Structure and Multi-Locale Support - -The GroupFolder backend MUST organize dashboard content as JSON files in a structured directory hierarchy that supports optional locale-based separation, enabling future multi-language dashboard configurations. - -@e2e exclude these scenarios assert the private path-resolution and serialisation details of `GroupFolderContentStorage` (`resolvePath()` returning `LaunchPad/.json` vs `LaunchPad//.json`, and the `json_encode` flags used on write). Nothing about a file's path or its pretty-printing reaches the browser — REQ-GFSB-010 requires the HTTP response to be identical either way — and the `groupfolders` app is not installed on the Playwright CI fixture, so the backend cannot be selected there in the first place. NOTE: the locale branch of `resolvePath()` currently has no unit test either; that is a real gap in `tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php`, and it is a unit-test gap, not an e2e one. - -#### Scenario: File path resolution without locale - -- GIVEN a dashboard UUID "abc-123-def-456" exists -- AND the system has no locale preference set (or locale is empty) -- WHEN `GroupFolderContentStorage` resolves the file path -- THEN the path MUST be `LaunchPad/abc-123-def-456.json` -- AND reading this file MUST return the full dashboard content object - -#### Scenario: File path resolution with locale - -- GIVEN a dashboard UUID "abc-123-def-456" and a locale preference "nl" (Dutch) -- WHEN `GroupFolderContentStorage` resolves the file path -- THEN the path MUST be `LaunchPad/nl/abc-123-def-456.json` -- AND the system MUST create the `LaunchPad/nl/` directory if it does not exist -- AND reading this file MUST return the locale-specific dashboard content - -#### Scenario: Fallback when locale-specific file is missing - -- GIVEN a dashboard UUID with a locale preference "nl" but no `LaunchPad/nl/{uuid}.json` file exists -- AND a fallback file `LaunchPad/{uuid}.json` exists -- WHEN `GroupFolderContentStorage::read()` is called -- THEN the system MUST: - - Attempt to read `LaunchPad/nl/{uuid}.json` first - - Fall back to `LaunchPad/{uuid}.json` if the locale-specific file does not exist - - OR raise `DashboardNotFoundException` if neither exists (decision: implement fallback if operationally useful; document choice) -- NOTE: Fallback behavior is optional; current implementation can skip this and always require exact locale match - -#### Scenario: Content is JSON-encoded on write - -- GIVEN a dashboard content array `{"widgets": [...], "layout": {...}}` -- WHEN `GroupFolderContentStorage::write(uuid, $content)` is called -- THEN the system MUST JSON-encode the array via `json_encode($content, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)` -- AND write the JSON string to the file at the resolved path -- AND the file MUST be human-readable for manual inspection by administrators - -### Requirement: REQ-GFSB-005 Dependency Check and Error Handling - -The system MUST verify that the `groupfolders` Nextcloud app is installed before attempting GroupFolder operations, and MUST fail with a clear, actionable error if the app is not available. - -@e2e exclude every scenario here is fault injection — the `groupfolders` app removed mid-operation, permission denied, disk full, a network error on remote storage. None of those states can be induced from a browser, and reaching them at all needs `launchpad.content_storage = 'groupfolder'`, which the Playwright CI fixture cannot select because the `groupfolders` app is not installed there. Verified unit coverage of the app-missing branch: `tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php` (`testReadThrowsGroupFoldersNotInstalledExceptionWhenAppMissing`, `testExistsReturnsFalseWhenAppMissing`) and `tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php` for the exception hierarchy and the exact message constant. - -#### Scenario: GroupFolders app is required but missing - -- GIVEN a LaunchPad instance with `launchpad.content_storage = 'groupfolder'` -- AND the `groupfolders` Nextcloud app is not installed -- WHEN a dashboard read/write/delete operation is attempted -- THEN the system MUST throw `GroupFoldersNotInstalledException` extending `DashboardContentStorageException` -- AND the exception message MUST be: "The 'groupfolders' Nextcloud app is required for GroupFolder storage backend but is not installed. Please install it via the app store or contact your administrator." -- AND the HTTP response MUST return HTTP 503 (Service Unavailable) - -#### Scenario: GroupFolders app becomes unavailable mid-operation -- GIVEN `GroupFolderContentStorage` is in use and the app was previously installed -- AND the `groupfolders` app is suddenly disabled or uninstalled -- WHEN a read/write operation encounters a failure related to the missing app -- THEN the system MUST catch the underlying error -- AND throw `DashboardContentStorageException` with HTTP 503 status -- AND log the error at WARN level with full context -- AND NOT silently fall back to the database backend +This capability is **withdrawn**. It was never wired into the application, and +the decision recorded in launchpad#87 is to retire it rather than complete it. + +The capability intended to abstract dashboard content storage behind a +read/write/delete interface so an operator could choose between a database +backend and a Nextcloud GroupFolder backend. The interface, both backends, the +factory, the admin setting, the migration command and the toggle command were +all written. **The wiring that would have made any of them run was not.** + +## Why it was withdrawn rather than completed + +Task 5 of the original change ("wire `DashboardContentStorageFactory` into +`DashboardService` so get/create/update/delete route through the active +backend") was ticked in `tasks.md` but never performed. The measured +consequence, at the time of withdrawal: + +- `DashboardService::readDashboardContent()`, `writeDashboardContent()` and + `deleteDashboardContent()` each had **zero callers** anywhere in the + repository. `DashboardContentStorageFactory::getStorage()` was called only by + those three methods, so the entire backend tree was unreachable. +- Nothing ever populated `oc_launchpad_dashboards.content`. Widgets live in + `WidgetPlacement` rows and always have; `Dashboard::jsonSerialize()` emitted + a `content` key that was always `null`. +- `launchpad:storage:migrate-to-groupfolder` read a column that was always + null, decoded it, and wrote the result to a GroupFolder — migrating nothing. +- `DashboardApiController::storageUnavailableResponse()`, the HTTP 503 path for + a storage failure, also had zero callers. + +Completing it instead would have required deciding whether the `content` blob +or the `WidgetPlacement` rows are the source of truth for a dashboard's layout. +The original spec asserted the blob (REQ-GFSB-002); the code has used the rows +since long before this capability existed. Wiring the blob in as a second +source of truth invites divergence between the two, and that is an +architectural decision that belongs in an ADR, not in a quality sweep. The +capability was not in use by anyone, so retiring it costs nothing and removes +the divergence risk permanently. + +## What was removed + +`lib/Service/DashboardContentStorage/` (interface, both backends, three +exception classes), `lib/Service/DashboardContentStorageFactory.php`, the three +`DashboardService` facade methods and the factory constructor parameter, +`DashboardApiController::storageUnavailableResponse()`, +`DashboardMapper::findAll()` (whose only caller was the migration command), +`lib/Command/MigrateStorageToGroupFolder.php`, +`lib/Command/ToggleStorageSetting.php` and their `` registrations, the +`content` and `locale` properties on the `Dashboard` entity together with +`decodeContent()` and their `jsonSerialize()` keys, and the four PHPUnit +classes that covered the removed tree. + +The `content` and `locale` columns are dropped by +`lib/Migration/Version002009Date20260811000000.php`. Dropping them is safe +because nothing ever wrote to them: the only writer, +`DbContentStorage::write()`, was reachable only through the factory, and the +factory was reachable only through the three orphaned facade methods. + +## Deliberately NOT removed, and why + +The `launchpad.content_storage` admin setting, `AdminSettingKey::CONTENT_STORAGE`, +`SetupWizardService::{getContentStorage,setContentStorage,hasGroupfolderApp}`, +`AdminController::setWizardStorage`, its route, and step 2 of the setup wizard +are **retained**. They belong to the `setup-wizard` capability, which *writes* +the setting; this capability was the *reader*. The setting was already inert +before this change — nothing read it, because the readers were orphaned — so +removing the reader does not make it worse. + +Retiring the wizard step is a separate, reviewable decision that changes the +`setup-wizard` spec, and it is tracked on launchpad#87 rather than folded into +this retirement. Anyone completing that follow-up should note that +`tools/spec-annotations-allowlist.txt` carries entries for those three +`SetupWizardService` methods and for `AdminController::setWizardStorage`. + +`lib/Migration/Version002001Date20260603000000.php` and +`DashboardTableBuilder::addContentStorageColumns()` are also retained: a shipped +migration is part of the version ledger and deleting it would desynchronise +instances that have already run it. The new migration drops what it added. -#### Scenario: All I/O errors are wrapped in storage exception - -- GIVEN any I/O operation on the GroupFolder fails (permission denied, disk full, network error for remote storage) -- WHEN `GroupFolderContentStorage` encounters the error -- THEN the system MUST catch the underlying exception -- AND throw or re-throw `DashboardContentStorageException` with a descriptive message including the operation (read/write/delete) and the dashboard UUID -- AND HTTP 503 MUST be returned to the client with error key `dashboard_content_storage_unavailable` - -### Requirement: REQ-GFSB-006 Admin Setting for Backend Selection - -The system MUST provide an admin-accessible setting that controls which storage backend is used for all dashboard operations. The setting MUST be persistent and validated on update. - -#### Scenario: Retrieve current storage backend setting - -- GIVEN an administrator navigates to LaunchPad admin settings -- WHEN the admin fetches `GET /api/admin/settings` -- THEN the response MUST include `{"launchpad.content_storage": "db"}` (or `"groupfolder"` if changed) -- AND the response MUST include all other admin settings unchanged - -@e2e exclude the scenario asserts a JSON field in a `GET /api/admin/settings` body, which Newman owns here and already requests (`tests/integration/launchpad.postman_collection.json`, folder "Admin - Settings"). LaunchPad renders that value nowhere a browser could read it: the Setup Wizard's storage step is the only UI for the backend, and it does not in fact display the persisted value — `GET /api/admin/setup-wizard/state` returns only `{complete, currentRecommendedStep, stepStatuses}`, so `SetupWizardModal.loadState()` reads `data.contentStorage` as `undefined` and pins the radio to "database" on every instance. Asserting the radio would therefore assert the fallback, not the setting. That mismatch is a product bug and is reported separately; it is not closed by an e2e annotation. - -#### Scenario: Change storage backend setting - -- GIVEN the current setting is `launchpad.content_storage = "db"` -- WHEN the admin sends `PUT /api/admin/settings` with body `{"launchpad.content_storage": "groupfolder"}` -- THEN the system MUST validate the value (enum: `db` or `groupfolder`) -- AND persist the new setting in the admin settings table -- AND return HTTP 200 with the updated setting -- AND all subsequent dashboard operations MUST use the new backend - -@e2e exclude the only value a change could move to is `groupfolder`, and the Playwright CI fixture cannot accept it: the `groupfolders` app is not installed there, so `AdminController::setWizardStorage()` answers HTTP 400 "GroupFolder app is not installed." for the only value a change could move to. The `PUT /api/admin/settings` half is an HTTP contract this repository routes to Newman, whose collection already exercises `PUT /api/admin/settings` (`tests/integration/launchpad.postman_collection.json`, folder "Admin - Settings"). - -#### Scenario: Invalid storage backend value is rejected - -- GIVEN the admin sends a PUT request with `{"launchpad.content_storage": "redis"}` -- WHEN the endpoint processes the request -- THEN the system MUST reject the value as invalid -- AND return HTTP 400 with error message `"Invalid value for launchpad.content_storage. Must be 'db' or 'groupfolder'."` -- AND the setting MUST NOT be changed - -@e2e exclude a browser cannot send `"redis"`: the only UI for this setting is the Setup Wizard's storage step, which offers exactly two radio inputs (`storage-database`, `storage-groupfolder`) and can therefore never produce an out-of-enum value. Rejecting one requires a hand-built HTTP request, which is Newman's job here. The validation itself lives at `lib/Service/AdminSettingsService.php` and emits that exact message string. - -#### Scenario: Non-admin cannot change backend setting - -- GIVEN a regular user "alice" (non-administrator) -- WHEN she sends `PUT /api/admin/settings` with body `{"launchpad.content_storage": "..."}` (any value) -- THEN the system MUST return HTTP 403 (Forbidden) -- AND the setting MUST NOT be changed - -@e2e exclude a non-admin never reaches this control through a browser — the Setup Wizard lives inside `/settings/admin/launchpad`, which Nextcloud's own admin-settings framework refuses to render for a non-admin, so the 403 under test is the app's and the browser would only ever demonstrate the framework's. Proving the app's own guard needs a hand-built request as a non-admin, which is Newman's job here; its collection already asserts non-admin rejection on `/api/role-feature-permissions`. - -### Requirement: REQ-GFSB-007 Fail-Closed Guarantee and No Silent Fallback - -The system MUST never silently fall back from a configured backend to a different one. If the configured backend is unavailable, the operation MUST fail with a clear error and not attempt to use an alternative backend. - -@e2e exclude fail-closed is an assertion about what does NOT happen (no second backend is tried) under a fault that has to be injected — a deleted GroupFolder, a stripped ACL, a lost database connection. A browser cannot induce any of those, and the absence of a fallback attempt leaves no trace in the rendered page. The third scenario is explicitly about the tests themselves rather than about product behaviour. Verified backend-selection coverage: `tests/Unit/Service/DashboardContentStorageFactoryTest.php` (`testGetStorageReturnsDbWhenSettingIsDatabase`, `testGetStorageReturnsGroupFolderWhenSettingIsGroupfolder`, `testGetStorageDefaultsToDbForUnknownValue`) — the factory returns exactly one backend and has no fallback branch to take. - -#### Scenario: GroupFolder backend is unavailable, no fallback to database - -- GIVEN `launchpad.content_storage = 'groupfolder'` is configured -- AND the GroupFolder is deleted or becomes unreachable (ACL stripped, Nextcloud storage issue) -- WHEN a user attempts to read or write a dashboard -- THEN the system MUST NOT attempt to fall back to database storage -- AND MUST return HTTP 503 with error key `dashboard_content_storage_unavailable` -- AND the error message MUST indicate the configured backend is unavailable - -#### Scenario: Database backend failure does not trigger GroupFolder attempt - -- GIVEN `launchpad.content_storage = 'db'` is configured -- AND a database error occurs (e.g., connection lost, table locked) -- WHEN a dashboard operation fails -- THEN the system MUST NOT attempt to use GroupFolder as a fallback -- AND MUST return HTTP 503 or appropriate database error -- AND the failure MUST be logged with full context - -#### Scenario: Fail-closed behavior is tested and enforced - -- GIVEN the tests for storage layer exception handling -- WHEN a configured backend throws an exception -- THEN the system MUST NOT mask it or attempt fallback -- AND the exception MUST propagate to the controller with HTTP 503 - -### Requirement: REQ-GFSB-008 One-Time Migration Command - -The system MUST provide a console command that migrates all existing dashboards from the database backend to the GroupFolder backend in a single operation, with idempotent semantics allowing safe re-execution. - -@e2e exclude the subject is an `occ` console command (`launchpad:storage:migrate-to-groupfolder`, `lib/Command/MigrateStorageToGroupFolder.php`) — its console output, its progress log and its process exit code have no browser surface, and its target backend needs the `groupfolders` app the Playwright CI fixture does not install. Verified unit coverage: `tests/Unit/Command/MigrateStorageToGroupFolderTest.php` (`testHandleMigratesAllDashboards`, `testHandleSkipsAlreadyMigratedDashboards`, `testHandleExitsWithErrorOnPartialFailure`). The retention scenario states an either/or with no MUST and asserts no single behaviour. - -#### Scenario: Migration command copies all dashboards - -- GIVEN a LaunchPad instance with 10 dashboards stored in the database -- WHEN an administrator runs `launchpad:storage:migrate-to-groupfolder` via the console -- THEN the system MUST: - - Query all dashboard records via `DashboardMapper::findAll()` - - For each dashboard, read its content from the database - - Write the content to the GroupFolder backend via `GroupFolderContentStorage` - - Log progress (e.g., "Migrated 5/10 dashboards") -- AND the command MUST exit with code 0 on success -- AND the console output MUST confirm the total count migrated - -#### Scenario: Migration skips already-migrated dashboards - -- GIVEN some dashboards have already been migrated to GroupFolder -- WHEN the migration command is run again -- THEN the system MUST detect that the content already exists in GroupFolder (via `exists()` check) -- AND skip re-copying it without raising an error -- AND log a message like "Dashboard {uuid} already migrated, skipping" -- AND the command MUST remain idempotent and exit with code 0 - -#### Scenario: Migration handles errors gracefully - -- GIVEN a migration is in progress and encounters a write error on dashboard 7 of 10 -- WHEN the error occurs (e.g., GroupFolder permission issue) -- THEN the system MUST: - - Log the error with the dashboard UUID and details - - Continue processing remaining dashboards (not fail-fast) - - Output a summary at the end: "Migrated 6/10, 1 error" - - Return exit code 1 to signal partial failure -- AND the operator can re-run the command to retry failed dashboards - -#### Scenario: Retention policy after migration (optional decision point) - -- GIVEN migration has completed successfully -- THEN either: - - (A) The system deletes the content from the database (clean cutover), OR - - (B) The system leaves the database content in place for rollback safety -- NOTE: Decision is implementation-specific; document the choice in the command help text and release notes. Recommended: option (A) with a dry-run flag to preview changes. - -### Requirement: REQ-GFSB-009 Transparent Backend Switching During Transition - -Existing dashboards MUST remain readable from their original backend during a transition period, allowing operators to gradually migrate content without service disruption. - -@e2e exclude both scenarios require `launchpad.content_storage = 'groupfolder'` to be the active setting, which the Playwright CI fixture cannot reach: the `groupfolders` app is not installed there, so `AdminController::setWizardStorage()` answers HTTP 400 "GroupFolder app is not installed." to any attempt to select it. The first scenario is additionally undecided in the spec itself — it carries a NOTE deferring whether dual-backend reads are supported at all, so there is no settled behaviour to assert. - -#### Scenario: Database-backed dashboard is readable during GroupFolder transition - -- GIVEN `launchpad.content_storage = 'db'` and dashboard "D1" with content in the database -- WHEN the operator switches the setting to `launchpad.content_storage = 'groupfolder'` but does not run the migration command -- AND a user requests `GET /api/dashboard/D1` -- THEN the system MUST still read from the database (the configured backend is now GroupFolder, but D1 has not been migrated yet) -- NOTE: This scenario requires the API to attempt both backends in order (try configured, fall back to alternate for read-only). Decide during implementation whether to support this or require migration before switching. - -#### Scenario: Create new dashboards with the currently configured backend - -- GIVEN `launchpad.content_storage = 'groupfolder'` is now configured -- AND a user creates a new dashboard "D2" after switching -- WHEN the new dashboard is created -- THEN the system MUST persist its content to the GroupFolder backend, not the database -- AND existing dashboards remain in their original backend until explicitly migrated - -### Requirement: REQ-GFSB-010 No API Changes Required - -The storage backend MUST be transparent to all existing API clients. Dashboard read/write/delete endpoints MUST not change their contracts, error codes (except for the new HTTP 503 case), or response formats. - -@e2e exclude these are HTTP response-shape assertions about `/api/dashboard*` — JSON field names and status codes — which this repository routes to Newman rather than Playwright by standing convention (`playwright.config.ts` excludes `**/api-direct/**` for exactly this reason, and `tests/integration/launchpad.postman_collection.json` is the contract suite that runs in the same workflow). A rendered dashboard cannot show that a response is byte-identical to what a different backend would have produced. The 503 scenario also needs the configured backend to be unavailable, which is fault injection on a backend CI does not provision. NOTE: no test currently asserts the `dashboard_content_storage_unavailable` error key emitted at `lib/Controller/DashboardApiController.php`; that is an uncovered branch and it belongs to the unit or Newman suite, not to a browser. - -#### Scenario: API response format is unchanged - -- GIVEN a client calls `GET /api/dashboard/{uuid}` -- AND the dashboard content is stored in either backend -- WHEN the endpoint returns a response -- THEN the response format MUST be identical: same JSON structure, same field names, same HTTP status codes -- AND the client MUST not be able to determine which backend was used - -#### Scenario: Create endpoint response is unchanged - -- GIVEN a client calls `POST /api/dashboard` with widget content -- WHEN the system uses the configured backend (db or groupfolder) -- THEN the response MUST return HTTP 201 and the full dashboard object -- AND the response format MUST be identical to the current implementation - -#### Scenario: New error response for unavailable storage backend - -- GIVEN the configured backend is unavailable -- WHEN any dashboard operation is attempted -- THEN the system MUST return HTTP 503 (Service Unavailable) -- AND the response body MUST include `{"error": "dashboard_content_storage_unavailable", "message": "..."}` -- AND this is the ONLY new error code; all other HTTP codes remain as they are today +## Requirements +None. This capability has no requirements; it is withdrawn. Its original +requirements REQ-GFSB-001 through REQ-GFSB-010 and their 35 scenarios are +preserved in the archived change at +`openspec/changes/archive/2026-06-14-groupfolder-storage-backend/`, and the +`- [x]` tick on Task 5 there is corrected to `- [ ]` with a note, because the +project record asserted a wiring that did not exist. diff --git a/tests/Unit/Command/MigrateStorageToGroupFolderTest.php b/tests/Unit/Command/MigrateStorageToGroupFolderTest.php deleted file mode 100644 index d34835fb..00000000 --- a/tests/Unit/Command/MigrateStorageToGroupFolderTest.php +++ /dev/null @@ -1,254 +0,0 @@ - - * @copyright 2026 Conduction b.v. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * SPDX-FileCopyrightText: 2026 LaunchPad Contributors - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace Unit\Command; - -use OCA\LaunchPad\Command\MigrateStorageToGroupFolder; -use OCA\LaunchPad\Db\Dashboard; -use OCA\LaunchPad\Db\DashboardMapper; -use OCA\LaunchPad\Service\CommandService; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageException; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFolderContentStorage; -use OCP\IUserSession; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; - -/** - * Expose the protected handle() method for testing. - */ -class TestableMigrateStorageToGroupFolder extends MigrateStorageToGroupFolder -{ - /** - * Make handle() publicly callable for unit tests. - * - * @param InputInterface $input CLI input. - * @param OutputInterface $output CLI output. - * - * @return int Exit code. - */ - public function publicHandle(InputInterface $input, OutputInterface $output): int - { - return $this->handle(input: $input, output: $output); - }//end publicHandle() -}//end class - -/** - * Unit tests for {@see MigrateStorageToGroupFolder}. - */ -class MigrateStorageToGroupFolderTest extends TestCase -{ - - /** - * Command service mock. - * - * @var CommandService&MockObject - */ - private $commandService; - - /** - * User session mock. - * - * @var IUserSession&MockObject - */ - private $userSession; - - /** - * Dashboard mapper mock. - * - * @var DashboardMapper&MockObject - */ - private $dashboardMapper; - - /** - * GroupFolder storage mock. - * - * @var GroupFolderContentStorage&MockObject - */ - private $groupFolderStorage; - - /** - * Command under test (with public handle() access). - * - * @var TestableMigrateStorageToGroupFolder - */ - private TestableMigrateStorageToGroupFolder $command; - - /** - * Set up fresh mocks and the command under test for every test. - * - * @return void - */ - protected function setUp(): void - { - $logger = $this->createMock(originalClassName: LoggerInterface::class); - - $this->commandService = new CommandService(logger: $logger); - $this->userSession = $this->createMock(originalClassName: IUserSession::class); - $this->dashboardMapper = $this->createMock(originalClassName: DashboardMapper::class); - $this->groupFolderStorage = $this->createMock(originalClassName: GroupFolderContentStorage::class); - - $this->command = new TestableMigrateStorageToGroupFolder( - commandService: $this->commandService, - userSession: $this->userSession, - dashboardMapper: $this->dashboardMapper, - groupFolderStorage: $this->groupFolderStorage - ); - }//end setUp() - - /** - * Build a Dashboard entity stub with the given UUID and raw content JSON. - * - * @param string $uuid The dashboard UUID. - * @param string|null $rawContent Raw JSON string for the content column. - * - * @return Dashboard The configured dashboard entity. - */ - private function makeDashboard(string $uuid, ?string $rawContent): Dashboard - { - $dashboard = new Dashboard(); - // phpcs:disable CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $dashboard->setUuid($uuid); - $dashboard->setContent($rawContent); - // phpcs:enable - return $dashboard; - }//end makeDashboard() - - /** - * Build a mock InputInterface that returns false for the prune-source option. - * - * @return InputInterface&MockObject The configured input mock. - */ - private function makeInput(): InputInterface - { - $input = $this->createMock(originalClassName: InputInterface::class); - $input->method('getOption')->willReturn(false); - return $input; - }//end makeInput() - - /** - * handle() MUST call groupFolderStorage->write() once per dashboard when - * all dashboards have valid UUIDs and non-empty content, and MUST exit - * with EXIT_SUCCESS. - * - * @return void - */ - public function testHandleMigratesAllDashboards(): void - { - $dashboards = [ - $this->makeDashboard(uuid: 'uuid-1', rawContent: json_encode(['key' => 'a'])), - $this->makeDashboard(uuid: 'uuid-2', rawContent: json_encode(['key' => 'b'])), - ]; - - $this->dashboardMapper - ->method('findAll') - ->willReturn($dashboards); - - $this->groupFolderStorage - ->method('exists') - ->willReturn(false); - - $this->groupFolderStorage - ->expects($this->exactly(2)) - ->method('write'); - - $output = $this->createMock(originalClassName: OutputInterface::class); - - $exitCode = $this->command->publicHandle( - input: $this->makeInput(), - output: $output - ); - - $this->assertSame(expected: CommandService::EXIT_SUCCESS, actual: $exitCode); - }//end testHandleMigratesAllDashboards() - - /** - * handle() MUST skip dashboards that are already present in the GroupFolder - * backend (idempotent migration) and MUST exit with EXIT_SUCCESS. - * - * @return void - */ - public function testHandleSkipsAlreadyMigratedDashboards(): void - { - $dashboards = [ - $this->makeDashboard(uuid: 'uuid-1', rawContent: json_encode(['key' => 'a'])), - ]; - - $this->dashboardMapper - ->method('findAll') - ->willReturn($dashboards); - - $this->groupFolderStorage - ->method('exists') - ->willReturn(true); - - $this->groupFolderStorage - ->expects($this->never()) - ->method('write'); - - $output = $this->createMock(originalClassName: OutputInterface::class); - - $exitCode = $this->command->publicHandle( - input: $this->makeInput(), - output: $output - ); - - $this->assertSame(expected: CommandService::EXIT_SUCCESS, actual: $exitCode); - }//end testHandleSkipsAlreadyMigratedDashboards() - - /** - * handle() MUST exit with EXIT_ERROR when at least one dashboard fails - * to write to the GroupFolder backend. - * - * @return void - */ - public function testHandleExitsWithErrorOnPartialFailure(): void - { - $dashboards = [ - $this->makeDashboard(uuid: 'uuid-1', rawContent: json_encode(['key' => 'a'])), - ]; - - $this->dashboardMapper - ->method('findAll') - ->willReturn($dashboards); - - $this->groupFolderStorage - ->method('exists') - ->willReturn(false); - - $this->groupFolderStorage - ->method('write') - ->willThrowException( - new DashboardContentStorageException(message: 'Write failed') - ); - - $output = $this->createMock(originalClassName: OutputInterface::class); - - $exitCode = $this->command->publicHandle( - input: $this->makeInput(), - output: $output - ); - - $this->assertSame(expected: CommandService::EXIT_ERROR, actual: $exitCode); - }//end testHandleExitsWithErrorOnPartialFailure() -}//end class diff --git a/tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php b/tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php deleted file mode 100644 index dee19981..00000000 --- a/tests/Unit/Service/DashboardContentStorage/DashboardContentStorageExceptionTest.php +++ /dev/null @@ -1,89 +0,0 @@ - - * @copyright 2026 Conduction b.v. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * SPDX-FileCopyrightText: 2026 LaunchPad Contributors - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace Unit\Service\DashboardContentStorage; - -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardContentStorageException; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardNotFoundException; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFoldersNotInstalledException; -use PHPUnit\Framework\TestCase; - -/** - * Tests for the DashboardContentStorage exception hierarchy. - */ -class DashboardContentStorageExceptionTest extends TestCase -{ - - /** - * DashboardNotFoundException MUST extend DashboardContentStorageException - * so callers can catch either the base or the specific type. - * - * @return void - */ - public function testDashboardNotFoundExceptionExtendsDashboardContentStorageException(): void - { - $exception = new DashboardNotFoundException(message: 'not found'); - $this->assertInstanceOf( - expected: DashboardContentStorageException::class, - actual: $exception - ); - }//end testDashboardNotFoundExceptionExtendsDashboardContentStorageException() - - /** - * GroupFoldersNotInstalledException MUST extend DashboardContentStorageException - * so callers can catch the base type for all storage failures. - * - * @return void - */ - public function testGroupFoldersNotInstalledExceptionExtendsDashboardContentStorageException(): void - { - $exception = new GroupFoldersNotInstalledException(); - $this->assertInstanceOf( - expected: DashboardContentStorageException::class, - actual: $exception - ); - }//end testGroupFoldersNotInstalledExceptionExtendsDashboardContentStorageException() - - /** - * GroupFoldersNotInstalledException::MESSAGE MUST be a non-empty string - * that admins can read in logs and UI notifications. - * - * @return void - */ - public function testGroupFoldersNotInstalledExceptionMessageConstantIsNonEmpty(): void - { - $this->assertIsString(actual: GroupFoldersNotInstalledException::MESSAGE); - $this->assertNotEmpty(actual: GroupFoldersNotInstalledException::MESSAGE); - }//end testGroupFoldersNotInstalledExceptionMessageConstantIsNonEmpty() - - /** - * Constructing GroupFoldersNotInstalledException with no arguments MUST - * produce an exception whose message equals the MESSAGE constant. - * - * @return void - */ - public function testGroupFoldersNotInstalledExceptionMessageMatchesConstant(): void - { - $exception = new GroupFoldersNotInstalledException(); - $this->assertSame( - expected: GroupFoldersNotInstalledException::MESSAGE, - actual: $exception->getMessage() - ); - }//end testGroupFoldersNotInstalledExceptionMessageMatchesConstant() -}//end class diff --git a/tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php b/tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php deleted file mode 100644 index 182665f6..00000000 --- a/tests/Unit/Service/DashboardContentStorage/DbContentStorageTest.php +++ /dev/null @@ -1,249 +0,0 @@ - - * @copyright 2026 Conduction b.v. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * SPDX-FileCopyrightText: 2026 LaunchPad Contributors - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace Unit\Service\DashboardContentStorage; - -use OCA\LaunchPad\Db\Dashboard; -use OCA\LaunchPad\Db\DashboardMapper; -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardNotFoundException; -use OCA\LaunchPad\Service\DashboardContentStorage\DbContentStorage; -use OCP\AppFramework\Db\DoesNotExistException; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; - -/** - * Unit tests for {@see DbContentStorage}. - */ -class DbContentStorageTest extends TestCase -{ - - /** - * Dashboard mapper mock. - * - * @var DashboardMapper&MockObject - */ - private $dashboardMapper; - - /** - * PSR-3 logger mock. - * - * @var LoggerInterface&MockObject - */ - private $logger; - - /** - * Service under test. - * - * @var DbContentStorage - */ - private DbContentStorage $storage; - - /** - * Set up fresh mocks and the service under test for every test. - * - * @return void - */ - protected function setUp(): void - { - $this->dashboardMapper = $this->createMock(originalClassName: DashboardMapper::class); - $this->logger = $this->createMock(originalClassName: LoggerInterface::class); - - $this->storage = new DbContentStorage( - dashboardMapper: $this->dashboardMapper, - logger: $this->logger - ); - }//end setUp() - - /** - * Build a Dashboard mock that supports the magic __call getter/setter for `content`. - * - * @param string|null $contentValue The value returned by getContent(). - * - * @return Dashboard&MockObject - */ - private function makeDashboardMock(?string $contentValue=null): Dashboard - { - $mock = $this->getMockBuilder(Dashboard::class) - ->addMethods(['getContent', 'setContent']) - ->getMock(); - - $mock->method('getContent')->willReturn($contentValue); - - return $mock; - }//end makeDashboardMock() - - /** - * read() MUST return the JSON-decoded content array when the dashboard - * exists and its content column holds a valid JSON object. - * - * @return void - */ - public function testReadReturnsDecodedContent(): void - { - $dashboard = $this->makeDashboardMock( - contentValue: (string) json_encode(['widgets' => []]) - ); - - $this->dashboardMapper - ->method('findByUuid') - ->willReturn($dashboard); - - $result = $this->storage->read(uuid: 'test-uuid'); - - $this->assertSame(expected: ['widgets' => []], actual: $result); - }//end testReadReturnsDecodedContent() - - /** - * read() MUST throw DashboardNotFoundException when the mapper raises - * DoesNotExistException for the given UUID. - * - * @return void - */ - public function testReadThrowsDashboardNotFoundExceptionWhenUuidMissing(): void - { - $this->dashboardMapper - ->method('findByUuid') - ->willThrowException(new DoesNotExistException('not found')); - - $this->expectException(exception: DashboardNotFoundException::class); - - $this->storage->read(uuid: 'missing-uuid'); - }//end testReadThrowsDashboardNotFoundExceptionWhenUuidMissing() - - /** - * read() MUST return an empty array when the dashboard row exists but - * its content column is null. - * - * @return void - */ - public function testReadReturnsEmptyArrayWhenContentIsNull(): void - { - $dashboard = $this->makeDashboardMock(contentValue: null); - - $this->dashboardMapper - ->method('findByUuid') - ->willReturn($dashboard); - - $result = $this->storage->read(uuid: 'test-uuid'); - - $this->assertSame(expected: [], actual: $result); - }//end testReadReturnsEmptyArrayWhenContentIsNull() - - /** - * write() MUST call mapper->update() exactly once after encoding and - * setting the content on the retrieved dashboard entity. - * - * @return void - */ - public function testWriteCallsMapperUpdate(): void - { - $dashboard = $this->getMockBuilder(Dashboard::class) - ->addMethods(['getContent', 'setContent']) - ->getMock(); - - $this->dashboardMapper - ->method('findByUuid') - ->willReturn($dashboard); - - $this->dashboardMapper - ->expects($this->once()) - ->method('update'); - - $this->storage->write(uuid: 'test-uuid', content: ['widgets' => []]); - }//end testWriteCallsMapperUpdate() - - /** - * delete() MUST call mapper->update() once when the dashboard exists. - * - * @return void - */ - public function testDeleteCallsUpdate(): void - { - $dashboard = $this->getMockBuilder(Dashboard::class) - ->addMethods(['getContent', 'setContent']) - ->getMock(); - - $this->dashboardMapper - ->method('findByUuid') - ->willReturn($dashboard); - - $this->dashboardMapper - ->expects($this->once()) - ->method('update'); - - $this->storage->delete(uuid: 'test-uuid'); - }//end testDeleteCallsUpdate() - - /** - * delete() MUST be a no-op (no exception, no update) when the UUID - * does not exist — soft-delete semantics per interface contract. - * - * @return void - */ - public function testDeleteNoOpsWhenUuidNotFound(): void - { - $this->dashboardMapper - ->method('findByUuid') - ->willThrowException(new DoesNotExistException('not found')); - - $this->dashboardMapper - ->expects($this->never()) - ->method('update'); - - // No exception expected. - $this->storage->delete(uuid: 'missing-uuid'); - - $this->assertTrue(condition: true); - }//end testDeleteNoOpsWhenUuidNotFound() - - /** - * exists() MUST return true when the dashboard has a non-empty content - * column value. - * - * @return void - */ - public function testExistsReturnsTrueWhenContentNotEmpty(): void - { - $dashboard = $this->makeDashboardMock(contentValue: '{}'); - - $this->dashboardMapper - ->method('findByUuid') - ->willReturn($dashboard); - - $this->assertTrue(condition: $this->storage->exists(uuid: 'test-uuid')); - }//end testExistsReturnsTrueWhenContentNotEmpty() - - /** - * exists() MUST return false (without throwing) when the mapper raises - * DoesNotExistException for the given UUID. - * - * @return void - */ - public function testExistsReturnsFalseWhenNotFound(): void - { - $this->dashboardMapper - ->method('findByUuid') - ->willThrowException(new DoesNotExistException('not found')); - - $this->assertFalse(condition: $this->storage->exists(uuid: 'missing-uuid')); - }//end testExistsReturnsFalseWhenNotFound() -}//end class diff --git a/tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php b/tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php deleted file mode 100644 index cc049f35..00000000 --- a/tests/Unit/Service/DashboardContentStorage/GroupFolderContentStorageTest.php +++ /dev/null @@ -1,238 +0,0 @@ - - * @copyright 2026 Conduction b.v. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * SPDX-FileCopyrightText: 2026 LaunchPad Contributors - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace Unit\Service\DashboardContentStorage; - -use OCA\LaunchPad\Service\DashboardContentStorage\DashboardNotFoundException; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFolderContentStorage; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFoldersNotInstalledException; -use OCP\App\IAppManager; -use OCP\Files\File; -use OCP\Files\Folder; -use OCP\Files\IRootFolder; -use OCP\Files\Node; -use OCP\Files\NotFoundException; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; - -/** - * Unit tests for {@see GroupFolderContentStorage}. - */ -class GroupFolderContentStorageTest extends TestCase -{ - - /** - * Virtual filesystem root mock. - * - * @var IRootFolder&MockObject - */ - private $rootFolder; - - /** - * App manager mock. - * - * @var IAppManager&MockObject - */ - private $appManager; - - /** - * PSR-3 logger mock. - * - * @var LoggerInterface&MockObject - */ - private $logger; - - /** - * Service under test. - * - * @var GroupFolderContentStorage - */ - private GroupFolderContentStorage $storage; - - /** - * Set up fresh mocks and the service under test for every test. - * - * @return void - */ - protected function setUp(): void - { - $this->rootFolder = $this->createMock(originalClassName: IRootFolder::class); - $this->appManager = $this->createMock(originalClassName: IAppManager::class); - $this->logger = $this->createMock(originalClassName: LoggerInterface::class); - - $this->storage = new GroupFolderContentStorage( - rootFolder: $this->rootFolder, - appManager: $this->appManager, - logger: $this->logger - ); - }//end setUp() - - /** - * read() MUST throw GroupFoldersNotInstalledException immediately when - * the groupfolders app is not installed. - * - * @return void - */ - public function testReadThrowsGroupFoldersNotInstalledExceptionWhenAppMissing(): void - { - $this->appManager - ->method('isInstalled') - ->with('groupfolders') - ->willReturn(false); - - $this->expectException(exception: GroupFoldersNotInstalledException::class); - - $this->storage->read(uuid: 'test-uuid'); - }//end testReadThrowsGroupFoldersNotInstalledExceptionWhenAppMissing() - - /** - * read() MUST throw DashboardNotFoundException when the groupfolders app - * is installed but the file does not exist for the given UUID. - * - * @return void - */ - public function testReadThrowsDashboardNotFoundExceptionWhenFileNotFound(): void - { - $this->appManager - ->method('isInstalled') - ->willReturn(true); - - $this->rootFolder - ->method('get') - ->willThrowException(new NotFoundException(message: 'File not found')); - - $this->expectException(exception: DashboardNotFoundException::class); - - $this->storage->read(uuid: 'test-uuid'); - }//end testReadThrowsDashboardNotFoundExceptionWhenFileNotFound() - - /** - * read() MUST return the JSON-decoded content array when the groupfolders - * app is installed and the file exists with valid JSON content. - * - * @return void - */ - public function testReadReturnsDecodedContent(): void - { - $this->appManager - ->method('isInstalled') - ->willReturn(true); - - $fileNode = $this->createMock(originalClassName: File::class); - $fileNode->method('getContent')->willReturn(json_encode(['widgets' => []])); - - $this->rootFolder - ->method('get') - ->willReturn($fileNode); - - $result = $this->storage->read(uuid: 'test-uuid'); - - $this->assertSame(expected: ['widgets' => []], actual: $result); - }//end testReadReturnsDecodedContent() - - /** - * exists() MUST return false without throwing when the groupfolders app - * is not installed. - * - * @return void - */ - public function testExistsReturnsFalseWhenAppMissing(): void - { - $this->appManager - ->method('isInstalled') - ->willReturn(false); - - $this->assertFalse(condition: $this->storage->exists(uuid: 'test-uuid')); - }//end testExistsReturnsFalseWhenAppMissing() - - /** - * exists() MUST return true when the groupfolders app is installed and - * the file node can be retrieved from the root folder. - * - * @return void - */ - public function testExistsReturnsTrueWhenFileExists(): void - { - $this->appManager - ->method('isInstalled') - ->willReturn(true); - - $fileNode = $this->createMock(originalClassName: Node::class); - - $this->rootFolder - ->method('get') - ->willReturn($fileNode); - - $this->assertTrue(condition: $this->storage->exists(uuid: 'test-uuid')); - }//end testExistsReturnsTrueWhenFileExists() - - /** - * write() MUST call rootFolder->newFolder('/LaunchPad') when the LaunchPad - * GroupFolder does not yet exist, ensuring the directory is provisioned - * on demand before the content file is written. - * - * @return void - */ - public function testWriteCreatesGroupFolderWhenMissing(): void - { - $this->appManager - ->method('isInstalled') - ->willReturn(true); - - $folderNode = $this->createMock(originalClassName: Folder::class); - $fileNode = $this->createMock(originalClassName: File::class); - $folderNode->method('newFile')->willReturn($fileNode); - - // Track call count to distinguish first /LaunchPad lookup from subsequent ones. - $callCount = 0; - - $this->rootFolder - ->method('get') - ->willReturnCallback( - function (string $path) use ($folderNode, &$callCount): Node { - // First call: get('/LaunchPad') in ensureLaunchPadGroupFolder — throw. - if ($callCount === 0 && $path === '/LaunchPad') { - $callCount++; - throw new NotFoundException('Folder not found'); - } - - $callCount++; - return $folderNode; - } - ); - - $this->rootFolder - ->expects($this->once()) - ->method('newFolder') - ->with('/LaunchPad') - ->willReturn($folderNode); - - // write() will call get() for the file path; the file mock will not - // be found (NotFoundException), so write falls back to newFile(). - // We accept any outcome as long as newFolder was called once. - try { - $this->storage->write(uuid: 'test-uuid', content: ['widgets' => []]); - } catch (\Throwable) { - // File write details are not the focus of this test. - } - }//end testWriteCreatesGroupFolderWhenMissing() -}//end class diff --git a/tests/Unit/Service/DashboardContentStorageFactoryTest.php b/tests/Unit/Service/DashboardContentStorageFactoryTest.php deleted file mode 100644 index c1d5dd92..00000000 --- a/tests/Unit/Service/DashboardContentStorageFactoryTest.php +++ /dev/null @@ -1,133 +0,0 @@ - - * @copyright 2026 Conduction b.v. - * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 - * - * SPDX-FileCopyrightText: 2026 LaunchPad Contributors - * SPDX-License-Identifier: EUPL-1.2 - */ - -declare(strict_types=1); - -namespace Unit\Service; - -use OCA\LaunchPad\Service\DashboardContentStorage\DbContentStorage; -use OCA\LaunchPad\Service\DashboardContentStorage\GroupFolderContentStorage; -use OCA\LaunchPad\Service\DashboardContentStorageFactory; -use OCA\LaunchPad\Service\SetupWizardService; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; - -/** - * Unit tests for {@see DashboardContentStorageFactory}. - */ -class DashboardContentStorageFactoryTest extends TestCase -{ - - /** - * Database-backed storage mock. - * - * @var DbContentStorage&MockObject - */ - private $dbStorage; - - /** - * GroupFolder-backed storage mock. - * - * @var GroupFolderContentStorage&MockObject - */ - private $groupFolderStorage; - - /** - * Setup wizard service mock. - * - * @var SetupWizardService&MockObject - */ - private $wizardService; - - /** - * Factory under test. - * - * @var DashboardContentStorageFactory - */ - private DashboardContentStorageFactory $factory; - - /** - * Set up fresh mocks and the factory under test for every test. - * - * @return void - */ - protected function setUp(): void - { - $this->dbStorage = $this->createMock(originalClassName: DbContentStorage::class); - $this->groupFolderStorage = $this->createMock(originalClassName: GroupFolderContentStorage::class); - $this->wizardService = $this->createMock(originalClassName: SetupWizardService::class); - - $this->factory = new DashboardContentStorageFactory( - dbStorage: $this->dbStorage, - groupFolderStorage: $this->groupFolderStorage, - wizardService: $this->wizardService - ); - }//end setUp() - - /** - * getStorage() MUST return the DbContentStorage instance when the - * persisted setting value is 'database'. - * - * @return void - */ - public function testGetStorageReturnsDbWhenSettingIsDatabase(): void - { - $this->wizardService - ->method('getContentStorage') - ->willReturn('database'); - - $result = $this->factory->getStorage(); - - $this->assertSame(expected: $this->dbStorage, actual: $result); - }//end testGetStorageReturnsDbWhenSettingIsDatabase() - - /** - * getStorage() MUST return the GroupFolderContentStorage instance when - * the persisted setting value is 'groupfolder'. - * - * @return void - */ - public function testGetStorageReturnsGroupFolderWhenSettingIsGroupfolder(): void - { - $this->wizardService - ->method('getContentStorage') - ->willReturn(SetupWizardService::STORAGE_GROUPFOLDER); - - $result = $this->factory->getStorage(); - - $this->assertSame(expected: $this->groupFolderStorage, actual: $result); - }//end testGetStorageReturnsGroupFolderWhenSettingIsGroupfolder() - - /** - * getStorage() MUST fall back to the DbContentStorage instance for any - * unrecognised setting value, ensuring a safe default. - * - * @return void - */ - public function testGetStorageDefaultsToDbForUnknownValue(): void - { - $this->wizardService - ->method('getContentStorage') - ->willReturn('unknown'); - - $result = $this->factory->getStorage(); - - $this->assertSame(expected: $this->dbStorage, actual: $result); - }//end testGetStorageDefaultsToDbForUnknownValue() -}//end class From 2b87e5ebc3e266e432ea1806344b91196a9eb2e1 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 22:59:03 +0200 Subject: [PATCH 3/6] chore(openspec): add the missing .openspec.yaml to the retirement change Every other active change dir under openspec/changes/ carries one (schema/created/status); the retirement change was missing it. --- .../changes/retire-groupfolder-storage-backend/.openspec.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 openspec/changes/retire-groupfolder-storage-backend/.openspec.yaml diff --git a/openspec/changes/retire-groupfolder-storage-backend/.openspec.yaml b/openspec/changes/retire-groupfolder-storage-backend/.openspec.yaml new file mode 100644 index 00000000..d3f15c41 --- /dev/null +++ b/openspec/changes/retire-groupfolder-storage-backend/.openspec.yaml @@ -0,0 +1,3 @@ +schema: spec-driven +created: 2026-08-11 +status: implemented From 536ea3e16bd9217a812ffe78b37ed037eafa8777 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 11 Aug 2026 23:02:04 +0200 Subject: [PATCH 4/6] docs(spec): annotate the 84 changed frontend methods gate-16 reports (84 -> 0) gate-16 (spec-coverage) reported 84 changed methods without an @spec tag against base origin/beta. All 84 were in src/; zero were PHP -- the 273 lib/**/*.php files in the same diff are fully annotated already, established with a positive control (WeatherService.php carries 2 tags, HealthPingController 3) so that "all in one layer" is a measurement rather than a failed lookup. Closed with 84 real @spec anchors and exactly 2 reason-bearing exclusions. The anchors point at the specs the methods actually implement -- principally clock-weather-widgets, iframe-embed-widget, live-data-tile-widget, service-health-ping and tile-quick-search. Every one resolves: the canonical check_spec_anchors.py reports 0 dangling anchors across all of src/. That matters because a dangling anchor fails SILENTLY in this gate family, so an unresolvable tag would have made the gate green while asserting nothing. The two exclusions, both verified against the source rather than argued from the scenario text: ClockWidget.vue::pointOnCircle -- pure polar-to-cartesian trigonometry. The body reads neither this.now nor this.content, so it encodes no clock, timezone, format or accessibility behaviour. Verified its only two callers are tickCoords and handCoords, and both carry the REQ-CLOCK-003 tag. useTileClickTracking.js::__resetTileClickTrackingForTest -- test-only harness hook. Verified by grep that its only importer anywhere in the repo is src/composables/__tests__/useTileClickTracking.spec.js; it implements no product behaviour, only clearing cachedEnabled / inflightConfigPromise so each test starts from a cold cache. Both reasons name a concrete artifact (a caller, an importing test file) rather than a state of the world, which is the shape that survives re-checking. This diff is comment-only and provably so: 0 added and 0 removed lines fall outside a docblock. No runtime behaviour can change. Verification: gate-16 (base origin/beta) 84 -> 0 check_spec_anchors.py 0 dangling anchors across src/ tools/check-spec-annotations.php unchanged at 12 pre-existing PHP findings (TileClickMapper, WidgetPlacement, TileClick, ImportLaunchpadRegister, Version002007) -- none introduced here, none in a file this PR touches phpcs 0 errors, 468 warnings, 211 files (baseline) No gate was weakened: no baseline, no allowlist widening, no skipped test. --- src/components/OrgNavigationItem.vue | 1 + src/components/RuntimeShellSearch.vue | 94 +++++++++++++++ .../Widgets/Renderers/ClockWidget.vue | 110 ++++++++++++++++-- .../Widgets/Renderers/ClockWidgetForm.vue | 38 +++++- .../Widgets/Renderers/ContainerChild.vue | 6 +- .../Widgets/Renderers/IframeWidget.vue | 14 ++- .../Widgets/Renderers/WeatherWidget.vue | 41 ++++++- .../Widgets/Renderers/WeatherWidgetForm.vue | 24 +++- .../admin/OrgNavigationEditorRow.vue | 14 ++- src/composables/useTileClickTracking.js | 1 + src/composables/useTileSearch.js | 5 + src/dialogs/CreateGroupDashboardModal.vue | 9 +- src/dialogs/GroupDashboardRenameDialog.vue | 3 + src/dialogs/PublicSharePasswordDialog.vue | 11 ++ src/modals/DashboardConfigModal.vue | 19 ++- src/modals/TileEditor.vue | 19 ++- src/services/iconCatalogue.js | 5 + src/services/weatherClient.js | 1 + src/stores/groupDashboards.js | 4 +- src/stores/kioskPlaylists.js | 10 +- src/stores/publicShares.js | 14 ++- src/views/DashboardPublicShareView.vue | 10 ++ src/views/Views.vue | 8 ++ src/views/WorkspaceApp.vue | 10 +- 24 files changed, 431 insertions(+), 40 deletions(-) diff --git a/src/components/OrgNavigationItem.vue b/src/components/OrgNavigationItem.vue index f3deb161..392bbe4e 100644 --- a/src/components/OrgNavigationItem.vue +++ b/src/components/OrgNavigationItem.vue @@ -163,6 +163,7 @@ export default { * silently fall back to the default icon; picker-emitted paths and * URLs pass through unchanged. * + * @spec openspec/specs/dashboard-icons/spec.md#req-icon-009 * @return {string|null} value for `CnDashboardIcon :name`. */ iconName() { diff --git a/src/components/RuntimeShellSearch.vue b/src/components/RuntimeShellSearch.vue index 3323ecf7..1b8c12dc 100644 --- a/src/components/RuntimeShellSearch.vue +++ b/src/components/RuntimeShellSearch.vue @@ -158,10 +158,25 @@ export default { }, computed: { + /** + * The id the `