Use CustomFields to power Conduit auxiliary dictionaries

Summary: Ref T2222. Moves this Conduit stuff over.

Test Plan: Made Conduit calls, saw data in results.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

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

src/__phutil_library_map__.php

@@ -2722,10 +2722,10 @@ phutil_register_library_map(array(
     'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod',
     'ConduitAPI_differential_getdiff_Method' => 'ConduitAPIMethod',
     'ConduitAPI_differential_getrawdiff_Method' => 'ConduitAPIMethod',
-    'ConduitAPI_differential_getrevision_Method' => 'ConduitAPIMethod',
+    'ConduitAPI_differential_getrevision_Method' => 'ConduitAPI_differential_Method',
     'ConduitAPI_differential_getrevisioncomments_Method' => 'ConduitAPI_differential_Method',
     'ConduitAPI_differential_parsecommitmessage_Method' => 'ConduitAPIMethod',
-    'ConduitAPI_differential_query_Method' => 'ConduitAPIMethod',
+    'ConduitAPI_differential_query_Method' => 'ConduitAPI_differential_Method',
     'ConduitAPI_differential_querydiffs_Method' => 'ConduitAPIMethod',
     'ConduitAPI_differential_setdiffproperty_Method' => 'ConduitAPIMethod',
     'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPI_differential_Method',

src/applications/differential/conduit/ConduitAPI_differential_Method.php

@@ -148,4 +148,30 @@ abstract class ConduitAPI_differential_Method extends ConduitAPIMethod {
     $editor->applyTransactions($revision, $xactions);
   }
 
+  protected function loadCustomFieldsForRevisions(
+    PhabricatorUser $viewer,
+    array $revisions) {
+    assert_instances_of($revisions, 'DifferentialRevision');
+
+    $results = array();
+    foreach ($revisions as $revision) {
+      // TODO: This is inefficient and issues a query for each object.
+      $field_list = PhabricatorCustomField::getObjectFields(
+        $revision,
+        PhabricatorCustomField::ROLE_CONDUIT);
+
+      $field_list
+        ->setViewer($viewer)
+        ->readFieldsFromStorage($revision);
+
+      foreach ($field_list->getFields() as $field) {
+        $field_key = $field->getFieldKeyForConduit();
+        $value = $field->getConduitDictionaryValue();
+        $results[$revision->getPHID()][$field_key] = $value;
+      }
+    }
+
+    return $results;
+  }
+
 }

src/applications/differential/conduit/ConduitAPI_differential_getrevision_Method.php

@@ -1,10 +1,7 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_differential_getrevision_Method
-  extends ConduitAPIMethod {
+  extends ConduitAPI_differential_Method {
 
   public function getMethodStatus() {
     return self::METHOD_STATUS_DEPRECATED;
@@ -73,9 +70,9 @@ final class ConduitAPI_differential_getrevision_Method
       );
     }
 
-    $auxiliary_fields = $this->loadAuxiliaryFields(
+    $field_data = $this->loadCustomFieldsForRevisions(
-      $revision,
+      $request->getUser(),
-      $request->getUser());
+      array($revision));
 
     $dict = array(
       'id' => $revision->getID(),
@@ -93,29 +90,10 @@ final class ConduitAPI_differential_getrevision_Method
       'reviewerPHIDs' => $reviewer_phids,
       'diffs' => $diff_dicts,
       'commits' => $commit_dicts,
-      'auxiliary' => $auxiliary_fields,
+      'auxiliary' => idx($field_data, $revision->getPHID(), array())
     );
 
     return $dict;
   }
 
-  private function loadAuxiliaryFields(
-    DifferentialRevision $revision,
-    PhabricatorUser $user) {
-    $aux_fields = DifferentialFieldSelector::newSelector()
-      ->getFieldSpecifications();
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_field->setUser($user);
-      if (!$aux_field->shouldAppearOnConduitView()) {
-        unset($aux_fields[$key]);
-      }
-    }
-
-    $aux_fields = DifferentialAuxiliaryField::loadFromStorage(
-      $revision,
-      $aux_fields);
-
-    return mpull($aux_fields, 'getValueForConduit', 'getKeyForConduit');
-  }
-
 }

src/applications/differential/conduit/ConduitAPI_differential_query_Method.php

