Use application PHIDs in Releeph, plus more

Summary:
Ref T2715. Ref T3551. Ref T603. This does a few things, but they're all sort of small:

  - We commonly use a `getX()` / `attachX()` pattern, but have very similar code in the `getX()` method every time. Provide a convenience method to make this pattern easier to write.
  - We use `willFilterPage()` in many queries, but it currently is called with zero or more results. This means we have a lot of "if no results, return nothing" boilerplate. Make it call only for one or more results.
  - Implement `PhabricatorPolicyInterface` on `ReleephBranch`. A branch has the same policy as its project.
  - Implement `ReleephBranchQuery`.
  - Move the branch PHID type to application PHID infrastructure.

Test Plan: Browsed Releeph. Used `phid.query` to query branch PHIDs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603, T2715, T3551

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

src/__phutil_library_map__.php

@@ -1032,6 +1032,7 @@ phutil_register_library_map(array(
     'PhabricatorDaemonManagementStopWorkflow' => 'applications/daemon/management/PhabricatorDaemonManagementStopWorkflow.php',
     'PhabricatorDaemonManagementWorkflow' => 'applications/daemon/management/PhabricatorDaemonManagementWorkflow.php',
     'PhabricatorDaemonReference' => 'infrastructure/daemon/control/PhabricatorDaemonReference.php',
+    'PhabricatorDataNotAttachedException' => 'infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php',
     'PhabricatorDebugController' => 'applications/system/PhabricatorDebugController.php',
     'PhabricatorDefaultFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorDefaultFileStorageEngineSelector.php',
     'PhabricatorDefaultSearchEngineSelector' => 'applications/search/selector/PhabricatorDefaultSearchEngineSelector.php',
@@ -1876,6 +1877,7 @@ phutil_register_library_map(array(
     'ReleephBranchEditor' => 'applications/releeph/editor/ReleephBranchEditor.php',
     'ReleephBranchNamePreviewController' => 'applications/releeph/controller/branch/ReleephBranchNamePreviewController.php',
     'ReleephBranchPreviewView' => 'applications/releeph/view/branch/ReleephBranchPreviewView.php',
+    'ReleephBranchQuery' => 'applications/releeph/query/ReleephBranchQuery.php',
     'ReleephBranchTemplate' => 'applications/releeph/view/branch/ReleephBranchTemplate.php',
     'ReleephBranchViewController' => 'applications/releeph/controller/branch/ReleephBranchViewController.php',
     'ReleephCommitFinder' => 'applications/releeph/commitfinder/ReleephCommitFinder.php',
@@ -1896,9 +1898,9 @@ phutil_register_library_map(array(
     'ReleephFieldSpecificationIncompleteException' => 'applications/releeph/field/exception/ReleephFieldSpecificationIncompleteException.php',
     'ReleephIntentFieldSpecification' => 'applications/releeph/field/specification/ReleephIntentFieldSpecification.php',
     'ReleephLevelFieldSpecification' => 'applications/releeph/field/specification/ReleephLevelFieldSpecification.php',
-    'ReleephObjectHandleLoader' => 'applications/releeph/ReleephObjectHandleLoader.php',
     'ReleephOriginalCommitFieldSpecification' => 'applications/releeph/field/specification/ReleephOriginalCommitFieldSpecification.php',
     'ReleephPHIDConstants' => 'applications/releeph/ReleephPHIDConstants.php',
+    'ReleephPHIDTypeBranch' => 'applications/releeph/phid/ReleephPHIDTypeBranch.php',
     'ReleephPHIDTypeProject' => 'applications/releeph/phid/ReleephPHIDTypeProject.php',
     'ReleephPHIDTypeRequest' => 'applications/releeph/phid/ReleephPHIDTypeRequest.php',
     'ReleephProject' => 'applications/releeph/storage/ReleephProject.php',
@@ -3007,6 +3009,7 @@ phutil_register_library_map(array(
     'PhabricatorDaemonManagementStatusWorkflow' => 'PhabricatorDaemonManagementWorkflow',
     'PhabricatorDaemonManagementStopWorkflow' => 'PhabricatorDaemonManagementWorkflow',
     'PhabricatorDaemonManagementWorkflow' => 'PhutilArgumentWorkflow',
+    'PhabricatorDataNotAttachedException' => 'Exception',
     'PhabricatorDebugController' => 'PhabricatorController',
     'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector',
     'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector',
@@ -3910,7 +3913,11 @@ phutil_register_library_map(array(
     'ProjectRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'QueryFormattingTestCase' => 'PhabricatorTestCase',
     'ReleephAuthorFieldSpecification' => 'ReleephFieldSpecification',
-    'ReleephBranch' => 'ReleephDAO',
+    'ReleephBranch' =>
+    array(
+      0 => 'ReleephDAO',
+      1 => 'PhabricatorPolicyInterface',
+    ),
     'ReleephBranchAccessController' => 'ReleephProjectController',
     'ReleephBranchBoxView' => 'AphrontView',
     'ReleephBranchCommitFieldSpecification' => 'ReleephFieldSpecification',
@@ -3919,6 +3926,7 @@ phutil_register_library_map(array(
     'ReleephBranchEditor' => 'PhabricatorEditor',
     'ReleephBranchNamePreviewController' => 'ReleephController',
     'ReleephBranchPreviewView' => 'AphrontFormControl',
+    'ReleephBranchQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'ReleephBranchViewController' => 'ReleephProjectController',
     'ReleephCommitFinderException' => 'Exception',
     'ReleephCommitMessageFieldSpecification' => 'ReleephFieldSpecification',
@@ -3936,6 +3944,7 @@ phutil_register_library_map(array(
     'ReleephIntentFieldSpecification' => 'ReleephFieldSpecification',
     'ReleephLevelFieldSpecification' => 'ReleephFieldSpecification',
     'ReleephOriginalCommitFieldSpecification' => 'ReleephFieldSpecification',
+    'ReleephPHIDTypeBranch' => 'PhabricatorPHIDType',
     'ReleephPHIDTypeProject' => 'PhabricatorPHIDType',
     'ReleephPHIDTypeRequest' => 'PhabricatorPHIDType',
     'ReleephProject' =>

src/applications/releeph/ReleephObjectHandleLoader.php

@@ -1,45 +0,0 @@
-<?php
-
-final class ReleephObjectHandleLoader {
-
-  public function loadHandles(array $phids) {
-    $types = array();
-
-    foreach ($phids as $phid) {
-      $type = phid_get_type($phid);
-      $types[$type][] = $phid;
-    }
-
-    $handles = array();
-
-    foreach ($types as $type => $phids) {
-      switch ($type) {
-
-        case ReleephPHIDConstants::PHID_TYPE_REBR:
-          $object = new ReleephBranch();
-
-          $branches = $object->loadAllWhere('phid IN (%Ls)', $phids);
-          $branches = mpull($branches, null, 'getPHID');
-
-          foreach ($phids as $phid) {
-            $branch = $branches[$phid];
-            $handle = new PhabricatorObjectHandle();
-            $handle->setPHID($phid);
-            $handle->setType($type);
-            $handle->setURI($branch->getURI());
-            $handle->setName($branch->getBasename());
-            $handle->setFullName($branch->getName());
-            $handle->setComplete(true);
-            $handles[$phid] = $handle;
-          }
-          break;
-
-        default:
-          throw new Exception('unknown type '.$type);
-      }
-    }
-
-    return $handles;
-  }
-
-}

src/applications/releeph/phid/ReleephPHIDTypeBranch.php

@@ -0,0 +1,47 @@
+<?php
+
+final class ReleephPHIDTypeBranch extends PhabricatorPHIDType {
+
+  const TYPECONST = 'REBR';
+
+  public function getTypeConstant() {
+    return self::TYPECONST;
+  }
+
+  public function getTypeName() {
+    return pht('Releeph Branch');
+  }
+
+  public function newObject() {
+    return new ReleephBranch();
+  }
+
+  public function loadObjects(
+    PhabricatorObjectQuery $query,
+    array $phids) {
+
+    return id(new ReleephBranchQuery())
+      ->setViewer($query->getViewer())
+      ->withPHIDs($phids)
+      ->execute();
+  }
+
+  public function loadHandles(
+    PhabricatorHandleQuery $query,
+    array $handles,
+    array $objects) {
+
+    foreach ($handles as $phid => $handle) {
+      $branch = $objects[$phid];
+
+      $handle->setURI($branch->getURI());
+      $handle->setName($branch->getBasename());
+      $handle->setFullName($branch->getName());
+    }
+  }
+
+  public function canLoadNamedObject($name) {
+    return false;
+  }
+
+}

src/applications/releeph/query/ReleephBranchQuery.php

@@ -0,0 +1,76 @@
+<?php
+
+final class ReleephBranchQuery
+  extends PhabricatorCursorPagedPolicyAwareQuery {
+
+  private $ids;
+  private $phids;
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
+  public function withPHIDs(array $phids) {
+    $this->phids = $phids;
+    return $this;
+  }
+
+  public function loadPage() {
+    $table = new ReleephBranch();
+    $conn_r = $table->establishConnection('r');
+
+    $data = queryfx_all(
+      $conn_r,
+      'SELECT * FROM %T %Q %Q %Q',
+      $table->getTableName(),
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
+
+    return $table->loadAllFromArray($data);
+  }
+
+  public function willFilterPage(array $branches) {
+    $project_ids = mpull($branches, 'getReleephProjectID');
+
+    $projects = id(new ReleephProjectQuery())
+      ->withIDs($project_ids)
+      ->setViewer($this->getViewer())
+      ->execute();
+
+    foreach ($branches as $key => $branch) {
+      $project_id = $project_ids[$key];
+      if (isset($projects[$project_id])) {
+        $branch->attachProject($projects[$project_id]);
+      } else {
+        unset($branches[$key]);
+      }
+    }
+
+    return $branches;
+  }
+
+  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+    $where = array();
+
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'phid IN (%Ls)',
+        $this->phids);
+    }
+
+    $where[] = $this->buildPagingClause($conn_r);
+
+    return $this->formatWhereClause($where);
+  }
+
+}

src/applications/releeph/query/ReleephProjectQuery.php

@@ -4,6 +4,7 @@ final class ReleephProjectQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
   private $active;
+  private $ids;
   private $phids;
 
   private $order    = 'order-id';
@@ -20,6 +21,11 @@ final class ReleephProjectQuery
     return $this;
   }
 
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
+    return $this;
+  }
+
   public function withPHIDs(array $phids) {
     $this->phids = $phids;
     return $this;
@@ -50,6 +56,13 @@ final class ReleephProjectQuery
         $this->active);
     }
 
+    if ($this->ids) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id IN (%Ls)',
+        $this->ids);
+    }
+
     if ($this->phids) {
       $where[] = qsprintf(
         $conn_r,

src/applications/releeph/storage/ReleephBranch.php

@@ -1,6 +1,7 @@
 <?php
 
-final class ReleephBranch extends ReleephDAO {
+final class ReleephBranch extends ReleephDAO
+  implements PhabricatorPolicyInterface {
 
   protected $phid;
   protected $releephProjectID;
@@ -21,6 +22,8 @@ final class ReleephBranch extends ReleephDAO {
 
   protected $details = array();
 
+  private $project = self::ATTACHABLE;
+
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
@@ -31,8 +34,7 @@ final class ReleephBranch extends ReleephDAO {
   }
 
   public function generatePHID() {
-    return PhabricatorPHID::generateNewPHID(
-      ReleephPHIDConstants::PHID_TYPE_REBR);
+    return PhabricatorPHID::generateNewPHID(ReleephPHIDTypeBranch::TYPECONST);
   }
 
   public function getDetail($key, $default = null) {
@@ -151,4 +153,29 @@ final class ReleephBranch extends ReleephDAO {
     return $this->getIsActive();
   }
 
+  public function attachProject(ReleephProject $project) {
+    $this->project = $project;
+    return $this;
+  }
+
+  public function getProject() {
+    return $this->assertAttached($this->project);
+  }
+
+
+/* -(  PhabricatorPolicyInterface  )----------------------------------------- */
+
+
+  public function getCapabilities() {
+    return $this->getProject()->getCapabilities();
+  }
+
+  public function getPolicy($capability) {
+    return $this->getProject()->getPolicy($capability);
+  }
+
+  public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    return $this->getProject()->hasAutomaticCapability($capability, $viewer);
+  }
+
 }

src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php

@@ -176,7 +176,12 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
         $page = array();
       }
 
+      if ($page) {
         $visible = $this->willFilterPage($page);
+      } else {
+        $visible = array();
+      }
+
       $visible = $filter->apply($visible);
       foreach ($visible as $key => $result) {
         ++$count;
@@ -272,6 +277,9 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
    * you to drop some items from the result set without creating problems with
    * pagination or cursor updates.
    *
+   * This method will only be called if data is available. Implementations
+   * do not need to handle the case of no results specially.
+   *
    * @param   list<wild>  Results from `loadPage()`.
    * @return  list<PhabricatorPolicyInterface> Objects for policy filtering.
    * @task policyimpl

src/infrastructure/storage/lisk/PhabricatorDataNotAttachedException.php

@@ -0,0 +1,35 @@
+<?php
+
+final class PhabricatorDataNotAttachedException extends Exception {
+
+  public function __construct(PhabricatorLiskDAO $dao) {
+    $stack = debug_backtrace();
+
+    // Shift off `PhabricatorDataNotAttachedException::__construct()`.
+    array_shift($stack);
+    // Shift off `PhabricatorLiskDAO::assertAttached()`.
+    array_shift($stack);
+
+    $frame = head($stack);
+    $via = null;
+    if (is_array($frame)) {
+      $method = idx($frame, 'function');
+      if (preg_match('/^get[A-Z]/', $method)) {
+        $via = " (via {$method}())";
+      }
+    }
+
+    $class = get_class($dao);
+
+    $message =
+      "Attempting to access attached data on {$class}{$via}, but the data is ".
+      "not actually attached. Before accessing attachable data on an object, ".
+      "you must load and attach it.\n\n".
+      "Data is normally attached by calling the corresponding needX() ".
+      "method on the Query class when the object is loaded. You can also ".
+      "call the corresponding attachX() method explicitly.";
+
+    parent::__construct($message);
+  }
+
+}

src/infrastructure/storage/lisk/PhabricatorLiskDAO.php

@@ -9,6 +9,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
   private $edges = array();
   private static $namespaceStack = array();
 
+  const ATTACHABLE = "<attachable>";
 
 /* -(  Managing Edges  )----------------------------------------------------- */
 
@@ -207,4 +208,12 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
     return $result;
   }
 
+
+  protected function assertAttached($property) {
+    if ($property === self::ATTACHABLE) {
+      throw new PhabricatorDataNotAttachedException($this);
+    }
+    return $property;
+  }
+
 }