Remove DiffusionBranchInformation in favor of DiffusionRepositoryRef

Summary: Ref T4327. At some point these two very similar classes got introduced. Collapse `DiffusionBranchInformation` into the nearly identical `DiffusionRepositoryRef`, which enjoys slightly more generality and support.

Test Plan: Viewed branch overview and detail pages. Ran `repository refs` and `repository discover`. Grepped for removed symbols.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4327

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

src/__phutil_library_map__.php

@@ -467,7 +467,6 @@ phutil_register_library_map(array(
     'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
     'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
     'DifferentialViewPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialViewPolicyFieldSpecification.php',
-    'DiffusionBranchInformation' => 'applications/diffusion/data/DiffusionBranchInformation.php',
     'DiffusionBranchTableController' => 'applications/diffusion/controller/DiffusionBranchTableController.php',
     'DiffusionBranchTableView' => 'applications/diffusion/view/DiffusionBranchTableView.php',
     'DiffusionBrowseController' => 'applications/diffusion/controller/DiffusionBrowseController.php',
@@ -3014,6 +3013,7 @@ phutil_register_library_map(array(
       1 => 'PhabricatorApplicationSearchResultsControllerInterface',
     ),
     'DiffusionRepositoryNewController' => 'DiffusionController',
+    'DiffusionRepositoryRef' => 'Phobject',
     'DiffusionRepositoryRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'DiffusionResolveUserQuery' => 'Phobject',
     'DiffusionSSHGitReceivePackWorkflow' => 'DiffusionSSHGitWorkflow',

src/applications/diffusion/conduit/ConduitAPI_diffusion_branchquery_Method.php

@@ -21,62 +21,53 @@ final class ConduitAPI_diffusion_branchquery_Method
     );
   }
 
+
   protected function getGitResult(ConduitAPIRequest $request) {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
-    $limit = $request->getValue('limit');
-    $offset = $request->getValue('offset');
 
     $refs = id(new DiffusionLowLevelGitRefQuery())
       ->setRepository($repository)
       ->withIsOriginBranch(true)
       ->execute();
 
-    $branches = array();
+    return $this->processBranchRefs($request, $refs);
-    foreach ($refs as $ref) {
+  }
-      $branch = id(new DiffusionBranchInformation())
-        ->setName($ref->getShortName())
-        ->setHeadCommitIdentifier($ref->getCommitIdentifier());
 
-      if (!$repository->shouldTrackBranch($branch->getName())) {
+  protected function getMercurialResult(ConduitAPIRequest $request) {
-        continue;
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    $refs = id(new DiffusionLowLevelMercurialBranchesQuery())
+      ->setRepository($repository)
+      ->execute();
+
+    return $this->processBranchRefs($request, $refs);
+  }
+
+  private function processBranchRefs(ConduitAPIRequest $request, array $refs) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $offset = $request->getValue('offset');
+    $limit = $request->getValue('limit');
+
+    foreach ($refs as $key => $ref) {
+      if (!$repository->shouldTrackBranch($ref->getShortName())) {
+        unset($refs[$key]);
       }
-
-      $branches[] = $branch->toDictionary();
     }
 
     // NOTE: We can't apply the offset or limit until here, because we may have
     // filtered untrackable branches out of the result set.
 
     if ($offset) {
-      $branches = array_slice($branches, $offset);
+      $refs = array_slice($refs, $offset);
     }
 
     if ($limit) {
-      $branches = array_slice($branches, 0, $limit);
+      $refs = array_slice($refs, 0, $limit);
     }
 
-    return $branches;
+    return mpull($refs, 'toDictionary');
-  }
-
-  protected function getMercurialResult(ConduitAPIRequest $request) {
-    $drequest = $this->getDiffusionRequest();
-    $repository = $drequest->getRepository();
-    $offset = $request->getValue('offset');
-    $limit = $request->getValue('limit');
-
-    $branches = id(new DiffusionLowLevelMercurialBranchesQuery())
-      ->setRepository($repository)
-      ->execute();
-
-    if ($offset) {
-      $branches = array_slice($branches, $offset);
-    }
-
-    if ($limit) {
-      $branches = array_slice($branches, 0, $limit);
-    }
-
-    return mpull($branches, 'toDictionary');
   }
 }

src/applications/diffusion/conduit/ConduitAPI_diffusion_commitbranchesquery_Method.php

@@ -39,7 +39,7 @@ final class ConduitAPI_diffusion_commitbranchesquery_Method
         $commit);
       return DiffusionGitBranch::parseRemoteBranchOutput(
         $contains,
-        DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
+        DiffusionGitBranch::DEFAULT_GIT_REMOTE);
     }
   }
 
@@ -48,7 +48,10 @@ final class ConduitAPI_diffusion_commitbranchesquery_Method
     $repository = $drequest->getRepository();
     $commit = $request->getValue('commit');
 
