launchpad-reviewers team mailing list archive

[Bayard 'kit' Randel <kit.randel@xxxxxxxxxxxxx>]

Bayard 'kit' Randel has proposed merging lp:~blr/turnip/api-clone into lp:turnip.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~blr/turnip/api-clone/+merge/253942

Add clone param to RepoAPI providing init from existing repo (clone) e.g.

POST /repo {'repo_path': existing_repo_path, 'clone_path': new_repo_path})
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/turnip/api-clone into lp:turnip.

=== modified file 'turnip/api/store.py'
--- turnip/api/store.py	2015-03-13 03:44:09 +0000
+++ turnip/api/store.py	2015-03-24 21:30:45 +0000
@@ -3,6 +3,8 @@
 import itertools
 import os
 import shutil
+import urllib
+import urlparse
 
 from pygit2 import (
     GitError,
@@ -10,6 +12,7 @@
     GIT_OBJ_COMMIT,
     GIT_OBJ_TREE,
     GIT_OBJ_TAG,
+    clone_repository,
     init_repository,
     Repository,
     )
@@ -60,23 +63,33 @@
         }
 
 
-def init_repo(repo, is_bare=True):
-    """Initialise a git repository."""
-    if os.path.exists(repo):
-        raise Exception("Repository '%s' already exists" % repo)
-    repo_path = init_repository(repo, is_bare)
-    return repo_path
+def is_valid_new_path(path):
+    """Verify repo path is new, or raise Exception."""
+    if os.path.exists(path):
+        raise Exception("Repository '%s' already exists" % path)
+    return True
+
+
+def init_repo(repo_path, clone_path=None, is_bare=True):
+    """Initialise a new git repository or clone from existing."""
+    if clone_path:
+        assert is_valid_new_path(clone_path)
+        repo_url = urlparse.urljoin('file:', urllib.pathname2url(repo_path))
+        repo = clone_repository(repo_url, clone_path, is_bare)
+    else:
+        assert is_valid_new_path(repo_path)
+        repo = init_repository(repo_path, is_bare)
+    return repo.path
 
 
 def open_repo(repo_path):
     """Open an existing git repository."""
-    repo = Repository(repo_path)
-    return repo
-
-
-def delete_repo(repo):
+    return Repository(repo_path)
+
+
+def delete_repo(repo_path):
     """Permanently delete a git repository from repo store."""
-    shutil.rmtree(repo)
+    shutil.rmtree(repo_path)
 
 
 def get_refs(repo_path):

=== modified file 'turnip/api/tests/test_api.py'
--- turnip/api/tests/test_api.py	2015-03-13 03:24:22 +0000
+++ turnip/api/tests/test_api.py	2015-03-24 21:30:45 +0000
@@ -14,7 +14,11 @@
 from webtest import TestApp
 
 from turnip import api
-from turnip.api.tests.test_helpers import RepoFactory
+from turnip.api.tests.test_helpers import (
+    get_revlist,
+    open_repo,
+    RepoFactory,
+    )
 
 
 class ApiTestCase(TestCase):
@@ -24,8 +28,9 @@
         repo_store = self.useFixture(TempDir()).path
         self.useFixture(EnvironmentVariable("REPO_STORE", repo_store))
         self.app = TestApp(api.main({}))
-        self.repo_path = str(uuid.uuid1())
+        self.repo_path = uuid.uuid1().hex
         self.repo_store = os.path.join(repo_store, self.repo_path)
+        self.repo_root = repo_store
         self.commit = {'ref': 'refs/heads/master', 'message': 'test commit.'}
         self.tag = {'ref': 'refs/tags/tag0', 'message': 'tag message'}
 
@@ -33,9 +38,31 @@
         resp = self.app.get('/repo/{}/{}'.format(self.repo_path, ref))
         return resp.json
 
-    def test_repo_create(self):
+    def test_repo_init(self):
         resp = self.app.post_json('/repo', {'repo_path': self.repo_path})
