Separate "browse ref -> commit" and "commit -> upstream" hardpoints from arc browse workflow

Summary:
Ref T10895. Currently, the "browse as a commit" code does these lookups, but the code can't be reused.

For T10895, I want to introduce "browse as revision", but it will need to do the same ref lookup. Separate this as a hardpoint so the code can be shared via hardpoint/ref infrastructure.

Test Plan: Ran `arc browse master` and similar commands.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10895

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

src/__phutil_library_map__.php

@@ -42,6 +42,7 @@ phutil_register_library_map(array(
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php',
     'ArcanistBranchRef' => 'ref/ArcanistBranchRef.php',
     'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php',
+    'ArcanistBrowseCommitHardpointLoader' => 'browse/loader/ArcanistBrowseCommitHardpointLoader.php',
     'ArcanistBrowseCommitURIHardpointLoader' => 'browse/loader/ArcanistBrowseCommitURIHardpointLoader.php',
     'ArcanistBrowseObjectNameURIHardpointLoader' => 'browse/loader/ArcanistBrowseObjectNameURIHardpointLoader.php',
     'ArcanistBrowsePathURIHardpointLoader' => 'browse/loader/ArcanistBrowsePathURIHardpointLoader.php',
@@ -84,6 +85,7 @@ phutil_register_library_map(array(
     'ArcanistCommentStyleXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php',
     'ArcanistCommentStyleXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistCommentStyleXHPASTLinterRuleTestCase.php',
     'ArcanistCommitRef' => 'ref/ArcanistCommitRef.php',
+    'ArcanistCommitUpstreamHardpointLoader' => 'loader/ArcanistCommitUpstreamHardpointLoader.php',
     'ArcanistCommitWorkflow' => 'workflow/ArcanistCommitWorkflow.php',
     'ArcanistCompilerLintRenderer' => 'lint/renderer/ArcanistCompilerLintRenderer.php',
     'ArcanistComposerLinter' => 'lint/linter/ArcanistComposerLinter.php',
@@ -480,6 +482,7 @@ phutil_register_library_map(array(
     'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistBranchRef' => 'ArcanistRef',
     'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow',
+    'ArcanistBrowseCommitHardpointLoader' => 'ArcanistHardpointLoader',
     'ArcanistBrowseCommitURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader',
     'ArcanistBrowseObjectNameURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader',
     'ArcanistBrowsePathURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader',
@@ -522,6 +525,7 @@ phutil_register_library_map(array(
     'ArcanistCommentStyleXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistCommentStyleXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
     'ArcanistCommitRef' => 'ArcanistRef',
+    'ArcanistCommitUpstreamHardpointLoader' => 'ArcanistHardpointLoader',
     'ArcanistCommitWorkflow' => 'ArcanistWorkflow',
     'ArcanistCompilerLintRenderer' => 'ArcanistLintRenderer',
     'ArcanistComposerLinter' => 'ArcanistLinter',

src/browse/loader/ArcanistBrowseCommitHardpointLoader.php

@@ -0,0 +1,81 @@
+<?php
+
+final class ArcanistBrowseCommitHardpointLoader
+  extends ArcanistHardpointLoader {
+
+  const LOADERKEY = 'browse.ref.commit';
+
+  public function canLoadRepositoryAPI(ArcanistRepositoryAPI $api) {
+    return true;
+  }
+
+  public function canLoadRef(ArcanistRef $ref) {
+    return ($ref instanceof ArcanistBrowseRef);
+  }
+
+  public function canLoadHardpoint(ArcanistRef $ref, $hardpoint) {
+    return ($hardpoint == 'commitRefs');
+  }
+
+  public function loadHardpoints(array $refs, $hardpoint) {
+    $query = $this->getQuery();
+
+    $api = $query->getRepositoryAPI();
+    if (!$api) {
+      return array();
+    }
+
+    $repository_ref = $query->getRepositoryRef();
+    if (!$repository_ref) {
+      return array();
+    }
+    $repository_phid = $repository_ref->getPHID();
+
+    $commit_map = array();
+    foreach ($refs as $key => $ref) {
+      $is_commit = $ref->hasType(
+        ArcanistBrowseCommitURIHardpointLoader::BROWSETYPE);
+
+      $token = $ref->getToken();
+
+      if ($token === '.') {
+        // Git resolves "." like HEAD, but we want to treat it as "browse the
+        // current directory" instead in all cases.
+        continue;
+      }
+
+      if ($token === null) {
+        if ($is_commit) {
+          $token = $api->getHeadCommit();
+        } else {
+          continue;
+        }
+      }
+
+      try {
+        $commit = $api->getCanonicalRevisionName($token);
+        if ($commit) {
+          $commit_map[$commit][] = $key;
+        }
+      } catch (Exception $ex) {
+        // Ignore anything we can't resolve.
+      }
+    }
+
+    if (!$commit_map) {
+      return array();
+    }
+
+    $results = array();
+    foreach ($commit_map as $commit_identifier => $ref_keys) {
+      foreach ($ref_keys as $key) {
+        $commit_ref = id(new ArcanistCommitRef())
+          ->setCommitHash($commit_identifier);
+        $results[$key][] = $commit_ref;
+      }
+    }
+
+    return $results;
+  }
+
+}

src/browse/loader/ArcanistBrowseCommitURIHardpointLoader.php

@@ -34,69 +34,55 @@ final class ArcanistBrowseCommitURIHardpointLoader
   }
 
   public function loadHardpoints(array $refs, $hardpoint) {
-    $api = $this->getQuery()->getRepositoryAPI();
+    $query = $this->getQuery();
+
+    $api = $query->getRepositoryAPI();
     if (!$api) {
       return array();
     }
 
-    $repository_ref = $this->getQuery()->getRepositoryRef();
+    $repository_ref = $query->getRepositoryRef();
     if (!$repository_ref) {
       return array();
     }
 
-    $repository_phid = $repository_ref->getPHID();
-
     $refs = $this->getRefsWithSupportedTypes($refs);
 
-    $commit_map = array();
+    if (!$refs) {
-    foreach ($refs as $key => $ref) {
-      $is_commit = $ref->hasType('commit');
-
-      $token = $ref->getToken();
-
-      if ($token === '.') {
-        // Git resolves "." like HEAD, but we want to treat it as "browse the
-        // current directory" instead in all cases.
-        continue;
-      }
-
-      if ($token === null) {
-        if ($is_commit) {
-          $token = $api->getHeadCommit();
-        } else {
-          continue;
-        }
-      }
-
-      try {
-        $commit = $api->getCanonicalRevisionName($token);
-        if ($commit) {
-          $commit_map[$commit][] = $key;
-        }
-      } catch (Exception $ex) {
-        // Ignore anything we can't resolve.
-      }
-    }
-
-    if (!$commit_map) {
       return array();
     }
 
-    $commit_info = $this->resolveCall(
+    $this->newQuery($refs)
-      'diffusion.querycommits',
+      ->needHardpoints(
-      array(
+        array(
-        'repositoryPHID' => $repository_phid,
+          'commitRefs',
-        'names' => array_keys($commit_map),
+        ))
-      ));
+      ->execute();
+
+    $commit_refs = array();
+    foreach ($refs as $key => $ref) {
+      foreach ($ref->getCommitRefs() as $commit_ref) {
+        $commit_refs[] = $commit_ref;
+      }
+    }
+
+    $this->newQuery($commit_refs)
+      ->needHardpoints(
+        array(
+          'upstream',
+        ))
+      ->execute();
 
     $results = array();
-    foreach ($commit_info['identifierMap'] as $commit_key => $commit_phid) {
+    foreach ($refs as $key => $ref) {
-      foreach ($commit_map[$commit_key] as $key) {
+      $commit_refs = $ref->getCommitRefs();
-        $commit_uri = $commit_info['data'][$commit_phid]['uri'];
+      foreach ($commit_refs as $commit_ref) {
-
+        $uri = $commit_ref->getURI();
-        $results[$key][] = id(new ArcanistBrowseURIRef())
+        if ($uri !== null) {
-          ->setURI($commit_uri)
+          $results[$key][] = id(new ArcanistBrowseURIRef())
-          ->setType('commit');
+            ->setURI()
+            ->setType(self::BROWSETYPE);
+        }
       }
     }
 

src/browse/ref/ArcanistBrowseRef.php

@@ -13,6 +13,10 @@ final class ArcanistBrowseRef
 
   public function defineHardpoints() {
     return array(
+      'commitRefs' => array(
+        'type' => 'ArcanistCommitRef',
+        'vector' => true,
+      ),
       'uris' => array(
         'type' => 'ArcanistBrowseURIRef',
         'vector' => true,
@@ -61,4 +65,8 @@ final class ArcanistBrowseRef
     return $this->getHardpoint('uris');
   }
 
+  public function getCommitRefs() {
+    return $this->getHardpoint('commitRefs');
+  }
+
 }

src/loader/ArcanistCommitUpstreamHardpointLoader.php

@@ -0,0 +1,59 @@
+<?php
+
+final class ArcanistCommitUpstreamHardpointLoader
+  extends ArcanistHardpointLoader {
+
+  const LOADERKEY = 'commit.conduit';
+
+  public function canLoadRepositoryAPI(ArcanistRepositoryAPI $api) {
+    return true;
+  }
+
+  public function canLoadRef(ArcanistRef $ref) {
+    return ($ref instanceof ArcanistCommitRef);
+  }
+
+  public function canLoadHardpoint(ArcanistRef $ref, $hardpoint) {
+    return ($hardpoint == 'upstream');
+  }
+
+  public function loadHardpoints(array $refs, $hardpoint) {
+    $query = $this->getQuery();
+
+    $repository_ref = $query->getRepositoryRef();
+    if (!$repository_ref) {
+      return array_fill_keys(array_keys($refs), null);
+    }
+    $repository_phid = $repository_ref->getPHID();
+
+    $commit_map = array();
+    foreach ($refs as $key => $ref) {
+      $hash = $ref->getCommitHash();
+      $commit_map[$hash][] = $key;
+    }
+
+    $commit_info = $this->resolveCall(
+      'diffusion.querycommits',
+      array(
+        'repositoryPHID' => $repository_phid,
+        'names' => array_keys($commit_map),
+      ));
+
+    $results = array();
+    foreach ($commit_map as $hash => $keys) {
+      $commit_phid = idx($commit_info['identifierMap'], $hash);
+      if ($commit_phid) {
+        $commit_data = idx($commit_info['data'], $commit_phid);
+      } else {
+        $commit_data = null;
+      }
+
+      foreach ($keys as $key) {
+        $results[$key] = $commit_data;
+      }
+    }
+
+    return $results;
+  }
+
+}

src/loader/ArcanistHardpointLoader.php

@@ -25,10 +25,28 @@ abstract class ArcanistHardpointLoader
   }
 
   final protected function newQuery(array $refs) {
-    return id(new ArcanistRefQuery())
+    $result = id(new ArcanistRefQuery())
-      ->setRepositoryAPI($this->getQuery()->getRepositoryAPI())
       ->setConduitEngine($this->getQuery()->getConduitEngine())
       ->setRefs($refs);
+
+    $query = $this->getQuery();
+
+    $repository_api = $query->getRepositoryAPI();
+    if ($repository_api) {
+      $result->setRepositoryAPI($repository_api);
+    }
+
+    $repository_ref = $query->getRepositoryRef();
+    if ($repository_ref) {
+      $result->setRepositoryRef($repository_ref);
+    }
+
+    $working_ref = $query->getWorkingCopyRef();
+    if ($working_ref) {
+      $result->setWorkingCopyRef($working_ref);
+    }
+
+    return $result;
   }
 
   final public function getLoaderKey() {

src/ref/ArcanistCommitRef.php

@@ -7,6 +7,7 @@ final class ArcanistCommitRef
   private $treeHash;
   private $commitEpoch;
   private $authorEpoch;
+  private $upstream;
 
   public function getRefIdentifier() {
     return pht('Commit %s', $this->getCommitHash());
@@ -17,6 +18,9 @@ final class ArcanistCommitRef
       'message' => array(
         'type' => 'string',
       ),
+      'upstream' => array(
+        'type' => 'wild',
+      ),
     );
   }
 
@@ -73,4 +77,18 @@ final class ArcanistCommitRef
     return $this->getHardpoint('message');
   }
 
+  public function getURI() {
+    return $this->getUpstreamProperty('uri');
+  }
+
+  private function getUpstreamProperty($key, $default = null) {
+    $upstream = $this->getHardpoint('upstream');
+
+    if (!$upstream) {
+      return $default;
+    }
+
+    return idx($upstream, $key, $default);
+  }
+
 }