Private method getFilePath()

[candrianarijaona]

I need to customize the file path of the cache key, why not to put this method to protected, so we can just overload it in a child class?

[prisis]

Why you need to customize the file path?

[candrianarijaona]

For example to allow to store the file on a subdirectory, with a cache key containing "/".

[prisis]

But you can set the path in __construct or with setFolder to your subdirectory

[candrianarijaona]

The setFolder sets your global directory. Think about this case: your global directory is /var/cache, and you want to store file in /var/cache/article/ ?

[Nyholm]

I would recommend against that. That would make the pool stateful. Why not use namespace cache or hierarchy? That would achieve the same goal, right?

[cryptiklemur]

Either that, or different instances of cache

[guyradford]

I think this goes back to an issue I raised a merge request for a while ago..

The FilesystemCachePool stores files using the cache Key, but a cache Key allows characters that are not permitted by file systems, Also some file systems (Windows) are not case sensitive therefore 'ABC' and 'abc' are different cache key but could be stored in the same file on disk.

I proposed doing something like a base64 encode of the cache key to create a filename, but this would not work on windows.

In our project (we are on linux) we extended the FilesystemCachePool as below:

    /**
     * @param string $key
     *
     * @throws InvalidArgumentException
     *
     * @return string
     */
    private function getFilePath($key)
    {
        $key = str_replace('=', '.', base64_encode($key));
        if (!preg_match('|^[a-zA-Z0-9_\.! ]+$|', $key)) {
            throw new InvalidArgumentException(sprintf('Invalid key "%s". Valid keys must match [a-zA-Z0-9_\.! ].', $key));
        }

        return sprintf('%s/%s', $this->folder, $key);
    }

Hope that helps a little @candrianarijaona

Guy

[candrianarijaona]

Thanks for your answer @guyradford . The problem is even if you extend the class FilesystemCachePool, you cannot override the method getFilePath as it is private. If it was protected, then it's ok.

[guyradford]

Good point, probably explains why we hadn't extended it as planned but implemented the class is full :(

On 17 Mar 2017 15:23, "Claude Andrianarijaona" notifications@github.com wrote:

Thanks for your answer @guyradford https://github.com/guyradford . The problem is even if you extend the class FilesystemCachePool, you cannot override the method getFilePath as it is private. If it was protected, then it's ok.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/php-cache/issues/issues/93#issuecomment-287384566, or mute the thread https://github.com/notifications/unsubscribe-auth/AChLQJAjwlMEEvuBiSiuUramPx0-dhysks5rmqVwgaJpZM4MNJuq .

[Nyholm]

What character that passes through here is not supported on file system?

[candrianarijaona]

It's not about a special character. But here is what I've done to allow us to save cache file on a subdirectory

/**
 * @param string $key
 *
 * @throws InvalidArgumentException
 *
 * @return string
 */
private function getFilePath($key)
{
    if (!preg_match('|^[a-zA-Z0-9_\-\.! ]+$|', $key)) {
        throw new InvalidArgumentException(sprintf('Invalid key "%s". Valid keys must match [a-zA-Z0-9_\.! ].', $key));
    }

    $cacheDir = str_replace('_', '/', $key);

    return sprintf('%s/%s', $this->folder, $cacheDir);
 }

[guyradford]

@Nyholm

I wrote a test to test the special characters in CacheKey against the FileSystemPool.

<?php
namespace Tests\Common\ServiceProviders;

use Cache\Adapter\Common\CacheItem;
use Cache\Adapter\Filesystem\FilesystemCachePool;
use League\Flysystem\Adapter\Local;
use League\Flysystem\Filesystem;
use PHPUnit_Framework_TestCase;

class CacheKeyWithFileSystemPoolTest extends PHPUnit_Framework_TestCase
{
    public function dataProvider_cacheKey(){
        return [
          ['_'],
          ['\\'],
          ['.'],
          ['!'],
          [' '],

          ['key_'],
          ['key\\'],
          ['key.'],
          ['key!'],
          ['key '],

          ['_key'],
          ['\\key'],
          ['.key'],
          ['!key'],
          [' key'],

          ['key_key'],
          ['key\\key'],
          ['key.key'],
          ['key!key'],
          ['key key'],
        ];
    }

    /**
     * @param string $key
     *
     * @dataProvider dataProvider_cacheKey
     */
    public function test_validKey_which_fail_in_fileCache($key)
    {

        $filesystemAdapter = new Local(realpath(ROOT . 'cache/fileCache'));
        $filesystem        = new Filesystem($filesystemAdapter);
        $this->cache = new FilesystemCachePool($filesystem);

        $itemToSave = (new CacheItem($key))
            ->set('value')
            ->expiresAfter(2);

        $this->assertTrue($this->cache->save($itemToSave));

    }
}

