From dcfffe8b0706a57ac58e03341fd360f4cc38ce1b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 12:16:57 +0000 Subject: [PATCH 1/8] Add a private internal API endpoint returning in which branch(es) a commit is --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 66639e7..18d1e32 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -403,3 +403,88 @@ def get_ticket_template(repo, username=None): }) response.status_code = 404 return response + + +@PV.route('/branches/commit/', methods=['POST']) +@localonly +def get_branches_of_commit(): + """ Return the list of branches that have the specified commit in + """ + form = pagure.forms.ConfirmationForm() + if not form.validate_on_submit(): + response = flask.jsonify({ + 'code': 'ERROR', + 'message': 'Invalid input submitted', + }) + response.status_code = 400 + return response + + commit_id = flask.request.form.get('commit_id', '').strip() or None + if not commit_id: + response = flask.jsonify({ + 'code': 'ERROR', + 'message': 'No commit id submitted', + }) + response.status_code = 400 + return response + + repo = pagure.lib.get_project( + pagure.SESSION, + flask.request.form.get('repo', '').strip() or None, + user=flask.request.form.get('repouser', '').strip() or None) + + if not repo: + response = flask.jsonify({ + 'code': 'ERROR', + 'message': 'No repo found with the information provided', + }) + response.status_code = 404 + return response + + reponame = pagure.get_repo_path(repo) + repo_obj = pygit2.Repository(reponame) + + branches = [] + if not repo_obj.head_is_unborn: + compare_branch = repo_obj.lookup_branch( + repo_obj.head.shorthand) + else: + compare_branch = None + + for branchname in repo_obj.listall_branches(): + branch = repo_obj.lookup_branch(branchname) + + diff_commits = [] + + if not repo_obj.is_empty and repo_obj.listall_branches() > 1: + + + merge_commit = None + + if compare_branch: + merge_commit = repo_obj.merge_base( + compare_branch.get_object().hex, + branch.get_object().hex + ).hex + + repo_commit = repo_obj[branch.get_object().hex] + + for commit in repo_obj.walk( + repo_commit.oid.hex, pygit2.GIT_SORT_TIME): + if commit.oid.hex == merge_commit: + break + if commit.oid.hex == commit_id: + branches.append(branchname) + break + + # If we didn't find the commit in any branch and there is one, then it + # is in the default branch. + if not branches and compare_branch: + branches.append(compare_branch.branch_name) + + return flask.jsonify( + { + 'code': 'OK', + 'message': branches, + } + ) From d7d58309ef7e4a4ff164509a03494f2e8731ac90 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 12:19:26 +0000 Subject: [PATCH 2/8] Add on the commit detail page the branch(es) the commit is in --- diff --git a/pagure/templates/commit.html b/pagure/templates/commit.html index 9bb7d42..126a236 100644 --- a/pagure/templates/commit.html +++ b/pagure/templates/commit.html @@ -153,5 +153,32 @@ $('#diff_list').toggle(); }); }); + $.ajax({ + url: '{{ url_for("internal_ns.get_branches_of_commit") }}' , + type: 'POST', + data: { + repo: "{{ repo.name }}", + repouser: "{{ repo.user.user if repo.is_fork else '' }}", + commit_id: "{{ commitid }}", + csrf_token: "{{ form.csrf_token.current_token }}", + }, + dataType: 'json', + success: function(res) { + if (res.message.length == 0){ + return; + } + var _br = ''; + for (var i = 0; i < res.message.length; ++i) { + if (_br.length > 0){ + _br += ', '; + } + _br += res.message[i] + } + var el = $('#diff-file-1'); + el.before( + '
' + + _br + '
'); + } + }); {% endblock %} diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index eadb3b4..97a6b5d 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -728,6 +728,7 @@ def view_commit(repo, commitid, username=None): commit=commit, diff=diff, watch=watch, + form=pagure.forms.ConfirmationForm(), ) From 014417cf9ba961aabea594e64ef32455267f8336 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 12:22:14 +0000 Subject: [PATCH 3/8] Let's ensure the element we want to use exists and bail otherwise --- diff --git a/pagure/templates/commit.html b/pagure/templates/commit.html index 126a236..f68a19e 100644 --- a/pagure/templates/commit.html +++ b/pagure/templates/commit.html @@ -175,6 +175,9 @@ _br += res.message[i] } var el = $('#diff-file-1'); + if (!el){ + return; + } el.before( '
' + _br + '
'); From d7bd31c6e26f17017a8e9a1c9cf5ff3296fa07e6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 15:08:56 +0000 Subject: [PATCH 4/8] Add a check that the git repo actually exists and return a 404 otherwise --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 18d1e32..d0d1ee9 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -441,7 +441,16 @@ def get_branches_of_commit(): response.status_code = 404 return response - reponame = pagure.get_repo_path(repo) + reponame = os.path.join(pagure.APP.config['GIT_FOLDER'], repo.path) + + if not os.path.exists(reponame): + response = flask.jsonify({ + 'code': 'ERROR', + 'message': 'No git repo found with the information provided', + }) + response.status_code = 404 + return response + repo_obj = pygit2.Repository(reponame) branches = [] From 0dfccc26d68f8ccddc9dc3a6d0948fee57213868 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 15:09:50 +0000 Subject: [PATCH 5/8] Raise an error if the commit cannot be found in the repo --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index d0d1ee9..57c6d09 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -453,6 +453,16 @@ def get_branches_of_commit(): repo_obj = pygit2.Repository(reponame) + try: + commit_id in repo_obj + except: + response = flask.jsonify({ + 'code': 'ERROR', + 'message': 'This commit could not be found in this repo', + }) + response.status_code = 404 + return response + branches = [] if not repo_obj.head_is_unborn: compare_branch = repo_obj.lookup_branch( From f6100f2f6846f06437248986fb93373980bad330 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 15:10:36 +0000 Subject: [PATCH 6/8] Rename messages to branches to reflect what is actually in it --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 57c6d09..7146628 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -504,6 +504,6 @@ def get_branches_of_commit(): return flask.jsonify( { 'code': 'OK', - 'message': branches, + 'branches': branches, } ) diff --git a/pagure/templates/commit.html b/pagure/templates/commit.html index f68a19e..63224de 100644 --- a/pagure/templates/commit.html +++ b/pagure/templates/commit.html @@ -164,15 +164,15 @@ }, dataType: 'json', success: function(res) { - if (res.message.length == 0){ + if (res.branches.length == 0){ return; } var _br = ''; - for (var i = 0; i < res.message.length; ++i) { + for (var i = 0; i < res.branches.length; ++i) { if (_br.length > 0){ _br += ', '; } - _br += res.message[i] + _br += res.branches[i] } var el = $('#diff-file-1'); if (!el){ From 9f395f6d748fbaa42c36e6c64e501e2afa7144db Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 23 2016 15:10:58 +0000 Subject: [PATCH 7/8] Add unit-tests for the get_branches_of_commit endpoint --- diff --git a/tests/test_pagure_flask_internal.py b/tests/test_pagure_flask_internal.py index d812b13..effabc7 100644 --- a/tests/test_pagure_flask_internal.py +++ b/tests/test_pagure_flask_internal.py @@ -812,6 +812,190 @@ class PagureFlaskInternaltests(tests.Modeltests): js_data = json.loads(output.data) self.assertDictEqual(js_data, exp) + def test_get_branches_of_commit(self): + ''' Test the get_branches_of_commit from the internal API. ''' + tests.create_projects(self.session) + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(pagure.APP, user): + output = self.app.get('/test/adduser') + csrf_token = output.data.split( + b'name="csrf_token" type="hidden" value="')[1].split(b'">')[0] + + # No CSRF token + data = { + 'repo': 'fakerepo', + 'commit_id': 'foo', + } + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 400) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + {u'code': u'ERROR', u'message': u'Invalid input submitted'} + ) + + # Invalid repo + data = { + 'repo': 'fakerepo', + 'commit_id': 'foo', + 'csrf_token': csrf_token, + } + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 404) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + { + u'code': u'ERROR', + u'message': u'No repo found with the information provided' + } + ) + + # Rigth repo, no commit + data = { + 'repo': 'test', + 'csrf_token': csrf_token, + } + + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 400) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + {u'code': u'ERROR', u'message': u'No commit id submitted'} + ) + + # Request is fine, but git repo doesn't exist + data = { + 'repo': 'test', + 'commit_id': 'foo', + 'csrf_token': csrf_token, + } + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 404) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + { + u'code': u'ERROR', + u'message': u'No git repo found with the information provided' + } + ) + + # Create a git repo to play with + gitrepo = os.path.join(tests.HERE, 'test.git') + self.assertFalse(os.path.exists(gitrepo)) + os.makedirs(gitrepo) + repo = pygit2.init_repository(gitrepo) + + # Create a file in that git repo + with open(os.path.join(gitrepo, 'sources'), 'w') as stream: + stream.write('foo\n bar') + repo.index.add('sources') + repo.index.write() + + # Commits the files added + tree = repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + repo.create_commit( + 'refs/heads/master', # the name of the reference to update + author, + committer, + 'Add sources file for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [] + ) + + first_commit = repo.revparse_single('HEAD') + + # Edit the sources file again + with open(os.path.join(gitrepo, 'sources'), 'w') as stream: + stream.write('foo\n bar\nbaz\n boose') + repo.index.add('sources') + repo.index.write() + + # Commits the files added + tree = repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + repo.create_commit( + 'refs/heads/feature', # the name of the reference to update + author, + committer, + 'Add baz and boose to the sources\n\n There are more objects to ' + 'consider', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [first_commit.oid.hex] + ) + + # Create another file in the master branch + with open(os.path.join(gitrepo, '.gitignore'), 'w') as stream: + stream.write('*~') + repo.index.add('.gitignore') + repo.index.write() + + # Commits the files added + tree = repo.index.write_tree() + author = pygit2.Signature( + 'Alice Author', 'alice@authors.tld') + committer = pygit2.Signature( + 'Cecil Committer', 'cecil@committers.tld') + commit_hash = repo.create_commit( + 'refs/heads/feature_branch', # the name of the reference to update + author, + committer, + 'Add .gitignore file for testing', + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [first_commit.oid.hex] + ) + + # All good but the commit id + data = { + 'repo': 'test', + 'commit_id': 'foo', + 'csrf_token': csrf_token, + } + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 404) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + { + u'code': u'ERROR', + u'message': 'This commit could not be found in this repo' + } + ) + + # All good + data = { + 'repo': 'test', + 'commit_id': commit_hash, + 'csrf_token': csrf_token, + } + output = self.app.post('/pv/branches/commit/', data=data) + self.assertEqual(output.status_code, 200) + js_data = json.loads(output.data.decode('utf-8')) + self.assertDictEqual( + js_data, + { + u'code': u'OK', + u'branches': ['feature_branch'], + } + ) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( From c03453ea486b3811439a319ecbf87d0b342bd12e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 25 2016 12:02:05 +0000 Subject: [PATCH 8/8] Fix typos and code style pointed out by @vivekanand1101 --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 7146628..dfb5b54 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -441,9 +441,9 @@ def get_branches_of_commit(): response.status_code = 404 return response - reponame = os.path.join(pagure.APP.config['GIT_FOLDER'], repo.path) + repopath = os.path.join(pagure.APP.config['GIT_FOLDER'], repo.path) - if not os.path.exists(reponame): + if not os.path.exists(repopath): response = flask.jsonify({ 'code': 'ERROR', 'message': 'No git repo found with the information provided', @@ -451,7 +451,7 @@ def get_branches_of_commit(): response.status_code = 404 return response - repo_obj = pygit2.Repository(reponame) + repo_obj = pygit2.Repository(repopath) try: commit_id in repo_obj @@ -473,11 +473,8 @@ def get_branches_of_commit(): for branchname in repo_obj.listall_branches(): branch = repo_obj.lookup_branch(branchname) - diff_commits = [] - if not repo_obj.is_empty and repo_obj.listall_branches() > 1: - merge_commit = None if compare_branch: