chore: apply strict rector rules on appstore

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2026-04-28 16:32:45 +02:00
parent f6a37dc608
commit 3f8710500c
13 changed files with 184 additions and 99 deletions

View file

@ -13,12 +13,9 @@ use OCP\AppFramework\Bootstrap\IBootContext;
use OCP\AppFramework\Bootstrap\IBootstrap;
use OCP\AppFramework\Bootstrap\IRegistrationContext;
class Application extends App implements IBootstrap {
final class Application extends App implements IBootstrap {
public const APP_ID = 'appstore';
/**
* @param array $urlParams
*/
public function __construct(array $urlParams = []) {
parent::__construct(self::APP_ID, $urlParams);
}

View file

@ -44,17 +44,17 @@ class ApiController extends OCSController {
public function __construct(
IRequest $request,
private IConfig $config,
private IAppConfig $appConfig,
private AppManager $appManager,
private DependencyAnalyzer $dependencyAnalyzer,
private CategoryFetcher $categoryFetcher,
private AppFetcher $appFetcher,
private IFactory $l10nFactory,
private BundleFetcher $bundleFetcher,
private Installer $installer,
private IRegistry $subscriptionRegistry,
private LoggerInterface $logger,
private readonly IConfig $config,
private readonly IAppConfig $appConfig,
private readonly AppManager $appManager,
private readonly DependencyAnalyzer $dependencyAnalyzer,
private readonly CategoryFetcher $categoryFetcher,
private readonly AppFetcher $appFetcher,
private readonly IFactory $l10nFactory,
private readonly BundleFetcher $bundleFetcher,
private readonly Installer $installer,
private readonly IRegistry $subscriptionRegistry,
private readonly LoggerInterface $logger,
) {
parent::__construct(Application::APP_ID, $request);
}
@ -71,7 +71,7 @@ class ApiController extends OCSController {
$currentLanguage = substr($this->l10nFactory->findLanguage(), 0, 2);
$categories = $this->categoryFetcher->get();
$categories = array_map(fn ($category) => [
$categories = array_map(fn (array $category): array => [
'id' => $category['id'],
'displayName' => $category['translations'][$currentLanguage]['name'] ?? $category['translations']['en']['name'],
], $categories);
@ -82,7 +82,7 @@ class ApiController extends OCSController {
/**
* Get all available apps
*
* @return DataResponse<Http::STATUS_OK, list<array{id: string, name: string, description: string, ...}>, array{}>
* @return DataResponse<Http::STATUS_OK, list<array{id: string, name: string, groups: list<string>, internal: bool, isCompatible: bool, missingDependencies?: list<string>, missingMaxNextcloudVersion: bool, missingMinNextcloudVersion: bool, ...<array-key, mixed>}>, array{}>
*
* 200: The apps were found successfully
*/
@ -90,6 +90,7 @@ class ApiController extends OCSController {
public function listApps(): DataResponse {
$apps = $this->getAllApps();
/** @var array<string>|mixed $ignoreMaxApps */
$ignoreMaxApps = $this->config->getSystemValue('app_install_overwrite', []);
if (!is_array($ignoreMaxApps)) {
$this->logger->warning('The value given for app_install_overwrite is not an array. Ignoring...');
@ -97,7 +98,7 @@ class ApiController extends OCSController {
}
// Extend existing app details
$apps = array_map(function (array $appData) use ($ignoreMaxApps) {
$apps = array_map(function (array $appData) use ($ignoreMaxApps): array {
if (isset($appData['appstoreData'])) {
$appstoreData = $appData['appstoreData'];
$appData['screenshot'] = $this->createProxyPreviewUrl($appstoreData['screenshots'][0]['url'] ?? '');
@ -106,7 +107,7 @@ class ApiController extends OCSController {
}
$newVersion = $this->installer->isUpdateAvailable($appData['id']);
if ($newVersion) {
if ($newVersion !== false) {
$appData['update'] = $newVersion;
}
@ -120,6 +121,7 @@ class ApiController extends OCSController {
$groups = [$groups];
}
}
$appData['groups'] = $groups;
$appData['canUninstall'] = !$appData['active'] && $appData['removable'];
@ -136,11 +138,10 @@ class ApiController extends OCSController {
return $appData;
}, $apps);
usort($apps, $this->sortApps(...));
/**
* @var list<array{id: string, name: string, description: string, ...}> $apps
* @var list<array{id: string, name: string, groups: list<string>, internal: bool, isCompatible: bool, missingDependencies?: list<string>, missingMaxNextcloudVersion: bool, missingMinNextcloudVersion: bool, ...<array-key, mixed>}> $apps
*/
usort($apps, $this->sortApps(...));
return new DataResponse($apps);
}
@ -174,16 +175,17 @@ class ApiController extends OCSController {
$this->installer->installApp($appId);
if (count($groups) > 0) {
if ($groups !== []) {
$this->appManager->enableAppForGroups($appId, $this->getGroupList($groups));
} else {
$this->appManager->enableApp($appId);
}
$updateRequired = $this->appManager->isUpgradeRequired($appId);
return new DataResponse(['update_required' => $updateRequired]);
} catch (\Throwable $e) {
$this->logger->error('could not enable app', ['exception' => $e]);
throw new OCSException('could not enable app', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
} catch (\Throwable $throwable) {
$this->logger->error('could not enable app', ['exception' => $throwable]);
throw new OCSException('could not enable app', Http::STATUS_INTERNAL_SERVER_ERROR, $throwable);
}
}
@ -204,9 +206,9 @@ class ApiController extends OCSController {
$appId = $this->appManager->cleanAppId($appId);
$this->appManager->disableApp($appId);
return new DataResponse([]);
} catch (\Exception $e) {
$this->logger->error('could not disable app', ['exception' => $e]);
throw new OCSException('could not disable app', Http::STATUS_INTERNAL_SERVER_ERROR, $e);
} catch (\Exception $exception) {
$this->logger->error('could not disable app', ['exception' => $exception]);
throw new OCSException('could not disable app', Http::STATUS_INTERNAL_SERVER_ERROR, $exception);
}
}
@ -231,6 +233,7 @@ class ApiController extends OCSController {
$this->appManager->clearAppsCache();
return new DataResponse([]);
}
throw new OCSException('could not remove app', Http::STATUS_INTERNAL_SERVER_ERROR);
}
@ -255,9 +258,9 @@ class ApiController extends OCSController {
if ($result === false) {
throw new \Exception('Update failed');
}
} catch (\Exception $ex) {
} catch (\Exception $exception) {
$this->config->setSystemValue('maintenance', false);
throw new OCSException('could not update app', Http::STATUS_INTERNAL_SERVER_ERROR, $ex);
throw new OCSException('could not update app', Http::STATUS_INTERNAL_SERVER_ERROR, $exception);
}
return new DataResponse([]);
@ -298,10 +301,11 @@ class ApiController extends OCSController {
if ($url === '') {
return '';
}
return 'https://usercontent.apps.nextcloud.com/' . base64_encode($url);
}
private function fetchApps() {
private function fetchApps(): void {
$appClass = new \OC_App();
$apps = $appClass->listAllApps();
foreach ($apps as $app) {
@ -316,6 +320,7 @@ class ApiController extends OCSController {
$app['screenshot'] = $this->createProxyPreviewUrl($appScreenshot);
}
$this->allApps[$app['id']] = $app;
}
@ -352,17 +357,16 @@ class ApiController extends OCSController {
if (empty($this->allApps)) {
$this->fetchApps();
}
return $this->allApps;
}
/**
* Get all apps for a category from the app store
*
* @param string $requestedCategory
* @return array
* @throws \Exception
*/
private function getAppsForCategory($requestedCategory = ''): array {
private function getAppsForCategory(string $requestedCategory = ''): array {
$versionParser = new VersionParser();
$formattedApps = [];
$apps = $this->appFetcher->get();
@ -375,6 +379,7 @@ class ApiController extends OCSController {
$isInCategory = true;
}
}
if (!$isInCategory) {
continue;
}
@ -383,14 +388,17 @@ class ApiController extends OCSController {
if (!isset($app['releases'][0]['rawPlatformVersionSpec'])) {
continue;
}
$nextcloudVersion = $versionParser->getVersion($app['releases'][0]['rawPlatformVersionSpec']);
$nextcloudVersionDependencies = [];
if ($nextcloudVersion->getMinimumVersion() !== '') {
$nextcloudVersionDependencies['nextcloud']['@attributes']['min-version'] = $nextcloudVersion->getMinimumVersion();
}
if ($nextcloudVersion->getMaximumVersion() !== '') {
$nextcloudVersionDependencies['nextcloud']['@attributes']['max-version'] = $nextcloudVersion->getMaximumVersion();
}
$phpVersion = $versionParser->getVersion($app['releases'][0]['rawPhpVersionSpec']);
try {
@ -404,12 +412,15 @@ class ApiController extends OCSController {
if ($phpVersion->getMinimumVersion() !== '') {
$phpDependencies['php']['@attributes']['min-version'] = $phpVersion->getMinimumVersion();
}
if ($phpVersion->getMaximumVersion() !== '') {
$phpDependencies['php']['@attributes']['max-version'] = $phpVersion->getMaximumVersion();
}
if (isset($app['releases'][0]['minIntSize'])) {
$phpDependencies['php']['@attributes']['min-int-size'] = $app['releases'][0]['minIntSize'];
}
$authors = '';
foreach ($app['authors'] as $key => $author) {
$authors .= $author['name'];
@ -474,24 +485,28 @@ class ApiController extends OCSController {
return $formattedApps;
}
private function getGroupList(array $groups) {
/**
* @param string[] $groups - The group ids to fetch
* @return list<IGroup> - The list of groups matching the given group ids
*/
private function getGroupList(array $groups): array {
$groupManager = Server::get(IGroupManager::class);
$groupsList = [];
foreach ($groups as $group) {
$groupItem = $groupManager->get($group);
if ($groupItem instanceof IGroup) {
$groupsList[] = $groupManager->get($group);
$groupsList[] = $groupItem;
}
}
return $groupsList;
}
private function sortApps($a, $b) {
$a = (string)$a['name'];
$b = (string)$b['name'];
if ($a === $b) {
return 0;
}
return ($a < $b) ? -1 : 1;
/**
* @param array{name: string, ...} $a
* @param array{name: string, ...} $b
*/
private function sortApps(array $a, array $b): int {
return $a['name'] <=> $b['name'];
}
}

View file

@ -37,16 +37,16 @@ use Psr\Log\LoggerInterface;
* @psalm-import-type AppStoreFetcherDiscoverElement from ResponseDefinitions
*/
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
class DiscoverController extends OCSController {
final class DiscoverController extends OCSController {
private IAppData $appData;
private readonly IAppData $appData;
public function __construct(
IRequest $request,
IAppDataFactory $appDataFactory,
private IClientService $clientService,
private AppDiscoverFetcher $discoverFetcher,
private LoggerInterface $logger,
private readonly IClientService $clientService,
private readonly AppDiscoverFetcher $discoverFetcher,
private readonly LoggerInterface $logger,
) {
parent::__construct(Application::APP_ID, $request);
$this->appData = $appDataFactory->get(Application::APP_ID);
@ -84,18 +84,17 @@ class DiscoverController extends OCSController {
$getEtag = $this->discoverFetcher->getETag() ?? date('Y-m');
$etag = trim($getEtag, '"');
$folder = null;
try {
$folder = $this->appData->getFolder('app-discover-cache');
$this->cleanUpImageCache($folder, $etag);
} catch (\Throwable $e) {
} catch (\Throwable) {
$folder = $this->appData->newFolder('app-discover-cache');
}
// Get the current cache folder
try {
$folder = $folder->getFolder($etag);
} catch (NotFoundException $e) {
} catch (NotFoundException) {
$folder = $folder->newFolder($etag);
}
@ -103,9 +102,7 @@ class DiscoverController extends OCSController {
$hashName = md5($fileName);
$allFiles = $folder->getDirectoryListing();
// Try to find the file
$file = array_filter($allFiles, function (ISimpleFile $file) use ($hashName) {
return str_starts_with($file->getName(), $hashName);
});
$file = array_filter($allFiles, fn (ISimpleFile $file): bool => str_starts_with($file->getName(), $hashName));
// Get the first entry
$file = reset($file);
// If not found request from Web
@ -165,15 +162,11 @@ class DiscoverController extends OCSController {
// Hosts that need further verification
// Github is only allowed if from our organization
$ALLOWED_HOSTS = ['github.com', 'raw.githubusercontent.com'];
if (!in_array($urlInfo['host'], $ALLOWED_HOSTS)) {
if (!in_array($urlInfo['host'], $ALLOWED_HOSTS, true)) {
return false;
}
if (str_starts_with($urlInfo['path'], '/nextcloud/') || str_starts_with($urlInfo['path'], '/nextcloud-gmbh/')) {
return true;
}
return false;
return str_starts_with($urlInfo['path'], '/nextcloud/') || str_starts_with($urlInfo['path'], '/nextcloud-gmbh/');
}
/**
@ -189,7 +182,7 @@ class DiscoverController extends OCSController {
if ($dir->getName() !== $etag) {
$dir->delete();
}
} catch (NotPermittedException $e) {
} catch (NotPermittedException) {
// ignore folder for now
}
}

View file

@ -30,27 +30,22 @@ use OCP\Server;
use OCP\Util;
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
class PageController extends Controller {
final class PageController extends Controller {
public function __construct(
IRequest $request,
private IL10N $l10n,
private IConfig $config,
private Installer $installer,
private AppManager $appManager,
private IURLGenerator $urlGenerator,
private IInitialState $initialState,
private BundleFetcher $bundleFetcher,
private INavigationManager $navigationManager,
private readonly IL10N $l10n,
private readonly IConfig $config,
private readonly Installer $installer,
private readonly AppManager $appManager,
private readonly IURLGenerator $urlGenerator,
private readonly IInitialState $initialState,
private readonly BundleFetcher $bundleFetcher,
private readonly INavigationManager $navigationManager,
) {
parent::__construct(Application::APP_ID, $request);
}
/**
* @psalm-suppress UndefinedClass AppAPI is shipped since 30.0.1
*
* @return TemplateResponse
*/
#[NoCSRFRequired]
#[FrontpageRoute(verb: 'GET', url: '/settings/apps', root: '')]
#[FrontpageRoute(verb: 'GET', url: '/settings/apps/{category}', defaults: ['category' => ''], root: '')]
@ -65,8 +60,12 @@ class PageController extends Controller {
if ($this->appManager->isEnabledForAnyone('app_api')) {
try {
/**
* @psalm-suppress UndefinedClass AppAPI is shipped since 30.0.1
*/
Server::get(ExAppsPageService::class)->provideAppApiState($this->initialState);
} catch (\Psr\Container\NotFoundExceptionInterface|\Psr\Container\ContainerExceptionInterface $e) {
} catch (\Psr\Container\NotFoundExceptionInterface|\Psr\Container\ContainerExceptionInterface) {
// nop
}
}
@ -83,19 +82,24 @@ class PageController extends Controller {
}
private function getAppsWithUpdates() {
private function getAppsWithUpdates(): array {
$appClass = new \OC_App();
$apps = $appClass->listAllApps();
/** @var array{id: string, ...} $app */
foreach ($apps as $key => $app) {
$newVersion = $this->installer->isUpdateAvailable($app['id']);
if ($newVersion === false) {
unset($apps[$key]);
}
}
return $apps;
}
private function getBundles() {
/**
* @return list<array{name: string, id: string, appIdentifiers: list<string>}>
*/
private function getBundles(): array {
$result = [];
$bundles = $this->bundleFetcher->getBundles();
foreach ($bundles as $bundle) {
@ -105,6 +109,7 @@ class PageController extends Controller {
'appIdentifiers' => $bundle->getAppIdentifiers()
];
}
return $result;
}
}

View file

@ -17,10 +17,10 @@ use OCP\Search\ISearchQuery;
use OCP\Search\SearchResult;
use OCP\Search\SearchResultEntry;
class AppSearch implements IProvider {
final readonly class AppSearch implements IProvider {
public function __construct(
protected INavigationManager $navigationManager,
protected IL10N $l,
private INavigationManager $navigationManager,
private IL10N $l,
) {
}
@ -44,8 +44,8 @@ class AppSearch implements IProvider {
$entries = $this->navigationManager->getAll('all');
$searchTitle = $this->l->t('Apps');
$term = $query->getFilter('term')?->get();
if (empty($term)) {
$term = (string)$query->getFilter('term')?->get();
if ($term === '') {
return SearchResult::complete($searchTitle, []);
}

View file

@ -236,7 +236,11 @@
"required": [
"id",
"name",
"description"
"groups",
"internal",
"isCompatible",
"missingMaxNextcloudVersion",
"missingMinNextcloudVersion"
],
"properties": {
"id": {
@ -245,8 +249,29 @@
"name": {
"type": "string"
},
"description": {
"type": "string"
"groups": {
"type": "array",
"items": {
"type": "string"
}
},
"internal": {
"type": "boolean"
},
"isCompatible": {
"type": "boolean"
},
"missingDependencies": {
"type": "array",
"items": {
"type": "string"
}
},
"missingMaxNextcloudVersion": {
"type": "boolean"
},
"missingMinNextcloudVersion": {
"type": "boolean"
}
}
}

View file

@ -1,5 +1,7 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later

View file

@ -25,18 +25,29 @@ use Psr\Log\LoggerInterface;
use Test\TestCase;
#[\PHPUnit\Framework\Attributes\Group('DB')]
class ApiControllerTest extends TestCase {
final class ApiControllerTest extends TestCase {
private IRequest&MockObject $request;
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private AppManager&MockObject $appManager;
private DependencyAnalyzer&MockObject $dependencyAnalyzer;
private CategoryFetcher&MockObject $categoryFetcher;
private AppFetcher&MockObject $appFetcher;
private IFactory&MockObject $l10nFactory;
private BundleFetcher&MockObject $bundleFetcher;
private Installer&MockObject $installer;
private IRegistry&MockObject $subscriptionRegistry;
private LoggerInterface&MockObject $logger;
private ApiController $apiController;

View file

@ -23,16 +23,25 @@ use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
#[\PHPUnit\Framework\Attributes\Group('DB')]
class PageControllerTest extends TestCase {
final class PageControllerTest extends TestCase {
private IRequest&MockObject $request;
private IL10N&MockObject $l10n;
private IConfig&MockObject $config;
private INavigationManager&MockObject $navigationManager;
private AppManager&MockObject $appManager;
private BundleFetcher&MockObject $bundleFetcher;
private Installer&MockObject $installer;
private IURLGenerator&MockObject $urlGenerator;
private IInitialState&MockObject $initialState;
private PageController $pageController;
protected function setUp(): void {

View file

@ -12,6 +12,7 @@ return (require __DIR__ . '/rector-shared.php')
$nextcloudDir . '/build/rector-strict.php',
$nextcloudDir . '/core/BackgroundJobs/ExpirePreviewsJob.php',
$nextcloudDir . '/lib/public/IContainer.php',
$nextcloudDir . '/apps/appstore',
$nextcloudDir . '/apps/dav/lib/Connector/Sabre/Node.php',
$nextcloudDir . '/apps/files_versions/lib/Versions/IMetadataVersion.php',
$nextcloudDir . '/lib/private/Settings/AuthorizedGroup.php',

View file

@ -36,7 +36,7 @@ abstract class Bundle {
/**
* Get the list of app identifiers in the bundle
*
* @return array
* @return list<string>
*/
abstract public function getAppIdentifiers();
}

View file

@ -17244,7 +17244,11 @@
"required": [
"id",
"name",
"description"
"groups",
"internal",
"isCompatible",
"missingMaxNextcloudVersion",
"missingMinNextcloudVersion"
],
"properties": {
"id": {
@ -17253,8 +17257,29 @@
"name": {
"type": "string"
},
"description": {
"type": "string"
"groups": {
"type": "array",
"items": {
"type": "string"
}
},
"internal": {
"type": "boolean"
},
"isCompatible": {
"type": "boolean"
},
"missingDependencies": {
"type": "array",
"items": {
"type": "string"
}
},
"missingMaxNextcloudVersion": {
"type": "boolean"
},
"missingMinNextcloudVersion": {
"type": "boolean"
}
}
}

View file

@ -7,7 +7,7 @@
errorLevel="1"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor-bin/psalm/vendor/vimeo/psalm/config.xsd"
xsi:schemaLocation="https://getpsalm.org/schema/config https://getpsalm.org/schema/config"
resolveFromConfigFile="false"
findUnusedBaselineEntry="true"
findUnusedCode="false"
@ -35,11 +35,18 @@
<file name="lib/public/DB/QueryBuilder/ITypedQueryBuilder.php"/>
<file name="lib/private/Share20/ShareHelper.php"/>
<file name="lib/public/Share/IShareHelper.php"/>
<directory name="apps/appstore/lib" />
<ignoreFiles>
<!-- Missing types of the AppFetcher and the OC_Apps class for return types -->
<file name="apps/appstore/lib/Controller/ApiController.php" />
<!-- ... -->
<directory name="apps/**/composer"/>
<directory name="lib/composer"/>
<directory name="lib/l10n"/>
<directory name="3rdparty"/>
<directory name="lib/public"/>
<directory name="lib/private"/>
<directory name="build"/>
</ignoreFiles>
</projectFiles>
<extraFiles>
@ -60,10 +67,5 @@
<directory name="."/>
</errorLevel>
</InternalMethod>
<UnusedVariable>
<errorLevel type="suppress">
<file name="build/psalm/ITypedQueryBuilderTest.php"/>
</errorLevel>
</UnusedVariable>
</issueHandlers>
</psalm>