launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18419
Re: [Merge] lp:~blr/turnip/api-init-with-alternates into lp:turnip
Review: Approve code
Diff comments:
> === modified file 'requirements.txt'
> --- requirements.txt 2015-03-12 13:10:48 +0000
> +++ requirements.txt 2015-04-29 04:42:20 +0000
> @@ -1,3 +1,4 @@
> +contextlib2==0.4.0
> cornice==0.19.0
> lazr.sshserver==0.1.1
> PasteDeploy==1.5.2
>
> === modified file 'setup.py'
> --- setup.py 2015-04-24 09:37:22 +0000
> +++ setup.py 2015-04-29 04:42:20 +0000
> @@ -15,6 +15,7 @@
> README = f.read()
>
> requires = [
> + 'contextlib2',
You'll need to add this to lp:~canonical-launchpad-branches/turnip/dependencies.
> 'cornice',
> 'lazr.sshserver',
> # Should be 0.22.1 once released; for the time being we carry cherry-picks.
>
> === modified file 'turnip/api/store.py'
> --- turnip/api/store.py 2015-04-25 17:46:51 +0000
> +++ turnip/api/store.py 2015-04-29 04:42:20 +0000
> @@ -1,8 +1,13 @@
> # Copyright 2015 Canonical Ltd. All rights reserved.
>
> +from contextlib2 import (
> + contextmanager,
> + ExitStack,
> + )
> import itertools
> import os
> import shutil
> +import uuid
>
> from pygit2 import (
> clone_repository,
> @@ -63,6 +68,10 @@
> }
>
>
> +def is_bare_repo(repo_path):
> + return not os.path.exists(os.path.join(repo_path, '.git'))
> +
> +
> def is_valid_new_path(path):
> """Verify repo path is new, or raise Exception."""
> if os.path.exists(path):
> @@ -70,7 +79,25 @@
> return True
>
>
> -def init_repo(repo_path, clone_from=None, clone_refs=False, is_bare=True):
> +def alternates_path(repo_path):
> + """Git object alternates path.
> + See http://git-scm.com/docs/gitrepository-layout
> + """
> + return os.path.join(repo_path, 'objects', 'info', 'alternates')
> +
> +
> +def write_alternates(repo_path, alternate_repo_paths):
> + with open(alternates_path(repo_path), "w") as f:
> + for path in alternate_repo_paths:
> + if is_bare_repo(path):
> + objects_path = os.path.join(path, 'objects')
> + else:
> + objects_path = os.path.join(path, '.git', 'objects')
> + f.write("{}\n".format(objects_path))
> +
> +
> +def init_repo(repo_path, clone_from=None, clone_refs=False,
> + alternate_repo_paths=None, is_bare=True):
> """Initialise a new git repository or clone from existing."""
> assert is_valid_new_path(repo_path)
> init_repository(repo_path, is_bare)
> @@ -97,12 +124,37 @@
> for ref in from_repo.listall_references():
> to_repo.create_reference(
> ref, from_repo.lookup_reference(ref).target)
> +
> + if alternate_repo_paths:
> + write_alternates(repo_path, alternate_repo_paths)
> return repo_path
>
>
> -def open_repo(repo_path):
> - """Open an existing git repository."""
> - return Repository(repo_path)
> +@contextmanager
> +def open_repo(repo_store, repo_name):
> + """Open an existing git repository. Optionally create an ephemeral
> + repository with alternates if repo_path contains ':'.
> +
> + :param repo_store: path to repository root.
> + :param repo_name: repository name.
> + """
> + if ':' in repo_name:
> + try:
> + # create ephemeral repo with alternates set from both
> + repos = [os.path.join(repo_store, repo)
> + for repo in repo_name.split(':')]
> + tmp_repo_path = os.path.join(repo_store,
> + 'ephemeral-' + uuid.uuid4().hex)
> + ephemeral_repo_path = init_repo(
> + tmp_repo_path,
> + alternate_repo_paths=repos)
> + repo = Repository(ephemeral_repo_path)
> + yield repo
> + finally:
> + delete_repo(ephemeral_repo_path)
> + else:
> + repo_path = os.path.join(repo_store, repo_name)
> + yield Repository(repo_path)
>
>
> def delete_repo(repo_path):
> @@ -110,32 +162,32 @@
> shutil.rmtree(repo_path)
>
>
> -def get_refs(repo_path):
> +def get_refs(repo_store, repo_name):
> """Return all refs for a git repository."""
> - repo = open_repo(repo_path)
> - refs = {}
> - for ref in repo.listall_references():
> - git_object = repo.lookup_reference(ref).peel()
> - # Filter non-unicode refs, as refs are treated as unicode
> - # given json is unable to represent arbitrary byte strings.
> - try:
> - ref.decode('utf-8')
> - except UnicodeDecodeError:
> - pass
> - else:
> - refs.update(format_ref(ref, git_object))
> - return refs
> -
> -
> -def get_ref(repo_path, ref):
> + with open_repo(repo_store, repo_name) as repo:
> + refs = {}
> + for ref in repo.listall_references():
> + git_object = repo.lookup_reference(ref).peel()
> + # Filter non-unicode refs, as refs are treated as unicode
> + # given json is unable to represent arbitrary byte strings.
> + try:
> + ref.decode('utf-8')
> + except UnicodeDecodeError:
> + pass
> + else:
> + refs.update(format_ref(ref, git_object))
> + return refs
> +
> +
> +def get_ref(repo_store, repo_name, ref):
> """Return a specific ref for a git repository."""
> - repo = open_repo(repo_path)
> - git_object = repo.lookup_reference(ref.encode('utf-8')).peel()
> - ref_obj = format_ref(ref, git_object)
> - return ref_obj
> -
> -
> -def get_common_ancestor_diff(repo_path, sha1_target, sha1_source,
> + with open_repo(repo_store, repo_name) as repo:
> + git_object = repo.lookup_reference(ref.encode('utf-8')).peel()
> + ref_obj = format_ref(ref, git_object)
> + return ref_obj
> +
> +
> +def get_common_ancestor_diff(repo_store, repo_name, sha1_target, sha1_source,
> context_lines=3):
> """Get diff of common ancestor and source diff.
>
> @@ -143,93 +195,98 @@
> :param sha1_source: source sha1 for merge base.
> :param context_lines: num unchanged lines that define a hunk boundary.
> """
> - repo = open_repo(repo_path)
> - common_ancestor = repo.merge_base(sha1_target, sha1_source)
> - return get_diff(repo_path, common_ancestor, sha1_source, context_lines)
> -
> -
> -def get_merge_diff(repo_path, sha1_base, sha1_head, context_lines=3):
> + with open_repo(repo_store, repo_name) as repo:
> + common_ancestor = repo.merge_base(sha1_target, sha1_source)
> + return get_diff(repo_store, repo_name, common_ancestor,
> + sha1_source, context_lines)
> +
> +
> +def get_merge_diff(repo_store, repo_name, sha1_base,
> + sha1_head, context_lines=3):
> """Get diff of common ancestor and source diff.
>
> :param sha1_base: target sha1 for merge.
> :param sha1_head: source sha1 for merge.
> :param context_lines: num unchanged lines that define a hunk boundary.
> """
> - repo = open_repo(repo_path)
> - merged_index = repo.merge_commits(sha1_base, sha1_head)
> - conflicts = set()
> - if merged_index.conflicts is not None:
> - for conflict in list(merged_index.conflicts):
> - path = [entry for entry in conflict if entry is not None][0].path
> - conflicts.add(path)
> - merged_file = repo.merge_file_from_index(*conflict)
> - blob_oid = repo.create_blob(merged_file)
> - merged_index.add(IndexEntry(path, blob_oid, GIT_FILEMODE_BLOB))
> - del merged_index.conflicts[path]
> - diff = merged_index.diff_to_tree(
> - repo[sha1_base].tree, context_lines=context_lines).patch
> - shas = [sha1_base, sha1_head]
> - commits = [get_commit(repo_path, sha, repo) for sha in shas]
> - diff = {'commits': commits, 'patch': diff, 'conflicts': sorted(conflicts)}
> - return diff
> -
> -
> -def get_diff(repo_path, sha1_from, sha1_to, context_lines=3):
> + with open_repo(repo_store, repo_name) as repo:
> + merged_index = repo.merge_commits(sha1_base, sha1_head)
> + conflicts = set()
> + if merged_index.conflicts is not None:
> + for conflict in list(merged_index.conflicts):
> + path = [entry for entry in conflict
> + if entry is not None][0].path
> + conflicts.add(path)
> + merged_file = repo.merge_file_from_index(*conflict)
> + blob_oid = repo.create_blob(merged_file)
> + merged_index.add(IndexEntry(path, blob_oid, GIT_FILEMODE_BLOB))
> + del merged_index.conflicts[path]
> + diff = merged_index.diff_to_tree(
> + repo[sha1_base].tree, context_lines=context_lines).patch
> + shas = [sha1_base, sha1_head]
> + commits = [get_commit(repo_store, repo_name, sha, repo)
> + for sha in shas]
> + diff = {'commits': commits, 'patch': diff,
> + 'conflicts': sorted(conflicts)}
> + return diff
> +
> +
> +def get_diff(repo_store, repo_name, sha1_from, sha1_to, context_lines=3):
> """Get patch and associated commits of two sha1s.
>
> :param sha1_from: diff from sha1.
> :param sha1_to: diff to sha1.
> :param context_lines: num unchanged lines that define a hunk boundary.
> """
> - repo = open_repo(repo_path)
> - shas = [sha1_from, sha1_to]
> - commits = [get_commit(repo_path, sha, repo) for sha in shas]
> - diff = {
> - 'commits': commits,
> - 'patch': repo.diff(commits[0]['sha1'], commits[1]['sha1'],
> - False, 0, context_lines).patch
> + with open_repo(repo_store, repo_name) as repo:
> + shas = [sha1_from, sha1_to]
> + commits = [get_commit(repo_store, repo_name, sha, repo)
> + for sha in shas]
> + diff = {
> + 'commits': commits,
> + 'patch': repo.diff(commits[0]['sha1'], commits[1]['sha1'],
> + False, 0, context_lines).patch
> }
> - return diff
> -
> -
> -def get_log(repo_path, start=None, limit=None, stop=None):
> + return diff
> +
> +
> +def get_log(repo_store, repo_name, start=None, limit=None, stop=None):
> """Return a commit collection from HEAD or optionally a start oid.
>
> :param start: sha1 or branch to start listing commits from.
> :param limit: limit number of commits to return.
> :param stop: ignore a commit (and its ancestors).
> """
> - repo = open_repo(repo_path)
> - if not start:
> - start = repo.head.target # walk from HEAD
> - walker = repo.walk(start)
> - if stop:
> - walker.hide(stop) # filter stop sha1 and its ancestors
> - if limit > 0:
> - walker = itertools.islice(walker, limit)
> - commits = [format_commit(commit) for commit in walker]
> - return commits
> -
> -
> -def get_commit(repo_path, commit_oid, repo=None):
> + with open_repo(repo_store, repo_name) as repo:
> + if not start:
> + start = repo.head.target # walk from HEAD
> + walker = repo.walk(start)
> + if stop:
> + walker.hide(stop) # filter stop sha1 and its ancestors
> + if limit > 0:
> + walker = itertools.islice(walker, limit)
> + 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."""
> - if not repo:
> - repo = open_repo(repo_path)
> - git_object = repo.get(commit_oid)
> - if git_object is None:
> - raise GitError('Object {} does not exist in repository {}.'.format(
> - commit_oid, repo_path))
> - commit = format_commit(git_object)
> - return commit
> -
> -
> -def get_commits(repo_path, commit_oids):
> + 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:
> + raise GitError('Object {} does not exist in repository {}.'.format(
> + commit_oid, repo_name))
> + return format_commit(git_object)
> +
> +
> +def get_commits(repo_store, repo_name, commit_oids):
> """Return a collection of commit objects from a list of oids."""
> - repo = open_repo(repo_path)
> - commits = []
> - for commit in commit_oids:
> - try:
> - commits.append(get_commit(repo_path, commit, repo))
> - except GitError:
> - pass
> - return commits
> + with open_repo(repo_store, repo_name) as repo:
> + commits = []
> + for commit in commit_oids:
> + try:
> + commits.append(get_commit(repo_store, repo_name, commit, repo))
> + except GitError:
> + pass
> + return commits
>
> === modified file 'turnip/api/tests/test_api.py'
> --- turnip/api/tests/test_api.py 2015-04-27 13:16:08 +0000
> +++ turnip/api/tests/test_api.py 2015-04-29 04:42:20 +0000
> @@ -74,6 +74,59 @@
> self.assertEqual(200, resp.status_code)
> self.assertIn(new_repo_path, resp.json['repo_url'])
>
> + def test_cross_repo_merge_diff(self):
> + """Merge diff can be requested across 2 repositories."""
> + factory = RepoFactory(self.repo_store)
> + c1 = factory.add_commit('foo', 'foobar.txt')
> + factory.set_head(c1)
> +
> + repo2_name = uuid.uuid4().hex
> + factory2 = RepoFactory(
> + os.path.join(self.repo_root, repo2_name), clone_from=factory)
> + c2 = factory.add_commit('bar', 'foobar.txt', parents=[c1])
> + c3 = factory2.add_commit('baz', 'foobar.txt', parents=[c1])
> +
> + resp = self.app.get('/repo/{}:{}/compare-merge/{}:{}'.format(
> + self.repo_path, repo2_name, c2, c3))
> + self.assertIn('-bar', resp.json['patch'])
> +
> + def test_cross_repo_diff(self):
> + """Diff can be requested across 2 repositories."""
> + factory = RepoFactory(self.repo_store)
> + c1 = factory.add_commit('foo', 'foobar.txt')
> + factory.set_head(c1)
> +
> + repo2_name = uuid.uuid4().hex
> + factory2 = RepoFactory(
> + os.path.join(self.repo_root, repo2_name), clone_from=factory)
> + c2 = factory.add_commit('bar', 'foobar.txt', parents=[c1])
> + c3 = factory2.add_commit('baz', 'foobar.txt', parents=[c1])
> +
> + resp = self.app.get('/repo/{}:{}/compare/{}..{}'.format(
> + self.repo_path, repo2_name, c2, c3))
> + self.assertIn('-bar', resp.json['patch'])
> + self.assertIn('+baz', resp.json['patch'])
> +
> + def test_cross_repo_diff_invalid_repo(self):
> + """Cross repo diff with invalid repo returns HTTP 404."""
> + resp = self.app.get('/repo/1:2/compare-merge/3:4', expect_errors=True)
> + self.assertEqual(404, resp.status_code)
> +
> + def test_cross_repo_diff_invalid_commit(self):
> + """Cross repo diff with an invalid commit returns HTTP 404."""
> + factory = RepoFactory(self.repo_store)
> + c1 = factory.add_commit('foo', 'foobar.txt')
> + factory.set_head(c1)
> +
> + repo2_name = uuid.uuid4().hex
> + RepoFactory(
> + os.path.join(self.repo_root, repo2_name), clone_from=factory)
> + c2 = factory.add_commit('bar', 'foobar.txt', parents=[c1])
> +
> + resp = self.app.get('/repo/{}:{}/diff/{}:{}'.format(
> + self.repo_path, repo2_name, c2, 'invalid'), expect_errors=True)
> + self.assertEqual(404, resp.status_code)
> +
> def test_repo_delete(self):
> self.app.post_json('/repo', {'repo_path': self.repo_path})
> resp = self.app.delete('/repo/{}'.format(self.repo_path))
> @@ -244,12 +297,15 @@
> """Ensure expected changes exist in diff patch."""
> repo = RepoFactory(self.repo_store)
> c1 = repo.add_commit('foo\nbar\nbaz\n', 'blah.txt')
> - c2_right = repo.add_commit('quux\nbar\nbaz\n', 'blah.txt', parents=[c1])
> - c3_right = repo.add_commit('quux\nbar\nbaz\n', 'blah.txt', parents=[c2_right])
> + c2_right = repo.add_commit('quux\nbar\nbaz\n', 'blah.txt',
> + parents=[c1])
> + c3_right = repo.add_commit('quux\nbar\nbaz\n', 'blah.txt',
> + parents=[c2_right])
> c2_left = repo.add_commit('foo\nbar\nbar\n', 'blah.txt', parents=[c1])
> - c3_left = repo.add_commit('foo\nbar\nbar\n', 'blah.txt', parents=[c2_left])
> + c3_left = repo.add_commit('foo\nbar\nbar\n', 'blah.txt',
> + parents=[c2_left])
>
> - resp = self.app.get('/repo/{}/compare-merge/{}/{}'.format(
> + resp = self.app.get('/repo/{}/compare-merge/{}:{}'.format(
> self.repo_path, c3_right, c3_left))
> self.assertIn(' quux', resp.json_body['patch'])
> self.assertIn('-baz', resp.json_body['patch'])
> @@ -264,7 +320,7 @@
> c2_left = repo.add_commit('foo\nbar\n', 'blah.txt', parents=[c1])
> c2_right = repo.add_commit('foo\nbaz\n', 'blah.txt', parents=[c1])
>
> - resp = self.app.get('/repo/{}/compare-merge/{}/{}'.format(
> + resp = self.app.get('/repo/{}/compare-merge/{}:{}'.format(
> self.repo_path, c2_left, c2_right))
> self.assertIn(dedent("""\
> +<<<<<<< blah.txt
>
> === modified file 'turnip/api/tests/test_helpers.py'
> --- turnip/api/tests/test_helpers.py 2015-04-14 15:12:31 +0000
> +++ turnip/api/tests/test_helpers.py 2015-04-29 04:42:20 +0000
> @@ -2,9 +2,12 @@
>
> import itertools
> import os
> +import urllib
> +import urlparse
> import uuid
>
> from pygit2 import (
> + clone_repository,
> init_repository,
> GIT_FILEMODE_BLOB,
> GIT_OBJ_COMMIT,
> @@ -29,7 +32,7 @@
> """Builds a git repository in a user defined state."""
>
> def __init__(self, repo_path=None, num_commits=None,
> - num_branches=None, num_tags=None):
> + num_branches=None, num_tags=None, clone_from=None):
> self.author = Signature('Test Author', 'author@xxxxxxx')
> self.branches = []
> self.committer = Signature('Test Commiter', 'committer@xxxxxxx')
> @@ -38,7 +41,10 @@
> self.num_commits = num_commits
> self.num_tags = num_tags
> self.repo_path = repo_path
> - self.repo = self.init_repo()
> + if clone_from:
> + self.repo = self.clone_repo(clone_from)
> + else:
> + self.repo = self.init_repo()
>
> @property
> def commits(self):
> @@ -62,6 +68,9 @@
> blob_content, tree_id, parents)
> return oid
>
> + def set_head(self, oid):
> + self.repo.create_reference('refs/heads/master', oid)
> +
> def add_branch(self, name, oid):
> commit = self.repo.get(oid)
> branch = self.repo.create_branch('branch-{}'.format(name), commit)
> @@ -133,6 +142,12 @@
> def init_repo(self):
> return init_repository(self.repo_path)
>
> + def clone_repo(self, repo_factory):
> + """Return a pygit2 repo object cloned from an existing factory repo."""
> + clone_from_url = urlparse.urljoin(
> + 'file:', urllib.pathname2url(repo_factory.repo.path))
> + return clone_repository(clone_from_url, self.repo_path)
> +
> def build(self):
> """Return a repo, optionally with generated commits and tags."""
> if self.num_branches:
>
> === modified file 'turnip/api/tests/test_store.py'
> --- turnip/api/tests/test_store.py 2015-04-25 12:50:07 +0000
> +++ turnip/api/tests/test_store.py 2015-04-29 04:42:20 +0000
> @@ -8,20 +8,29 @@
>
> import os.path
> import subprocess
> +import uuid
>
> -from fixtures import TempDir
> +from fixtures import (
> + EnvironmentVariable,
> + TempDir,
> + )
> import pygit2
> from testtools import TestCase
>
> from turnip.api import store
> -from turnip.api.tests.test_helpers import RepoFactory
> +from turnip.api.tests.test_helpers import (
> + open_repo,
> + RepoFactory,
> + )
>
>
> class InitTestCase(TestCase):
>
> def setUp(self):
> super(InitTestCase, self).setUp()
> - self.root = self.useFixture(TempDir()).path
> + self.repo_store = self.useFixture(TempDir()).path
> + self.useFixture(EnvironmentVariable("REPO_STORE", self.repo_store))
> + self.repo_path = os.path.join(self.repo_store, uuid.uuid1().hex)
>
> def assertAllLinkCounts(self, link_count, path):
> count = 0
> @@ -44,19 +53,55 @@
> self.assertNotIn(absent, out)
>
> def makeOrig(self):
> - self.orig_path = os.path.join(self.root, 'orig/')
> + self.orig_path = os.path.join(self.repo_store, 'orig/')
> orig = RepoFactory(
> self.orig_path, num_branches=3, num_commits=2).build()
> self.orig_refs = orig.listall_references()
> self.master_oid = orig.lookup_reference('refs/heads/master').target
> self.orig_objs = os.path.join(self.orig_path, '.git/objects')
>
> + def assert_alternate_exists(self, alternate_path, repo_path):
> + """Assert alternate_path exists in alternates at repo_path."""
> + objects_path = '{}\n'.format(
> + os.path.join(alternate_path, 'objects'))
> + with open(store.alternates_path(repo_path), 'r') as alts:
> + alts_content = alts.read()
> + self.assertIn(objects_path, alts_content)
> +
> def test_from_scratch(self):
> - path = os.path.join(self.root, 'repo/')
> + path = os.path.join(self.repo_store, 'repo/')
> self.assertEqual(path, store.init_repo(path))
> r = pygit2.Repository(path)
> self.assertEqual([], r.listall_references())
>
> + def test_open_ephemeral_repo(self):
> + """Opening a repo where a repo name contains ':' should return
> + a new ephemeral repo.
> + """
> + repos = [uuid.uuid4().hex, uuid.uuid4().hex]
> + repo_name = '{}:{}'.format(repos[0], repos[1])
> + alt_path = os.path.join(self.repo_store, repos[0])
> + with store.open_repo(self.repo_store, repo_name) as repo:
> + self.assert_alternate_exists(alt_path, repo.path)
> +
> + def test_repo_with_alternates(self):
> + """Ensure objects path is defined correctly in repo alternates."""
> + factory = RepoFactory(self.repo_path)
> + new_repo_path = os.path.join(self.repo_store, uuid.uuid1().hex)
> + repo_path_with_alt = store.init_repo(
> + new_repo_path, alternate_repo_paths=[factory.repo.path])
> + self.assert_alternate_exists(factory.repo.path, repo_path_with_alt)
> +
> + def test_repo_alternates_objects_shared(self):
> + """Ensure objects are shared from alternate repo."""
> + factory = RepoFactory(self.repo_path)
> + commit_oid = factory.add_commit('foo', 'foobar.txt')
> + new_repo_path = os.path.join(self.repo_store, uuid.uuid4().hex)
> + repo_path_with_alt = store.init_repo(
> + new_repo_path, alternate_repo_paths=[factory.repo.path])
> + repo_with_alt = open_repo(repo_path_with_alt)
> + self.assertEqual(commit_oid.hex, repo_with_alt.get(commit_oid).hex)
> +
> def test_clone_with_refs(self):
> self.makeOrig()
> self.assertAllLinkCounts(1, self.orig_objs)
> @@ -64,7 +109,7 @@
> # init_repo with clone_from=orig and clone_refs=True creates a
> # repo with the same set of refs. And the objects are copied
> # too.
> - to_path = os.path.join(self.root, 'to/')
> + to_path = os.path.join(self.repo_store, 'to/')
> store.init_repo(to_path, clone_from=self.orig_path, clone_refs=True)
> to = pygit2.Repository(to_path)
> self.assertIsNot(None, to[self.master_oid])
> @@ -88,7 +133,7 @@
>
> # init_repo with clone_from=orig and clone_refs=False creates a
> # repo without any refs, but the objects are copied.
> - to_path = os.path.join(self.root, 'to/')
> + to_path = os.path.join(self.repo_store, 'to/')
> store.init_repo(to_path, clone_from=self.orig_path, clone_refs=False)
> to = pygit2.Repository(to_path)
> self.assertIsNot(None, to[self.master_oid])
>
> === modified file 'turnip/api/views.py'
> --- turnip/api/views.py 2015-04-27 13:16:08 +0000
> +++ turnip/api/views.py 2015-04-29 04:42:20 +0000
> @@ -17,9 +17,9 @@
> 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):
> +def validate_path(func):
> + """Decorator validates repo path from request name and repo_store."""
> + def validate_path_decorator(self):
> name = self.request.matchdict['name']
> if not name:
> self.request.errors.add('body', 'name', 'repo name is missing')
> @@ -28,9 +28,8 @@
> 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
> + return func(self, self.repo_store, name)
> + return validate_path_decorator
>
>
> class BaseAPI(object):
> @@ -79,10 +78,11 @@
> except GitError:
> return exc.HTTPConflict() # 409
>
> - @repo_path
> - def delete(self, repo_path):
> + @validate_path
> + def delete(self, repo_store, repo_name):
> """Delete an existing git repository."""
> try:
> + repo_path = os.path.join(repo_store, repo_name)
> store.delete_repo(repo_path)
> except (IOError, OSError):
> return exc.HTTPNotFound() # 404
> @@ -97,18 +97,18 @@
> super(RefAPI, self).__init__()
> self.request = request
>
> - @repo_path
> - def collection_get(self, repo_path):
> + @validate_path
> + def collection_get(self, repo_store, repo_name):
> try:
> - return store.get_refs(repo_path)
> + return store.get_refs(repo_store, repo_name)
> except (KeyError, GitError):
> return exc.HTTPNotFound() # 404
>
> - @repo_path
> - def get(self, repo_path):
> + @validate_path
> + def get(self, repo_store, repo_name):
> ref = 'refs/' + self.request.matchdict['ref']
> try:
> - return store.get_ref(repo_path, ref)
> + return store.get_ref(repo_store, repo_name, ref)
> except (KeyError, GitError):
> return exc.HTTPNotFound()
>
> @@ -121,13 +121,14 @@
> Two dots provides a simple diff, equivalent to `git diff A B`.
> Three dots provides the symmetric or common ancestor diff, equivalent
> to `git diff $(git-merge-base A B) B`.
> + {name} can be two : separated repositories, for a cross repository diff.
> """
> def __init__(self, request):
> super(DiffAPI, self).__init__()
> self.request = request
>
> - @repo_path
> - def get(self, repo_path):
> + @validate_path
> + def get(self, repo_store, repo_name):
> """Returns diff of two commits."""
> commits = re.split('(\.{2,3})', self.request.matchdict['commits'])
> context_lines = int(self.request.params.get('context_lines', 3))
> @@ -135,7 +136,8 @@
> return exc.HTTPBadRequest()
> try:
> diff_type = commits[1]
> - args = (repo_path, commits[0], commits[2], context_lines)
> + args = (repo_store, repo_name, commits[0],
> + commits[2], context_lines)
> if diff_type == '..':
> patch = store.get_diff(*args)
> elif diff_type == '...':
> @@ -146,23 +148,24 @@
> return patch
>
>
> -@resource(path='/repo/{name}/compare-merge/{base}/{head}')
> +@resource(path='/repo/{name}/compare-merge/{base}:{head}')
> class DiffMergeAPI(BaseAPI):
> """Provides an HTTP API for merge previews.
>
> {head} will be merged into {base} and the diff from {base} returned.
> + {name} can be two : separated repositories, for a cross repository diff.
> """
> def __init__(self, request):
> super(DiffMergeAPI, self).__init__()
> self.request = request
>
> - @repo_path
> - def get(self, repo_path):
> + @validate_path
> + def get(self, repo_store, repo_name):
> """Returns diff of two commits."""
> context_lines = int(self.request.params.get('context_lines', 3))
> try:
> patch = store.get_merge_diff(
> - repo_path, self.request.matchdict['base'],
> + repo_store, repo_name, self.request.matchdict['base'],
> self.request.matchdict['head'], context_lines)
> except (ValueError, GitError):
> # invalid pygit2 sha1's return ValueError: 1: Ambiguous lookup
> @@ -179,21 +182,21 @@
> super(CommitAPI, self).__init__()
> self.request = request
>
> - @repo_path
> - def get(self, repo_path):
> + @validate_path
> + def get(self, repo_store, repo_name):
> commit_sha1 = self.request.matchdict['sha1']
> try:
> - commit = store.get_commit(repo_path, commit_sha1)
> + commit = store.get_commit(repo_store, repo_name, commit_sha1)
> except GitError:
> return exc.HTTPNotFound()
> return commit
>
> - @repo_path
> - def collection_post(self, repo_path):
> + @validate_path
> + def collection_post(self, repo_store, repo_name):
> """Get commits in bulk."""
> commits = extract_json_data(self.request).get('commits')
> try:
> - commits = store.get_commits(repo_path, commits)
> + commits = store.get_commits(repo_store, repo_name, commits)
> except GitError:
> return exc.HTTPNotFound()
> return commits
> @@ -207,15 +210,15 @@
> super(LogAPI, self).__init__()
> self.request = request
>
> - @repo_path
> - def get(self, repo_path):
> + @validate_path
> + def get(self, repo_store, repo_name):
> """Get log by sha1, filtered by limit and stop."""
> sha1 = self.request.matchdict['sha1']
> limit = int(self.request.params.get('limit', -1))
> stop = self.request.params.get('stop')
>
> try:
> - log = store.get_log(repo_path, sha1, limit, stop)
> + log = store.get_log(repo_store, repo_name, sha1, limit, stop)
> except GitError:
> return exc.HTTPNotFound()
> return log
>
--
https://code.launchpad.net/~blr/turnip/api-init-with-alternates/+merge/256715
Your team Launchpad code reviewers is subscribed to branch lp:turnip.
References
