Modernize search engine selection

Summary: Remove the `PhabricatorDefaultSearchEngineSelector` class. This is quite similar to D12053.

Test Plan: Went to `/view/PhabricatorSearchApplication/` and saw the storage engine configuration. Set `search.elastic.host` and saw the highlighted storage engine change.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12670

src/__phutil_library_map__.php

@@ -1744,7 +1744,6 @@ phutil_register_library_map(array(
     'PhabricatorDatabaseSetupCheck' => 'applications/config/check/PhabricatorDatabaseSetupCheck.php',
     'PhabricatorDateTimeSettingsPanel' => 'applications/settings/panel/PhabricatorDateTimeSettingsPanel.php',
     'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php',
-    'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php',
     'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php',
     'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php',
     'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php',
@@ -1776,7 +1775,7 @@ phutil_register_library_map(array(
     'PhabricatorEdgeType' => 'infrastructure/edges/type/PhabricatorEdgeType.php',
     'PhabricatorEditor' => 'infrastructure/PhabricatorEditor.php',
     'PhabricatorElasticSearchEngine' => 'applications/search/engine/PhabricatorElasticSearchEngine.php',
-    'PhabricatorElasticSetupCheck' => 'applications/config/check/PhabricatorElasticSetupCheck.php',
+    'PhabricatorElasticSearchSetupCheck' => 'applications/config/check/PhabricatorElasticSearchSetupCheck.php',
     'PhabricatorEmailAddressesSettingsPanel' => 'applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php',
     'PhabricatorEmailFormatSettingsPanel' => 'applications/settings/panel/PhabricatorEmailFormatSettingsPanel.php',
     'PhabricatorEmailLoginController' => 'applications/auth/controller/PhabricatorEmailLoginController.php',
@@ -2501,6 +2500,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchAbstractDocument' => 'applications/search/index/PhabricatorSearchAbstractDocument.php',
     'PhabricatorSearchApplication' => 'applications/search/application/PhabricatorSearchApplication.php',
     'PhabricatorSearchApplicationSearchEngine' => 'applications/search/query/PhabricatorSearchApplicationSearchEngine.php',
+    'PhabricatorSearchApplicationStorageEnginePanel' => 'applications/search/applicationpanel/PhabricatorSearchApplicationStorageEnginePanel.php',
     'PhabricatorSearchAttachController' => 'applications/search/controller/PhabricatorSearchAttachController.php',
     'PhabricatorSearchBaseController' => 'applications/search/controller/PhabricatorSearchBaseController.php',
     'PhabricatorSearchConfigOptions' => 'applications/search/config/PhabricatorSearchConfigOptions.php',
@@ -2516,7 +2516,6 @@ phutil_register_library_map(array(
     'PhabricatorSearchDocumentTypeDatasource' => 'applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php',
     'PhabricatorSearchEditController' => 'applications/search/controller/PhabricatorSearchEditController.php',
     'PhabricatorSearchEngine' => 'applications/search/engine/PhabricatorSearchEngine.php',
-    'PhabricatorSearchEngineSelector' => 'applications/search/selector/PhabricatorSearchEngineSelector.php',
     'PhabricatorSearchField' => 'applications/search/constants/PhabricatorSearchField.php',
     'PhabricatorSearchHovercardController' => 'applications/search/controller/PhabricatorSearchHovercardController.php',
     'PhabricatorSearchIndexer' => 'applications/search/index/PhabricatorSearchIndexer.php',
@@ -5140,7 +5139,6 @@ phutil_register_library_map(array(
     'PhabricatorDatabaseSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorDateTimeSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorDebugController' => 'PhabricatorController',
-    'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector',
     'PhabricatorDestructionEngine' => 'Phobject',
     'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorSettingsPanel',
@@ -5169,7 +5167,7 @@ phutil_register_library_map(array(
     'PhabricatorEdgeType' => 'Phobject',
     'PhabricatorEditor' => 'Phobject',
     'PhabricatorElasticSearchEngine' => 'PhabricatorSearchEngine',
-    'PhabricatorElasticSetupCheck' => 'PhabricatorSetupCheck',
+    'PhabricatorElasticSearchSetupCheck' => 'PhabricatorSetupCheck',
     'PhabricatorEmailAddressesSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorEmailFormatSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorEmailLoginController' => 'PhabricatorAuthController',
@@ -5973,6 +5971,7 @@ phutil_register_library_map(array(
     'PhabricatorScheduleTaskTriggerAction' => 'PhabricatorTriggerAction',
     'PhabricatorSearchApplication' => 'PhabricatorApplication',
     'PhabricatorSearchApplicationSearchEngine' => 'PhabricatorApplicationSearchEngine',
+    'PhabricatorSearchApplicationStorageEnginePanel' => 'PhabricatorApplicationConfigurationPanel',
     'PhabricatorSearchAttachController' => 'PhabricatorSearchBaseController',
     'PhabricatorSearchBaseController' => 'PhabricatorController',
     'PhabricatorSearchConfigOptions' => 'PhabricatorApplicationConfigOptions',

src/applications/config/check/PhabricatorElasticSearchSetupCheck.php

@@ -1,14 +1,15 @@
 <?php
 
-final class PhabricatorElasticSetupCheck extends PhabricatorSetupCheck {
+final class PhabricatorElasticSearchSetupCheck extends PhabricatorSetupCheck {
 
   public function getDefaultGroup() {
     return self::GROUP_OTHER;
   }
 
   protected function executeChecks() {
-    if (PhabricatorDefaultSearchEngineSelector::shouldUseElasticSearch()) {
+    if ($this->shouldUseElasticSearchEngine()) {
-      $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
+      $engine = new PhabricatorElasticSearchEngine();
+
       if (!$engine->indexExists()) {
         $summary = pht(
           'You enabled Elasticsearch but the index does not exist.');
@@ -40,4 +41,10 @@ final class PhabricatorElasticSetupCheck extends PhabricatorSetupCheck {
       }
     }
   }
+
+  protected function shouldUseElasticSearchEngine() {
+    $search_engine = PhabricatorSearchEngine::loadEngine();
+    return $search_engine instanceof PhabricatorElasticSearchEngine;
+  }
+
 }

src/applications/config/check/PhabricatorExtraConfigSetupCheck.php

@@ -252,6 +252,10 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
 
       'style.monospace' => $monospace_reason,
       'style.monospace.windows' => $monospace_reason,
+
+      'search.engine-selector' => pht(
+        'Phabricator now automatically discovers available search engines '.
+        'at runtime.'),
     );
 
     return $ancient_config;

src/applications/config/check/PhabricatorMySQLSetupCheck.php

@@ -116,7 +116,8 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
     }
 
     $stopword_file = self::loadRawConfigValue('ft_stopword_file');
-    if (!PhabricatorDefaultSearchEngineSelector::shouldUseElasticSearch()) {
+
+    if ($this->shouldUseMySQLSearchEngine()) {
       if ($stopword_file === null) {
         $summary = pht(
           'Your version of MySQL does not support configuration of a '.
@@ -190,7 +191,7 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
 
     $min_len = self::loadRawConfigValue('ft_min_word_len');
     if ($min_len >= 4) {
-      if (!PhabricatorDefaultSearchEngineSelector::shouldUseElasticSearch()) {
+      if ($this->shouldUseMySQLSearchEngine()) {
         $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace');
 
         $summary = pht(
@@ -235,8 +236,7 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
 
     $bool_syntax = self::loadRawConfigValue('ft_boolean_syntax');
     if ($bool_syntax != ' |-><()~*:""&^') {
-      if (!PhabricatorDefaultSearchEngineSelector::shouldUseElasticSearch()) {
+      if ($this->shouldUseMySQLSearchEngine()) {
-
         $summary = pht(
           'MySQL is configured to search on fulltext indexes using "OR" by '.
           'default. Using "AND" is usually the desired behaviour.');
@@ -340,4 +340,9 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
 
   }
 
+  protected function shouldUseMySQLSearchEngine() {
+    $search_engine = PhabricatorSearchEngine::loadEngine();
+    return $search_engine instanceof PhabricatorMySQLSearchEngine;
+  }
+
 }

src/applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php

@@ -61,8 +61,6 @@ final class PhabricatorFilesApplicationStorageEnginePanel
       );
     }
 
-    $table =
-
     $table = id(new AphrontTableView($rows))
       ->setNoDataString(pht('No storage engines available.'))
       ->setHeaders(

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -563,7 +563,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     $fulltext_query->setParameter('types',
       array(ManiphestTaskPHIDType::TYPECONST));
 
-    $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
+    $engine = PhabricatorSearchEngine::loadEngine();
     $fulltext_results = $engine->executeSearch($fulltext_query);
 
     if (empty($fulltext_results)) {

src/applications/search/applicationpanel/PhabricatorSearchApplicationStorageEnginePanel.php

@@ -0,0 +1,82 @@
+<?php
+
+final class PhabricatorSearchApplicationStorageEnginePanel
+  extends PhabricatorApplicationConfigurationPanel {
+
+  public function getPanelKey() {
+    return 'search';
+  }
+
+  public function shouldShowForApplication(
+    PhabricatorApplication $application) {
+    return $application instanceof PhabricatorSearchApplication;
+  }
+
+  public function buildConfigurationPagePanel() {
+    $viewer = $this->getViewer();
+    $application = $this->getApplication();
+
+    $active_engine = PhabricatorSearchEngine::loadEngine();
+    $engines = PhabricatorSearchEngine::loadAllEngines();
+
+    $rows = array();
+    $rowc = array();
+
+    foreach ($engines as $key => $engine) {
+      try {
+        $index_exists = $engine->indexExists() ? pht('Yes') : pht('No');
+      } catch (Exception $ex) {
+        $index_exists = pht('N/A');
+      }
+
+      try {
+        $index_is_sane = $engine->indexIsSane() ? pht('Yes') : pht('No');
+      } catch (Exception $ex) {
+        $index_is_sane = pht('N/A');
+      }
+
+      if ($engine == $active_engine) {
+        $rowc[] = 'highlighted';
+      } else {
+        $rowc[] = null;
+      }
+
+      $rows[] = array(
+        $key,
+        get_class($engine),
+        $index_exists,
+        $index_is_sane,
+      );
+    }
+
+    $table = id(new AphrontTableView($rows))
+      ->setNoDataString(pht('No search engines available.'))
+      ->setHeaders(
+        array(
+          pht('Key'),
+          pht('Class'),
+          pht('Index Exists'),
+          pht('Index Is Sane'),
+        ))
+      ->setRowClasses($rowc)
+      ->setColumnClasses(
+        array(
+          '',
+          'wide',
+          '',
+        ));
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Search Engines'))
+      ->appendChild($table);
+
+    return $box;
+  }
+
+  public function handlePanelRequest(
+    AphrontRequest $request,
+    PhabricatorController $controller) {
+    return new Aphront404Response();
+  }
+
+}

src/applications/search/config/PhabricatorSearchConfigOptions.php

@@ -21,21 +21,6 @@ final class PhabricatorSearchConfigOptions
 
   public function getOptions() {
     return array(
-      $this->newOption(
-        'search.engine-selector',
-        'class',
-        'PhabricatorDefaultSearchEngineSelector')
-        ->setBaseClass('PhabricatorSearchEngineSelector')
-        ->setSummary(pht('Search engine selector.'))
-        ->setDescription(
-          pht(
-            'Phabricator uses a search engine selector to choose which '.
-            'search engine to use when indexing and reconstructing '.
-            'documents, and when executing queries. You can override the '.
-            'engine selector to provide a new selector class which can '.
-            'select some custom engine you implement, if you want to store '.
-            'your documents in some search engine which does not have '.
-            'default support.')),
       $this->newOption('search.elastic.host', 'string', null)
         ->setLocked(true)
         ->setDescription(pht('Elastic Search host.'))

src/applications/search/engine/PhabricatorElasticSearchEngine.php

@@ -1,13 +1,31 @@
 <?php
 
 final class PhabricatorElasticSearchEngine extends PhabricatorSearchEngine {
+
   private $uri;
   private $index;
   private $timeout;
 
-  public function __construct($uri, $index) {
+  public function getEngineIdentifier() {
+    return 'elasticsearch';
+  }
+
+  public function getEnginePriority() {
+    return 10;
+  }
+
+  public function isEnabled() {
+    return (bool)PhabricatorEnv::getEnvConfig('search.elastic.host');
+  }
+
+  public function setURI($uri) {
     $this->uri = $uri;
+    return $this;
+  }
+
+  public function setIndex($index) {
     $this->index = $index;
+    return $this;
   }
 
   public function setTimeout($timeout) {
@@ -15,6 +33,14 @@ final class PhabricatorElasticSearchEngine extends PhabricatorSearchEngine {
     return $this;
   }
 
+  public function getURI() {
+    return $this->uri;
+  }
+
+  public function getIndex() {
+    return $this->index;
+  }
+
   public function getTimeout() {
     return $this->timeout;
   }
@@ -99,7 +125,7 @@ final class PhabricatorElasticSearchEngine extends PhabricatorSearchEngine {
       $spec[] = array(
         'simple_query_string' => array(
           'query'  => $query->getParameter('query'),
-          'fields' => array( 'field.corpus' ),
+          'fields' => array('field.corpus'),
         ),
       );
 

src/applications/search/engine/PhabricatorMySQLSearchEngine.php

@@ -2,12 +2,24 @@
 
 final class PhabricatorMySQLSearchEngine extends PhabricatorSearchEngine {
 
+  public function getEngineIdentifier() {
+    return 'mysql';
+  }
+
+  public function getEnginePriority() {
+    return 100;
+  }
+
+  public function isEnabled() {
+    return true;
+  }
+
   public function reindexAbstractDocument(
     PhabricatorSearchAbstractDocument $doc) {
 
     $phid = $doc->getPHID();
     if (!$phid) {
-      throw new Exception('Document has no PHID!');
+      throw new Exception(pht('Document has no PHID!'));
     }
 
     $store = new PhabricatorSearchDocument();
@@ -31,7 +43,7 @@ final class PhabricatorMySQLSearchEngine extends PhabricatorSearchEngine {
       queryfx(
         $conn_w,
         'INSERT INTO %T (phid, phidType, field, auxPHID, corpus) '.
-        ' VALUES (%s, %s, %s, %ns, %s)',
+        'VALUES (%s, %s, %s, %ns, %s)',
         $field_dao->getTableName(),
         $phid,
         $doc->getDocumentType(),
@@ -63,9 +75,9 @@ final class PhabricatorMySQLSearchEngine extends PhabricatorSearchEngine {
     if ($sql) {
       queryfx(
         $conn_w,
-        'INSERT INTO %T'.
+        'INSERT INTO %T '.
-        ' (phid, relatedPHID, relation, relatedType, relatedTime) '.
+        '(phid, relatedPHID, relation, relatedType, relatedTime) '.
-        ' VALUES %Q',
+        'VALUES %Q',
         $rship_dao->getTableName(),
         implode(', ', $sql));
     }

src/applications/search/engine/PhabricatorSearchEngine.php

@@ -1,6 +1,5 @@
 <?php
 
-
 /**
  * Base class for Phabricator search engine providers. Each engine must offer
  * three capabilities: indexing, searching, and reconstruction (this can be
@@ -8,6 +7,49 @@
  */
 abstract class PhabricatorSearchEngine {
 
+/* -(  Engine Metadata  )---------------------------------------------------- */
+
+  /**
+   * Return a unique, nonempty string which identifies this storage engine.
+   *
+   * @return string Unique string for this engine, max length 32.
+   * @task meta
+   */
+  abstract public function getEngineIdentifier();
+
+  /**
+   * Prioritize this engine relative to other engines.
+   *
+   * Engines with a smaller priority number get an opportunity to write files
+   * first. Generally, lower-latency filestores should have lower priority
+   * numbers, and higher-latency filestores should have higher priority
+   * numbers. Setting priority to approximately the number of milliseconds of
+   * read latency will generally produce reasonable results.
+   *
+   * In conjunction with filesize limits, the goal is to store small files like
+   * profile images, thumbnails, and text snippets in lower-latency engines,
+   * and store large files in higher-capacity engines.
+   *
+   * @return float Engine priority.
+   * @task meta
+   */
+  abstract public function getEnginePriority();
+
+  /**
+   * Return `true` if the engine is currently writable.
+   *
+   * Engines that are disabled or missing configuration should return `false`
+   * to prevent new writes. If writes were made with this engine in the past,
+   * the application may still try to perform reads.
+   *
+   * @return bool True if this engine can support new writes.
+   * @task meta
+   */
+  abstract public function isEnabled();
+
+
+/* -(  Managing Documents  )------------------------------------------------- */
+
   /**
    * Update the index for an abstract document.
    *
@@ -17,7 +59,6 @@ abstract class PhabricatorSearchEngine {
   abstract public function reindexAbstractDocument(
     PhabricatorSearchAbstractDocument $document);
 
-
   /**
    * Reconstruct the document for a given PHID. This is used for debugging
    * and does not need to be perfect if it is unreasonable to implement it.
@@ -27,7 +68,6 @@ abstract class PhabricatorSearchEngine {
    */
   abstract public function reconstructDocument($phid);
 
-
   /**
    * Execute a search query.
    *
@@ -53,9 +93,70 @@ abstract class PhabricatorSearchEngine {
   }
 
   /**
-   * Do any sort of setup for the search index
+   * Do any sort of setup for the search index.
    *
    * @return void
    */
   public function initIndex() {}
+
+
+/* -(  Loading Storage Engines  )-------------------------------------------- */
+
+  /**
+   * @task load
+   */
+  public static function loadAllEngines() {
+    static $engines;
+
+    if ($engines === null) {
+      $objects = id(new PhutilSymbolLoader())
+        ->setAncestorClass(__CLASS__)
+        ->loadObjects();
+
+      $map = array();
+      foreach ($objects as $engine) {
+        $key = $engine->getEngineIdentifier();
+        if (empty($map[$key])) {
+          $map[$key] = $engine;
+        } else {
+          throw new Exception(
+            pht(
+              'Search engines "%s" and "%s" have the same engine identifier '.
+              '"%s". Each storage engine must have a unique identifier.',
+              get_class($engine),
+              get_class($map[$key]),
+              $key));
+        }
+      }
+
+      $map = msort($map, 'getEnginePriority');
+
+      $engines = $map;
+    }
+
+    return $engines;
+  }
+
+  /**
+   * @task load
+   */
+  public static function loadActiveEngines() {
+    $engines = self::loadAllEngines();
+
+    $active = array();
+    foreach ($engines as $key => $engine) {
+      if (!$engine->isEnabled()) {
+        continue;
+      }
+
+      $active[$key] = $engine;
+    }
+
+    return $active;
+  }
+
+  public static function loadEngine() {
+    return head(self::loadActiveEngines());
+  }
+
 }

src/applications/search/index/PhabricatorSearchDocumentIndexer.php

@@ -69,7 +69,7 @@ abstract class PhabricatorSearchDocumentIndexer extends Phobject {
         $this->indexProjects($document, $object);
       }
 
-      $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
+      $engine = PhabricatorSearchEngine::loadEngine();
       try {
         $engine->reindexAbstractDocument($document);
       } catch (Exception $ex) {

src/applications/search/management/PhabricatorSearchManagementInitWorkflow.php

@@ -13,7 +13,7 @@ final class PhabricatorSearchManagementInitWorkflow
   public function execute(PhutilArgumentParser $args) {
     $console = PhutilConsole::getConsole();
 
-    $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
+    $engine = PhabricatorSearchEngine::loadEngine();
 
     $work_done = false;
     if (!$engine->indexExists()) {

src/applications/search/query/PhabricatorSearchDocumentQuery.php

@@ -74,7 +74,7 @@ final class PhabricatorSearchDocumentQuery
       ->setParameter('offset', $this->getOffset())
       ->setParameter('limit', $this->getRawResultLimit());
 
-    $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
+    $engine = PhabricatorSearchEngine::loadEngine();
 
     return $engine->executeSearch($query);
   }

src/applications/search/selector/PhabricatorDefaultSearchEngineSelector.php

@@ -1,19 +0,0 @@
-<?php
-
-final class PhabricatorDefaultSearchEngineSelector
-  extends PhabricatorSearchEngineSelector {
-
-  public function newEngine() {
-    if (self::shouldUseElasticSearch()) {
-      $elastic_host = PhabricatorEnv::getEnvConfig('search.elastic.host');
-      $elastic_index = PhabricatorEnv::getEnvConfig('search.elastic.namespace');
-      return new PhabricatorElasticSearchEngine($elastic_host, $elastic_index);
-    }
-    return new PhabricatorMySQLSearchEngine();
-  }
-
-  public static function shouldUseElasticSearch() {
-    return (bool)PhabricatorEnv::getEnvConfig('search.elastic.host');
-  }
-
-}

src/applications/search/selector/PhabricatorSearchEngineSelector.php

@@ -1,15 +0,0 @@
-<?php
-
-abstract class PhabricatorSearchEngineSelector {
-
-  final public function __construct() {
-    // <empty>
-  }
-
-  abstract public function newEngine();
-
-  final public static function newSelector() {
-    return PhabricatorEnv::newObjectFromConfig('search.engine-selector');
-  }
-
-}