From 3f8710500c96df7b84ebfe860abe474af66b0fa3 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 28 Apr 2026 16:32:45 +0200 Subject: [PATCH] chore: apply strict rector rules on appstore Signed-off-by: Ferdinand Thiessen --- apps/appstore/lib/AppInfo/Application.php | 5 +- .../appstore/lib/Controller/ApiController.php | 95 +++++++++++-------- .../lib/Controller/DiscoverController.php | 29 +++--- .../lib/Controller/PageController.php | 39 ++++---- apps/appstore/lib/Search/AppSearch.php | 10 +- apps/appstore/openapi.json | 31 +++++- apps/appstore/templates/empty.php | 2 + .../tests/Controller/ApiControllerTest.php | 13 ++- .../tests/Controller/PageControllerTest.php | 11 ++- build/rector-strict.php | 1 + lib/private/App/AppStore/Bundles/Bundle.php | 2 +- openapi.json | 31 +++++- psalm-strict.xml | 14 +-- 13 files changed, 184 insertions(+), 99 deletions(-) diff --git a/apps/appstore/lib/AppInfo/Application.php b/apps/appstore/lib/AppInfo/Application.php index 3c5dd05a3b2..9eed4437e5f 100644 --- a/apps/appstore/lib/AppInfo/Application.php +++ b/apps/appstore/lib/AppInfo/Application.php @@ -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); } diff --git a/apps/appstore/lib/Controller/ApiController.php b/apps/appstore/lib/Controller/ApiController.php index 1c0f3a05570..f56ff641722 100644 --- a/apps/appstore/lib/Controller/ApiController.php +++ b/apps/appstore/lib/Controller/ApiController.php @@ -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, array{}> + * @return DataResponse, internal: bool, isCompatible: bool, missingDependencies?: list, missingMaxNextcloudVersion: bool, missingMinNextcloudVersion: bool, ...}>, array{}> * * 200: The apps were found successfully */ @@ -90,6 +90,7 @@ class ApiController extends OCSController { public function listApps(): DataResponse { $apps = $this->getAllApps(); + /** @var array|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 $apps + * @var list, internal: bool, isCompatible: bool, missingDependencies?: list, missingMaxNextcloudVersion: bool, missingMinNextcloudVersion: bool, ...}> $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 - 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']; } } diff --git a/apps/appstore/lib/Controller/DiscoverController.php b/apps/appstore/lib/Controller/DiscoverController.php index 754c0642545..6abfbd0322d 100644 --- a/apps/appstore/lib/Controller/DiscoverController.php +++ b/apps/appstore/lib/Controller/DiscoverController.php @@ -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 } } diff --git a/apps/appstore/lib/Controller/PageController.php b/apps/appstore/lib/Controller/PageController.php index 5c9a1f360f6..7612772667e 100644 --- a/apps/appstore/lib/Controller/PageController.php +++ b/apps/appstore/lib/Controller/PageController.php @@ -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}> + */ + 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; } } diff --git a/apps/appstore/lib/Search/AppSearch.php b/apps/appstore/lib/Search/AppSearch.php index 5aaa40bbdb6..61040d666bb 100644 --- a/apps/appstore/lib/Search/AppSearch.php +++ b/apps/appstore/lib/Search/AppSearch.php @@ -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, []); } diff --git a/apps/appstore/openapi.json b/apps/appstore/openapi.json index f51051554e2..126e651248f 100644 --- a/apps/appstore/openapi.json +++ b/apps/appstore/openapi.json @@ -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" } } } diff --git a/apps/appstore/templates/empty.php b/apps/appstore/templates/empty.php index e64f6391bda..872a2b99f5b 100644 --- a/apps/appstore/templates/empty.php +++ b/apps/appstore/templates/empty.php @@ -1,5 +1,7 @@ */ abstract public function getAppIdentifiers(); } diff --git a/openapi.json b/openapi.json index 4cdd23def69..c5134f54514 100644 --- a/openapi.json +++ b/openapi.json @@ -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" } } } diff --git a/psalm-strict.xml b/psalm-strict.xml index d8f2488e2fa..5d9d09162a0 100644 --- a/psalm-strict.xml +++ b/psalm-strict.xml @@ -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 @@ + + + + + + + @@ -60,10 +67,5 @@ - - - - -