Skip to content

Commit c6abcfb

Browse files
MorrisJobkeMorrisJobke
authored
Merge pull request #23635 from nextcloud/backport/23374/stable18
[stable18] Only retry fetching app store data once every 5 minutes in case it fails
2 parents a4cfac8 + 338498f commit c6abcfb

File tree

2 files changed

+52
-76
lines changed

2 files changed

+52
-76
lines changed

‎lib/private/App/AppStore/Fetcher/Fetcher.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
abstract class Fetcher {
4343
const INVALIDATE_AFTER_SECONDS = 3600;
44+
const RETRY_AFTER_FAILURE_SECONDS = 300;
4445

4546
/** @var IAppData */
4647
protected $appData;
@@ -90,6 +91,9 @@ public function __construct(Factory $appDataFactory,
9091
*/
9192
protected function fetch($ETag, $content) {
9293
$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
94+
if ((int)$this->config->getAppValue('settings', 'appstore-fetcher-lastFailure', '0') > time() - self::RETRY_AFTER_FAILURE_SECONDS) {
95+
return [];
96+
}
9397

9498
if (!$appstoreenabled) {
9599
return [];
@@ -105,7 +109,12 @@ protected function fetch($ETag, $content) {
105109
}
106110

107111
$client = $this->clientService->newClient();
108-
$response = $client->get($this->getEndpoint(), $options);
112+
try {
113+
$response = $client->get($this->getEndpoint(), $options);
114+
} catch (ConnectException $e) {
115+
$this->config->setAppValue('settings', 'appstore-fetcher-lastFailure', (string)time());
116+
throw $e;
117+
}
109118

110119
$responseJson = [];
111120
if ($response->getStatusCode() === Http::STATUS_NOT_MODIFIED) {
@@ -114,6 +123,7 @@ protected function fetch($ETag, $content) {
114123
$responseJson['data'] = json_decode($response->getBody(), true);
115124
$ETag = $response->getHeader('ETag');
116125
}
126+
$this->config->deleteAppValue('settings', 'appstore-fetcher-lastFailure');
117127

118128
$responseJson['timestamp'] = $this->timeFactory->getTime();
119129
$responseJson['ncversion'] = $this->getVersion();
@@ -170,6 +180,11 @@ public function get() {
170180
// Refresh the file content
171181
try {
172182
$responseJson = $this->fetch($ETag, $content);
183+
184+
if (empty($responseJson)) {
185+
return [];
186+
}
187+
173188
$file->putContent(json_encode($responseJson));
174189
return json_decode($file->getContent(), true)['data'];
175190
} catch (ConnectException $e) {

‎tests/lib/App/AppStore/Fetcher/FetcherBase.php‎

Lines changed: 36 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -120,32 +120,19 @@ public function testGetWithAlreadyExistingFileAndUpToDateTimestampAndVersion() {
120120

121121
public function testGetWithNotExistingFileAndUpToDateTimestampAndVersion() {
122122
$this->config
123-
->expects($this->at(0))
124-
->method('getSystemValue')
125-
->with('appstoreenabled', true)
126-
->willReturn(true);
127-
$this->config
128-
->expects($this->at(1))
129-
->method('getSystemValue')
130-
->with('has_internet_connection', true)
131-
->willReturn(true);
132-
$this->config
133-
->expects($this->at(2))
134-
->method('getSystemValue')
135-
->with('appstoreenabled', true)
136-
->willReturn(true);
137-
$this->config
138-
->expects($this->at(3))
139123
->method('getSystemValue')
140-
->with('appstoreurl', 'https://apps.nextcloud.com/api/v1')
141-
->willReturn('https://apps.nextcloud.com/api/v1');
142-
$this->config
143-
->expects($this->at(4))
144-
->method('getSystemValue')
145-
->with(
146-
$this->equalTo('version'),
147-
$this->anything()
148-
)->willReturn('11.0.0.2');
124+
->willReturnCallback(function ($var, $default) {
125+
if ($var === 'appstoreenabled') {
126+
return true;
127+
} elseif ($var === 'has_internet_connection') {
128+
return true;
129+
} elseif ($var === 'appstoreurl') {
130+
return 'https://apps.nextcloud.com/api/v1';
131+
} elseif ($var === 'version') {
132+
return '11.0.0.2';
133+
}
134+
return $default;
135+
});
149136

150137
$folder = $this->createMock(ISimpleFolder::class);
151138
$file = $this->createMock(ISimpleFile::class);
@@ -294,32 +281,19 @@ public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() {
294281

295282
public function testGetWithAlreadyExistingFileAndNoVersion() {
296283
$this->config
297-
->expects($this->at(0))
298-
->method('getSystemValue')
299-
->with('appstoreenabled', true)
300-
->willReturn(true);
301-
$this->config
302-
->expects($this->at(1))
303-
->method('getSystemValue')
304-
->with('has_internet_connection', true)
305-
->willReturn(true);
306-
$this->config
307-
->expects($this->at(2))
308-
->method('getSystemValue')
309-
->with('appstoreenabled', true)
310-
->willReturn(true);
311-
$this->config
312-
->expects($this->at(3))
313284
->method('getSystemValue')
314-
->with('appstoreurl', 'https://apps.nextcloud.com/api/v1')
315-
->willReturn('https://apps.nextcloud.com/api/v1');
316-
$this->config
317-
->expects($this->at(4))
318-
->method('getSystemValue')
319-
->with(
320-
$this->equalTo('version'),
321-
$this->anything()
322-
)->willReturn('11.0.0.2');
285+
->willReturnCallback(function ($var, $default) {
286+
if ($var === 'appstoreenabled') {
287+
return true;
288+
} elseif ($var === 'has_internet_connection') {
289+
return true;
290+
} elseif ($var === 'appstoreurl') {
291+
return 'https://apps.nextcloud.com/api/v1';
292+
} elseif ($var === 'version') {
293+
return '11.0.0.2';
294+
}
295+
return $default;
296+
});
323297

324298
$folder = $this->createMock(ISimpleFolder::class);
325299
$file = $this->createMock(ISimpleFile::class);
@@ -391,32 +365,19 @@ public function testGetWithAlreadyExistingFileAndNoVersion() {
391365

392366
public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
393367
$this->config
394-
->expects($this->at(0))
395-
->method('getSystemValue')
396-
->with('appstoreenabled', true)
397-
->willReturn(true);
398-
$this->config
399-
->expects($this->at(1))
400368
->method('getSystemValue')
401-
->with('has_internet_connection', true)
402-
->willReturn(true);
403-
$this->config
404-
->expects($this->at(2))
405-
->method('getSystemValue')
406-
->with('appstoreenabled', true)
407-
->willReturn(true);
408-
$this->config
409-
->expects($this->at(3))
410-
->method('getSystemValue')
411-
->with('appstoreurl', 'https://apps.nextcloud.com/api/v1')
412-
->willReturn('https://apps.nextcloud.com/api/v1');
413-
$this->config
414-
->expects($this->at(4))
415-
->method('getSystemValue')
416-
->with(
417-
$this->equalTo('version'),
418-
$this->anything()
419-
)->willReturn('11.0.0.2');
369+
->willReturnCallback(function ($var, $default) {
370+
if ($var === 'appstoreenabled') {
371+
return true;
372+
} elseif ($var === 'has_internet_connection') {
373+
return true;
374+
} elseif ($var === 'appstoreurl') {
375+
return 'https://apps.nextcloud.com/api/v1';
376+
} elseif ($var === 'version') {
377+
return '11.0.0.2';
378+
}
379+
return $default;
380+
});
420381

421382
$folder = $this->createMock(ISimpleFolder::class);
422383
$file = $this->createMock(ISimpleFile::class);

0 commit comments

Comments
 (0)