From e1d398248992b6486d48d15ff5cb2082923d5b78 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Mon, 14 Sep 2015 21:16:13 -0700 Subject: [PATCH] CRM_Core_BAO_Setting - Remove redundant static cache Before CRM-16373, the setting cache was stored in a static property of the BAO. This data is now loaded into objects -- which are stored in the container. This is essentially the same for a typical request, and (for unit-testing) it provides simpler cleanup between tests. --- CRM/Activity/Page/AJAX.php | 26 +---- CRM/Core/BAO/Setting.php | 123 +------------------- Civi/Core/SettingsManager.php | 28 +++++ Civi/Core/SettingsMetadata.php | 16 ++- tests/phpunit/CRM/Core/BAO/SettingTest.php | 5 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 4 +- tests/phpunit/api/v3/SettingTest.php | 2 +- 7 files changed, 44 insertions(+), 160 deletions(-) diff --git a/CRM/Activity/Page/AJAX.php b/CRM/Activity/Page/AJAX.php index a9c88ca2c9..f8ab3517b7 100644 --- a/CRM/Activity/Page/AJAX.php +++ b/CRM/Activity/Page/AJAX.php @@ -481,34 +481,14 @@ class CRM_Activity_Page_AJAX { $session = CRM_Core_Session::singleton(); $userID = $session->get('userID'); if ($userID) { - //flush cache before setting filter to account for global cache (memcache) - $domainID = CRM_Core_Config::domainID(); - $cacheKey = CRM_Core_BAO_Setting::inCache( - CRM_Core_BAO_Setting::PERSONAL_PREFERENCES_NAME, - 'activity_tab_filter', - NULL, - $userID, - TRUE, - $domainID, - TRUE - ); - if ($cacheKey) { - CRM_Core_BAO_Setting::flushCache($cacheKey); - } - $activityFilter = array( 'activity_type_filter_id' => empty($params['activity_type_id']) ? '' : CRM_Utils_Type::escape($params['activity_type_id'], 'Integer'), 'activity_type_exclude_filter_id' => empty($params['activity_type_exclude_id']) ? '' : CRM_Utils_Type::escape($params['activity_type_exclude_id'], 'Integer'), ); - CRM_Core_BAO_Setting::setItem( - $activityFilter, - CRM_Core_BAO_Setting::PERSONAL_PREFERENCES_NAME, - 'activity_tab_filter', - NULL, - $userID, - $userID - ); + /** @var \Civi\Core\SettingsBag $cSettings */ + $cSettings = Civi::service('settings_manager')->getBagByContact(CRM_Core_Config::domainID(), $userID); + $cSettings->set('activity_tab_filter', $activityFilter); } CRM_Utils_JSON::output($activities); diff --git a/CRM/Core/BAO/Setting.php b/CRM/Core/BAO/Setting.php index 1de904c988..df8e90e659 100644 --- a/CRM/Core/BAO/Setting.php +++ b/CRM/Core/BAO/Setting.php @@ -59,98 +59,6 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting { URL_PREFERENCES_NAME = 'URL Preferences', LOCALIZATION_PREFERENCES_NAME = 'Localization Preferences', SEARCH_PREFERENCES_NAME = 'Search Preferences'; - static $_cache = NULL; - - /** - * Checks whether an item is present in the in-memory cache table - * - * @param string $group - * (required) The group name of the item. - * @param string $name - * (required) The name of the setting. - * @param int $componentID - * The optional component ID (so components can share the same name space). - * @param int $contactID - * If set, this is a contactID specific setting, else its a global setting. - * @param bool|int $load if true, load from local cache (typically memcache) - * - * @param int $domainID - * @param bool $force - * - * @return bool - * true if item is already in cache - */ - public static function inCache( - $group, - $name, - $componentID = NULL, - $contactID = NULL, - $load = FALSE, - $domainID = NULL, - $force = FALSE - ) { - if (!isset(self::$_cache)) { - self::$_cache = array(); - } - - $cacheKey = "CRM_Setting_{$group}_{$componentID}_{$contactID}_{$domainID}"; - - if ( - $load && - ($force || !isset(self::$_cache[$cacheKey])) - ) { - - // check in civi cache if present (typically memcache) - $globalCache = CRM_Utils_Cache::singleton(); - $result = $globalCache->get($cacheKey); - if ($result) { - - self::$_cache[$cacheKey] = $result; - } - } - - return isset(self::$_cache[$cacheKey]) ? $cacheKey : NULL; - } - - /** - * Allow key o be cleared. - * @param string $cacheKey - */ - public static function flushCache($cacheKey) { - unset(self::$_cache[$cacheKey]); - $globalCache = CRM_Utils_Cache::singleton(); - $globalCache->delete($cacheKey); - } - - /** - * @param $values - * @param $group - * @param int $componentID - * @param int $contactID - * @param int $domainID - * - * @return string - */ - public static function setCache( - $values, - $group, - $componentID = NULL, - $contactID = NULL, - $domainID = NULL - ) { - if (!isset(self::$_cache)) { - self::$_cache = array(); - } - - $cacheKey = "CRM_Setting_{$group}_{$componentID}_{$contactID}_{$domainID}"; - - self::$_cache[$cacheKey] = $values; - - $globalCache = CRM_Utils_Cache::singleton(); - $result = $globalCache->set($cacheKey, $values); - - return $cacheKey; - } /** * @param $group @@ -403,12 +311,6 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting { $dao->save(); $dao->free(); - - // also save in cache if needed - $cacheKey = self::inCache($group, $name, $componentID, $contactID, FALSE, $domainID); - if ($cacheKey) { - self::$_cache[$cacheKey][$name] = $value; - } } /** @@ -585,7 +487,7 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting { $domainID = NULL, $profile = NULL ) { - return \Civi\Core\SettingsMetadata::getMetadata($filters, $domainID, $profile); + return \Civi\Core\SettingsMetadata::getMetadata($filters, $domainID); } /** @@ -762,29 +664,6 @@ class CRM_Core_BAO_Setting extends CRM_Core_DAO_Setting { self::setItem($optionValue, $group, $name); } - /** - * Determine what, if any, overrides have been provided - * for a setting. - * - * @param $group - * @param string $name - * @param $default - * - * @return mixed, NULL or an overriden value - */ - protected static function getOverride($group, $name, $default) { - global $civicrm_setting; - if ($group && $name && isset($civicrm_setting[$group][$name])) { - return $civicrm_setting[$group][$name]; - } - elseif ($group && !isset($name) && isset($civicrm_setting[$group])) { - return $civicrm_setting[$group]; - } - else { - return $default; - } - } - /** * Civicrm_setting didn't exist before 4.1.alpha1 and this function helps taking decisions during upgrade * diff --git a/Civi/Core/SettingsManager.php b/Civi/Core/SettingsManager.php index 6b3f18bc56..7194da1a7b 100644 --- a/Civi/Core/SettingsManager.php +++ b/Civi/Core/SettingsManager.php @@ -289,4 +289,32 @@ class SettingsManager { return $result; } + /** + * Flush all in-memory and persistent caches related to settings. + * + * @return $this + */ + public function flush() { + $this->mandatory = NULL; + + $this->cache->flush(); + \Civi::cache('settings')->flush(); // SettingsMetadata; not guaranteed to use same cache. + + foreach ($this->bagsByDomain as $bag) { + /** @var SettingsBag $bag */ + $bag->loadValues(); + $bag->loadDefaults($this->getDefaults('domain')); + $bag->loadMandatory($this->getMandatory('domain')); + } + + foreach ($this->bagsByDomain as $bag) { + /** @var SettingsBag $bag */ + $bag->loadValues(); + $bag->loadDefaults($this->getDefaults('contact')); + $bag->loadMandatory($this->getMandatory('contact')); + } + + return $this; + } + } diff --git a/Civi/Core/SettingsMetadata.php b/Civi/Core/SettingsMetadata.php index 46a9ad3028..fd70a8ca99 100644 --- a/Civi/Core/SettingsMetadata.php +++ b/Civi/Core/SettingsMetadata.php @@ -53,7 +53,6 @@ class SettingsMetadata { * * @param array $filters * @param int $domainID - * @param null $profile * * @return array * the following information as appropriate for each setting @@ -66,13 +65,13 @@ class SettingsMetadata { * - description * - help_text */ - public static function getMetadata( - $filters = array(), - $domainID = NULL, - $profile = NULL - ) { + public static function getMetadata($filters = array(), $domainID = NULL) { + if ($domainID === NULL) { + $domainID = \CRM_Core_Config::domainID(); + } + $cache = \Civi::cache('settings'); - $cacheString = 'settingsMetadata_' . $domainID . '_' . $profile; + $cacheString = 'settingsMetadata_' . $domainID . '_'; // the caching into 'All' seems to be a duplicate of caching to // settingsMetadata__ - I think the reason was to cache all settings as defined & then those altered by a hook $settingsMetadata = $cache->get($cacheString); @@ -89,7 +88,7 @@ class SettingsMetadata { } } - \CRM_Utils_Hook::alterSettingsMetaData($settingsMetadata, $domainID, $profile); + \CRM_Utils_Hook::alterSettingsMetaData($settingsMetadata, $domainID, NULL); if (!$cached) { $cache->set($cacheString, $settingsMetadata); @@ -129,7 +128,6 @@ class SettingsMetadata { $settings = include $file; $settingMetaData = array_merge($settingMetaData, $settings); } - \Civi::cache('settings')->set(self::ALL, $settingMetaData); return $settingMetaData; } diff --git a/tests/phpunit/CRM/Core/BAO/SettingTest.php b/tests/phpunit/CRM/Core/BAO/SettingTest.php index 8cf695c334..18310aa74f 100644 --- a/tests/phpunit/CRM/Core/BAO/SettingTest.php +++ b/tests/phpunit/CRM/Core/BAO/SettingTest.php @@ -137,8 +137,9 @@ class CRM_Core_BAO_SettingTest extends CiviUnitTestCase { $currentDomain = CRM_Core_Config::domainID(); // we are setting up an artificial situation here as we are trying to drive out // previous memory of this setting so we need to flush it out - $cachekey = CRM_Core_BAO_Setting::inCache('CiviCRM Preferences', 'max_attachments', NULL, NULL, TRUE, $currentDomain); - CRM_Core_BAO_Setting::flushCache($cachekey); + //$cachekey = CRM_Core_BAO_Setting::inCache('CiviCRM Preferences', 'max_attachments', NULL, NULL, TRUE, $currentDomain); + //CRM_Core_BAO_Setting::flushCache($cachekey); + Civi::service('settings_manager')->flush(); CRM_Core_BAO_Setting::updateSettingsFromMetaData(); //check current domain $value = civicrm_api('setting', 'getvalue', array( diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 10fb69f976..cc09cae39c 100755 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2843,9 +2843,7 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) * @return void */ public function setMockSettingsMetaData($extras) { - CRM_Core_BAO_Setting::$_cache = array(); - $this->callAPISuccess('system', 'flush', array()); - CRM_Core_BAO_Setting::$_cache = array(); + Civi::service('settings_manager')->flush(); CRM_Utils_Hook::singleton() ->setHook('civicrm_alterSettingsMetaData', function (&$metadata, $domainId, $profile) use ($extras) { diff --git a/tests/phpunit/api/v3/SettingTest.php b/tests/phpunit/api/v3/SettingTest.php index 2c33d18cee..c1335ddb32 100644 --- a/tests/phpunit/api/v3/SettingTest.php +++ b/tests/phpunit/api/v3/SettingTest.php @@ -110,7 +110,7 @@ class api_v3_SettingTest extends CiviUnitTestCase { */ public function testGetFieldsCaching() { $settingsMetadata = array(); - Civi::cache('settings')->set('settingsMetadata__', $settingsMetadata); + Civi::cache('settings')->set('settingsMetadata_' . \CRM_Core_Config::domainID() . '_', $settingsMetadata); Civi::cache('settings')->set(\Civi\Core\SettingsMetadata::ALL, $settingsMetadata); $result = $this->callAPISuccess('setting', 'getfields', array()); $this->assertArrayNotHasKey('customCSSURL', $result['values']); -- 2.25.1