StaticAssets: handle svg files

classes/StaticAssets.class.php

@@ -19,39 +19,40 @@ class StaticAssets
     const PACKAGE_JS = 'js';
     const PACKAGE_CSS = 'css';
 
-    private $app;
-    private $debug;
-    private $router;
+    private NanoApp $app;
+    private \Nano\Debug $debug;
+    private Router $router;
 
     // application's root directory
-    private $localRoot;
+    private string $localRoot;
 
     // cache buster value
-    private $cb;
+    private int $cb;
 
     // path to Content Delivery Network (if used)
-    private $cdnPath;
+    private ?string $cdnPath;
 
     // should cache buster value be prepended to an URL?
     // example: /r200/foo/bar.js [true]
     // example: /foo/bar.js?r=200 [false]
-    private $prependCacheBuster;
+    private bool $prependCacheBuster;
 
     // is StaticAssets in debug mode?
     // add debug=1 to URL
-    private $debugMode = false;
+    private bool $debugMode = false;
 
     // registered packages
-    private $packages;
+    private array $packages;
 
     // list of supported extensions with their mime types
-    private $types = [
+    private array $types = [
         'css' => 'text/css; charset=utf-8',
         'js' => 'application/javascript; charset=utf-8',
         'gif' => 'image/gif',
         'png' => 'image/png',
         'jpg' => 'image/jpeg',
         'ico' => 'image/x-icon',
+        'svg' => 'image/svg+xml',
     ];
 
     /**
@@ -71,15 +72,15 @@ public function __construct(NanoApp $app)
         $config = $this->app->getConfig();
 
         $this->cb = intval($config->get('assets.cb', 1));
-        $this->cdnPath = $config->get('assets.cdnPath', false);
+        $this->cdnPath = $config->get('assets.cdnPath', default: null);
         $this->prependCacheBuster = $config->get('assets.prependCacheBuster', true) === true;
         $this->packages = $config->get('assets.packages', []);
     }
 
     /**
      * Turn debug mode on/off
      */
-    public function setDebugMode($inDebugMode)
+    public function setDebugMode($inDebugMode): void
     {
         $this->debugMode = ($inDebugMode != false);
 
@@ -91,7 +92,7 @@ public function setDebugMode($inDebugMode)
     /**
      * Is debug mode on?
      */
-    public function inDebugMode()
+    public function inDebugMode(): bool
     {
         return $this->debugMode === true;
     }
@@ -103,7 +104,7 @@ public function inDebugMode()
      * @return StaticAssetsProcessor
      * @throws Exception
      */
-    public function getProcessor($assetType)
+    public function getProcessor(string $assetType): StaticAssetsProcessor
     {
         $className = sprintf('StaticAssets%s', ucfirst($assetType));
 
@@ -117,33 +118,33 @@ public function getProcessor($assetType)
     /**
      * Get current cache buster value (used to invalidate cached assets)
      */
-    public function getCacheBuster()
+    public function getCacheBuster(): int
     {
         return $this->cb;
     }
 
     /**
      * Get path to CDN host
      *
-     * @return string|false CDN path or false if not defined
+     * @return string|null CDN path or false if not defined
      */
-    public function getCDNPath()
+    public function getCDNPath(): ?string
     {
         return $this->cdnPath;
     }
 
     /**
      * Returns whether given package exists
      */
-    public function packageExists($packageName)
+    public function packageExists($packageName): bool
     {
         return isset($this->packages[$packageName]);
     }
 
     /**
      * Get list of assets of given type from given packages
      */
-    private function getPackagesItems(array $packagesNames, $type)
+    private function getPackagesItems(array $packagesNames, $type): bool|array
     {
         $assets = [];
 
@@ -163,7 +164,7 @@ private function getPackagesItems(array $packagesNames, $type)
     /**
      * Get list of external assets of given type from given packages
      */
-    private function getPackagesExternalItems(array $packagesNames, $type)
+    private function getPackagesExternalItems(array $packagesNames, $type): bool|array
     {
         $assets = [];
 
@@ -182,8 +183,11 @@ private function getPackagesExternalItems(array $packagesNames, $type)
 
     /**
      * Remove packages with no assets of a given type
+     *
+     * @param string[] $packages
+     * @return string[]
      */
-    public function filterOutEmptyPackages(array $packages, $type)
+    public function filterOutEmptyPackages(array $packages, $type): array
     {
         $ret = [];
 
@@ -203,7 +207,7 @@ public function filterOutEmptyPackages(array $packages, $type)
      *
      * Dependencies are returned before provided packages to maintain correct loading order
      */
-    public function resolveDependencies(array $packages)
+    public function resolveDependencies(array $packages): bool|array
     {
         $ret = $packages;
 
@@ -226,17 +230,15 @@ public function resolveDependencies(array $packages)
         }
 
         // make array contains unique values and fix indexing
-        $ret = array_values(array_unique($ret));
-
-        return $ret;
+        return array_values(array_unique($ret));
     }
 
     /**
      * Get package name from given path
      */
-    public function getPackageName($path)
+    public function getPackageName($path): bool|string
     {
-        if (strpos($path, self::PACKAGE_URL_PREFIX) === 0) {
+        if (str_starts_with($path, self::PACKAGE_URL_PREFIX)) {
             // remove package URL prefix
             $path = substr($path, strlen(self::PACKAGE_URL_PREFIX));
 
@@ -255,7 +257,7 @@ public function getPackageName($path)
      */
     private function preprocessRequestPath($path)
     {
-        if (strpos($path, '/r') === 0) {
+        if (str_starts_with($path, '/r')) {
             $path = preg_replace('#^/r\d+#', '', $path);
         }
 
@@ -265,7 +267,7 @@ private function preprocessRequestPath($path)
     /**
      * Get full local path from request's path to given asset
      */
-    public function getLocalPath($path)
+    public function getLocalPath($path): string
     {
         return $this->localRoot . $this->preprocessRequestPath($path);
     }
@@ -276,7 +278,7 @@ public function getLocalPath($path)
     public function getUrlForAsset($asset)
     {
         // check for external assets
-        if (strpos($asset, 'http') === 0) {
+        if (str_starts_with($asset, 'http')) {
             return $asset;
         }
 
@@ -306,7 +308,7 @@ public function getUrlForAsset($asset)
         // perform a rewrite for CDN
         $cdnPath = $this->getCDNPath();
 
-        if ($cdnPath !== false) {
+        if (is_string($cdnPath)) {
             $prefix = $this->router->getPathPrefix();
             $url = $cdnPath . Router::SEPARATOR . substr($url, strlen($prefix));
         }
@@ -370,8 +372,9 @@ public function getUrlsForPackages(array $packages, $type)
      * Serve given request for a static asset / package
      *
      * This is an entry point
+     * @throws Exception
      */
-    public function serve(Request $request)
+    public function serve(Request $request): bool
     {
         $ext = $request->getExtension();
         $response = $this->app->getResponse();
@@ -415,7 +418,7 @@ public function serve(Request $request)
         $response->setContent($content);
 
         // caching
-        // @see @see http://developer.yahoo.com/performance/rules.html
+        // @see http://developer.yahoo.com/performance/rules.html
         $response->setCacheDuration(30 * 86400 /* a month */);
         return true;
     }
@@ -424,8 +427,9 @@ public function serve(Request $request)
      * Serve single static asset
      *
      * Performs additional checks and returns minified version of an asset
+     * @throws Exception
      */
-    private function serveSingleAsset($requestPath, $ext)
+    private function serveSingleAsset($requestPath, $ext): bool|string
     {
         // get local path to the asset
         $localPath = $this->getLocalPath($requestPath);
@@ -462,8 +466,9 @@ private function serveSingleAsset($requestPath, $ext)
 
     /**
      * Serve package(s) of static assets
+     * @throws Exception
      */
-    private function servePackage($package, $ext)
+    private function servePackage($package, $ext): bool|string
     {
         if (!in_array($ext, [self::PACKAGE_CSS, self::PACKAGE_JS])) {
             $this->debug->log("Package can only be JS or CSS package");

tests/StaticAssetsTest.php

@@ -88,6 +88,8 @@ public function testServeTypeCheck()
             '/statics/reset.css' => true,
             '/statics/blank.gif' => true,
             '/statics/rss.png' => true,
+            '/statics/favicon.ico' => true,
+            '/statics/favicon.svg' => true,
             // package
             '/package/core.js' => true,
             '/package/foo.css' => true,
@@ -98,8 +100,8 @@ public function testServeTypeCheck()
             $static = $this->getStaticAssets();
             $response = $this->app->getResponse();
 
-            $this->assertEquals($expected, $static->serve($request));
-            $this->assertEquals($expected ? Response::OK : Response::NOT_IMPLEMENTED, $response->getResponseCode());
+            $this->assertEquals($expected, $static->serve($request), "The StaticAssetts::serve('{$asset}') should return " . json_encode($expected));
+            $this->assertEquals($expected ? Response::OK : Response::NOT_IMPLEMENTED, $response->getResponseCode(), 'Response code should maych');
         }
     }
 
@@ -229,7 +231,7 @@ public function testGetUrlForAssetAndPackage()
         $static = $this->getStaticAssets();
         $cb = $static->getCacheBuster();
 
-        $this->assertFalse($static->getCDNPath());
+        $this->assertNull($static->getCDNPath());
 
         $this->assertEquals("/site/r{$cb}/statics/jquery.foo.js", $static->getUrlForAsset('/statics/jquery.foo.js'));
         $this->assertEquals(["/site/r{$cb}/package/core.js"], $static->getUrlsForPackage('core', 'js'));
@@ -259,20 +261,20 @@ public function testGetUrlForFile()
 
         $root = $this->app->getDirectory();
 
-        $this->assertFalse($static->getCDNPath());
+        $this->assertNull($static->getCDNPath());
 
         $this->assertEquals("/site/r{$cb}/statics/head.js", $static->getUrlForFile($root . '/statics/head.js'));
     }
 
     public function testGetUrlForAssetAndPackageWithCDN()
     {
-        $cdnPath = 'http://cdn.net/sitepath';
+        $cdnPath = 'https://cdn.net/sitepath';
         $this->app->getConfig()->set('assets.cdnPath', $cdnPath);
 
         $static = $this->getStaticAssets();
         $cb = $static->getCacheBuster();
 
-        $this->assertEquals($cdnPath, $static->getCDNPath());
+        $this->assertSame($cdnPath, $static->getCDNPath());
 
         $this->assertEquals("{$cdnPath}/r{$cb}/statics/jquery.foo.js", $static->getUrlForAsset('/statics/jquery.foo.js'));
         $this->assertEquals(["{$cdnPath}/r{$cb}/package/core.js"], $static->getUrlsForPackage('core', 'js'));
@@ -299,7 +301,7 @@ public function testGetUrlForExternalPackage()
         $static = $this->getStaticAssets();
         $cb = $static->getCacheBuster();
 
-        $this->assertFalse($static->getCDNPath());
+        $this->assertNull($static->getCDNPath());
 
         $this->assertEquals([
             'http://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js',