diff --git a/lib/Service/CachedRequestService.php b/lib/Service/CachedRequestService.php index 8437310d31..f9e6b83627 100644 --- a/lib/Service/CachedRequestService.php +++ b/lib/Service/CachedRequestService.php @@ -42,11 +42,19 @@ public function get(): ?string { return $cached; } - $folder = $this->getAppDataFolder(); - if ($folder->fileExists($this->cacheKey)) { - $value = $folder->getFile($this->cacheKey)->getContent(); - $cache->set($this->cacheKey, $value, 3600); - return $value; + try { + $folder = $this->getAppDataFolder(); + if ($folder->fileExists($this->cacheKey)) { + $value = $folder->getFile($this->cacheKey)->getContent(); + try { + $cache->set($this->cacheKey, $value, 3600); + } catch (\Throwable $e) { + $this->logger->warning('Could not write remote data to cache for ' . $this->cacheKey, ['exception' => $e]); + } + return $value; + } + } catch (\Throwable $e) { + $this->logger->warning('Could not read cached remote data for ' . $this->cacheKey, ['exception' => $e]); } return null; @@ -83,9 +91,14 @@ public function fetch(): string { public function resetCache(): void { $cache = $this->cacheFactory->createDistributed('richdocuments'); $cache->remove($this->cacheKey); - $folder = $this->getAppDataFolder(); - if ($folder->fileExists($this->cacheKey)) { - $folder->getFile($this->cacheKey)->delete(); + + try { + $folder = $this->getAppDataFolder(); + if ($folder->fileExists($this->cacheKey)) { + $folder->getFile($this->cacheKey)->delete(); + } + } catch (\Throwable $e) { + $this->logger->warning('Could not remove cached remote data for ' . $this->cacheKey, ['exception' => $e]); } } diff --git a/lib/Service/DiscoveryService.php b/lib/Service/DiscoveryService.php index 9a001b39a8..7f52dd95ec 100644 --- a/lib/Service/DiscoveryService.php +++ b/lib/Service/DiscoveryService.php @@ -120,8 +120,13 @@ public function getProofKeyOld(): ?ProofKey { } private function getParsed(): SimpleXMLElement { + $raw = $this->get(); + if ($raw === null) { + throw new \Exception('No discovery data available'); + } + try { - return new SimpleXMLElement($this->get()); + return new SimpleXMLElement($raw); } catch (\Exception $e) { $this->logger->error($e->getMessage()); throw new \Exception('Could not parse discovery XML'); diff --git a/lib/WOPI/Parser.php b/lib/WOPI/Parser.php index 632a2ca7e1..4852f9d951 100644 --- a/lib/WOPI/Parser.php +++ b/lib/WOPI/Parser.php @@ -36,8 +36,15 @@ public function getUrlSrcValue(string $appName): string { */ private function getUrlSrc(string $mimetype): array { $discovery = $this->discoveryService->get(); + if ($discovery === null) { + throw new Exception('No discovery data available'); + } + $this->logger->debug('WOPI::getUrlSrc discovery: {discovery}', ['discovery' => $discovery]); $discoveryParsed = simplexml_load_string($discovery); + if ($discoveryParsed === false) { + throw new Exception('Could not parse WOPI discovery response'); + } $result = $discoveryParsed->xpath(sprintf('/wopi-discovery/net-zone/app[@name=\'%s\']/action', $mimetype)); if ($result && count($result) > 0) { diff --git a/tests/lib/Service/CachedRequestServiceTest.php b/tests/lib/Service/CachedRequestServiceTest.php index 970be415dd..8ba49088c2 100644 --- a/tests/lib/Service/CachedRequestServiceTest.php +++ b/tests/lib/Service/CachedRequestServiceTest.php @@ -11,7 +11,10 @@ use OCA\Richdocuments\Service\CachedRequestService; use OCP\Files\AppData\IAppDataFactory; +use OCP\Files\GenericFileException; use OCP\Files\IAppData; +use OCP\Files\NotFoundException; +use OCP\Files\NotPermittedException; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\Http\Client\IClient; @@ -118,6 +121,80 @@ public function testGetReturnsFileValueIfCacheMiss(): void { $this->assertEquals('from-file', $result); } + public function testGetReturnsFileValueIfCachingTheValueFails(): void { + $this->cache->expects($this->once()) + ->method('get') + ->with('unittest-cachekey') + ->willReturn(null); + $this->folder->expects($this->once()) + ->method('fileExists') + ->with('unittest-cachekey') + ->willReturn(true); + $this->folder->expects($this->once()) + ->method('getFile') + ->with('unittest-cachekey') + ->willReturn($this->file); + $this->file->expects($this->once()) + ->method('getContent') + ->willReturn('from-file'); + $this->cache->expects($this->once()) + ->method('set') + ->with('unittest-cachekey', 'from-file', 3600) + ->willThrowException(new \RuntimeException('Distributed cache unavailable')); + $this->logger->expects($this->once())->method('warning'); + + $this->assertEquals('from-file', $this->service->get()); + } + + public function testGetReturnsNullIfCachedFileIsUnreadable(): void { + $this->cache->expects($this->once()) + ->method('get') + ->with('unittest-cachekey') + ->willReturn(null); + $this->folder->expects($this->once()) + ->method('fileExists') + ->with('unittest-cachekey') + ->willReturn(true); + $this->folder->expects($this->once()) + ->method('getFile') + ->with('unittest-cachekey') + ->willReturn($this->file); + $this->file->expects($this->once()) + ->method('getContent') + ->willThrowException(new GenericFileException()); + $this->cache->expects($this->never())->method('set'); + $this->logger->expects($this->once())->method('warning'); + + $this->assertNull($this->service->get()); + } + + public function testGetReturnsNullIfAppDataFolderIsUnavailable(): void { + $appData = $this->createMock(IAppData::class); + $appData->method('getFolder')->willThrowException(new NotFoundException()); + $appData->method('newFolder')->willThrowException(new \RuntimeException('appdata unavailable')); + + $appDataFactory = $this->createMock(IAppDataFactory::class); + $appDataFactory->method('get')->willReturn($appData); + + $service = new TestCachedRequestService( + $this->clientService, + $this->cacheFactory, + $appDataFactory, + $this->appConfig, + $this->logger, + 'unittest-cachekey' + ); + + $this->cache->expects($this->once()) + ->method('get') + ->with('unittest-cachekey') + ->willReturn(null); + $this->cache->expects($this->never())->method('set'); + $this->logger->expects($this->once())->method('warning'); + + $this->assertNull($service->get()); + } + public function testResetCacheRemovesCacheAndFile(): void { $this->cache->expects($this->once())->method('remove')->with('unittest-cachekey'); $this->folder->method('fileExists')->with('unittest-cachekey')->willReturn(true); @@ -128,6 +205,19 @@ public function testResetCacheRemovesCacheAndFile(): void { $this->assertTrue(true); // No exceptions thrown } + public function testResetCacheWhenCachedFileCannotBeRemoved(): void { + $this->cache->expects($this->once())->method('remove')->with('unittest-cachekey'); + $this->folder->method('fileExists')->with('unittest-cachekey')->willReturn(true); + $this->folder->method('getFile')->with('unittest-cachekey')->willReturn($this->file); + $this->file->expects($this->once()) + ->method('delete') + ->willThrowException(new NotPermittedException()); + $this->logger->expects($this->once())->method('warning'); + + $this->service->resetCache(); + $this->assertTrue(true); // No exceptions thrown + } + public function testGetLastUpdateReturnsMTimeIfFileExists(): void { $this->folder->expects($this->once()) ->method('fileExists') diff --git a/tests/lib/Service/CapabilitiesServiceTest.php b/tests/lib/Service/CapabilitiesServiceTest.php index 1b9b2cb1a3..67a1349f98 100644 --- a/tests/lib/Service/CapabilitiesServiceTest.php +++ b/tests/lib/Service/CapabilitiesServiceTest.php @@ -12,7 +12,9 @@ use OCA\Richdocuments\Service\CapabilitiesService; use OCP\App\IAppManager; use OCP\Files\AppData\IAppDataFactory; +use OCP\Files\GenericFileException; use OCP\Files\IAppData; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; @@ -62,4 +64,40 @@ public function testFetchRefreshesCapabilities(): void { $service->fetch(); $this->assertSame('2.0', $service->getProductVersion()); } + + public function testGetCapabilitiesWithUnreadableCachedFile(): void { + $cache = $this->createStub(ICache::class); + $cache->method('get')->willReturn(null); + $cacheFactory = $this->createStub(ICacheFactory::class); + $cacheFactory->method('createDistributed')->willReturn($cache); + + $file = $this->createStub(ISimpleFile::class); + $file->method('getContent')->willThrowException(new GenericFileException()); + $folder = $this->createStub(ISimpleFolder::class); + $folder->method('fileExists')->willReturn(true); + $folder->method('getFile')->willReturn($file); + + $appData = $this->createStub(IAppData::class); + $appData->method('getFolder')->willReturn($folder); + $appDataFactory = $this->createStub(IAppDataFactory::class); + $appDataFactory->method('get')->willReturn($appData); + + $config = $this->createStub(IConfig::class); + $config->method('getAppValue')->willReturn('https://collabora.example'); + + $service = new CapabilitiesService( + $this->createStub(IClientService::class), + $cacheFactory, + $appDataFactory, + $this->createStub(IAppConfig::class), + $this->createStub(LoggerInterface::class), + $config, + $this->createStub(IAppManager::class), + $this->createStub(IL10N::class), + ); + + $this->assertSame([], $service->getCapabilities()); + $this->assertNull($service->getProductVersion()); + $this->assertFalse($service->hasTemplateSource()); + } } diff --git a/tests/lib/Service/DiscoveryServiceTest.php b/tests/lib/Service/DiscoveryServiceTest.php index 1de4bd90b6..6df13095e1 100644 --- a/tests/lib/Service/DiscoveryServiceTest.php +++ b/tests/lib/Service/DiscoveryServiceTest.php @@ -93,6 +93,27 @@ public function testGetProofKey(): void { $this->assertEquals('world', $proofKey->getExponent()); } + public function testHasProofKeyWithoutDiscoveryData(): void { + $this->discoveryService->method('get') + ->willReturn(null); + + $this->assertFalse($this->discoveryService->hasProofKey()); + } + + public function testGetProofKeyWithoutDiscoveryData(): void { + $this->discoveryService->method('get') + ->willReturn(null); + + $this->assertNull($this->discoveryService->getProofKey()); + } + + public function testGetProofKeyOldWithoutDiscoveryData(): void { + $this->discoveryService->method('get') + ->willReturn(null); + + $this->assertNull($this->discoveryService->getProofKeyOld()); + } + public function testGetProofKeyOld(): void { $discoveryXml = << diff --git a/tests/lib/WOPI/ParserTest.php b/tests/lib/WOPI/ParserTest.php new file mode 100644 index 0000000000..f0c5a38cfb --- /dev/null +++ b/tests/lib/WOPI/ParserTest.php @@ -0,0 +1,100 @@ +discoveryService = $this->createMock(DiscoveryService::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->parser = new Parser($this->discoveryService, $this->logger); + } + + public function testGetUrlSrcValue(): void { + $discoveryXml = << + + + + + + + END; + + $this->discoveryService->method('get') + ->willReturn($discoveryXml); + + $this->assertEquals( + 'https://collabora.example.com/browser/abc/cool.html?', + $this->parser->getUrlSrcValue('application/vnd.oasis.opendocument.text') + ); + } + + public function testGetUrlSrcValueWithoutDiscoveryData(): void { + $this->discoveryService->method('get') + ->willReturn(null); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('No discovery data available'); + + $this->parser->getUrlSrcValue('application/vnd.oasis.opendocument.text'); + } + + public function testGetUrlSrcValueWithUnparseableDiscoveryData(): void { + $this->discoveryService->method('get') + ->willReturn('this is not xml'); + + $useInternalErrors = libxml_use_internal_errors(true); + + try { + $this->expectException(Exception::class); + $this->expectExceptionMessage('Could not parse WOPI discovery response'); + + $this->parser->getUrlSrcValue('application/vnd.oasis.opendocument.text'); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($useInternalErrors); + } + } + + public function testGetUrlSrcValueWithUnknownMimetype(): void { + $discoveryXml = << + + + + + + + END; + + $this->discoveryService->method('get') + ->willReturn($discoveryXml); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Could not find urlsrc for application/vnd.ms-excel'); + + $this->parser->getUrlSrcValue('application/vnd.ms-excel'); + } +}