Merge pull request #23374 from nextcloud/bugfix/noid/app-fetch-retry

Only retry fetching app store data once every 5 minutes in case it fails

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);