Asset pipeline performance improvements (#1003)

- Do not minify files whose filename already indicates that they are minified (.min.js or .min.css)
- Use file locking to avoid redundant minifaction work

module/VuFindTheme/src/VuFindTheme/View/Helper/ConcatTrait.php

@@ -6,6 +6,7 @@
  * PHP version 5
  *
  * Copyright (C) Villanova University 2016.
+ * Copyright (C) The National Library of Finland 2017.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2,
@@ -23,6 +24,7 @@
  * @category VuFind
  * @package  View_Helpers
  * @author   Demian Katz <demian.katz@villanova.edu>
+ * @author   Ere Maijala <ere.maijala@helsinki.fi>
  * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
  * @link     https://vufind.org/wiki/development Wiki
  */
@@ -36,6 +38,7 @@ use VuFindTheme\ThemeInfo;
  * @category VuFind
  * @package  View_Helpers
  * @author   Demian Katz <demian.katz@villanova.edu>
+ * @author   Ere Maijala <ere.maijala@helsinki.fi>
  * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
  * @link     https://vufind.org/wiki/development:testing:unit_tests Wiki
  */
@@ -228,20 +231,84 @@ trait ConcatTrait
         $filename = md5($group['key']) . '.min.' . $this->getFileType();
         $concatPath = $this->getResourceCacheDir() . $filename;
         if (!file_exists($concatPath)) {
-            $minifier = $this->getMinifier();
-            foreach ($group['items'] as $item) {
-                $details = $this->themeInfo->findContainingTheme(
-                    $this->getFileType() . '/' . $this->getResourceFilePath($item),
-                    ThemeInfo::RETURN_ALL_DETAILS
-                );
-                $minifier->add($details['path']);
+            $lockfile = "$concatPath.lock";
+            $handle = fopen($lockfile, 'c+');
+            if (!is_resource($handle)) {
+                throw new \Exception("Could not open lock file $lockfile");
+            }
+            if (!flock($handle, LOCK_EX)) {
+                fclose($handle);
+                throw new \Exception("Could not lock file $lockfile");
             }
-            $minifier->minify($concatPath);
+            // Check again if file exists after acquiring the lock
+            if (!file_exists($concatPath)) {
+                try {
+                    $this->createConcatenatedFile($concatPath, $group);
+                } catch (\Exception $e) {
+                    flock($handle, LOCK_UN);
+                    fclose($handle);
+                    throw $e;
+                }
+            }
+            flock($handle, LOCK_UN);
+            fclose($handle);
         }
 
         return $urlHelper('home') . 'cache/' . $filename;
     }
 
+    /**
+     * Create a concatenated file from the given group of files
+     *
+     * @param string $concatPath Resulting file path
+     * @param array  $group      Object containing 'key' and stdobj file 'items'
+     *
+     * @throws \Exception
+     * @return void
+     */
+    protected function createConcatenatedFile($concatPath, $group)
+    {
+        $data = [];
+        foreach ($group['items'] as $item) {
+            $details = $this->themeInfo->findContainingTheme(
+                $this->getFileType() . '/'
+                . $this->getResourceFilePath($item),
+                ThemeInfo::RETURN_ALL_DETAILS
+            );
+            $data[] = $this->getMinifiedData($details, $concatPath);
+        }
+        // Separate each file's data with a new line so that e.g. a file
+        // ending in a comment doesn't cause the next one to also get commented out.
+        file_put_contents($concatPath, implode("\n", $data));
+    }
+
+    /**
+     * Get minified data for a file
+     *
+     * @param array  $details    File details
+     * @param string $concatPath Target path for the resulting file (used in minifier
+     * for path mapping)
+     *
+     * @throws \Exception
+     * @return string
+     */
+    protected function getMinifiedData($details, $concatPath)
+    {
+        if ($this->isMinifiable($details['path'])) {
+            $minifier = $this->getMinifier();
+            $minifier->add($details['path']);
+            $data = $minifier->execute($concatPath);
+        } else {
+            $data = file_get_contents($details['path']);
+            if (false === $data) {
+                throw new \Exception(
+                    "Could not read file {$details['path']}"
+                );
+            }
+        }
+        return $data;
+    }
+
     /**
      * Process and return items in index order
      *
@@ -290,6 +357,20 @@ trait ConcatTrait
         );
     }
 
+    /**
+     * Check if a file is minifiable i.e. does not have a pattern that denotes it's
+     * already minified
+     *
+     * @param string $filename File name
+     *
+     * @return bool
+     */
+    protected function isMinifiable($filename)
+    {
+        $basename = basename($filename);
+        return preg_match('/\.min\.(js|css)/', $basename) === 0;
+    }
+
     /**
      * Can we use the asset pipeline?
      *

module/VuFindTheme/src/VuFindTheme/View/Helper/HeadScript.php

@@ -39,7 +39,9 @@ use VuFindTheme\ThemeInfo;
  */
 class HeadScript extends \Zend\View\Helper\HeadScript
 {
-    use ConcatTrait;
+    use ConcatTrait {
+        getMinifiedData as getBaseMinifiedData;
+    }
 
     /**
      * Theme information service
@@ -154,4 +156,24 @@ class HeadScript extends \Zend\View\Helper\HeadScript
     {
         return new \MatthiasMullie\Minify\JS();
     }
+
+    /**
+     * Get minified data for a file
+     *
+     * @param array  $details    File details
+     * @param string $concatPath Target path for the resulting file (used in minifier
+     * for path mapping)
+     *
+     * @throws \Exception
+     * @return string
+     */
+    protected function getMinifiedData($details, $concatPath)
+    {
+        $data = $this->getBaseMinifiedData($details, $concatPath);
+        // Play it safe by terminating a script with a semicolon
+        if (substr(trim($data), -1, 1) !== ';') {
+            $data .= ';';
+        }
+        return $data;
+    }
 }