Port MySQL settings to PHP

Summary: - Ports MySQL settings to PHP. - Removes "mysql.retries" -- this existed only because Magic Numbers Are Bad, but there is no concievable reason it should ever be set to anything other than 3. - Introduced "Hidden" config, which isn't visible from the web (for SaaS, we'll just mark anything with secret keys as "hidden"). - Introduced "Masked" config, which will be masked in darkconsole once that gets updated. - "Hidden" implies "Masked" and "Locked". - Moved "storage.default-namespace" here -- it probably makes more sense than core; this was my bad in T2255. - Put cancel button back for hidden/locked config. - Introduce 'class' config type. Test Plan: Viewed MySQL options. None are editable. Reviewers: codeblock, btrahan Reviewed By: codeblock CC: aran Maniphest Tasks: T2255 Differential Revision: https://secure.phabricator.com/D4326
2024-11-22 14:52:41 +01:00 · 2013-01-03 06:01:14 -08:00 · 2013-01-03 06:01:14 -08:00 · 0902543fc8
commit 0902543fc8
parent 3b3808c476
10 changed files with 207 additions and 49 deletions
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@ -180,9 +180,6 @@ return array(
  // (e.g., db.example.com:1234).
  'mysql.host' => 'localhost',

-  // The number of times to try reconnecting to the MySQL database
-  'mysql.connection-retries' => 3,
-
  // Phabricator supports PHP extensions MySQL and MySQLi. It is possible to
  // implement also other access mechanism (e.g. PDO_MySQL). The class must
  // extend AphrontMySQLDatabaseConnectionBase.
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -938,6 +938,7 @@ phutil_register_library_map(array(
    'PhabricatorMetaMTAViewController' => 'applications/metamta/controller/PhabricatorMetaMTAViewController.php',
    'PhabricatorMetaMTAWorker' => 'applications/metamta/PhabricatorMetaMTAWorker.php',
    'PhabricatorMustVerifyEmailController' => 'applications/auth/controller/PhabricatorMustVerifyEmailController.php',
+    'PhabricatorMySQLConfigOptions' => 'applications/config/option/PhabricatorMySQLConfigOptions.php',
    'PhabricatorMySQLFileStorageEngine' => 'applications/files/engine/PhabricatorMySQLFileStorageEngine.php',
    'PhabricatorNotificationBuilder' => 'applications/notification/builder/PhabricatorNotificationBuilder.php',
    'PhabricatorNotificationClearController' => 'applications/notification/controller/PhabricatorNotificationClearController.php',
@ -2262,6 +2263,7 @@ phutil_register_library_map(array(
    'PhabricatorMetaMTAViewController' => 'PhabricatorMetaMTAController',
    'PhabricatorMetaMTAWorker' => 'PhabricatorWorker',
    'PhabricatorMustVerifyEmailController' => 'PhabricatorAuthController',
+    'PhabricatorMySQLConfigOptions' => 'PhabricatorApplicationConfigOptions',
    'PhabricatorMySQLFileStorageEngine' => 'PhabricatorFileStorageEngine',
    'PhabricatorNotificationClearController' => 'PhabricatorNotificationController',
    'PhabricatorNotificationController' => 'PhabricatorController',
--- a/src/applications/config/controller/PhabricatorConfigEditController.php
+++ b/src/applications/config/controller/PhabricatorConfigEditController.php
@ -100,6 +100,15 @@ final class PhabricatorConfigEditController
      $error_view = id(new AphrontErrorView())
        ->setTitle(pht('You broke everything!'))
        ->setErrors($errors);
+    } else if ($option->getHidden()) {
+      $msg = pht(
+        "This configuration is hidden and can not be edited or viewed from ".
+        "the web interface.");
+
+      $error_view = id(new AphrontErrorView())
+        ->setTitle(pht('Configuration Hidden'))
+        ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
+        ->appendChild('<p>'.phutil_escape_html($msg).'</p>');
    } else if ($option->getLocked()) {
      $msg = pht(
        "This configuration is locked and can not be edited from the web ".
@ -111,10 +120,14 @@ final class PhabricatorConfigEditController
        ->appendChild('<p>'.phutil_escape_html($msg).'</p>');
    }

-    $control = $this->renderControl(
-      $option,
-      $display_value,
-      $e_value);
+    if ($option->getHidden()) {
+      $control = null;
+    } else {
+      $control = $this->renderControl(
+        $option,
+        $display_value,
+        $e_value);
+    }

    $engine = new PhabricatorMarkupEngine();
    $engine->addObject($option, 'description');
@ -135,14 +148,15 @@ final class PhabricatorConfigEditController
          ->setValue($description))
      ->appendChild($control);

+    $submit_control = id(new AphrontFormSubmitControl())
+      ->addCancelButton($done_uri);
+
    if (!$option->getLocked()) {
-      $form
-        ->appendChild(
-          id(new AphrontFormSubmitControl())
-            ->addCancelButton($done_uri)
-            ->setValue(pht('Save Config Entry')));
+      $submit_control->setValue(pht('Save Config Entry'));
    }

+    $form->appendChild($submit_control);
+
    $examples = $this->renderExamples($option);
    if ($examples) {
      $form->appendChild(
@ -151,10 +165,12 @@ final class PhabricatorConfigEditController
          ->setValue($examples));
    }

-    $form->appendChild(
-      id(new AphrontFormMarkupControl())
-        ->setLabel(pht('Default'))
-        ->setValue($this->renderDefaults($option)));
+    if (!$option->getHidden()) {
+      $form->appendChild(
+        id(new AphrontFormMarkupControl())
+          ->setLabel(pht('Default'))
+          ->setValue($this->renderDefaults($option)));
+    }

    $title = pht('Edit %s', $this->key);
    $short = pht('Edit');
@ -256,6 +272,20 @@ final class PhabricatorConfigEditController
            break;
        }
        break;
+      case 'class':
+        if (!class_exists($value)) {
+          $e_value = pht('Invalid');
+          $errors[] = pht('Class does not exist.');
+        } else {
+          $base = $option->getBaseClass();
+          if (!is_subclass_of($value, $base)) {
+            $e_value = pht('Invalid');
+            $errors[] = pht('Class is not of valid type.');
+          } else {
+            $set_value = $value;
+          }
+        }
+        break;
      default:
        $json = json_decode($value, true);
        if ($json === null && strtolower($value) != 'null') {
@ -319,11 +349,26 @@ final class PhabricatorConfigEditController
        $control = id(new AphrontFormSelectControl())
          ->setOptions(
            array(
-              ''      => '(Use Default)',
+              ''      => pht('(Use Default)'),
              'true'  => idx($option->getOptions(), 0),
              'false' => idx($option->getOptions(), 1),
            ));
        break;
+      case 'class':
+        $symbols = id(new PhutilSymbolLoader())
+          ->setType('class')
+          ->setAncestorClass($option->getBaseClass())
+          ->setConcreteOnly(true)
+          ->selectSymbolsWithoutLoading();
+        $names = ipull($symbols, 'name', 'name');
+        sort($names);
+        $names = array(
+          '' => pht('(Use Default)'),
+        ) + $names;
+
+        $control = id(new AphrontFormSelectControl())
+          ->setOptions($names);
+        break;
      case 'list<string>':
        $control = id(new AphrontFormTextAreaControl())
          ->setCaption(pht('Separate values with newlines or commas.'));
--- a/src/applications/config/controller/PhabricatorConfigGroupController.php
+++ b/src/applications/config/controller/PhabricatorConfigGroupController.php
@ -67,22 +67,24 @@ final class PhabricatorConfigGroupController

    $list = new PhabricatorObjectItemListView();
    foreach ($options as $option) {
-      $current_value = PhabricatorEnv::getEnvConfig($option->getKey());
-      $current_value = $this->prettyPrintJSON($current_value);
-      $current_value = phutil_render_tag(
-        'div',
-        array(
-          'class' => 'config-options-current-value',
-        ),
-        '<span>'.pht('Current Value:').'</span> '.
-        phutil_escape_html($current_value));
-
-
      $item = id(new PhabricatorObjectItemView())
        ->setHeader($option->getKey())
        ->setHref('/config/edit/'.$option->getKey().'/')
-        ->addAttribute(phutil_escape_html($option->getSummary()))
-        ->appendChild($current_value);
+        ->addAttribute(phutil_escape_html($option->getSummary()));
+
+      if (!$option->getHidden()) {
+        $current_value = PhabricatorEnv::getEnvConfig($option->getKey());
+        $current_value = $this->prettyPrintJSON($current_value);
+        $current_value = phutil_render_tag(
+          'div',
+          array(
+            'class' => 'config-options-current-value',
+          ),
+          '<span>'.pht('Current Value:').'</span> '.
+          phutil_escape_html($current_value));
+
+        $item->appendChild($current_value);
+      }

      $db_value = idx($db_values, $option->getKey());
      if ($db_value && !$db_value->getIsDeleted()) {
@ -91,7 +93,9 @@ final class PhabricatorConfigGroupController
        $item->addIcon('edit-grey', pht('Default'));
      }

-      if ($option->getLocked()) {
+      if ($option->getHidden()) {
+        $item->addIcon('unpublish', pht('Hidden'));
+      } else if ($option->getLocked()) {
        $item->addIcon('lock', pht('Locked'));
      }

--- a/src/applications/config/option/PhabricatorApplicationConfigOptions.php
+++ b/src/applications/config/option/PhabricatorApplicationConfigOptions.php
@ -41,6 +41,21 @@ abstract class PhabricatorApplicationConfigOptions extends Phobject {
              $option->getKey()));
        }
        break;
+      case 'class':
+        $symbols = id(new PhutilSymbolLoader())
+          ->setType('class')
+          ->setAncestorClass($option->getBaseClass())
+          ->setConcreteOnly(true)
+          ->selectSymbolsWithoutLoading();
+        $names = ipull($symbols, 'name', 'name');
+        if (empty($names[$value])) {
+          throw new PhabricatorConfigValidationException(
+            pht(
+              "Option '%s' value must name a class extending '%s'.",
+              $option->getKey(),
+              $option->getBaseClass()));
+        }
+        break;
      case 'list<string>':
        $valid = true;
        if (!is_array($value)) {
--- a/src/applications/config/option/PhabricatorConfigOption.php
+++ b/src/applications/config/option/PhabricatorConfigOption.php
@ -13,6 +13,39 @@ final class PhabricatorConfigOption
  private $group;
  private $examples;
  private $locked;
+  private $hidden;
+  private $masked;
+  private $baseClass;
+
+  public function setBaseClass($base_class) {
+    $this->baseClass = $base_class;
+    return $this;
+  }
+
+  public function getBaseClass() {
+    return $this->baseClass;
+  }
+
+  public function setMasked($masked) {
+    $this->masked = $masked;
+    return $this;
+  }
+
+  public function getMasked() {
+    if ($this->getHidden()) {
+      return true;
+    }
+    return $this->masked;
+  }
+
+  public function setHidden($hidden) {
+    $this->hidden = $hidden;
+    return $this;
+  }
+
+  public function getHidden() {
+    return $this->hidden;
+  }

  public function setLocked($locked) {
    $this->locked = $locked;
@ -20,6 +53,9 @@ final class PhabricatorConfigOption
  }

  public function getLocked() {
+    if ($this->getHidden()) {
+      return true;
+    }
    return $this->locked;
  }

--- a/src/applications/config/option/PhabricatorCoreConfigOptions.php
+++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php
@ -65,19 +65,6 @@ final class PhabricatorCoreConfigOptions
            "and a call to 'Leap Into Action'. If you'd prefer more ".
            "traditional UI strings like 'Submit', you can set this flag to ".
            "disable most of the jokes and easter eggs.")),
-       $this->newOption('storage.default-namespace', 'string', 'phabricator')
-        // NOTE: Lock this, since editing it from the web torpedoes an install.
-        ->setLocked(true)
-        ->setSummary(
-          pht("The namespace that Phabricator databases should use."))
-        ->setDescription(
-          pht(
-            "Phabricator puts databases in a namespace, which defualts to ".
-            "'phabricator' -- for instance, the Differential database is ".
-            "named 'phabricator_differential' by default. You can change ".
-            "this namespace if you want. Normally, you should not do this ".
-            "unless you are developing Phabricator and using namespaces to ".
-            "separate multiple sandbox datasets.")),
       $this->newOption('environment.append-paths', 'list<string>', null)
        ->setSummary(
          pht("These paths get appended to your \$PATH envrionment variable."))
--- a/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php
+++ b/src/applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php
@ -15,7 +15,7 @@ final class PhabricatorExtendingPhabricatorConfigOptions
    return array(
      $this->newOption('load-libraries', 'list<string>', null)
        ->setSummary(pht("Paths to additional phutil libraries to load."))
-        ->addExample('/srv/our-sekrit-libs/sekrit-phutil', 'Valid Setting'),
+        ->addExample('/srv/our-libs/sekrit-phutil', pht('Valid Setting')),
      $this->newOption('events.listeners', 'list<string>', null)
        ->setSummary(
          pht("Listeners receive callbacks when interesting things occur."))
@ -25,7 +25,7 @@ final class PhabricatorExtendingPhabricatorConfigOptions
            "listeners, which will receive callbacks when interesting things ".
            "occur. Specify a list of classes which extend ".
            "PhabricatorEventListener here."))
-        ->addExample('MyEventListener', 'Valid Setting'),
+        ->addExample('MyEventListener', pht('Valid Setting')),
      $this->newOption(
        'celerity.resource-path',
        'string',
@ -36,7 +36,7 @@ final class PhabricatorExtendingPhabricatorConfigOptions
          pht(
            "Path to custom celerity resource map relative to ".
            "'phabricator/src'. See also `scripts/celerity_mapper.php`."))
-        ->addExample('local/my_celerity_map.php', 'Valid Setting'),
+        ->addExample('local/my_celerity_map.php', pht('Valid Setting')),
    );
  }

