Add basic typechecking support to Conduit

Summary:
Ref T9964. I want to show users what we're expecting in "constraints", and let constraints like "authors=epriestley" work to make things easier.

I'm generally very happy with the "HTTPParameterType" stuff from EditEngine, so add a parallel set of "ConduitParameterType" classes. These are a little simpler than the HTTP ones, but have a little more validation logic.

Test Plan:
This is really just a proof of concept; some of these fields are now filled in:

{F1023845}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

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

src/__phutil_library_map__.php

@@ -231,14 +231,18 @@ phutil_register_library_map(array(
     'ConduitException' => 'applications/conduit/protocol/exception/ConduitException.php',
     'ConduitGetCapabilitiesConduitAPIMethod' => 'applications/conduit/method/ConduitGetCapabilitiesConduitAPIMethod.php',
     'ConduitGetCertificateConduitAPIMethod' => 'applications/conduit/method/ConduitGetCertificateConduitAPIMethod.php',
+    'ConduitListParameterType' => 'applications/conduit/parametertype/ConduitListParameterType.php',
     'ConduitLogGarbageCollector' => 'applications/conduit/garbagecollector/ConduitLogGarbageCollector.php',
     'ConduitMethodDoesNotExistException' => 'applications/conduit/protocol/exception/ConduitMethodDoesNotExistException.php',
     'ConduitMethodNotFoundException' => 'applications/conduit/protocol/exception/ConduitMethodNotFoundException.php',
+    'ConduitParameterType' => 'applications/conduit/parametertype/ConduitParameterType.php',
     'ConduitPingConduitAPIMethod' => 'applications/conduit/method/ConduitPingConduitAPIMethod.php',
     'ConduitQueryConduitAPIMethod' => 'applications/conduit/method/ConduitQueryConduitAPIMethod.php',
     'ConduitResultSearchEngineExtension' => 'applications/conduit/query/ConduitResultSearchEngineExtension.php',
     'ConduitSSHWorkflow' => 'applications/conduit/ssh/ConduitSSHWorkflow.php',
+    'ConduitStringListParameterType' => 'applications/conduit/parametertype/ConduitStringListParameterType.php',
     'ConduitTokenGarbageCollector' => 'applications/conduit/garbagecollector/ConduitTokenGarbageCollector.php',
+    'ConduitUserListParameterType' => 'applications/conduit/parametertype/ConduitUserListParameterType.php',
     'ConpherenceColumnViewController' => 'applications/conpherence/controller/ConpherenceColumnViewController.php',
     'ConpherenceConduitAPIMethod' => 'applications/conpherence/conduit/ConpherenceConduitAPIMethod.php',
     'ConpherenceConfigOptions' => 'applications/conpherence/config/ConpherenceConfigOptions.php',
@@ -4071,14 +4075,18 @@ phutil_register_library_map(array(
     'ConduitException' => 'Exception',
     'ConduitGetCapabilitiesConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitGetCertificateConduitAPIMethod' => 'ConduitAPIMethod',
+    'ConduitListParameterType' => 'ConduitParameterType',
     'ConduitLogGarbageCollector' => 'PhabricatorGarbageCollector',
     'ConduitMethodDoesNotExistException' => 'ConduitMethodNotFoundException',
     'ConduitMethodNotFoundException' => 'ConduitException',
+    'ConduitParameterType' => 'Phobject',
     'ConduitPingConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitQueryConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitResultSearchEngineExtension' => 'PhabricatorSearchEngineExtension',
     'ConduitSSHWorkflow' => 'PhabricatorSSHWorkflow',
+    'ConduitStringListParameterType' => 'ConduitListParameterType',
     'ConduitTokenGarbageCollector' => 'PhabricatorGarbageCollector',
+    'ConduitUserListParameterType' => 'ConduitListParameterType',
     'ConpherenceColumnViewController' => 'ConpherenceController',
     'ConpherenceConduitAPIMethod' => 'ConduitAPIMethod',
     'ConpherenceConfigOptions' => 'PhabricatorApplicationConfigOptions',

src/applications/conduit/parametertype/ConduitListParameterType.php

@@ -0,0 +1,37 @@
+<?php
+
+abstract class ConduitListParameterType
+  extends ConduitParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    $value = parent::getParameterValue();
+
+    if (!is_array($value)) {
+      $this->raiseValidationException(
+        $request,
+        $key,
+        pht('Expected a list, but value is not a list.'));
+    }
+
+    $actual_keys = array_keys($value);
+    if ($value) {
+      $natural_keys = range(0, count($value) - 1);
+    } else {
+      $natural_keys = array();
+    }
+
+    if ($actual_keys !== $natural_keys) {
+      $this->raiseValidationException(
+        $request,
+        $key,
+        pht('Expected a list, but value is an object.'));
+    }
+
+    return $value;
+  }
+
+  protected function getParameterDefault() {
+    return array();
+  }
+
+}

src/applications/conduit/parametertype/ConduitParameterType.php

@@ -0,0 +1,97 @@
+<?php
+
+/**
+ * Defines how to read a value from a Conduit request.
+ *
+ * This class behaves like @{class:AphrontHTTPParameterType}, but for Conduit.
+ */
+abstract class ConduitParameterType extends Phobject {
+
+
+  private $viewer;
+
+
+  final public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+
+  final public function getViewer() {
+    if (!$this->viewer) {
+      throw new PhutilInvalidStateException('setViewer');
+    }
+    return $this->viewer;
+  }
+
+
+  final public function getExists(array $request, $key) {
+    return $this->getValueExists($request, $key);
+  }
+
+
+  final public function getValue(array $request, $key) {
+    if (!$this->getExists($request, $key)) {
+      return $this->getParameterDefault();
+    }
+
+    return $this->getParameterValue($request, $key);
+  }
+
+
+  final public function getDefaultValue() {
+    return $this->getParameterDefault();
+  }
+
+
+  final public function getTypeName() {
+    return $this->getParameterTypeName();
+  }
+
+
+  final public function getFormatDescriptions() {
+    return $this->getParameterFormatDescriptions();
+  }
+
+
+  final public function getExamples() {
+    return $this->getParameterExamples();
+  }
+
+  protected function raiseValidationException(array $request, $key, $message) {
+    // TODO: Specialize this so we can give users more tailored messages from
+    // Conduit.
+    throw new Exception($message);
+  }
+
+
+  final public static function getAllTypes() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getTypeName')
+      ->setSortMethod('getTypeName')
+      ->execute();
+  }
+
+
+  protected function getParameterExists(array $request, $key) {
+    return array_key_exists($key, $request);
+  }
+
+  protected function getParameterValue(array $request, $key) {
+    return $request[$key];
+  }
+
+  abstract protected function getParameterTypeName();
+
+
+  abstract protected function getParameterFormatDescriptions();
+
+
+  abstract protected function getParameterExamples();
+
+  protected function getParameterDefault() {
+    return null;
+  }
+
+}

src/applications/conduit/parametertype/ConduitStringListParameterType.php

@@ -0,0 +1,40 @@
+<?php
+
+final class ConduitStringListParameterType
+  extends ConduitListParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    $list = parent::getParameterValue();
+
+    foreach ($list as $idx => $item) {
+      if (!is_string($item)) {
+        $this->raiseValidationException(
+          $request,
+          $key,
+          pht(
+            'Expected a list of strings, but item with index "%s" is '.
+            'not a string.',
+            $idx));
+      }
+    }
+
+    return $list;
+  }
+
+  protected function getParameterTypeName() {
+    return 'list<string>';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('List of strings.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      '["mango", "nectarine"]',
+    );
+  }
+
+}

