diff --git a/lib/private/App/AppStore/Fetcher/AppFetcher.php b/lib/private/App/AppStore/Fetcher/AppFetcher.php index bbf4b00245b33..c14a6228ca38e 100644 --- a/lib/private/App/AppStore/Fetcher/AppFetcher.php +++ b/lib/private/App/AppStore/Fetcher/AppFetcher.php @@ -60,80 +60,95 @@ protected function fetch($ETag, $content, $allowUnstable = false) { return []; } - $allowPreReleases = $allowUnstable || $this->getChannel() === 'beta' || $this->getChannel() === 'daily' || $this->getChannel() === 'git'; - $allowNightly = $allowUnstable || $this->getChannel() === 'daily' || $this->getChannel() === 'git'; + $channel = $this->getChannel(); + $allowPreReleases = $allowUnstable || $channel === 'beta' || $channel === 'daily' || $channel === 'git'; + $allowNightly = $allowUnstable || $channel === 'daily' || $channel === 'git'; + + $versionParser = new VersionParser(); + $ncVersion = $this->getVersion(); + $currentPhpVersion = PHP_VERSION; + $ignoreMaxVersion = $this->ignoreMaxVersion; + + /** @var array $platformSpecCache */ + $platformSpecCache = []; + /** @var array $phpSpecCache */ + $phpSpecCache = []; foreach ($response['data'] as $dataKey => $app) { - $releases = []; + $bestRelease = null; - // Filter all compatible releases + // Filter compatible releases foreach ($app['releases'] as $release) { - // Exclude all nightly and pre-releases if required - if (($allowNightly || $release['isNightly'] === false) - && ($allowPreReleases || !str_contains($release['version'], '-'))) { - // Exclude all versions not compatible with the current version - try { - $versionParser = new VersionParser(); - $serverVersion = $versionParser->getVersion($release['rawPlatformVersionSpec']); - $ncVersion = $this->getVersion(); - $minServerVersion = $serverVersion->getMinimumVersion(); - $maxServerVersion = $serverVersion->getMaximumVersion(); - $minFulfilled = $this->compareVersion->isCompatible($ncVersion, $minServerVersion, '>='); - $maxFulfilled = $maxServerVersion !== '' - && $this->compareVersion->isCompatible($ncVersion, $maxServerVersion, '<='); - $isPhpCompatible = true; - if (($release['rawPhpVersionSpec'] ?? '*') !== '*') { - $phpVersion = $versionParser->getVersion($release['rawPhpVersionSpec']); - $minPhpVersion = $phpVersion->getMinimumVersion(); - $maxPhpVersion = $phpVersion->getMaximumVersion(); - $minPhpFulfilled = $minPhpVersion === '' || $this->compareVersion->isCompatible( - PHP_VERSION, - $minPhpVersion, - '>=' - ); - $maxPhpFulfilled = $maxPhpVersion === '' || $this->compareVersion->isCompatible( - PHP_VERSION, - $maxPhpVersion, - '<=' - ); - - $isPhpCompatible = $minPhpFulfilled && $maxPhpFulfilled; - } - if ($minFulfilled && ($this->ignoreMaxVersion || $maxFulfilled) && $isPhpCompatible) { - $releases[] = $release; + // Exclude nightly builds + if ($release['isNightly'] !== false && !$allowNightly) { + continue; + } + + // Exclude pre-releases + if (str_contains($release['version'], '-') && !$allowPreReleases) { + continue; + } + + try { + $rawPlatformVersionSpec = $release['rawPlatformVersionSpec'] ?? null; + if ($rawPlatformVersionSpec === null) { + continue; // no spec; treat as incompatible, skip + } + if (!isset($platformSpecCache[$rawPlatformVersionSpec])) { + $serverVersion = $versionParser->getVersion($rawPlatformVersionSpec); + $platformSpecCache[$rawPlatformVersionSpec] = [ + $serverVersion->getMinimumVersion(), + $serverVersion->getMaximumVersion(), + ]; + } + + [$minServerVersion, $maxServerVersion] = $platformSpecCache[$rawPlatformVersionSpec]; + + $minFulfilled = $this->compareVersion->isCompatible($ncVersion, $minServerVersion, '>='); + $maxFulfilled = $maxServerVersion !== '' && $this->compareVersion->isCompatible($ncVersion, $maxServerVersion, '<='); + + $isPhpCompatible = true; + + $rawPhpVersionSpec = $release['rawPhpVersionSpec'] ?? '*'; + if ($rawPhpVersionSpec !== '*') { + if (!isset($phpSpecCache[$rawPhpVersionSpec])) { + $phpVersion = $versionParser->getVersion($rawPhpVersionSpec); + $phpSpecCache[$rawPhpVersionSpec] = [ + $phpVersion->getMinimumVersion(), + $phpVersion->getMaximumVersion(), + ]; } - } catch (\InvalidArgumentException $e) { - $this->logger->warning($e->getMessage(), [ - 'exception' => $e, - ]); + + [$minPhpVersion, $maxPhpVersion] = $phpSpecCache[$rawPhpVersionSpec]; + + $minPhpFulfilled = $minPhpVersion === '' || $this->compareVersion->isCompatible($currentPhpVersion, $minPhpVersion, '>='); + $maxPhpFulfilled = $maxPhpVersion === '' || $this->compareVersion->isCompatible($currentPhpVersion, $maxPhpVersion, '<='); + + $isPhpCompatible = $minPhpFulfilled && $maxPhpFulfilled; + } + + $isCompatible = $minFulfilled && ($ignoreMaxVersion || $maxFulfilled) && $isPhpCompatible; + + if (!$isCompatible) { + continue; } + + $betterRelease = $bestRelease === null || version_compare((string)$release['version'], (string)$bestRelease['version'], '>'); + if ($betterRelease) { + $bestRelease = $release; + } + } catch (\InvalidArgumentException $e) { + $this->logger->warning($e->getMessage(), [ 'exception' => $e, ]); } } - if (empty($releases)) { + if ($bestRelease === null) { // Remove apps that don't have a matching release $response['data'][$dataKey] = []; continue; } - // Get the highest version - $versions = []; - foreach ($releases as $release) { - $versions[] = $release['version']; - } - usort($versions, function ($version1, $version2) { - return version_compare($version1, $version2); - }); - $versions = array_reverse($versions); - if (isset($versions[0])) { - $highestVersion = $versions[0]; - foreach ($releases as $release) { - if ((string)$release['version'] === (string)$highestVersion) { - $response['data'][$dataKey]['releases'] = [$release]; - break; - } - } - } + $response['data'][$dataKey]['releases'] = [$bestRelease]; } $response['data'] = array_values(array_filter($response['data'])); @@ -158,12 +173,13 @@ public function get($allowUnstable = false): array { if (empty($apps)) { return []; } - $allowList = $this->config->getSystemValue('appsallowlist'); // If the admin specified a allow list, filter apps from the appstore + $allowList = $this->config->getSystemValue('appsallowlist'); if (is_array($allowList) && $this->registry->delegateHasValidSubscription()) { - return array_filter($apps, function ($app) use ($allowList) { - return in_array($app['id'], $allowList); + $allowSet = array_flip($allowList); + return array_filter($apps, static function ($app) use ($allowSet) { + return isset($allowSet[$app['id']]); }); } diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index 3304b414ad747..d1f61748f70fe 100644 --- a/lib/private/App/AppStore/Fetcher/Fetcher.php +++ b/lib/private/App/AppStore/Fetcher/Fetcher.php @@ -134,6 +134,7 @@ public function get($allowUnstable = false) { $ETag = ''; $content = ''; + $cachedData = null; try { // File does already exists @@ -141,6 +142,10 @@ public function get($allowUnstable = false) { $jsonBlob = json_decode($file->getContent(), true); if (is_array($jsonBlob)) { + if (isset($jsonBlob['data']) && is_array($jsonBlob['data'])) { + $cachedData = $jsonBlob['data']; + } + // No caching when the version has been updated if (isset($jsonBlob['ncversion']) && $jsonBlob['ncversion'] === $this->getVersion()) { // If the timestamp is older than 3600 seconds request the files new @@ -170,20 +175,17 @@ public function get($allowUnstable = false) { $responseJson = $this->fetch($ETag, $content, $allowUnstable); if (empty($responseJson) || empty($responseJson['data'])) { - return []; + return is_array($cachedData) ? $cachedData : []; } $file->putContent(json_encode($responseJson)); - return json_decode($file->getContent(), true)['data']; + return $responseJson['data']; } catch (ConnectException $e) { $this->logger->warning('Could not connect to appstore: ' . $e->getMessage(), ['app' => 'appstoreFetcher']); - return []; + return is_array($cachedData) ? $cachedData : []; } catch (\Exception $e) { - $this->logger->warning($e->getMessage(), [ - 'exception' => $e, - 'app' => 'appstoreFetcher', - ]); - return []; + $this->logger->warning($e->getMessage(), ['exception' => $e, 'app' => 'appstoreFetcher',]); + return is_array($cachedData) ? $cachedData : []; } } diff --git a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php index c6f0df7d2fb6f..34f58621f2661 100644 --- a/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php +++ b/tests/lib/App/AppStore/Fetcher/AppFetcherTest.php @@ -2250,4 +2250,97 @@ public function testGetAppsAllowlistCustomAppstore(): void { $this->assertEquals(count($apps), 1); $this->assertEquals($apps[0]['id'], 'contacts'); } + + public function testGetKeepsHighestCompatibleReleaseOnly(): void { + $this->config->method('getSystemValueString') + ->willReturnCallback(function ($key, $default) { + if ($key === 'version') { + return '30.0.0'; + } elseif ($key === 'appstoreurl' && $default === 'https://apps.nextcloud.com/api/v1') { + return 'https://custom.appsstore.endpoint/api/v1'; + } + return $default; + }); + $this->config->method('getSystemValueBool') + ->willReturnArgument(1); + + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); + $folder + ->expects($this->once()) + ->method('getFile') + ->with('apps.json') + ->willThrowException(new NotFoundException()); + $folder + ->expects($this->once()) + ->method('newFile') + ->with('apps.json') + ->willReturn($file); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('/') + ->willReturn($folder); + + $client = $this->createMock(IClient::class); + $this->clientService + ->expects($this->once()) + ->method('newClient') + ->willReturn($client); + + $response = $this->createMock(IResponse::class); + $client + ->expects($this->once()) + ->method('get') + ->with('https://custom.appsstore.endpoint/api/v1/apps.json') + ->willReturn($response); + + $response + ->expects($this->once()) + ->method('getBody') + ->willReturn(json_encode([ + [ + 'id' => 'testapp', + 'releases' => [ + [ + 'version' => '1.0.0', + 'isNightly' => false, + 'rawPhpVersionSpec' => '*', + 'rawPlatformVersionSpec' => '>=30 <=30', + ], + [ + 'version' => '1.5.0', + 'isNightly' => false, + 'rawPhpVersionSpec' => '*', + 'rawPlatformVersionSpec' => '>=30 <=30', + ], + [ + 'version' => '2.0.0', + 'isNightly' => false, + 'rawPhpVersionSpec' => '*', + 'rawPlatformVersionSpec' => '>=31 <=31', + ], + ], + ], + ], JSON_THROW_ON_ERROR)); + $response->method('getHeader') + ->with($this->equalTo('ETag')) + ->willReturn('"myETag"'); + + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(1234); + + $file + ->expects($this->once()) + ->method('putContent'); + + $result = $this->fetcher->get(); + + $this->assertCount(1, $result); + $this->assertSame('testapp', $result[0]['id']); + $this->assertCount(1, $result[0]['releases']); + $this->assertSame('1.5.0', $result[0]['releases'][0]['version']); + } } diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index 4784e76d574f6..66f972198538f 100644 --- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php +++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php @@ -149,9 +149,8 @@ public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion(): void ->method('putContent') ->with($fileData); $file - ->expects($this->once()) - ->method('getContent') - ->willReturn($fileData); + ->expects($this->never()) + ->method('getContent'); $this->timeFactory ->expects($this->once()) ->method('getTime') @@ -199,12 +198,9 @@ public function testGetWithAlreadyExistingFileAndOutdatedTimestamp(): void { ->method('putContent') ->with($fileData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - '{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.2"}', - $fileData - ); + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.2"}'); $this->timeFactory ->expects($this->exactly(2)) ->method('getTime') @@ -275,12 +271,9 @@ public function testGetWithAlreadyExistingFileAndNoVersion(): void { ->method('putContent') ->with($fileData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - '{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}', - $fileData - ); + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}'); $this->timeFactory ->expects($this->once()) ->method('getTime') @@ -348,12 +341,9 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion(): void { ->method('putContent') ->with($fileData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - '{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.1"', - $fileData - ); + ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}},"ncversion":"11.0.0.1"'); $this->timeFactory ->method('getTime') ->willReturn(1201); @@ -388,12 +378,75 @@ public function testGetWithAlreadyExistingFileAndOutdatedVersion(): void { $this->assertSame($expected, $this->fetcher->get()); } - public function testGetWithExceptionInClient(): void { + public function testGetWithExceptionInClientReturnsStaleCachedData(): void { $this->config->method('getSystemValueString') + ->willReturnCallback(function ($key, $default) { + if ($key === 'version') { + return '11.0.0.2'; + } + return $default; + }); + $this->config->method('getSystemValueBool') ->willReturnArgument(1); + + $folder = $this->createMock(ISimpleFolder::class); + $file = $this->createMock(ISimpleFile::class); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('/') + ->willReturn($folder); + $folder + ->expects($this->once()) + ->method('getFile') + ->with($this->fileName) + ->willReturn($file); + $file + ->expects($this->once()) + ->method('getContent') + ->willReturn('{"timestamp":1200,"data":[{"id":"MyApp"}],"ncversion":"11.0.0.2","ETag":"\"myETag\""}'); + + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(4801); + + $client = $this->createMock(IClient::class); + $this->clientService + ->expects($this->once()) + ->method('newClient') + ->willReturn($client); + $client + ->expects($this->once()) + ->method('get') + ->with($this->endpoint) + ->willThrowException(new \Exception('refresh failed')); + + $expected = [ + [ + 'id' => 'MyApp', + ], + ]; + + $this->assertSame($expected, $this->fetcher->get()); + } + + public function testGetReturnsStaleCachedDataWhenRefreshReturnsEmptyResult(): void { + $this->config->method('getSystemValueString') + ->willReturnCallback(function ($key, $default) { + if ($key === 'version') { + return '11.0.0.2'; + } + return $default; + }); $this->config->method('getSystemValueBool') ->willReturnArgument(1); + $this->config->method('getAppValue') + ->willReturnMap([ + ['settings', 'appstore-fetcher-lastFailure', '0', '4800'], + ]); + $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class); $this->appData @@ -409,19 +462,102 @@ public function testGetWithExceptionInClient(): void { $file ->expects($this->once()) ->method('getContent') - ->willReturn('{"timestamp":1200,"data":{"MyApp":{"id":"MyApp"}}}'); + ->willReturn('{"timestamp":1200,"data":[{"id":"MyApp"}],"ncversion":"11.0.0.2","ETag":"\"myETag\""}'); + + $this->timeFactory + ->expects($this->exactly(2)) + ->method('getTime') + ->willReturnOnConsecutiveCalls(4801, 4801); + + $expected = [ + [ + 'id' => 'MyApp', + ], + ]; + + $this->assertSame($expected, $this->fetcher->get()); + } + + public function testGetReturnsFreshDataWithoutReadingBackWrittenCache(): void { + $this->config + ->method('getSystemValueString') + ->willReturnCallback(function ($var, $default) { + if ($var === 'appstoreurl') { + return 'https://apps.nextcloud.com/api/v1'; + } elseif ($var === 'version') { + return '11.0.0.2'; + } + return $default; + }); + $this->config->method('getSystemValueBool') + ->willReturnArgument(1); + + $folder = $this->createMock(ISimpleFolder::class); + $file = $this->createMock(ISimpleFile::class); + $this->appData + ->expects($this->once()) + ->method('getFolder') + ->with('/') + ->willReturn($folder); + $folder + ->expects($this->once()) + ->method('getFile') + ->with($this->fileName) + ->willThrowException(new NotFoundException()); + $folder + ->expects($this->once()) + ->method('newFile') + ->with($this->fileName) + ->willReturn($file); + $client = $this->createMock(IClient::class); $this->clientService ->expects($this->once()) ->method('newClient') ->willReturn($client); + + $response = $this->createMock(IResponse::class); $client ->expects($this->once()) ->method('get') ->with($this->endpoint) - ->willThrowException(new \Exception()); + ->willReturn($response); + + $response + ->expects($this->once()) + ->method('getBody') + ->willReturn('[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}]'); + $response + ->method('getHeader') + ->with($this->equalTo('ETag')) + ->willReturn('"myETag"'); + + $fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}'; + $file + ->expects($this->once()) + ->method('putContent') + ->with($fileData); - $this->assertSame([], $this->fetcher->get()); + $file + ->expects($this->never()) + ->method('getContent'); + + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(1502); + + $expected = [ + [ + 'id' => 'MyNewApp', + 'foo' => 'foo', + ], + [ + 'id' => 'bar', + ], + ]; + + $this->assertSame($expected, $this->fetcher->get()); } public function testGetMatchingETag(): void { @@ -462,12 +598,9 @@ public function testGetMatchingETag(): void { ->method('putContent') ->with($newData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - $origData, - $newData, - ); + ->willReturn($origData); $this->timeFactory ->expects($this->exactly(2)) ->method('getTime') @@ -545,12 +678,9 @@ public function testGetNoMatchingETag(): void { ->method('putContent') ->with($fileData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - '{"data":[{"id":"MyOldApp","abc":"def"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}', - $fileData, - ); + ->willReturn('{"data":[{"id":"MyOldApp","abc":"def"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}'); $this->timeFactory ->expects($this->exactly(2)) ->method('getTime') @@ -636,12 +766,9 @@ public function testFetchAfterUpgradeNoETag(): void { ->method('putContent') ->with($fileData); $file - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('getContent') - ->willReturnOnConsecutiveCalls( - '{"data":[{"id":"MyOldApp","abc":"def"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}', - $fileData - ); + ->willReturn('{"data":[{"id":"MyOldApp","abc":"def"}],"timestamp":1200,"ncversion":"11.0.0.2","ETag":"\"myETag\""}'); $client = $this->createMock(IClient::class); $this->clientService ->expects($this->once())