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

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

diff --git a/lib/private/App/AppStore/Fetcher/Fetcher.php b/lib/private/App/AppStore/Fetcher/Fetcher.php
index a844d9d79133ec1659b9a48cc8e3130335f2e0c9..305e3172c100851030c0d27dd252dde30f057cd5 100644 (file)
--- 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 c3d9ec43cc4173e534c3584de33dbd61c17a1200..531fdf41e789ef158a30a41fbc627a964fb8d2db 100644 (file)
--- 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);