Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Oct 16, 2022

Conversation

ncla
Copy link
Collaborator

@ncla ncla commented Oct 15, 2022

Fixes #171.
Probably addresses #146 too.

@ncla
Copy link
Collaborator Author

ncla commented Oct 16, 2022

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 ncla marked this pull request as draft October 16, 2022 14:31
@ncla
Copy link
Collaborator Author

ncla commented Oct 16, 2022

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

@ncla ncla marked this pull request as ready for review October 16, 2022 19:54
@ncla
Copy link
Collaborator Author

ncla commented Oct 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to read file from location issue
1 participant