launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/turnip/default-branch into lp:turnip.

Commit message:
Add GET and PATCH methods to RepoAPI to allow getting and setting the default branch (a.k.a. "HEAD").

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/default-branch/+merge/259493

Add GET and PATCH methods to RepoAPI to allow getting and setting the default branch (a.k.a. "HEAD").
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/turnip/default-branch into lp:turnip.

=== modified file 'turnip/api/store.py'
--- turnip/api/store.py	2015-04-30 19:26:04 +0000
+++ turnip/api/store.py	2015-05-19 12:55:50 +0000
@@ -157,6 +157,16 @@
         yield Repository(repo_path)
 
 
+def get_default_branch(repo_path):
+    repo = Repository(repo_path)
+    return repo.lookup_reference('HEAD').target
+
+
+def set_default_branch(repo_path, target):
+    repo = Repository(repo_path)
+    repo.set_head(target)
+
+
 def delete_repo(repo_path):
     """Permanently delete a git repository from repo store."""
     shutil.rmtree(repo_path)

=== modified file 'turnip/api/tests/test_api.py'
--- turnip/api/tests/test_api.py	2015-04-30 19:26:04 +0000
+++ turnip/api/tests/test_api.py	2015-05-19 12:55:50 +0000
@@ -35,6 +35,11 @@
         self.commit = {'ref': 'refs/heads/master', 'message': 'test commit.'}
         self.tag = {'ref': 'refs/tags/tag0', 'message': 'tag message'}
 
+    def assertReferencesEqual(self, repo, expected, observed):
+        self.assertEqual(
+            repo.lookup_reference(expected).peel().oid,
+            repo.lookup_reference(observed).peel().oid)
+
     def get_ref(self, ref):
         resp = self.app.get('/repo/{}/{}'.format(self.repo_path, ref))
         return resp.json
@@ -74,6 +79,40 @@
         self.assertEqual(200, resp.status_code)
         self.assertIn(new_repo_path, resp.json['repo_url'])
 
+    def test_repo_get(self):
+        """The GET method on a repository returns its properties."""
+        factory = RepoFactory(self.repo_store, num_branches=2, num_commits=1)
+        factory.build()
+        factory.repo.set_head('refs/heads/branch-0')
+
+        resp = self.app.get('/repo/{}'.format(self.repo_path))
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual({'default_branch': 'refs/heads/branch-0'}, resp.json)
+
+    def test_repo_get_default_branch_missing(self):
+        """default_branch is returned even if that branch has been deleted."""
+        factory = RepoFactory(self.repo_store, num_branches=2, num_commits=1)
+        factory.build()
+        factory.repo.set_head('refs/heads/branch-0')
+        factory.repo.lookup_reference('refs/heads/branch-0').delete()
+
+        resp = self.app.get('/repo/{}'.format(self.repo_path))
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual({'default_branch': 'refs/heads/branch-0'}, resp.json)
+
+    def test_repo_patch_default_branch(self):
+        """A repository's default branch ("HEAD") can be changed."""
+        factory = RepoFactory(self.repo_store, num_branches=2, num_commits=1)
+        factory.build()
+        factory.repo.set_head('refs/heads/branch-0')
+        self.assertReferencesEqual(factory.repo, 'refs/heads/branch-0', 'HEAD')
+
+        resp = self.app.patch_json(
+            '/repo/{}'.format(self.repo_path),
+            {'default_branch': 'refs/heads/branch-1'})
+        self.assertEqual(204, resp.status_code)
+        self.assertReferencesEqual(factory.repo, 'refs/heads/branch-1', 'HEAD')
+
     def test_cross_repo_merge_diff(self):
         """Merge diff can be requested across 2 repositories."""
         factory = RepoFactory(self.repo_store)

=== modified file 'turnip/api/views.py'
--- turnip/api/views.py	2015-04-29 04:42:05 +0000
+++ turnip/api/views.py	2015-05-19 12:55:50 +0000
@@ -79,6 +79,41 @@
             return exc.HTTPConflict()  # 409
 
     @validate_path
+    def get(self, repo_store, repo_name):
+        """Get properties of an existing git repository."""
+        repo_path = os.path.join(repo_store, repo_name)
+        if not os.path.exists(repo_path):
+            self.request.errors.add(
+                'body', 'name', 'repository does not exist')
+            raise exc.HTTPNotFound()
+        return {
+            'default_branch': store.get_default_branch(repo_path),
+            }
+
+    def _patch_default_branch(self, repo_path, value):
+        try:
+            store.set_default_branch(repo_path, value)
+        except (KeyError, ValueError, GitError):
+            raise exc.HTTPBadRequest()
+
+    @validate_path
+    def patch(self, repo_store, repo_name):
+        """Change properties of an existing git repository."""
+        repo_path = os.path.join(repo_store, repo_name)
+        if not os.path.exists(repo_path):
+            self.request.errors.add(
+                'body', 'name', 'repository does not exist')
+            raise exc.HTTPNotFound()
+        data = extract_json_data(self.request)
+        for key in data:
+            if not hasattr(self, "_patch_%s" % key):
+                self.request.errors.add('body', key, 'unknown property')
+                raise exc.HTTPBadRequest()
+        for key, value in data.items():
+            getattr(self, "_patch_%s" % key)(repo_path, value)
+        return exc.HTTPNoContent()
+
+    @validate_path
     def delete(self, repo_store, repo_name):
         """Delete an existing git repository."""
         try: