From c67f41a56e8225a09196eb3092200000535ef1ac Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 09 2018 07:15:47 +0000 Subject: Add a new API endpoint to retrieve the list of files changed in a PR This new API endpoint allows to retrieve the list of files changed as well as the number of lines added/removed and whether the file was added removed, modified or renamed (cf the status field). Fixes https://pagure.io/pagure/issue/3686 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index b54ac71..f565ea8 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -118,6 +118,7 @@ class APIERROR(enum.Enum): "by their author." ) ETRACKERREADONLY = "The issue tracker of this project is read-only" + ENOPRSTATS = "No statistics could be computed for this PR" def get_authorized_api_project(session, repo, user=None, namespace=None): diff --git a/pagure/api/fork.py b/pagure/api/fork.py index f8d97bc..9c2ab9e 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -1251,3 +1251,193 @@ def api_pull_request_create(repo, username=None, namespace=None): jsonout = flask.jsonify(request.to_json(public=True, api=True)) return jsonout + + +@API.route("//pull-request//diffstats") +@API.route("///pull-request//diffstats") +@API.route("/fork///pull-request//diffstats") +@API.route( + "/fork////pull-request//" + "diffstats" +) +@api_method +def api_pull_request_diffstats(repo, requestid, username=None, namespace=None): + """ + Pull-request diff statistics + ---------------------------- + Retrieve the statistics about the diff of a specific pull request. + + :: + + GET /api/0//pull-request//diffstats + GET /api/0///pull-request//diffstats + + :: + + GET /api/0/fork///pull-request//diffstats + GET /api/0/fork////pull-request//diffstats + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "README.rst": { + "lines_added": 1, + "lines_removed": 1, + "old_path": "README.rst", + "status": "M" + }, + "blame_file.txt": { + "lines_added": 0, + "lines_removed": 0, + "old_path": "blame_file", + "status": "R" + }, + "test": { + "lines_added": 0, + "lines_removed": 8, + "old_path": "test", + "status": "D" + }, + "test3": { + "lines_added": 3, + "lines_removed": 0, + "old_path": "test3", + "status": "A" + } + } + + + + """ # noqa + + repo = get_authorized_api_project( + flask.g.session, repo, user=username, namespace=namespace + ) + + if repo is None: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if not repo.settings.get("pull_requests", True): + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.EPULLREQUESTSDISABLED + ) + + request = pagure.lib.search_pull_requests( + flask.g.session, project_id=repo.id, requestid=requestid + ) + + if not request: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOREQ) + + if request.remote: + repopath = pagure.utils.get_remote_repo_path( + request.remote_git, request.branch_from + ) + parentpath = pagure.utils.get_repo_path(request.project) + else: + repo_from = request.project_from + parentpath = pagure.utils.get_repo_path(request.project) + repopath = parentpath + if repo_from: + repopath = pagure.utils.get_repo_path(repo_from) + + repo_obj = pygit2.Repository(repopath) + orig_repo = pygit2.Repository(parentpath) + + diff_commits = [] + diff = None + # Closed pull-request + if request.status != "Open": + commitid = request.commit_stop + try: + for commit in repo_obj.walk(commitid, pygit2.GIT_SORT_NONE): + diff_commits.append(commit) + if commit.oid.hex == request.commit_start: + break + except KeyError: + # This happens when repo.walk() cannot find commitid + pass + + if diff_commits: + # Ensure the first commit in the PR as a parent, otherwise + # point to it + start = diff_commits[-1].oid.hex + if diff_commits[-1].parents: + start = diff_commits[-1].parents[0].oid.hex + + # If the start and the end commits are the same, it means we are, + # dealing with one commit that has no parent, so just diff that + # one commit + if start == diff_commits[0].oid.hex: + diff = diff_commits[0].tree.diff_to_tree(swap=True) + else: + diff = repo_obj.diff( + repo_obj.revparse_single(start), + repo_obj.revparse_single(diff_commits[0].oid.hex), + ) + else: + try: + diff_commits, diff = pagure.lib.git.diff_pull_request( + flask.g.session, request, repo_obj, orig_repo + ) + except pagure.exceptions.PagureException as err: + flask.flash("%s" % err, "error") + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + flask.flash( + "Could not update this pull-request in the database", "error" + ) + + if diff: + diff.find_similar() + + output = {} + if diff: + for patch in diff: + linesadded = patch.line_stats[1] + linesremoved = patch.line_stats[2] + if hasattr(patch, "new_file_path"): + # Older pygit2 + status = patch.status + if patch.new_file_path != patch.old_file_path: + status = "R" + output[patch.new_file_path] = { + "status": patch.status, + "old_path": patch.old_file_path, + "lines_added": linesadded, + "lines_removed": linesremoved, + } + elif hasattr(patch, "delta"): + # Newer pygit2 + if ( + patch.delta.new_file.mode == 0 + and patch.delta.old_file.mode in [33188, 33261] + ): + status = "D" + elif ( + patch.delta.new_file.mode in [33188, 33261] + and patch.delta.old_file.mode == 0 + ): + status = "A" + elif patch.delta.new_file.mode in [ + 33188, + 33261, + ] and patch.delta.old_file.mode in [33188, 33261]: + status = "M" + if patch.delta.new_file.path != patch.delta.old_file.path: + status = "R" + output[patch.delta.new_file.path] = { + "status": status, + "old_path": patch.delta.old_file.path, + "lines_added": linesadded, + "lines_removed": linesremoved, + } + else: + raise pagure.exceptions.APIError(400, error_code=APIERROR.ENOPRSTATS) + + jsonout = flask.jsonify(output) + return jsonout diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index b8afdc3..45149dd 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -237,7 +237,7 @@ class PagureFlaskApitests(tests.SimplePagureTest): output = self.app.get('/api/0/-/error_codes') self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) - self.assertEqual(len(data), 32) + self.assertEqual(len(data), 33) self.assertEqual( sorted(data.keys()), [ @@ -248,10 +248,10 @@ class PagureFlaskApitests(tests.SimplePagureTest): u'EMODIFYPROJECTNOTALLOWED', u'ENEWPROJECTDISABLED', u'ENOCODE', u'ENOCOMMENT', u'ENOCOMMIT', u'ENOGROUP', u'ENOISSUE', u'ENOPRCLOSE', u'ENOPROJECT', u'ENOPROJECTS', - u'ENOREQ', u'ENOSIGNEDOFF', u'ENOTASSIGNED', u'ENOTASSIGNEE', - u'ENOTHIGHENOUGH', u'ENOTMAINADMIN', u'ENOUSER', u'EPRSCORE', - u'EPULLREQUESTSDISABLED', u'ETIMESTAMP', u'ETRACKERDISABLED', - u'ETRACKERREADONLY' + u'ENOPRSTATS', u'ENOREQ', u'ENOSIGNEDOFF', u'ENOTASSIGNED', + u'ENOTASSIGNEE', u'ENOTHIGHENOUGH', u'ENOTMAINADMIN', + u'ENOUSER', u'EPRSCORE', u'EPULLREQUESTSDISABLED', + u'ETIMESTAMP', u'ETRACKERDISABLED', u'ETRACKERREADONLY' ] ) diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index f17a9bd..a8f7c47 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -2703,6 +2703,176 @@ class PagureFlaskApiForktests(tests.Modeltests): } ) +class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests): + """ Tests for the flask API of pagure for the diff stats endpoint of PRs + """ + + maxDiff = None + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiForkPRDiffStatstests, self).setUp() + + pagure.config.config['REQUESTS_FOLDER'] = None + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + tests.create_projects_git(os.path.join(self.path, 'requests'), + bare=True) + tests.add_readme_git_repo(os.path.join(self.path, 'repos', 'test.git')) + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), ncommits=5) + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), branch='test') + + # Create the pull-request to close + repo = pagure.lib.get_authorized_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=repo, + branch_from='test', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_no_repo(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/invalid/pull-request/404/diffstats') + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {'error': 'Project not found', 'error_code': 'ENOPROJECT'} + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_no_pr(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/test/pull-request/404/diffstats') + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {'error': 'Pull-Request not found', 'error_code': 'ENOREQ'} + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_modified(self): + """ Test the api_pull_request_merge method of the flask api. """ + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + 'sources': { + 'lines_added': 10, + 'lines_removed': 0, + 'old_path': 'sources', + 'status': 'M' + } + } + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_added_mofidied(self): + """ Test the api_pull_request_merge method of the flask api. """ + tests.add_commit_git_repo( + os.path.join(self.path, 'repos', 'test.git'), ncommits=5) + tests.add_readme_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + readme_name='README.md', branch='test') + + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertTrue( + data in + [ + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "sources", + "status": "M" + } + }, + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 10, + "lines_removed": 0, + "old_path": "sources", + "status": "M" + } + } + ] + ) + + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_api_pull_request_diffstats_file_modified_deleted(self): + """ Test the api_pull_request_merge method of the flask api. """ + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + pagure.lib.tasks.update_pull_request(repo.requests[0].uid) + + tests.add_readme_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + readme_name='README.md', branch='test') + tests.remove_file_git_repo( + os.path.join(self.path, 'repos', 'test.git'), + filename='sources', branch='test') + + repo = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(repo.requests), 1) + pagure.lib.tasks.update_pull_request(repo.requests[0].uid) + + output = self.app.get('/api/0/test/pull-request/1/diffstats') + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + "README.md": { + "lines_added": 5, + "lines_removed": 0, + "old_path": "README.md", + "status": "A" + }, + "sources": { + "lines_added": 0, + "lines_removed": 5, + "old_path": "sources", + "status": "D" + } + } + ) + if __name__ == '__main__': unittest.main(verbosity=2)