@@ -1,10 +1,7 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_differential_query_Method
-  extends ConduitAPIMethod {
+  extends ConduitAPI_differential_Method {
 
   public function getMethodDescription() {
     return "Query Differential revisions which match certain criteria.";
@@ -191,6 +188,10 @@ final class ConduitAPI_differential_query_Method
 
     $revisions = $query->execute();
 
+    $field_data = $this->loadCustomFieldsForRevisions(
+      $request->getUser(),
+      $revisions);
+
     $results = array();
     foreach ($revisions as $revision) {
       $diff = $revision->getActiveDiff();
@@ -199,11 +200,11 @@ final class ConduitAPI_differential_query_Method
       }
 
       $id = $revision->getID();
-      $auxiliary_fields = $this->loadAuxiliaryFields(
+      $phid = $revision->getPHID();
-                                 $revision, $request->getUser());
+
       $result = array(
         'id'                  => $id,
-        'phid'                => $revision->getPHID(),
+        'phid'                => $phid,
         'title'               => $revision->getTitle(),
         'uri'                 => PhabricatorEnv::getProductionURI('/D'.$id),
         'dateCreated'         => $revision->getDateCreated(),
@@ -222,7 +223,7 @@ final class ConduitAPI_differential_query_Method
         'reviewers'           => array_values($revision->getReviewers()),
         'ccs'                 => array_values($revision->getCCPHIDs()),
         'hashes'              => $revision->getHashes(),
-        'auxiliary'           => $auxiliary_fields,
+        'auxiliary'           => idx($field_data, $phid, array()),
         'arcanistProjectPHID' => $diff->getArcanistProjectPHID()
       );
 
@@ -238,23 +239,4 @@ final class ConduitAPI_differential_query_Method
     return $results;
   }
 
-  private function loadAuxiliaryFields(
-    DifferentialRevision $revision,
-    PhabricatorUser $user) {
-    $aux_fields = DifferentialFieldSelector::newSelector()
-      ->getFieldSpecifications();
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_field->setUser($user);
-      if (!$aux_field->shouldAppearOnConduitView()) {
-        unset($aux_fields[$key]);
-      }
-    }
-
-    $aux_fields = DifferentialAuxiliaryField::loadFromStorage(
-      $revision,
-      $aux_fields);
-
-    return mpull($aux_fields, 'getValueForConduit', 'getKeyForConduit');
-  }
-
 }

src/applications/differential/customfield/DifferentialBlameRevisionField.php

@@ -111,4 +111,8 @@ final class DifferentialBlameRevisionField
     return $this->getValue();
   }
 
+  public function shouldAppearInConduitDictionary() {
+    return true;
+  }
+
 }

src/applications/differential/customfield/DifferentialCoreCustomField.php

@@ -126,4 +126,8 @@ abstract class DifferentialCoreCustomField
     return $this->getValue();
   }
 
+  public function getConduitDictionaryValue() {
+    return $this->getValue();
+  }
+
 }

src/applications/differential/customfield/DifferentialDependsOnField.php

@@ -7,6 +7,10 @@ final class DifferentialDependsOnField
     return 'differential:depends-on';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'phabricator:depends-on';
+  }
+
   public function getFieldName() {
     return pht('Depends On');
   }
@@ -45,4 +49,14 @@ final class DifferentialDependsOnField
     );
   }
 
+  public function shouldAppearInConduitDictionary() {
+    return true;
+  }
+
+  public function getConduitDictionaryValue() {
+    return PhabricatorEdgeQuery::loadDestinationPHIDs(
+      $this->getObject()->getPHID(),
+      PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV);
+  }
+
 }

src/applications/differential/customfield/DifferentialJIRAIssuesField.php

@@ -268,4 +268,8 @@ final class DifferentialJIRAIssuesField
     return implode(', ', $value);
   }
 
+  public function shouldAppearInConduitDictionary() {
+    return true;
+  }
+
 }

src/applications/differential/customfield/DifferentialRevertPlanField.php

@@ -143,4 +143,8 @@ final class DifferentialRevertPlanField
     return $this->getValue();
   }
 
+  public function shouldAppearInConduitDictionary() {
+    return true;
+  }
+
 }

src/applications/differential/customfield/DifferentialStoredCustomField.php

@@ -44,4 +44,8 @@ abstract class DifferentialStoredCustomField
     return $this;
   }
 
+  public function getConduitDictionaryValue() {
+    return $this->getValue();
+  }
+
 }

src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php

