Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors theming app - part 4 #45146

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 8 additions & 28 deletions apps/theming/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,33 +41,13 @@
* @package OCA\Theming
*/
class Capabilities implements IPublicCapability {

/** @var ThemingDefaults */
protected $theming;

/** @var Util */
protected $util;

/** @var IURLGenerator */
protected $url;

/** @var IConfig */
protected $config;

protected IUserSession $userSession;

/**
* @param ThemingDefaults $theming
* @param Util $util
* @param IURLGenerator $url
* @param IConfig $config
*/
public function __construct(ThemingDefaults $theming, Util $util, IURLGenerator $url, IConfig $config, IUserSession $userSession) {
$this->theming = $theming;
$this->util = $util;
$this->url = $url;
$this->config = $config;
$this->userSession = $userSession;
public function __construct(
protected ThemingDefaults $theming,
protected Util $util,
protected IURLGenerator $url,
protected IConfig $config,
protected IUserSession $userSession,
) {
}

/**
Expand All @@ -92,7 +72,7 @@ public function __construct(ThemingDefaults $theming, Util $util, IURLGenerator
* },
* }
*/
public function getCapabilities() {
public function getCapabilities(): array {
$color = $this->theming->getDefaultColorPrimary();
// Same as in DefaultTheme
if ($color === BackgroundService::DEFAULT_COLOR) {
Expand Down
1 change: 0 additions & 1 deletion apps/theming/lib/ITheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
* @since 25.0.0
*/
interface ITheme {

public const TYPE_THEME = 1;
public const TYPE_FONT = 2;

Expand Down
44 changes: 10 additions & 34 deletions apps/theming/lib/IconBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,35 +31,18 @@
use OCP\Files\SimpleFS\ISimpleFile;

class IconBuilder {
/** @var ThemingDefaults */
private $themingDefaults;
/** @var Util */
private $util;
/** @var ImageManager */
private $imageManager;

/**
* IconBuilder constructor.
*
* @param ThemingDefaults $themingDefaults
* @param Util $util
* @param ImageManager $imageManager
*/
public function __construct(
ThemingDefaults $themingDefaults,
Util $util,
ImageManager $imageManager
private ThemingDefaults $themingDefaults,
private Util $util,
private ImageManager $imageManager,
) {
$this->themingDefaults = $themingDefaults;
$this->util = $util;
$this->imageManager = $imageManager;
}

/**
* @param $app string app name
* @return string|false image blob
*/
public function getFavicon($app) {
public function getFavicon($app): bool|string {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $app has no provided type
if (!$this->imageManager->shouldReplaceIcons()) {
return false;
}
Expand Down Expand Up @@ -102,7 +85,7 @@
* @param $app string app name
* @return string|false image blob
*/
public function getTouchIcon($app) {
public function getTouchIcon($app): bool|string {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $app has no provided type
try {
$icon = $this->renderAppIcon($app, 512);
if ($icon === false) {
Expand All @@ -125,16 +108,13 @@
* @param $size int size of the icon in px
* @return Imagick|false
*/
public function renderAppIcon($app, $size) {
public function renderAppIcon($app, $size): Imagick|bool {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $app has no provided type

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $size has no provided type
$appIcon = $this->util->getAppIcon($app);
if ($appIcon === false) {
return false;
}
if ($appIcon instanceof ISimpleFile) {
$appIconContent = $appIcon->getContent();
$mime = $appIcon->getMimeType();
} else {
$appIconContent = file_get_contents($appIcon);

Check failure on line 117 in apps/theming/lib/IconBuilder.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedFile

apps/theming/lib/IconBuilder.php:117:40: TaintedFile: Detected tainted file handling (see https://psalm.dev/255)
$mime = mime_content_type($appIcon);
}

Expand All @@ -150,8 +130,8 @@
'<rect x="0" y="0" rx="100" ry="100" width="512" height="512" style="fill:' . $color . ';" />' .
'</svg>';
// resize svg magic as this seems broken in Imagemagick
if ($mime === "image/svg+xml" || substr($appIconContent, 0, 4) === "<svg") {
if (substr($appIconContent, 0, 5) !== "<?xml") {
if ($mime === "image/svg+xml" || str_starts_with($appIconContent, "<svg")) {
if (!str_starts_with($appIconContent, "<?xml")) {
$svg = "<?xml version=\"1.0\"?>".$appIconContent;
} else {
$svg = $appIconContent;
Expand All @@ -163,11 +143,7 @@
$res = $tmp->getImageResolution();
$tmp->destroy();

if ($x > $y) {
$max = $x;
} else {
$max = $y;
}
$max = max($x, $y);

// convert svg to resized image
$appIconFile = new Imagick();
Expand Down Expand Up @@ -227,12 +203,12 @@
* @param $image string relative path to svg file in app directory
* @return string|false content of a colorized svg file
*/
public function colorSvg($app, $image) {
public function colorSvg($app, $image): bool|string {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $app has no provided type

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $image has no provided type
$imageFile = $this->util->getAppImage($app, $image);
if ($imageFile === false || $imageFile === "") {
return false;
}
$svg = file_get_contents($imageFile);

Check failure on line 211 in apps/theming/lib/IconBuilder.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedFile

apps/theming/lib/IconBuilder.php:211:28: TaintedFile: Detected tainted file handling (see https://psalm.dev/255)
if ($svg !== false && $svg !== "") {
$color = $this->util->elementColor($this->themingDefaults->getColorPrimary());
$svg = $this->util->colorizeSvg($svg, $color);
Expand Down
23 changes: 7 additions & 16 deletions apps/theming/lib/ImageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,11 @@ public function getImageUrl(string $key): string {
return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter;
}

switch ($key) {
case 'logo':
case 'logoheader':
case 'favicon':
return $this->urlGenerator->imagePath('core', 'logo/logo.png') . '?v=' . $cacheBusterCounter;
case 'background':
return $this->urlGenerator->linkTo(Application::APP_ID, 'img/background/' . BackgroundService::DEFAULT_BACKGROUND_IMAGE);
}
return '';
return match ($key) {
'logo', 'logoheader', 'favicon' => $this->urlGenerator->imagePath('core', 'logo/logo.png') . '?v=' . $cacheBusterCounter,
'background' => $this->urlGenerator->linkTo(Application::APP_ID, 'img/background/' . BackgroundService::DEFAULT_BACKGROUND_IMAGE),
default => '',
};
}

/**
Expand Down Expand Up @@ -166,8 +162,8 @@ public function getCacheFolder(): ISimpleFolder {
* Get a file from AppData
*
* @param string $filename
* @return ISimpleFile
* @throws NotFoundException
* @return \OCP\Files\SimpleFS\ISimpleFile
* @throws NotPermittedException
*/
public function getCachedImage(string $filename): ISimpleFile {
Expand All @@ -178,9 +174,6 @@ public function getCachedImage(string $filename): ISimpleFile {
/**
* Store a file for theming in AppData
*
* @param string $filename
* @param string $data
* @return \OCP\Files\SimpleFS\ISimpleFile
* @throws NotFoundException
* @throws NotPermittedException
*/
Expand Down Expand Up @@ -342,10 +335,8 @@ public function cleanup() {
/**
* Check if Imagemagick is enabled and if SVG is supported
* otherwise we can't render custom icons
*
* @return bool
*/
public function shouldReplaceIcons() {
public function shouldReplaceIcons(): bool {
$cache = $this->cacheFactory->createDistributed('theming-' . $this->urlGenerator->getBaseUrl());
if ($value = $cache->get('shouldReplaceIcons')) {
return (bool)$value;
Expand Down
1 change: 0 additions & 1 deletion apps/theming/lib/Themes/DarkHighContrastTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use OCA\Theming\ITheme;

class DarkHighContrastTheme extends DarkTheme implements ITheme {

public function getId(): string {
return 'dark-highcontrast';
}
Expand Down
1 change: 0 additions & 1 deletion apps/theming/lib/Themes/DarkTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use OCA\Theming\ITheme;

class DarkTheme extends DefaultTheme implements ITheme {

public function getId(): string {
return 'dark';
}
Expand Down
36 changes: 10 additions & 26 deletions apps/theming/lib/Themes/DefaultTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,35 +39,19 @@
class DefaultTheme implements ITheme {
use CommonThemeTrait;

public Util $util;
public ThemingDefaults $themingDefaults;
public IUserSession $userSession;
public IURLGenerator $urlGenerator;
public ImageManager $imageManager;
public IConfig $config;
public IL10N $l;
public IAppManager $appManager;

public string $defaultPrimaryColor;
public string $primaryColor;

public function __construct(Util $util,
ThemingDefaults $themingDefaults,
IUserSession $userSession,
IURLGenerator $urlGenerator,
ImageManager $imageManager,
IConfig $config,
IL10N $l,
IAppManager $appManager) {
$this->util = $util;
$this->themingDefaults = $themingDefaults;
$this->userSession = $userSession;
$this->urlGenerator = $urlGenerator;
$this->imageManager = $imageManager;
$this->config = $config;
$this->l = $l;
$this->appManager = $appManager;

public function __construct(
public Util $util,
public ThemingDefaults $themingDefaults,
public IUserSession $userSession,
public IURLGenerator $urlGenerator,
public ImageManager $imageManager,
public IConfig $config,
public IL10N $l,
public IAppManager $appManager,
) {
$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary();
$this->primaryColor = $this->themingDefaults->getColorPrimary();

Expand Down
3 changes: 1 addition & 2 deletions apps/theming/lib/Themes/DyslexiaFont.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use OCA\Theming\ITheme;

class DyslexiaFont extends DefaultTheme implements ITheme {

public function getId(): string {
return 'opendyslexic';
}
Expand Down Expand Up @@ -77,7 +76,7 @@ public function getCustomCss(): string {
url('$fontPathOtf') format('opentype'),
url('$fontPathTtf') format('truetype');
}

@font-face {
font-family: 'OpenDyslexic';
font-style: normal;
Expand Down
1 change: 0 additions & 1 deletion apps/theming/lib/Themes/HighContrastTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use OCA\Theming\ITheme;

class HighContrastTheme extends DefaultTheme implements ITheme {

public function getId(): string {
return 'light-highcontrast';
}
Expand Down
1 change: 0 additions & 1 deletion apps/theming/lib/Themes/LightTheme.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use OCA\Theming\ITheme;

class LightTheme extends DefaultTheme implements ITheme {

public function getId(): string {
return 'light';
}
Expand Down
Loading
Loading