launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> === modified file 'turnip/api/store.py'
> --- turnip/api/store.py	2015-03-11 01:52:35 +0000
> +++ turnip/api/store.py	2015-03-13 02:52:51 +0000
> @@ -1,9 +1,11 @@
>  # Copyright 2015 Canonical Ltd.  All rights reserved.
>  
> +import itertools
>  import os
>  import shutil
>  
>  from pygit2 import (
> +    GitError,
>      GIT_OBJ_BLOB,
>      GIT_OBJ_COMMIT,
>      GIT_OBJ_TREE,
> @@ -13,7 +15,6 @@
>      )
>  
>  
> -
>  REF_TYPE_NAME = {
>      GIT_OBJ_COMMIT: 'commit',
>      GIT_OBJ_TREE: 'tree',
> @@ -23,6 +24,7 @@
>  
>  
>  def format_ref(ref, git_object):
> +    """Return a formatted object dict from a ref."""
>      return {
>          ref: {
>              "object": {
> @@ -33,6 +35,31 @@
>          }
>  
>  
> +def format_commit(git_object):
> +    """Return a formatted commit object dict."""
> +    if git_object.type != GIT_OBJ_COMMIT:
> +        raise GitError('Invalid type: object {} is not a commit.'.format(
> +            git_object.oid.hex))
> +    parents = [parent.hex for parent in git_object.parent_ids]
> +    return {
> +        'sha1': git_object.oid.hex,
> +        'message': git_object.message,
> +        'author': format_signature(git_object.author),
> +        'committer': format_signature(git_object.committer),
> +        'parents': parents,
> +        'tree': git_object.tree.hex
> +        }
> +
> +
> +def format_signature(signature):
> +    """Return a formatted signature dict."""
> +    return {
> +        'name': signature.name,
> +        'email': signature.email,
> +        'time': signature.time
> +        }
> +
> +
>  def init_repo(repo, is_bare=True):
>      """Initialise a git repository."""
>      if os.path.exists(repo):
> @@ -75,3 +102,38 @@
>      git_object = repo.lookup_reference(ref).peel()
>      ref_obj = format_ref(ref, git_object)
>      return ref_obj
> +
> +
> +def get_log(repo_path, 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:
> +        walker = itertools.islice(walker, int(limit))

limit should already be an int.

> +    commits = [format_commit(commit) for commit in walker]
> +    return commits
> +
> +
> +def get_commit(repo_path, 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)
> +    commit = format_commit(git_object)
> +    return commit
> +
> +
> +def get_commits(repo_path, commit_oids):
> +    """Return a collection of commit objects from a list of oids."""
> +    repo = open_repo(repo_path)
> +    commits = [get_commit(repo_path, commit, repo) for commit in commit_oids]
> +    return commits
> 
> === modified file 'turnip/api/tests/test_api.py'
> --- turnip/api/tests/test_api.py	2015-03-11 23:15:14 +0000
> +++ turnip/api/tests/test_api.py	2015-03-13 02:52:51 +0000
> @@ -125,6 +125,93 @@
>          resp = self.get_ref(tag)
>          self.assertTrue(tag in resp)
>  
> +    def test_repo_get_commit(self):
> +        factory = RepoFactory(self.repo_store)
> +        message = 'Computers make me angry.'
> +        commit_oid = factory.add_commit(message, 'foobar.txt')
> +
> +        resp = self.app.get('/repo/{}/commits/{}'.format(
> +            self.repo_path, commit_oid.hex))
> +        commit_resp = resp.json
> +        self.assertEqual(commit_oid.hex, commit_resp['sha1'])
> +        self.assertEqual(message, commit_resp['message'])
> +
> +    def test_repo_get_commit_collection(self):
> +        """Ensure commits can be returned in bulk."""
> +        factory = RepoFactory(self.repo_store, num_commits=10)
> +        factory.build()
> +        bulk_commits = {'commits': [c.hex for c in factory.commits[0::2]]}
> +
> +        resp = self.app.post_json('/repo/{}/commits'.format(
> +            self.repo_path), bulk_commits)
> +        self.assertEqual(len(resp.json), 5)
> +        self.assertEqual(bulk_commits['commits'][0], resp.json[0]['sha1'])
> +
> +    def test_repo_get_log_signatures(self):
> +        """Ensure signatures are correct."""
> +        factory = RepoFactory(self.repo_store)
> +        committer = factory.makeSignature(u'村上 春樹'.encode('utf-8'),
> +                                          u'tsukuru@猫の町.co.jp'.encode('utf-8'),
> +                                          encoding='utf-8')
> +        author = factory.makeSignature(
> +            u'Владимир Владимирович Набоков'.encode('utf-8'),
> +            u'Набоко@zembla.ru'.encode('utf-8'), encoding='utf-8')
> +        oid = factory.add_commit('Obfuscate colophon.', 'path.foo',
> +                                 author=author, committer=committer)
> +        resp = self.app.get('/repo/{}/log/{}'.format(self.repo_path, oid))
> +        self.assertEqual(resp.json[0]['author']['name'], author.name)
> +
> +    def test_repo_get_log(self):
> +        factory = RepoFactory(self.repo_store, num_commits=4)
> +        factory.build()
> +        commits_from = factory.commits[2].hex
> +        resp = self.app.get('/repo/{}/log/{}'.format(
> +            self.repo_path, commits_from))
> +        self.assertEqual(len(resp.json), 3)
> +
> +    def test_repo_get_unicode_log(self):
> +        factory = RepoFactory(self.repo_store)
> +        message = u'나는 김치 사랑'.encode('utf-8')
> +        message2 = u'(╯°□°)╯︵ ┻━┻'.encode('utf-8')
> +        oid = factory.add_commit(message, '자장면/짜장면.py')
> +        oid2 = factory.add_commit(message2, '엄마야!.js', [oid])
> +
> +        resp = self.app.get('/repo/{}/log/{}'.format(self.repo_path, oid2))
> +        self.assertEqual(resp.json[0]['message'],
> +                         message2.decode('utf-8', 'replace'))
> +        self.assertEqual(resp.json[1]['message'],
> +                         message.decode('utf-8', 'replace'))
> +
> +    def test_repo_get_non_unicode_log(self):
> +        """Ensure that non-unicode data is discarded."""
> +        factory = RepoFactory(self.repo_store)
> +        message = '\xe9\xe9\xe9'  # latin-1
> +        oid = factory.add_commit(message, 'foo.py')
> +        resp = self.app.get('/repo/{}/log/{}'.format(self.repo_path, oid))
> +        self.assertEqual(resp.json[0]['message'],
> +                         message.decode('utf-8', 'replace'))
> +
> +    def test_repo_get_log_with_limit(self):
> +        """Ensure the commit log can filtered by limit."""
> +        factory = RepoFactory(self.repo_store, num_commits=10)
> +        repo = factory.build()
> +        head = repo.head.target
> +        resp = self.app.get('/repo/{}/log/{}?limit=5'.format(
> +            self.repo_path, head))
> +        self.assertEqual(len(resp.json), 5)
> +
> +    def test_repo_get_log_with_stop(self):
> +        """Ensure the commit log can be filtered by a stop commit."""
> +        factory = RepoFactory(self.repo_store, num_commits=10)
> +        repo = factory.build()
> +        stop_commit = factory.commits[4]
> +        excluded_commit = factory.commits[5]
> +        head = repo.head.target
> +        resp = self.app.get('/repo/{}/log/{}?stop={}'.format(
> +            self.repo_path, head, stop_commit))
> +        self.assertEqual(len(resp.json), 5)
> +        self.assertNotIn(resp.json, excluded_commit)
> +
>  
>  if __name__ == '__main__':
>      unittest.main()
> 
> === modified file 'turnip/api/tests/test_helpers.py'
> --- turnip/api/tests/test_helpers.py	2015-03-10 03:22:33 +0000
> +++ turnip/api/tests/test_helpers.py	2015-03-13 02:52:51 +0000
> @@ -18,6 +18,7 @@
>      def __init__(self, repo_path=None, num_commits=None, num_tags=None):
>          self.author = Signature('Test Author', 'author@xxxxxxx')
>          self.committer = Signature('Test Commiter', 'committer@xxxxxxx')
> +        self.commits = []
>          self.num_commits = num_commits
>          self.num_tags = num_tags
>          self.repo_path = repo_path
> @@ -30,18 +31,19 @@
>          return list(self.repo.walk(last.id, GIT_SORT_TIME))
>  
>      def add_commit(self, blob_content, file_path, parents=[],
> -                   ref='refs/heads/master'):
> +                   ref='refs/heads/master', author=None, committer=None):
>          """Create a commit from blob_content and file_path."""
>          repo = self.repo
> -
> +        if not author:
> +            author = self.author
> +        if not committer:
> +            committer = self.committer
>          blob_oid = repo.create_blob(blob_content)
>          blob_entry = IndexEntry(file_path, blob_oid, GIT_FILEMODE_BLOB)
>          repo.index.add(blob_entry)
>          tree_id = repo.index.write_tree()
> -        oid = repo.create_commit(ref,
> -                                 self.author,
> -                                 self.committer,
> -                                 'commit', tree_id, parents)
> +        oid = repo.create_commit(ref, author, committer,
> +                                 blob_content, tree_id, parents)
>          return oid
>  
>      def add_tag(self, tag_name, tag_message, oid):
> @@ -50,6 +52,10 @@
>          repo.create_tag(tag_name, oid, GIT_OBJ_COMMIT,
>                          self.committer, tag_message)
>  
> +    def makeSignature(self, name, email, encoding='utf-8'):
> +        """Return an author or commiter."""

"committer"

> +        return Signature(name, email, encoding=encoding)
> +
>      def stage(self, file_path):
>          """Stage a file and return a tree id."""
>          repo = self.repo
> @@ -69,6 +75,7 @@
>              self.stage(test_file)
>  
>              commit_oid = self.add_commit(blob_content, test_file, parents)
> +            self.commits.append(commit_oid)
>              parents = [commit_oid]
>  
>      def generate_tags(self, num_tags):
> 
> === modified file 'turnip/api/views.py'
> --- turnip/api/views.py	2015-03-11 23:15:14 +0000
> +++ turnip/api/views.py	2015-03-13 02:52:51 +0000
> @@ -27,14 +27,19 @@
>      return repo_path_decorator
>  
>  
> +class BaseAPI(object):
> +    def __init__(self):
> +        config = TurnipConfig()
> +        self.repo_store = config.get('repo_store')
> +
> +
>  @resource(collection_path='/repo', path='/repo/{name}')
> -class RepoAPI(object):
> +class RepoAPI(BaseAPI):
>      """Provides HTTP API for repository actions."""
>  
>      def __init__(self, request):
> -        config = TurnipConfig()
> +        super(RepoAPI, self).__init__()
>          self.request = request
> -        self.repo_store = config.get('repo_store')
>  
>      def collection_post(self):
>          """Initialise a new git repository."""
> @@ -60,13 +65,12 @@
>  
>  @resource(collection_path='/repo/{name}/refs',
>            path='/repo/{name}/refs/{ref:.*}')
> -class RefAPI(object):
> +class RefAPI(BaseAPI):
>      """Provides HTTP API for git references."""
>  
>      def __init__(self, request):
> -        config = TurnipConfig()
> +        super(RefAPI, self).__init__()
>          self.request = request
> -        self.repo_store = config.get('repo_store')
>  
>      @repo_path
>      def collection_get(self, repo_path):
> @@ -82,3 +86,54 @@
>              return store.get_ref(repo_path, ref)
>          except (KeyError, GitError):
>              return exc.HTTPNotFound()
> +
> +
> +@resource(collection_path='/repo/{name}/commits',
> +          path='/repo/{name}/commits/{sha1}')
> +class CommitAPI(BaseAPI):
> +    """Provides HTTP API for git commits."""
> +
> +    def __init__(self, request):
> +        super(CommitAPI, self).__init__()
> +        self.request = request
> +
> +    @repo_path
> +    def get(self, repo_path):
> +        commit_sha1 = self.request.matchdict['sha1']
> +        try:
> +            commit = store.get_commit(repo_path, commit_sha1)
> +        except GitError:
> +            return exc.HTTPNotFound()
> +        return commit
> +
> +    @repo_path
> +    def collection_post(self, repo_path):
> +        """Get commits in bulk."""
> +        commits = extract_json_data(self.request).get('commits')
> +        try:
> +            commits = store.get_commits(repo_path, commits)
> +        except GitError:
> +            return exc.HTTPNotFound()
> +        return commits
> +
> +
> +@resource(path='/repo/{name}/log/{sha1}')
> +class LogAPI(BaseAPI):
> +    """Provides HTTP API for git logs."""
> +
> +    def __init__(self, request):
> +        super(LogAPI, self).__init__()
> +        self.request = request
> +
> +    @repo_path
> +    def get(self, repo_path):
> +        """Get log by sha1, filtered by limit and stop."""
> +        sha1 = self.request.matchdict['sha1']
> +        limit = self.request.params.get('limit')
> +        stop = self.request.params.get('stop')
> +
> +        try:
> +            log = store.get_log(repo_path, sha1, limit, stop)
> +        except GitError:
> +            return exc.HTTPNotFound()
> +        return log
> 


-- 
https://code.launchpad.net/~blr/turnip/commit-api/+merge/252696
Your team Launchpad code reviewers is subscribed to branch lp:turnip.