Publisher does not support the Fluid field type. Please do not contact asking when support will be available.

If you purchased an add-on from expressionengine.com, be sure to visit boldminded.com/claim to add the license to your account here on boldminded.com.

Ticket: speedy file driver ‘deleteItem’ not working as the ‘key’ does not include ‘/item’

Status Backlog
Add-on / Version Speedy 1.8.1
Severity
EE Version 7.3.13

Hop Studios

Oct 26, 2023

Hey Brian,

I ran into an issue with the file driver when I was trying to use {exp:speedy:clear} tag.

I created this simple template to test it:

{if segment_3 == 'clear'}
{exp:speedy:clear items="local/quizzes/test_clear"}
cleared
{if:else}
{exp:speedy:fragment
    ttl="{global:ttl_1_week}"
    driver="{global:env_speedy_driver}"
}
Test the thing!
{/exp:speedy:fragment}
{/if}

Visiting /quizzes/test_clear generates the fragment properly, but when I add the ‘/clear’ segment, nothing happens and the fragment is not cleared.
I checked the mod code and traced it to the /speedy/Service/Drivers/FilesystemDriver.php file.

In the ‘deleteItem()’ function this was not working properly. I thought it was permissions, checked thoroughly, but couldn’t find any issues in my tests.

Original function code line ~33 in FilesystemDriver.php

public function deleteItem($key)
    {
        $file = $this->getFilename($key);

        try {
            return @unlink($file) || !file_exists($file);
        } catch (\Exception $exception) {
            // Fail silently. @ isn't enough for PHP 8.
        }

        return false;
    }

Here is my revised function which appears to be working for me in my test case, also lots of comments.

/**
     * Attempts to delete an item, which could be a file or a directory, based on the provided key.
     *
     * If the target is a file, it will try to delete the file directly.
     * If the target is a directory and has a known structure (e.g., containing an 'item' file),
     * it will attempt to delete the specific content first and then the directory.
     * If any deletion operation fails or encounters an exception, the function will return false.
     *
     * @param string $key The key to determine the item (file or directory) to be deleted.
     *
     * @return bool True if the deletion was successful, false otherwise.
     */
    public function deleteItem($key)
    {
        $file = $this->getFilename($key);  // Get the filename or directory path based on the key.


        if (is_file($file)) {
            // target is a file.
            try {
                // Attempt to delete the file. Return true if successful, false otherwise.
                return @unlink($file) || !file_exists($file);
            } catch (\Exception $exception) {
                // Fail silently if an exception occurs.
                // This catch block is necessary as the "@" suppression operator is not sufficient for PHP 8.
            }
        } elseif (is_dir($file)) {
            // target is a directory.
            try {
                // If there's a known structure within the directory (e.g., an 'item' file),
                // try to delete that specific content first.
                if (file_exists($file . '/item')) {
                    @unlink($file . '/item');
                }
                // Attempt to remove the directory. Return true if successful, false otherwise.
                // Note: rmdir can only delete empty directories.
                return @rmdir($file) || !file_exists($file);
            } catch (\Exception $exception) {
                // Fail silently if an exception occurs.
                // This catch block is necessary as the "@" suppression operator is not sufficient for PHP 8.
            }
        }

        // If neither file nor directory deletion was successful, return false.
        return false;
    }


I haven’t thoroughly tested other drives or static, but it seems like this helps solves the bug I ran into.

Let me know if you have any comments or questions.

Rowan

 

#1

BoldMinded (Brian)

I don’t think deleteItem should be deleting more than 1 file. The singular method name implies it only deletes 1 thing. If anything it should be calling deletePath() in this scenario.

I’d rather see the update happen somewhere else, but I’m not sure where at this time. I’ll have to find some time to replicate it. If this works for you now that’s fine, but just note that this could change in a later release.

#2

Hop Studios

Hey Brian,

Thanks for the reply on this one.

I didn’t see the ‘deletePath()’ function or the clearCachePath() function thanks for pointing that out.

I tried using $this->deletePath($file); in my modified code, but then traced through the deletePath() function and was unable to get it to work.

Seems like the ‘getfilename’ function is not returning a correct path.

I added some debugging do your function:

public function deletePath($path)
    {
        dump($path);
        $file = $this->getFilename($path);
        dump($file);

        return $this->clearCachePath($file);
    }

I noticed the string was concatenated, and no longer valid:

string(84) “/var/www/html/contentzsystem/user/cache/speedy/default_site/local/quizzes/test_clear” string(144) “/var/www/html/contentzsystem/user/cache/speedy/default_site//var/www/html/contentzsystem/user/cache/speedy/default_site/local/quizzes/test_clear”

I think there is a problem with getFilename() as the siteCachePath is already included in the key in this instance, maybe its the way i’m calling it?

Seems like getFilename might expect the key to be relative path not full path? Here’s my modified code in ../speedy/Service/Drivers/AbstractFilesystemDriver.php line 114:

/**
     * @param string $key
     * @return string
     */
    protected function getFilename($key, $validateKey = true)
    {
        if ($validateKey) {
            CacheItem::validateKey($key);
        }

        // Strip prefix as it is included in cache path
        $prefix = 'speedy/' . $this->siteName . '/';
        if (strpos($key, $prefix) === 0) {
            $key = substr($key, strlen($prefix));
        }

        // Ensure that the siteCachePath is not duplicated in the key
        if (strpos($key, $this->siteCachePath) !== 0) {
            $key = $this->siteCachePath . '/' . $key;
        }

        return $key;
    }

As for the ‘The singular method name implies it only deletes 1 thing’ I totally understand this. For me, we are creating a couple fragments in a specific folder, so it makes more sense to wipe the whole folder instead of having to call out individual fragments.

I suppose you could clarify the documentation here instead of changing the code.

{exp:speedy:clear} Use this in your templates to clear specific cache items. Parameters tags=”apple|oranges” Clear one or more tags and all items cached with that tag. items=”local/blog/footer|local/blog/header” clear one or more specfic cache items.

– the ‘items’ part doesn’t work as documented here, as the ‘item’ path needs to be included in the clear tag.

Am I missing something else perhaps? Seems like the items are excluding the ‘item’ distinction and will refer to folders if we did it as documented here.

For me, I can target the ‘item’ using this:

{exp:speedy:clear items="local/quizzes/test_clear/item"}

This works OK.

Rowan

#3

Hop Studios

I will revert my changes to FilesystemDriver to avoid ‘hacks’ and add the ‘item’ to my clear code for now. I’d wager a documentation change is better than a code change.

My issue with getFilename() is likely due to my use of it being a hack to begin with. 😉 likely safe to disregard.

thanks for your help.

Rowan

Login to reply