diff --git a/appinfo/info.xml b/appinfo/info.xml index 92e1eb51..7a87a156 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -7,7 +7,7 @@ > ⚠️ **Active development — not for production use yet.** This app is under active development. Although it may carry a stable release status, **please do not use it in production environments before 12 June 2026.** See [conduction.nl/apps](https://conduction.nl/apps) for release planning and what this app does. Versioniq gives Nextcloud administrators the ability to roll back apps to previous versions or install specific newer versions. Essential for debugging, testing compatibility, and recovering from broken updates. - 1.6.6 + 1.6.7-unstable.20261007150243 EUPL-1.2 Conduction B.V. Versioniq @@ -104,6 +104,14 @@ Versioniq gives Nextcloud administrators the ability to roll back apps to previo replacement. Idempotent — a fresh install removes nothing. --> OCA\Versioniq\Repair\RemoveRetiredCronJobs + + OCA\Versioniq\Repair\MarkPayloadCacheLazy OCA\Versioniq\Repair\MigrateAppConfigKeys @@ -113,6 +121,9 @@ Versioniq gives Nextcloud administrators the ability to roll back apps to previo this block is the one that actually fires on the upgrade the step exists for. --> OCA\Versioniq\Repair\MigrateSchemaApplicationId + + OCA\Versioniq\Repair\MarkPayloadCacheLazy diff --git a/lib/Repair/MarkPayloadCacheLazy.php b/lib/Repair/MarkPayloadCacheLazy.php new file mode 100644 index 00000000..01895bb6 --- /dev/null +++ b/lib/Repair/MarkPayloadCacheLazy.php @@ -0,0 +1,122 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + + +namespace OCA\Versioniq\Repair; + +use OCA\Versioniq\AppInfo\Application; +use OCA\Versioniq\Service\Source\AppStoreSource; +use OCP\IAppConfig; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Psr\Log\LoggerInterface; +use Throwable; + +/** + * Marks every existing `appstore.payload.*` / `appstore.payload_ts.*` row lazy. + * + * WHY. Until this release {@see AppStoreSource} wrote its payload cache as + * EAGER app config, and Nextcloud loads every eager row of every app on every + * request. One payload per managed app, ~4 MB for mail alone: measured on a + * dev instance 2026-10-06, 1,660 rows / 36 MB took ~300 ms of a ~450 ms + * `status.php`, slowing every page of the instance, not just this app's. + * + * The code now writes these keys lazy, and a write flips an existing row's + * flag. But a row is only rewritten when its app is listed again after the + * TTL, so a payload for an app nobody looks at would stay eager forever. This + * step flips them all at once. + * + * Idempotent: `updateLazy()` on a row that is already lazy changes nothing, so + * a fresh install and a second run both pass through without effect. Never + * raises: a cache row is not worth an aborted upgrade. + * + * @psalm-suppress UnusedClass Nextcloud instantiates repair steps from the + * `` block in appinfo/info.xml, which psalm does not read. + */ +class MarkPayloadCacheLazy implements IRepairStep { + + /** + * @param IAppConfig $appConfig The app config store. + * @param LoggerInterface $logger The logger. + */ + public function __construct( + private IAppConfig $appConfig, + private LoggerInterface $logger, + ) { + }//end __construct() + + /** + * The step's name, as shown by `occ upgrade`. + * + * @return string The name. + * + * @spec exclude Performance repair of a cache row's storage flag; no + * capability spec covers how the payload cache is stored. + */ + public function getName(): string { + return 'Mark the App Store payload cache as lazy app config'; + }//end getName() + + /** + * Flip each payload cache key to lazy. + * + * @param IOutput $output The upgrade output. + * + * @return void + * + * @spec exclude Performance repair of a cache row's storage flag; no + * capability spec covers how the payload cache is stored. + */ + public function run(IOutput $output): void { + try { + $keys = $this->appConfig->getKeys(Application::APP_ID); + } catch (Throwable $e) { + $this->logger->warning( + '[MarkPayloadCacheLazy] Could not list app config keys: ' . $e->getMessage(), + ['app' => Application::APP_ID, 'exception' => $e] + ); + $output->warning('Could not list app config keys: ' . $e->getMessage()); + return; + } + + $flipped = 0; + foreach ($keys as $key) { + if (str_starts_with($key, AppStoreSource::PAYLOAD_CACHE_PREFIX) === false + && str_starts_with($key, AppStoreSource::PAYLOAD_CACHE_TS_PREFIX) === false + ) { + continue; + } + + try { + if ($this->appConfig->updateLazy(Application::APP_ID, $key, true) === true) { + $flipped++; + } + } catch (Throwable $e) { + // Reported, not raised — see the class docblock. + $this->logger->warning( + '[MarkPayloadCacheLazy] Could not mark ' . $key . ' lazy: ' . $e->getMessage(), + ['app' => Application::APP_ID, 'exception' => $e] + ); + } + } + + $output->info('Marked ' . $flipped . ' App Store payload cache keys lazy'); + + }//end run() +}//end class diff --git a/lib/Service/Source/AppStoreSource.php b/lib/Service/Source/AppStoreSource.php index b3bda555..5d954ad8 100644 --- a/lib/Service/Source/AppStoreSource.php +++ b/lib/Service/Source/AppStoreSource.php @@ -57,8 +57,22 @@ class AppStoreSource implements SourceInterface, AdvisorySourceInterface { * versions available" rather than a clear failure. */ private const FETCH_TIMEOUT_SECONDS = 180; - private const PAYLOAD_CACHE_PREFIX = 'appstore.payload.'; - private const PAYLOAD_CACHE_TS_PREFIX = 'appstore.payload_ts.'; + + /** + * Key prefixes of the payload cache. Public so the repair step that marks + * existing rows lazy names the same keys this class writes. + * + * EVERY READ AND WRITE OF THESE KEYS PASSES `lazy: true`. One payload per + * managed app, and a single one (mail) runs to ~4 MB, so the cache grows to + * tens of megabytes. Nextcloud loads every NON-lazy app-config row on every + * request, for every app, so an eager cache made the whole instance pay for + * it: measured 2026-10-06, 1,660 eager rows / 36 MB took ~300 ms of a + * ~450 ms `status.php`, and 55 ms once marked lazy. + * + * @see \OCA\Versioniq\Repair\MarkPayloadCacheLazy + */ + public const PAYLOAD_CACHE_PREFIX = 'appstore.payload.'; + public const PAYLOAD_CACHE_TS_PREFIX = 'appstore.payload_ts.'; /** * What the current uncached fetch met, for the connection report @@ -271,13 +285,14 @@ private function readCachedPayload(string $appId, bool $ignoreTtl): ?array { Application::APP_ID, self::PAYLOAD_CACHE_TS_PREFIX . $appId, '0', + lazy: true, ); if ($cachedAt <= 0 || (time() - $cachedAt) >= self::PAYLOAD_CACHE_TTL_SECONDS) { return null; } } - $raw = $this->appConfig->getValueString(Application::APP_ID, self::PAYLOAD_CACHE_PREFIX . $appId, ''); + $raw = $this->appConfig->getValueString(Application::APP_ID, self::PAYLOAD_CACHE_PREFIX . $appId, '', lazy: true); if ($raw === '') { return null; } @@ -304,11 +319,13 @@ private function writeCachedPayload(string $appId, array $payload): void { Application::APP_ID, self::PAYLOAD_CACHE_PREFIX . $appId, json_encode($payload, JSON_THROW_ON_ERROR), + lazy: true, ); $this->appConfig->setValueString( Application::APP_ID, self::PAYLOAD_CACHE_TS_PREFIX . $appId, (string)time(), + lazy: true, ); } catch (Throwable) { // Cache write problems are non-fatal by design. diff --git a/tests/unit/Repair/MarkPayloadCacheLazyTest.php b/tests/unit/Repair/MarkPayloadCacheLazyTest.php new file mode 100644 index 00000000..98da9063 --- /dev/null +++ b/tests/unit/Repair/MarkPayloadCacheLazyTest.php @@ -0,0 +1,110 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://www.conduction.nl + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\Versioniq\Tests\Unit\Repair; + +use OCA\Versioniq\Repair\MarkPayloadCacheLazy; +use OCP\IAppConfig; +use OCP\Migration\IOutput; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use RuntimeException; + +/** + * phpcs:disable CustomSniffs.Functions.NamedParameters + * + * @covers \OCA\Versioniq\Repair\MarkPayloadCacheLazy + * + * @spec exclude Performance repair of a cache row's storage flag; no + * capability spec covers how the payload cache is stored. + */ +final class MarkPayloadCacheLazyTest extends TestCase { + + /** + * Only payload cache keys are flipped, and each one is flipped TO lazy. + * + * Asserts the keys and the flag, not a call count: flipping `pin.*` or + * `policy.*` would also move them out of the preload, and a count-only + * assertion would not notice. + * + * @return void + */ + public function testFlipsOnlyPayloadCacheKeysToLazy(): void { + $flipped = []; + + $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('getKeys')->with('versioniq')->willReturn([ + 'appstore.payload.mail', + 'appstore.payload_ts.mail', + 'appstore.payload.calendar', + 'appstore.api_base', + 'pin.openregister', + 'policy.hermiq', + ]); + $appConfig->method('updateLazy')->willReturnCallback( + function (string $app, string $key, bool $lazy) use (&$flipped): bool { + $this->assertSame('versioniq', $app); + $flipped[$key] = $lazy; + + return true; + }, + ); + + $step = new MarkPayloadCacheLazy($appConfig, $this->createMock(LoggerInterface::class)); + $step->run($this->createMock(IOutput::class)); + + $this->assertSame( + [ + 'appstore.payload.mail' => true, + 'appstore.payload_ts.mail' => true, + 'appstore.payload.calendar' => true, + ], + $flipped, + ); + } + + /** + * One key that cannot be updated is logged, and the rest still run. + * + * @return void + */ + public function testAFailingKeyDoesNotAbortTheStep(): void { + $flipped = []; + + $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('getKeys')->willReturn(['appstore.payload.mail', 'appstore.payload.deck']); + $appConfig->method('updateLazy')->willReturnCallback( + function (string $app, string $key, bool $lazy) use (&$flipped): bool { + if ($key === 'appstore.payload.mail') { + throw new RuntimeException('boom'); + } + $flipped[] = $key; + + return true; + }, + ); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('warning'); + + $step = new MarkPayloadCacheLazy($appConfig, $logger); + $step->run($this->createMock(IOutput::class)); + + $this->assertSame(['appstore.payload.deck'], $flipped); + } +} diff --git a/tests/unit/Service/Source/AppStoreSourceTest.php b/tests/unit/Service/Source/AppStoreSourceTest.php index dcc5ef28..8cf3c5b2 100644 --- a/tests/unit/Service/Source/AppStoreSourceTest.php +++ b/tests/unit/Service/Source/AppStoreSourceTest.php @@ -261,6 +261,70 @@ function (string $app, string $key, string $default = '') use (&$store): string $this->assertSame($first['versions'], $second['versions'], 'cached listing must match the fetched one'); } + /** + * The payload cache must be LAZY app config, on write and on read. + * + * Nextcloud preloads every eager app-config row of every app on every + * request. Written eagerly, this cache put 36 MB into that preload and + * ~300 ms onto every request of the instance (measured 2026-10-06). A + * test that only checks the round trip passes either way, so this one + * records the flag each call carries. + * + * @spec exclude Storage flag of an internal cache; the version-management + * spec says the payload is cached, not how the row is stored. + */ + public function testPayloadCacheIsReadAndWrittenLazy(): void { + $body = json_encode([ + 'data' => [[ + 'id' => 'openregister', + 'releases' => [['version' => '2.3.0']], + ]], + ], JSON_THROW_ON_ERROR); + + $response = $this->createMock(IResponse::class); + $response->method('getStatusCode')->willReturn(200); + $response->method('getBody')->willReturn($body); + + $client = $this->createMock(IClient::class); + $client->method('get')->willReturn($response); + $clientService = $this->createMock(IClientService::class); + $clientService->method('newClient')->willReturn($client); + + $store = []; + $writes = []; + $reads = []; + $config = $this->createMock(IConfig::class); + $config->method('getSystemValueString')->willReturn('28.0.0'); + $appConfig = $this->createMock(IAppConfig::class); + $appConfig->method('setValueString')->willReturnCallback( + function (string $app, string $key, string $value, bool $lazy = false) use (&$store, &$writes): bool { + $store[$key] = $value; + $writes[$key] = $lazy; + + return true; + }, + ); + $appConfig->method('getValueString')->willReturnCallback( + function (string $app, string $key, string $default = '', bool $lazy = false) use (&$store, &$reads): string { + $reads[$key] = $lazy; + + return $store[$key] ?? $default; + }, + ); + + $l10nFactory = $this->createMock(IFactory::class); + $l10nFactory->method('findLanguage')->willReturn('en'); + + $source = new AppStoreSource($clientService, $config, $appConfig, $l10nFactory); + $source->listVersions('openregister', $this->binding()); + $source->listVersions('openregister', $this->binding()); + + foreach (['appstore.payload.openregister', 'appstore.payload_ts.openregister'] as $key) { + $this->assertTrue($writes[$key] ?? false, $key . ' must be written lazy'); + $this->assertTrue($reads[$key] ?? false, $key . ' must be read lazy'); + } + } + public function testExpiredCacheIsRefetched(): void { $body = json_encode([ 'data' => [['id' => 'openregister', 'releases' => [['version' => '2.3.0']]]],