in cache buster only look at activated plugins, not loaded plugins #16880
Conversation
tsteur
added
not-in-changelog
| sort($plugins); | ||
|
|
||
| $pluginsInfo = ''; | ||
| foreach ($plugins as $pluginName) { | ||
| $plugin = Manager::getInstance()->getLoadedPlugin($pluginName); | ||
| $pluginsInfo .= $plugin->getPluginName() . $plugin->getVersion() . ','; | ||
| if ($manager->isPluginLoaded($pluginName)) { |
There was a problem hiding this comment.
Should we do this check here? If a plugin is in the activated plugins list, but for some reason isn't loaded, shouldn't the cache buster stay the same? I think we'd only change it if a plugin is activated or deactivated, not if a plugin isn't loaded for some reason.
There was a problem hiding this comment.
@diosmosis I'm not 100% sure but here's my thought. The asset manager posts an event to get all JS or CSS files. The event dispatcher in https://github.com/matomo-org/matomo/blob/4.3.1/core/EventDispatcher.php#L97 uses getPluginsLoadedAndActivated() and events will be only posted to plugins that are active and loaded.
Meaning the hash would need to change if suddenly a plugin is loaded that wasn't loaded before because the JS/CSS would change too. I'm assuming a plugin may be activated but not loaded if it's missing from the filesystem (eg temporarily). Maybe there are also other reasons like when we unload a plugin in WhiteLabel etc.
I would probably keep the check but not 100% sure if it the thinking makes sense
There was a problem hiding this comment.
I see, no your logic makes sense. The plugins used are the ones that are specifically used to generate the JS/CSS, and there can be a discrepancy with the list of activated plugins and list of loaded plugins.
Description:
This might fix #16672 . We could see if it then happens again. There's probably not much point in writing a test for it because it wouldn't really test the real scenario I suppose. Generally the thought is cache buster should only include active plugins, not loaded plugins. Maybe tag manager was loaded for this user but not activated.
Review