diff --git a/pagure/lib/link.py b/pagure/lib/link.py index e86df26..dc89116 100644 --- a/pagure/lib/link.py +++ b/pagure/lib/link.py @@ -15,37 +15,38 @@ from __future__ import unicode_literals import re import pagure.lib.query import pagure.exceptions +import pagure.utils +from pagure.config import config as pagure_config FIXES = [ - re.compile(r"(?:.*\s+)?fixe?[sd]?:?\s*?#(\d+)", re.I), - re.compile( - r"(?:.*\s+)?fixe?[sd]?:?\s*?https?://.*/([a-zA-z0-9_][a-zA-Z0-9-_]*)" - r"/(?:issue|pull-request)/(\d+)", - re.I, - ), - re.compile(r"(?:.*\s+)?merge?[sd]?:?\s*?#(\d+)", re.I), - re.compile( - r"(?:.*\s+)?merge?[sd]?:?\s*?https?://.*/([a-zA-z0-9_][a-zA-Z0-9-_]*)" - r"/(?:issue|pull-request)/(\d+)", - re.I, - ), - re.compile(r"(?:.*\s+)?close?[sd]?:?\s*?#(\d+)", re.I), + re.compile(r"(?:.*\s+)?{0}?[sd]?:?\s*?#(\d+)".format(kw), re.I) + for kw in ["fixe", "merge", "close"] +] +FIXES += [ re.compile( - r"(?:.*\s+)?close?[sd]?:?\s*?https?://.*/([a-zA-z0-9_][a-zA-Z0-9-_]*)" - r"/(?:issue|pull-request)/(\d+)", + r"(?:.*\s+)?{0}?[sd]?:?\s*?{1}" + r"(/.*?/(?:issue|pull-request)/\d+)".format( + kw, pagure_config["APP_URL"].rstrip("/") + ), re.I, - ), + ) + for kw in ["fixe", "merge", "close"] ] + RELATES = [ - re.compile(r"(?:.*\s+)?relate[sd]?:?\s*?(?:to)?\s*?#(\d+)", re.I), - re.compile(r"(?:.*\s+)?relate[sd]?:?\s?#(\d+)", re.I), + re.compile(r"(?:.*\s+)?{0}?[sd]?:?\s*?(?:to)?\s*?#(\d+)".format(kw), re.I) + for kw in ["relate"] +] +RELATES += [ re.compile( - r"(?:.*\s+)?relate[sd]?:?\s*?(?:to)?\s*?" - r"https?://.*/([a-zA-z0-9_][a-zA-Z0-9-_]*)/issue/(\d+)", + r"(?:.*\s+)?{0}?[sd]?:?\s*?(?:to)?\s*?{1}(/.*?/issue/\d+)".format( + kw, pagure_config["APP_URL"].rstrip("/") + ), re.I, - ), + ) + for kw in ["relate"] ] @@ -87,12 +88,22 @@ def get_relation( for motif in regex: relid = None project = None - if motif.match(text): - if len(motif.match(text).groups()) >= 2: - relid = motif.match(text).group(2) - project = motif.match(text).group(1) - else: - relid = motif.match(text).group(1) + got_match = motif.match(text) + if got_match: + relid = got_match.group(1) + if not relid.isdigit(): + ( + username, + namespace, + reponame, + objtype, + relid, + ) = pagure.utils.parse_path(relid) + repo = pagure.lib.query.get_authorized_project( + session, reponame, user=username, namespace=namespace + ) + if not repo: + continue if relid: relation = pagure.lib.query.search_issues( diff --git a/tests/test_pagure_hooks_pagure_hook.py b/tests/test_pagure_hooks_pagure_hook.py new file mode 100644 index 0000000..aa768ef --- /dev/null +++ b/tests/test_pagure_hooks_pagure_hook.py @@ -0,0 +1,174 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2018 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +from __future__ import unicode_literals + +import unittest +import sys +import os + +import mock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.hooks.pagure_hook +import tests + + +class PagureHooksPagureHooktests(tests.SimplePagureTest): + """ Tests for pagure.hooks.pagure_hook """ + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureHooksPagureHooktests, self).setUp() + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + + # Add one issue to each projects otherwise we won't be able to find + project = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=project, + title='Test issue', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + project = pagure.lib.query.get_authorized_project( + self.session, 'test2') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=project, + title='Test issue on test2', + content='We should work on this, really', + user='foo', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue on test2') + + # Create a fork of test for foo with its own ticket + item = pagure.lib.model.Project( + user_id=2, # foo + name='test', + is_fork=True, + parent_id=1, + description='test project #1', + hook_token='aaabbbccc_foo', + ) + item.close_status = [ + 'Invalid', 'Insufficient data', 'Fixed', 'Duplicate'] + self.session.add(item) + self.session.commit() + project = pagure.lib.query.get_authorized_project( + self.session, 'test', user="foo") + msg = pagure.lib.query.new_issue( + session=self.session, + repo=project, + title='Test issue on fork/foo/test', + content='We should work on this, really', + user='foo', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue on fork/foo/test') + + self.folder = os.path.join(self.path, 'repos', 'test.git') + + # Add a README to the git repo - First commit + tests.add_readme_git_repo(self.folder) + + @mock.patch("pagure.hooks.pagure_hook.fixes_relation") + def test_generate_revision_change_log_short_url(self, fixes_relation): + """ Test generate_revision_change_log when the comment contains + a short link to the same project. + """ + + # Add a commit with an url in the commit message + tests.add_content_to_git( + self.folder, branch='master', filename='sources', content='foo', + message='Test commit message\n\nFixes #1' + ) + + project = pagure.lib.query.get_authorized_project(self.session, 'test') + + pagure.hooks.pagure_hook.generate_revision_change_log( + session=self.session, + project=project, + username=None, + repodir=self.folder, + new_commits_list=['HEAD'] + ) + fixes_relation.assert_called_once_with( + mock.ANY, None, mock.ANY, project.issues[0], + 'http://localhost.localdomain/') + + @mock.patch("pagure.hooks.pagure_hook.fixes_relation") + def test_generate_revision_change_log_full_url(self, fixes_relation): + """ Test generate_revision_change_log when the comment contains + a full link to another project. + """ + + # Add a commit with an url in the commit message + tests.add_content_to_git( + self.folder, branch='master', filename='sources', content='foo', + message='Test commit message\n\n' + 'Fixes http://localhost.localdomain/test2/issue/1' + ) + + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + project2 = pagure.lib.query.get_authorized_project( + self.session, 'test2') + + pagure.hooks.pagure_hook.generate_revision_change_log( + session=self.session, + project=project, + username=None, + repodir=self.folder, + new_commits_list=['HEAD'] + ) + fixes_relation.assert_called_once_with( + mock.ANY, None, mock.ANY, project2.issues[0], + 'http://localhost.localdomain/') + + @mock.patch("pagure.hooks.pagure_hook.fixes_relation") + def test_generate_revision_change_log_full_url_fork(self, fixes_relation): + """ Test generate_revision_change_log when the comment contains + a full link to a fork. + """ + + # Add a commit with an url in the commit message + tests.add_content_to_git( + self.folder, branch='master', filename='sources', content='foo', + message='Test commit message\n\n' + 'Fixes http://localhost.localdomain/fork/foo/test/issue/1' + ) + + project = pagure.lib.query.get_authorized_project( + self.session, 'test') + project_fork = pagure.lib.query.get_authorized_project( + self.session, 'test', user="foo") + + pagure.hooks.pagure_hook.generate_revision_change_log( + session=self.session, + project=project, + username=None, + repodir=self.folder, + new_commits_list=['HEAD'] + ) + fixes_relation.assert_called_once_with( + mock.ANY, None, mock.ANY, project_fork.issues[0], + 'http://localhost.localdomain/') + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tests/test_pagure_lib_link.py b/tests/test_pagure_lib_link.py index d5e7b35..cb7df6e 100644 --- a/tests/test_pagure_lib_link.py +++ b/tests/test_pagure_lib_link.py @@ -33,9 +33,9 @@ COMMENTS = [ 'Did you see #1?', 'This is a duplicate of #2', 'This is a fixes #3', - 'Might be worth looking at https://fedorahosted.org/pagure/tests2/issue/4', + 'Might be worth looking at http://localhost.localdomain/pagure/tests2/issue/4', 'This relates to #5', - 'Could this be related to https://fedorahosted.org/pagure/tests2/issue/6', + 'Could this be related to http://localhost.localdomain/test/issue/6', ] @@ -186,21 +186,23 @@ class PagureLibLinktests(tests.Modeltests): self.assertEqual( str(link), '[Issue(3, project:test, user:pingou, title:issue 3)]') + self.assertEqual(len(link), 1) + self.assertEqual(link[0].project.fullname, 'test') else: self.assertEqual(link, []) def test_relates_regex(self): ''' Test the relates regex present in pagure.lib.link. ''' - text = 'relates to http://localhost/fork/pingou/test/issue/1' + text = 'relates to http://localhost.localdomain/fork/pingou/test/issue/1' for index, regex in enumerate(pagure.lib.link.RELATES): - if index == 2: + if index == 1: self.assertNotEqual(regex.match(text), None) else: self.assertEqual(regex.match(text), None) - text = 'relates http://209.132.184.222/fork/pingou/test/issue/1' + text = 'relates http://localhost.localdomain/fork/pingou/test/issue/1' for index, regex in enumerate(pagure.lib.link.RELATES): - if index == 2: + if index == 1: self.assertNotEqual(regex.match(text), None) else: self.assertEqual(regex.match(text), None) @@ -213,16 +215,16 @@ class PagureLibLinktests(tests.Modeltests): self.assertEqual(regex.match(text), None) text = 'Could this be related to '\ - ' https://fedorahosted.org/pagure/tests2/issue/6' + ' http://localhost.localdomain/pagure/tests2/issue/6' for index, regex in enumerate(pagure.lib.link.RELATES): - if index == 2: + if index == 1: self.assertNotEqual(regex.match(text), None) else: self.assertEqual(regex.match(text), None) - text = 'relates https://localhost/SSSD/ding-libs/issue/31' + text = 'relates http://localhost.localdomain/SSSD/ding-libs/issue/31' for index, regex in enumerate(pagure.lib.link.RELATES): - if index == 2: + if index == 1: self.assertNotEqual(regex.match(text), None) else: self.assertEqual(regex.match(text), None) @@ -238,34 +240,44 @@ class PagureLibLinktests(tests.Modeltests): if match: break self.assertNotEqual(match, None) - self.assertEqual(len(match.groups()), 2) + self.assertEqual(len(match.groups()), 1) self.assertEqual(match.groups(), groups) data = [ # [string, groups] ] - project_match('fixes http://localhost/fork/pingou/test/issue/1', - ('test', '1')) - project_match('fix http://209.132.184.222/fork/pingou/test/issue/1', - ('test', '1')) - project_match('Could this be fixes ' - ' https://fedorahosted.org/pagure/tests2/issue/6', - ('tests2', '6')) - project_match('merged https://pagure.io/myproject/pull-request/70', - ('myproject', '70')) - project_match('Now we merge https://pagure.io/myproject/pull-request/99', - ('myproject', '99')) - project_match('Merges http://localhost/fork/pingou/test/issue/1', - ('test', '1')) - project_match('Merges: http://localhost/fork/pingou/test/issue/12', - ('test', '12')) - project_match('Merged http://localhost/fork/pingou/test/issue/123#', - ('test', '123')) - project_match('Merge: http://localhost/fork/pingou/test/issue/1234#foo', - ('test', '1234')) - project_match('Merges: https://localhost/SSSD/ding-libs/pull-request/3188', - ('ding-libs', '3188')) + project_match( + 'fixes ' + 'http://localhost.localdomain/fork/pingou/test/issue/1', + ('/fork/pingou/test/issue/1',)) + project_match( + 'Could this be fixes ' + ' http://localhost.localdomain/pagure/tests2/issue/6', + ('/pagure/tests2/issue/6',)) + project_match( + 'merged http://localhost.localdomain/myproject/pull-request/70', + ('/myproject/pull-request/70',)) + project_match( + 'Now we merge http://localhost.localdomain/myproject/pull-request/99', + ('/myproject/pull-request/99',)) + project_match( + 'Merges http://localhost.localdomain/fork/pingou/test/issue/1', + ('/fork/pingou/test/issue/1',)) + project_match( + 'Merges: http://localhost.localdomain/fork/pingou/test/issue/12', + ('/fork/pingou/test/issue/12',)) + project_match( + 'Merged http://localhost.localdomain/fork/pingou/test/issue/123#', + ('/fork/pingou/test/issue/123',)) + project_match('Merge: http://localhost.localdomain/fork/pingou/test/issue/1234#foo', + ('/fork/pingou/test/issue/1234',)) + project_match( + 'Merges: http://localhost.localdomain/SSSD/ding-libs/pull-request/3188', + ('/SSSD/ding-libs/pull-request/3188',)) + project_match( + 'Fixes: http://localhost.localdomain/fedpkg/issue/220', + ('/fedpkg/issue/220',)) # issue matches def issue_match(text, issue):