From 10eda21e5cc7a0fbe8ba1870b44fd4648de983e7 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 02 2016 16:51:38 +0000
Subject: [PATCH 1/5] Adjust the markdown processor to have 1 regex for all cross-project links


Until now we had two regex/extensions for the cross-project links. One
for forks one for regular project but this lead to the situation where
if you have a pattern such as <user>/<project>#<id> the first part
(<user>) would be linked to the fork of the project <project> of that
user and the second part (<project>#<id>) would link to the ticket/PR
with this id in the original project.
That was because the regex for cross-link project was matching the
content of the link generated by the cross-link fork.
By moving to one regex for all, we fix this situation.

---

diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py
index 063d9a6..4fd7f5e 100644
--- a/pagure/pfmarkdown.py
+++ b/pagure/pfmarkdown.py
@@ -31,8 +31,12 @@ import pagure.lib
 
 
 MENTION_RE = r'@(\w+)'
-EXPLICIT_FORK_ISSUE_RE = r'(\w+)/([\w-]+)#([0-9]+)'
-EXPLICIT_MAIN_ISSUE_RE = r'([\w-]+)#([0-9]+)'
+EXPLICIT_LINK_RE = r'(?<!\w)'\
+'(fork[s]?/)?'\
+'([a-zA-Z0-9_-]*?/)?'\
+'([a-zA-Z0-9_-]*?/)?'\
+'([a-zA-Z0-9_-]+)'\
+'#(?P<id>[0-9]+)'
 IMPLICIT_ISSUE_RE = r'[^|\w](?<!\w)#([0-9]+)'
 IMPLICIT_PR_RE = r'[^|\w](?<!\w)PR#([0-9]+)'
 STRIKE_THROUGH_RE = r'~~(\w+)~~'
@@ -59,53 +63,42 @@ class MentionPattern(markdown.inlinepatterns.Pattern):
         return element
 
 
-class ExplicitForkIssuePattern(markdown.inlinepatterns.Pattern):
-    """ Explicit fork issue pattern. """
+class ExplicitLinkPattern(markdown.inlinepatterns.Pattern):
+    """ Explicit link pattern. """
 
     def handleMatch(self, m):
         """ When the pattern matches, update the text. """
-        user = markdown.util.AtomicString(m.group(2))
-        repo = markdown.util.AtomicString(m.group(3))
-        idx = markdown.util.AtomicString(m.group(4))
-        text = '%s/%s#%s' % (user, repo, idx)
-        try:
-            idx = int(idx)
-        except:
-            return text
-
-        issue = _issue_exists(user, repo, idx)
-        if issue:
-            return _obj_anchor_tag(user, repo, issue, text)
-
-
-        request = _pr_exists(user, repo, idx)
-        if request:
-            return _obj_anchor_tag(user, repo, request, text)
-
-        return text
+        is_fork = m.group(2)
+        user = m.group(3)
+        namespace = m.group(4)
+        repo = m.group(5)
+        idx = m.group(6)
+        text = '%s#%s' % (repo, idx)
 
+        if not is_fork and user:
+            namespace = user
+            user = None
 
-class ExplicitMainIssuePattern(markdown.inlinepatterns.Pattern):
-    """ Explicit issue pattern (for non-fork project). """
+        if namespace:
+            namespace = namespace.rstrip('/')
+            text = '%s/%s' % (namespace, text)
+        if user:
+            user = user.rstrip('/')
+            text = '%s/%s' % (user.rstrip('/'), text)
 
-    def handleMatch(self, m):
-        """ When the pattern matches, update the text. """
-        repo = markdown.util.AtomicString(m.group(2))
-        idx = markdown.util.AtomicString(m.group(3))
-        text = '%s#%s' % (repo, idx)
         try:
             idx = int(idx)
         except:
             return text
 
-        issue = _issue_exists(None, repo, idx)
+        issue = _issue_exists(user, namespace, repo, idx)
         if issue:
-            return _obj_anchor_tag(None, repo, issue, text)
+            return _obj_anchor_tag(user, namespace, repo, issue, text)
 
 
-        request = _pr_exists(None, repo, idx)
+        request = _pr_exists(user, namespace, repo, idx)
         if request:
-            return _obj_anchor_tag(None, repo, request, text)
+            return _obj_anchor_tag(user, namespace, repo, request, text)
 
         return text
 
@@ -127,10 +120,12 @@ class ImplicitIssuePattern(markdown.inlinepatterns.Pattern):
             url = flask.request.url
         except RuntimeError:
             return text
-        repo = user = None
+        repo = namespace = user = None
 
         if flask.request.args.get('user'):
             user = flask.request.args.get('user')
+        if flask.request.args.get('namespace'):
+            namespace = flask.request.args.get('namespace')
         if flask.request.args.get('repo'):
             repo = flask.request.args.get('repo')
 
@@ -140,13 +135,13 @@ class ImplicitIssuePattern(markdown.inlinepatterns.Pattern):
             else:
                 repo = url.split(root)[1].split('/', 1)[0]
 
-        issue = _issue_exists(user, repo, idx)
+        issue = _issue_exists(user, namespace, repo, idx)
         if issue:
-            return _obj_anchor_tag(user, repo, issue, text)
+            return _obj_anchor_tag(user, namespace, repo, issue, text)
 
-        request = _pr_exists(user, repo, idx)
+        request = _pr_exists(user, namespace, repo, idx)
         if request:
-            return _obj_anchor_tag(user, repo, request, text)
+            return _obj_anchor_tag(user, namespace, repo, request, text)
 
         return text
 
@@ -168,10 +163,12 @@ class ImplicitPRPattern(markdown.inlinepatterns.Pattern):
             url = flask.request.url
         except RuntimeError:
             return text
-        repo = user = None
+        repo = namespace = user = None
 
         if flask.request.args.get('user'):
             user = flask.request.args.get('user')
+        if flask.request.args.get('namespace'):
+            namespace = flask.request.args.get('namespace')
         if flask.request.args.get('repo'):
             repo = flask.request.args.get('repo')
 
@@ -181,13 +178,13 @@ class ImplicitPRPattern(markdown.inlinepatterns.Pattern):
             else:
                 repo = url.split(root)[1].split('/', 1)[0]
 
-        issue = _issue_exists(user, repo, idx)
+        issue = _issue_exists(user, namespace, repo, idx)
         if issue:
-            return _obj_anchor_tag(user, repo, issue, text)
+            return _obj_anchor_tag(user, namespace, repo, issue, text)
 
-        request = _pr_exists(user, repo, idx)
+        request = _pr_exists(user, namespace, repo, idx)
         if request:
-            return _obj_anchor_tag(user, repo, request, text)
+            return _obj_anchor_tag(user, namespace, repo, request, text)
 
         return text
 
@@ -218,9 +215,7 @@ class PagureExtension(markdown.extensions.Extension):
             md.inlinePatterns['implicit_pr'] = \
                 ImplicitPRPattern(IMPLICIT_PR_RE)
             md.inlinePatterns['explicit_fork_issue'] = \
-                ExplicitForkIssuePattern(EXPLICIT_FORK_ISSUE_RE)
-            md.inlinePatterns['explicit_main_issue'] = \
-                ExplicitMainIssuePattern(EXPLICIT_MAIN_ISSUE_RE)
+                ExplicitLinkPattern(EXPLICIT_LINK_RE)
             md.inlinePatterns['implicit_issue'] = \
                 ImplicitIssuePattern(IMPLICIT_ISSUE_RE)
 
@@ -234,10 +229,10 @@ def makeExtension(*arg, **kwargs):
     return PagureExtension(**kwargs)
 
 
-def _issue_exists(user, repo, idx):
+def _issue_exists(user, namespace, repo, idx):
     """ Utility method checking if a given issue exists. """
     repo_obj = pagure.lib.get_project(
-        pagure.SESSION, name=repo, user=user)
+        pagure.SESSION, name=repo, user=user, namespace=namespace)
     if not repo_obj:
         return False
 
