From c5792f698adeb871a626250886bc6b01b7753e06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Wed, 23 Sep 2020 17:48:48 +0200 Subject: [PATCH 1/2] Add occ command to set theming values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/theming/appinfo/info.xml | 3 + apps/theming/lib/Command/UpdateConfig.php | 135 ++++++++++++++++++ .../lib/Controller/ThemingController.php | 68 +-------- apps/theming/lib/ImageManager.php | 76 ++++++++-- apps/theming/lib/ThemingDefaults.php | 8 +- lib/private/Server.php | 2 +- 6 files changed, 215 insertions(+), 77 deletions(-) create mode 100644 apps/theming/lib/Command/UpdateConfig.php diff --git a/apps/theming/appinfo/info.xml b/apps/theming/appinfo/info.xml index 1e6fe5adb0..b0cf59c245 100644 --- a/apps/theming/appinfo/info.xml +++ b/apps/theming/appinfo/info.xml @@ -25,4 +25,7 @@ OCA\Theming\Settings\Admin OCA\Theming\Settings\Section + + OCA\Theming\Command\UpdateConfig + diff --git a/apps/theming/lib/Command/UpdateConfig.php b/apps/theming/lib/Command/UpdateConfig.php new file mode 100644 index 0000000000..7d616879dc --- /dev/null +++ b/apps/theming/lib/Command/UpdateConfig.php @@ -0,0 +1,135 @@ + + * + * @author Julius Härtl + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Theming\Command; + +use OCA\Theming\ImageManager; +use OCA\Theming\ThemingDefaults; +use OCP\IConfig; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +class UpdateConfig extends Command { + public const SUPPORTED_KEYS = [ + 'name', 'url', 'imprintUrl', 'privacyUrl', 'slogan', 'color' + ]; + + public const SUPPORTED_IMAGE_KEYS = [ + 'background', 'logo', 'favicon', 'logoheader' + ]; + + private $themingDefaults; + private $imageManager; + private $config; + + public function __construct(ThemingDefaults $themingDefaults, ImageManager $imageManager, IConfig $config) { + parent::__construct(); + + $this->themingDefaults = $themingDefaults; + $this->imageManager = $imageManager; + $this->config = $config; + } + + protected function configure() { + $this + ->setName('theming:config') + ->setDescription('Set theming app config values') + ->addArgument( + 'key', + InputArgument::OPTIONAL, + 'Key to update the theming app configuration (leave empty to get a list of all configured values)' . PHP_EOL . + 'One of: ' . implode(', ', self::SUPPORTED_KEYS) + ) + ->addArgument( + 'value', + InputArgument::OPTIONAL, + 'Value to set (leave empty to obtain the current value)' + ) + ->addOption( + 'reset', + 'r', + InputOption::VALUE_NONE, + 'Reset the given config key to default' + ); + } + + + protected function execute(InputInterface $input, OutputInterface $output): int { + $key = $input->getArgument('key'); + $value = $input->getArgument('value'); + + if ($key === null) { + $output->writeln('Current theming config:'); + foreach (self::SUPPORTED_KEYS as $key) { + $value = $this->config->getAppValue('theming', $key, ''); + $output->writeln('- ' . $key . ': ' . $value . ''); + } + foreach (self::SUPPORTED_IMAGE_KEYS as $key) { + $value = $this->config->getAppValue('theming', $key . 'Mime', ''); + $output->writeln('- ' . $key . ': ' . $value . ''); + } + return 0; + } + + if (!in_array($key, self::SUPPORTED_KEYS, true)) { + $output->writeln('Invalid config key provided'); + return 1; + } + + if ($input->getOption('reset')) { + $defaultValue = $this->themingDefaults->undo($key); + $output->writeln('Reset ' . $key . ' to ' . $defaultValue . ''); + return 0; + } + + if ($value === null) { + $value = $this->config->getAppValue('theming', $key, ''); + if ($value !== '') { + $output->writeln('' . $key . ' is currently set to ' . $value . ''); + } else { + $output->writeln('' . $key . ' is currently not set'); + } + return 0; + } + + if (in_array($key, self::SUPPORTED_IMAGE_KEYS, true)) { + if (file_exists(__DIR__ . $value)) { + $value = __DIR__ . $value; + } + if (!file_exists($value)) { + $output->writeln('File could not be found: ' . $value . ''); + return 1; + } + $value = $this->imageManager->updateImage($key, $value); + $key = $key . 'Mime'; + } + + $this->themingDefaults->set($key, $value); + $output->writeln('Updated ' . $key . ' to ' . $value . ''); + + return 0; + } +} diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 241f88dde6..b15ac22183 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -221,9 +221,6 @@ class ThemingController extends Controller { * @throws NotPermittedException */ public function uploadImage(): DataResponse { - // logo / background - // new: favicon logo-header - // $key = $this->request->getParam('key'); $image = $this->request->getUploadedFile('image'); $error = null; @@ -256,23 +253,14 @@ class ThemingController extends Controller { ); } - $name = ''; try { - $folder = $this->appData->getFolder('images'); - } catch (NotFoundException $e) { - $folder = $this->appData->newFolder('images'); - } - - $this->imageManager->delete($key); - - $target = $folder->newFile($key); - $supportedFormats = $this->getSupportedUploadImageFormats($key); - $detectedMimeType = mime_content_type($image['tmp_name']); - if (!in_array($image['type'], $supportedFormats) || !in_array($detectedMimeType, $supportedFormats)) { + $mime = $this->imageManager->updateImage($key, $image['tmp_name']); + $this->themingDefaults->set($key . 'Mime', $mime); + } catch (\Exception $e) { return new DataResponse( [ 'data' => [ - 'message' => $this->l10n->t('Unsupported image type'), + 'message' => $e->getMessage() ], 'status' => 'failure', ], @@ -280,32 +268,7 @@ class ThemingController extends Controller { ); } - if ($key === 'background' && strpos($detectedMimeType, 'image/svg') === false) { - // Optimize the image since some people may upload images that will be - // either to big or are not progressive rendering. - $newImage = @imagecreatefromstring(file_get_contents($image['tmp_name'], 'r')); - - // Preserve transparency - imagesavealpha($newImage, true); - imagealphablending($newImage, true); - - $tmpFile = $this->tempManager->getTemporaryFile(); - $newWidth = imagesx($newImage) < 4096 ? imagesx($newImage) : 4096; - $newHeight = imagesy($newImage) / (imagesx($newImage) / $newWidth); - $outputImage = imagescale($newImage, $newWidth, $newHeight); - - imageinterlace($outputImage, 1); - imagepng($outputImage, $tmpFile, 8); - imagedestroy($outputImage); - - $target->putContent(file_get_contents($tmpFile, 'r')); - } else { - $target->putContent(file_get_contents($image['tmp_name'], 'r')); - } $name = $image['name']; - - $this->themingDefaults->set($key.'Mime', $image['type']); - $cssCached = $this->scssCacher->process(\OC::$SERVERROOT, 'core/css/css-variables.scss', 'core'); return new DataResponse( @@ -322,24 +285,6 @@ class ThemingController extends Controller { ); } - /** - * Returns a list of supported mime types for image uploads. - * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available. - * - * @param string $key The image key, e.g. "favicon" - * @return array - */ - private function getSupportedUploadImageFormats(string $key): array { - $supportedFormats = ['image/jpeg', 'image/png', 'image/gif',]; - - if ($key !== 'favicon' || $this->imageManager->shouldReplaceIcons() === true) { - $supportedFormats[] = 'image/svg+xml'; - $supportedFormats[] = 'image/svg'; - } - - return $supportedFormats; - } - /** * Revert setting to default value * @@ -352,11 +297,6 @@ class ThemingController extends Controller { // reprocess server scss for preview $cssCached = $this->scssCacher->process(\OC::$SERVERROOT, 'core/css/css-variables.scss', 'core'); - if (strpos($setting, 'Mime') !== -1) { - $imageKey = str_replace('Mime', '', $setting); - $this->imageManager->delete($imageKey); - } - return new DataResponse( [ 'data' => diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index bf52fd8176..1b4848638f 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -36,6 +36,7 @@ use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICacheFactory; use OCP\IConfig; use OCP\ILogger; +use OCP\ITempManager; use OCP\IURLGenerator; class ImageManager { @@ -52,27 +53,22 @@ class ImageManager { private $cacheFactory; /** @var ILogger */ private $logger; + /** @var ITempManager */ + private $tempManager; - /** - * ImageManager constructor. - * - * @param IConfig $config - * @param IAppData $appData - * @param IURLGenerator $urlGenerator - * @param ICacheFactory $cacheFactory - * @param ILogger $logger - */ public function __construct(IConfig $config, IAppData $appData, IURLGenerator $urlGenerator, ICacheFactory $cacheFactory, - ILogger $logger + ILogger $logger, + ITempManager $tempManager ) { $this->config = $config; $this->appData = $appData; $this->urlGenerator = $urlGenerator; $this->cacheFactory = $cacheFactory; $this->logger = $logger; + $this->tempManager = $tempManager; } public function getImageUrl(string $key, bool $useSvg = true): string { @@ -211,6 +207,66 @@ class ImageManager { } } + public function updateImage(string $key, string $tmpFile) { + $this->delete($key); + + try { + $folder = $this->appData->getFolder('images'); + } catch (NotFoundException $e) { + $folder = $this->appData->newFolder('images'); + } + + $target = $folder->newFile($key); + $supportedFormats = $this->getSupportedUploadImageFormats($key); + $detectedMimeType = mime_content_type($tmpFile); + if (!in_array($detectedMimeType, $supportedFormats, true)) { + throw new \Exception('Unsupported image type'); + } + + if ($key === 'background' && strpos($detectedMimeType, 'image/svg') === false) { + // Optimize the image since some people may upload images that will be + // either to big or are not progressive rendering. + $newImage = @imagecreatefromstring(file_get_contents($tmpFile)); + + // Preserve transparency + imagesavealpha($newImage, true); + imagealphablending($newImage, true); + + $tmpFile = $this->tempManager->getTemporaryFile(); + $newWidth = (int)(imagesx($newImage) < 4096 ? imagesx($newImage) : 4096); + $newHeight = (int)(imagesy($newImage) / (imagesx($newImage) / $newWidth)); + $outputImage = imagescale($newImage, $newWidth, $newHeight); + + imageinterlace($outputImage, 1); + imagepng($outputImage, $tmpFile, 8); + imagedestroy($outputImage); + + $target->putContent(file_get_contents($tmpFile, 'r')); + } else { + $target->putContent(file_get_contents($tmpFile)); + } + + return $detectedMimeType; + } + + /** + * Returns a list of supported mime types for image uploads. + * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available. + * + * @param string $key The image key, e.g. "favicon" + * @return array + */ + private function getSupportedUploadImageFormats(string $key): array { + $supportedFormats = ['image/jpeg', 'image/png', 'image/gif']; + + if ($key !== 'favicon' || $this->shouldReplaceIcons() === true) { + $supportedFormats[] = 'image/svg+xml'; + $supportedFormats[] = 'image/svg'; + } + + return $supportedFormats; + } + /** * remove cached files that are not required any longer * diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index e568084662..3fdbc1a61a 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -398,6 +398,7 @@ class ThemingDefaults extends \OC_Defaults { $this->config->deleteAppValue('theming', $setting); $this->increaseCacheBuster(); + $returnValue = ''; switch ($setting) { case 'name': $returnValue = $this->getEntity(); @@ -411,8 +412,11 @@ class ThemingDefaults extends \OC_Defaults { case 'color': $returnValue = $this->getColorPrimary(); break; - default: - $returnValue = ''; + case 'logo': + case 'logoheader': + case 'background': + case 'favicon': + $this->imageManager->delete($setting); break; } diff --git a/lib/private/Server.php b/lib/private/Server.php index 189b00511e..224ef68a7c 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1126,7 +1126,7 @@ class Server extends ServerContainer implements IServerContainer { $c->getURLGenerator(), $c->getMemCacheFactory(), new Util($c->getConfig(), $this->getAppManager(), $c->getAppDataDir('theming')), - new ImageManager($c->getConfig(), $c->getAppDataDir('theming'), $c->getURLGenerator(), $this->getMemCacheFactory(), $this->getLogger()), + new ImageManager($c->getConfig(), $c->getAppDataDir('theming'), $c->getURLGenerator(), $this->getMemCacheFactory(), $this->getLogger(), $this->getTempManager()), $c->getAppManager(), $c->getNavigationManager() ); From 0dd6819b5fa94b296f39b01373402d9dd475442e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 24 Sep 2020 08:20:03 +0200 Subject: [PATCH 2/2] Fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../Controller/ThemingControllerTest.php | 93 ++++--------------- apps/theming/tests/ImageManagerTest.php | 60 +++++++++++- 2 files changed, 78 insertions(+), 75 deletions(-) diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index b9c291c957..9f6b41cb50 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -49,12 +49,12 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\SimpleFS\ISimpleFile; -use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\ITempManager; use OCP\IURLGenerator; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ThemingControllerTest extends TestCase { @@ -98,12 +98,12 @@ class ThemingControllerTest extends TestCase { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->imageManager = $this->createMock(ImageManager::class); - $this->timeFactory = $this->createMock(ITimeFactory::class); - $this->timeFactory->expects($this->any()) + $timeFactory = $this->createMock(ITimeFactory::class); + $timeFactory->expects($this->any()) ->method('getTime') ->willReturn(123); - $this->overwriteService(ITimeFactory::class, $this->timeFactory); + $this->overwriteService(ITimeFactory::class, $timeFactory); $this->themingController = new ThemingController( 'theming', @@ -293,12 +293,9 @@ class ThemingControllerTest extends TestCase { return $str; }); - $folder = $this->createMock(ISimpleFolder::class); - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); + $this->imageManager->expects($this->once()) + ->method('updateImage') + ->willThrowException(new \Exception('Unsupported image type')); $expected = new DataResponse( [ @@ -337,12 +334,9 @@ class ThemingControllerTest extends TestCase { return $str; }); - $folder = $this->createMock(ISimpleFolder::class); - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); + $this->imageManager->expects($this->once()) + ->method('updateImage') + ->willThrowException(new \Exception('Unsupported image type')); $expected = new DataResponse( [ @@ -398,31 +392,6 @@ class ThemingControllerTest extends TestCase { return $str; }); - - $file = $this->createMock(ISimpleFile::class); - $folder = $this->createMock(ISimpleFolder::class); - if ($folderExists) { - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); - } else { - $this->appData - ->expects($this->at(0)) - ->method('getFolder') - ->with('images') - ->willThrowException(new NotFoundException()); - $this->appData - ->expects($this->at(1)) - ->method('newFolder') - ->with('images') - ->willReturn($folder); - } - $folder->expects($this->once()) - ->method('newFile') - ->with('logo') - ->willReturn($file); $this->urlGenerator->expects($this->once()) ->method('linkTo') ->willReturn('serverCss'); @@ -430,6 +399,10 @@ class ThemingControllerTest extends TestCase { ->method('getImageUrl') ->with('logo') ->willReturn('imageUrl'); + + $this->imageManager->expects($this->once()) + ->method('updateImage'); + $expected = new DataResponse( [ 'data' => @@ -474,30 +447,8 @@ class ThemingControllerTest extends TestCase { return $str; }); - $file = $this->createMock(ISimpleFile::class); - $folder = $this->createMock(ISimpleFolder::class); - if ($folderExists) { - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); - } else { - $this->appData - ->expects($this->at(0)) - ->method('getFolder') - ->with('images') - ->willThrowException(new NotFoundException()); - $this->appData - ->expects($this->at(1)) - ->method('newFolder') - ->with('images') - ->willReturn($folder); - } - $folder->expects($this->once()) - ->method('newFile') - ->with('background') - ->willReturn($file); + $this->imageManager->expects($this->once()) + ->method('updateImage'); $this->urlGenerator->expects($this->once()) ->method('linkTo') @@ -548,12 +499,9 @@ class ThemingControllerTest extends TestCase { return $str; }); - $folder = $this->createMock(ISimpleFolder::class); - $this->appData - ->expects($this->once()) - ->method('getFolder') - ->with('images') - ->willReturn($folder); + $this->imageManager->expects($this->once()) + ->method('updateImage') + ->willThrowException(new \Exception('Unsupported image type')); $expected = new DataResponse( [ @@ -723,9 +671,6 @@ class ThemingControllerTest extends TestCase { ->method('linkTo') ->with('', '/core/css/someHash-css-variables.scss') ->willReturn('/nextcloudWebroot/core/css/someHash-css-variables.scss'); - $this->imageManager->expects($this->once()) - ->method('delete') - ->with($filename); $expected = new DataResponse( [ diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php index 1187ecf036..c62b51d733 100644 --- a/apps/theming/tests/ImageManagerTest.php +++ b/apps/theming/tests/ImageManagerTest.php @@ -35,7 +35,9 @@ use OCP\Files\SimpleFS\ISimpleFolder; use OCP\ICacheFactory; use OCP\IConfig; use OCP\ILogger; +use OCP\ITempManager; use OCP\IURLGenerator; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ImageManagerTest extends TestCase { @@ -52,6 +54,8 @@ class ImageManagerTest extends TestCase { private $cacheFactory; /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ private $logger; + /** @var ITempManager|MockObject */ + private $tempManager; protected function setUp(): void { parent::setUp(); @@ -60,12 +64,14 @@ class ImageManagerTest extends TestCase { $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->logger = $this->createMock(ILogger::class); + $this->tempManager = $this->createMock(ITempManager::class); $this->imageManager = new ImageManager( $this->config, $this->appData, $this->urlGenerator, $this->cacheFactory, - $this->logger + $this->logger, + $this->tempManager ); } @@ -326,4 +332,56 @@ class ImageManagerTest extends TestCase { ->willReturn($folders[2]); $this->imageManager->cleanup(); } + + + public function dataUpdateImage() { + return [ + ['background', __DIR__ . '/../../../tests/data/testimage.png', true, true], + ['background', __DIR__ . '/../../../tests/data/testimage.png', false, true], + ['background', __DIR__ . '/../../../tests/data/testimage.jpg', true, true], + ['logo', __DIR__ . '/../../../tests/data/testimagelarge.svg', true, false], + ]; + } + + /** + * @dataProvider dataUpdateImage + */ + public function testUpdateImage($key, $tmpFile, $folderExists, $shouldConvert) { + $file = $this->createMock(ISimpleFile::class); + $folder = $this->createMock(ISimpleFolder::class); + $oldFile = $this->createMock(ISimpleFile::class); + $folder->expects($this->any()) + ->method('getFile') + ->willReturn($oldFile); + if ($folderExists) { + $this->appData + ->expects($this->any()) + ->method('getFolder') + ->with('images') + ->willReturn($folder); + } else { + $this->appData + ->expects($this->any()) + ->method('getFolder') + ->with('images') + ->willThrowException(new NotFoundException()); + $this->appData + ->expects($this->any()) + ->method('newFolder') + ->with('images') + ->willReturn($folder); + } + $folder->expects($this->once()) + ->method('newFile') + ->with($key) + ->willReturn($file); + + if ($shouldConvert) { + $this->tempManager->expects($this->once()) + ->method('getTemporaryFile') + ->willReturn('/tmp/randomtempfile-theming'); + } + + $this->imageManager->updateImage($key, $tmpFile); + } }