From 1c797ed03cdd839a42a92733fdacf87975f0e6f6 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Tue, 29 Sep 2026 15:00:49 +0200 Subject: [PATCH 1/4] feat: Add setup checks and verify WOPI connectivity Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Julius Knorr --- .github/workflows/cypress-e2e.yml | 13 ++-- .github/workflows/integration.yml | 12 ++- appinfo/routes.php | 1 + lib/AppInfo/Application.php | 5 ++ lib/Command/ActivateConfig.php | 8 ++ lib/Controller/SettingsController.php | 26 +++++-- lib/Service/CapabilitiesService.php | 12 +++ lib/Service/ConnectivityService.php | 76 +++++++++++++++++-- lib/SetupCheck/CollaboraUpdate.php | 72 ++++++++++++++++++ lib/SetupCheck/ConnectivityCheck.php | 51 +++++++++++++ .../lib/Controller/SettingsControllerTest.php | 32 +++++++- tests/psalm-baseline.xml | 19 +++-- tests/run-integration.sh | 5 +- tests/stub.phpstub | 12 +++ 14 files changed, 308 insertions(+), 36 deletions(-) create mode 100644 lib/SetupCheck/CollaboraUpdate.php create mode 100644 lib/SetupCheck/ConnectivityCheck.php diff --git a/.github/workflows/cypress-e2e.yml b/.github/workflows/cypress-e2e.yml index f2a27710f1..0c255b71a7 100644 --- a/.github/workflows/cypress-e2e.yml +++ b/.github/workflows/cypress-e2e.yml @@ -45,8 +45,8 @@ jobs: collabora: image: ${{ matrix.code-image == 'release' && 'collabora/code:latest' || 'juliushaertl/nc-code-nightly:latest' }} # zizmor: ignore[unpinned-images] env: - extra_params: '--o:ssl.enable=false --o:home_mode.enable=true' - aliasgroup1: 'http://172.17.0.1' + extra_params: '--o:ssl.enable=false --o:home_mode.enable=true --o:logging.level=debug' + aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" options: --cap-add SYS_ADMIN --cap-add SYS_CHROOT @@ -153,11 +153,10 @@ jobs: php occ app:enable --force spreed php occ app:list php occ config:system:set trusted_domains 1 --value="172.17.0.1" + php occ config:system:set overwrite.cli.url --value="http://172.17.0.1:8081" - php occ config:app:set richdocuments wopi_url --value="http://localhost:9980" - php occ config:app:set richdocuments public_wopi_url --value="http://localhost:9980" php occ config:system:set allow_local_remote_servers --value true --type bool - php occ richdocuments:activate-config + php occ richdocuments:activate-config --wopi-url="http://localhost:9980" --callback-url="http://172.17.0.1:8081" curl http://admin:admin@localhost:8081/ocs/v1.php/cloud/capabilities\?format\=json -H 'OCS-APIRequest: true' @@ -228,6 +227,10 @@ jobs: path: data/nextcloud.log retention-days: 5 + - name: Dump docker logs on failure + if: failure() + uses: jwalton/gh-docker-logs@v2 + summary: runs-on: ubuntu-latest timeout-minutes: 30 diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index b07f5d5495..581be16b5a 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -69,7 +69,7 @@ jobs: image: ${{ matrix.code-image == 'release' && 'collabora/code:latest' || 'juliushaertl/nc-code-nightly:latest' }} # zizmor: ignore[unpinned-images] env: extra_params: '--o:ssl.enable=false' - aliasgroup1: 'http://nextcloud' + aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" @@ -109,6 +109,7 @@ jobs: mkdir data echo '"\OC\Memcache\APCu","hashing_default_password"=>true];' > config/config.php ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ config:system:set overwrite.cli.url --value="http://172.17.0.1:8081" ./occ app:enable --force ${{ env.APP_NAME }} - name: Run ${{ matrix.scenarios }} integration tests @@ -144,7 +145,7 @@ jobs: image: ghcr.io/juliusknorr/nextcloud-dev-code:latest # zizmor: ignore[unpinned-images] env: extra_params: '--o:ssl.enable=false' - aliasgroup1: 'http://nextcloud' + aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" @@ -184,6 +185,7 @@ jobs: mkdir data echo '"\OC\Memcache\APCu","hashing_default_password"=>true];' > config/config.php ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ config:system:set overwrite.cli.url --value="http://172.17.0.1:8081" ./occ app:enable --force ${{ env.APP_NAME }} - name: Run ${{ matrix.scenarios }} integration tests @@ -221,7 +223,7 @@ jobs: image: ghcr.io/juliusknorr/nextcloud-dev-code:latest # zizmor: ignore[unpinned-images] env: extra_params: '--o:ssl.enable=false' - aliasgroup1: 'http://nextcloud' + aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" @@ -261,6 +263,7 @@ jobs: mkdir data echo '"\OC\Memcache\APCu","hashing_default_password"=>true];' > config/config.php ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ config:system:set overwrite.cli.url --value="http://172.17.0.1:8081" ./occ app:enable --force ${{ env.APP_NAME }} - name: Run ${{ matrix.scenarios }} integration tests @@ -306,7 +309,7 @@ jobs: image: ghcr.io/juliusknorr/nextcloud-dev-code:latest # zizmor: ignore[unpinned-images] env: extra_params: '--o:ssl.enable=false' - aliasgroup1: 'http://nextcloud' + aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" @@ -345,6 +348,7 @@ jobs: echo '"\OC\Memcache\APCu","hashing_default_password"=>true];' > config/config.php ./occ maintenance:install --verbose --database=oci --database-name=XE --database-host=127.0.0.1 --database-port=1521 --database-user=system --database-pass=rootpassword --admin-user admin --admin-pass password php -f index.php + ./occ config:system:set overwrite.cli.url --value="http://172.17.0.1:8081" ./occ app:enable --force ${{ env.APP_NAME }} - name: Run ${{ matrix.scenarios }} integration tests diff --git a/appinfo/routes.php b/appinfo/routes.php index e17267f6ce..2b6ca6e163 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -30,6 +30,7 @@ ['name' => 'settings#getSettings', 'url' => 'ajax/settings.php', 'verb' => 'GET'], ['name' => 'settings#updateWatermarkSettings', 'url' => 'settings/watermark', 'verb' => 'POST'], ['name' => 'settings#checkSettings', 'url' => 'settings/check', 'verb' => 'GET'], + ['name' => 'settings#accessCheck', 'url' => 'settings/access-check', 'verb' => 'GET'], ['name' => 'settings#demoServers', 'url' => 'settings/demo', 'verb' => 'GET'], ['name' => 'settings#getFontNames', 'url' => 'settings/fonts', 'verb' => 'GET'], ['name' => 'settings#getJsonFontList', 'url' => 'settings/fonts.json', 'verb' => 'GET'], diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 6ab1102f5f..ff2e3c0014 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -40,6 +40,8 @@ use OCA\Richdocuments\Preview\Pdf; use OCA\Richdocuments\Preview\Rtf; use OCA\Richdocuments\Reference\OfficeTargetReferenceProvider; +use OCA\Richdocuments\SetupCheck\CollaboraUpdate; +use OCA\Richdocuments\SetupCheck\ConnectivityCheck; use OCA\Richdocuments\Storage\SecureViewWrapper; use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationProvider; use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationTaskType; @@ -122,6 +124,9 @@ public function register(IRegistrationContext $context): void { $context->registerTaskProcessingProvider(TextToSpreadsheetProvider::class); $context->registerTaskProcessingProvider(SlideDeckGenerationProvider::class); $context->registerTaskProcessingTaskType(SlideDeckGenerationTaskType::class); + + $context->registerSetupCheck(CollaboraUpdate::class); + $context->registerSetupCheck(ConnectivityCheck::class); } #[\Override] diff --git a/lib/Command/ActivateConfig.php b/lib/Command/ActivateConfig.php index 804b491225..558fbfc3f3 100644 --- a/lib/Command/ActivateConfig.php +++ b/lib/Command/ActivateConfig.php @@ -73,6 +73,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } + try { + $this->connectivityService->testWopiAccess($output); + } catch (\Throwable $e) { + $output->writeln('Failed to verify WOPI connectivity'); + $output->writeln($e->getMessage()); + return 1; + } + try { $this->connectivityService->autoConfigurePublicUrl(); } catch (\Throwable $e) { diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 2c522fb633..051d3dd582 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -25,6 +25,8 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; @@ -38,7 +40,6 @@ use OCP\PreConditionNotMetException; use OCP\Util; use Psr\Log\LoggerInterface; -use Symfony\Component\Console\Output\NullOutput; class SettingsController extends Controller { /** Settings file URLs are generated with a settings token and carry no document context. */ @@ -76,11 +77,20 @@ public function __construct( parent::__construct($appName, $request); } + /** + * Callback for the Collabora WOPI access check. Collabora requires a 200 response and cannot + * handle a body without Content-Length, which the PHP built-in webserver does not add. + */ + #[PublicPage] + #[NoCSRFRequired] + public function accessCheck(): DataDisplayResponse { + $body = 'OK'; + return new DataDisplayResponse($body, Http::STATUS_OK, ['Content-Type' => 'text/plain', 'Content-Length' => (string)strlen($body)]); + } + public function checkSettings(): DataResponse { try { - $output = new NullOutput(); - $this->connectivityService->testDiscovery($output); - $this->connectivityService->testCapabilities($output); + $this->connectivityService->test(); } catch (\Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return new DataResponse([ @@ -192,9 +202,11 @@ public function setSettings( } try { - $output = new NullOutput(); - $this->connectivityService->testDiscovery($output); - $this->connectivityService->testCapabilities($output); + $this->connectivityService->testDiscovery(); + $this->connectivityService->testCapabilities(); + if ($wopi_url !== null) { + $this->connectivityService->testWopiAccess(); + } $this->connectivityService->autoConfigurePublicUrl(); } catch (\Throwable $e) { return new JSONResponse([ diff --git a/lib/Service/CapabilitiesService.php b/lib/Service/CapabilitiesService.php index cf338b81cf..63d003fc0c 100644 --- a/lib/Service/CapabilitiesService.php +++ b/lib/Service/CapabilitiesService.php @@ -110,6 +110,18 @@ public function getProductName(): string { return $this->l10n->t('Nextcloud Office (Collabora)'); } + public function isEnterprise(): bool { + return ($this->getCapabilities()['productName'] ?? false) === 'Collabora Online'; + } + + public function isCode(): bool { + return ($this->getCapabilities()['productName'] ?? false) === 'Collabora Online Development Edition'; + } + + public function hasWopiAccessCheck(): bool { + return $this->getCapabilities()['hasWopiAccessCheck'] ?? false; + } + public function hasOtherOOXMLApps(): bool { if ($this->appManager->isEnabledForUser('officeonline')) { return true; diff --git a/lib/Service/ConnectivityService.php b/lib/Service/ConnectivityService.php index cbe6b9c93e..4758b4da88 100644 --- a/lib/Service/ConnectivityService.php +++ b/lib/Service/ConnectivityService.php @@ -8,8 +8,12 @@ namespace OCA\Richdocuments\Service; use Exception; +use GuzzleHttp\Exception\ClientException; use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\WOPI\Parser; +use OCP\Http\Client\IClientService; +use OCP\IL10N; +use OCP\IURLGenerator; use Symfony\Component\Console\Output\OutputInterface; class ConnectivityService { @@ -17,35 +21,84 @@ public function __construct( private AppConfig $appConfig, private DiscoveryService $discoveryService, private CapabilitiesService $capabilitiesService, + private IClientService $clientService, + private IURLGenerator $urlGenerator, private Parser $parser, + private IL10N $l10n, ) { } /** * @throws Exception */ - public function testDiscovery(OutputInterface $output): void { + public function testDiscovery(?OutputInterface $output = null): void { $this->discoveryService->resetCache(); $this->discoveryService->fetch(); - $output->writeln('✓ Fetched /hosting/discovery endpoint'); + $output?->writeln('✓ Fetched /hosting/discovery endpoint'); $this->parser->getUrlSrcValue('application/vnd.openxmlformats-officedocument.wordprocessingml.document'); - $output->writeln('✓ Valid mimetype response'); + $output?->writeln('✓ Valid mimetype response'); // FIXME: Optional when allowing generic WOPI servers $this->parser->getUrlSrcValue('Capabilities'); - $output->writeln('✓ Valid capabilities entry'); + $output?->writeln('✓ Valid capabilities entry'); } - public function testCapabilities(OutputInterface $output): void { + public function testCapabilities(?OutputInterface $output = null): void { $this->capabilitiesService->resetCache(); $this->capabilitiesService->fetch(); - $output->writeln('✓ Fetched /hosting/capabilities endpoint'); + $output?->writeln('✓ Fetched /hosting/capabilities endpoint'); if ($this->capabilitiesService->getCapabilities() === []) { throw new \Exception('Empty capabilities, unexpected result from ' . $this->capabilitiesService->getCapabilitiesEndpoint()); } - $output->writeln('✓ Detected WOPI server: ' . $this->capabilitiesService->getServerProductName() . ' ' . $this->capabilitiesService->getProductVersion() . ''); + $output?->writeln('✓ Detected WOPI server: ' . $this->capabilitiesService->getServerProductName() . ' ' . $this->capabilitiesService->getProductVersion() . ''); + } + + public function testWopiAccess(?OutputInterface $output = null): void { + $client = $this->clientService->newClient(); + + if (!$this->capabilitiesService->hasWopiAccessCheck()) { + return; + } + + $url = str_replace('/hosting/capabilities', '/hosting/wopiAccessCheck', $this->capabilitiesService->getCapabilitiesEndpoint()); + $callbackUrl = $this->appConfig->getNextcloudUrl() ?: trim($this->urlGenerator->getAbsoluteURL(''), '/'); + + try { + $result = $client->post($url, array_merge(RemoteOptionsService::getDefaultOptions(), [ + 'body' => json_encode(['callbackUrl' => $callbackUrl . '/index.php/apps/richdocuments/settings/access-check']), 'headers' => ['Content-Type' => 'application/json'] + ])); + } catch (ClientException $e) { + $result = $e->getResponse(); + } + $response = json_decode($result->getBody(), true); + + $errorMessage = match ($response['status']) { + 'Ok' => null, + 'NotHttpSuccess' => $this->l10n->t('The connection was successful but the response to the request was not 200'), + 'HostNotFound' => $this->l10n->t('DNS error, the host is not known by the Collabora Online server'), + 'WopiHostNotAllowed' => $this->l10n->t('The host for this request is not allowed to be used as a WOPI Host, this is likely a configuration issue in coolwsd.xml'), + 'ConnectionAborted' => $this->l10n->t('The connection was aborted by the destination server'), + 'CertificateValidation' => $this->l10n->t('The certificate of the response is invalid or otherwise not accepted'), + 'SelfSignedCertificate' => $this->l10n->t('The certificate of the response is self-signed and not trusted by the system'), + 'ExpiredCertificate' => $this->l10n->t('The certificate of the response is expired'), + 'SslHandshakeFail' => $this->l10n->t('Couldn’t establish an SSL/TLS connection'), + 'MissingSsl' => $this->l10n->t('The response wasn’t using SSL/TLS contrary to expected'), + 'NotHttps' => $this->l10n->t('HTTPS is expected to connect to Collabora Online as the WOPI host uses it. This is necessary to prevent mixed content errors.'), + 'NoScheme' => $this->l10n->t('A scheme (http:// or https://) for the WOPI host URL must be specified'), + 'Timeout' => $this->l10n->t('The request didn’t get a response within the time frame allowed'), + 'UnspecifiedError' => $this->l10n->t('The response could not be read, for example because the connection was closed before the full response was received. Check the server logs of Collabora for more details.'), + default => $this->l10n->t('Unknown error "%s". Check the server logs of Collabora for more details.', [$response['status']]), + }; + + if ($errorMessage) { + throw new \Exception( + $this->l10n->t('The Collabora server could not properly reach the Nextcloud server at %s', [$callbackUrl]) . ' ' . $errorMessage + ); + } + + $output?->writeln('WOPI access was verified'); } /** @@ -60,4 +113,13 @@ public function autoConfigurePublicUrl(): void { $detectedUrl = $this->appConfig->domainOnly($determinedUrl); $this->appConfig->setAppValue('public_wopi_url', $detectedUrl); } + + /** + * @throws Exception + */ + public function test(?OutputInterface $output = null): void { + $this->testDiscovery($output); + $this->testCapabilities($output); + $this->testWopiAccess($output); + } } diff --git a/lib/SetupCheck/CollaboraUpdate.php b/lib/SetupCheck/CollaboraUpdate.php new file mode 100644 index 0000000000..b7a23baa20 --- /dev/null +++ b/lib/SetupCheck/CollaboraUpdate.php @@ -0,0 +1,72 @@ +l10n->t('Collabora server version check'); + } + + #[\Override] + public function run(): SetupResult { + $client = $this->clientService->newClient(); + + $url = null; + if ($this->capabilitiesService->isCode()) { + $url = 'https://rating.collaboraonline.com/UpdateCheck'; + } + + if ($this->capabilitiesService->isEnterprise()) { + $url = 'https://rating.collaboraonline.com/UpdateCheck?product=cool'; + } + + if ($url === null) { + return SetupResult::success(); + } + + // FIXME internet conection check config + + $response = $client->get($url, ['timeout' => 5]); + $response = json_decode($response->getBody(), true); + $latestVersion = $response['coolwsd_version'] ?? null; + + $installedVersion = $this->capabilitiesService->getProductVersion(); + + if ($latestVersion !== null && version_compare($latestVersion, $installedVersion, '>')) { + return SetupResult::warning($this->l10n->t('Collabora server version is out of date. Currently using %s, new version is available: %s', [$installedVersion, $latestVersion])); + } + + return SetupResult::success(); + } +} diff --git a/lib/SetupCheck/ConnectivityCheck.php b/lib/SetupCheck/ConnectivityCheck.php new file mode 100644 index 0000000000..04344e54f5 --- /dev/null +++ b/lib/SetupCheck/ConnectivityCheck.php @@ -0,0 +1,51 @@ +l10n->t('Collabora server connectivity check'); + } + + #[\Override] + public function run(): SetupResult { + try { + $this->connectivityService->test(); + } catch (\Exception $e) { + return SetupResult::error($this->l10n->t('Collabora is not configured properly:') . ' ' . $e->getMessage()); + } + + return SetupResult::success(); + } +} diff --git a/tests/lib/Controller/SettingsControllerTest.php b/tests/lib/Controller/SettingsControllerTest.php index da2980c4fd..10e8f1b31f 100644 --- a/tests/lib/Controller/SettingsControllerTest.php +++ b/tests/lib/Controller/SettingsControllerTest.php @@ -39,6 +39,7 @@ class SettingsControllerTest extends TestCase { private IUserManager $userManager; private PermissionManager $permissionManager; private SettingsService $settingsService; + private ConnectivityService $connectivityService; protected function setUp(): void { parent::setUp(); @@ -54,6 +55,7 @@ protected function setUp(): void { $this->createMock(ISystemTagObjectMapper::class), ); $this->settingsService = $this->createMock(SettingsService::class); + $this->connectivityService = $this->createMock(ConnectivityService::class); } private function makeController(): SettingsController { @@ -63,7 +65,7 @@ private function makeController(): SettingsController { $this->createMock(IL10N::class), $this->createMock(AppConfig::class), $this->createMock(IConfig::class), - $this->createMock(ConnectivityService::class), + $this->connectivityService, $this->createMock(DiscoveryService::class), $this->createMock(CapabilitiesService::class), $this->createMock(DemoService::class), @@ -212,4 +214,32 @@ public function testGetSettingsFileScopesToTheEditorNotTheOwner(): void { $this->assertSame(Http::STATUS_OK, $response->getStatus()); } + + public function testAccessCheckSendsContentLength(): void { + $response = $this->makeController()->accessCheck(); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame('OK', $response->render()); + $this->assertSame('2', $response->getHeaders()['Content-Length']); + } + + /** @dataProvider dataSetSettingsWopiAccessCheck */ + public function testSetSettingsOnlyChecksWopiAccessWhenWopiUrlChanges(?string $wopiUrl, int $expectedChecks): void { + $this->connectivityService->expects($this->once())->method('testDiscovery'); + $this->connectivityService->expects($this->once())->method('testCapabilities'); + $this->connectivityService->expects($this->exactly($expectedChecks))->method('testWopiAccess'); + + $response = $this->makeController()->setSettings( + $wopiUrl, null, null, null, null, null, null, null, null, 'client-id', 'secret', + ); + + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + } + + public static function dataSetSettingsWopiAccessCheck(): array { + return [ + 'wopi_url unchanged' => [null, 0], + 'wopi_url changed' => ['https://collabora.example', 1], + ]; + } } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 9e0fe758a0..3a9150692b 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -47,12 +47,6 @@ - - - - - - fetchPreview($template, $x, $y, $a, $forceIcon, $mode)]]> @@ -96,10 +90,15 @@ - - - - + + post($url, array_merge(RemoteOptionsService::getDefaultOptions(), [ + 'body' => json_encode(['callbackUrl' => $callbackUrl . '/index.php/apps/richdocuments/settings/access-check']), 'headers' => ['Content-Type' => 'application/json'] + ])); + } catch (ClientException $e) { + $result = $e->getResponse(); + }]]> + diff --git a/tests/run-integration.sh b/tests/run-integration.sh index d04e46bc75..bb16449a32 100755 --- a/tests/run-integration.sh +++ b/tests/run-integration.sh @@ -44,14 +44,15 @@ curl --fail http://localhost:$PORT_COOL/hosting/capabilities PHP_CLI_SERVER_WORKERS=10 php -S localhost:$PORT_SERVERA -t $OC_PATH & PHPPIDA=$! -PHP_CLI_SERVER_WORKERS=10 php -S localhost:$PORT_SERVERB -t $OC_PATH & +PHP_CLI_SERVER_WORKERS=10 php -S 0.0.0.0:$PORT_SERVERB -t $OC_PATH & PHPPIDB=$! +$OCC config:system:set allow_local_remote_servers --value true --type bool +$OCC config:system:set trusted_domains 1 --value="172.17.0.1" $OCC config:app:set richdocuments wopi_url --value="http://localhost:9980" $OCC config:app:set richdocuments public_wopi_url --value="http://localhost:9980" $OCC richdocuments:activate-config -$OCC config:system:set allow_local_remote_servers --value true --type bool $OCC config:system:set gs.trustedHosts 0 --value="localhost:$PORT_SERVERA" $OCC config:system:set gs.trustedHosts 1 --value="localhost:$PORT_SERVERB" diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 8c88d79ceb..3d8be10737 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -203,6 +203,18 @@ namespace Symfony\Component\HttpFoundation { } } +namespace Symfony\Component\Console\Output { + interface OutputInterface { + public function writeln(string $text): void; + } +} + +namespace GuzzleHttp\Exception { + class ClientException { + public function getResponse(); + } +} + #[\Attribute(Attribute::TARGET_PARAMETER)] class SensitiveParameter { } From f0bb531fc88c8328bc4cc64da172883a72d0a6d4 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Tue, 29 Sep 2026 15:00:49 +0200 Subject: [PATCH 2/4] feat: Add setup check for missing WOPI proof keys Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Julius Knorr --- lib/AppInfo/Application.php | 2 + lib/SetupCheck/ProofKeyCheck.php | 49 ++++++++++++++++++++++ tests/lib/SetupCheck/ProofKeyCheckTest.php | 39 +++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 lib/SetupCheck/ProofKeyCheck.php create mode 100644 tests/lib/SetupCheck/ProofKeyCheckTest.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index ff2e3c0014..e2bf29ea2b 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -42,6 +42,7 @@ use OCA\Richdocuments\Reference\OfficeTargetReferenceProvider; use OCA\Richdocuments\SetupCheck\CollaboraUpdate; use OCA\Richdocuments\SetupCheck\ConnectivityCheck; +use OCA\Richdocuments\SetupCheck\ProofKeyCheck; use OCA\Richdocuments\Storage\SecureViewWrapper; use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationProvider; use OCA\Richdocuments\TaskProcessing\SlideDeckGenerationTaskType; @@ -127,6 +128,7 @@ public function register(IRegistrationContext $context): void { $context->registerSetupCheck(CollaboraUpdate::class); $context->registerSetupCheck(ConnectivityCheck::class); + $context->registerSetupCheck(ProofKeyCheck::class); } #[\Override] diff --git a/lib/SetupCheck/ProofKeyCheck.php b/lib/SetupCheck/ProofKeyCheck.php new file mode 100644 index 0000000000..7d713ac800 --- /dev/null +++ b/lib/SetupCheck/ProofKeyCheck.php @@ -0,0 +1,49 @@ +l10n->t('Collabora WOPI proof keys'); + } + + #[\Override] + public function run(): SetupResult { + if ($this->appConfig->getCollaboraUrlInternal() === '' || $this->discoveryService->hasProofKey()) { + return SetupResult::success(); + } + + return SetupResult::warning( + $this->l10n->t('The Collabora server does not provide WOPI proof keys, so requests from it cannot be verified. Generate proof keys on the Collabora server.'), + self::DOCS_URL, + ); + } +} diff --git a/tests/lib/SetupCheck/ProofKeyCheckTest.php b/tests/lib/SetupCheck/ProofKeyCheckTest.php new file mode 100644 index 0000000000..786c321b63 --- /dev/null +++ b/tests/lib/SetupCheck/ProofKeyCheckTest.php @@ -0,0 +1,39 @@ + ['', false, SetupResult::SUCCESS], + 'proof key present' => ['https://collabora.example', true, SetupResult::SUCCESS], + 'proof key missing' => ['https://collabora.example', false, SetupResult::WARNING], + ]; + } + + /** @dataProvider dataRun */ + public function testRun(string $wopiUrl, bool $hasProofKey, string $expected): void { + $appConfig = $this->createStub(AppConfig::class); + $appConfig->method('getCollaboraUrlInternal')->willReturn($wopiUrl); + $discoveryService = $this->createStub(DiscoveryService::class); + $discoveryService->method('hasProofKey')->willReturn($hasProofKey); + + $check = new ProofKeyCheck($this->createStub(IL10N::class), $appConfig, $discoveryService); + + $this->assertSame($expected, $check->run()->getSeverity()); + } +} From 61aee934ce80321f8a847c70442b30f686272492 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Mon, 12 Oct 2026 19:58:16 +0200 Subject: [PATCH 3/4] fix(setup-check): address review feedback - Keep cached discovery and capabilities when a connectivity check fails - Skip the connectivity check when Collabora is not configured - Respect has_internet_connection and handle failures in the update check - Handle server errors and invalid responses from wopiAccessCheck - Pin gh-docker-logs action Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Julius Knorr --- .github/workflows/cypress-e2e.yml | 2 +- lib/Service/ConnectivityService.php | 18 ++-- lib/SetupCheck/CollaboraUpdate.php | 35 ++++---- lib/SetupCheck/ConnectivityCheck.php | 14 ++-- tests/lib/Service/ConnectivityServiceTest.php | 82 +++++++++++++++++++ tests/lib/SetupCheck/CollaboraUpdateTest.php | 64 +++++++++++++++ .../lib/SetupCheck/ConnectivityCheckTest.php | 42 ++++++++++ tests/psalm-baseline.xml | 11 --- tests/stub.phpstub | 2 +- 9 files changed, 222 insertions(+), 48 deletions(-) create mode 100644 tests/lib/Service/ConnectivityServiceTest.php create mode 100644 tests/lib/SetupCheck/CollaboraUpdateTest.php create mode 100644 tests/lib/SetupCheck/ConnectivityCheckTest.php diff --git a/.github/workflows/cypress-e2e.yml b/.github/workflows/cypress-e2e.yml index 0c255b71a7..89dcda5af9 100644 --- a/.github/workflows/cypress-e2e.yml +++ b/.github/workflows/cypress-e2e.yml @@ -229,7 +229,7 @@ jobs: - name: Dump docker logs on failure if: failure() - uses: jwalton/gh-docker-logs@v2 + uses: jwalton/gh-docker-logs@2741064ab9d7af54b0b1ffb6076cf64c16f0220e # v2.2.2 summary: runs-on: ubuntu-latest diff --git a/lib/Service/ConnectivityService.php b/lib/Service/ConnectivityService.php index 4758b4da88..23c3f632f0 100644 --- a/lib/Service/ConnectivityService.php +++ b/lib/Service/ConnectivityService.php @@ -8,7 +8,7 @@ namespace OCA\Richdocuments\Service; use Exception; -use GuzzleHttp\Exception\ClientException; +use GuzzleHttp\Exception\BadResponseException; use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\WOPI\Parser; use OCP\Http\Client\IClientService; @@ -32,7 +32,6 @@ public function __construct( * @throws Exception */ public function testDiscovery(?OutputInterface $output = null): void { - $this->discoveryService->resetCache(); $this->discoveryService->fetch(); $output?->writeln('✓ Fetched /hosting/discovery endpoint'); @@ -45,7 +44,6 @@ public function testDiscovery(?OutputInterface $output = null): void { } public function testCapabilities(?OutputInterface $output = null): void { - $this->capabilitiesService->resetCache(); $this->capabilitiesService->fetch(); $output?->writeln('✓ Fetched /hosting/capabilities endpoint'); @@ -56,12 +54,12 @@ public function testCapabilities(?OutputInterface $output = null): void { } public function testWopiAccess(?OutputInterface $output = null): void { - $client = $this->clientService->newClient(); - if (!$this->capabilitiesService->hasWopiAccessCheck()) { return; } + $client = $this->clientService->newClient(); + $url = str_replace('/hosting/capabilities', '/hosting/wopiAccessCheck', $this->capabilitiesService->getCapabilitiesEndpoint()); $callbackUrl = $this->appConfig->getNextcloudUrl() ?: trim($this->urlGenerator->getAbsoluteURL(''), '/'); @@ -69,12 +67,12 @@ public function testWopiAccess(?OutputInterface $output = null): void { $result = $client->post($url, array_merge(RemoteOptionsService::getDefaultOptions(), [ 'body' => json_encode(['callbackUrl' => $callbackUrl . '/index.php/apps/richdocuments/settings/access-check']), 'headers' => ['Content-Type' => 'application/json'] ])); - } catch (ClientException $e) { + } catch (BadResponseException $e) { $result = $e->getResponse(); } - $response = json_decode($result->getBody(), true); + $status = json_decode((string)$result->getBody(), true)['status'] ?? throw new \Exception('Unexpected response from ' . $url); - $errorMessage = match ($response['status']) { + $errorMessage = match ($status) { 'Ok' => null, 'NotHttpSuccess' => $this->l10n->t('The connection was successful but the response to the request was not 200'), 'HostNotFound' => $this->l10n->t('DNS error, the host is not known by the Collabora Online server'), @@ -89,7 +87,7 @@ public function testWopiAccess(?OutputInterface $output = null): void { 'NoScheme' => $this->l10n->t('A scheme (http:// or https://) for the WOPI host URL must be specified'), 'Timeout' => $this->l10n->t('The request didn’t get a response within the time frame allowed'), 'UnspecifiedError' => $this->l10n->t('The response could not be read, for example because the connection was closed before the full response was received. Check the server logs of Collabora for more details.'), - default => $this->l10n->t('Unknown error "%s". Check the server logs of Collabora for more details.', [$response['status']]), + default => $this->l10n->t('Unknown error "%s". Check the server logs of Collabora for more details.', [$status]), }; if ($errorMessage) { @@ -98,7 +96,7 @@ public function testWopiAccess(?OutputInterface $output = null): void { ); } - $output?->writeln('WOPI access was verified'); + $output?->writeln('✓ Verified WOPI access from Collabora to ' . $callbackUrl . ''); } /** diff --git a/lib/SetupCheck/CollaboraUpdate.php b/lib/SetupCheck/CollaboraUpdate.php index b7a23baa20..1191b1e781 100644 --- a/lib/SetupCheck/CollaboraUpdate.php +++ b/lib/SetupCheck/CollaboraUpdate.php @@ -11,8 +11,8 @@ use OCA\Richdocuments\Service\CapabilitiesService; use OCP\Http\Client\IClientService; +use OCP\IConfig; use OCP\IL10N; -use OCP\IURLGenerator; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; use Psr\Log\LoggerInterface; @@ -22,7 +22,7 @@ class CollaboraUpdate implements ISetupCheck { public function __construct( protected IL10N $l10n, protected CapabilitiesService $capabilitiesService, - protected IURLGenerator $urlGenerator, + protected IConfig $config, protected IClientService $clientService, protected LoggerInterface $logger, ) { @@ -40,28 +40,27 @@ public function getName(): string { #[\Override] public function run(): SetupResult { - $client = $this->clientService->newClient(); - - $url = null; - if ($this->capabilitiesService->isCode()) { - $url = 'https://rating.collaboraonline.com/UpdateCheck'; - } - - if ($this->capabilitiesService->isEnterprise()) { - $url = 'https://rating.collaboraonline.com/UpdateCheck?product=cool'; + $installedVersion = $this->capabilitiesService->getProductVersion(); + if ($installedVersion === null || !$this->config->getSystemValueBool('has_internet_connection', true)) { + return SetupResult::success(); } + $url = match (true) { + $this->capabilitiesService->isCode() => 'https://rating.collaboraonline.com/UpdateCheck', + $this->capabilitiesService->isEnterprise() => 'https://rating.collaboraonline.com/UpdateCheck?product=cool', + default => null, + }; if ($url === null) { return SetupResult::success(); } - // FIXME internet conection check config - - $response = $client->get($url, ['timeout' => 5]); - $response = json_decode($response->getBody(), true); - $latestVersion = $response['coolwsd_version'] ?? null; - - $installedVersion = $this->capabilitiesService->getProductVersion(); + try { + $response = $this->clientService->newClient()->get($url, ['timeout' => 5]); + $latestVersion = json_decode((string)$response->getBody(), true)['coolwsd_version'] ?? null; + } catch (\Exception $e) { + $this->logger->info('Failed to fetch latest Collabora version', ['exception' => $e]); + return SetupResult::info($this->l10n->t('Could not check for Collabora server updates')); + } if ($latestVersion !== null && version_compare($latestVersion, $installedVersion, '>')) { return SetupResult::warning($this->l10n->t('Collabora server version is out of date. Currently using %s, new version is available: %s', [$installedVersion, $latestVersion])); diff --git a/lib/SetupCheck/ConnectivityCheck.php b/lib/SetupCheck/ConnectivityCheck.php index 04344e54f5..cb2f34ab39 100644 --- a/lib/SetupCheck/ConnectivityCheck.php +++ b/lib/SetupCheck/ConnectivityCheck.php @@ -9,22 +9,18 @@ namespace OCA\Richdocuments\SetupCheck; +use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\Service\ConnectivityService; -use OCP\Http\Client\IClientService; use OCP\IL10N; -use OCP\IURLGenerator; use OCP\SetupCheck\ISetupCheck; use OCP\SetupCheck\SetupResult; -use Psr\Log\LoggerInterface; class ConnectivityCheck implements ISetupCheck { public function __construct( protected IL10N $l10n, + protected AppConfig $appConfig, protected ConnectivityService $connectivityService, - protected IURLGenerator $urlGenerator, - protected IClientService $clientService, - protected LoggerInterface $logger, ) { } @@ -40,9 +36,13 @@ public function getName(): string { #[\Override] public function run(): SetupResult { + if ($this->appConfig->getCollaboraUrlInternal() === '') { + return SetupResult::success(); + } + try { $this->connectivityService->test(); - } catch (\Exception $e) { + } catch (\Throwable $e) { return SetupResult::error($this->l10n->t('Collabora is not configured properly:') . ' ' . $e->getMessage()); } diff --git a/tests/lib/Service/ConnectivityServiceTest.php b/tests/lib/Service/ConnectivityServiceTest.php new file mode 100644 index 0000000000..c1b4b256a1 --- /dev/null +++ b/tests/lib/Service/ConnectivityServiceTest.php @@ -0,0 +1,82 @@ +createStub(AppConfig::class); + $appConfig->method('getNextcloudUrl')->willReturn('https://nextcloud.example'); + $capabilitiesService = $this->createStub(CapabilitiesService::class); + $capabilitiesService->method('hasWopiAccessCheck')->willReturn($hasWopiAccessCheck); + $capabilitiesService->method('getCapabilitiesEndpoint')->willReturn('https://collabora.example/hosting/capabilities'); + $response = $this->createStub(IResponse::class); + $response->method('getBody')->willReturn($body); + $client->method('post')->willReturn($response); + $clientService = $this->createStub(IClientService::class); + $clientService->method('newClient')->willReturn($client); + $l10n = $this->createStub(IL10N::class); + $l10n->method('t')->willReturnArgument(0); + + return new ConnectivityService( + $appConfig, + $this->createStub(DiscoveryService::class), + $capabilitiesService, + $clientService, + $this->createStub(IURLGenerator::class), + $this->createStub(Parser::class), + $l10n, + ); + } + + public function testWopiAccessSkippedWithoutCapability(): void { + $client = $this->createMock(IClient::class); + $client->expects($this->never())->method('post'); + + $this->makeService(false, null, $client)->testWopiAccess(); + } + + public function testWopiAccessOk(): void { + $client = $this->createMock(IClient::class); + $service = $this->makeService(true, '{"status":"Ok"}', $client); + $client->expects($this->once())->method('post')->with( + 'https://collabora.example/hosting/wopiAccessCheck', + $this->callback(fn (array $options) => json_decode($options['body'], true)['callbackUrl'] === 'https://nextcloud.example/index.php/apps/richdocuments/settings/access-check'), + ); + + $service->testWopiAccess(); + } + + public static function dataWopiAccessFailure(): array { + return [ + 'known error' => ['{"status":"WopiHostNotAllowed"}'], + 'unknown error' => ['{"status":"SomethingNew"}'], + 'invalid response' => [''], + ]; + } + + /** @dataProvider dataWopiAccessFailure */ + public function testWopiAccessFailure(string $body): void { + $this->expectException(\Exception::class); + + $this->makeService(true, $body, $this->createStub(IClient::class))->testWopiAccess(); + } +} diff --git a/tests/lib/SetupCheck/CollaboraUpdateTest.php b/tests/lib/SetupCheck/CollaboraUpdateTest.php new file mode 100644 index 0000000000..a942fbac38 --- /dev/null +++ b/tests/lib/SetupCheck/CollaboraUpdateTest.php @@ -0,0 +1,64 @@ + [null, true, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], + 'no internet connection' => ['25.04.1', false, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], + 'up to date' => ['25.04.2', true, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], + 'outdated' => ['25.04.1', true, '{"coolwsd_version":"25.04.2"}', SetupResult::WARNING], + 'unexpected response' => ['25.04.1', true, 'error', SetupResult::SUCCESS], + 'request failed' => ['25.04.1', true, null, SetupResult::INFO], + ]; + } + + /** @dataProvider dataRun */ + public function testRun(?string $installedVersion, bool $hasInternet, ?string $body, string $expected): void { + $capabilitiesService = $this->createStub(CapabilitiesService::class); + $capabilitiesService->method('getProductVersion')->willReturn($installedVersion); + $capabilitiesService->method('isCode')->willReturn(true); + $config = $this->createStub(IConfig::class); + $config->method('getSystemValueBool')->willReturn($hasInternet); + + $client = $this->createStub(IClient::class); + if ($body === null) { + $client->method('get')->willThrowException(new \Exception('timeout')); + } else { + $response = $this->createStub(IResponse::class); + $response->method('getBody')->willReturn($body); + $client->method('get')->willReturn($response); + } + $clientService = $this->createStub(IClientService::class); + $clientService->method('newClient')->willReturn($client); + + $check = new CollaboraUpdate( + $this->createStub(IL10N::class), + $capabilitiesService, + $config, + $clientService, + $this->createStub(LoggerInterface::class), + ); + + $this->assertSame($expected, $check->run()->getSeverity()); + } +} diff --git a/tests/lib/SetupCheck/ConnectivityCheckTest.php b/tests/lib/SetupCheck/ConnectivityCheckTest.php new file mode 100644 index 0000000000..9d67ee43fa --- /dev/null +++ b/tests/lib/SetupCheck/ConnectivityCheckTest.php @@ -0,0 +1,42 @@ + ['', false, 0, SetupResult::SUCCESS], + 'reachable' => ['https://collabora.example', false, 1, SetupResult::SUCCESS], + 'unreachable' => ['https://collabora.example', true, 1, SetupResult::ERROR], + ]; + } + + /** @dataProvider dataRun */ + public function testRun(string $wopiUrl, bool $fails, int $expectedTests, string $expected): void { + $appConfig = $this->createStub(AppConfig::class); + $appConfig->method('getCollaboraUrlInternal')->willReturn($wopiUrl); + $connectivityService = $this->createMock(ConnectivityService::class); + $test = $connectivityService->expects($this->exactly($expectedTests))->method('test'); + if ($fails) { + $test->willThrowException(new \Exception('unreachable')); + } + + $check = new ConnectivityCheck($this->createStub(IL10N::class), $appConfig, $connectivityService); + + $this->assertSame($expected, $check->run()->getSeverity()); + } +} diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 3a9150692b..76c513953b 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -89,17 +89,6 @@ - - - post($url, array_merge(RemoteOptionsService::getDefaultOptions(), [ - 'body' => json_encode(['callbackUrl' => $callbackUrl . '/index.php/apps/richdocuments/settings/access-check']), 'headers' => ['Content-Type' => 'application/json'] - ])); - } catch (ClientException $e) { - $result = $e->getResponse(); - }]]> - - diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 3d8be10737..ee014f61b2 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -210,7 +210,7 @@ namespace Symfony\Component\Console\Output { } namespace GuzzleHttp\Exception { - class ClientException { + class BadResponseException extends \Exception { public function getResponse(); } } From d5d6f7e6f4927ed09c0ce4c5c30962e798e10cb8 Mon Sep 17 00:00:00 2001 From: Julius Knorr Date: Mon, 12 Oct 2026 21:00:58 +0200 Subject: [PATCH 4/4] fix(setup-check): cache update check and refresh fetched capabilities - Cache the latest Collabora version for 6 hours - Reset in-memory capabilities after fetching them - Drop debug logging from cypress Collabora container Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Julius Knorr --- .github/workflows/cypress-e2e.yml | 2 +- lib/Service/CachedRequestService.php | 2 +- lib/Service/CapabilitiesService.php | 6 ++ lib/SetupCheck/CollaboraUpdate.php | 23 +++++-- tests/lib/Service/CapabilitiesServiceTest.php | 65 +++++++++++++++++++ tests/lib/SetupCheck/CollaboraUpdateTest.php | 11 +++- 6 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 tests/lib/Service/CapabilitiesServiceTest.php diff --git a/.github/workflows/cypress-e2e.yml b/.github/workflows/cypress-e2e.yml index 89dcda5af9..2d4c5d1a57 100644 --- a/.github/workflows/cypress-e2e.yml +++ b/.github/workflows/cypress-e2e.yml @@ -45,7 +45,7 @@ jobs: collabora: image: ${{ matrix.code-image == 'release' && 'collabora/code:latest' || 'juliushaertl/nc-code-nightly:latest' }} # zizmor: ignore[unpinned-images] env: - extra_params: '--o:ssl.enable=false --o:home_mode.enable=true --o:logging.level=debug' + extra_params: '--o:ssl.enable=false --o:home_mode.enable=true' aliasgroup1: 'http://172.17.0.1:8081' ports: - "9980:9980" diff --git a/lib/Service/CachedRequestService.php b/lib/Service/CachedRequestService.php index 319578e120..8437310d31 100644 --- a/lib/Service/CachedRequestService.php +++ b/lib/Service/CachedRequestService.php @@ -66,7 +66,7 @@ public function getLastUpdate(): ?int { * @return string * @throws \Exception */ - final public function fetch(): string { + public function fetch(): string { $cache = $this->cacheFactory->createDistributed('richdocuments'); $client = $this->clientService->newClient(); diff --git a/lib/Service/CapabilitiesService.php b/lib/Service/CapabilitiesService.php index 63d003fc0c..8fd30d84df 100644 --- a/lib/Service/CapabilitiesService.php +++ b/lib/Service/CapabilitiesService.php @@ -142,6 +142,12 @@ public function getCapabilitiesEndpoint(): ?string { return rtrim($remoteHost, '/') . '/hosting/capabilities'; } + #[\Override] + public function fetch(): string { + $this->capabilities = null; + return parent::fetch(); + } + #[\Override] protected function sendRequest(IClient $client): string { $response = $client->get($this->getCapabilitiesEndpoint(), $this->getDefaultRequestOptions()); diff --git a/lib/SetupCheck/CollaboraUpdate.php b/lib/SetupCheck/CollaboraUpdate.php index 1191b1e781..667f0bf99a 100644 --- a/lib/SetupCheck/CollaboraUpdate.php +++ b/lib/SetupCheck/CollaboraUpdate.php @@ -11,6 +11,7 @@ use OCA\Richdocuments\Service\CapabilitiesService; use OCP\Http\Client\IClientService; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\SetupCheck\ISetupCheck; @@ -18,12 +19,14 @@ use Psr\Log\LoggerInterface; class CollaboraUpdate implements ISetupCheck { + private const CACHE_TTL = 6 * 3600; public function __construct( protected IL10N $l10n, protected CapabilitiesService $capabilitiesService, protected IConfig $config, protected IClientService $clientService, + protected ICacheFactory $cacheFactory, protected LoggerInterface $logger, ) { } @@ -54,15 +57,21 @@ public function run(): SetupResult { return SetupResult::success(); } - try { - $response = $this->clientService->newClient()->get($url, ['timeout' => 5]); - $latestVersion = json_decode((string)$response->getBody(), true)['coolwsd_version'] ?? null; - } catch (\Exception $e) { - $this->logger->info('Failed to fetch latest Collabora version', ['exception' => $e]); - return SetupResult::info($this->l10n->t('Could not check for Collabora server updates')); + $cache = $this->cacheFactory->createDistributed('richdocuments'); + $cacheKey = 'latest_version_' . md5($url); + $latestVersion = $cache->get($cacheKey); + if ($latestVersion === null) { + try { + $response = $this->clientService->newClient()->get($url, ['timeout' => 5]); + $latestVersion = (string)(json_decode((string)$response->getBody(), true)['coolwsd_version'] ?? ''); + } catch (\Exception $e) { + $this->logger->info('Failed to fetch latest Collabora version', ['exception' => $e]); + return SetupResult::info($this->l10n->t('Could not check for Collabora server updates')); + } + $cache->set($cacheKey, $latestVersion, self::CACHE_TTL); } - if ($latestVersion !== null && version_compare($latestVersion, $installedVersion, '>')) { + if ($latestVersion !== '' && version_compare($latestVersion, $installedVersion, '>')) { return SetupResult::warning($this->l10n->t('Collabora server version is out of date. Currently using %s, new version is available: %s', [$installedVersion, $latestVersion])); } diff --git a/tests/lib/Service/CapabilitiesServiceTest.php b/tests/lib/Service/CapabilitiesServiceTest.php new file mode 100644 index 0000000000..1b9b2cb1a3 --- /dev/null +++ b/tests/lib/Service/CapabilitiesServiceTest.php @@ -0,0 +1,65 @@ +createStub(ICache::class); + $cache->method('get')->willReturnOnConsecutiveCalls('{"productVersion":"1.0"}', '{"productVersion":"2.0"}'); + $cacheFactory = $this->createStub(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn($cache); + + $appData = $this->createStub(IAppData::class); + $appData->method('getFolder')->willReturn($this->createStub(ISimpleFolder::class)); + $appDataFactory = $this->createStub(IAppDataFactory::class); + $appDataFactory->method('get')->willReturn($appData); + + $response = $this->createStub(IResponse::class); + $response->method('getBody')->willReturn('{"productVersion":"2.0"}'); + $client = $this->createStub(IClient::class); + $client->method('get')->willReturn($response); + $clientService = $this->createStub(IClientService::class); + $clientService->method('newClient')->willReturn($client); + + $config = $this->createStub(IConfig::class); + $config->method('getAppValue')->willReturn('https://collabora.example'); + + $service = new CapabilitiesService( + $clientService, + $cacheFactory, + $appDataFactory, + $this->createStub(IAppConfig::class), + $this->createStub(LoggerInterface::class), + $config, + $this->createStub(IAppManager::class), + $this->createStub(IL10N::class), + ); + + $this->assertSame('1.0', $service->getProductVersion()); + $service->fetch(); + $this->assertSame('2.0', $service->getProductVersion()); + } +} diff --git a/tests/lib/SetupCheck/CollaboraUpdateTest.php b/tests/lib/SetupCheck/CollaboraUpdateTest.php index a942fbac38..8c3abcabbb 100644 --- a/tests/lib/SetupCheck/CollaboraUpdateTest.php +++ b/tests/lib/SetupCheck/CollaboraUpdateTest.php @@ -14,6 +14,8 @@ use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; use OCP\SetupCheck\SetupResult; @@ -24,6 +26,8 @@ class CollaboraUpdateTest extends TestCase { public static function dataRun(): array { return [ 'not configured' => [null, true, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], + 'cached outdated' => ['25.04.1', true, null, SetupResult::WARNING, '25.04.2'], + 'cached unknown' => ['25.04.1', true, null, SetupResult::SUCCESS, ''], 'no internet connection' => ['25.04.1', false, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], 'up to date' => ['25.04.2', true, '{"coolwsd_version":"25.04.2"}', SetupResult::SUCCESS], 'outdated' => ['25.04.1', true, '{"coolwsd_version":"25.04.2"}', SetupResult::WARNING], @@ -33,7 +37,7 @@ public static function dataRun(): array { } /** @dataProvider dataRun */ - public function testRun(?string $installedVersion, bool $hasInternet, ?string $body, string $expected): void { + public function testRun(?string $installedVersion, bool $hasInternet, ?string $body, string $expected, ?string $cachedVersion = null): void { $capabilitiesService = $this->createStub(CapabilitiesService::class); $capabilitiesService->method('getProductVersion')->willReturn($installedVersion); $capabilitiesService->method('isCode')->willReturn(true); @@ -50,12 +54,17 @@ public function testRun(?string $installedVersion, bool $hasInternet, ?string $b } $clientService = $this->createStub(IClientService::class); $clientService->method('newClient')->willReturn($client); + $cache = $this->createStub(ICache::class); + $cache->method('get')->willReturn($cachedVersion); + $cacheFactory = $this->createStub(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn($cache); $check = new CollaboraUpdate( $this->createStub(IL10N::class), $capabilitiesService, $config, $clientService, + $cacheFactory, $this->createStub(LoggerInterface::class), );