Cache/File.php : + file locking option (still need cache stress test)

min/lib/Minify.php

@@ -56,15 +56,18 @@ class Minify {
      * need to recombine files, minify and encode the output.
      *
      * @param mixed $cache object with identical interface as Minify_Cache_File or
-     * a directory path. (default = '')
+     * a directory path. (default = '')
+     * 
+     * @param bool $fileLocking (default = true) This only applies if the first
+     * parameter is a string.
      *
      * @return null
      */
-    public static function setCache($cache = '')
+    public static function setCache($cache = '', $fileLocking = true)
     {
         if (is_string($cache)) {
             require_once 'Minify/Cache/File.php';
-            self::$_cache = new Minify_Cache_File($cache);
+            self::$_cache = new Minify_Cache_File($cache, $fileLocking);
         } else {
             self::$_cache = $cache;
         }

min/lib/Minify/Cache/File.php

@@ -6,12 +6,13 @@
 
 class Minify_Cache_File {
     
-    public function __construct($path = '')
+    public function __construct($path = '', $fileLocking = false)
     {
         if (! $path) {
             require_once 'Solar/Dir.php';
             $path = rtrim(Solar_Dir::tmp(), DIRECTORY_SEPARATOR);
-        }
+        }
+        $this->_locking = $fileLocking;
         $this->_path = $path;
     }
     
@@ -26,7 +27,20 @@ class Minify_Cache_File {
      */
     public function store($id, $data)
     {
-        return self::_verifiedWrite($this->_path . '/' . $id, $data);
+        $flag = $this->_locking
+            ? LOCK_EX
+            : null;
+        if (is_file($this->_path . '/' . $id)) {
+            @unlink($this->_path . '/' . $id);
+        }
+        if (! @file_put_contents($this->_path . '/' . $id, $data, $flag)) {
+            return false;
+        }
+        if ($data !== $this->fetch($id)) {
+            @unlink($file);
+            return false;
+        }
+        return true;
     }
     
     /**
@@ -53,7 +67,7 @@ class Minify_Cache_File {
     public function isValid($id, $srcMtime)
     {
         $file = $this->_path . '/' . $id;
-        return (file_exists($file) && (filemtime($file) >= $srcMtime));
+        return (is_file($file) && (filemtime($file) >= $srcMtime));
     }
     
     /**
@@ -63,7 +77,15 @@ class Minify_Cache_File {
      */
     public function display($id)
     {
-        readfile($this->_path . '/' . $id);
+        if ($this->_locking) {
+            $fp = fopen($this->_path . '/' . $id, 'rb');
+            flock($fp, LOCK_SH);
+            fpassthru($fp);
+            flock($fp, LOCK_UN);
+            fclose($fp);
+        } else {
+            readfile($this->_path . '/' . $id);            
+        }
     }
     
 	/**
@@ -75,29 +97,18 @@ class Minify_Cache_File {
      */
     public function fetch($id)
     {
-        return file_get_contents($this->_path . '/' . $id);
+        if ($this->_locking) {
+            $fp = fopen($this->_path . '/' . $id, 'rb');
+            flock($fp, LOCK_SH);
+            $ret = stream_get_contents($fp);
+            flock($fp, LOCK_UN);
+            fclose($fp);
+            return $ret;
+        } else {
+            return file_get_contents($this->_path . '/' . $id);
+        }
     }
     
     private $_path = null;
-    
+    private $_locking = null;
-	/**
-     * Write data to file and verify its contents
-     * 
-     * @param string $file path
-     * 
-     * @param string $data
-     * 
-     * @return bool success
-     */
-    private static function _verifiedWrite($file, $data)
-    {
-        if (! @file_put_contents($file, $data)) {
-            return false;
-        }
-        if (md5($data) !== md5_file($file)) {
-            @unlink($file);
-            return false;
-        }
-        return true;
-    }
 }

min_extras/unit_tests/test_Minify_Cache_File.php

@@ -5,8 +5,8 @@ require_once 'Minify/Cache/File.php';
 
 function test_Minify_Cache_File()
 {
-    $data = str_repeat(md5('testing'), 160);
+    $data = str_repeat(md5(time()), 160);
-    $id = 'Minify_test_cache';
+    $id = 'Minify_test_cache_noLock';
     $prefix = 'Minify_Cache_File : ';
     
     $cache = new Minify_Cache_File();
@@ -24,7 +24,27 @@ function test_Minify_Cache_File()
     
     assertTrue($data === $displayed, $prefix . 'display');
     
-    assertTrue($data === $cache->fetch($id), $prefix . 'fetch');
+    assertTrue($data === $cache->fetch($id), $prefix . 'fetch');
+    
+    // test with locks
+    
+    $id = 'Minify_test_cache_withLock';
+    $cache = new Minify_Cache_File('', true);
+    
+    assertTrue(true === $cache->store($id, $data), $prefix . 'store w/ lock');
+    
+    assertTrue(strlen($data) === $cache->getSize($id), $prefix . 'getSize');
+    
+    assertTrue(true === $cache->isValid($id, $_SERVER['REQUEST_TIME'] - 10), $prefix . 'isValid');
+    
+    ob_start();
+    $cache->display($id);
+    $displayed = ob_get_contents();
+    ob_end_clean();
+    
+    assertTrue($data === $displayed, $prefix . 'display w/ lock');
+    
+    assertTrue($data === $cache->fetch($id), $prefix . 'fetch w/ lock');
 }
 
 test_Minify_Cache_File();