Begin modularizing fields in SearchEngines

Summary:
Ref T8441. Ref T7715. Add a layer of indirection to fields in search engines. This will allow us to:

  - Simplify SearchEngine code, which has collected a lot of duplication around expressing what is effectively field types.
  - Automatically add fields like "Spaces" and "Projects" (primary driver for T8441).
  - Reorder or hide fields (not sure if we really want to do this, but it seems plausible, and this will let us play around with it, at least).
  - Drive Conduit Query methods via SearchEngines, so the same code specifies both the search UI and the `application.query` endpoint (primary driver in T7715).

Test Plan:
  - Searched for stuff in Paste, everything behaved exaclty like it used to (except that I removed the "no language" checkbox, which seemed like fluff from a bygone era).
  - Searched for stuff in other applications, saw no changes.
  - Hit date field errors.
  - Used query strings to specify values.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7715, T8441

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

src/__phutil_library_map__.php

@@ -2503,6 +2503,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchController' => 'applications/search/controller/PhabricatorSearchController.php',
     'PhabricatorSearchDAO' => 'applications/search/storage/PhabricatorSearchDAO.php',
     'PhabricatorSearchDatasource' => 'applications/search/typeahead/PhabricatorSearchDatasource.php',
+    'PhabricatorSearchDateField' => 'applications/search/field/PhabricatorSearchDateField.php',
     'PhabricatorSearchDeleteController' => 'applications/search/controller/PhabricatorSearchDeleteController.php',
     'PhabricatorSearchDocument' => 'applications/search/storage/document/PhabricatorSearchDocument.php',
     'PhabricatorSearchDocumentField' => 'applications/search/storage/document/PhabricatorSearchDocumentField.php',
@@ -2513,6 +2514,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchDocumentTypeDatasource' => 'applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php',
     'PhabricatorSearchEditController' => 'applications/search/controller/PhabricatorSearchEditController.php',
     'PhabricatorSearchEngine' => 'applications/search/engine/PhabricatorSearchEngine.php',
+    'PhabricatorSearchField' => 'applications/search/field/PhabricatorSearchField.php',
     'PhabricatorSearchHovercardController' => 'applications/search/controller/PhabricatorSearchHovercardController.php',
     'PhabricatorSearchIndexer' => 'applications/search/index/PhabricatorSearchIndexer.php',
     'PhabricatorSearchManagementIndexWorkflow' => 'applications/search/management/PhabricatorSearchManagementIndexWorkflow.php',
