JCache was updated a while ago in a way where it won't try to load a cache handler if caching is set to disabled for the active instance. So this really shouldn't be needed.

Also, there is an intention to make the base storage class abstract in 4.0, so this change would just be undone as soon as we sort out that one.

I guess to me the better fix is to figure out what's still trying to load the handler even with caching disabled (aside from a delete request, that should be able to try and delete stuff even with caching turned off), or just start catching the thrown JCacheException implementations and ignore the cache errors.

Hmm. On a closer look maybe it is not actually trying to load the handler. So, yes, that seems to have been fixed. Then I still wonder why it's setting the handler to 'file' here at all anyway. Shouldn't that line just be removed?

There is or was an issue on J3.7.3 with cache clean() method.
Some 3rd party extensions use cache (but system cache disabled) and then after it saves record, it call $cache->clean()''. If cache is not available (handler not available) then it throws an exception.

Ya, clean is the method I didn't put the "if no caching then return early" logic into. Like I said, it really should be possible to clear the cache store even if you've disabled caching (think a temporary workflow where you've made changes, have cache turned off to test them, and want/need to clear the cache store before re-enabling it).

Shouldn't that line just be removed?

Personally I'd just remove that line. Though if we can get #16621 merged in it should probably change to use that in-memory adapter if we're going to insist on overwriting the global configuration. Which then leads me to question what code paths are actually using the caching API that this is needed and why we want to force the files handler instead of respecting the user's config.

For J3.7 I use patch:

@@ -296,7 +308,19 @@ class JCache
                // Get the default group
                $group = $group ?: $this->_options['defaultgroup'];
 
-               return $this->_getStorage()->clean($group, $mode);
+               try
+               {
+                       return $this->_getStorage()->clean($group, $mode);
+               }
+               catch (JCacheException $e)
+               {
+                       if (!$this->getCaching())
+                       {
+                               return false;
+                       }
+
+                       throw $e;
+               }
        }
 
        /**

I can create a PR if such patch is OK

Seems fair to me.

Wouldn't it be simpler as:

public function clean($group = null, $mode = 'group')
{
    if (!$this->getCaching())
    {
        return false;
    }

    // Get the default group
    $group = $group ?: $this->_options['defaultgroup'];

    return $this->_getStorage()->clean($group, $mode);
}

But anyway, as far as switching to some specific cache just to clear it, it doesn't make a lot of sense. If the system is not using file cache but then you clear file cache, what have you done? Not much. Meanwhile, the system's actual cache is not cleared. Also, if you're running something like finder_indexer from the command line and your system is using apc for caching, you're out of luck anyway.

Status is set on "Needs Review".

The problem this was meant to solve seems to have been solved in another way so...