@@ -249,10 +244,10 @@ def _issue_exists(user, repo, idx):
     return issue_obj
 
 
-def _pr_exists(user, repo, idx):
+def _pr_exists(user, namespace, repo, idx):
     """ Utility method checking if a given PR exists. """
     repo_obj = pagure.lib.get_project(
-        pagure.SESSION, name=repo, user=user)
+        pagure.SESSION, name=repo, user=user, namespace=namespace)
     if not repo_obj:
         return False
 
@@ -264,14 +259,16 @@ def _pr_exists(user, repo, idx):
     return pr_obj
 
 
-def _obj_anchor_tag(user, repo, obj, text):
+def _obj_anchor_tag(user, namespace, repo, obj, text):
     """ Utility method generating the link to an issue or a PR. """
     if obj.isa == 'issue':
         url = flask.url_for(
-            'view_issue', username=user, repo=repo, issueid=obj.id)
+            'view_issue', username=user, namespace=namespace, repo=repo,
+             issueid=obj.id)
     else:
         url = flask.url_for(
-            'request_pull', username=user, repo=repo, requestid=obj.id)
+            'request_pull', username=user, namespace=namespace, repo=repo,
+            requestid=obj.id)
 
     element = markdown.util.etree.Element("a")
     element.set('href', url)

