Handle http queries below /

Previously git review could not properly query Gerrit servers via http
if the root of the Gerrit api was below /. This is because it was always
rooting the changes query api at /changes instead of /foo/changes if
hosted at /foo.

To fix this we read the project name from the config so that we can
remove the project name suffix from the urls then append changes/ to the
resulting url to get a properly rooted query url.

Note this was never a problem with ssh because ssh can't be hosted as
some subpath. Everything is rooted with ssh and gerrit.

Change-Id: I46e21dfdbbb7f60aa89a7b028501c0d953ae1d7f

3 files changed, 51 insertions, 19 deletions

diff --git a/git_review/cmd.py b/git_review/cmd.py
index 7695dbe..5897799 100755
--- a/git_review/cmd.py
+++ b/git_review/cmd.py
@@ -180,7 +180,7 @@ def git_credentials(url):
     """Return credentials using git credential or None."""
     cmd = 'git', 'credential', 'fill'
     stdin = 'url=%s' % url
-    rc, out = run_command_status(*cmd, stdin=stdin)
+    rc, out = run_command_status(*cmd, stdin=stdin.encode('utf-8'))
     if rc:
         return None
     data = dict(l.split('=', 1) for l in out.splitlines())
@@ -558,29 +558,49 @@ def parse_gerrit_ssh_params_from_git_url(git_url):
     return (hostname, username, port, project_name)
 
 
-def query_reviews(remote_url, change=None, current_patch_set=True,
-                  exception=CommandFailed, parse_exc=Exception):
+def query_reviews(remote_url, project=None, change=None,
+                  current_patch_set=True, exception=CommandFailed,
+                  parse_exc=Exception):
     if remote_url.startswith('http://') or remote_url.startswith('https://'):
         query = query_reviews_over_http
     else:
         query = query_reviews_over_ssh
     return query(remote_url,
+                 project=project,
                  change=change,
                  current_patch_set=current_patch_set,
                  exception=exception,
                  parse_exc=parse_exc)
 
 
-def query_reviews_over_http(remote_url, change=None, current_patch_set=True,
-                            exception=CommandFailed, parse_exc=Exception):
-    url = urljoin(remote_url, '/changes/')
+def query_reviews_over_http(remote_url, project=None, change=None,
+                            current_patch_set=True, exception=CommandFailed,
+                            parse_exc=Exception):
+    if project:
+        # Remove any trailing .git suffixes for project to url comparison
+        clean_url = os.path.splitext(remote_url)[0]
+        clean_project = os.path.splitext(project)[0]
+        if clean_url.endswith(clean_project):
+            # Get the "root" url for gerrit by removing the project from the
+            # url. For example:
+            # https://example.com/foo/project.git gets truncated to
+            # https://example.com/foo/ regardless of whether or not none,
+            # either, or both of the remote_url or project strings end
+            # with .git.
+            remote_url = clean_url[:-len(clean_project)]
+    url = urljoin(remote_url, 'changes/')
     if change:
         if current_patch_set:
             url += '?q=%s&o=CURRENT_REVISION' % change
         else:
             url += '?q=%s&o=ALL_REVISIONS' % change
     else:
-        project_name = re.sub(r"^/|(\.git$)", "", urlparse(remote_url).path)
+        if project:
+            project_name = re.sub(r"^/|(\.git$)", "",
+                                  project)
+        else:
+            project_name = re.sub(r"^/|(\.git$)", "",
+                                  urlparse(remote_url).path)
         params = urlencode({'q': 'project:%s status:open' % project_name})
         url += '?' + params
 
@@ -611,8 +631,9 @@ def query_reviews_over_http(remote_url, change=None, current_patch_set=True,
     return reviews
 
 
-def query_reviews_over_ssh(remote_url, change=None, current_patch_set=True,
-                           exception=CommandFailed, parse_exc=Exception):
+def query_reviews_over_ssh(remote_url, project=None, change=None,
+                           current_patch_set=True, exception=CommandFailed,
+                           parse_exc=Exception):
     (hostname, username, port, project_name) = \
         parse_gerrit_ssh_params_from_git_url(remote_url)
 
@@ -1074,11 +1095,12 @@ class ReviewsPrinter(object):
         print("Found %d items for review" % total_reviews)
 
 
-def list_reviews(remote, with_topic=False):
+def list_reviews(remote, project, with_topic=False):
     remote_url = get_remote_url(remote)
 
     reviews = []
     for r in query_reviews(remote_url,
+                           project=project,
                            exception=CannotQueryOpenChangesets,
                            parse_exc=CannotParseOpenChangesets):
         reviews.append(Review(r))
@@ -1163,7 +1185,7 @@ class BranchTrackingMismatch(GitReviewException):
     EXIT_CODE = 70
 
 
-def fetch_review(review, masterbranch, remote):
+def fetch_review(review, masterbranch, remote, project):
     remote_url = get_remote_url(remote)
 
     review_arg = review
@@ -1171,6 +1193,7 @@ def fetch_review(review, masterbranch, remote):
     current_patch_set = patchset_number is None
 
     review_infos = query_reviews(remote_url,
+                                 project=project,
                                  change=review,
                                  current_patch_set=current_patch_set,
                                  exception=CannotQueryPatchSet,
@@ -1556,7 +1579,8 @@ def _main():
                            branch, remote, options.rebase)
             return
         local_branch, remote_branch = fetch_review(options.changeidentifier,
-                                                   branch, remote)
+                                                   branch, remote,
+                                                   config['project'])
         if options.download:
             checkout_review(local_branch, remote, remote_branch)
         else:
