From 73bc108451169aa8ba95eba3749c6450de2ded2e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Sat, 27 Aug 2016 12:38:15 +0200 Subject: [PATCH] Increase theming performance 1. Set proper caching headers (`Pragma: cache`) 2. Resize image proportionally to a max size of 1920px 3. Store images with progressive mode This resizes a previous 2.8 MB picture to 300kb and makes it rendering going down from 11 seconds to less than 1 here. And future requests won't have to download the file newly. --- apps/theming/js/settings-admin.js | 8 +-- .../lib/Controller/ThemingController.php | 34 ++++++++++- .../Controller/ThemingControllerTest.php | 57 +++++++++++++++++++ 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/apps/theming/js/settings-admin.js b/apps/theming/js/settings-admin.js index 0de3b28bab0..c896da321c8 100644 --- a/apps/theming/js/settings-admin.js +++ b/apps/theming/js/settings-admin.js @@ -157,8 +157,8 @@ $(document).ready(function () { submit: function(e, response) { OC.msg.startSaving('#theming_settings_msg'); }, - fail: function (e, data){ - OC.msg.finishedSaving('#theming_settings_msg', response); + fail: function (e, response){ + OC.msg.finishedError('#theming_settings_msg', response._response.jqXHR.responseJSON.data.message); } }; var uploadParamsLogin = { @@ -171,8 +171,8 @@ $(document).ready(function () { submit: function(e, response) { OC.msg.startSaving('#theming_settings_msg'); }, - fail: function (e, data){ - OC.msg.finishedSaving('#theming_settings_msg', response); + fail: function (e, response){ + OC.msg.finishedError('#theming_settings_msg', response._response.jqXHR.responseJSON.data.message); } }; diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 8a7aaec6b5e..67300b17c6d 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -171,7 +171,8 @@ class ThemingController extends Controller { 'message' => $this->l->t('No file uploaded') ] ], - Http::STATUS_UNPROCESSABLE_ENTITY); + Http::STATUS_UNPROCESSABLE_ENTITY + ); } $name = ''; if(!empty($newLogo)) { @@ -182,7 +183,30 @@ class ThemingController extends Controller { } if(!empty($newBackgroundLogo)) { $target = $this->rootFolder->newFile('themedbackgroundlogo'); - stream_copy_to_stream(fopen($newBackgroundLogo['tmp_name'], 'r'), $target->fopen('w')); + + $image = @imagecreatefromstring(file_get_contents($newBackgroundLogo['tmp_name'], 'r')); + if($image === false) { + return new DataResponse( + [ + 'data' => [ + 'message' => $this->l->t('Unsupported image type'), + ], + 'status' => 'failure', + ], + Http::STATUS_UNPROCESSABLE_ENTITY + ); + } + + // Optimize the image since some people may upload images that will be + // either to big or are not progressive rendering. + if(function_exists('imagescale')) { + // FIXME: Once PHP 5.5.0 is a requirement the above check can be removed + $image = imagescale($image, 1920); + } + imageinterlace($image, 1); + imagejpeg($image, $target->fopen('w'), 75); + imagedestroy($image); + $this->template->set('backgroundMime', $newBackgroundLogo['type']); $name = $newBackgroundLogo['name']; } @@ -236,6 +260,7 @@ class ThemingController extends Controller { $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $response->addHeader('Content-Disposition', 'attachment'); $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'logoMime', '')); + $response->addHeader('Pragma', 'cache'); return $response; } @@ -256,6 +281,7 @@ class ThemingController extends Controller { $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); $response->addHeader('Content-Disposition', 'attachment'); $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, 'backgroundMime', '')); + $response->addHeader('Pragma', 'cache'); return $response; } @@ -358,6 +384,7 @@ class ThemingController extends Controller { $response = new DataDownloadResponse($responseCss, 'style', 'text/css'); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); $response->cacheFor(3600); return $response; } @@ -378,8 +405,9 @@ class ThemingController extends Controller { }; })();'; $response = new Http\DataDisplayResponse($responseJS); - $response->addHeader("Content-type","text/javascript"); + $response->addHeader('Content-type', 'text/javascript'); $response->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $response->addHeader('Pragma', 'cache'); $response->cacheFor(3600); return $response; } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index da2137e9da0..6171b30215e 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -214,6 +214,7 @@ class ThemingControllerTest extends TestCase { $destination = \OC::$server->getTempManager()->getTemporaryFolder(); touch($tmpLogo); + file_put_contents($tmpLogo, file_get_contents(__DIR__ . '/../../../../tests/data/desktopapp.png')); $this->request ->expects($this->at(0)) ->method('getUploadedFile') @@ -261,6 +262,52 @@ class ThemingControllerTest extends TestCase { $this->assertEquals($expected, $this->themingController->updateLogo()); } + public function testUpdateLogoLoginScreenUploadWithInvalidImage() { + $tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg'; + $destination = \OC::$server->getTempManager()->getTemporaryFolder(); + + touch($tmpLogo); + file_put_contents($tmpLogo, file_get_contents(__DIR__ . '/../../../../tests/data/data.zip')); + $this->request + ->expects($this->at(0)) + ->method('getUploadedFile') + ->with('uploadlogo') + ->willReturn(null); + $this->request + ->expects($this->at(1)) + ->method('getUploadedFile') + ->with('upload-login-background') + ->willReturn([ + 'tmp_name' => $tmpLogo, + 'type' => 'text/svg', + 'name' => 'logo.svg', + ]); + $this->l10n + ->expects($this->once()) + ->method('t') + ->with('Unsupported image type') + ->willReturn('Unsupported image type'); + $file = $this->getMockBuilder('\\OCP\\Files\\File') + ->disableOriginalConstructor() + ->getMock(); + $this->rootFolder + ->expects($this->once()) + ->method('newFile') + ->with('themedbackgroundlogo') + ->willReturn($file); + $expected = new DataResponse( + [ + 'data' => + [ + 'message' => 'Unsupported image type', + ], + 'status' => 'failure' + ], + Http::STATUS_UNPROCESSABLE_ENTITY + ); + $this->assertEquals($expected, $this->themingController->updateLogo()); + } + public function testUndo() { $this->l10n ->expects($this->once()) @@ -311,6 +358,7 @@ class ThemingControllerTest extends TestCase { $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); $expected->addHeader('Content-Disposition', 'attachment'); $expected->addHeader('Content-Type', 'text/svg'); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getLogo()); } @@ -340,6 +388,7 @@ class ThemingControllerTest extends TestCase { $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); $expected->addHeader('Content-Disposition', 'attachment'); $expected->addHeader('Content-Type', 'image/png'); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getLoginBackground()); } @@ -422,6 +471,7 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -510,6 +560,7 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -553,6 +604,7 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -588,6 +640,7 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -687,6 +740,7 @@ class ThemingControllerTest extends TestCase { $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -791,6 +845,7 @@ class ThemingControllerTest extends TestCase { $expected = new Http\DataDownloadResponse($expectedData, 'style', 'text/css'); $expected->cacheFor(3600); $expected->addHeader('Expires', date(\DateTime::RFC2822, 123)); + $expected->addHeader('Pragma', 'cache'); @$this->assertEquals($expected, $this->themingController->getStylesheet()); } @@ -825,6 +880,7 @@ class ThemingControllerTest extends TestCase { $expected = new Http\DataDisplayResponse($expectedResponse); $expected->addHeader("Content-type","text/javascript"); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); $expected->cacheFor(3600); @$this->assertEquals($expected, $this->themingController->getJavascript()); } @@ -858,6 +914,7 @@ class ThemingControllerTest extends TestCase { $expected = new Http\DataDisplayResponse($expectedResponse); $expected->addHeader("Content-type","text/javascript"); $expected->addHeader('Expires', date(\DateTime::RFC2822, $this->timeFactory->getTime())); + $expected->addHeader('Pragma', 'cache'); $expected->cacheFor(3600); @$this->assertEquals($expected, $this->themingController->getJavascript()); } -- 2.39.5