From: Tim Otten <totten@civicrm.org>
Date: Tue, 15 Sep 2015 04:16:13 +0000 (-0700)
Subject: CRM_Core_BAO_Setting - Remove redundant static cache
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=e1d398248992b6486d48d15ff5cb2082923d5b78;p=civicrm-core.git

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.
---

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']);