Remove wasteful double-caching of settings metadata

Settings metadata was being cached once per domain and one extra time for good measure.
hook_civicrm_alterSettingsMetaData() was invoked even when fetching cached data,
even though the hook-altered data was stored in the cache.

Now that the hook is invoked only once and the results cached, tests have been tweaked
to cache after setting the hook instead of before.

diff --git a/Civi/Core/SettingsMetadata.php b/Civi/Core/SettingsMetadata.php
index 00d04d34215c39bde99cb5a66a0990faafc9440e..6aeaa3c490a2613702086dd1af5637890fdf4dad 100644 (file)
--- a/Civi/Core/SettingsMetadata.php
+++ b/Civi/Core/SettingsMetadata.php
@@ -33,8 +33,6 @@ namespace Civi\Core;
  */
 class SettingsMetadata {
 
-  const ALL = 'all';
-
   /**
    * WARNING: This interface may change.
    *
@@ -72,25 +70,14 @@ class SettingsMetadata {
 
     $cache = \Civi::cache('settings');
     $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);
-    $cached = is_array($settingsMetadata);
-
-    if (!$cached) {
-      $settingsMetadata = $cache->get(self::ALL);
-      if (empty($settingsMetadata)) {
-        global $civicrm_root;
-        $metaDataFolders = [$civicrm_root . '/settings'];
-        \CRM_Utils_Hook::alterSettingsFolders($metaDataFolders);
-        $settingsMetadata = self::loadSettingsMetaDataFolders($metaDataFolders);
-        $cache->set(self::ALL, $settingsMetadata);
-      }
-    }
-
-    \CRM_Utils_Hook::alterSettingsMetaData($settingsMetadata, $domainID, NULL);
 
-    if (!$cached) {
+    if (!is_array($settingsMetadata)) {
+      global $civicrm_root;
+      $metaDataFolders = [$civicrm_root . '/settings'];
+      \CRM_Utils_Hook::alterSettingsFolders($metaDataFolders);
+      $settingsMetadata = self::loadSettingsMetaDataFolders($metaDataFolders);
+      \CRM_Utils_Hook::alterSettingsMetaData($settingsMetadata, $domainID, NULL);
       $cache->set($cacheString, $settingsMetadata);
     }
 

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 39b9811ea8a6342dfc170d6097bc7bcd13f042c6..c6c473e4303a3521c2278b2f327069e76860001a 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -1942,13 +1942,13 @@ AND    ( TABLE_NAME LIKE 'civicrm_value_%' )
    * @return void
    */
   public function setMockSettingsMetaData($extras) {
-    Civi::service('settings_manager')->flush();
-
     CRM_Utils_Hook::singleton()
       ->setHook('civicrm_alterSettingsMetaData', function (&$metadata, $domainId, $profile) use ($extras) {
         $metadata = array_merge($metadata, $extras);
       });
 
+    Civi::service('settings_manager')->flush();
+
     $fields = $this->callAPISuccess('setting', 'getfields', array());
     foreach ($extras as $key => $spec) {
       $this->assertNotEmpty($spec['title']);

diff --git a/tests/phpunit/api/v3/SettingTest.php b/tests/phpunit/api/v3/SettingTest.php
index aa75cf711572617c653ccb5d98c8753c923f53a3..f8d6ed962064d9eee8ed375fdf68cb42688112b0 100644 (file)
--- a/tests/phpunit/api/v3/SettingTest.php
+++ b/tests/phpunit/api/v3/SettingTest.php
@@ -110,7 +110,6 @@ class api_v3_SettingTest extends CiviUnitTestCase {
   public function testGetFieldsCaching() {
     $settingsMetadata = array();
     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']);
     $this->quickCleanup(array('civicrm_cache'));