Fix tag content display in Git

Summary: Fixes the junk I broke in D7484. Before that, tag content was a side effect of resolving the ref name. Now, fetch it explicitly in `diffusion.tagsquery`.

Test Plan: Looked at a tag, saw the annotation/message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php

@@ -7,8 +7,7 @@ final class ConduitAPI_diffusion_tagsquery_Method
   extends ConduitAPI_diffusion_abstractquery_Method {
 
   public function getMethodDescription() {
-    return
+    return pht('Retrieve information about tags in a repository.');
-      'Find tags for a given commit or list tags in the repository.';
   }
 
   public function defineReturnType() {
@@ -17,7 +16,9 @@ final class ConduitAPI_diffusion_tagsquery_Method
 
   protected function defineCustomParamTypes() {
     return array(
+      'names' => 'optional list<string>',
       'commit' => 'optional string',
+      'needMessages' => 'optional bool',
       'offset' => 'optional int',
       'limit' => 'optional int',
     );
@@ -28,50 +29,41 @@ final class ConduitAPI_diffusion_tagsquery_Method
     $repository = $drequest->getRepository();
     $commit = $drequest->getRawCommit();
 
+    $commit_filter = null;
+    if ($commit) {
+      $commit_filter = $this->loadTagNamesForCommit($commit);
+    }
+
+    $name_filter = $request->getValue('names', null);
+
+    $all_tags = $this->loadGitTagList();
+    $all_tags = mpull($all_tags, null, 'getName');
+    if ($name_filter !== null) {
+      $all_tags = array_intersect_key($all_tags, array_fuse($name_filter));
+    }
+    if ($commit_filter !== null) {
+      $all_tags = array_intersect_key($all_tags, array_fuse($commit_filter));
+    }
+    $tags = array_values($all_tags);
+
     $offset = $request->getValue('offset');
     $limit = $request->getValue('limit');
-
-    if (!$commit) {
-      return $this->loadGitTagList($offset, $limit);
-    }
-
-    list($err, $stdout) = $repository->execLocalCommand(
-      'tag -l --contains %s',
-      $commit);
-
-    if ($err) {
-      // Git exits with an error code if the commit is bogus.
-      return array();
-    }
-
-    $stdout = trim($stdout);
-    if (!strlen($stdout)) {
-      return array();
-    }
-
-    $tag_names = explode("\n", $stdout);
-    $tag_names = array_fill_keys($tag_names, true);
-
-    $tags = $this->loadGitTagList($offset = 0, $limit = 0, $serialize = false);
-
-    $result = array();
-    foreach ($tags as $tag) {
-      if (isset($tag_names[$tag->getName()])) {
-        $result[] = $tag->toDictionary();
-      }
-    }
-
     if ($offset) {
-      $result = array_slice($result, $offset);
+      $tags = array_slice($tags, $offset);
-    }
-    if ($limit) {
-      $result = array_slice($result, 0, $limit);
     }
 
-    return $result;
+    if ($limit) {
+      $tags = array_slice($tags, 0, $limit);
+    }
+
+    if ($request->getValue('needMessages')) {
+      $this->loadMessagesForTags($all_tags);
+    }
+
+    return mpull($tags, 'toDictionary');
   }
 
-  private function loadGitTagList($offset, $limit, $serialize=true) {
+  private function loadGitTagList() {
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
 
@@ -94,17 +86,68 @@ final class ConduitAPI_diffusion_tagsquery_Method
       $tags[] = $tag;
     }
 
-    if ($offset) {
+    return $tags;
-      $tags = array_slice($tags, $offset);
+  }
+
+  private function loadTagNamesForCommit($commit) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    list($err, $stdout) = $repository->execLocalCommand(
+      'tag -l --contains %s',
+      $commit);
+
+    if ($err) {
+      // Git exits with an error code if the commit is bogus.
+      return array();
     }
 
-    if ($limit) {
+    $stdout = rtrim($stdout, "\n");
-      $tags = array_slice($tags, 0, $limit);
+    if (!strlen($stdout)) {
+      return array();
     }
 
-    if ($serialize) {
+    $tag_names = explode("\n", $stdout);
-      $tags = mpull($tags, 'toDictionary');
+    $tag_names = array_fill_keys($tag_names, true);
+
+    return $tag_names;
+  }
+
+  private function loadMessagesForTags(array $tags) {
+    assert_instances_of($tags, 'DiffusionRepositoryTag');
+
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+
+    $futures = array();
+    foreach ($tags as $key => $tag) {
+      $futures[$key] = $repository->getLocalCommandFuture(
+        'cat-file tag %s',
+        $tag->getName());
     }
+
+    Futures($futures)->resolveAll();
+
+    foreach ($tags as $key => $tag) {
+      $future = $futures[$key];
+      list($err, $stdout) = $future->resolve();
+
+      $message = null;
+      if ($err) {
+        // Not all tags are actually "tag" objects: a "tag" object is only
+        // created if you provide a message or sign the tag. Tags created with
+        // `git tag x [commit]` are "lightweight tags" and `git cat-file tag`
+        // will fail on them. This is fine: they don't have messages.
+      } else {
+        $parts = explode("\n\n", $stdout, 2);
+        if (count($parts) == 2) {
+          $message = last($parts);
+        }
+      }
+
+      $tag->attachMessage($message);
+    }
+
     return $tags;
   }
 

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -146,13 +146,25 @@ abstract class DiffusionBrowseController extends DiffusionController {
         ),
         $drequest->getRepository()->formatCommitName($stable_commit)));
 
-    if ($drequest->getTagContent()) {
+    if ($drequest->getCommitType() == 'tag') {
-      $view->addProperty(
+      $symbolic = $drequest->getSymbolicCommit();
-        pht('Tag'),
+      $view->addProperty(pht('Tag'), $symbolic);
-        $drequest->getSymbolicCommit());
 
-      $view->addSectionHeader(pht('Tag Content'));
+      $tags = $this->callConduitWithDiffusionRequest(
-      $view->addTextContent($this->markupText($drequest->getTagContent()));
+        'diffusion.tagsquery',
+        array(
+          'names' => array($symbolic),
+          'needMessages' => true,
+        ));
+      $tags = DiffusionRepositoryTag::newFromConduit($tags);
+
+      $tags = mpull($tags, null, 'getName');
+      $tag = idx($tags, $symbolic);
+
+      if ($tag && strlen($tag->getMessage())) {
+        $view->addSectionHeader(pht('Tag Content'));
+        $view->addTextContent($this->markupText($tag->getMessage()));
+      }
     }
 
     return $view;

src/applications/diffusion/data/DiffusionRepositoryTag.php

@@ -9,6 +9,8 @@ final class DiffusionRepositoryTag {
   private $description;
   private $type;
 
+  private $message = false;
+
   public function setType($type) {
     $this->type = $type;
     return $this;
@@ -63,26 +65,51 @@ final class DiffusionRepositoryTag {
     return $this->author;
   }
 
+  public function attachMessage($message) {
+    $this->message = $message;
+    return $this;
+  }
+
+  public function getMessage() {
+    if ($this->message === false) {
+      throw new Exception("Message is not attached!");
+    }
+    return $this->message;
+  }
+
   public function toDictionary() {
-    return array(
+    $dict = array(
       'author' => $this->getAuthor(),
       'epoch' => $this->getEpoch(),
       'commitIdentifier' => $this->getCommitIdentifier(),
       'name' => $this->getName(),
       'description' => $this->getDescription(),
-      'type' => $this->getType());
+      'type' => $this->getType(),
+    );
+
+    if ($this->message !== false) {
+      $dict['message'] = $this->message;
+    }
+
+    return $dict;
   }
 
   public static function newFromConduit(array $dicts) {
     $tags = array();
     foreach ($dicts as $dict) {
-      $tags[] = id(new DiffusionRepositoryTag())
+      $tag = id(new DiffusionRepositoryTag())
         ->setAuthor($dict['author'])
         ->setEpoch($dict['epoch'])
         ->setCommitIdentifier($dict['commitIdentifier'])
         ->setName($dict['name'])
         ->setDescription($dict['description'])
         ->setType($dict['type']);
+
+      if (array_key_exists('message', $dict)) {
+        $tag->attachMessage($dict['message']);
+      }
+
+      $tags[] = $tag;
     }
     return $tags;
   }

src/applications/diffusion/request/DiffusionRequest.php

@@ -646,6 +646,10 @@ abstract class DiffusionRequest {
     $this->tagContent = $commit_data['tagContent'];
   }
 
+  public function getCommitType() {
+    return $this->commitType;
+  }
+
   private function queryStableCommitName() {
     if ($this->commit) {
       $this->stableCommitName = $this->commit;