From 9d7ba80bf118a9fe8633bbcd9b9c6ca382afae6d Mon Sep 17 00:00:00 2001 From: Faraz Samapoor Date: Wed, 1 May 2024 09:58:03 +0330 Subject: [PATCH] Improves readability mainly by utilizing PHP8's constructor property promotion feature. Signed-off-by: Faraz Samapoor --- apps/theming/lib/Command/UpdateConfig.php | 32 +++++------ .../theming/lib/Controller/IconController.php | 27 ++-------- .../lib/Controller/ThemingController.php | 53 ++++++++----------- .../lib/Controller/UserThemeController.php | 33 ++++-------- .../BeforeTemplateRenderedListener.php | 22 ++------ 5 files changed, 55 insertions(+), 112 deletions(-) diff --git a/apps/theming/lib/Command/UpdateConfig.php b/apps/theming/lib/Command/UpdateConfig.php index de180db6ce960..a4a3d83e79dd6 100644 --- a/apps/theming/lib/Command/UpdateConfig.php +++ b/apps/theming/lib/Command/UpdateConfig.php @@ -36,19 +36,15 @@ class UpdateConfig extends Command { 'name', 'url', 'imprintUrl', 'privacyUrl', 'slogan', 'color', 'disable-user-theming' ]; - private $themingDefaults; - private $imageManager; - private $config; - - public function __construct(ThemingDefaults $themingDefaults, ImageManager $imageManager, IConfig $config) { + public function __construct( + private ThemingDefaults $themingDefaults, + private ImageManager $imageManager, + private IConfig $config, + ) { parent::__construct(); - - $this->themingDefaults = $themingDefaults; - $this->imageManager = $imageManager; - $this->config = $config; } - protected function configure() { + protected function configure(): void { $this ->setName('theming:config') ->setDescription('Set theming app config values') @@ -87,18 +83,18 @@ protected function execute(InputInterface $input, OutputInterface $output): int $value = $this->config->getAppValue('theming', $key . 'Mime', ''); $output->writeln('- ' . $key . ': ' . $value . ''); } - return 0; + return self::SUCCESS; } if (!in_array($key, self::SUPPORTED_KEYS, true) && !in_array($key, ImageManager::SUPPORTED_IMAGE_KEYS, true)) { $output->writeln('Invalid config key provided'); - return 1; + return self::FAILURE; } if ($input->getOption('reset')) { $defaultValue = $this->themingDefaults->undo($key); $output->writeln('Reset ' . $key . ' to ' . $defaultValue . ''); - return 0; + return self::SUCCESS; } if ($value === null) { @@ -108,7 +104,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } else { $output->writeln('' . $key . ' is currently not set'); } - return 0; + return self::SUCCESS; } if ($key === 'background' && $value === 'backgroundColor') { @@ -119,11 +115,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int if (in_array($key, ImageManager::SUPPORTED_IMAGE_KEYS, true)) { if (!str_starts_with($value, '/')) { $output->writeln('The image file needs to be provided as an absolute path: ' . $value . '.'); - return 1; + return self::FAILURE; } if (!file_exists($value)) { $output->writeln('File could not be found: ' . $value . '.'); - return 1; + return self::FAILURE; } $value = $this->imageManager->updateImage($key, $value); $key = $key . 'Mime'; @@ -131,12 +127,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($key === 'color' && !preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) { $output->writeln('The given color is invalid: ' . $value . ''); - return 1; + return self::FAILURE; } $this->themingDefaults->set($key, $value); $output->writeln('Updated ' . $key . ' to ' . $value . ''); - return 0; + return self::SUCCESS; } } diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 216ca88d375df..f24439b4ec9b3 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -43,33 +43,16 @@ use OCP\IRequest; class IconController extends Controller { - /** @var ThemingDefaults */ - private $themingDefaults; - /** @var IconBuilder */ - private $iconBuilder; - /** @var ImageManager */ - private $imageManager; - /** @var FileAccessHelper */ - private $fileAccessHelper; - /** @var IAppManager */ - private $appManager; - public function __construct( $appName, IRequest $request, - ThemingDefaults $themingDefaults, - IconBuilder $iconBuilder, - ImageManager $imageManager, - FileAccessHelper $fileAccessHelper, - IAppManager $appManager + private ThemingDefaults $themingDefaults, + private IconBuilder $iconBuilder, + private ImageManager $imageManager, + private FileAccessHelper $fileAccessHelper, + private IAppManager $appManager ) { parent::__construct($appName, $request); - - $this->themingDefaults = $themingDefaults; - $this->iconBuilder = $iconBuilder; - $this->imageManager = $imageManager; - $this->fileAccessHelper = $fileAccessHelper; - $this->appManager = $appManager; } /** diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 91012d1e37a69..aeecb3ee60023 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -70,50 +70,33 @@ class ThemingController extends Controller { public const VALID_UPLOAD_KEYS = ['header', 'logo', 'logoheader', 'background', 'favicon']; - private ThemingDefaults $themingDefaults; private IL10N $l10n; - private IConfig $config; - private ITempManager $tempManager; - private IAppData $appData; - private IURLGenerator $urlGenerator; - private IAppManager $appManager; - private ImageManager $imageManager; - private ThemesService $themesService; public function __construct( $appName, IRequest $request, - IConfig $config, - ThemingDefaults $themingDefaults, + private IConfig $config, + private ThemingDefaults $themingDefaults, IL10N $l, - ITempManager $tempManager, - IAppData $appData, - IURLGenerator $urlGenerator, - IAppManager $appManager, - ImageManager $imageManager, - ThemesService $themesService + private ITempManager $tempManager, + private IAppData $appData, + private IURLGenerator $urlGenerator, + private IAppManager $appManager, + private ImageManager $imageManager, + private ThemesService $themesService, ) { parent::__construct($appName, $request); - $this->themingDefaults = $themingDefaults; $this->l10n = $l; - $this->config = $config; - $this->tempManager = $tempManager; - $this->appData = $appData; - $this->urlGenerator = $urlGenerator; - $this->appManager = $appManager; - $this->imageManager = $imageManager; - $this->themesService = $themesService; } /** * @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin) * @param string $setting * @param string $value - * @return DataResponse * @throws NotPermittedException */ - public function updateStylesheet($setting, $value) { + public function updateStylesheet($setting, $value): DataResponse { $value = trim($value); $error = null; switch ($setting) { @@ -188,7 +171,7 @@ public function updateStylesheet($setting, $value) { * @return DataResponse * @throws NotPermittedException */ - public function updateAppMenu($setting, $value) { + public function updateAppMenu($setting, $value): DataResponse { $error = null; switch ($setting) { case 'defaultApps': @@ -226,8 +209,15 @@ public function updateAppMenu($setting, $value) { * Check that a string is a valid http/https url */ private function isValidUrl(string $url): bool { - return ((str_starts_with($url, 'http://') || str_starts_with($url, 'https://')) && - filter_var($url, FILTER_VALIDATE_URL) !== false); + if (!str_starts_with($url, 'http://') && !str_starts_with($url, 'https://')) { + return false; + } + + if (filter_var($url, FILTER_VALIDATE_URL) === false) { + return false; + } + + return true; } /** @@ -369,7 +359,7 @@ public function undoAll(): DataResponse { * 200: Image returned * 404: Image not found */ - public function getImage(string $key, bool $useSvg = true) { + public function getImage(string $key, bool $useSvg = true): NotFoundResponse|FileDisplayResponse { try { $file = $this->imageManager->getImage($key, $useSvg); } catch (NotFoundException $e) { @@ -407,7 +397,7 @@ public function getImage(string $key, bool $useSvg = true) { * 200: Stylesheet returned * 404: Theme not found */ - public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false) { + public function getThemeStylesheet(string $themeId, bool $plain = false, bool $withCustomCss = false): NotFoundResponse|DataDisplayResponse { $themes = $this->themesService->getThemes(); if (!in_array($themeId, array_keys($themes))) { return new NotFoundResponse(); @@ -430,7 +420,6 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w $compiler = new Compiler(); $compiledCss = $compiler->compileString("[data-theme-$themeId] { $variables $customCss }"); $css = $compiledCss->getCss(); - ; } try { diff --git a/apps/theming/lib/Controller/UserThemeController.php b/apps/theming/lib/Controller/UserThemeController.php index 657c20036d36c..a526c41928e53 100644 --- a/apps/theming/lib/Controller/UserThemeController.php +++ b/apps/theming/lib/Controller/UserThemeController.php @@ -54,33 +54,20 @@ * @psalm-import-type ThemingBackground from ResponseDefinitions */ class UserThemeController extends OCSController { - protected ?string $userId = null; - private IConfig $config; - private IUserSession $userSession; - private ThemesService $themesService; - private ThemingDefaults $themingDefaults; - private BackgroundService $backgroundService; - - public function __construct(string $appName, + public function __construct( + string $appName, IRequest $request, - IConfig $config, - IUserSession $userSession, - ThemesService $themesService, - ThemingDefaults $themingDefaults, - BackgroundService $backgroundService) { + private IConfig $config, + private IUserSession $userSession, + private ThemesService $themesService, + private ThemingDefaults $themingDefaults, + private BackgroundService $backgroundService, + ) { parent::__construct($appName, $request); - $this->config = $config; - $this->userSession = $userSession; - $this->themesService = $themesService; - $this->themingDefaults = $themingDefaults; - $this->backgroundService = $backgroundService; - - $user = $userSession->getUser(); - if ($user !== null) { - $this->userId = $user->getUID(); - } + + $this->userId = $userSession->getUser()?->getUID(); } /** diff --git a/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php b/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php index d845c9a1091c4..52d1211db4786 100644 --- a/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php +++ b/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php @@ -41,25 +41,13 @@ /** @template-implements IEventListener */ class BeforeTemplateRenderedListener implements IEventListener { - - private IInitialState $initialState; - private ContainerInterface $container; - private ThemeInjectionService $themeInjectionService; - private IUserSession $userSession; - private IConfig $config; - public function __construct( - IInitialState $initialState, - ContainerInterface $container, - ThemeInjectionService $themeInjectionService, - IUserSession $userSession, - IConfig $config + private IInitialState $initialState, + private ContainerInterface $container, + private ThemeInjectionService $themeInjectionService, + private IUserSession $userSession, + private IConfig $config, ) { - $this->initialState = $initialState; - $this->container = $container; - $this->themeInjectionService = $themeInjectionService; - $this->userSession = $userSession; - $this->config = $config; } public function handle(Event $event): void {