src/applications/conduit/parametertype/ConduitUserListParameterType.php

@@ -0,0 +1,33 @@
+<?php
+
+final class ConduitUserListParameterType
+  extends ConduitListParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    $list = parent::getParameterValue();
+    return id(new PhabricatorUserPHIDResolver())
+      ->setViewer($this->getViewer())
+      ->resolvePHIDs($list);
+  }
+
+  protected function getParameterTypeName() {
+    return 'list<user>';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('List of user PHIDs.'),
+      pht('List of usernames.'),
+      pht('List with a mixture of PHIDs and usernames.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      '["PHID-USER-1111"]',
+      '["alincoln"]',
+      '["PHID-USER-2222", "alincoln"]',
+    );
+  }
+
+}

src/applications/conduit/protocol/ConduitAPIRequest.php

@@ -14,6 +14,10 @@ final class ConduitAPIRequest extends Phobject {
     return coalesce(idx($this->params, $key), $default);
   }
 
+  public function getValueExists($key) {
+    return array_key_exists($key, $this->params);
+  }
+
   public function getAllParameters() {
     return $this->params;
   }

src/applications/paste/query/PhabricatorPasteSearchEngine.php

@@ -47,6 +47,7 @@ final class PhabricatorPasteSearchEngine
       id(new PhabricatorUsersSearchField())
         ->setAliases(array('authors'))
         ->setKey('authorPHIDs')
