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

Closes #2588
slimphp · Mar 2, 2019 · 177b3d1 · 177b3d1
2 parents 7dc705d + f735bb6
commit 177b3d1
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 48 deletions.
diff --git a/Slim/Router.php b/Slim/Router.php
@@ -182,12 +182,19 @@ public function setBasePath(string $basePath): self
      */
     public function setCacheFile(?string $cacheFile): RouterInterface
     {
-        $this->cacheFile = $cacheFile;
+        if ($cacheFile && file_exists($cacheFile) && !is_readable($cacheFile)) {
+            throw new RuntimeException(
+                sprintf('Router cache file `%s` is not readable', $cacheFile)
+            );
+        }
 
-        if (is_string($cacheFile) && !is_writable(dirname($cacheFile))) {
-            throw new RuntimeException('Router cacheFile directory must be writable');
+        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
@@ -54,7 +54,7 @@
     },
     "autoload-dev": {
         "files": [
-            "tests/Assets/HeaderFunctions.php"
+            "tests/Assets/PhpFunctionOverrides.php"
         ]
     },
     "scripts": {

diff --git a/tests/Assets/HeaderFunctions.php → tests/Assets/HeaderStack.php b/tests/Assets/HeaderFunctions.php → tests/Assets/HeaderStack.php
@@ -6,7 +6,7 @@
  * We put these into the Slim namespace, so that Slim\App will use these versions of header() and
  * headers_sent() when we test its output.
  */
-namespace Slim;
+namespace Slim\Tests\Assets;
 
 /**
  * Zend Framework (http://framework.zend.com/)
@@ -31,7 +31,7 @@
 /**
  * Store output artifacts
  */
-class HeaderStackTestAsset
+class HeaderStack
 {
     /**
      * @var string[][]
@@ -84,31 +84,3 @@ public static function has($header)
         return false;
     }
 }
-
-/**
- * Have headers been sent?
- *
- * @return false
- */
-function headers_sent()
-{
-    return false;
-}
-
-/**
- * Emit a header, without creating actual output artifacts
- *
- * @param string   $string
- * @param bool     $replace
- * @param int|null $statusCode
- */
-function header($string, $replace = true, $statusCode = null)
-{
-    HeaderStackTestAsset::push(
-        [
-            'header'      => $string,
-            'replace'     => $replace,
-            'status_code' => $statusCode,
-        ]
-    );
-}
diff --git a/tests/Assets/PhpFunctionOverrides.php b/tests/Assets/PhpFunctionOverrides.php
@@ -0,0 +1,68 @@
+<?php
+/**
+ * Override PHP namespaced functions into the Slim\App namespace.
+ *
+ * header() and headers_sent() are from Zend-Diactoros zend-diactoros/test/TestAsset/Functions.php and are
+ * Copyright (c) 2015-2016 Zend Technologies USA Inc. (http://www.zend.com) and are licensed under the New BSD License
+ * at https://github.com/zendframework/zend-diactoros/blob/master/LICENSE.md.
+ *
+ */
+namespace Slim;
+
+use Slim\Tests\Assets\HeaderStack;
+
+/**
+ * Have headers been sent?
+ *
+ * @return false
+ */
+function headers_sent()
+{
+    return false;
+}
+
+/**
+ * Emit a header, without creating actual output artifacts
+ *
+ * @param string   $string
+ * @param bool     $replace
+ * @param int|null $statusCode
+ */
+function header($string, $replace = true, $statusCode = null)
+{
+    HeaderStack::push(
+        [
+            'header'      => $string,
+            'replace'     => $replace,
+            'status_code' => $statusCode,
+        ]
+    );
+}
+
+/**
+ * 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/ResponseEmitterTest.php b/tests/ResponseEmitterTest.php
@@ -8,8 +8,8 @@
  */
 namespace Slim\Tests;
 
-use Slim\HeaderStackTestAsset;
 use Slim\ResponseEmitter;
+use Slim\Tests\Assets\HeaderStack;
 use Slim\Tests\Mocks\MockStream;
 use Slim\Tests\Mocks\SmallChunksStream;
 
@@ -21,12 +21,12 @@ class ResponseEmitterTest extends TestCase
 {
     public function setUp()
     {
-        HeaderStackTestAsset::reset();
+        HeaderStack::reset();
     }
 
     public function tearDown()
     {
-        HeaderStackTestAsset::reset();
+        HeaderStack::reset();
     }
 
     public function testRespond()
@@ -47,8 +47,8 @@ public function testRespondNoContent()
         $responseEmitter = new ResponseEmitter();
         $responseEmitter->emit($response);
 
-        $this->assertEquals(false, HeaderStackTestAsset::has('Content-Type'));
-        $this->assertEquals(false, HeaderStackTestAsset::has('Content-Length'));
+        $this->assertEquals(false, HeaderStack::has('Content-Type'));
+        $this->assertEquals(false, HeaderStack::has('Content-Length'));
         $this->expectOutputString('');
     }
 
@@ -143,7 +143,7 @@ public function testResponseReplacesPreviouslySetHeaders()
             ['header' => 'HTTP/1.1 200 OK', 'replace' => true, 'status_code' => 200],
         ];
 
-        $this->assertSame($expectedStack, HeaderStackTestAsset::stack());
+        $this->assertSame($expectedStack, HeaderStack::stack());
     }
 
     public function testResponseDoesNotReplacePreviouslySetSetCookieHeaders()
@@ -161,7 +161,7 @@ public function testResponseDoesNotReplacePreviouslySetSetCookieHeaders()
             ['header' => 'HTTP/1.1 200 OK', 'replace' => true, 'status_code' => 200],
         ];
 
-        $this->assertSame($expectedStack, HeaderStackTestAsset::stack());
+        $this->assertSame($expectedStack, HeaderStack::stack());
     }
 
     public function testIsResponseEmptyWithNonEmptyBodyAndTriggeringStatusCode()

diff --git a/tests/RouterTest.php b/tests/RouterTest.php
@@ -23,6 +23,16 @@ class RouterTest extends TestCase
     /** @var Router */
     protected $router;
 
+    /** @var string */
+    protected $cacheFile;
+
+    public function tearDown()
+    {
+        if (file_exists($this->cacheFile)) {
+            unlink($this->cacheFile);
+        }
+    }
+
     public function setUp()
     {
         $callableResolver = new CallableResolver();
@@ -315,16 +325,33 @@ public function testPathForWithModifiedRoutePattern()
     }
 
     /**
-     * Test if cacheFile is not a writable directory
-     *
-     * @expectedException \RuntimeException
-     * @expectedExceptionMessage Router cacheFile directory must be writable
+     * Test cache file exists but is not writable
      */
-    public function testSettingInvalidCacheFileNotExisting()
+    public function testCacheFileExistsAndIsNotReadable()
     {
-        $this->router->setCacheFile(
-            dirname(__FILE__) . uniqid(microtime(true)) . '/' . uniqid(microtime(true))
+        $this->cacheFile = __DIR__ . '/non-readable.cache';
+        file_put_contents($this->cacheFile, '<?php return []; ?>');
+
+        $this->expectException(
+            '\RuntimeException',
+            sprintf('Router cache file `%s` is not readable', $this->cacheFile)
         );
+
+        $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->expectException(
+            '\RuntimeException',
+            sprintf('Router cache file directory `%s` is not writable', dirname($cacheFile))
+        );
+
+        $this->router->setCacheFile($cacheFile);
     }
 
     /**