Refactors the scss svg functions

Signed-off-by: Michael Weimann <mail@michael-weimann.eu>

apps/theming/css/theming.scss

@@ -12,9 +12,9 @@ $invert: luma($color-primary) > 0.6;
 
 @if ($has-custom-logo == false) {
 	@if ($invert) {
-		$image-logo: url('../../../../svg/core/logo/logo/000000?v=1');
+		$image-logo: url(icon-color-path('logo', 'logo', #000000, 1, true));
 	} @else {
-		$image-logo: url('../../../../svg/core/logo/logo/ffffff?v=1');
+		$image-logo: url(icon-color-path('logo', 'logo', #ffffff, 1, true));
 	}
 }
 

core/Controller/SvgController.php

@@ -29,7 +29,6 @@ use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
-use OCP\Files\NotFoundException;
 use OCP\App\IAppManager;
 use OCP\IRequest;
 
@@ -99,7 +98,6 @@ class SvgController extends Controller {
 		return $this->getSvg($path, $color, $fileName);
 	}
 
-
 	/**
 	 * Generate svg from filename with the requested color
 	 *
@@ -119,7 +117,7 @@ class SvgController extends Controller {
 		}
 
 		// add fill (fill is not present on black elements)
-		$fillRe = '/<((circle|rect|path)((!fill)[a-z0-9 =".\-#():;])+)\/>/mi';
+		$fillRe = '/<((circle|rect|path)((?!fill)[a-z0-9 =".\-#():;])+)\/>/mi';
 		$svg = preg_replace($fillRe, '<$1 fill="#' . $color . '"/>', $svg);
 
 		// replace any fill or stroke colors

core/css/functions.scss

@@ -19,31 +19,55 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
- 
+
+/**
+ * Removes the "#" from a color.
+ *
+ * @param string $color The color
+ * @return string The color without #
+ */
+@function remove-hash-from-color($color) {
+	$index: str-index(inspect($color), '#');
+	@if $index {
+		$color: str-slice(inspect($color), 2);
+	}
+	@return $color;
+}
+
+/**
+ * Calculates the URL to the svg under the SVG API.
+ *
+ * @param string $icon the icon filename
+ * @param string $dir the icon folder within /core/img if $core or app name
+ * @param string $color the desired color in hexadecimal
+ * @param int [$version] the version of the file
+ * @param bool [$core] search icon in core
+ * @return string The URL to the svg.
+ */
+@function icon-color-path($icon, $dir, $color, $version: 1, $core: false) {
+	$color: remove-hash-from-color($color);
+	@if $core {
+		@return '#{$webroot}/svg/core/#{$dir}/#{$icon}/#{$color}?v=#{$version}';
+	} @else {
+		@return '#{$webroot}/svg/#{$dir}/#{$icon}/#{$color}?v=#{$version}';
+	}
+}
+
 /**
  * SVG COLOR API
- * 
+ *
  * @param string $icon the icon filename
  * @param string $dir the icon folder within /core/img if $core or app name
  * @param string $color the desired color in hexadecimal
  * @param int $version the version of the file
  * @param bool [$core] search icon in core
  *
- * @returns string the url to the svg api endpoint
+ * @returns A background image with the url to the set to the requested icon.
  */
 @mixin icon-color($icon, $dir, $color, $version: 1, $core: false) {
-	// remove # from color
-	// inspect cast int to string
-	$index: str-index(inspect($color), '#');
-	@if $index {
-		$color: str-slice(inspect($color), 2);
-	}
+	$color: remove-hash-from-color($color);
 	$varName: "--icon-#{$icon}-#{$color}";
-	@if $core {
-		#{$varName}: url('#{$webroot}/svg/core/#{$dir}/#{$icon}/#{$color}?v=#{$version}');
-	} @else {
-		#{$varName}: url('#{$webroot}/svg/#{$dir}/#{$icon}/#{$color}?v=#{$version}');
-	}
+	#{$varName}: url(icon-color-path($icon, $dir, $color, $version, $core));
 	background-image: var(#{$varName});
 }
 

core/img/logo/logo.svg

@@ -1 +1 @@
-<svg width="256" height="128" version="1.1" viewBox="0 0 256 128" xmlns="http://www.w3.org/2000/svg"><g fill="none" stroke="#fff" stroke-width="22"><circle cx="40" cy="64" r="26"/><circle cx="216" cy="64" r="26"/><circle cx="128" cy="64" r="46"/></g></svg>
+<svg width="256" height="128" version="1.1" viewBox="0 0 256 128" xmlns="http://www.w3.org/2000/svg"><g fill="none" stroke-width="22"><circle cx="40" cy="64" r="26" stroke="#fff" fill="none"/><circle cx="216" cy="64" r="26" stroke="#fff" fill="none"/><circle cx="128" cy="64" r="46" stroke="#fff" fill="none"/></g></svg>

tests/Core/Controller/SvgControllerTest.php

@@ -0,0 +1,139 @@
+<?php
+declare (strict_types = 1);
+/**
+ * @copyright Copyright (c) 2018 Michael Weimann <mail@michael-weimann.eu>
+ *
+ * @author Michael Weimann <mail@michael-weimann.eu>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Tests\Core\Controller;
+
+use OC\AppFramework\Http;
+use OC\Core\Controller\SvgController;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\IRequest;
+use Test\TestCase;
+
+/**
+ * This class provides test cases for the svg controller
+ */
+class SvgControllerTest extends TestCase {
+
+	const TEST_IMAGES_SOURCE_PATH = __DIR__ . '/../../data/svg';
+	const TEST_IMAGES_PATH = __DIR__ . '/../../../core/img/testImages';
+	const TEST_IMAGE_MIXED = 'mixed-source.svg';
+	const TEST_IMAGE_RECT = 'rect-black.svg';
+	const TEST_IMAGES = [
+		self::TEST_IMAGE_MIXED,
+		self::TEST_IMAGE_RECT,
+	];
+
+	/**
+	 * @var SvgController
+	 */
+	private $svgController;
+
+	/**
+	 * Copy test svgs into the core img "test" directory.
+	 *
+	 * @beforeClass
+	 * @return void
+	 */
+	public static function copyTestImagesIntoPlace() {
+		mkdir(self::TEST_IMAGES_PATH);
+		foreach (self::TEST_IMAGES as $testImage) {
+			copy(
+				self::TEST_IMAGES_SOURCE_PATH .'/' . $testImage,
+				self::TEST_IMAGES_PATH . '/' . $testImage
+			);
+		}
+	}
+
+	/**
+	 * Removes the test svgs from the core img "test" directory.
+	 *
+	 * @afterClass
+	 * @return void
+	 */
+	public static function removeTestImages() {
+		foreach (self::TEST_IMAGES as $testImage) {
+			unlink(self::TEST_IMAGES_PATH . '/' . $testImage);
+		}
+		rmdir(self::TEST_IMAGES_PATH);
+	}
+
+	/**
+	 * Setups a SVG controller instance for tests.
+	 *
+	 * @before
+	 * @return void
+	 */
+	public function setupSvgController() {
+		$request = $this->getMockBuilder(IRequest::class)->getMock();
+		$timeFactory = $this->getMockBuilder(ITimeFactory::class)->getMock();
+		$appManager = $this->getMockBuilder(IAppManager::class)->getMock();
+		$this->svgController = new SvgController('core', $request, $timeFactory, $appManager);
+	}
+
+	/**
+	 * Checks that requesting an unknown image results in a 404.
+	 *
+	 * @test
+	 * @return void
+	 */
+	public function testGetSvgFromCoreNotFound() {
+		$response = $this->svgController->getSvgFromCore('huhuu', '2342', '#ff0000');
+		self::assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
+	}
+
+	/**
+	 * Provides svg coloring test data.
+	 *
+	 * @return array
+	 */
+	public function provideGetSvgFromCoreTestData(): array {
+		return [
+			'mixed' => ['mixed-source', 'f00', file_get_contents(self::TEST_IMAGES_SOURCE_PATH . '/mixed-red.svg')],
+			'black rect' => ['rect-black', 'f00', file_get_contents(self::TEST_IMAGES_SOURCE_PATH . '/rect-red.svg')],
+		];
+	}
+
+	/**
+	 * Tests that retrieving a colored SVG works.
+	 *
+	 * @test
+	 * @dataProvider provideGetSvgFromCoreTestData
+	 * @param string $name The requested svg name
+	 * @param string $color The requested color
+	 * @param string $expected The expected svg
+	 * @return void
+	 */
+	public function testGetSvgFromCore(string $name, string $color, string $expected) {
+		$response = $this->svgController->getSvgFromCore('testImages', $name, $color);
+
+		self::assertEquals(Http::STATUS_OK, $response->getStatus());
+
+		$headers = $response->getHeaders();
+		self::assertArrayHasKey('Content-Type', $headers);
+		self::assertEquals($headers['Content-Type'], 'image/svg+xml');
+
+		self::assertEquals($expected, $response->getData());
+	}
+}

tests/data/svg/mixed-red.svg

@@ -0,0 +1 @@
+<svg width="50mm" height="50mm" version="1.1" viewBox="0 0 50 50" xmlns="http://www.w3.org/2000/svg"><g transform="translate(0,-247)"><rect x="3.3609" y="250.93" width="16.253" height="15.497" fill="#f00"/><ellipse cx="37.089" cy="257.97" rx="8.5517" ry="7.9375" fill="#f00"/><circle cx="12.19" cy="281.31" r="10" fill="#f00"/><rect x="29.293" y="272.81" width="16.253" height="15.497" fill="#f00"/></g></svg>

tests/data/svg/mixed-source.svg

@@ -0,0 +1 @@
+<svg width="50mm" height="50mm" version="1.1" viewBox="0 0 50 50" xmlns="http://www.w3.org/2000/svg"><g transform="translate(0,-247)"><rect x="3.3609" y="250.93" width="16.253" height="15.497"/><ellipse cx="37.089" cy="257.97" rx="8.5517" ry="7.9375" fill="#123456"/><circle cx="12.19" cy="281.31" r="10"/><rect x="29.293" y="272.81" width="16.253" height="15.497" fill="#6474a5"/></g></svg>

tests/data/svg/rect-black.svg

@@ -0,0 +1 @@
+<svg width="50mm" height="50mm" version="1.1" viewBox="0 0 50 50" xmlns="http://www.w3.org/2000/svg"><g transform="translate(0,-247)"><rect x="10" y="257" width="30" height="30"/></g></svg>

tests/data/svg/rect-red.svg

@@ -0,0 +1 @@
+<svg width="50mm" height="50mm" version="1.1" viewBox="0 0 50 50" xmlns="http://www.w3.org/2000/svg"><g transform="translate(0,-247)"><rect x="10" y="257" width="30" height="30" fill="#f00"/></g></svg>