Skip to content

Commit

Permalink
Jetpack CRM: Fix hundreds of unneeded db queries (#40711)
Browse files Browse the repository at this point in the history 
* Add a cache to mitigate the problem.

* changelog

* Add line break at the end of the changelog file

* Update cache when making single settings requests

* Declare variable to satisfy static code analysis

* Force settings refresh from db

* Change self to static to allow dynamic resolution

* Refactor entire mitigation cache for issue 3504

* Change $key default value from 0 to false
  • Loading branch information
@gogdzl
gogdzl authored Jan 6, 2025
1 parent 1f043dd commit 7c21388
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 24 deletions.
2 changes: 1 addition & 1 deletion projects/plugins/crm/admin/settings/general.page.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
$confirmAct = false;

// } Retrieve all settings
$settings = $zbs->settings->getAll();
$settings = $zbs->settings->getAll( true );

// } #WH OR - need these lists?
// } Autologgers:
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/crm/changelog/fix-crm-3504-hundreds-of-unnedded-db-queries-fix
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

CRM: Fix bug which caused duplicated queries to run.
153 changes: 130 additions & 23 deletions projects/plugins/crm/includes/ZeroBSCRM.DAL3.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,87 @@ class zbsDAL {
*/
private $cache = array();

/**
* This is a temporary fix for issue #3504, until we don't change how we load settings this should help to avoid slowing down websites.
*
* @var array Associative array used for caching settings (key: 'settings') and a flag to indicate if all settings were loaded (key: 'contains_all').
*/
private static $mitigation_cache_for_issue_3504;

/**
* Resets the mitigation cache for issue #3504.
*
* Initializes the static cache property to its default state, clearing cached settings
* and resetting the "contains all" flag to false.
*
* @return void
*/
public static function reset_mitigation_cache_for_issue_3504() {
static::$mitigation_cache_for_issue_3504 = array(
'settings' => array(),
'contains_all' => false,
);
}

/**
* Retrieves a specific setting from the cache.
*
* @param string $key The setting key.
* @return mixed|null The value of the setting or null if not found.
*/
private static function mitigation_cache_for_issue_3504_single_setting( $key ) {
return static::$mitigation_cache_for_issue_3504['settings'][ $key ] ?? null;
}

/**
* Checks if a specific setting exists in the cache.
*
* @param string $key The setting key.
* @return bool True if the setting exists, false otherwise.
*/
private static function mitigation_cache_for_issue_3504_contains_key( $key ) {
return isset( static::$mitigation_cache_for_issue_3504['settings'][ $key ] );
}

/**
* Checks if all settings are loaded.
*
* @return bool True if all settings are loaded, false otherwise.
*/
private static function mitigation_cache_for_issue_3504_contains_all_settings() {
return static::$mitigation_cache_for_issue_3504['contains_all'];
}

/**
* Sets a specific setting in the cache.
*
* @param string $key The setting key.
* @param mixed $value The setting value.
* @return void
*/
private static function mitigation_cache_for_issue_3504_set_single_setting( $key, $value ) {
static::$mitigation_cache_for_issue_3504['settings'][ $key ] = $value;
}

/**
* Sets the "contains all" flag in the cache.
*
* @param bool $contains_all Whether all settings are loaded.
* @return void
*/
private static function mitigation_cache_for_issue_3504_set_contains_all( $contains_all ) {
static::$mitigation_cache_for_issue_3504['contains_all'] = $contains_all;
}

/**
* Retrieves all cached settings.
*
* @return array An array of all cached settings.
*/
private static function mitigation_cache_for_issue_3504_all_settings() {
return static::$mitigation_cache_for_issue_3504['settings'];
}

// ===============================================================================
// =========== SUB DAL LAYERS ==================================================
// These hold sub-objects, e.g. contact
Expand Down Expand Up @@ -1630,19 +1711,16 @@ private function tidy_objlink($obj=false){
* @return bool result
*/
public function setting( $key = '', $default = false, $accept_cached = false){

if ( !empty( $key ) ){

return $this->getSetting(array(

'key' => $key,
'fullDetails' => false,
'default' => $default,
'accept_cached' => $accept_cached

));

}
if ( ! empty( $key ) ) {
return $this->getSetting(
array(
'key' => $key,
'fullDetails' => false,
'default' => $default,
'accept_cached' => $accept_cached,
)
);
}

return $default;
}
Expand Down Expand Up @@ -1687,6 +1765,13 @@ public function userSetting($userID=-1,$key='',$default=false){
*/
public function getSetting($args=array()){

$key = isset( $args['key'] ) ? $args['key'] : false;
$fullDetails = isset( $args['fullDetails'] ) ? $args['fullDetails'] : false; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase

if ( ! $fullDetails && static::mitigation_cache_for_issue_3504_contains_key( $key ) ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase, VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
return static::mitigation_cache_for_issue_3504_single_setting( $key ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
}

#} =========== LOAD ARGS ==============
$defaultArgs = array(

Expand Down Expand Up @@ -1794,6 +1879,9 @@ public function getSetting($args=array()){

$setting = $this->tidy_settingSingular($potentialRes);

// We only update the mitigation cache when $fullDetails is false.
static::mitigation_cache_for_issue_3504_set_single_setting( $key, $setting ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable

// cache (commonly retrieved)
$this->update_cache_var( 'setting_' . $key, $setting );

Expand All @@ -1805,6 +1893,11 @@ public function getSetting($args=array()){

} // / if ID

if ( ! $fullDetails ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase, VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
// We only update the mitigation cache when $fullDetails is false.
static::mitigation_cache_for_issue_3504_set_single_setting( $key, $default ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
}

return $default;

}
Expand All @@ -1818,6 +1911,13 @@ public function getSetting($args=array()){
* @return array of settings lines
*/
public function getSettings($args=array()){
// If we have already loaded all settings in our cache, return them.
// In this function we won't care if the values have changed or not because when they become invalid due to other functions, these functions should reset the cache.
if ( static::mitigation_cache_for_issue_3504_contains_all_settings() ) {
return static::mitigation_cache_for_issue_3504_all_settings();
}
// Reseting any single settings we may have already loaded.
static::reset_mitigation_cache_for_issue_3504();

#} ============ LOAD ARGS =============
$defaultArgs = array(
Expand Down Expand Up @@ -1889,20 +1989,21 @@ public function getSettings($args=array()){

#} Has results, tidy + return
foreach ($potentialRes as $resDataLine) {
// We are only caching the singular setting (i.e. the value), because it is good enough.
static::mitigation_cache_for_issue_3504_set_single_setting( $resDataLine->zbsset_key, $this->tidy_settingSingular( $resDataLine ) ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase

// DEBUG echo $resDataLine->zbsset_key.' = ';

if ($fullDetails){
// tidy
$resArr = $this->tidy_setting($resDataLine);
$res[$resArr['key']] = $resArr;
} else
$res[$resDataLine->zbsset_key] = $this->tidy_settingSingular($resDataLine);

if ( $fullDetails ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase, VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
$resArr = $this->tidy_setting( $resDataLine ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
$res[ $resArr['key'] ] = $resArr; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
} else {
$res[ $resDataLine->zbsset_key ] = $this->tidy_settingSingular( $resDataLine ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
}
}
}

return $res;
static::mitigation_cache_for_issue_3504_set_contains_all( true );

return $res;
}

/**
Expand Down Expand Up @@ -2005,6 +2106,9 @@ public function addUpdateSetting($args=array()){
); foreach ($defaultArgs as $argK => $argV){ $$argK = $argV; if (is_array($args) && isset($args[$argK])) { if (is_array($args[$argK])){ $newData = $$argK; if (!is_array($newData)) $newData = array(); foreach ($args[$argK] as $subK => $subV){ $newData[$subK] = $subV; }$$argK = $newData;} else { $$argK = $args[$argK]; } } }
#} =========== / LOAD ARGS ============

// Invalidates our whole cache.
static::reset_mitigation_cache_for_issue_3504();

#} ========== CHECK FIELDS ============

$id = (int)$id;
Expand Down Expand Up @@ -7772,3 +7876,6 @@ public function segmentBuildDirectOrClause( ...$args ) { // phpcs:ignore WordPre
/ Middle Man funcs (until DAL3.0)
====================================================== */
} // / DAL class

// Initialize the static mitigation cache.
zbsDAL::reset_mitigation_cache_for_issue_3504();

0 comments on commit 7c21388

Please sign in to comment.