@@ -1569,7 +1593,7 @@ def _main():
         return
     elif options.list:
         with_topic = options.list > 1
-        list_reviews(remote, with_topic=with_topic)
+        list_reviews(remote, config['project'], with_topic=with_topic)
         return
 
     if options.custom_script:
diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py
index de00486..71012ac 100644
--- a/git_review/tests/test_git_review.py
+++ b/git_review/tests/test_git_review.py
@@ -486,6 +486,8 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
                                                 'test/test_project2')
         self._run_git('fetch', project2_uri, 'HEAD')
         self._run_git('checkout', 'FETCH_HEAD')
+        # We have to rewrite the .gitreview file after this checkout.
+        self._create_gitreview_file()
         self._simple_change('project2: test1', 'project2: change1, open')
         self._run_git('push', project2_uri, 'HEAD:refs/for/master')
 
diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py
index 4f9fc29..ef4ad5f 100644
--- a/git_review/tests/test_unit.py
+++ b/git_review/tests/test_unit.py
@@ -110,7 +110,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
 
         mock_query.return_value = self.reviews
         with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
-            cmd.list_reviews(None)
+            cmd.list_reviews(None, None)
             console_output = output.getvalue().split('\n')
 
         self.assertEqual(
@@ -126,7 +126,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
 
         mock_query.return_value = self.reviews
         with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
-            cmd.list_reviews(None, with_topic=True)
+            cmd.list_reviews(None, None, with_topic=True)
             console_output = output.getvalue().split('\n')
 
         self.assertEqual(
@@ -142,7 +142,7 @@ class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
 
         mock_query.return_value = self.reviews
         with mock.patch('sys.stdout', new_callable=io.StringIO) as output:
-            cmd.list_reviews(None)
+            cmd.list_reviews(None, None)
             console_output = output.getvalue().split('\n')
 
         wrapper = textwrap.TextWrapper(replace_whitespace=False,
@@ -295,8 +295,10 @@ class GitReviewUnitTest(testtools.TestCase):
         url = 'http://user@gerrit.example.com'
 
         cmd.run_http_exc(FakeException, url)
+        # This gets encoded to utf8 which means the type passed down
+        # is bytes.
         mock_run.assert_called_once_with('git', 'credential', 'fill',
-                                         stdin='url=%s' % url)
+                                         stdin=b'url=%s' % url.encode('utf-8'))
         calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
         mock_get.assert_has_calls(calls)
 
@@ -311,8 +313,10 @@ class GitReviewUnitTest(testtools.TestCase):
             self.fails('Exception expected')
         except FakeException as err:
             self.assertEqual(cmd.http_code_2_return_code(401), err.code)
+        # This gets encoded to utf8 which means the type passed down
+        # is bytes.
         mock_run.assert_called_once_with('git', 'credential', 'fill',
-                                         stdin='url=%s' % url)
+                                         stdin=b'url=%s' % url.encode('utf-8'))
         calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
         mock_get.assert_has_calls(calls)
 
@@ -327,8 +331,10 @@ class GitReviewUnitTest(testtools.TestCase):
             self.fails('Exception expected')
         except FakeException as err:
             self.assertEqual(cmd.http_code_2_return_code(401), err.code)
+        # This gets encoded to utf8 which means the type passed down
+        # is bytes.
         mock_run.assert_called_once_with('git', 'credential', 'fill',
-                                         stdin='url=%s' % url)
+                                         stdin=b'url=%s' % url.encode('utf-8'))
         mock_get.assert_called_once_with(url)
 
     @mock.patch('sys.argv', ['argv0', '--track', 'branch'])