-    // TODO: This should use `branches`.
+    // TODO: This should use `branches`. Also, this entire method's API needs
+    // to be fixed to support multiple branch heads in Mercurial. We should
+    // probably return `DiffusionRepositoryRefs` and probably merge this into
+    // `diffusion.branchesquery`.
 
     list($contains) = $repository->execxLocalCommand(
       'log --template %s --limit 1 --rev %s --',

src/applications/diffusion/controller/DiffusionBranchTableController.php

@@ -18,15 +18,16 @@ final class DiffusionBranchTableController extends DiffusionController {
     $pager->setOffset($request->getInt('offset'));
 
     // TODO: Add support for branches that contain commit
-    $branches = DiffusionBranchInformation::newFromConduit(
+    $branches = $this->callConduitWithDiffusionRequest(
-      $this->callConduitWithDiffusionRequest(
+      'diffusion.branchquery',
-        'diffusion.branchquery',
+      array(
-        array(
+        'offset' => $pager->getOffset(),
-          'offset' => $pager->getOffset(),
+        'limit' => $pager->getPageSize() + 1
-          'limit' => $pager->getPageSize() + 1
+      ));
-        )));
     $branches = $pager->sliceResults($branches);
 
+    $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches);
+
     $content = null;
     if (!$branches) {
       $content = $this->renderStatusMessage(
@@ -35,7 +36,7 @@ final class DiffusionBranchTableController extends DiffusionController {
     } else {
       $commits = id(new DiffusionCommitQuery())
         ->setViewer($viewer)
-        ->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier'))
+        ->withIdentifiers(mpull($branches, 'getCommitIdentifier'))
         ->withRepository($repository)
         ->execute();
 

src/applications/diffusion/controller/DiffusionRepositoryController.php

@@ -261,12 +261,11 @@ final class DiffusionRepositoryController extends DiffusionController {
 
     $limit = 15;
 
-    $branches = DiffusionBranchInformation::newFromConduit(
+    $branches = $this->callConduitWithDiffusionRequest(
-      $this->callConduitWithDiffusionRequest(
+      'diffusion.branchquery',
-        'diffusion.branchquery',
+      array(
-        array(
+        'limit' => $limit + 1,
-          'limit' => $limit + 1,
+      ));
-        )));
     if (!$branches) {
       return null;
     }
@@ -274,9 +273,11 @@ final class DiffusionRepositoryController extends DiffusionController {
     $more_branches = (count($branches) > $limit);
     $branches = array_slice($branches, 0, $limit);
 
+    $branches = DiffusionRepositoryRef::loadAllFromDictionaries($branches);
+
     $commits = id(new DiffusionCommitQuery())
       ->setViewer($viewer)
-      ->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier'))
+      ->withIdentifiers(mpull($branches, 'getCommitIdentifier'))
       ->withRepository($drequest->getRepository())
       ->execute();
 

src/applications/diffusion/data/DiffusionBranchInformation.php

@@ -1,56 +0,0 @@
-<?php
-
-final class DiffusionBranchInformation {
-
-  const DEFAULT_GIT_REMOTE = 'origin';
-
-  private $name;
-  private $headCommitIdentifier;
-
-  public function setName($name) {
-    $this->name = $name;
-    return $this;
-  }
-
-  public function getName() {
-    return $this->name;
-  }
-
-  public function setHeadCommitIdentifier($head_commit_identifier) {
-    $this->headCommitIdentifier = $head_commit_identifier;
-    return $this;
-  }
-
-  public function getHeadCommitIdentifier() {
-    return $this->headCommitIdentifier;
-  }
-
-  public static function newFromConduit(array $dicts) {
-    $branches = array();
-    foreach ($dicts as $dict) {
-      $branches[] = id(new DiffusionBranchInformation())
-        ->setName($dict['name'])
-        ->setHeadCommitIdentifier($dict['headCommitIdentifier']);
-    }
-    return $branches;
-  }
-
-  public function toDictionary() {
-    return array(
-      'name' => $this->getName(),
-      'headCommitIdentifier' => $this->getHeadCommitIdentifier()
-    );
-  }
-
-  // TODO: These are hacks to make this compatible with DiffusionRepositoryRef
-  // for PhabricatorRepositoryRefEngine. The two classes should be merged.
-
-  public function getShortName() {
-    return $this->getName();
-  }
-
-  public function getCommitIdentifier() {
-    return $this->getHeadCommitIdentifier();
-  }
-
-}

src/applications/diffusion/data/DiffusionGitBranch.php

@@ -2,6 +2,8 @@
 
 final class DiffusionGitBranch {
 
+  const DEFAULT_GIT_REMOTE = 'origin';
+
   /**
    * Parse the output of 'git branch -r --verbose --no-abbrev' or similar into
    * a map. For instance:

src/applications/diffusion/data/DiffusionRepositoryRef.php

@@ -1,10 +1,13 @@
 <?php
 
-final class DiffusionRepositoryRef {
+/**
+ * @task serialization Serializing Repository Refs
+ */
+final class DiffusionRepositoryRef extends Phobject {
 
   private $shortName;
   private $commitIdentifier;
-  private $rawFields;
+  private $rawFields = array();
 
   public function setRawFields(array $raw_fields) {
     $this->rawFields = $raw_fields;
@@ -33,4 +36,31 @@ final class DiffusionRepositoryRef {
     return $this->shortName;
   }
 
+
+/* -(  Serialization  )------------------------------------------------------ */
+
+
+  public function toDictionary() {
+    return array(
+      'shortName' => $this->shortName,
+      'commitIdentifier' => $this->commitIdentifier,
+      'rawFields' => $this->rawFields,
+    );
+  }
+
+  public static function newFromDictionary(array $dict) {
+    return id(new DiffusionRepositoryRef())
+      ->setShortName($dict['shortName'])
+      ->setCommitIdentifier($dict['commitIdentifier'])
+      ->setRawFields($dict['rawFields']);
+  }
+
+  public static function loadAllFromDictionaries(array $dictionaries) {
+    $refs = array();
+    foreach ($dictionaries as $dictionary) {
+      $refs[] = self::newFromDictionary($dictionary);
+    }
+    return $refs;
+  }
+
 }

src/applications/diffusion/query/lowlevel/DiffusionLowLevelGitRefQuery.php

@@ -32,7 +32,7 @@ final class DiffusionLowLevelGitRefQuery extends DiffusionLowLevelQuery {
       if ($repository->isWorkingCopyBare()) {
         $prefix = 'refs/heads/';
       } else {
-        $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE;
+        $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE;
         $prefix = 'refs/remotes/'.$remote.'/';
       }
     } else {

src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php

@@ -18,9 +18,9 @@ final class DiffusionLowLevelMercurialBranchesQuery
 
     $lines = ArcanistMercurialParser::parseMercurialBranches($stdout);
     foreach ($lines as $name => $spec) {
-      $branches[] = id(new DiffusionBranchInformation())
+      $branches[] = id(new DiffusionRepositoryRef())
-        ->setName($name)
+        ->setShortName($name)
-        ->setHeadCommitIdentifier($spec['rev']);
+        ->setCommitIdentifier($spec['rev']);
     }
 
     return $branches;

src/applications/diffusion/request/DiffusionGitRequest.php

@@ -39,7 +39,7 @@ final class DiffusionGitRequest extends DiffusionRequest {
     if ($this->repository->isWorkingCopyBare()) {
       return $branch;
     } else {
-      $remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE;
+      $remote = DiffusionGitBranch::DEFAULT_GIT_REMOTE;
       return $remote.'/'.$branch;
     }
   }

src/applications/diffusion/view/DiffusionBranchTableView.php

@@ -6,12 +6,13 @@ final class DiffusionBranchTableView extends DiffusionView {
   private $commits = array();
 
   public function setBranches(array $branches) {
-    assert_instances_of($branches, 'DiffusionBranchInformation');
+    assert_instances_of($branches, 'DiffusionRepositoryRef');
     $this->branches = $branches;
     return $this;
   }
 
   public function setCommits(array $commits) {
+    assert_instances_of($commits, 'PhabricatorRepositoryCommit');
     $this->commits = mpull($commits, null, 'getCommitIdentifier');
     return $this;
   }
@@ -23,7 +24,7 @@ final class DiffusionBranchTableView extends DiffusionView {
     $rows = array();
     $rowc = array();
     foreach ($this->branches as $branch) {
-      $commit = idx($this->commits, $branch->getHeadCommitIdentifier());
+      $commit = idx($this->commits, $branch->getCommitIdentifier());
       if ($commit) {
         $details = $commit->getSummary();
         $datetime = phabricator_datetime($commit->getEpoch(), $this->user);
@@ -39,7 +40,7 @@ final class DiffusionBranchTableView extends DiffusionView {
             'href' => $drequest->generateURI(
               array(
                 'action' => 'history',
-                'branch' => $branch->getName(),
+                'branch' => $branch->getShortName(),
               ))
           ),
           pht('History')),
@@ -49,17 +50,17 @@ final class DiffusionBranchTableView extends DiffusionView {
             'href' => $drequest->generateURI(
               array(
                 'action' => 'browse',
-                'branch' => $branch->getName(),
+                'branch' => $branch->getShortName(),
               )),
           ),
-          $branch->getName()),
+          $branch->getShortName()),
         self::linkCommit(
           $drequest->getRepository(),
-          $branch->getHeadCommitIdentifier()),
+          $branch->getCommitIdentifier()),
         $datetime,
         AphrontTableView::renderSingleDisplayLine($details),
       );
-      if ($branch->getName() == $current_branch) {
+      if ($branch->getShortName() == $current_branch) {
         $rowc[] = 'highlighted';
       } else {
         $rowc[] = null;