Formalize custom Conduit fields on objects

Summary: Ref T9964. This just adds more structure to application fields, to make it harder to make typos and easier to validate them later. Test Plan: Viewed APIs, called some APIs, saw good documentation and correct results. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9964 Differential Revision: https://secure.phabricator.com/D14776
2024-11-19 05:12:41 +01:00 · 2015-12-14 06:42:21 -08:00 · 2015-12-14 06:42:21 -08:00 · 0a50219f1b
commit 0a50219f1b
parent 10cdc55cf7
12 changed files with 141 additions and 90 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -1897,6 +1897,7 @@ phutil_register_library_map(array(
    'PhabricatorConduitRequestExceptionHandler' => 'aphront/handler/PhabricatorConduitRequestExceptionHandler.php',
    'PhabricatorConduitResultInterface' => 'applications/conduit/interface/PhabricatorConduitResultInterface.php',
    'PhabricatorConduitSearchEngine' => 'applications/conduit/query/PhabricatorConduitSearchEngine.php',
+    'PhabricatorConduitSearchFieldSpecification' => 'applications/conduit/interface/PhabricatorConduitSearchFieldSpecification.php',
    'PhabricatorConduitTestCase' => '__tests__/PhabricatorConduitTestCase.php',
    'PhabricatorConduitToken' => 'applications/conduit/storage/PhabricatorConduitToken.php',
    'PhabricatorConduitTokenController' => 'applications/conduit/controller/PhabricatorConduitTokenController.php',
@ -6014,6 +6015,7 @@ phutil_register_library_map(array(
    'PhabricatorConduitRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
    'PhabricatorConduitResultInterface' => 'PhabricatorPHIDInterface',
    'PhabricatorConduitSearchEngine' => 'PhabricatorApplicationSearchEngine',
+    'PhabricatorConduitSearchFieldSpecification' => 'Phobject',
    'PhabricatorConduitTestCase' => 'PhabricatorTestCase',
    'PhabricatorConduitToken' => array(
      'PhabricatorConduitDAO',
--- a/src/applications/conduit/interface/PhabricatorConduitResultInterface.php
+++ b/src/applications/conduit/interface/PhabricatorConduitResultInterface.php
@ -16,10 +16,10 @@ interface PhabricatorConduitResultInterface

  public function getFieldSpecificationsForConduit() {
    return array(
-      'name' => array(
-        'type' => 'string',
-        'description' => pht('The name of the object.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('name')
+        ->setType('string')
+        ->setDescription(pht('The name of the object.')),
    );
  }

--- a/src/applications/conduit/interface/PhabricatorConduitSearchFieldSpecification.php
+++ b/src/applications/conduit/interface/PhabricatorConduitSearchFieldSpecification.php
@ -0,0 +1,37 @@
+<?php
+
+final class PhabricatorConduitSearchFieldSpecification
+  extends Phobject {
+
+  private $key;
+  private $type;
+  private $description;
+
+  public function setKey($key) {
+    $this->key = $key;
+    return $this;
+  }
+
+  public function getKey() {
+    return $this->key;
+  }
+
+  public function setType($type) {
+    $this->type = $type;
+    return $this;
+  }
+
+  public function getType() {
+    return $this->type;
+  }
+
+  public function setDescription($description) {
+    $this->description = $description;
+    return $this;
+  }
+
+  public function getDescription() {
+    return $this->description;
+  }
+
+}
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@ -399,30 +399,30 @@ final class ManiphestTask extends ManiphestDAO

  public function getFieldSpecificationsForConduit() {
    return array(
-      'title' => array(
-        'type' => 'string',
-        'description' => pht('The name of the object.'),
-      ),
-      'authorPHID' => array(
-        'type' => 'phid',
-        'description' => pht('Original task author.'),
-      ),
-      'ownerPHID' => array(
-        'type' => 'phid?',
-        'description' => pht('Current task owner.'),
-      ),
-      'status' => array(
-        'type' => 'string',
-        'description' => pht('Current task status.'),
-      ),
-      'priority' => array(
-        'type' => 'int',
-        'description' => pht('Task priority.'),
-      ),
-      'subpriority' => array(
-        'type' => 'double',
-        'description' => pht('Order within priority level.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('title')
+        ->setType('string')
+        ->setDescription(pht('The title of the task.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('authorPHID')
+        ->setType('phid')
+        ->setDescription(pht('Original task author.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('ownerPHID')
+        ->setType('phid?')
+        ->setDescription(pht('Current task owner, if task is assigned.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('status')
+        ->setType('string')
+        ->setDescription(pht('Task status.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('priority')
+        ->setType('int')
+        ->setDescription(pht('Task priority.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('subpriority')
+        ->setType('double')
+        ->setDescription(pht('Order within priority level.')),
    );
  }

--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@ -391,22 +391,22 @@ final class PhabricatorOwnersPackage

  public function getFieldSpecificationsForConduit() {
    return array(
-      'name' => array(
-        'type' => 'string',
-        'description' => pht('The name of the package.'),
-      ),
-      'description' => array(
-        'type' => 'string',
-        'description' => pht('The package description.'),
-      ),
-      'status' => array(
-        'type' => 'string',
-        'description' => pht('Active or archived status of the package.'),
-      ),
-      'owners' => array(
-        'type' => 'list<map<string, wild>>',
-        'description' => pht('List of package owners.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('name')
+        ->setType('string')
+        ->setDescription(pht('The name of the package.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('description')
+        ->setType('string')
+        ->setDescription(pht('The package description.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('status')
+        ->setType('string')
+        ->setDescription(pht('Active or archived status of the package.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('owners')
+        ->setType('list<map<string, wild>>')
+        ->setDescription(pht('List of package owners.')),
    );
  }

--- a/src/applications/paste/storage/PhabricatorPaste.php
+++ b/src/applications/paste/storage/PhabricatorPaste.php
@ -257,22 +257,22 @@ final class PhabricatorPaste extends PhabricatorPasteDAO

  public function getFieldSpecificationsForConduit() {
    return array(
-      'title' => array(
-        'type' => 'string',
-        'description' => pht('The name of the object.'),
-      ),
-      'authorPHID' => array(
-        'type' => 'phid',
-        'description' => pht('User PHID of the author.'),
-      ),
-      'language' => array(
-        'type' => 'string?',
-        'description' => pht('Language to use for syntax highlighting.'),
-      ),
-      'status' => array(
-        'type' => 'string',
-        'description' => pht('Active or archived status of the paste.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('title')
+        ->setType('string')
+        ->setDescription(pht('The title of the paste.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('authorPHID')
+        ->setType('phid')
+        ->setDescription(pht('User PHID of the author.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('language')
+        ->setType('string?')
+        ->setDescription(pht('Language to use for syntax highlighting.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('status')
+        ->setType('string')
+        ->setDescription(pht('Active or arhived status of the paste.')),
    );
  }

--- a/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php
+++ b/src/applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php
@ -23,11 +23,10 @@ final class PhabricatorPolicySearchEngineExtension

  public function getFieldSpecificationsForConduit($object) {
    return array(
-      'policy' => array(
-        'type' => 'map<string, wild>',
-        'description' => pht(
-          'Map of capabilities to current policies.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('policy')
+        ->setType('map<string, wild>')
+        ->setDescription(pht('Map of capabilities to current policies.')),
    );
  }

--- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
+++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
@ -1129,7 +1129,7 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
    $attachments = $this->getConduitSearchAttachments();

    // TODO: Validate this better.
-    $attachment_specs = $request->getValue('attachments');
+    $attachment_specs = $request->getValue('attachments', array());
    $attachments = array_select_keys(
      $attachments,
      array_keys($attachment_specs));
@ -1203,12 +1203,23 @@ abstract class PhabricatorApplicationSearchEngine extends Phobject {
    $extensions = $this->getConduitFieldExtensions();
    $object = $this->newQuery()->newResultObject();

-    $specifications = array();
+    $map = array();
    foreach ($extensions as $extension) {
-      $specifications += $extension->getFieldSpecificationsForConduit($object);
+      $specifications = $extension->getFieldSpecificationsForConduit($object);
+      foreach ($specifications as $specification) {
+        $key = $specification->getKey();
+        if (isset($map[$key])) {
+          throw new Exception(
+            pht(
+              'Two field specifications share the same key ("%s"). Each '.
+              'specification must have a unique key.',
+              $key));
+        }
+        $map[$key] = $specification;
+      }
    }

-    return $specifications;
+    return $map;
  }

  private function getEngineExtensions() {
--- a/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php
+++ b/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php
@ -373,8 +373,8 @@ EOTEXT

    $rows = array();
    foreach ($specs as $key => $spec) {
-      $type = idx($spec, 'type');
-      $description = idx($spec, 'description');
+      $type = $spec->getType();
+      $description = $spec->getDescription();
      $rows[] = array(
        $key,
        $type,
--- a/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php
+++ b/src/applications/search/engineextension/PhabricatorLiskSearchEngineExtension.php
@ -31,16 +31,16 @@ final class PhabricatorLiskSearchEngineExtension

  public function getFieldSpecificationsForConduit($object) {
    return array(
-      'dateCreated' => array(
-        'type' => 'int',
-        'description' => pht(
-          'Epoch timestamp when the object was created.'),
-      ),
-      'dateModified' => array(
-        'type' => 'int',
-        'description' => pht(
-          'Epoch timestamp when the object was last updated.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('dateCreated')
+        ->setType('int')
+        ->setDescription(
+          pht('Epoch timestamp when the object was created.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('dateModified')
+        ->setType('int')
+        ->setDescription(
+          pht('Epoch timestamp when the object was last updated.')),
    );
  }

--- a/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php
+++ b/src/applications/spaces/engineextension/PhabricatorSpacesSearchEngineExtension.php
@ -55,11 +55,11 @@ final class PhabricatorSpacesSearchEngineExtension

  public function getFieldSpecificationsForConduit($object) {
    return array(
-      'spacePHID' => array(
-        'type' => 'phid?',
-        'description' => pht(
-          'PHID of the policy space this object is part of.'),
-      ),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('spacePHID')
+        ->setType('phid?')
+        ->setDescription(
+          pht('PHID of the policy space this object is part of.')),
    );
  }

--- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php
+++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldSearchEngineExtension.php
@ -69,10 +69,12 @@ final class PhabricatorCustomFieldSearchEngineExtension
    $map = array();
    foreach ($fields->getFields() as $field) {
      $key = $field->getModernFieldKey();
-      $map[$key] = array(
-        'type' => 'wild',
-        'description' => $field->getFieldDescription(),
-      );
+
+      // TODO: These should have proper types.
+      $map[] = id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey($key)
+        ->setType('wild')
+        ->setDescription($field->getFieldDescription());
    }

    return $map;