@@ -2523,6 +2525,9 @@ phutil_register_library_map(array(
     'PhabricatorSearchRelationship' => 'applications/search/constants/PhabricatorSearchRelationship.php',
     'PhabricatorSearchResultView' => 'applications/search/view/PhabricatorSearchResultView.php',
     'PhabricatorSearchSelectController' => 'applications/search/controller/PhabricatorSearchSelectController.php',
+    'PhabricatorSearchStringListField' => 'applications/search/field/PhabricatorSearchStringListField.php',
+    'PhabricatorSearchTokenizerField' => 'applications/search/field/PhabricatorSearchTokenizerField.php',
+    'PhabricatorSearchUsersField' => 'applications/search/field/PhabricatorSearchUsersField.php',
     'PhabricatorSearchWorker' => 'applications/search/worker/PhabricatorSearchWorker.php',
     'PhabricatorSecurityConfigOptions' => 'applications/config/option/PhabricatorSecurityConfigOptions.php',
     'PhabricatorSecuritySetupCheck' => 'applications/config/check/PhabricatorSecuritySetupCheck.php',
@@ -4679,6 +4684,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationPanelController' => 'PhabricatorApplicationsController',
     'PhabricatorApplicationQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorApplicationSearchController' => 'PhabricatorSearchBaseController',
+    'PhabricatorApplicationSearchEngine' => 'Phobject',
     'PhabricatorApplicationStatusView' => 'AphrontView',
     'PhabricatorApplicationTransaction' => array(
       'PhabricatorLiskDAO',
@@ -5995,6 +6001,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchController' => 'PhabricatorSearchBaseController',
     'PhabricatorSearchDAO' => 'PhabricatorLiskDAO',
     'PhabricatorSearchDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
+    'PhabricatorSearchDateField' => 'PhabricatorSearchField',
     'PhabricatorSearchDeleteController' => 'PhabricatorSearchBaseController',
     'PhabricatorSearchDocument' => 'PhabricatorSearchDAO',
     'PhabricatorSearchDocumentField' => 'PhabricatorSearchDAO',
@@ -6004,6 +6011,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchDocumentRelationship' => 'PhabricatorSearchDAO',
     'PhabricatorSearchDocumentTypeDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhabricatorSearchEditController' => 'PhabricatorSearchBaseController',
+    'PhabricatorSearchField' => 'Phobject',
     'PhabricatorSearchHovercardController' => 'PhabricatorSearchBaseController',
     'PhabricatorSearchManagementIndexWorkflow' => 'PhabricatorSearchManagementWorkflow',
     'PhabricatorSearchManagementInitWorkflow' => 'PhabricatorSearchManagementWorkflow',
@@ -6012,6 +6020,9 @@ phutil_register_library_map(array(
     'PhabricatorSearchPreferencesSettingsPanel' => 'PhabricatorSettingsPanel',
     'PhabricatorSearchResultView' => 'AphrontView',
     'PhabricatorSearchSelectController' => 'PhabricatorSearchBaseController',
+    'PhabricatorSearchStringListField' => 'PhabricatorSearchField',
+    'PhabricatorSearchTokenizerField' => 'PhabricatorSearchField',
+    'PhabricatorSearchUsersField' => 'PhabricatorSearchTokenizerField',
     'PhabricatorSearchWorker' => 'PhabricatorWorker',
     'PhabricatorSecurityConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorSecuritySetupCheck' => 'PhabricatorSetupCheck',

src/applications/paste/query/PhabricatorPasteSearchEngine.php

@@ -11,24 +11,6 @@ final class PhabricatorPasteSearchEngine
     return 'PhabricatorPasteApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
-    $saved = new PhabricatorSavedQuery();
-    $saved->setParameter(
-      'authorPHIDs',
-      $this->readUsersFromRequest($request, 'authors'));
-
-    $languages = $request->getStrList('languages');
-    if ($request->getBool('noLanguage')) {
-      $languages[] = null;
-    }
-    $saved->setParameter('languages', $languages);
-
-    $saved->setParameter('createdStart', $request->getStr('createdStart'));
-    $saved->setParameter('createdEnd', $request->getStr('createdEnd'));
-
-    return $saved;
-  }
-
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
     $query = id(new PhabricatorPasteQuery())
       ->needContent(true)
@@ -49,49 +31,22 @@ final class PhabricatorPasteSearchEngine
     return $query;
   }
 
-  public function buildSearchForm(
+  protected function buildCustomSearchFields() {
-    AphrontFormView $form,
+    return array(
-    PhabricatorSavedQuery $saved_query) {
+      id(new PhabricatorSearchUsersField())
-    $author_phids = $saved_query->getParameter('authorPHIDs', array());
+        ->setAliases(array('authors'))
-
+        ->setKey('authorPHIDs')
-    $languages = $saved_query->getParameter('languages', array());
+        ->setLabel(pht('Authors')),
-    $no_language = false;
+      id(new PhabricatorSearchStringListField())
-    foreach ($languages as $key => $language) {
+        ->setKey('languages')
-      if ($language === null) {
+        ->setLabel(pht('Languages')),
-        $no_language = true;
+      id(new PhabricatorSearchDateField())
-        unset($languages[$key]);
+        ->setKey('createdStart')
-        continue;
+        ->setLabel(pht('Created After')),
-      }
+      id(new PhabricatorSearchDateField())
-    }
+        ->setKey('createdEnd')
-
+        ->setLabel(pht('Created Before')),
-    $form
+    );
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorPeopleDatasource())
-          ->setName('authors')
-          ->setLabel(pht('Authors'))
-          ->setValue($author_phids))
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setName('languages')
-          ->setLabel(pht('Languages'))
-          ->setValue(implode(', ', $languages)))
-      ->appendChild(
-        id(new AphrontFormCheckboxControl())
-          ->addCheckbox(
-            'noLanguage',
-            1,
-            pht('Find Pastes with no specified language.'),
-            $no_language));
-
-    $this->buildDateRange(
-      $form,
-      $saved_query,
-      'createdStart',
-      pht('Created After'),
-      'createdEnd',
-      pht('Created Before'));
-
   }
 
   protected function getURI($path) {

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -14,7 +14,7 @@
  * @task exec       Paging and Executing Queries
  * @task render     Rendering Results
  */
-abstract class PhabricatorApplicationSearchEngine {
+abstract class PhabricatorApplicationSearchEngine extends Phobject {
 
   private $application;
   private $viewer;
@@ -69,8 +69,20 @@ abstract class PhabricatorApplicationSearchEngine {
    * @param AphrontRequest The search request.
    * @return PhabricatorSavedQuery
    */
-  abstract public function buildSavedQueryFromRequest(
+  public function buildSavedQueryFromRequest(AphrontRequest $request) {
-    AphrontRequest $request);
+    $fields = $this->buildSearchFields();
+    $viewer = $this->requireViewer();
+
+    $saved = new PhabricatorSavedQuery();
+    foreach ($fields as $field) {
+      $field->setViewer($viewer);
+
+      $value = $field->readValueFromRequest($request);
+      $saved->setParameter($field->getKey(), $value);
+    }
+
+    return $saved;
+  }
 
   /**
    * Executes the saved query.
@@ -88,9 +100,42 @@ abstract class PhabricatorApplicationSearchEngine {
    * @param PhabricatorSavedQuery The query from which to build the form.
    * @return void
    */
-  abstract public function buildSearchForm(
+  public function buildSearchForm(
     AphrontFormView $form,
-    PhabricatorSavedQuery $query);
+    PhabricatorSavedQuery $saved) {
+
+    $fields = $this->buildSearchFields();
+    $viewer = $this->requireViewer();
+
+    foreach ($fields as $field) {
+      $field->setViewer($viewer);
+      $field->readValueFromSavedQuery($saved);
+    }
+
+    foreach ($fields as $field) {
+      foreach ($field->getErrors() as $error) {
+        $this->addError(last($error));
+      }
+    }
+
+    foreach ($fields as $field) {
+      $field->appendToForm($form);
+    }
+  }
+
+  protected function buildSearchFields() {
+    $fields = array();
+
+    foreach ($this->buildCustomSearchFields() as $field) {
+      $fields[] = $field;
+    }
+
+    return $fields;
+  }
+
+  protected function buildCustomSearchFields() {
+    throw new PhutilMethodNotImplementedException();
+  }
 
   public function getErrors() {
     return $this->errors;

src/applications/search/field/PhabricatorSearchDateField.php

@@ -0,0 +1,37 @@
+<?php
+
+final class PhabricatorSearchDateField
+  extends PhabricatorSearchField {
+
+  protected function newControl() {
+    return new AphrontFormTextControl();
+  }
+
+  protected function getValueFromRequest(AphrontRequest $request, $key) {
+    return $request->getStr($key);
+  }
+
+  protected function validateControlValue($value) {
+    if (!strlen($value)) {
+      return;
+    }
+
+    $epoch = $this->parseDateTime($value);
+    if ($epoch) {
+      return;
+    }
+
+    $this->addError(
+      pht('Invalid'),
+      pht('Date value for "%s" can not be parsed.', $this->getLabel()));
+  }
+
+  protected function parseDateTime($value) {
+    if (!strlen($value)) {
+      return null;
+    }
+
+    return PhabricatorTime::parseLocalTime($value, $this->getViewer());
+  }
+
+}

src/applications/search/field/PhabricatorSearchField.php

@@ -0,0 +1,260 @@
+<?php
+
+/**
+ * @task config Configuring Fields
+ * @task error Handling Errors
+ * @task io Reading and Writing Field Values
+ * @task util Utility Methods
+ */
+abstract class PhabricatorSearchField extends Phobject {
+
+  private $key;
+  private $viewer;
+  private $value;
+  private $label;
+  private $aliases = array();
+  private $errors = array();
+
+
+/* -(  Configuring Fields  )------------------------------------------------- */
+
+
+  /**
+   * Set the primary key for the field, like `projectPHIDs`.
+   *
+   * You can set human-readable aliases with @{method:setAliases}.
+   *
+   * The key should be a short, unique (within a search engine) string which
+   * does not contain any special characters.
+   *
+   * @param string Unique key which identifies the field.
+   * @return this
+   * @task config
+   */
+  public function setKey($key) {
+    $this->key = $key;
+    return $this;
+  }
+
+
+  /**
+   * Get the field's key.
+   *
+   * @return string Unique key for this field.
+   * @task config
+   */
+  public function getKey() {
+    return $this->key;
+  }
+
+
+  /**
+   * Set a human-readable label for the field.
+   *
+   * This should be a short text string, like "Reviewers" or "Colors".
+   *
+   * @param string Short, human-readable field label.
+   * @return this
+   * task config
+   */
+  public function setLabel($label) {
+    $this->label = $label;
+    return $this;
+  }
+
+
+  /**
+   * Get the field's human-readable label.
+   *
+   * @return string Short, human-readable field label.
+   * @task config
+   */
+  public function getLabel() {
+    return $this->label;
+  }
+
+
+  /**
+   * Set the acting viewer.
+   *
+   * Engines do not need to do this explicitly; it will be done on their
+   * behalf by the caller.
+   *
+   * @param PhabricatorUser Viewer.
+   * @return this
+   * @task config
+   */
+  public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+
+  /**
+   * Get the acting viewer.
+   *
+   * @return PhabricatorUser Viewer.
+   * @task config
+   */
+  public function getViewer() {
+    return $this->viewer;
+  }
+
+
+  /**
+   * Provide alternate field aliases, usually more human-readable versions
+   * of the key.
+   *
+   * These aliases can be used when building GET requests, so you can provide
+   * an alias like `authors` to let users write `&authors=alincoln` instead of
+   * `&authorPHIDs=alincoln`. This is a little easier to use.
+   *
+   * @param list<string> List of aliases for this field.
+   * @return this
+   * @task config
+   */
+  public function setAliases(array $aliases) {
+    $this->aliases = $aliases;
+    return $this;
+  }
+
+
+  /**
+   * Get aliases for this field.
+   *
+   * @return list<string> List of aliases for this field.
+   * @task config
+   */
+  public function getAliases() {
+    return $this->aliases;
+  }
+
+
+/* -(  Handling Errors  )---------------------------------------------------- */
+
+
+  protected function addError($short, $long) {
+    $this->errors[] = array($short, $long);
+    return $this;
+  }
+
+  public function getErrors() {
+    return $this->errors;
+  }
+
+  protected function validateControlValue($value) {
+    return;
+  }
+
+  protected function getShortError() {
+    $error = head($this->getErrors());
+    if ($error) {
+      return head($error);
+    }
+    return null;
+  }
+
+
+/* -(  Reading and Writing Field Values  )----------------------------------- */
+
+
+  public function readValueFromRequest(AphrontRequest $request) {
+    $check = array_merge(array($this->getKey()), $this->getAliases());
+    foreach ($check as $key) {
+      if ($this->getValueExistsInRequest($request, $key)) {
+        return $this->getValueFromRequest($request, $key);
+      }
+    }
+    return $this->getDefaultValue();
+  }
+
+  protected function getValueExistsInRequest(AphrontRequest $request, $key) {
+    return $request->getExists($key);
+  }
+
+  abstract protected function getValueFromRequest(
+    AphrontRequest $request,
+    $key);
+
+  public function readValueFromSavedQuery(PhabricatorSavedQuery $saved) {
+    $value = $saved->getParameter(
+      $this->getKey(),
+      $this->getDefaultValue());
+    $this->value = $this->didReadValueFromSavedQuery($value);
+    $this->validateControlValue($value);
+    return $this;
+  }
+
+  protected function didReadValueFromSavedQuery($value) {
+    return $value;
+  }
+
+  public function getValue() {
+    return $this->value;
+  }
+
+  protected function getValueForControl() {
+    return $this->value;
+  }
+
+  protected function getDefaultValue() {
+    return null;
+  }
+
+
+/* -(  Rendering Controls  )------------------------------------------------- */
+
+
+  abstract protected function newControl();
+
+
+  protected function renderControl() {
+    // TODO: We should `setError($this->getShortError())` here, but it looks
+    // terrible in the form layout.
+
+    return $this->newControl()
+      ->setValue($this->getValueForControl())
+      ->setName($this->getKey())
+      ->setLabel($this->getLabel());
+  }
+
+  public function appendToForm(AphrontFormView $form) {
+    $form->appendControl($this->renderControl());
+    return $this;
+  }
+
+
+/* -(  Utility Methods )----------------------------------------------------- */
+
+
+  /**
+   * Read a list of items from the request, in either array format or string
+   * format:
+   *
+   *   list[]=item1&list[]=item2
+   *   list=item1,item2
+   *
+   * This provides flexibility when constructing URIs, especially from external
+   * sources.
+   *
+   * @param AphrontRequest  Request to read strings from.
+   * @param string          Key to read in the request.
+   * @return list<string>   List of values.
+   * @task utility
+   */
+  protected function getListFromRequest(
+    AphrontRequest $request,
+    $key) {
+
+    $list = $request->getArr($key, null);
+    if ($list === null) {
+      $list = $request->getStrList($key);
+    }
+
+    if (!$list) {
+      return array();
+    }
+
+    return $list;
+  }
+}

src/applications/search/field/PhabricatorSearchStringListField.php

@@ -0,0 +1,22 @@
+<?php
+
+final class PhabricatorSearchStringListField
+  extends PhabricatorSearchField {
+
+  protected function getDefaultValue() {
+    return array();
+  }
+
+  protected function getValueFromRequest(AphrontRequest $request, $key) {
+    return $request->getStrList($key);
+  }
+
+  protected function newControl() {
+    return new AphrontFormTextControl();
+  }
+
+  protected function getValueForControl() {
+    return implode(', ', parent::getValueForControl());
+  }
+
+}

src/applications/search/field/PhabricatorSearchTokenizerField.php

@@ -0,0 +1,21 @@
+<?php
+
+abstract class PhabricatorSearchTokenizerField
+  extends PhabricatorSearchField {
+
+  protected function getDefaultValue() {
+    return array();
+  }
+
+  protected function getValueFromRequest(AphrontRequest $request, $key) {
+    return $this->getListFromRequest($request, $key);
+  }
+
+  protected function newControl() {
+    return id(new AphrontFormTokenizerControl())
+      ->setDatasource($this->newDatasource());
+  }
+
+  abstract protected function newDatasource();
+
+}

src/applications/search/field/PhabricatorSearchUsersField.php

@@ -0,0 +1,55 @@
+<?php
+
+final class PhabricatorSearchUsersField
+  extends PhabricatorSearchTokenizerField {
+
+  protected function getDefaultValue() {
+    return array();
+  }
+
+  protected function newDatasource() {
+    // TODO: Make this use PhabricatorPeopleUserFunctionDatasource once field
+    // support is a little more powerful.
+    return new PhabricatorPeopleDatasource();
+  }
+
+  protected function getValueFromRequest(AphrontRequest $request, $key) {
+    $list = $this->getListFromRequest($request, $key);
+    $allow_types = array();
+
+    $phids = array();
+    $names = array();
+    $allow_types = array_fuse($allow_types);
+    $user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
+    foreach ($list as $item) {
+      $type = phid_get_type($item);
+      if ($type == $user_type) {
+        $phids[] = $item;
+      } else if (isset($allow_types[$type])) {
+        $phids[] = $item;
+      } else {
+        if (PhabricatorTypeaheadDatasource::isFunctionToken($item)) {
+          // If this is a function, pass it through unchanged; we'll evaluate
+          // it later.
+          $phids[] = $item;
+        } else {
+          $names[] = $item;
+        }
+      }
+    }
+
+    if ($names) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer($this->getViewer())
+        ->withUsernames($names)
+        ->execute();
+      foreach ($users as $user) {
+        $phids[] = $user->getPHID();
+      }
+      $phids = array_unique($phids);
+    }
+
+    return $phids;
+  }
+
+}