-        self.assertEqual(resp.status_code, 200)
+        self.assertIn(self.repo_path, resp.json['repo_url'])
+        self.assertEqual(resp.status_code, 200)
+
+    def test_repo_init_with_invalid_repo_path(self):
+        resp = self.app.post_json('/repo', {'repo_path': '../1234'},
+                                  expect_errors=True)
+        self.assertEqual(resp.status_code, 500)
+
+    def test_repo_init_with_clone(self):
+        """Repo can be initialised with optional clone."""
+        factory = RepoFactory(self.repo_store, num_commits=2)
+        factory.build()
+        new_repo_path = uuid.uuid1().hex
+        resp = self.app.post_json('/repo', {'repo_path': self.repo_path,
+                                            'clone_path': new_repo_path})
+        repo1_revlist = get_revlist(factory.repo)
+        clone_path = resp.json['repo_url'].split('/')[-1]
+        repo2 = open_repo(os.path.join(self.repo_root, clone_path))
+        repo2_revlist = get_revlist(repo2)
+
+        self.assertEqual(repo1_revlist, repo2_revlist)
+        self.assertEqual(resp.status_code, 200)
+        self.assertIn(new_repo_path, resp.json['repo_url'])
 
     def test_repo_delete(self):
         self.app.post_json('/repo', {'repo_path': self.repo_path})

=== modified file 'turnip/api/tests/test_helpers.py'
--- turnip/api/tests/test_helpers.py	2015-03-13 03:44:09 +0000
+++ turnip/api/tests/test_helpers.py	2015-03-24 21:30:45 +0000
@@ -8,10 +8,21 @@
     GIT_OBJ_COMMIT,
     GIT_SORT_TIME,
     IndexEntry,
+    Repository,
     Signature,
     )
 
 
+def get_revlist(repo):
+    """Return revlist for a given pygit2 repo object."""
+    return [commit.oid.hex for commit in repo.walk(repo.head.target)]
+
+
+def open_repo(repo_path):
+    """Return a pygit2 repo object for a given path."""
+    return Repository(repo_path)
+
+
 class RepoFactory():
     """Builds a git repository in a user defined state."""
 

=== modified file 'turnip/api/views.py'
--- turnip/api/views.py	2015-03-13 03:44:09 +0000
+++ turnip/api/views.py	2015-03-24 21:30:45 +0000
@@ -11,6 +11,11 @@
 from turnip.api import store
 
 
+def is_valid_path(repo_store, repo_path):
+    """Ensure path in within repo root and has not been subverted."""
+    return os.path.realpath(repo_path).startswith(os.path.realpath(repo_store))
+
+
 def repo_path(func):
     """Decorator builds repo path from request name and repo_store."""
     def repo_path_decorator(self):
@@ -19,10 +24,10 @@
             self.request.errors.add('body', 'name', 'repo name is missing')
             return
         repo_path = os.path.join(self.repo_store, name)
-        if not os.path.realpath(repo_path).startswith(
-                os.path.realpath(self.repo_store)):
+        if not is_valid_path(self.repo_store, repo_path):
             self.request.errors.add('body', 'name', 'invalid path.')
             raise exc.HTTPInternalServerError()
+
         return func(self, repo_path)
     return repo_path_decorator
 
@@ -42,15 +47,28 @@
         self.request = request
 
     def collection_post(self):
-        """Initialise a new git repository."""
+        """Initialise a new git repository, or clone from an existing repo."""
         repo_path = extract_json_data(self.request).get('repo_path')
+        clone_path = extract_json_data(self.request).get('clone_path')
+
         if not repo_path:
             self.request.errors.add('body', 'repo_path',
                                     'repo_path is missing')
             return
         repo = os.path.join(self.repo_store, repo_path)
+        if not is_valid_path(self.repo_store, repo):
+            self.request.errors.add('body', 'name', 'invalid path.')
+            raise exc.HTTPInternalServerError()
+
+        if clone_path:
+            repo_clone = os.path.join(self.repo_store, clone_path)
+        else:
+            repo_clone = None
+
         try:
-            store.init_repo(repo)
+            new_repo_path = store.init_repo(repo, repo_clone)
+            repo_name = os.path.basename(os.path.normpath(new_repo_path))
+            return {'repo_url': '/'.join([self.request.url, repo_name])}
         except GitError:
             return exc.HTTPConflict()  # 409