Merge branch '3.x-RouterCacheFileImprovement' into 3.x

Closes #2586 Fixes #2568
slimphp · Feb 22, 2019 · 04e7689 · 04e7689
2 parents e97d122 + b4e7bbf
commit 04e7689
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 13 deletions.
diff --git a/Slim/Router.php b/Slim/Router.php
@@ -131,16 +131,22 @@ public function getBasePath()
     public function setCacheFile($cacheFile)
     {
         if (!is_string($cacheFile) && $cacheFile !== false) {
-            throw new InvalidArgumentException('Router cacheFile must be a string or false');
+            throw new InvalidArgumentException('Router cache file must be a string or false');
         }
 
-        $this->cacheFile = $cacheFile;
-
-        if ($cacheFile !== false && !is_writable(dirname($cacheFile))) {
-            throw new RuntimeException('Router cacheFile directory must be writable');
+        if ($cacheFile && file_exists($cacheFile) && !is_readable($cacheFile)) {
+            throw new RuntimeException(
+                sprintf('Router cache file `%s` is not readable', $cacheFile)
+            );
         }
 
+        if ($cacheFile && !file_exists($cacheFile) && !is_writable(dirname($cacheFile))) {
+            throw new RuntimeException(
+                sprintf('Router cache file directory `%s` is not writable', dirname($cacheFile))
+            );
+        }
 
+        $this->cacheFile = $cacheFile;
         return $this;
     }
 

diff --git a/composer.json b/composer.json
@@ -49,7 +49,7 @@
     },
     "autoload-dev": {
         "files": [
-          "tests/Assets/HeaderFunctions.php"
+          "tests/Assets/PhpFunctionOverrides.php"
         ]
     },
     "scripts": {

diff --git a/tests/Assets/HeaderFunctions.php → tests/Assets/PhpFunctionOverrides.php b/tests/Assets/HeaderFunctions.php → tests/Assets/PhpFunctionOverrides.php
@@ -57,3 +57,31 @@ function header($string, $replace = true, $statusCode = null)
         ]
     );
 }
+
+/**
+ * Is a file descriptor writable
+ *
+ * @param $file
+ * @return bool
+ */
+function is_readable($file)
+{
+    if (stripos($file, 'non-readable.cache') !== false) {
+        return false;
+    }
+    return true;
+}
+
+/**
+ * Is a path writable
+ *
+ * @param $path
+ * @return bool
+ */
+function is_writable($path)
+{
+    if (stripos($path, 'non-writable-directory') !== false) {
+        return false;
+    }
+    return true;
+}
diff --git a/tests/RouterTest.php b/tests/RouterTest.php
@@ -6,21 +6,31 @@
  * @copyright Copyright (c) 2011-2017 Josh Lockhart
  * @license   https://github.com/slimphp/Slim/blob/3.x/LICENSE.md (MIT License)
  */
+
 namespace Slim\Tests;
 
-use Slim\Container;
 use Slim\Router;
 
 class RouterTest extends \PHPUnit_Framework_TestCase
 {
     /** @var Router */
     protected $router;
 
+    /** @var string */
+    protected $cacheFile;
+
     public function setUp()
     {
         $this->router = new Router;
     }
 
+    public function tearDown()
+    {
+        if (file_exists($this->cacheFile)) {
+            unlink($this->cacheFile);
+        }
+    }
+
     public function testMap()
     {
         $methods = ['GET'];
@@ -333,24 +343,40 @@ public function testSettingInvalidCacheFileValue()
     {
         $this->setExpectedException(
             '\InvalidArgumentException',
-            'Router cacheFile must be a string'
+            'Router cache file must be a string'
         );
         $this->router->setCacheFile(['invalid']);
     }
 
     /**
-     * Test if cacheFile is not a writable directory
+     * Test cache file exists but is not writable
      */
-    public function testSettingInvalidCacheFileNotExisting()
+    public function testCacheFileExistsAndIsNotReadable()
     {
+        $this->cacheFile = __DIR__ . '/non-readable.cache';
+        file_put_contents($this->cacheFile, '<?php return []; ?>');
+
         $this->setExpectedException(
             '\RuntimeException',
-            'Router cacheFile directory must be writable'
+            sprintf('Router cache file `%s` is not readable', $this->cacheFile)
         );
 
-        $this->router->setCacheFile(
-            dirname(__FILE__) . uniqid(microtime(true)) . '/' . uniqid(microtime(true))
+        $this->router->setCacheFile($this->cacheFile);
+    }
+
+    /**
+     * Test cache file does not exist and directory is not writable
+     */
+    public function testCacheFileDoesNotExistsAndDirectoryIsNotWritable()
+    {
+        $cacheFile = __DIR__ . '/non-writable-directory/router.cache';
+
+        $this->setExpectedException(
+            '\RuntimeException',
+            sprintf('Router cache file directory `%s` is not writable', dirname($cacheFile))
         );
+
+        $this->router->setCacheFile($cacheFile);
     }
 
     /**