Refactors theming app - part 1

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('<error>Invalid config key provided</error>');
-			return 1;
+			return self::FAILURE;
 		}
 
 		if ($input->getOption('reset')) {
 			$defaultValue = $this->themingDefaults->undo($key);
 			$output->writeln('<info>Reset ' . $key . ' to ' . $defaultValue . '</info>');
-			return 0;
+			return self::SUCCESS;
 		}
 
 		if ($value === null) {
@@ -108,7 +104,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			} else {
 				$output->writeln('<info>' . $key . ' is currently not set</info>');
 			}
-			return 0;
+			return self::SUCCESS;
 		}
 
 		if ($key === 'background' && $value === 'backgroundColor') {
@@ -119,24 +115,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		if (in_array($key, ImageManager::SUPPORTED_IMAGE_KEYS, true)) {
 			if (!str_starts_with($value, '/')) {
 				$output->writeln('<error>The image file needs to be provided as an absolute path: ' . $value . '.</error>');
-				return 1;
+				return self::FAILURE;
 			}
 			if (!file_exists($value)) {
 				$output->writeln('<error>File could not be found: ' . $value . '.</error>');
-				return 1;
+				return self::FAILURE;
 			}
 			$value = $this->imageManager->updateImage($key, $value);
 			$key = $key . 'Mime';
 		}
 
 		if ($key === 'color' && !preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
 			$output->writeln('<error>The given color is invalid: ' . $value . '</error>');
-			return 1;
+			return self::FAILURE;
 		}
 
 		$this->themingDefaults->set($key, $value);
 		$output->writeln('<info>Updated ' . $key . ' to ' . $value . '</info>');
 
-		return 0;
+		return self::SUCCESS;
 	}
 }

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;
 	}
 
 	/**

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 {

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();
 	}
 
 	/**

apps/theming/lib/Listener/BeforeTemplateRenderedListener.php

@@ -41,25 +41,13 @@
 
 /** @template-implements IEventListener<BeforeTemplateRenderedEvent|BeforeLoginTemplateRenderedEvent> */
 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 {