Skip to content

Commit

Permalink
fix(AppDiscoverFetcher): Do not remove entries as expired that have n…
Browse files Browse the repository at this point in the history
…o expiry date

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed Mar 18, 2024
1 parent 79e9cdc commit 220964b
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 19 deletions.
2 changes: 1 addition & 1 deletion apps/settings/lib/Controller/AppSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public function viewApps(): TemplateResponse {
* @NoCSRFRequired
*/
public function getAppDiscoverJSON(): JSONResponse {
$data = $this->discoverFetcher->get();
$data = $this->discoverFetcher->get(true);
return new JSONResponse($data);
}

Expand Down
36 changes: 18 additions & 18 deletions lib/private/App/AppStore/Fetcher/AppDiscoverFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,41 +61,41 @@ public function __construct(
/**
* Get the app discover section entries
*
* @param bool $allowUnstable Include also expired and upcoming entries
* @param bool $allowUnstable Include also upcoming entries
*/
public function get($allowUnstable = false) {
$entries = parent::get(false);
$now = new DateTimeImmutable();

if (!$allowUnstable) {
$now = new DateTimeImmutable();

// Remove expired or future entries
return array_filter($entries, function (array $entry) use ($now) {
return array_filter($entries, function (array $entry) use ($now, $allowUnstable) {
// Always remove expired entries
if (isset($entry['expiryDate'])) {
try {
$date = new DateTimeImmutable($entry['date'] ?? '');
if ($date > $now) {
$expiryDate = new DateTimeImmutable($entry['expiryDate']);
if ($expiryDate < $now) {
return false;
}
} catch (\Throwable $e) {
// Invalid date format
// Invalid expiryDate format
return false;
}
}

// If not include upcoming entries, check for upcoming dates and remove those entries
if (!$allowUnstable && isset($entry['date'])) {
try {
$expiryDate = new DateTimeImmutable($entry['expiryDate'] ?? '');
if ($expiryDate < $now) {
$date = new DateTimeImmutable($entry['date']);
if ($date > $now) {
return false;
}
} catch (\Throwable $e) {
// Invalid expiryDate format
// Invalid date format
return false;
}

return true;
});
}

return $entries;
}
// Otherwise the entry is not time limited and should stay
return true;
});
}

public function getETag(): string|null {
Expand Down
133 changes: 133 additions & 0 deletions tests/lib/App/AppStore/Fetcher/AppDiscoverFetcherTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
<?php
/**
* @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de>
*
* @author Ferdinand Thiessen <opensource@fthiessen.de>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace Test\App\AppStore\Fetcher;

use OC\App\AppStore\Fetcher\AppDiscoverFetcher;
use OC\App\CompareVersion;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\Files\SimpleFS\ISimpleFolder;
use PHPUnit\Framework\MockObject\MockObject;

class AppDiscoverFetcherTest extends FetcherBase {
protected CompareVersion|MockObject $compareVersion;

protected function setUp(): void {
parent::setUp();
$this->fileName = 'discover.json';
$this->endpoint = 'https://apps.nextcloud.com/api/v1/discover.json';

$this->compareVersion = $this->createMock(CompareVersion::class);

$this->fetcher = new AppDiscoverFetcher(
$this->appDataFactory,
$this->clientService,
$this->timeFactory,
$this->config,
$this->logger,
$this->registry,
$this->compareVersion,
);
}

public function testAppstoreDisabled() {
$this->config
->method('getSystemValueBool')
->willReturnCallback(function ($var, $default) {
if ($var === 'appstoreenabled') {
return false;
}
return $default;
});
$this->appData
->expects($this->never())
->method('getFolder');

$this->assertEquals([], $this->fetcher->get());
}

public function testNoInternet() {
$this->config
->method('getSystemValueBool')
->willReturnCallback(function ($var, $default) {
if ($var === 'has_internet_connection') {
return false;
}
return $default;
});
$this->config
->method('getSystemValueString')
->willReturnCallback(function ($var, $default) {
return $default;
});
$this->appData
->expects($this->never())
->method('getFolder');

$this->assertEquals([], $this->fetcher->get());
}

/**
* @dataProvider dataGetETag
*/
public function testGetEtag(string|null $expected, bool $throws, string $content = '') {
$folder = $this->createMock(ISimpleFolder::class);
if (!$throws) {
$file = $this->createMock(ISimpleFile::class);
$file->expects($this->once())
->method('getContent')
->willReturn($content);
$folder->expects($this->once())
->method('getFile')
->with('discover.json')
->willReturn($file);
} else {
$folder->expects($this->once())
->method('getFile')
->with('discover.json')
->willThrowException(new NotFoundException(''));
}

$this->appData->expects($this->once())
->method('getFolder')
->with('/')
->willReturn($folder);

$etag = $this->fetcher->getETag();
$this->assertEquals($expected, $etag);
if ($expected !== null) {
$this->assertTrue(gettype($etag) === 'string');
}
}

public function dataGetETag(): array {
return [
'file not found' => [null, true],
'empty file' => [null, false, ''],
'missing etag' => [null, false, '{ "foo": "bar" }'],
'valid etag' => ['test', false, '{ "ETag": "test" }'],
'numeric etag' => ['132', false, '{ "ETag": 132 }'],
];
}
}

0 comments on commit 220964b

Please sign in to comment.