launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:extended-revisions into turnip:master.

Commit message:
Accept extended revision syntax in API endpoints

It's useful in some cases to be able to use git's extended revision
syntax in API endpoints, for example "compare/OID^..OID".  While we
could achieve a similar effect by looking up log information first, it's
simpler and faster to just accept gitrevisions(7) syntax.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/294700

Accept extended revision syntax in API endpoints

It's useful in some cases to be able to use git's extended revision
syntax in API endpoints, for example "compare/OID^..OID".  While we
could achieve a similar effect by looking up log information first, it's
simpler and faster to just accept gitrevisions(7) syntax.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:extended-revisions into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 86c722f..1291c13 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -22,6 +22,7 @@ from pygit2 import (
     GIT_SORT_TOPOLOGICAL,
     IndexEntry,
     init_repository,
+    Oid,
     Repository,
     )
 
@@ -414,15 +415,21 @@ def get_log(repo_store, repo_name, start=None, limit=None, stop=None):
         return [format_commit(commit) for commit in walker]
 
 
-def get_commit(repo_store, repo_name, commit_oid, repo=None):
-    """Return a single commit object from an oid."""
+def get_commit(repo_store, repo_name, revision, repo=None):
+    """Return a single commit object from a revision."""
     with ExitStack() as stack:
         if not repo:
             repo = stack.enter_context(open_repo(repo_store, repo_name))
-        git_object = repo.get(commit_oid)
-        if git_object is None:
+        try:
+            if isinstance(revision, Oid):
+                git_object = repo.get(revision)
+                if git_object is None:
+                    raise KeyError
+            else:
+                git_object = repo.revparse_single(revision)
+        except KeyError:
             raise GitError('Object {} does not exist in repository {}.'.format(
-                commit_oid, repo_name))
+                revision, repo_name))
         return format_commit(git_object)
 
 
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 1e81dae..90e610b 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -345,6 +345,17 @@ class ApiTestCase(TestCase):
             self.repo_path, c1, c1))
         self.assertEqual('', resp.json_body['patch'])
 
+    def test_repo_get_diff_extended_revision(self):
+        """get_diff can take general git revisions, not just sha1s."""
+        repo = RepoFactory(self.repo_store)
+        c1 = repo.add_commit('foo\n', 'foobar.txt')
+        c2 = repo.add_commit('bar\n', 'foobar.txt', parents=[c1])
+
+        path = '/repo/{}/compare/{}^..{}'.format(self.repo_path, c2, c2)
+        resp = self.app.get(path)
+        self.assertIn(b'-foo', resp.body)
+        self.assertIn(b'+bar', resp.body)
+
     def test_repo_diff_merge(self):
         """Ensure expected changes exist in diff patch."""
         repo = RepoFactory(self.repo_store)