diff --git a/apps/theming/composer/composer/autoload_classmap.php b/apps/theming/composer/composer/autoload_classmap.php index a68410ce3ce28..9b53c0f9fea27 100644 --- a/apps/theming/composer/composer/autoload_classmap.php +++ b/apps/theming/composer/composer/autoload_classmap.php @@ -17,9 +17,11 @@ 'OCA\\Theming\\IconBuilder' => $baseDir . '/../lib/IconBuilder.php', 'OCA\\Theming\\ImageManager' => $baseDir . '/../lib/ImageManager.php', 'OCA\\Theming\\Jobs\\MigrateBackgroundImages' => $baseDir . '/../lib/Jobs/MigrateBackgroundImages.php', + 'OCA\\Theming\\Jobs\\RestoreBackgroundImageColor' => $baseDir . '/../lib/Jobs/RestoreBackgroundImageColor.php', 'OCA\\Theming\\Listener\\BeforePreferenceListener' => $baseDir . '/../lib/Listener/BeforePreferenceListener.php', 'OCA\\Theming\\Listener\\BeforeTemplateRenderedListener' => $baseDir . '/../lib/Listener/BeforeTemplateRenderedListener.php', 'OCA\\Theming\\Migration\\InitBackgroundImagesMigration' => $baseDir . '/../lib/Migration/InitBackgroundImagesMigration.php', + 'OCA\\Theming\\Migration\\Version2006Date20240905111627' => $baseDir . '/../lib/Migration/Version2006Date20240905111627.php', 'OCA\\Theming\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', 'OCA\\Theming\\Service\\BackgroundService' => $baseDir . '/../lib/Service/BackgroundService.php', 'OCA\\Theming\\Service\\JSDataService' => $baseDir . '/../lib/Service/JSDataService.php', diff --git a/apps/theming/composer/composer/autoload_static.php b/apps/theming/composer/composer/autoload_static.php index 17b195827f970..184d9ed07615b 100644 --- a/apps/theming/composer/composer/autoload_static.php +++ b/apps/theming/composer/composer/autoload_static.php @@ -32,9 +32,11 @@ class ComposerStaticInitTheming 'OCA\\Theming\\IconBuilder' => __DIR__ . '/..' . '/../lib/IconBuilder.php', 'OCA\\Theming\\ImageManager' => __DIR__ . '/..' . '/../lib/ImageManager.php', 'OCA\\Theming\\Jobs\\MigrateBackgroundImages' => __DIR__ . '/..' . '/../lib/Jobs/MigrateBackgroundImages.php', + 'OCA\\Theming\\Jobs\\RestoreBackgroundImageColor' => __DIR__ . '/..' . '/../lib/Jobs/RestoreBackgroundImageColor.php', 'OCA\\Theming\\Listener\\BeforePreferenceListener' => __DIR__ . '/..' . '/../lib/Listener/BeforePreferenceListener.php', 'OCA\\Theming\\Listener\\BeforeTemplateRenderedListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeTemplateRenderedListener.php', 'OCA\\Theming\\Migration\\InitBackgroundImagesMigration' => __DIR__ . '/..' . '/../lib/Migration/InitBackgroundImagesMigration.php', + 'OCA\\Theming\\Migration\\Version2006Date20240905111627' => __DIR__ . '/..' . '/../lib/Migration/Version2006Date20240905111627.php', 'OCA\\Theming\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', 'OCA\\Theming\\Service\\BackgroundService' => __DIR__ . '/..' . '/../lib/Service/BackgroundService.php', 'OCA\\Theming\\Service\\JSDataService' => __DIR__ . '/..' . '/../lib/Service/JSDataService.php', diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 73ef78c7a8e8c..d6c89990b0284 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -22,6 +22,7 @@ use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\NotFoundResponse; +use OCP\AppFramework\Services\IAppConfig; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\IConfig; @@ -40,34 +41,19 @@ class ThemingController extends Controller { public const VALID_UPLOAD_KEYS = ['header', 'logo', 'logoheader', 'background', 'favicon']; - private ThemingDefaults $themingDefaults; - private IL10N $l10n; - private IConfig $config; - private IURLGenerator $urlGenerator; - private IAppManager $appManager; - private ImageManager $imageManager; - private ThemesService $themesService; - public function __construct( $appName, IRequest $request, - IConfig $config, - ThemingDefaults $themingDefaults, - IL10N $l, - IURLGenerator $urlGenerator, - IAppManager $appManager, - ImageManager $imageManager, - ThemesService $themesService + private IConfig $config, + private IAppConfig $appConfig, + private ThemingDefaults $themingDefaults, + private IL10N $l10n, + 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->urlGenerator = $urlGenerator; - $this->appManager = $appManager; - $this->imageManager = $imageManager; - $this->themesService = $themesService; } /** @@ -80,6 +66,7 @@ public function __construct( public function updateStylesheet($setting, $value) { $value = trim($value); $error = null; + $saved = false; switch ($setting) { case 'name': if (strlen($value) > 250) { @@ -118,16 +105,25 @@ public function updateStylesheet($setting, $value) { case 'primary_color': if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) { $error = $this->l10n->t('The given color is invalid'); + } else { + $this->appConfig->setAppValueString('primary_color', $value); + $saved = true; } break; case 'background_color': if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) { $error = $this->l10n->t('The given color is invalid'); + } else { + $this->appConfig->setAppValueString('background_color', $value); + $saved = true; } break; case 'disable-user-theming': - if ($value !== 'yes' && $value !== 'no') { + if (!in_array($value, ['yes', 'true', 'no', 'false'])) { $error = $this->l10n->t('Disable-user-theming should be true or false'); + } else { + $this->appConfig->setAppValueBool('disable-user-theming', $value === 'yes' || $value === 'true'); + $saved = true; } break; } @@ -140,7 +136,9 @@ public function updateStylesheet($setting, $value) { ], Http::STATUS_BAD_REQUEST); } - $this->themingDefaults->set($setting, $value); + if (!$saved) { + $this->themingDefaults->set($setting, $value); + } return new DataResponse([ 'data' => [ diff --git a/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php b/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php new file mode 100644 index 0000000000000..1ec659d0646df --- /dev/null +++ b/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php @@ -0,0 +1,205 @@ +runPreparation(); + break; + case self::STAGE_EXECUTE: + $this->runMigration(); + break; + default: + break; + } + } + + protected function runPreparation(): void { + try { + $qb = $this->dbc->getQueryBuilder(); + $qb2 = $this->dbc->getQueryBuilder(); + + $innerSQL = $qb2->select('userid') + ->from('preferences') + ->where($qb2->expr()->eq('configkey', $qb->createNamedParameter('background_color'))); + + // Get those users, that have a background_image set - not the default, but no background_color. + $result = $qb->selectDistinct('a.userid') + ->from('preferences', 'a') + ->leftJoin('a', $qb->createFunction('('.$innerSQL->getSQL().')'), 'b', 'a.userid = b.userid') + ->where($qb2->expr()->eq('a.configkey', $qb->createNamedParameter('background_image'))) + ->andWhere($qb2->expr()->neq('a.configvalue', $qb->createNamedParameter(BackgroundService::BACKGROUND_DEFAULT))) + ->andWhere($qb2->expr()->isNull('b.userid')) + ->executeQuery(); + + $userIds = $result->fetchAll(\PDO::FETCH_COLUMN); + $this->logger->info('Prepare to restore background information for {users} users', ['users' => count($userIds)]); + $this->storeUserIdsToProcess($userIds); + } catch (\Throwable $t) { + $this->jobList->add(self::class, ['stage' => self::STAGE_PREPARE]); + throw $t; + } + $this->jobList->add(self::class, ['stage' => self::STAGE_EXECUTE]); + } + + /** + * @throws NotPermittedException + * @throws NotFoundException + */ + protected function runMigration(): void { + $allUserIds = $this->readUserIdsToProcess(); + $notSoFastMode = count($allUserIds) > 1000; + + $userIds = array_slice($allUserIds, 0, 1000); + foreach ($userIds as $userId) { + $backgroundColor = $this->config->getUserValue($userId, Application::APP_ID, 'background_color'); + if ($backgroundColor !== '') { + continue; + } + + $background = $this->config->getUserValue($userId, Application::APP_ID, 'background_image'); + switch($background) { + case BackgroundService::BACKGROUND_DEFAULT: + $this->service->setDefaultBackground($userId); + break; + case BackgroundService::BACKGROUND_COLOR: + break; + case BackgroundService::BACKGROUND_CUSTOM: + $this->service->recalculateMeanColor($userId); + break; + default: + // shipped backgrounds + // do not alter primary color + $primary = $this->config->getUserValue($userId, Application::APP_ID, 'primary_color'); + if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$background])) { + $this->service->setShippedBackground($background, $userId); + } else { + $this->service->setDefaultBackground($userId); + } + // Restore primary + if ($primary !== '') { + $this->config->setUserValue($userId, Application::APP_ID, 'primary_color', $primary); + } + } + } + + if ($notSoFastMode) { + $remainingUserIds = array_slice($allUserIds, 1000); + $this->storeUserIdsToProcess($remainingUserIds); + $this->jobList->add(self::class, ['stage' => self::STAGE_EXECUTE]); + } else { + $this->deleteStateFile(); + } + } + + /** + * @throws NotPermittedException + * @throws NotFoundException + */ + protected function readUserIdsToProcess(): array { + $globalFolder = $this->appData->getFolder('global'); + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + try { + $userIds = \json_decode($file->getContent(), true); + } catch (NotFoundException $e) { + $userIds = []; + } + if ($userIds === null) { + $userIds = []; + } + } else { + $userIds = []; + } + return $userIds; + } + + /** + * @throws NotFoundException + */ + protected function storeUserIdsToProcess(array $userIds): void { + $storableUserIds = \json_encode($userIds); + $globalFolder = $this->appData->getFolder('global'); + try { + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + } else { + $file = $globalFolder->newFile(self::STATE_FILE_NAME); + } + $file->putContent($storableUserIds); + } catch (NotFoundException $e) { + } catch (NotPermittedException $e) { + $this->logger->warning('Lacking permissions to create {file}', + [ + 'app' => 'theming', + 'file' => self::STATE_FILE_NAME, + 'exception' => $e, + ] + ); + } + } + + /** + * @throws NotFoundException + */ + protected function deleteStateFile(): void { + $globalFolder = $this->appData->getFolder('global'); + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + try { + $file->delete(); + } catch (NotPermittedException $e) { + $this->logger->info('Could not delete {file} due to permissions. It is safe to delete manually inside data -> appdata -> theming -> global.', + [ + 'app' => 'theming', + 'file' => $file->getName(), + 'exception' => $e, + ] + ); + } + } + } +} diff --git a/apps/theming/lib/Migration/Version2006Date20240905111627.php b/apps/theming/lib/Migration/Version2006Date20240905111627.php new file mode 100644 index 0000000000000..dcec954f585a0 --- /dev/null +++ b/apps/theming/lib/Migration/Version2006Date20240905111627.php @@ -0,0 +1,88 @@ + Nextcloud 29 -> 30 +class Version2006Date20240905111627 implements \OCP\Migration\IMigrationStep { + + public function __construct( + private IJobList $jobList, + private IAppConfig $appConfig, + private IDBConnection $connection, + ) { + } + + public function name(): string { + return 'Restore custom primary color'; + } + + public function description(): string { + return 'Restore custom primary color after separating primary color from background color'; + } + + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + // nop + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + $this->restoreSystemColors($output); + + $userThemingEnabled = $this->appConfig->getValueBool('theming', 'disable-user-theming') === false; + if ($userThemingEnabled) { + $this->restoreUserColors($output); + } + + return null; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $output->info('Initialize restoring of background colors for custom background images'); + // This is done in a background job as this can take a lot of time for large instances + $this->jobList->add(RestoreBackgroundImageColor::class, ['stage' => RestoreBackgroundImageColor::STAGE_PREPARE]); + } + + private function restoreSystemColors(IOutput $output): void { + $defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'color', ''); + if ($defaultColor === '') { + $output->info('No custom system color configured - skipping'); + } else { + // Restore legacy value into new field + $this->appConfig->setValueString(Application::APP_ID, 'background_color', $defaultColor); + $this->appConfig->setValueString(Application::APP_ID, 'primary_color', $defaultColor); + // Delete legacy field + $this->appConfig->deleteKey(Application::APP_ID, 'color'); + // give some feedback + $output->info('Global primary color restored'); + } + } + + private function restoreUserColors(IOutput $output): void { + $output->info('Restoring user primary color'); + // For performance let the DB handle this + $qb = $this->connection->getQueryBuilder(); + // Rename the `background_color` config to `primary_color` as this was the behavior on Nextcloud 29 and older + // with Nextcloud 30 `background_color` is a new option to define the background color independent of the primary color. + $qb->update('preferences') + ->set('configkey', $qb->createNamedParameter('primary_color')) + ->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('background_color'))); + $qb->executeStatement(); + $output->info('Primary color of users restored'); + } +} diff --git a/apps/theming/lib/Service/BackgroundService.php b/apps/theming/lib/Service/BackgroundService.php index bd87bc1aa21b0..6cb2b4abeb9df 100644 --- a/apps/theming/lib/Service/BackgroundService.php +++ b/apps/theming/lib/Service/BackgroundService.php @@ -205,10 +205,15 @@ public function __construct( ) { } - public function setDefaultBackground(): void { - $this->config->deleteUserValue($this->userId, Application::APP_ID, 'background_image'); - $this->config->deleteUserValue($this->userId, Application::APP_ID, 'background_color'); - $this->config->deleteUserValue($this->userId, Application::APP_ID, 'primary_color'); + public function setDefaultBackground(?string $userId = null): void { + $userId = $userId ?? $this->userId; + if ($userId === null) { + throw new RuntimeException('No currently logged-in user'); + } + + $this->config->deleteUserValue($userId, Application::APP_ID, 'background_image'); + $this->config->deleteUserValue($userId, Application::APP_ID, 'background_color'); + $this->config->deleteUserValue($userId, Application::APP_ID, 'primary_color'); } /** @@ -227,9 +232,24 @@ public function setFileBackground($path): void { /** @var File $file */ $file = $userFolder->get($path); - $image = new \OCP\Image(); + $handle = $file->fopen('r'); + if ($handle === false) { + throw new InvalidArgumentException('Invalid image file'); + } + $this->getAppDataFolder()->newFile('background.jpg', $handle); + + $this->recalculateMeanColor(); + } + + public function recalculateMeanColor(?string $userId = null): void { + $userId = $userId ?? $this->userId; + if ($userId === null) { + throw new RuntimeException('No currently logged-in user'); + } - if ($image->loadFromFileHandle($file->fopen('r')) === false) { + $image = new \OCP\Image(); + $handle = $this->getAppDataFolder($userId)->getFile('background.jpg')->read(); + if ($handle === false || $image->loadFromFileHandle($handle) === false) { throw new InvalidArgumentException('Invalid image file'); } @@ -237,35 +257,43 @@ public function setFileBackground($path): void { if ($meanColor !== false) { $this->setColorBackground($meanColor); } - - $this->getAppDataFolder()->newFile('background.jpg', $file->fopen('r')); - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_image', self::BACKGROUND_CUSTOM); + $this->config->setUserValue($userId, Application::APP_ID, 'background_image', self::BACKGROUND_CUSTOM); } - public function setShippedBackground($fileName): void { - if ($this->userId === null) { + /** + * Set background of user to a shipped background identified by the filename + * @param string $filename The shipped background filename + * @param null|string $userId The user to set - defaults to currently logged in user + * @throws RuntimeException If neither $userId is specified nor a user is logged in + * @throws InvalidArgumentException If the specified filename does not match any shipped background + */ + public function setShippedBackground(string $filename, ?string $userId = null): void { + $userId = $userId ?? $this->userId; + if ($userId === null) { throw new RuntimeException('No currently logged-in user'); } - if (!array_key_exists($fileName, self::SHIPPED_BACKGROUNDS)) { + if (!array_key_exists($filename, self::SHIPPED_BACKGROUNDS)) { throw new InvalidArgumentException('The given file name is invalid'); } - $this->setColorBackground(self::SHIPPED_BACKGROUNDS[$fileName]['background_color']); - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_image', $fileName); - $this->config->setUserValue($this->userId, Application::APP_ID, 'primary_color', self::SHIPPED_BACKGROUNDS[$fileName]['primary_color']); + $this->setColorBackground(self::SHIPPED_BACKGROUNDS[$filename]['background_color'], $userId); + $this->config->setUserValue($userId, Application::APP_ID, 'background_image', $filename); + $this->config->setUserValue($userId, Application::APP_ID, 'primary_color', self::SHIPPED_BACKGROUNDS[$filename]['primary_color']); } /** * Set the background to color only + * @param string|null $userId The user to set the color - default to current logged-in user */ - public function setColorBackground(string $color): void { - if ($this->userId === null) { + public function setColorBackground(string $color, ?string $userId = null): void { + $userId = $userId ?? $this->userId; + if ($userId === null) { throw new RuntimeException('No currently logged-in user'); } if (!preg_match('/^#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) { throw new InvalidArgumentException('The given color is invalid'); } - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_color', $color); - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_image', self::BACKGROUND_COLOR); + $this->config->setUserValue($userId, Application::APP_ID, 'background_color', $color); + $this->config->setUserValue($userId, Application::APP_ID, 'background_image', self::BACKGROUND_COLOR); } public function deleteBackgroundImage(): void { @@ -366,19 +394,24 @@ function toHex(int $channel): string { /** * Storing the data in appdata/theming/users/USERID * - * @return ISimpleFolder + * @param string|null $userId The user to get the folder - default to current user * @throws NotPermittedException */ - private function getAppDataFolder(): ISimpleFolder { + private function getAppDataFolder(?string $userId = null): ISimpleFolder { + $userId = $userId ?? $this->userId; + if ($userId === null) { + throw new RuntimeException('No currently logged-in user'); + } + try { $rootFolder = $this->appData->getFolder('users'); } catch (NotFoundException $e) { $rootFolder = $this->appData->newFolder('users'); } try { - return $rootFolder->getFolder($this->userId); + return $rootFolder->getFolder($userId); } catch (NotFoundException $e) { - return $rootFolder->newFolder($this->userId); + return $rootFolder->newFolder($userId); } } } diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index f43e96a883051..9be830a61fda9 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -11,6 +11,7 @@ use OCP\App\IAppManager; use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; +use OCP\IAppConfig; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IL10N; @@ -39,6 +40,7 @@ class ThemingDefaults extends \OC_Defaults { */ public function __construct( private IConfig $config, + private IAppConfig $appConfig, private IL10N $l, private IUserSession $userSession, private IURLGenerator $urlGenerator, @@ -206,9 +208,9 @@ public function getColorBackground(): string { // user-defined background color if (!empty($user)) { - $userPrimaryColor = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_color', ''); - if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $userPrimaryColor)) { - return $userPrimaryColor; + $userBackgroundColor = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_color', ''); + if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $userBackgroundColor)) { + return $userBackgroundColor; } } @@ -221,7 +223,7 @@ public function getColorBackground(): string { */ public function getDefaultColorPrimary(): string { // try admin color - $defaultColor = $this->config->getAppValue(Application::APP_ID, 'primary_color', ''); + $defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'primary_color', ''); if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) { return $defaultColor; } @@ -234,7 +236,7 @@ public function getDefaultColorPrimary(): string { * Default background color only taking admin setting into account */ public function getDefaultColorBackground(): string { - $defaultColor = $this->config->getAppValue(Application::APP_ID, 'background_color', ''); + $defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'background_color'); if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) { return $defaultColor; } @@ -344,7 +346,7 @@ public function getScssVariables() { $variables['image-login-background'] = "url('".$this->imageManager->getImageUrl('background')."')"; $variables['image-login-plain'] = 'false'; - if ($this->config->getAppValue('theming', 'primary_color', '') !== '') { + if ($this->appConfig->getValueString(Application::APP_ID, 'primary_color', '') !== '') { $variables['color-primary'] = $this->getColorPrimary(); $variables['color-primary-text'] = $this->getTextColorPrimary(); $variables['color-primary-element'] = $this->util->elementColor($this->getColorPrimary()); @@ -520,6 +522,6 @@ public function getDefaultTextColorPrimary() { * Has the admin disabled user customization */ public function isUserThemingDisabled(): bool { - return $this->config->getAppValue('theming', 'disable-user-theming', 'no') === 'yes'; + return $this->appConfig->getValueBool(Application::APP_ID, 'disable-user-theming'); } } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 86231e7daf7ec..280bb8df4d173 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -13,6 +13,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Services\IAppConfig; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; @@ -24,28 +25,23 @@ use Test\TestCase; class ThemingControllerTest extends TestCase { - /** @var IRequest|MockObject */ - private $request; - /** @var IConfig|MockObject */ - private $config; - /** @var ThemingDefaults|MockObject */ - private $themingDefaults; - /** @var IL10N|MockObject */ - private $l10n; - /** @var ThemingController */ - private $themingController; - /** @var IAppManager|MockObject */ - private $appManager; - /** @var ImageManager|MockObject */ - private $imageManager; - /** @var IURLGenerator|MockObject */ - private $urlGenerator; - /** @var ThemesService|MockObject */ - private $themesService; + + private IRequest&MockObject $request; + private IConfig&MockObject $config; + private IAppConfig&MockObject $appConfig; + private ThemingDefaults&MockObject $themingDefaults; + private IL10N&MockObject $l10n; + private IAppManager&MockObject $appManager; + private ImageManager&MockObject $imageManager; + private IURLGenerator&MockObject $urlGenerator; + private ThemesService&MockObject $themesService; + + private ThemingController $themingController; protected function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->config = $this->createMock(IConfig::class); + $this->appConfig = $this->createMock(IAppConfig::class); $this->themingDefaults = $this->createMock(ThemingDefaults::class); $this->l10n = $this->createMock(L10N::class); $this->appManager = $this->createMock(IAppManager::class); @@ -64,6 +60,7 @@ protected function setUp(): void { 'theming', $this->request, $this->config, + $this->appConfig, $this->themingDefaults, $this->l10n, $this->urlGenerator, diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php index 0dbf4336e8ae3..8edbcbc53c1b5 100644 --- a/apps/theming/tests/ThemingDefaultsTest.php +++ b/apps/theming/tests/ThemingDefaultsTest.php @@ -10,8 +10,8 @@ use OCA\Theming\ThemingDefaults; use OCA\Theming\Util; use OCP\App\IAppManager; -use OCP\Files\IAppData; use OCP\Files\NotFoundException; +use OCP\IAppConfig; use OCP\ICache; use OCP\ICacheFactory; use OCP\IConfig; @@ -20,21 +20,20 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ThemingDefaultsTest extends TestCase { - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ - private $config; + private IAppConfig&MockObject $appConfig; + private IConfig&MockObject $config; + private \OC_Defaults $defaults; + /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ private $l10n; /** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */ private $userSession; /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ private $urlGenerator; - /** @var \OC_Defaults|\PHPUnit\Framework\MockObject\MockObject */ - private $defaults; - /** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */ - private $appData; /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ private $cacheFactory; /** @var ThemingDefaults */ @@ -54,6 +53,7 @@ class ThemingDefaultsTest extends TestCase { protected function setUp(): void { parent::setUp(); + $this->appConfig = $this->createMock(IAppConfig::class); $this->config = $this->createMock(IConfig::class); $this->l10n = $this->createMock(IL10N::class); $this->userSession = $this->createMock(IUserSession::class); @@ -72,6 +72,7 @@ protected function setUp(): void { ->willReturn(''); $this->template = new ThemingDefaults( $this->config, + $this->appConfig, $this->l10n, $this->userSession, $this->urlGenerator, @@ -398,25 +399,31 @@ public function testGetShortFooterInvalidPrivacy($invalidPrivacyUrl) { } public function testGetColorPrimaryWithDefault() { - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['theming', 'disable-user-theming', 'no', 'no'], - ['theming', 'primary_color', '', $this->defaults->getColorPrimary()], - ]); + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('theming', 'disable-user-theming') + ->willReturn(false); + $this->appConfig + ->expects(self::once()) + ->method('getValueString') + ->with('theming', 'primary_color', '') + ->willReturn($this->defaults->getColorPrimary()); $this->assertEquals($this->defaults->getColorPrimary(), $this->template->getColorPrimary()); } public function testGetColorPrimaryWithCustom() { - $this->config - ->expects($this->exactly(2)) - ->method('getAppValue') - ->willReturnMap([ - ['theming', 'disable-user-theming', 'no', 'no'], - ['theming', 'primary_color', '', '#fff'], - ]); + $this->appConfig + ->expects(self::once()) + ->method('getValueBool') + ->with('theming', 'disable-user-theming') + ->willReturn(false); + $this->appConfig + ->expects(self::once()) + ->method('getValueString') + ->with('theming', 'primary_color', '') + ->willReturn('#fff'); $this->assertEquals('#fff', $this->template->getColorPrimary()); } @@ -424,37 +431,37 @@ public function testGetColorPrimaryWithCustom() { public function dataGetColorPrimary() { return [ 'with fallback default' => [ - 'disableTheming' => 'no', + 'disableTheming' => false, 'primaryColor' => '', 'userPrimaryColor' => '', 'expected' => BackgroundService::DEFAULT_COLOR, ], 'with custom admin primary' => [ - 'disableTheming' => 'no', + 'disableTheming' => false, 'primaryColor' => '#aaa', 'userPrimaryColor' => '', 'expected' => '#aaa', ], 'with custom invalid admin primary' => [ - 'disableTheming' => 'no', + 'disableTheming' => false, 'primaryColor' => 'invalid', 'userPrimaryColor' => '', 'expected' => BackgroundService::DEFAULT_COLOR, ], 'with custom invalid user primary' => [ - 'disableTheming' => 'no', + 'disableTheming' => false, 'primaryColor' => '', 'userPrimaryColor' => 'invalid-name', 'expected' => BackgroundService::DEFAULT_COLOR, ], 'with custom user primary' => [ - 'disableTheming' => 'no', + 'disableTheming' => false, 'primaryColor' => '', 'userPrimaryColor' => '#bbb', 'expected' => '#bbb', ], 'with disabled user theming primary' => [ - 'disableTheming' => 'yes', + 'disableTheming' => true, 'primaryColor' => '#aaa', 'userPrimaryColor' => '#bbb', 'expected' => '#aaa', @@ -465,7 +472,7 @@ public function dataGetColorPrimary() { /** * @dataProvider dataGetColorPrimary */ - public function testGetColorPrimary(string $disableTheming, string $primaryColor, string $userPrimaryColor, string $expected) { + public function testGetColorPrimary(bool $disableTheming, string $primaryColor, string $userPrimaryColor, string $expected) { $user = $this->createMock(IUser::class); $this->userSession->expects($this->any()) ->method('getUser') @@ -473,13 +480,16 @@ public function testGetColorPrimary(string $disableTheming, string $primaryColor $user->expects($this->any()) ->method('getUID') ->willReturn('user'); - $this->config - ->expects($this->any()) - ->method('getAppValue') - ->willReturnMap([ - ['theming', 'disable-user-theming', 'no', $disableTheming], - ['theming', 'primary_color', '', $primaryColor], - ]); + $this->appConfig + ->expects(self::any()) + ->method('getValueBool') + ->with('theming', 'disable-user-theming') + ->willReturn($disableTheming); + $this->appConfig + ->expects(self::any()) + ->method('getValueString') + ->with('theming', 'primary_color', '') + ->willReturn($primaryColor); $this->config ->expects($this->any()) ->method('getUserValue') @@ -699,8 +709,14 @@ public function testGetScssVariables() { ['theming', 'backgroundMime', '', 'jpeg'], ['theming', 'logoheaderMime', '', 'jpeg'], ['theming', 'faviconMime', '', 'jpeg'], - ['theming', 'primary_color', '', $this->defaults->getColorPrimary()], - ['theming', 'primary_color', $this->defaults->getColorPrimary(), $this->defaults->getColorPrimary()], + ]); + + $this->appConfig + ->expects(self::atLeastOnce()) + ->method('getValueString') + ->willReturnMap([ + ['theming', 'primary_color', '', false, $this->defaults->getColorPrimary()], + ['theming', 'primary_color', $this->defaults->getColorPrimary(), false, $this->defaults->getColorPrimary()], ]); $this->util->expects($this->any())->method('invertTextColor')->with($this->defaults->getColorPrimary())->willReturn(false); diff --git a/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php b/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php index 175b7c1a42eb7..9d351c1d3f17e 100644 --- a/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php @@ -185,7 +185,7 @@ public function innerJoin($fromAlias, $join, $alias, $condition = null): self { } public function leftJoin($fromAlias, $join, $alias, $condition = null): self { - return $this->join($fromAlias, $join, $alias, $condition, PartitionQuery::JOIN_MODE_LEFT); + return $this->join($fromAlias, (string)$join, $alias, $condition, PartitionQuery::JOIN_MODE_LEFT); } public function join($fromAlias, $join, $alias, $condition = null, $joinMode = PartitionQuery::JOIN_MODE_INNER): self { diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index ec2581d7e9a8b..d1207ebdb97ad 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -764,7 +764,7 @@ public function innerJoin($fromAlias, $join, $alias, $condition = null) { * * * @param string $fromAlias The alias that points to a from clause. - * @param string $join The table name to join. + * @param string|IQueryFunction $join The table name or sub-query to join. * @param string $alias The alias of the join table. * @param string|ICompositeExpression|null $condition The condition for the join. * diff --git a/lib/private/Server.php b/lib/private/Server.php index ae2355f40c0ee..a6f7cc61faa30 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1167,6 +1167,7 @@ public function __construct($webRoot, \OC\Config $config) { ); return new ThemingDefaults( $c->get(\OCP\IConfig::class), + $c->get(\OCP\IAppConfig::class), $c->getL10N('theming'), $c->get(IUserSession::class), $c->get(IURLGenerator::class), diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index b673c5ef6ec77..0c2aa4669c1ce 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -541,12 +541,13 @@ public function innerJoin($fromAlias, $join, $alias, $condition = null); * * * @param string $fromAlias The alias that points to a from clause. - * @param string $join The table name to join. + * @param string|IQueryFunction $join The table name to join. * @param string $alias The alias of the join table. * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0 + * @since 30.0.0 Allow passing IQueryFunction as parameter for `$join` to allow join with a sub-query. * * @psalm-taint-sink sql $fromAlias * @psalm-taint-sink sql $join @@ -1001,11 +1002,14 @@ public function createFunction($call); public function getLastInsertId(): int; /** - * Returns the table name quoted and with database prefix as needed by the implementation + * Returns the table name quoted and with database prefix as needed by the implementation. + * If a query function is passed the function is casted to string, + * this allows passing functions as sub-queries for join expression. * * @param string|IQueryFunction $table * @return string * @since 9.0.0 + * @since 24.0.0 accepts IQueryFunction as parameter */ public function getTableName($table);