spatie / statamic-responsive-images

Responsive images for Statamic 3
MIT License
99 stars 29 forks source link

Fix placeholder svg generation failing when switching `cache` boolean value #174

Closed ncla closed 1 year ago

ncla commented 1 year ago

Fixes #171. Probably addresses #146 too.

ncla commented 1 year ago

I can't figure out how to write test for this properly. To reproduce the bug, you have to do the following:

  1. sail php please glide:clear && sail php artisan cache:clear && sail php please responsive:generate
  2. Have placeholder generated on assets.image_manipulations.cache => true by refreshing page
  3. Switch back cache to false and generate placeholder again

You should get the error reported in the original issue.

On user land, you refresh the pages for each step which in test environment is kinda difficult. GlideServiceProvider binds several things, some singletons are configured based on some config values and then bound, so there is a need to clear them. Of course there is App::forgetInstance that allows to clear these singletons, and they will be recreated when necessary. I just coulnd't get it working, I am just dumping the WIP test here for now with notes.

/**
 * @test
 */
public function it_generates_placeholder_data_url_when_toggling_cache_from_on_to_off()
{
    /**
     * Clear regular cache and path cache store
     * @see: https://statamic.dev/image-manipulation#path-cache-store
     */
    Cache::flush();
    $this->artisan(GlideClear::class);

    Config::set('statamic.assets.image_manipulation.cache', true);
    // Glide server has already initialized in service container, we clear it so the cache config value gets read.
//        App::forgetInstance(\League\Glide\Server::class);
//        app()->register(\Statamic\Providers\GlideServiceProvider::class);
    App::forgetInstance(\Statamic\Contracts\Imaging\UrlBuilder::class);
    App::forgetInstance(\Statamic\Contracts\Imaging\ImageManipulator::class);
    App::forgetInstance(\League\Glide\Server::class);
    App::forgetInstance(\Statamic\Imaging\PresetGenerator::class);
    App::forgetInstance(\Statamic\Imaging\ImageGenerator::class);

    ray(\Statamic\Facades\Glide::cacheDisk()->getConfig()['root']);

    $cacheDiskPathBefore = \Statamic\Facades\Glide::cacheDisk()->getConfig()['root'];

    // Generate placeholder
    $responsive = new Breakpoint($this->asset, 'default', 0, []);
    $firstPlaceholder = $responsive->placeholder();

    /**
     * We use Blink cache for placeholder generation that we need to clear
     * @see https://statamic.dev/extending/blink-cache
     * @see Breakpoint::placeholderSvg()
     */
    Blink::store()->flush();

    // Once again, because we are running in the same session, we need server instance to be forgotten
    // so that it uses different Filesystem that depends on the statamic.assets.image_manipulation.cache value
//        App::forgetInstance(\League\Glide\Server::class);
    Cache::flush();
//        app()->register(\Statamic\Providers\GlideServiceProvider::class);
    Config::set('statamic.assets.image_manipulation.cache', false);

    App::forgetInstance(\Statamic\Contracts\Imaging\UrlBuilder::class);
    App::forgetInstance(\Statamic\Contracts\Imaging\ImageManipulator::class);
    App::forgetInstance(\League\Glide\Server::class);
    App::forgetInstance(\Statamic\Imaging\PresetGenerator::class);
    App::forgetInstance(\Statamic\Imaging\ImageGenerator::class);

    $cacheDiskPathAfter = \Statamic\Facades\Glide::cacheDisk()->getConfig()['root'];

    // Generate placeholder again
    $responsive = new Breakpoint($this->asset, 'default', 0, []);
    $secondPlaceholder = $responsive->placeholder();

    $this->assertEquals($firstPlaceholder, $secondPlaceholder);
    $this->assertNotEquals($cacheDiskPathBefore, $cacheDiskPathAfter);
}
ncla commented 1 year ago

This can be reproduced on statamic/cms with glide:data_url tag too.

ncla commented 1 year ago

Removing

'cache' => Config::get('statamic.assets.image_manipulation.cache', false)

will make the test fail.

Will open an issue on statamic/cms a bit later.