mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-03 19:31:02 +01:00
getConfigEnv fails fast when key is not found and no default value is given.
Summary: T2345 getConfig throws an Exception when the key does not exist. Also removes dead code that throws an Exception. Test Plan: Reloaded the Phabricator home page. In the process, found 2 Exceptions thrown due to nonexistent keys. After addressing these problems, the home page loads without Exceptions. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4541
This commit is contained in:
parent
06bca93b47
commit
3e6fa43658
13 changed files with 29 additions and 53 deletions
|
@ -41,7 +41,7 @@ final class PhabricatorConfigOption
|
|||
}
|
||||
|
||||
return idx(
|
||||
PhabricatorEnv::getEnvConfig('config.mask', array()),
|
||||
PhabricatorEnv::getEnvConfig('config.mask'),
|
||||
$this->getKey(),
|
||||
false);
|
||||
}
|
||||
|
@ -57,7 +57,7 @@ final class PhabricatorConfigOption
|
|||
}
|
||||
|
||||
return idx(
|
||||
PhabricatorEnv::getEnvConfig('config.hide', array()),
|
||||
PhabricatorEnv::getEnvConfig('config.hide'),
|
||||
$this->getKey(),
|
||||
false);
|
||||
}
|
||||
|
@ -77,7 +77,7 @@ final class PhabricatorConfigOption
|
|||
}
|
||||
|
||||
return idx(
|
||||
PhabricatorEnv::getEnvConfig('config.lock', array()),
|
||||
PhabricatorEnv::getEnvConfig('config.lock'),
|
||||
$this->getKey(),
|
||||
false);
|
||||
}
|
||||
|
|
|
@ -127,6 +127,9 @@ final class PhabricatorCoreConfigOptions
|
|||
$this->newOption('phabricator.setup', 'bool', false)
|
||||
->setLocked(true)
|
||||
->setDescription(pht('Internal / deprecated.')),
|
||||
$this->newOption('test.value', 'wild', null)
|
||||
->setLocked(true)
|
||||
->setDescription(pht('Unit test value.')),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -562,11 +562,11 @@ final class DifferentialRevisionViewController extends DifferentialController {
|
|||
$status = $revision->getStatus();
|
||||
|
||||
$allow_self_accept = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-self-accept', false);
|
||||
'differential.allow-self-accept');
|
||||
$always_allow_close = PhabricatorEnv::getEnvConfig(
|
||||
'differential.always-allow-close', false);
|
||||
'differential.always-allow-close');
|
||||
$allow_reopen = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-reopen', false);
|
||||
'differential.allow-reopen');
|
||||
|
||||
if ($viewer_is_owner) {
|
||||
switch ($status) {
|
||||
|
|
|
@ -99,11 +99,11 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
|
|||
$actor_phid = $actor->getPHID();
|
||||
$actor_is_author = ($actor_phid == $revision->getAuthorPHID());
|
||||
$allow_self_accept = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-self-accept', false);
|
||||
'differential.allow-self-accept');
|
||||
$always_allow_close = PhabricatorEnv::getEnvConfig(
|
||||
'differential.always-allow-close', false);
|
||||
'differential.always-allow-close');
|
||||
$allow_reopen = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-reopen', false);
|
||||
'differential.allow-reopen');
|
||||
$revision_status = $revision->getStatus();
|
||||
|
||||
$revision->loadRelationships();
|
||||
|
@ -652,7 +652,7 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
|
|||
$removed_reviewers = $this->getRemovedReviewers();
|
||||
$reviewer_phids = $revision->getReviewers();
|
||||
$allow_self_accept = PhabricatorEnv::getEnvConfig(
|
||||
'differential.allow-self-accept', false);
|
||||
'differential.allow-self-accept');
|
||||
|
||||
$reviewer_phids_map = array_fill_keys($reviewer_phids, true);
|
||||
foreach ($added_reviewers as $k => $user_phid) {
|
||||
|
|
|
@ -154,7 +154,7 @@ final class DifferentialCommentMail extends DifferentialMail {
|
|||
$hunk_parser = new DifferentialHunkParser();
|
||||
|
||||
if (PhabricatorEnv::getEnvConfig(
|
||||
'metamta.differential.unified-comment-context', false)) {
|
||||
'metamta.differential.unified-comment-context')) {
|
||||
foreach ($changesets as $changeset) {
|
||||
$changeset->attachHunks($changeset->loadHunks());
|
||||
}
|
||||
|
@ -176,7 +176,7 @@ final class DifferentialCommentMail extends DifferentialMail {
|
|||
$inline_content = $inline->getContent();
|
||||
|
||||
if (!PhabricatorEnv::getEnvConfig(
|
||||
'metamta.differential.unified-comment-context', false)) {
|
||||
'metamta.differential.unified-comment-context')) {
|
||||
$body[] = $this->formatText("{$file}:{$range} {$inline_content}");
|
||||
} else {
|
||||
$body[] = "================";
|
||||
|
|
|
@ -370,8 +370,7 @@ final class DifferentialChangesetParser {
|
|||
|
||||
if (!$generated_guess) {
|
||||
$generated_path_regexps = PhabricatorEnv::getEnvConfig(
|
||||
'differential.generated-paths',
|
||||
array());
|
||||
'differential.generated-paths');
|
||||
foreach ($generated_path_regexps as $regexp) {
|
||||
if (preg_match($regexp, $this->changeset->getFilename())) {
|
||||
$generated_guess = true;
|
||||
|
|
|
@ -258,8 +258,7 @@ final class DifferentialDiff extends DifferentialDAO {
|
|||
->loadOneWhere('phid = %s', $this->getAuthorPHID());
|
||||
|
||||
$use_emails =
|
||||
PhabricatorEnv::getEnvConfig('differential.expose-emails-prudently',
|
||||
false);
|
||||
PhabricatorEnv::getEnvConfig('differential.expose-emails-prudently');
|
||||
|
||||
switch ($this->getSourceControlSystem()) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||
|
|
|
@ -104,7 +104,7 @@ final class PhabricatorFeedStoryPublisher {
|
|||
$this->sendNotification($chrono_key);
|
||||
}
|
||||
|
||||
$uris = PhabricatorEnv::getEnvConfig('feed.http-hooks', array());
|
||||
$uris = PhabricatorEnv::getEnvConfig('feed.http-hooks');
|
||||
foreach ($uris as $uri) {
|
||||
$task = PhabricatorWorker::scheduleTask(
|
||||
'FeedPublisherWorker',
|
||||
|
|
|
@ -84,9 +84,7 @@ final class ManiphestTaskPriority extends ManiphestConstants {
|
|||
*/
|
||||
public static function getDefaultPriority() {
|
||||
return PhabricatorEnv::getEnvConfig(
|
||||
'maniphest.default-priority',
|
||||
self::PRIORITY_TRIAGE
|
||||
);
|
||||
'maniphest.default-priority');
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -738,10 +738,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
|
|||
* specified by the `metamta.user-address-format` configuration value.
|
||||
*/
|
||||
private function getUserName($user) {
|
||||
$format = PhabricatorEnv::getEnvConfig(
|
||||
'metamta.user-address-format',
|
||||
'full'
|
||||
);
|
||||
$format = PhabricatorEnv::getEnvConfig('metamta.user-address-format');
|
||||
|
||||
switch ($format) {
|
||||
case 'short':
|
||||
|
|
|
@ -14,7 +14,6 @@ final class PhabricatorSearchAbstractDocument {
|
|||
private $relationships = array();
|
||||
|
||||
public static function getSupportedTypes() {
|
||||
$more = PhabricatorEnv::getEnvConfig('search.more-document-types', array());
|
||||
return array(
|
||||
PhabricatorPHIDConstants::PHID_TYPE_DREV => 'Differential Revisions',
|
||||
PhabricatorPHIDConstants::PHID_TYPE_CMIT => 'Repository Commits',
|
||||
|
@ -22,7 +21,7 @@ final class PhabricatorSearchAbstractDocument {
|
|||
PhabricatorPHIDConstants::PHID_TYPE_WIKI => 'Phriction Documents',
|
||||
PhabricatorPHIDConstants::PHID_TYPE_USER => 'Phabricator Users',
|
||||
PhabricatorPHIDConstants::PHID_TYPE_QUES => 'Ponder Questions',
|
||||
) + $more;
|
||||
);
|
||||
}
|
||||
|
||||
public function setPHID($phid) {
|
||||
|
|
10
src/infrastructure/env/PhabricatorEnv.php
vendored
10
src/infrastructure/env/PhabricatorEnv.php
vendored
|
@ -227,11 +227,17 @@ final class PhabricatorEnv {
|
|||
/**
|
||||
* Get the current configuration setting for a given key.
|
||||
*
|
||||
* If the key is not found, then throw an Exception.
|
||||
*
|
||||
* @task read
|
||||
*/
|
||||
public static function getEnvConfig($key, $default = null) {
|
||||
public static function getEnvConfig($key) {
|
||||
$result = self::$sourceStack->getKeys(array($key));
|
||||
return idx($result, $key, $default);
|
||||
if (array_key_exists($key, $result)) {
|
||||
return $result[$key];
|
||||
} else {
|
||||
throw new Exception("No config value specified for key '{$key}'.");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
|
|
@ -187,7 +187,6 @@ abstract class LiskDAO {
|
|||
private $__missingFields = array();
|
||||
private static $processIsolationLevel = 0;
|
||||
private static $transactionIsolationLevel = 0;
|
||||
private static $__checkedClasses = array();
|
||||
|
||||
private $__ephemeral = false;
|
||||
|
||||
|
@ -213,30 +212,6 @@ abstract class LiskDAO {
|
|||
|
||||
if ($this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
|
||||
$this->resetDirtyFields();
|
||||
|
||||
$this_class = get_class($this);
|
||||
if (empty(self::$__checkedClasses[$this_class])) {
|
||||
self::$__checkedClasses = true;
|
||||
|
||||
if (PhabricatorEnv::getEnvConfig('lisk.check_property_methods')) {
|
||||
$class = new ReflectionClass(get_class($this));
|
||||
$methods = $class->getMethods();
|
||||
$properties = $this->getProperties();
|
||||
foreach ($methods as $method) {
|
||||
$name = strtolower($method->getName());
|
||||
if (!(strncmp($name, 'get', 3) && strncmp($name, 'set', 3))) {
|
||||
$name = substr($name, 3);
|
||||
$declaring_class_name = $method->getDeclaringClass()->getName();
|
||||
if (isset($properties[$name]) &&
|
||||
$declaring_class_name !== 'LiskDAO') {
|
||||
throw new Exception(
|
||||
"Cannot implement method {$method->getName()} in ".
|
||||
"{$declaring_class_name}.");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue