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: Problem with this->cache in ext.system_messages.php

Status Resolved
Add-on / Version Custom System Messages 3.0.6
Severity
EE Version 5.3.2

Hop Studios

Oct 07, 2021

Hello,

I have been debugging some excessive queries on a client site and I just noticed an issue that appears to be caused by the caching mechanism in _get_settings not working as expected.

I have been working on some cache tuning with CE Cache and also Stash is in use in some places. These tools are working well to cache parts of the page in question which is greatly improving the performance and cutting down on the redundant queries, however I noticed that cache warming is taking a very long time with far too many queries.
I noted that this query ran over 300 times on one page load.

SELECT settings FROM exp_extensions WHERE enabled = 'y' AND class = 'System_messages_ext' LIMIT 1
#system/user/addons/system_messages/ext.system_messages.php L:48 System_messages_ext::_get_settings()

Tracing the extension code, I saw the section in the _get_settings function that the cache is not being set or read properly.

Here is my work-around for this issue:

OLD: 
        // Get the settings for the extension
        if(isset($this->cache['settings']) === FALSE || $force_refresh === TRUE) {
            // check the db for extension settings
            $query = ee()->db->query("SELECT settings FROM exp_extensions WHERE enabled = 'y' AND class = '" . __CLASS__ . "' LIMIT 1");

            // if there is a row and the row has settings
            if ($query->num_rows() > 0 && $query->row('settings') != '') {
                // save them to the cache
                $this->cache['settings'] = strip_slashes(unserialize($query->row('settings')));
            }
        }

        // check to see if the session has been set
        // if it has return the session
        // if not return false
        if(empty($this->cache['settings']) !== TRUE) {
            $settings = $this->cache['settings'];
        }		
		NEW: 
        $cache_key = '/SystemMessages/Settings';
        $settings = ee()->cache->get($cache_key, 1);

        if (empty($settings) || $force_refresh === TRUE) {
            $query = ee()->db->query("SELECT settings FROM exp_extensions WHERE enabled = 'y' AND class = '" . __CLASS__ . "' LIMIT 1");

            // if there is a row and the row has settings
            if ($query->num_rows() > 0 && $query->row('settings') != '') {
                // save them to the cache
                $settings = strip_slashes(unserialize($query->row('settings')));
                ee()->cache->save($cache_key, $settings, 14400, 1);
            }           
        }

I noticed you have a Cache class, but the __construct function is not being called. I suspect the query is being run each time Stash and CE Cache parse chunks of code, it runs the template_post_parse hook.

Let me know if you need any more info or if there is a better fix for this.

Rowan

#1

BoldMinded (Brian)

Thanks for pointing this out Rowan, I’ll take a look at it soon.

#2

BoldMinded (Brian)

These changes make sense. I’ll make sure to include it in the next release. Thanks for sharing!

Login to reply