diff --git a/lib/Service/DemoShowcasesService.php b/lib/Service/DemoShowcasesService.php index b4d1cd7fb..ebc72317d 100644 --- a/lib/Service/DemoShowcasesService.php +++ b/lib/Service/DemoShowcasesService.php @@ -291,6 +291,32 @@ public function installShowcase( string $lang = 'nl', bool $force = false, ): array { + $this->assertInstallable(showcaseId: $showcaseId); + + // M8: acquire an exclusive advisory lock keyed on the showcase ID + // before the existence check to serialise concurrent installs. + // Two simultaneous admin requests both pass the existence check + // before either inserts without this guard. + $lockKey = 'launchpad-showcase-install-' . $showcaseId; + $this->acquireInstallLock(lockKey: $lockKey, showcaseId: $showcaseId); + + try { + return $this->installLocked(showcaseId: $showcaseId, options: new ShowcaseInstallOptions(lang: $lang, force: $force)); + } finally { + $this->releaseInstallLock(lockKey: $lockKey); + } + }//end installShowcase() + + /** + * Refuse an unknown showcase or one whose archive is missing. + * + * @param string $showcaseId The showcase ID. + * + * @return void + * + * @throws ShowcaseNotFoundException When the showcase cannot be installed. + */ + private function assertInstallable(string $showcaseId): void { $this->assertKnownShowcase(showcaseId: $showcaseId); $zipPath = $this->getZipPath(showcaseId: $showcaseId); @@ -299,12 +325,19 @@ public function installShowcase( message: 'Showcase ZIP missing for ' . $showcaseId ); } + }//end assertInstallable() - // M8: acquire an exclusive advisory lock keyed on the showcase ID - // before the existence check to serialise concurrent installs. - // Two simultaneous admin requests both pass the existence check - // before either inserts without this guard. - $lockKey = 'launchpad-showcase-install-' . $showcaseId; + /** + * Take the exclusive install lock of a showcase. + * + * @param string $lockKey The lock path. + * @param string $showcaseId The showcase ID (for the message). + * + * @return void + * + * @throws RuntimeException When another install holds the lock. + */ + private function acquireInstallLock(string $lockKey, string $showcaseId): void { try { $this->stores->lockingProvider->acquireLock( path: $lockKey, @@ -317,32 +350,47 @@ public function installShowcase( previous: $e ); } + }//end acquireInstallLock() - try { - $existingUuid = $this->getInstalledUuid(showcaseId: $showcaseId); - if ($existingUuid !== '' && $force === false) { - return [ - 'installedDashboardUuid' => $existingUuid, - 'skippedWidgets' => [], - 'alreadyInstalled' => true, - ]; - } + /** + * Release the exclusive install lock of a showcase. + * + * @param string $lockKey The lock path. + * + * @return void + */ + private function releaseInstallLock(string $lockKey): void { + $this->stores->lockingProvider->releaseLock( + path: $lockKey, + type: ILockingProvider::LOCK_EXCLUSIVE + ); + }//end releaseInstallLock() - if ($existingUuid !== '' && $force === true) { - $this->uninstallShowcase(showcaseId: $showcaseId); - } + /** + * Install while the lock is held: reuse an existing install, or + * replace it when forced, else install fresh. + * + * @param string $showcaseId The showcase ID. + * @param ShowcaseInstallOptions $options The language and force flag. + * + * @return array{installedDashboardUuid:string, skippedWidgets:array, alreadyInstalled:bool} + */ + private function installLocked(string $showcaseId, ShowcaseInstallOptions $options): array { + $existingUuid = $this->getInstalledUuid(showcaseId: $showcaseId); + if ($existingUuid !== '' && $options->force === false) { + return [ + 'installedDashboardUuid' => $existingUuid, + 'skippedWidgets' => [], + 'alreadyInstalled' => true, + ]; + } - return $this->doInstallShowcase( - showcaseId: $showcaseId, - lang: $lang - ); - } finally { - $this->stores->lockingProvider->releaseLock( - path: $lockKey, - type: ILockingProvider::LOCK_EXCLUSIVE - ); - }//end try - }//end installShowcase() + if ($existingUuid !== '') { + $this->uninstallShowcase(showcaseId: $showcaseId); + } + + return $this->doInstallShowcase(showcaseId: $showcaseId, lang: $options->lang); + }//end installLocked() /** * Internal install logic (called while lock is held). diff --git a/lib/Service/ShowcaseInstallOptions.php b/lib/Service/ShowcaseInstallOptions.php new file mode 100644 index 000000000..100c4d162 --- /dev/null +++ b/lib/Service/ShowcaseInstallOptions.php @@ -0,0 +1,44 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @version GIT:auto + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @spec openspec/specs/demo-data-showcases/spec.md + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Service; + +/** + * The source language and the force flag of one showcase install. + * + * @spec openspec/specs/demo-data-showcases/spec.md + */ +final class ShowcaseInstallOptions { + /** + * Constructor. + * + * @param string $lang The source language (always `nl` in v1). + * @param bool $force Whether an existing install is replaced. + */ + public function __construct( + public readonly string $lang, + public readonly bool $force, + ) { + }//end __construct() +}//end class diff --git a/tests/Unit/Service/DemoShowcasesInstallGoldenTest.php b/tests/Unit/Service/DemoShowcasesInstallGoldenTest.php new file mode 100644 index 000000000..bd996ae29 --- /dev/null +++ b/tests/Unit/Service/DemoShowcasesInstallGoldenTest.php @@ -0,0 +1,233 @@ + + * @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\Db\Dashboard; +use OCA\LaunchPad\Db\DashboardMapper; +use OCA\LaunchPad\Db\WidgetPlacementMapper; +use OCA\LaunchPad\Exception\ShowcaseNotFoundException; +use OCA\LaunchPad\Service\DemoShowcasesService; +use OCA\LaunchPad\Service\ShowcasePlatform; +use OCA\LaunchPad\Service\ShowcaseStores; +use OCP\Dashboard\IManager; +use OCP\IAppConfig; +use OCP\IDBConnection; +use OCP\IURLGenerator; +use OCP\Lock\ILockingProvider; +use OCP\Lock\LockedException; +use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; +use RuntimeException; +use ZipArchive; + +class DemoShowcasesInstallGoldenTest extends TestCase { + /** @var list */ + private array $log = []; + + private DashboardMapper $dashboardMapper; + private ILockingProvider $lockingProvider; + private IAppConfig $appConfig; + private IDBConnection $db; + private DemoShowcasesService $service; + private string $fixtureDir; + + /** @var array */ + private array $installed = []; + + protected function setUp(): void { + $this->dashboardMapper = $this->createMock(DashboardMapper::class); + $this->lockingProvider = $this->createMock(ILockingProvider::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->db = $this->createMock(IDBConnection::class); + $manager = $this->createMock(IManager::class); + $manager->method('getWidgets')->willReturn([]); + + $this->lockingProvider->method('acquireLock')->willReturnCallback(function (string $path, int $type): void { + $this->log[] = 'acquire:' . $path . ':' . $type; + if (str_ends_with($path, 'busy')) { + throw new LockedException($path); + } + }); + $this->lockingProvider->method('releaseLock')->willReturnCallback(function (string $path, int $type): void { + $this->log[] = 'release:' . $path . ':' . $type; + }); + $this->appConfig->method('getValueString')->willReturnCallback(fn (string $app, string $key, string $default = ''): string => $this->installed[$key] ?? $default); + $this->appConfig->method('setValueString')->willReturnCallback(function (string $app, string $key, string $value): bool { + $this->log[] = 'mark:' . $key . '=' . $value; + $this->installed[$key] = $value; + return true; + }); + $this->appConfig->method('deleteKey')->willReturnCallback(function (string $app, string $key): void { + $this->log[] = 'unmark:' . $key; + unset($this->installed[$key]); + }); + $this->dashboardMapper->method('insert')->willReturnCallback(function (Dashboard $dashboard): Dashboard { + $this->log[] = 'insert'; + if ($dashboard->getName() === 'Broken') { + throw new RuntimeException('db down'); + } + + $dashboard->setId(5); + $dashboard->setUuid('new-uuid'); + return $dashboard; + }); + $this->dashboardMapper->method('findByUuid')->willReturnCallback(function (string $uuid): Dashboard { + $this->log[] = 'find:' . $uuid; + $dashboard = new Dashboard(); + $dashboard->setId(4); + $dashboard->setUuid($uuid); + return $dashboard; + }); + $this->dashboardMapper->method('delete')->willReturnCallback(function ($entity) { + $this->log[] = 'delete:' . $entity->getUuid(); + return $entity; + }); + + $urls = $this->createMock(IURLGenerator::class); + $this->service = new DemoShowcasesService( + new ShowcaseStores($this->dashboardMapper, $this->createMock(WidgetPlacementMapper::class), $this->db, $this->lockingProvider), + new ShowcasePlatform($this->appConfig, $manager, $urls), + new NullLogger(), + ); + $this->fixtureDir = sys_get_temp_dir() . '/launchpad-showcase-install-' . uniqid(); + mkdir($this->fixtureDir, 0o755, true); + $this->service->setDataDirForTesting(path: $this->fixtureDir); + } + + protected function tearDown(): void { + foreach (glob($this->fixtureDir . '/*/*') ?: [] as $file) { + unlink($file); + } + + foreach (glob($this->fixtureDir . '/*') ?: [] as $dir) { + rmdir($dir); + } + + rmdir($this->fixtureDir); + } + + private function archive(string $showcaseId, string $name = 'Showcase'): void { + $dir = $this->fixtureDir . '/' . $showcaseId; + mkdir($dir, 0o755, true); + $zip = new ZipArchive(); + $zip->open($dir . '/' . $showcaseId . '.zip', ZipArchive::CREATE); + $zip->addFromString('manifest.json', '{"showcaseName":"S"}'); + $zip->addFromString('dashboards/d.json', json_encode(['uuid' => 'd', 'name' => $name, 'widgets' => [], 'gridColumns' => 12])); + $zip->close(); + } + + public function testUnknownShowcaseIsRefusedBeforeTheLock(): void { + $this->expectException(ShowcaseNotFoundException::class); + $this->expectExceptionMessage('Unknown showcase ID: nope'); + + try { + $this->service->installShowcase(showcaseId: 'nope'); + } finally { + $this->assertSame([], $this->log); + } + } + + public function testMissingArchiveIsRefusedBeforeTheLock(): void { + $this->expectException(ShowcaseNotFoundException::class); + $this->expectExceptionMessage('Showcase ZIP missing for de-bron'); + + try { + $this->service->installShowcase(showcaseId: 'de-bron'); + } finally { + $this->assertSame([], $this->log); + } + } + + public function testBusyLockIsRefusedAndNotReleased(): void { + $this->archive('de-bron'); + $service = $this->service; + $lockingProvider = $this->createMock(ILockingProvider::class); + $lockingProvider->method('acquireLock')->willThrowException(new LockedException('x')); + $lockingProvider->expects($this->never())->method('releaseLock'); + $service = new DemoShowcasesService( + new ShowcaseStores($this->dashboardMapper, $this->createMock(WidgetPlacementMapper::class), $this->db, $lockingProvider), + new ShowcasePlatform($this->appConfig, $this->createMock(IManager::class), $this->createMock(IURLGenerator::class)), + new NullLogger(), + ); + $service->setDataDirForTesting(path: $this->fixtureDir); + + try { + $service->installShowcase(showcaseId: 'de-bron'); + $this->fail('expected a refusal'); + } catch (RuntimeException $e) { + $this->assertSame('Showcase installation already in progress for de-bron', $e->getMessage()); + $this->assertInstanceOf(LockedException::class, $e->getPrevious()); + $this->assertSame([], $this->log); + } + } + + public function testAlreadyInstalledIsReusedUnderTheLock(): void { + $this->archive('de-bron'); + $this->installed['showcase_installed_de-bron'] = 'old-uuid'; + + $result = $this->service->installShowcase(showcaseId: 'de-bron'); + + $this->assertSame(['installedDashboardUuid' => 'old-uuid', 'skippedWidgets' => [], 'alreadyInstalled' => true], $result); + $this->assertSame([ + 'acquire:launchpad-showcase-install-de-bron:' . ILockingProvider::LOCK_EXCLUSIVE, + 'release:launchpad-showcase-install-de-bron:' . ILockingProvider::LOCK_EXCLUSIVE, + ], $this->log); + } + + public function testForcedReinstallUninstallsFirst(): void { + $this->archive('de-bron'); + $this->installed['showcase_installed_de-bron'] = 'old-uuid'; + + $result = $this->service->installShowcase(showcaseId: 'de-bron', lang: 'en', force: true); + + $this->assertSame(['installedDashboardUuid' => 'new-uuid', 'skippedWidgets' => [], 'alreadyInstalled' => false], $result); + $lock = 'launchpad-showcase-install-de-bron:' . ILockingProvider::LOCK_EXCLUSIVE; + $this->assertSame('acquire:' . $lock, $this->log[0]); + $this->assertSame('release:' . $lock, end($this->log)); + $this->assertLessThan(array_search('insert', $this->log, true), array_search('unmark:showcase_installed_de-bron', $this->log, true)); + } + + public function testFreshInstallMarksAndReleases(): void { + $this->archive('de-bron'); + + $result = $this->service->installShowcase(showcaseId: 'de-bron', force: true); + + $this->assertFalse($result['alreadyInstalled']); + $this->assertNotContains('unmark:showcase_installed_de-bron', $this->log); + $this->assertContains('mark:showcase_installed_de-bron=new-uuid', $this->log); + $this->assertSame('release:launchpad-showcase-install-de-bron:' . ILockingProvider::LOCK_EXCLUSIVE, end($this->log)); + } + + public function testFailedInstallStillReleasesTheLock(): void { + $this->archive('de-bron', 'Broken'); + + try { + $this->service->installShowcase(showcaseId: 'de-bron'); + $this->fail('expected a failure'); + } catch (RuntimeException $e) { + $this->assertSame('Failed to persist showcase: db down', $e->getMessage()); + $this->assertSame('release:launchpad-showcase-install-de-bron:' . ILockingProvider::LOCK_EXCLUSIVE, end($this->log)); + } + } +}//end class