Skip to content

Commit c4356e1

Browse files
committed
fix(appstore): address review comments on GenericFileException handling
- Attempt delete before logging the warning, so the warning only fires when we know recovery will succeed - Log an error (not silently return) when delete itself fails - Use catch (\Exception) without variable (PHP 8) - Replace willReturnArgument(1) with explicit willReturn(true) in test - Add blank lines between logical blocks in test for readability Signed-off-by: Anna Larch <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5113b18 commit c4356e1

2 files changed

Lines changed: 6 additions & 3 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,13 @@ public function get($allowUnstable = false): array {
171171
// File does not already exist
172172
$file = $rootFolder->newFile($this->fileName);
173173
} catch (GenericFileException $e) {
174-
$this->logger->warning('Could not read appstore cache file, it will be refreshed', ['app' => 'appstoreFetcher', 'exception' => $e]);
175174
try {
176175
$file->delete();
177-
} catch (\Exception $deleteException) {
176+
} catch (\Exception) {
177+
$this->logger->error('Could not read appstore cache file', ['app' => 'appstoreFetcher', 'exception' => $e]);
178178
return [];
179179
}
180+
$this->logger->warning('Could not read appstore cache file, it will be refreshed', ['app' => 'appstoreFetcher', 'exception' => $e]);
180181
$file = $rootFolder->newFile($this->fileName);
181182
}
182183

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ public function testGetWithUnreadableCacheFileRecreatesAndFetches(): void {
698698
return $default;
699699
});
700700
$this->config->method('getSystemValueBool')
701-
->willReturnArgument(1);
701+
->willReturn(true);
702702

703703
$folder = $this->createMock(ISimpleFolder::class);
704704
$corruptedFile = $this->createMock(ISimpleFile::class);
@@ -725,6 +725,7 @@ public function testGetWithUnreadableCacheFileRecreatesAndFetches(): void {
725725
->method('newFile')
726726
->with($this->fileName)
727727
->willReturn($freshFile);
728+
728729
$client = $this->createMock(IClient::class);
729730
$this->clientService
730731
->expects($this->once())
@@ -743,6 +744,7 @@ public function testGetWithUnreadableCacheFileRecreatesAndFetches(): void {
743744
$response->method('getHeader')
744745
->with($this->equalTo('ETag'))
745746
->willReturn('"myETag"');
747+
746748
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
747749
$freshFile
748750
->expects($this->once())

0 commit comments

Comments
 (0)