Skip to content

Commit 4512c73

Browse files
juliusknorrjuliusknorr
committed
Only retry fetching app store data once every 5 minutes in case it fails
Signed-off-by: Julius Härtl <jus@bitgrid.net>
1 parent a41b273 commit 4512c73

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
@@ -42,6 +42,7 @@
4242

4343
abstract class Fetcher {
4444
public const INVALIDATE_AFTER_SECONDS = 3600;
45+
public const RETRY_AFTER_FAILURE_SECONDS = 300;
4546

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

9599
if (!$appstoreenabled) {
96100
return [];
@@ -107,7 +111,12 @@ protected function fetch($ETag, $content) {
107111
}
108112

109113
$client = $this->clientService->newClient();
110-
$response = $client->get($this->getEndpoint(), $options);
114+
try {
115+
$response = $client->get($this->getEndpoint(), $options);
116+
} catch (ConnectException $e) {
117+
$this->config->setAppValue('settings', 'appstore-fetcher-lastFailure', (string)time());
118+
throw $e;
119+
}
111120

112121
$responseJson = [];
113122
if ($response->getStatusCode() === Http::STATUS_NOT_MODIFIED) {
@@ -116,6 +125,7 @@ protected function fetch($ETag, $content) {
116125
$responseJson['data'] = json_decode($response->getBody(), true);
117126
$ETag = $response->getHeader('ETag');
118127
}
128+
$this->config->deleteAppValue('settings', 'appstore-fetcher-lastFailure');
119129

120130
$responseJson['timestamp'] = $this->timeFactory->getTime();
121131
$responseJson['ncversion'] = $this->getVersion();
@@ -175,6 +185,11 @@ public function get($allowUnstable = false) {
175185
// Refresh the file content
176186
try {
177187
$responseJson = $this->fetch($ETag, $content, $allowUnstable);
188+
189+
if (empty($responseJson)) {
190+
return [];
191+
}
192+
178193
// Don't store the apps request file
179194
if ($allowUnstable) {
180195
return $responseJson['data'];

‎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);
@@ -286,32 +273,19 @@ public function testGetWithAlreadyExistingFileAndOutdatedTimestamp() {
286273

287274
public function testGetWithAlreadyExistingFileAndNoVersion() {
288275
$this->config
289-
->expects($this->at(0))
290-
->method('getSystemValue')
291-
->with('appstoreenabled', true)
292-
->willReturn(true);
293-
$this->config
294-
->expects($this->at(1))
295-
->method('getSystemValue')
296-
->with('has_internet_connection', true)
297-
->willReturn(true);
298-
$this->config
299-
->expects($this->at(2))
300-
->method('getSystemValue')
301-
->with('appstoreenabled', true)
302-
->willReturn(true);
303-
$this->config
304-
->expects($this->at(3))
305276
->method('getSystemValue')
306-
->with('appstoreurl', 'https://apps.nextcloud.com/api/v1')
307-
->willReturn('https://apps.nextcloud.com/api/v1');
308-
$this->config
309-
->expects($this->at(4))
310-
->method('getSystemValue')
311-
->with(
312-
$this->equalTo('version'),
313-
$this->anything()
314-
)->willReturn('11.0.0.2');
277+
->willReturnCallback(function ($var, $default) {
278+
if ($var === 'appstoreenabled') {
279+
return true;
280+
} elseif ($var === 'has_internet_connection') {
281+
return true;
282+
} elseif ($var === 'appstoreurl') {
283+
return 'https://apps.nextcloud.com/api/v1';
284+
} elseif ($var === 'version') {
285+
return '11.0.0.2';
286+
}
287+
return $default;
288+
});
315289

316290
$folder = $this->createMock(ISimpleFolder::class);
317291
$file = $this->createMock(ISimpleFile::class);
@@ -375,32 +349,19 @@ public function testGetWithAlreadyExistingFileAndNoVersion() {
375349

376350
public function testGetWithAlreadyExistingFileAndOutdatedVersion() {
377351
$this->config
378-
->expects($this->at(0))
379-
->method('getSystemValue')
380-
->with('appstoreenabled', true)
381-
->willReturn(true);
382-
$this->config
383-
->expects($this->at(1))
384352
->method('getSystemValue')
385-
->with('has_internet_connection', true)
386-
->willReturn(true);
387-
$this->config
388-
->expects($this->at(2))
389-
->method('getSystemValue')
390-
->with('appstoreenabled', true)
391-
->willReturn(true);
392-
$this->config
393-
->expects($this->at(3))
394-
->method('getSystemValue')
395-
->with('appstoreurl', 'https://apps.nextcloud.com/api/v1')
396-
->willReturn('https://apps.nextcloud.com/api/v1');
397-
$this->config
398-
->expects($this->at(4))
399-
->method('getSystemValue')
400-
->with(
401-
$this->equalTo('version'),
402-
$this->anything()
403-
)->willReturn('11.0.0.2');
353+
->willReturnCallback(function ($var, $default) {
354+
if ($var === 'appstoreenabled') {
355+
return true;
356+
} elseif ($var === 'has_internet_connection') {
357+
return true;
358+
} elseif ($var === 'appstoreurl') {
359+
return 'https://apps.nextcloud.com/api/v1';
360+
} elseif ($var === 'version') {
361+
return '11.0.0.2';
362+
}
363+
return $default;
364+
});
404365

405366
$folder = $this->createMock(ISimpleFolder::class);
406367
$file = $this->createMock(ISimpleFile::class);

0 commit comments

Comments
 (0)