+        ->setConduitKey('authors')
         ->setLabel(pht('Authors')),
       id(new PhabricatorSearchStringListField())
         ->setKey('languages')

src/applications/people/searchfield/PhabricatorUsersSearchField.php

@@ -15,4 +15,8 @@ final class PhabricatorUsersSearchField
     return new PhabricatorPeopleUserFunctionDatasource();
   }
 
+  protected function newConduitParameterType() {
+    return new ConduitUserListParameterType();
+  }
+
 }

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -1173,7 +1173,34 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
   }
 
   public function getSearchFieldsForConduit() {
-    $fields = $this->buildSearchFields();
+    $standard_fields = $this->buildSearchFields();
+
+    $fields = array();
+    foreach ($standard_fields as $field_key => $field) {
+      $conduit_key = $field->getConduitKey();
+
+      if (isset($fields[$conduit_key])) {
+        $other = $fields[$conduit_key];
+        $other_key = $other->getKey();
+
+        throw new Exception(
+          pht(
+            'SearchFields "%s" (of class "%s") and "%s" (of class "%s") both '.
+            'define the same Conduit key ("%s"). Keys must be unique.',
+            $field_key,
+            get_class($field),
+            $other_key,
+            get_class($other),
+            $conduit_key));
+      }
+
+      $fields[$conduit_key] = $field;
+    }
+
+    $viewer = $this->requireViewer();
+    foreach ($fields as $key => $field) {
+      $field->setViewer($viewer);
+    }
 
     // These are handled separately for Conduit, so don't show them as
     // supported.
@@ -1187,7 +1214,6 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
 
   public function buildConduitResponse(ConduitAPIRequest $request) {
     $viewer = $this->requireViewer();
-    $fields = $this->buildSearchFields();
 
     $query_key = $request->getValue('queryKey');
     if (!strlen($query_key)) {
@@ -1207,12 +1233,16 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
       }
     }
 
-    foreach ($fields as $field) {
-      $field->setViewer($viewer);
-    }
-
     $constraints = $request->getValue('constraints', array());
 
+    $fields = $this->getSearchFieldsForConduit();
+
+    foreach ($fields as $key => $field) {
+      if (!$field->getConduitParameterType()) {
+        unset($fields[$key]);
+      }
+    }
+
     foreach ($fields as $field) {
       if (!$field->getValueExistsInConduitRequest($constraints)) {
         continue;

src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php

@@ -154,14 +154,20 @@ EOTEXT
     $table[] = "| `ids` | **IDs** | `list<int>` | {$desc_ids} |";
     $table[] = "| `phids` | **PHIDs** | `list<phid>` | {$desc_phids} |";
     foreach ($fields as $field) {
-      $key = $field->getKeyForConduit();
+      $key = $field->getConduitKey();
       $label = $field->getLabel();
 
-      // TODO: Support generating and surfacing this information.
+      $type_object = $field->getConduitParameterType();
-      $type = pht('TODO');
+      if ($type_object) {
-      $description = pht('TODO');
+        $type = '`'.$type_object->getTypeName().'`';
+        // TODO: Support generating and surfacing this information.
+        $description = pht('TODO');
+      } else {
+        $type = '';
+        $description = '//'.pht('Not Supported').'//';
+      }
 
-      $table[] = "| `{$key}` | **{$label}** | `{$type}` | {$description}";
+      $table[] = "| `{$key}` | **{$label}** | {$type} | {$description}";
     }
     $table = implode("\n", $table);
     $out[] = $table;

src/applications/search/field/PhabricatorSearchCheckboxesField.php

@@ -37,4 +37,8 @@ final class PhabricatorSearchCheckboxesField
     return $control;
   }
 
+  protected function newConduitParameterType() {
+    return new ConduitStringListParameterType();
+  }
+
 }

src/applications/search/field/PhabricatorSearchCustomFieldProxyField.php

@@ -38,7 +38,7 @@ final class PhabricatorSearchCustomFieldProxyField
     return null;
   }
 
