[#13520] - Expose cache_path config, catch exceptions cleaning cache on config save
- Fixed in Code Base
- 9 Jan 2017
- Medium
- Build: staging
- # 13520
- Diff
- mbabker:config-cache-fixes
User tests: Successful: Unsuccessful:
Summary of Changes
-
The code already has support for a
cache_pathparameter which allows a user to specify a custom cache path when using the filesystem for caching. This parameter is now exposed to the global configuration. -
When saving the global configuration, the code checking for the filesystem being usable did not account for this parameter and always used the hardcoded
JPATH_SITE . '/cache'path for this check; the value is now used. In addition to this, we will also check if a custom cache path was given for this check and cannot be written to but the default path can be, we'll gracefully fall back to the default (and warn the user of this). -
Clearing the cache store when disabling the cache should not result in a fatal error saving the global configuration. Wrap the
$cache->clean()call in try/catch blocks for theJCacheException*classes to handle these known errors (note this purposefully does not catch the parentRuntimeExceptionas try/catch blocks should generally be specific to known error conditions). -
Since we clear the cache store when disabling the cache, we should also do it when changing the handler. That is implemented now.
Testing Instructions
Saving the global config should still work no matter what.
Setting a value for the cache_path var should result in that being the cache path used for the filesystem cache, when empty the code should not set the variable to the saved configuration. If this path can't be written to, the default path should be used instead and the cache_path value not be saved.
When changing cache handlers or disabling an active cache, the cache store should be cleaned. If the cache store can't be reached then a warning should be shown to the user and not block the save operation.
Documentation Changes Required
N/A
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_config Language & Strings |
I changed the cache path to a non existen folder and i got an exception: The file Cache Storage is not supported on this platform. . But my cache handler is file so is supported. The directory doesn't exist tough.
The isSupported check is is_writable(JFactory::getConfig()->get('cache_path', JPATH_CACHE)); so it's dependent on the configuration right now. Separate PR if you want to make changes to that logic (which probably should be done).
Also is the site cache folder or the admin cache folder?
Seems to be consistently inconsistent. The com_admin system info view uses it for both apps, the cache handler itself uses it for both apps, com_cache uses it for only the site app (same for elsewhere in com_config). Should be reviewed. Also note this is the only cache handler which has application awareness; every other handler doesn't distinguish based on app (IMO that's the right move, let's just have one consistent cache folder for all the things).
Seems to be consistently inconsistent.
Oh ... then all is fine! I has afraid this one time it would be consistently consistent.
every other handler doesn't distinguish based on app (IMO that's the right move, let's just have one consistent cache folder for all the things).
Agree
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13520.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13520.
| Status | Pending | ⇒ | Ready to Commit |
RTC
| Status | Ready to Commit | ⇒ | Pending |
| Status | Pending | ⇒ | Ready to Commit |
@joomla-bot are you drunk?
wilsonge
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-01-09 15:24:54 |
| Closed_By | ⇒ | wilsonge | |
| Labels |
Added:
?
|
||
Maybe change to
if (!empty($data['cache_path']))
{
$path = $data['cache_path'];
}
elseif (empty($data['cache_path']) && !empty($prev['cache_path']))
{
$path = $prev['cache_path'];
}
else
{
$path = JPATH_SITE . '/cache';
}
Feel free to PR it.
I changed the cache path to a non existen folder and i got an exception:
The file Cache Storage is not supported on this platform.. But my cache handler isfileso is supported. The directory doesn't exist tough.Also is the site cache folder or the admin cache folder?