--- a/src/applications/config/option/PhabricatorMySQLConfigOptions.php
+++ b/src/applications/config/option/PhabricatorMySQLConfigOptions.php
@ -0,0 +1,73 @@
+<?php
+
+final class PhabricatorMySQLConfigOptions
+  extends PhabricatorApplicationConfigOptions {
+
+  public function getName() {
+    return pht("MySQL");
+  }
+
+  public function getDescription() {
+    return pht("Database configuration.");
+  }
+
+  public function getOptions() {
+    return array(
+      $this->newOption('mysql.host', 'string', 'localhost')
+        ->setLocked(true)
+        ->setDescription(
+          pht("MySQL database hostname."))
+        ->addExample('localhost', pht('MySQL on this machine'))
+        ->addExample('db.example.com:3300', pht('Nonstandard port')),
+      $this->newOption('mysql.user', 'string', 'root')
+        ->setLocked(true)
+        ->setDescription(
+          pht("MySQL username to use when connecting to the database.")),
+      $this->newOption('mysql.pass', 'string', null)
+        ->setHidden(true)
+        ->setDescription(
+          pht("MySQL password to use when connecting to the database.")),
+      $this->newOption(
+        'mysql.configuration-provider',
+        'class',
+        'DefaultDatabaseConfigurationProvider')
+        ->setLocked(true)
+        ->setBaseClass('DatabaseConfigurationProvider')
+        ->setSummary(
+          pht('Configure database configuration class.'))
+        ->setDescription(
+          pht(
+            "Phabricator chooses which database to connect to through a ".
+            "swappable configuration provider. You almost certainly do not ".
+            "need to change this.")),
+      $this->newOption(
+        'mysql.implementation',
+        'class',
+        'AphrontMySQLDatabaseConnection')
+        ->setLocked(true)
+        ->setBaseClass('AphrontMySQLDatabaseConnectionBase')
+        ->setSummary(
+          pht('Configure database connection class.'))
+        ->setDescription(
+          pht(
+            "Phabricator connects to MySQL through a swappable abstraction ".
+            "layer. You can choose an alternate implementation by setting ".
+            "this option. To provide your own implementation, extend ".
+            "`AphrontMySQLDatabaseConnectionBase`. It is very unlikely that ".
+            "you need to change this.")),
+      $this->newOption('storage.default-namespace', 'string', 'phabricator')
+        ->setLocked(true)
+        ->setSummary(
+          pht("The namespace that Phabricator databases should use."))
+        ->setDescription(
+          pht(
+            "Phabricator puts databases in a namespace, which defaults to ".
+            "'phabricator' -- for instance, the Differential database is ".
+            "named 'phabricator_differential' by default. You can change ".
+            "this namespace if you want. Normally, you should not do this ".
+            "unless you are developing Phabricator and using namespaces to ".
+            "separate multiple sandbox datasets.")),
+    );
+  }
+
+}
--- a/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php
+++ b/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php
@ -103,7 +103,6 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
      'mysql.configuration-provider',
      array($this, $mode, $namespace));

-    $retries = PhabricatorEnv::getEnvConfig('mysql.connection-retries');
    return PhabricatorEnv::newObjectFromConfig(
      'mysql.implementation',
      array(
@ -112,7 +111,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
          'pass'      => $conf->getPassword(),
          'host'      => $conf->getHost(),
          'database'  => $conf->getDatabase(),
-          'retries'   => $retries,
+          'retries'   => 3,
        ),
      ));
  }