-  public function getKeyForConduit() {
+  public function getConduitKey() {
     return $this->getCustomField()->getModernFieldKey();
   }
 

src/applications/search/field/PhabricatorSearchField.php

@@ -4,11 +4,13 @@
  * @task config Configuring Fields
  * @task error Handling Errors
  * @task io Reading and Writing Field Values
+ * @task conduit Integration with Conduit
  * @task util Utility Methods
  */
 abstract class PhabricatorSearchField extends Phobject {
 
   private $key;
+  private $conduitKey;
   private $viewer;
   private $value;
   private $label;
@@ -130,6 +132,37 @@ abstract class PhabricatorSearchField extends Phobject {
   }
 
 
+  /**
+   * Provide an alternate field key for Conduit.
+   *
+   * This can allow you to choose a more usable key for API endpoints.
+   * If no key is provided, the main key is used.
+   *
+   * @param string Alternate key for Conduit.
+   * @return this
+   * @task config
+   */
+  public function setConduitKey($conduit_key) {
+    $this->conduitKey = $conduit_key;
+    return $this;
+  }
+
+
+  /**
+   * Get the field key for use in Conduit.
+   *
+   * @return string Conduit key for this field.
+   * @task config
+   */
+  public function getConduitKey() {
+    if ($this->conduitKey !== null) {
+      return $this->conduitKey;
+    }
+
+    return $this->getKey();
+  }
+
+
 /* -(  Handling Errors  )---------------------------------------------------- */
 
 
@@ -205,14 +238,6 @@ abstract class PhabricatorSearchField extends Phobject {
     return $value;
   }
 
-  public function getValueExistsInConduitRequest(array $constraints) {
-    return array_key_exists($this->getKey(), $constraints);
-  }
-
-  public function readValueFromConduitRequest(array $constraints) {
-    return idx($constraints, $this->getKey());
-  }
-
 
 /* -(  Rendering Controls  )------------------------------------------------- */
 
@@ -238,6 +263,39 @@ abstract class PhabricatorSearchField extends Phobject {
   }
 
 
+/* -(  Integration with Conduit  )------------------------------------------- */
+
+
+  /**
+   * @task conduit
+   */
+  final public function getConduitParameterType() {
+    $type = $this->newConduitParameterType();
+
+    if ($type) {
+      $type->setViewer($this->getViewer());
+    }
+
+    return $type;
+  }
+
+  protected function newConduitParameterType() {
+    return null;
+  }
+
+  public function getValueExistsInConduitRequest(array $constraints) {
+    return $this->getConduitParameterType()->getExists(
+      $constraints,
+      $this->getConduitKey());
+  }
+
+  public function readValueFromConduitRequest(array $constraints) {
+    return $this->getConduitParameterType()->getValue(
+      $constraints,
+      $this->getConduitKey());
+  }
+
+
 /* -(  Utility Methods )----------------------------------------------------- */
 
 
@@ -273,12 +331,5 @@ abstract class PhabricatorSearchField extends Phobject {
   }
 
 
-  public function getKeyForConduit() {
-    // TODO: This shouldn't really be different, but internal handling of
-    // custom field keys is a bit of a mess for now.
-    return $this->getKey();
-  }
-
-
 
 }

src/applications/search/field/PhabricatorSearchStringListField.php

@@ -19,4 +19,8 @@ final class PhabricatorSearchStringListField
     return implode(', ', parent::getValueForControl());
   }
 
+  protected function newConduitParameterType() {
+    return new ConduitStringListParameterType();
+  }
+
 }