@@ -53,14 +53,6 @@ final class DifferentialBlameRevisionFieldSpecification
     return $engine->markupText($this->value);
   }
 
-  public function shouldAppearOnConduitView() {
-    return true;
-  }
-
-  public function getValueForConduit() {
-    return $this->value;
-  }
-
   public function shouldAppearOnCommitMessage() {
     return true;
   }

src/applications/differential/field/specification/DifferentialDependsOnFieldSpecification.php

@@ -35,16 +35,4 @@ final class DifferentialDependsOnFieldSpecification
       PhabricatorEdgeConfig::TYPE_DREV_DEPENDS_ON_DREV);
   }
 
-  public function shouldAppearOnConduitView() {
-    return true;
-  }
-
-  public function getValueForConduit() {
-    return $this->getDependentRevisionPHIDs();
-  }
-
-  public function getKeyForConduit() {
-    return 'phabricator:depends-on';
-  }
-
 }

src/applications/differential/field/specification/DifferentialFieldSpecification.php

@@ -13,7 +13,6 @@
  * @task view Extending the Revision View Interface
  * @task list Extending the Revision List Interface
  * @task mail Extending the E-mail Interface
- * @task conduit Extending the Conduit View Interface
  * @task commit Extending Commit Messages
  * @task load Loading Additional Data
  * @task context Contextual Data
@@ -384,35 +383,6 @@ abstract class DifferentialFieldSpecification {
     throw new DifferentialFieldSpecificationIncompleteException($this);
   }
 
-
-/* -(  Extending the Conduit Interface  )------------------------------------ */
-
-
-  /**
-   * @task conduit
-   */
-  public function shouldAppearOnConduitView() {
-    return false;
-  }
-
-  /**
-   * @task conduit
-   */
-  public function getValueForConduit() {
-    throw new DifferentialFieldSpecificationIncompleteException($this);
-  }
-
-  /**
-   * @task conduit
-   */
-  public function getKeyForConduit() {
-    $key = $this->getStorageKey();
-    if ($key === null) {
-      throw new DifferentialFieldSpecificationIncompleteException($this);
-    }
-    return $key;
-  }
-
 /* -(  Extending the Search Interface  )------------------------------------ */
 
   /**

src/applications/differential/field/specification/DifferentialJIRAIssuesFieldSpecification.php

@@ -65,14 +65,6 @@ final class DifferentialJIRAIssuesFieldSpecification
     return phutil_implode_html(phutil_tag('br'), $links);
   }
 
-  public function shouldAppearOnConduitView() {
-    return true;
-  }
-
-  public function getValueForConduit() {
-    return $this->value;
-  }
-
   public function shouldAppearOnCommitMessage() {
     return true;
   }

src/applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php

@@ -50,14 +50,6 @@ final class DifferentialRevertPlanFieldSpecification
     return $this->value;
   }
 
-  public function shouldAppearOnConduitView() {
-    return true;
-  }
-
-  public function getValueForConduit() {
-    return $this->value;
-  }
-
   public function shouldAppearOnCommitMessage() {
     return true;
   }

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -27,6 +27,7 @@ abstract class PhabricatorCustomField {
   const ROLE_VIEW                     = 'view';
   const ROLE_LIST                     = 'list';
   const ROLE_GLOBALSEARCH             = 'GlobalSearch';
+  const ROLE_CONDUIT                  = 'conduit';
 
 
 /* -(  Building Applications with Custom Fields  )--------------------------- */
@@ -257,6 +258,8 @@ abstract class PhabricatorCustomField {
         return $this->shouldAppearInListView();
       case self::ROLE_GLOBALSEARCH:
         return $this->shouldAppearInGlobalSearch();
+      case self::ROLE_CONDUIT:
+        return $this->shouldAppearInConduitDictionary();
       case self::ROLE_DEFAULT:
         return true;
       default:
@@ -1167,4 +1170,29 @@ abstract class PhabricatorCustomField {
   }
 
 
+/* -(  Conduit  )------------------------------------------------------------ */
+
+
+  /**
+   * @task conduit
+   */
+  public function shouldAppearInConduitDictionary() {
+    if ($this->proxy) {
+      return $this->proxy->shouldAppearInConduitDictionary();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task conduit
+   */
+  public function getConduitDictionaryValue() {
+    if ($this->proxy) {
+      return $this->proxy->getConduitDictionaryValue();
+    }
+    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
+  }
+
+
 }