Results:

vendor/bin/phpunit tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php
PHPUnit 4.7.3 by Sebastian Bergmann and contributors.

.EE...E....E...

Time: 9.61 seconds, Memory: 17.00MB

There were 4 errors:

1) Tests\Common\ServiceProviders\CacheKeyWithFileSystemPoolTest::test_validKey_which_fail_in_fileCache with data set #1 ('\')
Cache\Adapter\Common\Exception\InvalidArgumentException: Invalid key "\". Valid filenames must match [a-zA-Z0-9_\.! ].

/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:155
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:131
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:191
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:62

2) Tests\Common\ServiceProviders\CacheKeyWithFileSystemPoolTest::test_validKey_which_fail_in_fileCache with data set #2 ('.')
Cache\Adapter\Common\Exception\CachePoolException: Exception thrown when executing "save".

/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:283
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:193
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:62

Caused by
PHPUnit_Framework_Error_Warning: file_put_contents(/var/www/api/cache/fileCache/cache): failed to open stream: Is a directory

/var/www/api/vendor/league/flysystem/src/Adapter/Local.php:196
/var/www/api/vendor/league/flysystem/src/Filesystem.php:152
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:134
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:191
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:62

3) Tests\Common\ServiceProviders\CacheKeyWithFileSystemPoolTest::test_validKey_which_fail_in_fileCache with data set #6 ('key\')
Cache\Adapter\Common\Exception\InvalidArgumentException: Invalid key "key\". Valid filenames must match [a-zA-Z0-9_\.! ].

/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:155
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:131
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:191
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:62

4) Tests\Common\ServiceProviders\CacheKeyWithFileSystemPoolTest::test_validKey_which_fail_in_fileCache with data set #11 ('\key')
Cache\Adapter\Common\Exception\InvalidArgumentException: Invalid key "\key". Valid filenames must match [a-zA-Z0-9_\.! ].

/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:155
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:131
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:191
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:62

5) Tests\Common\ServiceProviders\CacheKeyWithFileSystemPoolTest::test_validKey_which_fail_in_fileCache with data set #16 ('key\key')
Cache\Adapter\Common\Exception\InvalidArgumentException: Invalid key "key\key". Valid filenames must match [a-zA-Z0-9_\.! ].

/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:155
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:131
/var/www/api/vendor/cache/adapter-common/AbstractCachePool.php:191
/var/www/api/vendor/cache/filesystem-adapter/FilesystemCachePool.php:170
/var/www/api/tests/Common/ServiceProviders/CacheKeyWithFileSystemPoolTest.php:64

As you can see it definitely did not like the \ in any part of the key or the . by its self.

This was on Debian 8.

[Nyholm]

Thank you. It seams about right.

The following are not valid: "\", "key\" and "\key". Number 2 is more interesting. We should have a test and handle that.

[pmercier]

Being able to specify a path based on the cache key would be a great addition for performance issues.

When using a file system cache on big web sites (around 200'000 unique documents) storing all the files inside one directory has an heavy impact on response time. A case that append frequently where is work.

Accessing a lone file inside a folder is usally something around 2 to 5ms. Accessing a file inside a folder with more thant 100'000 files inside is more like 5 seconds. For a cache system it's not very efficient.

Perhaps a stupid idea but here it's : instead of overriding the getFilePath, an inflector can be added, something like : setFilePathParser(callable $parser)

to achieve this :

    private function getFilePath($key)
    {
        if (!preg_match('|^[a-zA-Z0-9_\.! ]+$|', $key)) {
            throw new InvalidArgumentException(sprintf('Invalid key "%s". Valid filenames must match [a-zA-Z0-9_\.! ].', $key));
        }
        $path = $this->filePathParser ? $key : $this->filePathParser($key);
        return sprintf('%s/%s', $this->folder, $path);
    }

[pmercier]

@guyradford, @Nyholm Actually these special files names are prohibed on windows systems :

CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, LPT9

[joshbruce]

Think mine is along these lines as well, or maybe this is just a bad way to do things (new to caches in general and this library in particular). I will try to keep it brief, if it makes sense to be a separate topic/thread.

I'm integrating with GitHub using KnpLabs/php-github-api.

I would like the app I'm building to have the ability to switch between local storage or GitHub as a general user setting.

The system I've built (and is working) for local file storage works like a charm.

Now I'm setting up the cache concept and believe I would need to modify my app code to handle these two setups in really different ways, or lay aside the local storage concept (GitHub only) for a moment.

Believe setFolder might be useful when making the request, but not sure.