From e26c4bddab330040e6616821ae409fab834c27ed Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 1 Nov 2018 20:15:23 -0700
Subject: [PATCH] Replace magical "branch" behavior in "diffusion.branchquery"
 with an explicit "patterns"

Summary:
See PHI958. Ref T13210. Previously, see PHI720.

The use case for the magic in PHI720 involves multiple patterns, and no parameter can be passed to `branch` that will result in multiple patterns being passed to `git`.

Replace the implicit magic with an explicit `patterns` parameter.

This whole thing is a bit shaky but probably isn't hurting anything.

Test Plan:
  - Ran query with no `patterns`.
  - Ran query with invalid `patterns`, got readable error.
  - Ran query with various valid `patterns` (plain branch name, globs with "?" and "*"), got sensible results.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13210

Differential Revision: https://secure.phabricator.com/D19771
---
 .../DiffusionBranchQueryConduitAPIMethod.php  | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
index 62878c0291..ac2c223c1e 100644
--- a/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionBranchQueryConduitAPIMethod.php
@@ -21,6 +21,7 @@ final class DiffusionBranchQueryConduitAPIMethod
       'limit' => 'optional int',
       'offset' => 'optional int',
       'contains' => 'optional string',
+      'patterns' => 'optional list<string>',
     );
   }
 
@@ -31,15 +32,17 @@ final class DiffusionBranchQueryConduitAPIMethod
     $contains = $request->getValue('contains');
     if (strlen($contains)) {
 
-      // See PHI720. If the standard "branch" field is provided, use it
-      // as the "pattern" argument to "git branch ..." to let callers test
-      // for reachability from a particular branch head.
-      $pattern = $request->getValue('branch');
-      if (strlen($pattern)) {
-        $pattern_argv = array($pattern);
-      } else {
-        $pattern_argv = array();
-      }
+      // See PHI958 (and, earlier, PHI720). If "patterns" are provided, pass
+      // them to "git branch ..." to let callers test for reachability from
+      // particular branch heads.
+      $patterns_argv = $request->getValue('patterns', array());
+      PhutilTypeSpec::checkMap(
+        array(
+          'patterns' => $patterns_argv,
+        ),
+        array(
+          'patterns' => 'list<string>',
+        ));
 
       // NOTE: We can't use DiffusionLowLevelGitRefQuery here because
       // `git for-each-ref` does not support `--contains`.
@@ -47,14 +50,14 @@ final class DiffusionBranchQueryConduitAPIMethod
         list($stdout) = $repository->execxLocalCommand(
           'branch --verbose --no-abbrev --contains %s -- %Ls',
           $contains,
-          $pattern_argv);
+          $patterns_argv);
         $ref_map = DiffusionGitBranch::parseLocalBranchOutput(
           $stdout);
       } else {
         list($stdout) = $repository->execxLocalCommand(
           'branch -r --verbose --no-abbrev --contains %s -- %Ls',
           $contains,
-          $pattern_argv);
+          $patterns_argv);
         $ref_map = DiffusionGitBranch::parseRemoteBranchOutput(
           $stdout,
           DiffusionGitBranch::DEFAULT_GIT_REMOTE);