diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index bc102b7275..b30f3cac13 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -490,38 +490,28 @@ final class PhabricatorUser $settings = $this->loadGlobalSettings(); } - // NOTE: To slightly improve performance, we're using all settings here, - // not just settings that are enabled for the current viewer. It's fine to - // get the value of a setting that we wouldn't let the user edit in the UI. - $defaults = PhabricatorSetting::getAllSettings(); - if (array_key_exists($key, $settings)) { $value = $settings[$key]; - - // Make sure the value is valid before we return it. This makes things - // more robust when options are changed or removed. - if (isset($defaults[$key])) { - try { - id(clone $defaults[$key]) - ->setViewer($this) - ->assertValidValue($value); - - return $this->writeUserSettingCache($key, $value); - } catch (Exception $ex) { - // Fall through below and return the default value. - } - } else { - // This is an ad-hoc setting with no controlling object. - return $this->writeUserSettingCache($key, $value); - } + return $this->writeUserSettingCache($key, $value); } - if (isset($defaults[$key])) { - $value = id(clone $defaults[$key]) - ->setViewer($this) - ->getSettingDefaultValue(); + $cache = PhabricatorCaches::getRuntimeCache(); + $cache_key = "settings.defaults({$key})"; + $cache_map = $cache->getKeys(array($cache_key)); + + if ($cache_map) { + $value = $cache_map[$cache_key]; } else { - $value = null; + $defaults = PhabricatorSetting::getAllSettings(); + if (isset($defaults[$key])) { + $value = id(clone $defaults[$key]) + ->setViewer($this) + ->getSettingDefaultValue(); + } else { + $value = null; + } + + $cache->setKey($cache_key, $value); } return $this->writeUserSettingCache($key, $value); @@ -555,12 +545,16 @@ final class PhabricatorUser return $this->getUserSetting(PhabricatorTimezoneSetting::SETTINGKEY); } - private function loadGlobalSettings() { - $cache_key = 'user.settings.global'; - $cache = PhabricatorCaches::getRequestCache(); - $settings = $cache->getKey($cache_key); + public static function getGlobalSettingsCacheKey() { + return 'user.settings.globals.v1'; + } - if ($settings === null) { + private function loadGlobalSettings() { + $cache_key = self::getGlobalSettingsCacheKey(); + $cache = PhabricatorCaches::getMutableStructureCache(); + + $settings = $cache->getKey($cache_key); + if (!$settings) { $preferences = PhabricatorUserPreferences::loadGlobalPreferences($this); $settings = $preferences->getPreferences(); $cache->setKey($cache_key, $settings); @@ -1495,9 +1489,27 @@ final class PhabricatorUser throw new PhabricatorDataNotAttachedException($this); } + $user_phid = $this->getPHID(); + + // Try to read the actual cache before we generate a new value. We can + // end up here via Conduit, which does not use normal sessions and can + // not pick up a free cache load during session identification. + if ($user_phid) { + $raw_data = PhabricatorUserCache::readCaches( + $type, + $key, + array($user_phid)); + if (array_key_exists($user_phid, $raw_data)) { + $raw_value = $raw_data[$user_phid]; + $usable_value = $type->getValueFromStorage($raw_value); + $this->rawCacheData[$key] = $raw_value; + $this->usableCacheData[$key] = $usable_value; + return $usable_value; + } + } + $usable_value = $type->getDefaultValue(); - $user_phid = $this->getPHID(); if ($user_phid) { $map = $type->newValueForUsers($key, array($this)); if (array_key_exists($user_phid, $map)) { diff --git a/src/applications/people/storage/PhabricatorUserCache.php b/src/applications/people/storage/PhabricatorUserCache.php index c74b583923..2afb06a437 100644 --- a/src/applications/people/storage/PhabricatorUserCache.php +++ b/src/applications/people/storage/PhabricatorUserCache.php @@ -96,6 +96,26 @@ final class PhabricatorUserCache extends PhabricatorUserDAO { unset($unguarded); } + public static function readCaches( + PhabricatorUserCacheType $type, + $key, + array $user_phids) { + + $table = new self(); + $conn = $table->establishConnection('r'); + + $rows = queryfx_all( + $conn, + 'SELECT userPHID, cacheData FROM %T WHERE userPHID IN (%Ls) + AND cacheType = %s AND cacheIndex = %s', + $table->getTableName(), + $user_phids, + $type->getUserCacheType(), + PhabricatorHash::digestForIndex($key)); + + return ipull($rows, 'cacheData', 'userPHID'); + } + public static function clearCache($key, $user_phid) { return self::clearCaches($key, array($user_phid)); } diff --git a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php index cfb7fe105f..c2ec0a7094 100644 --- a/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php +++ b/src/applications/settings/editor/PhabricatorUserPreferencesEditor.php @@ -158,6 +158,9 @@ final class PhabricatorUserPreferencesEditor PhabricatorUserPreferencesCacheType::KEY_PREFERENCES, $user_phid); } else { + $cache = PhabricatorCaches::getMutableStructureCache(); + $cache->deleteKey(PhabricatorUserPreferences::getGlobalCacheKey()); + PhabricatorUserCache::clearCacheForAllUsers( PhabricatorUserPreferencesCacheType::KEY_PREFERENCES); } diff --git a/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php b/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php index 0609dd9ee8..2e2ab6dac0 100644 --- a/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php +++ b/src/applications/settings/setting/PhabricatorPinnedApplicationsSetting.php @@ -10,12 +10,7 @@ final class PhabricatorPinnedApplicationsSetting } public function getSettingDefaultValue() { - $viewer = $this->getViewer(); - - // If we're editing a template, just show every available application. - if (!$viewer) { - $viewer = PhabricatorUser::getOmnipotentUser(); - } + $viewer = PhabricatorUser::getOmnipotentUser(); $applications = id(new PhabricatorApplicationQuery()) ->setViewer($viewer)