B/C fix - Modules (including core) using the "cache" XML parameter always cached, regardless of that parameter's value

[fevangelou]

Pull Request for Issue #24917

Summary of Changes

(updated to provide more context and reflect comments below)
Many Joomla core modules (e.g. mod_menu, mod_custom etc.) as well as lots of third-party modules still use "cache" as the de-facto XML parameter to allow for a module to be cached along with the rest of Joomla's rendered content (when Joomla caching is set to "on") or to be excluded from Joomla's cache overall.

The current check in JModuleHelper::moduleCache() is currently invalid as it does not count for the XML parameter "cache" (present in most modules), but only for "owncache", which was introduced in Joomla 2.5.

By not including the check for "cache" in JModuleHelper::moduleCache(), a module that uses that XML parameter will always be cached regardless of its setting (when Joomla caching is enabled).

Testing Instructions

Switch Joomla cache on and set some (affected) Joomla module (e.g. mod_menu) to be excluded from cache.

Expected result

The module instance should not be cached.

Actual result

It's cached. That's because the XML file for that module utilizes the parameter name "cache" which is not checked before deciding whether a module should be cached or not.

Documentation Changes Required

I would recommend a pointer to this https://docs.joomla.org/J2.5:What%27s_new_in_Joomla_2.5 in the docs that refer to module development.

[mbabker]

You're on the right path but this looks incomplete. #20626 for reference.

The intent was to turn it into strict checks and support proper filter definitions. So the "owncache" param is strictly checked for both integer 0 and string 0, it looks like the "cache" param needs both of those checks as well.

FWIW, that line hasn't checked the "cache" param since at least 1.7 so something was probably messed up even before that PR if this patch is needed.

[fevangelou]

Actually, have a look at my intro here: #24917

Without "cache" in that if statement, all modules with that XML param (even core modules!) will always be cached, even if you set that param to "no".

[fevangelou]

In other words, I would recommend we patch this now, until more time is found to properly address cache flag overriding. Which by the way needs some reworking as I can't dynamically set for example "cachemode". It always switches to the preset "oldstatic" as referenced here: https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Document/Renderer/Html/ModuleRenderer.php#L84

[mbabker]

That's why the check essentially needs to be this:

// Both of these checks use strict checks purposefully to omit other falsy values, the form definition should use an integer filter
$ownCacheDisabled = $moduleparams->get('owncache') === 0 || $moduleparams->get('owncache') === '0';
$cacheDisabled = $moduleparams->get('cache') === 0 || $moduleparams->get('cache') === '0';

if ($ownCacheDisabled || $cacheDisabled || $conf->get('caching') == 0 || $user->get('id')) {
    $cache->setCaching(false);
}

It covers both the param that seems to have been missing forever and a day (this is the line's blame at 3.7.5 and shows it had no changes in years) while also moving forward with the bit about encouraging forms to be properly filtered and using correct data types.

[fevangelou]

Sorry, I missed the quotes.

You're right, it should be the 2 checks for owncache, plus 2 more for cache.

[fevangelou]

PR updated.

[fevangelou]

Thank you.

As a sidenote, (as I briefly mentioned above) I've found that "cachemode" cannot be dynamically set from within a module's code, but only through its XML parameters. So switching for example from the "oldstatic" caching mode to the newer (default) "static" or any of the other options that were introduced in Joomla 2.5 is not possible through a simple module update. The developer must add the hidden cachemode XML parameter and then a site admin must open ALL modules and simply resave them to get the new value in and allow for more efficient caching overall.

File: https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Document/Renderer/Html/ModuleRenderer.php#L83:

// Default for compatibility purposes. Set cachemode parameter or use JModuleHelper::moduleCache from within the module instead

In other words, adding just this in a module's code:

<?php

// module prep stuff here

$cacheparams = new stdClass;
$cacheparams->cachemode = 'safeuri';
$cacheparams->class = 'someHelper';
$cacheparams->method = 'someMethod';
$cacheparams->methodparams = $params;
$cacheparams->modeparams = array('whatever' => 'int');

$list = JModuleHelper::moduleCache($module, $params, $cacheparams);

// continue to render module output here

...would completely ignore the new cachemode set, maintaining "oldstatic" as this module's cachemode.

Do I understand correctly that the original intent was to be able to set cachemode dynamically?

[mbabker]

Do I understand correctly that the original intent was to be able to set cachemode dynamically?

Sounds like that was the case but it completely missed the mark.

[ghost]

@joomlabeat please test (how to: https://docs.joomla.org/Testing_Joomla!_patches)

[joomlabeat]

I have tested this item ✅ successfully on b82c9ca

Excluding modules from cache works the same for both caching levels.
*Not sure if this is desired though - as I have read documentation and articles around the web that says only conservative cache would allow modules to be excluded from cache by using their cache field.

As a sidenote to @fevangelou sidenote:
After saving a module with a specific cachemode for the first time, it looks like is impossible to update this setting by simply changing the cachemode default value in the xml. After the first saving, the cachemode field will always get the initial stored value from the database, the form will be populated with this value and will be re-submitted each time, disregarding what default value the xml might have.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24916.}

[brianteeman]

After saving a module with a specific cachemode for the first time, it looks like is impossible to update this setting by simply changing the cachemode default value in the xml. After the first saving, the cachemode field will always get the initial stored value from the database, the form will be populated with this value and will be re-submitted each time, disregarding what default value the xml might have.

Yes and that is the intended behaviour

[viocassel]

I have tested this item ✅ successfully on b82c9ca

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24916.}

[Quy]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24916.}

[HLeithner]

thx

[HLeithner]

thx

[ghost]

@viocassel can you please mail me: [email protected]?