PHPSocialNetwork/phpfastcache-bundle

"Calling many times CacheManager::getInstance()" during tests with dataProvider

r0b- opened this issue · 8 comments

r0b- commented

Hi,

my controller test cases fail if they have a dataProvider with more than 4 items (using Symfony's WebTestCase).
Error is:
app.ERROR: [Files] Calling many times CacheManager::getInstance() for already instanced drivers is a bad practice and have a significant impact on performances....

The cache instance getter is called through smyfony's DI when the controller get's instantiated.

So is it possible to disable the $badPracticeOmeter somehow or increase the limit or...?
Also I tried to call CacheManager::clearInstances on test teardown but it seems to be buggy (the instances array itself is not cleared so the $badPracticeOmeter counter is always incremented) :-(

Thanks
Rob

Hi Rob,

You you show me your code implementation in private please (Ore publicly if you want too).

Thanks you.

r0b- commented

This is the service wrapping phpfastcache:

class MyCacheService
{
    /**
     * @var ExtendedCacheItemPoolInterface
     */
    private $cacheInstance;

    public function __construct(Cache $cache)
    {
        $this->cacheInstance = $cache->get("my_cache");
    }
...

This is the controller using the service:

    public function indexAction(Request $request)
    {
            $myCacheService = $this->get("myproject.my_cache");
            ....
    }

And this is the test:

class IndexControllerTest extends WebTestCase
{
    /** @var Client */
    private $client;

    public function setUp()
    {
        $this->client = static::createClient(["debug" => true]);
    }

    public function routeProvider()
    {
        return [
            ["/a"], ["/b"], ["/c"], ["/d"], ["/e"]
        ];
    }

    /**
     * @dataProvider routeProvider
     */
    public function myTest($route)
    {
        $this->client->request("GET", $route);
        $this->assertTrue($this->client->getResponse()->isOk());
    }

For a reason that i ignore you call CacheManager multiple time and this is a bad practice.
Did you read this Wiki ?
https://github.com/PHPSocialNetwork/phpfastcache/wiki/%5BV5%5D-Why-calling-getInstance()-each-time-is-a-bad-practice-%3F

You must store and keep your cache instances :)

Also making a wrapper to make get/set syntax will considerably affect your application performances.
PhpFastCache is itself an abstract wrapper around drivers themselves. I strongly encourage you to use the Psr6 guideline.

r0b- commented

hmmmm, of course I've read the wiki, but the controller lifecycle is controlled by symfony during the tests.

However, is it possible that you fix the CacheManager::clearInstances method so that it actually clears the instances array (just add a self::$instances = [];)? That would help me a lot!

Can you go inside the phpfastcache vendor and alter the ClearInstance method like this please ?

from:

        foreach (self::$instances as &$instance) {
            unset($instance);
        }

        gc_collect_cycles();
        return !count(self::$instances);

to:

        foreach (self::$instances as $key => $instance) {
            unset(self::$instances[$key]);
        }

        gc_collect_cycles();
        return !count(self::$instances);

If it work i'll push the fix asap.
Or you can Pr also if you know how to do it :)

r0b- commented

Created a PR...
Thanks in advance for merging!

Merged, thanks you :)