From 367a7bd87e6ca053cc55cdcc040eff82a53f2ea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 14 Oct 2020 16:58:52 +0200 Subject: [PATCH] Only retry fetching app store data once every 5 minutes in case it fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/App/AppStore/Fetcher/Fetcher.php | 17 ++- .../lib/App/AppStore/Fetcher/FetcherBase.php | 111 ++++++------------ 2 files changed, 52 insertions(+), 76 deletions(-) diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php index a844d9d791..305e3172c1 100644 --- a/lib/private/App/AppStore/Fetcher/Fetcher.php +++ b/lib/private/App/AppStore/Fetcher/Fetcher.php @@ -42,6 +42,7 @@ use OCP\ILogger; abstract class Fetcher { public const INVALIDATE_AFTER_SECONDS = 3600; + public const RETRY_AFTER_FAILURE_SECONDS = 300; /** @var IAppData */ protected $appData; @@ -91,6 +92,9 @@ abstract class Fetcher { */ protected function fetch($ETag, $content) { $appstoreenabled = $this->config->getSystemValue('appstoreenabled', true); + if ((int)$this->config->getAppValue('settings', 'appstore-fetcher-lastFailure', '0') > time() - self::RETRY_AFTER_FAILURE_SECONDS) { + return []; + } if (!$appstoreenabled) { return []; @@ -107,7 +111,12 @@ abstract class Fetcher { } $client = $this->clientService->newClient(); - $response = $client->get($this->getEndpoint(), $options); + try { + $response = $client->get($this->getEndpoint(), $options); + } catch (ConnectException $e) { + $this->config->setAppValue('settings', 'appstore-fetcher-lastFailure', (string)time()); + throw $e; + } $responseJson = []; if ($response->getStatusCode() === Http::STATUS_NOT_MODIFIED) { @@ -116,6 +125,7 @@ abstract class Fetcher { $responseJson['data'] = json_decode($response->getBody(), true); $ETag = $response->getHeader('ETag'); } + $this->config->deleteAppValue('settings', 'appstore-fetcher-lastFailure'); $responseJson['timestamp'] = $this->timeFactory->getTime(); $responseJson['ncversion'] = $this->getVersion(); @@ -175,6 +185,11 @@ abstract class Fetcher { // Refresh the file content try { $responseJson = $this->fetch($ETag, $content, $allowUnstable); + + if (empty($responseJson)) { + return []; + } + // Don't store the apps request file if ($allowUnstable) { return $responseJson['data']; diff --git a/tests/lib/App/AppStore/Fetcher/FetcherBase.php b/tests/lib/App/AppStore/Fetcher/FetcherBase.php index c3d9ec43cc..531fdf41e7 100644 --- a/tests/lib/App/AppStore/Fetcher/FetcherBase.php +++ b/tests/lib/App/AppStore/Fetcher/FetcherBase.php @@ -120,32 +120,19 @@ abstract class FetcherBase extends TestCase { public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() { $this->config - ->expects($this->at(0)) ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(1)) - ->method('getSystemValue') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config - ->expects($this->at(2)) - ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(3)) - ->method('getSystemValue') - ->with('appstoreurl', 'https://apps.nextcloud.com/api/v1') - ->willReturn('https://apps.nextcloud.com/api/v1'); - $this->config - ->expects($this->at(4)) - ->method('getSystemValue') - ->with( - $this->equalTo('version'), - $this->anything() - )->willReturn('11.0.0.2'); + ->willReturnCallback(function ($var, $default) { + if ($var === 'appstoreenabled') { + return true; + } elseif ($var === 'has_internet_connection') { + return true; + } elseif ($var === 'appstoreurl') { + return 'https://apps.nextcloud.com/api/v1'; + } elseif ($var === 'version') { + return '11.0.0.2'; + } + return $default; + }); $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class); @@ -286,32 +273,19 @@ abstract class FetcherBase extends TestCase { public function testGetWithAlreadyExistingFileAndNoVersion() { $this->config - ->expects($this->at(0)) ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(1)) - ->method('getSystemValue') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config - ->expects($this->at(2)) - ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(3)) - ->method('getSystemValue') - ->with('appstoreurl', 'https://apps.nextcloud.com/api/v1') - ->willReturn('https://apps.nextcloud.com/api/v1'); - $this->config - ->expects($this->at(4)) - ->method('getSystemValue') - ->with( - $this->equalTo('version'), - $this->anything() - )->willReturn('11.0.0.2'); + ->willReturnCallback(function ($var, $default) { + if ($var === 'appstoreenabled') { + return true; + } elseif ($var === 'has_internet_connection') { + return true; + } elseif ($var === 'appstoreurl') { + return 'https://apps.nextcloud.com/api/v1'; + } elseif ($var === 'version') { + return '11.0.0.2'; + } + return $default; + }); $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class); @@ -375,32 +349,19 @@ abstract class FetcherBase extends TestCase { public function testGetWithAlreadyExistingFileAndOutdatedVersion() { $this->config - ->expects($this->at(0)) ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(1)) - ->method('getSystemValue') - ->with('has_internet_connection', true) - ->willReturn(true); - $this->config - ->expects($this->at(2)) - ->method('getSystemValue') - ->with('appstoreenabled', true) - ->willReturn(true); - $this->config - ->expects($this->at(3)) - ->method('getSystemValue') - ->with('appstoreurl', 'https://apps.nextcloud.com/api/v1') - ->willReturn('https://apps.nextcloud.com/api/v1'); - $this->config - ->expects($this->at(4)) - ->method('getSystemValue') - ->with( - $this->equalTo('version'), - $this->anything() - )->willReturn('11.0.0.2'); + ->willReturnCallback(function ($var, $default) { + if ($var === 'appstoreenabled') { + return true; + } elseif ($var === 'has_internet_connection') { + return true; + } elseif ($var === 'appstoreurl') { + return 'https://apps.nextcloud.com/api/v1'; + } elseif ($var === 'version') { + return '11.0.0.2'; + } + return $default; + }); $folder = $this->createMock(ISimpleFolder::class); $file = $this->createMock(ISimpleFile::class);