mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-14 02:42:40 +01:00
Make caches misses throw by default intead of inline-generating
Summary: Ref T4103. Ref T10078. Currently, when a user misses a cache we just build it for them. This is the behavior we want for the the viewer (so we don't have to build every cache up front if we don't actually need them), but not the right behavior for other users (since it allows performance problems to go undetected). Make inline cache generation strict by default, then make sure all the things that rely on cache data request the correct data (well, all of the things identified by unit tests, at least: there might be some more stuff I haven't hit yet). This fixes test failures in D16040, and backports a piece of that change. Test Plan: Identified and then fixed failures with `arc unit --everything`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4103, T10078 Differential Revision: https://secure.phabricator.com/D16042
This commit is contained in:
parent
7170b062e6
commit
2b344b2bb5
6 changed files with 29 additions and 4 deletions
|
@ -163,6 +163,7 @@ final class PhabricatorAuthSessionEngine extends Phobject {
|
||||||
$user = $user_table->loadFromArray($info);
|
$user = $user_table->loadFromArray($info);
|
||||||
|
|
||||||
$user->attachRawCacheData($cache_raw);
|
$user->attachRawCacheData($cache_raw);
|
||||||
|
$user->setAllowInlineCacheGeneration(true);
|
||||||
|
|
||||||
switch ($session_type) {
|
switch ($session_type) {
|
||||||
case PhabricatorAuthSession::TYPE_WEB:
|
case PhabricatorAuthSession::TYPE_WEB:
|
||||||
|
|
|
@ -59,6 +59,7 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
|
||||||
$users = id(new PhabricatorPeopleQuery())
|
$users = id(new PhabricatorPeopleQuery())
|
||||||
->setViewer($this->getViewer())
|
->setViewer($this->getViewer())
|
||||||
->withPHIDs($phids)
|
->withPHIDs($phids)
|
||||||
|
->needUserSettings(true)
|
||||||
->execute();
|
->execute();
|
||||||
$users = mpull($users, null, 'getPHID');
|
$users = mpull($users, null, 'getPHID');
|
||||||
|
|
||||||
|
|
|
@ -337,9 +337,12 @@ abstract class PhabricatorMailReplyHandler extends Phobject {
|
||||||
|
|
||||||
$all_phids = array_merge($to, $cc);
|
$all_phids = array_merge($to, $cc);
|
||||||
if ($all_phids) {
|
if ($all_phids) {
|
||||||
|
// We need user settings here because we'll check translations later
|
||||||
|
// when generating mail.
|
||||||
$users = id(new PhabricatorPeopleQuery())
|
$users = id(new PhabricatorPeopleQuery())
|
||||||
->setViewer(PhabricatorUser::getOmnipotentUser())
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||||
->withPHIDs($all_phids)
|
->withPHIDs($all_phids)
|
||||||
|
->needUserSettings(true)
|
||||||
->execute();
|
->execute();
|
||||||
$users = mpull($users, null, 'getPHID');
|
$users = mpull($users, null, 'getPHID');
|
||||||
|
|
||||||
|
|
|
@ -528,9 +528,6 @@ final class PhabricatorPeopleQuery
|
||||||
$cache_data = igroup($cache_data, 'userPHID');
|
$cache_data = igroup($cache_data, 'userPHID');
|
||||||
foreach ($user_map as $user_phid => $user) {
|
foreach ($user_map as $user_phid => $user) {
|
||||||
$raw_rows = idx($cache_data, $user_phid, array());
|
$raw_rows = idx($cache_data, $user_phid, array());
|
||||||
if (!$raw_rows) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
$raw_data = ipull($raw_rows, 'cacheData', 'cacheKey');
|
$raw_data = ipull($raw_rows, 'cacheData', 'cacheKey');
|
||||||
|
|
||||||
foreach ($keys as $key) {
|
foreach ($keys as $key) {
|
||||||
|
|
|
@ -65,6 +65,7 @@ final class PhabricatorUser
|
||||||
|
|
||||||
private $settingCacheKeys = array();
|
private $settingCacheKeys = array();
|
||||||
private $settingCache = array();
|
private $settingCache = array();
|
||||||
|
private $allowInlineCacheGeneration;
|
||||||
|
|
||||||
protected function readField($field) {
|
protected function readField($field) {
|
||||||
switch ($field) {
|
switch ($field) {
|
||||||
|
@ -483,7 +484,11 @@ final class PhabricatorUser
|
||||||
}
|
}
|
||||||
|
|
||||||
$settings_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES;
|
$settings_key = PhabricatorUserPreferencesCacheType::KEY_PREFERENCES;
|
||||||
|
if ($this->getPHID()) {
|
||||||
$settings = $this->requireCacheData($settings_key);
|
$settings = $this->requireCacheData($settings_key);
|
||||||
|
} else {
|
||||||
|
$settings = array();
|
||||||
|
}
|
||||||
|
|
||||||
$defaults = PhabricatorSetting::getAllEnabledSettings($this);
|
$defaults = PhabricatorSetting::getAllEnabledSettings($this);
|
||||||
|
|
||||||
|
@ -1486,6 +1491,10 @@ final class PhabricatorUser
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setAllowInlineCacheGeneration($allow_cache_generation) {
|
||||||
|
$this->allowInlineCacheGeneration = $allow_cache_generation;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @task cache
|
* @task cache
|
||||||
|
@ -1506,6 +1515,12 @@ final class PhabricatorUser
|
||||||
return $usable_value;
|
return $usable_value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// By default, we throw if a cache isn't available. This is consistent
|
||||||
|
// with the standard `needX()` + `attachX()` + `getX()` interaction.
|
||||||
|
if (!$this->allowInlineCacheGeneration) {
|
||||||
|
throw new PhabricatorDataNotAttachedException($this);
|
||||||
|
}
|
||||||
|
|
||||||
$usable_value = $type->getDefaultValue();
|
$usable_value = $type->getDefaultValue();
|
||||||
|
|
||||||
$user_phid = $this->getPHID();
|
$user_phid = $this->getPHID();
|
||||||
|
|
|
@ -202,6 +202,14 @@ abstract class PhabricatorTestCase extends PhutilTestCase {
|
||||||
$editor->setActor($user);
|
$editor->setActor($user);
|
||||||
$editor->createNewUser($user, $email);
|
$editor->createNewUser($user, $email);
|
||||||
|
|
||||||
|
// When creating a new test user, we prefill their setting cache as empty.
|
||||||
|
// This is a little more efficient than doing a query to load the empty
|
||||||
|
// settings.
|
||||||
|
$user->attachRawCacheData(
|
||||||
|
array(
|
||||||
|
PhabricatorUserPreferencesCacheType::KEY_PREFERENCES => '[]',
|
||||||
|
));
|
||||||
|
|
||||||
return $user;
|
return $user;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue