Skip to content

Commit

Permalink
Improves readability mainly by utilizing PHP8's constructor property …
Browse files Browse the repository at this point in the history 
…promotion feature.

Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
  • Loading branch information
@fsamapoor
fsamapoor committed May 1, 2024
1 parent d82fb01 commit 64d68b9
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 112 deletions.
32 changes: 14 additions & 18 deletions apps/theming/lib/Command/UpdateConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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) {
Expand All @@ -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') {
Expand All @@ -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;
}
}
27 changes: 5 additions & 22 deletions apps/theming/lib/Controller/IconController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
53 changes: 21 additions & 32 deletions apps/theming/lib/Controller/ThemingController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand Down
33 changes: 10 additions & 23 deletions apps/theming/lib/Controller/UserThemeController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
22 changes: 5 additions & 17 deletions apps/theming/lib/Listener/BeforeTemplateRenderedListener.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 64d68b9

Please sign in to comment.