Skip to content

Commit ca3cb7f

Browse files
committed
Only retry fetching app store data once every 5 minutes in case it failed
Signed-off-by: Julius Härtl <jus@bitgrid.net>
1 parent 9bdf0ee commit ca3cb7f

File tree

5 files changed

+42
-8
lines changed

5 files changed

+42
-8
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use OC\Files\AppData\Factory;
3636
use OCP\AppFramework\Utility\ITimeFactory;
3737
use OCP\Http\Client\IClientService;
38+
use OCP\ICacheFactory;
3839
use OCP\IConfig;
3940
use OCP\ILogger;
4041

@@ -59,13 +60,16 @@ public function __construct(Factory $appDataFactory,
5960
ITimeFactory $timeFactory,
6061
IConfig $config,
6162
CompareVersion $compareVersion,
62-
ILogger $logger) {
63+
ILogger $logger,
64+
ICacheFactory $cacheFactory
65+
) {
6366
parent::__construct(
6467
$appDataFactory,
6568
$clientService,
6669
$timeFactory,
6770
$config,
68-
$logger
71+
$logger,
72+
$cacheFactory
6973
);
7074

7175
$this->fileName = 'apps.json';

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
use OC\Files\AppData\Factory;
3131
use OCP\AppFramework\Utility\ITimeFactory;
3232
use OCP\Http\Client\IClientService;
33+
use OCP\ICacheFactory;
3334
use OCP\IConfig;
3435
use OCP\ILogger;
3536

@@ -45,13 +46,16 @@ public function __construct(Factory $appDataFactory,
4546
IClientService $clientService,
4647
ITimeFactory $timeFactory,
4748
IConfig $config,
48-
ILogger $logger) {
49+
ILogger $logger,
50+
ICacheFactory $cacheFactory
51+
) {
4952
parent::__construct(
5053
$appDataFactory,
5154
$clientService,
5255
$timeFactory,
5356
$config,
54-
$logger
57+
$logger,
58+
$cacheFactory
5559
);
5660
$this->fileName = 'categories.json';
5761
$this->endpointName = 'categories.json';

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
use OCP\Files\IAppData;
3838
use OCP\Files\NotFoundException;
3939
use OCP\Http\Client\IClientService;
40+
use OCP\ICache;
41+
use OCP\ICacheFactory;
4042
use OCP\IConfig;
4143
use OCP\ILogger;
4244

@@ -61,6 +63,8 @@ abstract class Fetcher {
6163
protected $version;
6264
/** @var string */
6365
protected $channel;
66+
/** @var ICache */
67+
private $cache;
6468

6569
/**
6670
* @param Factory $appDataFactory
@@ -73,12 +77,15 @@ public function __construct(Factory $appDataFactory,
7377
IClientService $clientService,
7478
ITimeFactory $timeFactory,
7579
IConfig $config,
76-
ILogger $logger) {
80+
ILogger $logger,
81+
ICacheFactory $cacheFactory
82+
) {
7783
$this->appData = $appDataFactory->get('appstore');
7884
$this->clientService = $clientService;
7985
$this->timeFactory = $timeFactory;
8086
$this->config = $config;
8187
$this->logger = $logger;
88+
$this->cache = $cacheFactory->createDistributed('appstore-fetcher');
8289
}
8390

8491
/**
@@ -91,6 +98,9 @@ public function __construct(Factory $appDataFactory,
9198
*/
9299
protected function fetch($ETag, $content) {
93100
$appstoreenabled = $this->config->getSystemValue('appstoreenabled', true);
101+
if ((bool)$this->cache->get('lastFetchFailure')) {
102+
return [];
103+
}
94104

95105
if (!$appstoreenabled) {
96106
return [];
@@ -107,7 +117,13 @@ protected function fetch($ETag, $content) {
107117
}
108118

109119
$client = $this->clientService->newClient();
110-
$response = $client->get($this->getEndpoint(), $options);
120+
try {
121+
$response = $client->get($this->getEndpoint(), $options);
122+
} catch (ConnectException $e) {
123+
// Only retry app store fetching once every 5 minutes when failing
124+
$this->cache->set('lastFetchFailure', true, 300);
125+
throw $e;
126+
}
111127

112128
$responseJson = [];
113129
if ($response->getStatusCode() === Http::STATUS_NOT_MODIFIED) {
@@ -175,6 +191,9 @@ public function get($allowUnstable = false) {
175191
// Refresh the file content
176192
try {
177193
$responseJson = $this->fetch($ETag, $content, $allowUnstable);
194+
if (empty($responseJson['data'])) {
195+
return [];
196+
}
178197
// Don't store the apps request file
179198
if ($allowUnstable) {
180199
return $responseJson['data'];

tests/lib/App/AppStore/Fetcher/AppFetcherTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use OCP\Http\Client\IClient;
3434
use OCP\Http\Client\IClientService;
3535
use OCP\Http\Client\IResponse;
36+
use OCP\ICacheFactory;
3637
use OCP\IConfig;
3738
use OCP\ILogger;
3839
use Test\TestCase;
@@ -1849,14 +1850,16 @@ protected function setUp(): void {
18491850
$this->config = $this->createMock(IConfig::class);
18501851
$this->compareVersion = new CompareVersion();
18511852
$this->logger = $this->createMock(ILogger::class);
1853+
$cacheFactory = $this->createMock(ICacheFactory::class);
18521854

18531855
$this->fetcher = new AppFetcher(
18541856
$factory,
18551857
$this->clientService,
18561858
$this->timeFactory,
18571859
$this->config,
18581860
$this->compareVersion,
1859-
$this->logger
1861+
$this->logger,
1862+
$cacheFactory
18601863
);
18611864
}
18621865

tests/lib/App/AppStore/Fetcher/CategoryFetcherTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,23 @@
2222
namespace Test\App\AppStore\Fetcher;
2323

2424
use OC\App\AppStore\Fetcher\CategoryFetcher;
25+
use OCP\ICacheFactory;
2526

2627
class CategoryFetcherTest extends FetcherBase {
2728
protected function setUp(): void {
2829
parent::setUp();
2930
$this->fileName = 'categories.json';
3031
$this->endpoint = 'https://apps.nextcloud.com/api/v1/categories.json';
3132

33+
$cacheFactory = $this->createMock(ICacheFactory::class);
34+
3235
$this->fetcher = new CategoryFetcher(
3336
$this->appDataFactory,
3437
$this->clientService,
3538
$this->timeFactory,
3639
$this->config,
37-
$this->logger
40+
$this->logger,
41+
$cacheFactory
3842
);
3943
}
4044

0 commit comments

Comments
 (0)