[Cache] remove is_writable check on filesystem cache #21025
Conversation
There was a problem hiding this comment.
when file_put_content fails (see https://github.com/symfony/symfony/pull/21025/files#diff-fe14fffdd6799f339b2daf1045d09360R133)
we should report more info. In the else block, I think we should call error_get_last and use the message there to throw a CacheException (the AbstractAdapter will deal with it properly).
| @@ -34,14 +34,11 @@ public function __construct($namespace = '', $defaultLifetime = 0, $directory = | |||
| $directory .= '/'.$namespace; | |||
| } | |||
| if (!file_exists($dir = $directory.'/.')) { | |||
| @mkdir($directory, 0777, true); | |||
| @mkdir($directory, 0755, true); | |||
There was a problem hiding this comment.
This should be reverted: the complete code bases uses 777 when calling mkdir.
| @@ -150,7 +147,7 @@ private function getFile($id, $mkdir = false) | |||
| $dir = $this->directory.strtoupper($hash[0].DIRECTORY_SEPARATOR.$hash[1].DIRECTORY_SEPARATOR); | |||
|
|
|||
| if ($mkdir && !file_exists($dir)) { | |||
| @mkdir($dir, 0777, true); | |||
| @mkdir($dir, 0755, true); | |||
|
OK but currently, the Adding an exception in this method will conflict with the boolean return IMO. // Symfony\Component\Cache\Adapter\FilesystemAdapter::doSave
if (!$ok && !is_writable($this->directory)) {
throw new CacheException(sprintf('Cache directory is not writable (%s)', $this->directory));
} |
|
True concern, and AbstractAdapter already has the required logic to do what you say: it retries saving each key individually when batch save fails. |
|
Thank you @4rthem. |
This PR was merged into the 3.1 branch. Discussion ---------- [Cache] remove is_writable check on filesystem cache | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21024 | License | MIT | Doc PR | - * Use 755 mode for directories creation * Remove `is_writable` check in FilesystemAdapter to avoid exception on read-only filesystems Commits ------- 4dc4f69 remove is_writable check on filesystem cache
is_writablecheck in FilesystemAdapter to avoid exception on read-only filesystems