Merge pull request #6956 from readthedocs/fix-trailing-slash-repo-url

Fix the trailing slash in our repo regexs

readthedocs/builds/models.py

@@ -497,7 +497,6 @@ def get_github_url(
         user, repo = get_github_username_repo(repo_url)
         if not user and not repo:
             return ''
-        repo = repo.rstrip('/')
 
         if not filename:
             # If there isn't a filename, we don't need a suffix
@@ -538,7 +537,6 @@ def get_gitlab_url(
         user, repo = get_gitlab_username_repo(repo_url)
         if not user and not repo:
             return ''
-        repo = repo.rstrip('/')
 
         if not filename:
             # If there isn't a filename, we don't need a suffix
@@ -567,7 +565,6 @@ def get_bitbucket_url(self, docroot, filename, source_suffix='.rst'):
         user, repo = get_bitbucket_username_repo(repo_url)
         if not user and not repo:
             return ''
-        repo = repo.rstrip('/')
 
         if not filename:
             # If there isn't a filename, we don't need a suffix
@@ -806,7 +803,6 @@ def get_commit_url(self):
                 if not user and not repo:
                     return ''
 
-                repo = repo.rstrip('/')
                 return GITHUB_PULL_REQUEST_COMMIT_URL.format(
                     user=user,
                     repo=repo,
@@ -818,7 +814,6 @@ def get_commit_url(self):
                 if not user and not repo:
                     return ''
 
-                repo = repo.rstrip('/')
                 return GITLAB_MERGE_REQUEST_COMMIT_URL.format(
                     user=user,
                     repo=repo,
@@ -832,7 +827,6 @@ def get_commit_url(self):
                 if not user and not repo:
                     return ''
 
-                repo = repo.rstrip('/')
                 return GITHUB_COMMIT_URL.format(
                     user=user,
                     repo=repo,
@@ -843,7 +837,6 @@ def get_commit_url(self):
                 if not user and not repo:
                     return ''
 
-                repo = repo.rstrip('/')
                 return GITLAB_COMMIT_URL.format(
                     user=user,
                     repo=repo,
@@ -854,7 +847,6 @@ def get_commit_url(self):
                 if not user and not repo:
                     return ''
 
-                repo = repo.rstrip('/')
                 return BITBUCKET_COMMIT_URL.format(
                     user=user,
                     repo=repo,

readthedocs/oauth/services/github.py

@@ -445,8 +445,9 @@ def send_build_status(self, build, commit, state):
         resp = None
 
         try:
+            statuses_url = f'https://api.github.com/repos/{owner}/{repo}/statuses/{commit}'
             resp = session.post(
-                f'https://api.github.com/repos/{owner}/{repo}/statuses/{commit}',
+                statuses_url,
                 data=json.dumps(data),
                 headers={'content-type': 'application/json'},
             )
@@ -461,9 +462,11 @@ def send_build_status(self, build, commit, state):
             if resp.status_code in [401, 403, 404]:
                 log.info(
                     'GitHub project does not exist or user does not have '
-                    'permissions: project=%s, user=%s',
+                    'permissions: project=%s, user=%s, status=%s, url=%s',
                     project,
                     self.user,
+                    resp.status_code,
+                    statuses_url,
                 )
                 return False
 

readthedocs/projects/constants.py

@@ -312,17 +312,23 @@
 
 GITHUB_REGEXS = [
     re.compile(r'github.com/(.+)/(.+)(?:\.git){1}$'),
+    # This must come before the one without a / to make sure we don't capture the /
+    re.compile(r'github.com/(.+)/(.+)/'),
     re.compile(r'github.com/(.+)/(.+)'),
     re.compile(r'github.com:(.+)/(.+)\.git$'),
 ]
 BITBUCKET_REGEXS = [
     re.compile(r'bitbucket.org/(.+)/(.+)\.git$'),
     re.compile(r'@bitbucket.org/(.+)/(.+)\.git$'),
-    re.compile(r'bitbucket.org/(.+)/(.+)/?'),
+    # This must come before the one without a / to make sure we don't capture the /
+    re.compile(r'bitbucket.org/(.+)/(.+)/'),
+    re.compile(r'bitbucket.org/(.+)/(.+)'),
     re.compile(r'bitbucket.org:(.+)/(.+)\.git$'),
 ]
 GITLAB_REGEXS = [
     re.compile(r'gitlab.com/(.+)/(.+)(?:\.git){1}$'),
+    # This must come before the one without a / to make sure we don't capture the /
+    re.compile(r'gitlab.com/(.+)/(.+)/'),
     re.compile(r'gitlab.com/(.+)/(.+)'),
     re.compile(r'gitlab.com:(.+)/(.+)\.git$'),
 ]

readthedocs/projects/tasks.py

@@ -2195,6 +2195,8 @@ def send_build_status(build_pk, commit, status):
     build = Build.objects.get(pk=build_pk)
     provider_name = build.project.git_provider_name
 
+    log.info('Sending build status. build=%s, project=%s', build.pk, build.project.slug)
+
     if provider_name in [GITHUB_BRAND, GITLAB_BRAND]:
         # get the service class for the project e.g: GitHubService.
         service_class = build.project.git_service_class()

readthedocs/rtd_tests/tests/test_oauth.py

@@ -201,9 +201,11 @@ def test_send_build_status_404_error(self, session, mock_logger):
         self.assertFalse(success)
         mock_logger.info.assert_called_with(
             'GitHub project does not exist or user does not have '
-            'permissions: project=%s, user=%s',
+            'permissions: project=%s, user=%s, status=%s, url=%s',
             self.project,
-            self.user
+            self.user,
+            404,
+            'https://api.github.com/repos/pypa/pip/statuses/297'
         )
 
     @mock.patch('readthedocs.oauth.services.github.log')