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

Signed-off-by: Julius Härtl <jus@bitgrid.net>

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();
+		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'];

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)
+			->willReturnCallback(function ($var, $default) {
-			->willReturn(true);
+				if ($var === 'appstoreenabled') {
-		$this->config
+					return true;
-			->expects($this->at(1))
+				} elseif ($var === 'has_internet_connection') {
-			->method('getSystemValue')
+					return true;
-			->with('has_internet_connection', true)
+				} elseif ($var === 'appstoreurl') {
-			->willReturn(true);
+					return 'https://apps.nextcloud.com/api/v1';
-		$this->config
+				} elseif ($var === 'version') {
-			->expects($this->at(2))
+					return '11.0.0.2';
-			->method('getSystemValue')
+				}
-			->with('appstoreenabled', true)
+				return $default;
-			->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');
 
 		$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)
+			->willReturnCallback(function ($var, $default) {
-			->willReturn(true);
+				if ($var === 'appstoreenabled') {
-		$this->config
+					return true;
-			->expects($this->at(1))
+				} elseif ($var === 'has_internet_connection') {
-			->method('getSystemValue')
+					return true;
-			->with('has_internet_connection', true)
+				} elseif ($var === 'appstoreurl') {
-			->willReturn(true);
+					return 'https://apps.nextcloud.com/api/v1';
-		$this->config
+				} elseif ($var === 'version') {
-			->expects($this->at(2))
+					return '11.0.0.2';
-			->method('getSystemValue')
+				}
-			->with('appstoreenabled', true)
+				return $default;
-			->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');
 
 		$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)
+			->willReturnCallback(function ($var, $default) {
-			->willReturn(true);
+				if ($var === 'appstoreenabled') {
-		$this->config
+					return true;
-			->expects($this->at(1))
+				} elseif ($var === 'has_internet_connection') {
-			->method('getSystemValue')
+					return true;
-			->with('has_internet_connection', true)
+				} elseif ($var === 'appstoreurl') {
-			->willReturn(true);
+					return 'https://apps.nextcloud.com/api/v1';
-		$this->config
+				} elseif ($var === 'version') {
-			->expects($this->at(2))
+					return '11.0.0.2';
-			->method('getSystemValue')
+				}
-			->with('appstoreenabled', true)
+				return $default;
-			->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');
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);