From 9cc1bd0f3254bba9d68526bce9dd2f58b1d4be9a Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 02 2016 16:51:38 +0000
Subject: [PATCH 2/5] Add unit-tests for pagure.lib.text2markdown


This should highly simplify the maintenance of this piece of code and
help with stability.

---

diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 2b49320..8cf952b 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2521,6 +2521,138 @@ class PagureLibtests(tests.Modeltests):
         watch_list = [obj.name for obj in watch_list_objs]
         self.assertEqual(watch_list, [])
 
+    def test_text2markdown(self):
+        ''' Test the test2markdown method in pagure.lib. '''
+        pagure.APP.config['TESTING'] = True
+        pagure.APP.config['SERVER_NAME'] = 'https://pagure.org'
+        pagure.SESSION = self.session
+        pagure.lib.SESSION = self.session
+        self.app = pagure.APP.test_client()
+
+        # This creates:
+        # project: test
+        # fork: pingou/test
+        # PR#1 to project test
+        self.test_new_pull_request()
+
+        # create PR#2 to project pingou/test
+        repo = pagure.lib.get_project(self.session, 'test')
+        forked_repo = pagure.lib.get_project(
+            self.session, 'test', user='pingou')
+        req = pagure.lib.new_pull_request(
+            requestid=2,
+            session=self.session,
+            repo_from=forked_repo,
+            branch_from='master',
+            repo_to=forked_repo,
+            branch_to='master',
+            title='test pull-request in fork',
+            user='pingou',
+            requestfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 2)
+        self.assertEqual(req.title, 'test pull-request in fork')
+
+        # Create the project ns/test
+        item = pagure.lib.model.Project(
+            user_id=1,  # pingou
+            name='test3',
+            namespace='ns',
+            description='test project #1',
+            hook_token='aaabbbcccdd',
+        )
+        item.close_status = ['Invalid', 'Insufficient data', 'Fixed', 'Duplicate']
+        self.session.add(item)
+        self.session.commit()
+
+        iss = pagure.lib.new_issue(
+            issue_id=4,
+            session=self.session,
+            repo=item,
+            title='test issue',
+            content='content test issue',
+            user='pingou',
+            ticketfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(iss.id, 4)
+        self.assertEqual(iss.title, 'test issue')
+
+        # Fork ns/test to pingou
+        item = pagure.lib.model.Project(
+            user_id=1,  # pingou
+            name='test',
+            namespace='ns',
+            description='Forked namespaced test project #1',
+            is_fork=True,
+            parent_id=item.id,
+            hook_token='aaabbbrrrbb',
+        )
+        self.session.add(item)
+        self.session.commit()
+
+        iss = pagure.lib.new_issue(
+            issue_id=7,
+            session=self.session,
+            repo=item,
+            title='test issue #7',
+            content='content test issue #7 in forked repo',
+            user='pingou',
+            ticketfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(iss.id, 7)
+        self.assertEqual(iss.title, 'test issue #7')
+
+        texts = [
+            'foo bar test#1 see?',
+            'foo bar pingou/test#2 I mean, really',
+            'foo bar fork/pingou/test#2 bouza!',
+            'foo bar forks/pingou/test#2 bouza!',
+            'foo bar ns/test3#4 bouza!',
+            'foo bar fork/user/ns/test#5 bouza!',
+            'foo bar fork/pingou/ns/test#7 bouza!',
+            'test#1 bazinga!',
+            'pingou opened the PR forks/pingou/test#2'
+        ]
+        expected = [
+            # 'foo bar test#1 see?',
+            '<p>foo bar <a href="http://https://pagure.org/test/pull-request/1"'
+            ' title="test pull-request">test#1</a> see?</p>',
+            # 'foo bar pingou/test#2 I mean, really', -- unknown namespace
+            '<p>foo bar pingou/test#2 I mean, really</p>',
+            # 'foo bar fork/pingou/test#2 bouza!',
+            '<p>foo bar <a href="http://https://pagure.org/fork/'
+            'pingou/test/pull-request/2" title="test pull-request in fork">'
+            'pingou/test#2</a> bouza!</p>',
+            # 'foo bar forks/pingou/test#2 bouza!',  -- the 's' doesn't matter
+            '<p>foo bar <a href="http://https://pagure.org/fork/'
+            'pingou/test/pull-request/2" title="test pull-request in fork">'
+            'pingou/test#2</a> bouza!</p>',
+            # 'foo bar ns/test3#4 bouza!',
+            '<p>foo bar <a href="http://https://pagure.org/ns/test3/issue/4"'
+            ' title="test issue">ns/test3#4</a> bouza!</p>',
+            # 'foo bar fork/user/ns/test#5 bouza!', -- unknown fork
+            '<p>foo bar user/ns/test#5 bouza!</p>',
+            # 'foo bar fork/pingou/ns/test#7 bouza!',
+            '<p>foo bar <a href="http://https://pagure.org/'
+            'fork/pingou/ns/test/issue/7" title="test issue #7">'
+            'pingou/ns/test#7</a> bouza!</p>',
+            # 'test#1 bazinga!',
+            '<p><a href="http://https://pagure.org/test/pull-request/1" '
+            'title="test pull-request">test#1</a> bazinga!</p>',
+            # 'pingou opened the PR forks/pingou/test#2'
+            '<p>pingou opened the PR <a href="http://https://pagure.org/'
+            'fork/pingou/test/pull-request/2" '
+            'title="test pull-request in fork">pingou/test#2</a></p>'
+        ]
+
+        with pagure.APP.app_context():
+            for idx, text in enumerate(texts):
+                html = pagure.lib.text2markdown(text)
+                self.assertEqual(html, expected[idx])
+
 
 if __name__ == '__main__':
     SUITE = unittest.TestLoader().loadTestsFromTestCase(PagureLibtests)

From 55850f26f120e92442b4203c04b2f4f78ddfb185 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 02 2016 16:51:38 +0000
Subject: [PATCH 3/5] Document the regex a little more to try to make it more comprehensible


---

diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py
index 4fd7f5e..dc8323f 100644
--- a/pagure/pfmarkdown.py
+++ b/pagure/pfmarkdown.py
@@ -31,12 +31,12 @@ import pagure.lib
 
 
 MENTION_RE = r'@(\w+)'
-EXPLICIT_LINK_RE = r'(?<!\w)'\
-'(fork[s]?/)?'\
-'([a-zA-Z0-9_-]*?/)?'\
-'([a-zA-Z0-9_-]*?/)?'\
-'([a-zA-Z0-9_-]+)'\
-'#(?P<id>[0-9]+)'
+EXPLICIT_LINK_RE = r'(?<!\w)'\  # Ensure we catch the motif from the start
+'(fork[s]?/)?'\  # See if there is a `forks/` at the start
+'([a-zA-Z0-9_-]*?/)?'\ # See if we have a `user/`
+'([a-zA-Z0-9_-]*?/)?'\  # See if we have a `namespace/`
+'([a-zA-Z0-9_-]+)'\  # Get the last part `project`
+'#(?P<id>[0-9]+)'  # Get the identifier `#<id>`
 IMPLICIT_ISSUE_RE = r'[^|\w](?<!\w)#([0-9]+)'
 IMPLICIT_PR_RE = r'[^|\w](?<!\w)PR#([0-9]+)'
 STRIKE_THROUGH_RE = r'~~(\w+)~~'

From e11381c8c9c2050690cc489e1485f7e4da499a0f Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 02 2016 20:02:43 +0000
Subject: [PATCH 4/5] Document the regex in a way that make the code compile and work


---

diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py
index dc8323f..2e28810 100644
--- a/pagure/pfmarkdown.py
+++ b/pagure/pfmarkdown.py
@@ -31,12 +31,19 @@ import pagure.lib
 
 
 MENTION_RE = r'@(\w+)'
-EXPLICIT_LINK_RE = r'(?<!\w)'\  # Ensure we catch the motif from the start
-'(fork[s]?/)?'\  # See if there is a `forks/` at the start
-'([a-zA-Z0-9_-]*?/)?'\ # See if we have a `user/`
-'([a-zA-Z0-9_-]*?/)?'\  # See if we have a `namespace/`
-'([a-zA-Z0-9_-]+)'\  # Get the last part `project`
-'#(?P<id>[0-9]+)'  # Get the identifier `#<id>`
+# Each line below correspond to a line of the regex:
+#  1) Ensure we catch the motif from the start
+#  2) See if there is a `forks/` at the start
+#  3) See if we have a `user/`
+#  4) See if we have a `namespace/`
+#  5) Get the last part `project`
+#  6) Get the identifier `#<id>`
+EXPLICIT_LINK_RE = r'(?<!\w)'\
+'(fork[s]?/)?'\
+'([a-zA-Z0-9_-]*?/)?'\
+'([a-zA-Z0-9_-]*?/)?'\
+'([a-zA-Z0-9_-]+)'\
+'#(?P<id>[0-9]+)'
 IMPLICIT_ISSUE_RE = r'[^|\w](?<!\w)#([0-9]+)'
 IMPLICIT_PR_RE = r'[^|\w](?<!\w)PR#([0-9]+)'
 STRIKE_THROUGH_RE = r'~~(\w+)~~'

From 818f90172bdab8ee7de95265e5e812e5d7afd99f Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 02 2016 20:02:58 +0000
Subject: [PATCH 5/5] Drop the protocol in the SERVER_NAME to avoid http://https://


---

diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 8cf952b..0ad9deb 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -2524,7 +2524,7 @@ class PagureLibtests(tests.Modeltests):
     def test_text2markdown(self):
         ''' Test the test2markdown method in pagure.lib. '''
         pagure.APP.config['TESTING'] = True
-        pagure.APP.config['SERVER_NAME'] = 'https://pagure.org'
+        pagure.APP.config['SERVER_NAME'] = 'pagure.org'
         pagure.SESSION = self.session
         pagure.lib.SESSION = self.session
         self.app = pagure.APP.test_client()
@@ -2618,32 +2618,32 @@ class PagureLibtests(tests.Modeltests):
         ]
         expected = [
             # 'foo bar test#1 see?',
-            '<p>foo bar <a href="http://https://pagure.org/test/pull-request/1"'
+            '<p>foo bar <a href="http://pagure.org/test/pull-request/1"'
             ' title="test pull-request">test#1</a> see?</p>',
             # 'foo bar pingou/test#2 I mean, really', -- unknown namespace
             '<p>foo bar pingou/test#2 I mean, really</p>',
             # 'foo bar fork/pingou/test#2 bouza!',
-            '<p>foo bar <a href="http://https://pagure.org/fork/'
+            '<p>foo bar <a href="http://pagure.org/fork/'
             'pingou/test/pull-request/2" title="test pull-request in fork">'
             'pingou/test#2</a> bouza!</p>',
             # 'foo bar forks/pingou/test#2 bouza!',  -- the 's' doesn't matter
-            '<p>foo bar <a href="http://https://pagure.org/fork/'
+            '<p>foo bar <a href="http://pagure.org/fork/'
             'pingou/test/pull-request/2" title="test pull-request in fork">'
             'pingou/test#2</a> bouza!</p>',
             # 'foo bar ns/test3#4 bouza!',
-            '<p>foo bar <a href="http://https://pagure.org/ns/test3/issue/4"'
+            '<p>foo bar <a href="http://pagure.org/ns/test3/issue/4"'
             ' title="test issue">ns/test3#4</a> bouza!</p>',
             # 'foo bar fork/user/ns/test#5 bouza!', -- unknown fork
             '<p>foo bar user/ns/test#5 bouza!</p>',
             # 'foo bar fork/pingou/ns/test#7 bouza!',
-            '<p>foo bar <a href="http://https://pagure.org/'
+            '<p>foo bar <a href="http://pagure.org/'
             'fork/pingou/ns/test/issue/7" title="test issue #7">'
             'pingou/ns/test#7</a> bouza!</p>',
             # 'test#1 bazinga!',
-            '<p><a href="http://https://pagure.org/test/pull-request/1" '
+            '<p><a href="http://pagure.org/test/pull-request/1" '
             'title="test pull-request">test#1</a> bazinga!</p>',
             # 'pingou opened the PR forks/pingou/test#2'
-            '<p>pingou opened the PR <a href="http://https://pagure.org/'
+            '<p>pingou opened the PR <a href="http://pagure.org/'
             'fork/pingou/test/pull-request/2" '
             'title="test pull-request in fork">pingou/test#2</a></p>'
         ]