From 4512c73293c8843173857c825b5c1c400788f5ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julius=20H=C3=A4rtl?= <jus@bitgrid.net>
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 <jus@bitgrid.net>
---
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 a844d9d7913..305e3172c10 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 c3d9ec43cc4..531fdf41e78 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);
--
GitLab