Merge pull request #14954 from owncloud/cache-image-path

Cache \OC\URLGenerator::imagePath

4 files changed, 51 insertions, 24 deletions

diff --git a/apps/files_sharing/public.php b/apps/files_sharing/public.php
index 50a6432dd95..9e4e8d23151 100644
--- a/apps/files_sharing/public.php
+++ b/apps/files_sharing/public.php
@@ -10,7 +10,7 @@
 
 // This file is just used to redirect the legacy sharing URLs (< ownCloud 8) to the new ones
 
-$urlGenerator = new \OC\URLGenerator(\OC::$server->getConfig());
+$urlGenerator = \OC::$server->getURLGenerator();
 $token = isset($_GET['t']) ? $_GET['t'] : '';
 $route = isset($_GET['download']) ? 'files_sharing.sharecontroller.downloadShare' : 'files_sharing.sharecontroller.showShare';
 
diff --git a/lib/private/server.php b/lib/private/server.php
index 6261337e0b1..c55817bb6b3 100644
--- a/lib/private/server.php
+++ b/lib/private/server.php
@@ -147,7 +147,11 @@ class Server extends SimpleContainer implements IServerContainer {
 		});
 		$this->registerService('URLGenerator', function (Server $c) {
 			$config = $c->getConfig();
-			return new \OC\URLGenerator($config);
+			$cacheFactory = $c->getMemCacheFactory();
+			return new \OC\URLGenerator(
+				$config,
+				$cacheFactory
+			);
 		});
 		$this->registerService('AppHelper', function ($c) {
 			return new \OC\AppHelper();
diff --git a/lib/private/urlgenerator.php b/lib/private/urlgenerator.php
index e87a6c354fb..93666a86780 100644
--- a/lib/private/urlgenerator.php
+++ b/lib/private/urlgenerator.php
@@ -9,6 +9,8 @@
 
 namespace OC;
 use OC_Defaults;
+use OCP\ICacheFactory;
+use OCP\IConfig;
 use OCP\IURLGenerator;
 use RuntimeException;
 
@@ -16,17 +18,19 @@ use RuntimeException;
  * Class to generate URLs
  */
 class URLGenerator implements IURLGenerator {
-
-	/**
-	 * @var \OCP\IConfig
-	 */
+	/** @var IConfig */
 	private $config;
+	/** @var ICacheFactory */
+	private $cacheFactory;
 
 	/**
-	 * @param \OCP\IConfig $config
+	 * @param IConfig $config
+	 * @param ICacheFactory $cacheFactory
 	 */
-	public function __construct($config) {
+	public function __construct(IConfig $config,
+								ICacheFactory $cacheFactory) {
 		$this->config = $config;
+		$this->cacheFactory = $cacheFactory;
 	}
 
 	/**
@@ -113,6 +117,12 @@ class URLGenerator implements IURLGenerator {
 	 * Returns the path to the image.
 	 */
 	public function imagePath($app, $image) {
+		$cache = $this->cacheFactory->create('imagePath');
+		$cacheKey = $app.'-'.$image;
+		if($key = $cache->get($cacheKey)) {
+			return $key;
+		}
+
 		// Read the selected theme from the config file
 		$theme = \OC_Util::getTheme();
 
@@ -120,33 +130,39 @@ class URLGenerator implements IURLGenerator {
 		$basename = substr(basename($image),0,-4);
 
 		// Check if the app is in the app folder
+		$path = '';
 		if (file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$image")) {
-			return \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image";
+			$path = \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$image";
 		} elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.svg")
 			&& file_exists(\OC::$SERVERROOT . "/themes/$theme/apps/$app/img/$basename.png")) {
-			return \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$basename.png";
+			$path =  \OC::$WEBROOT . "/themes/$theme/apps/$app/img/$basename.png";
 		} elseif (file_exists(\OC_App::getAppPath($app) . "/img/$image")) {
-			return \OC_App::getAppWebPath($app) . "/img/$image";
+			$path =  \OC_App::getAppWebPath($app) . "/img/$image";
 		} elseif (!file_exists(\OC_App::getAppPath($app) . "/img/$basename.svg")
 			&& file_exists(\OC_App::getAppPath($app) . "/img/$basename.png")) {
-			return \OC_App::getAppPath($app) . "/img/$basename.png";
+			$path =  \OC_App::getAppPath($app) . "/img/$basename.png";
 		} elseif (!empty($app) and file_exists(\OC::$SERVERROOT . "/themes/$theme/$app/img/$image")) {
-			return \OC::$WEBROOT . "/themes/$theme/$app/img/$image";
+			$path =  \OC::$WEBROOT . "/themes/$theme/$app/img/$image";
 		} elseif (!empty($app) and (!file_exists(\OC::$SERVERROOT . "/themes/$theme/$app/img/$basename.svg")
 			&& file_exists(\OC::$SERVERROOT . "/themes/$theme/$app/img/$basename.png"))) {
-			return \OC::$WEBROOT . "/themes/$theme/$app/img/$basename.png";
+			$path =  \OC::$WEBROOT . "/themes/$theme/$app/img/$basename.png";
 		} elseif (!empty($app) and file_exists(\OC::$SERVERROOT . "/$app/img/$image")) {
-			return \OC::$WEBROOT . "/$app/img/$image";
+			$path =  \OC::$WEBROOT . "/$app/img/$image";
 		} elseif (!empty($app) and (!file_exists(\OC::$SERVERROOT . "/$app/img/$basename.svg")
 			&& file_exists(\OC::$SERVERROOT . "/$app/img/$basename.png"))) {
-			return \OC::$WEBROOT . "/$app/img/$basename.png";
+			$path =  \OC::$WEBROOT . "/$app/img/$basename.png";
 		} elseif (file_exists(\OC::$SERVERROOT . "/themes/$theme/core/img/$image")) {
-			return \OC::$WEBROOT . "/themes/$theme/core/img/$image";
+			$path =  \OC::$WEBROOT . "/themes/$theme/core/img/$image";
 		} elseif (!file_exists(\OC::$SERVERROOT . "/themes/$theme/core/img/$basename.svg")
 			&& file_exists(\OC::$SERVERROOT . "/themes/$theme/core/img/$basename.png")) {
-			return \OC::$WEBROOT . "/themes/$theme/core/img/$basename.png";
+			$path =  \OC::$WEBROOT . "/themes/$theme/core/img/$basename.png";
 		} elseif (file_exists(\OC::$SERVERROOT . "/core/img/$image")) {
-			return \OC::$WEBROOT . "/core/img/$image";
+			$path =  \OC::$WEBROOT . "/core/img/$image";
+		}
+
+		if($path !== '') {
+			$cache->set($cacheKey, $path);
+			return $path;
 		} else {
 			throw new RuntimeException('image not found: image:' . $image . ' webroot:' . \OC::$WEBROOT . ' serverroot:' . \OC::$SERVERROOT);
 		}
diff --git a/tests/lib/urlgenerator.php b/tests/lib/urlgenerator.php
index a92aaddeb4c..60e1a86f16a 100644
--- a/tests/lib/urlgenerator.php
+++ b/tests/lib/urlgenerator.php
@@ -16,7 +16,8 @@ class Test_Urlgenerator extends \Test\TestCase {
 	public function testLinkToDocRoot($app, $file, $args, $expectedResult) {
 		\OC::$WEBROOT = '';
 		$config = $this->getMock('\OCP\IConfig');
-		$urlGenerator = new \OC\URLGenerator($config);
+		$cacheFactory = $this->getMock('\OCP\ICacheFactory');
+		$urlGenerator = new \OC\URLGenerator($config, $cacheFactory);
 		$result = $urlGenerator->linkTo($app, $file, $args);
 
 		$this->assertEquals($expectedResult, $result);
@@ -30,7 +31,8 @@ class Test_Urlgenerator extends \Test\TestCase {
 	public function testLinkToSubDir($app, $file, $args, $expectedResult) {
 		\OC::$WEBROOT = '/owncloud';
 		$config = $this->getMock('\OCP\IConfig');
-		$urlGenerator = new \OC\URLGenerator($config);
+		$cacheFactory = $this->getMock('\OCP\ICacheFactory');
+		$urlGenerator = new \OC\URLGenerator($config, $cacheFactory);
 		$result = $urlGenerator->linkTo($app, $file, $args);
 
 		$this->assertEquals($expectedResult, $result);
@@ -42,7 +44,8 @@ class Test_Urlgenerator extends \Test\TestCase {
 	public function testLinkToRouteAbsolute($route, $expected) {
 		\OC::$WEBROOT = '/owncloud';
 		$config = $this->getMock('\OCP\IConfig');
-		$urlGenerator = new \OC\URLGenerator($config);
+		$cacheFactory = $this->getMock('\OCP\ICacheFactory');
+		$urlGenerator = new \OC\URLGenerator($config, $cacheFactory);
 		$result = $urlGenerator->linkToRouteAbsolute($route);
 		$this->assertEquals($expected, $result);
 
@@ -79,7 +82,9 @@ class Test_Urlgenerator extends \Test\TestCase {
 	function testGetAbsoluteURLDocRoot($url, $expectedResult) {
 
 		\OC::$WEBROOT = '';
-		$urlGenerator = new \OC\URLGenerator(null);
+		$config = $this->getMock('\OCP\IConfig');
+		$cacheFactory = $this->getMock('\OCP\ICacheFactory');
+		$urlGenerator = new \OC\URLGenerator($config, $cacheFactory);
 		$result = $urlGenerator->getAbsoluteURL($url);
 
 		$this->assertEquals($expectedResult, $result);
@@ -93,7 +98,9 @@ class Test_Urlgenerator extends \Test\TestCase {
 	function testGetAbsoluteURLSubDir($url, $expectedResult) {
 
 		\OC::$WEBROOT = '/owncloud';
-		$urlGenerator = new \OC\URLGenerator(null);
+		$config = $this->getMock('\OCP\IConfig');
+		$cacheFactory = $this->getMock('\OCP\ICacheFactory');
+		$urlGenerator = new \OC\URLGenerator($config, $cacheFactory);
 		$result = $urlGenerator->getAbsoluteURL($url);
 
 		$this->assertEquals($expectedResult, $result);