diff --git a/reviewboard/hostingsvcs/bitbucket.py b/reviewboard/hostingsvcs/bitbucket.py index 8920ceb94dbd43a8fca45ea773b2e10059b7ab1a..47ad5dd568a0507eee182013063e5986af3c5770 100644 --- a/reviewboard/hostingsvcs/bitbucket.py +++ b/reviewboard/hostingsvcs/bitbucket.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import json +import logging from collections import defaultdict from django import forms @@ -64,7 +65,9 @@ class Bitbucket(HostingService): '', url(r'^hooks/close-submitted/$', - 'reviewboard.hostingsvcs.bitbucket.post_receive_hook_close_submitted'), + 'reviewboard.hostingsvcs.bitbucket' + '.post_receive_hook_close_submitted', + name='bitbucket-hooks-close-submitted'), ) supported_scmtools = ['Git', 'Mercurial'] @@ -266,36 +269,49 @@ class Bitbucket(HostingService): @require_POST -def post_receive_hook_close_submitted(request, *args, **kwargs): +def post_receive_hook_close_submitted(request, local_site_name=None, + repository_id=None, + hosting_service_id=None): """Closes review requests as submitted automatically after a push.""" if 'payload' not in request.POST: - return HttpResponse() + return HttpResponse(status=400) + + try: + payload = json.loads(request.POST['payload']) + except ValueError as e: + logging.error('The payload is not in JSON format: %s', e) + return HttpResponse(status=400) - payload = json.loads(request.POST['payload']) server_url = get_server_url(request) - review_id_to_commits = get_review_id_to_commits_map(payload, server_url) - close_all_review_requests(review_id_to_commits) + review_request_id_to_commits = \ + _get_review_request_id_to_commits_map(payload, server_url) + + if review_request_id_to_commits: + close_all_review_requests(review_request_id_to_commits, + local_site_name, repository_id, + hosting_service_id) + return HttpResponse() -def get_review_id_to_commits_map(payload, server_url): +def _get_review_request_id_to_commits_map(payload, server_url): """Returns a dictionary, mapping a review request ID to a list of commits. - If a commit's commit message does not contain a review request ID, we append - the commit to the key None. + If a commit's commit message does not contain a review request ID, we + append the commit to the key None. """ - review_id_to_commits_map = defaultdict(list) + review_request_id_to_commits_map = defaultdict(list) commits = payload.get('commits', []) for commit in commits: - commit_hash = commit.get('raw_node', None) - commit_message = commit.get('message', None) - branch_name = commit.get('branch', None) + commit_hash = commit.get('raw_node') + commit_message = commit.get('message') + branch_name = commit.get('branch') if branch_name: - review_request_id = get_review_request_id(commit_message, server_url, - commit_hash) - commit_entry = '%s (%s)' % (branch_name, commit_hash[:7]) - review_id_to_commits_map[review_request_id].append(commit_entry) + review_request_id = get_review_request_id( + commit_message, server_url, commit_hash) + review_request_id_to_commits_map[review_request_id].append( + '%s (%s)' % (branch_name, commit_hash[:7])) - return review_id_to_commits_map + return review_request_id_to_commits_map diff --git a/reviewboard/hostingsvcs/github.py b/reviewboard/hostingsvcs/github.py index c69e5c0e9b9fa69a0c7c0baf6a253c1481fb71fc..92bb6d1ef6bb32b5cac30321eea9eec12180487a 100644 --- a/reviewboard/hostingsvcs/github.py +++ b/reviewboard/hostingsvcs/github.py @@ -269,7 +269,8 @@ class GitHub(HostingService): '', url(r'^hooks/close-submitted/$', - 'reviewboard.hostingsvcs.github.post_receive_hook_close_submitted'), + 'reviewboard.hostingsvcs.github.post_receive_hook_close_submitted', + name='github-hooks-close-submitted') ) # This should be the prefix for every field on the plan forms. @@ -742,32 +743,35 @@ class GitHub(HostingService): @require_POST -def post_receive_hook_close_submitted(request, *args, **kwargs): +def post_receive_hook_close_submitted(request, local_site_name=None, + repository_id=None, + hosting_service_id=None): """Closes review requests as submitted automatically after a push.""" try: payload = json.loads(request.body) except ValueError as e: logging.error('The payload is not in JSON format: %s', e) - return HttpResponse(status=415) + return HttpResponse(status=400) server_url = get_server_url(request) - review_id_to_commits = _get_review_id_to_commits_map(payload, server_url) + review_request_id_to_commits = \ + _get_review_request_id_to_commits_map(payload, server_url) - if not review_id_to_commits: - return HttpResponse() - - close_all_review_requests(review_id_to_commits) + if review_request_id_to_commits: + close_all_review_requests(review_request_id_to_commits, + local_site_name, repository_id, + hosting_service_id) return HttpResponse() -def _get_review_id_to_commits_map(payload, server_url): +def _get_review_request_id_to_commits_map(payload, server_url): """Returns a dictionary, mapping a review request ID to a list of commits. If a commit's commit message does not contain a review request ID, we append the commit to the key None. """ - review_id_to_commits_map = defaultdict(list) + review_request_id_to_commits_map = defaultdict(list) ref_name = payload.get('ref') if not ref_name: @@ -785,7 +789,7 @@ def _get_review_id_to_commits_map(payload, server_url): review_request_id = get_review_request_id(commit_message, server_url, commit_hash) - commit_entry = '%s (%s)' % (branch_name, commit_hash[:7]) - review_id_to_commits_map[review_request_id].append(commit_entry) + review_request_id_to_commits_map[review_request_id].append( + '%s (%s)' % (branch_name, commit_hash[:7])) - return review_id_to_commits_map + return review_request_id_to_commits_map diff --git a/reviewboard/hostingsvcs/googlecode.py b/reviewboard/hostingsvcs/googlecode.py index 995f227936b0fb0cb0881993d6fcdb131c70c26b..e32f63e79e076f219b7bf31089389ad0805bb719 100644 --- a/reviewboard/hostingsvcs/googlecode.py +++ b/reviewboard/hostingsvcs/googlecode.py @@ -34,8 +34,11 @@ class GoogleCode(HostingService): repository_url_patterns = patterns( '', + url(r'^hooks/close-submitted/$', - 'reviewboard.hostingsvcs.googlecode.post_receive_hook_close_submitted'), + 'reviewboard.hostingsvcs.googlecode' + '.post_receive_hook_close_submitted', + name='googlecode-hooks-close-submitted'), ) repository_fields = { @@ -58,21 +61,30 @@ class GoogleCode(HostingService): @require_POST -def post_receive_hook_close_submitted(request, *args, **kwargs): +def post_receive_hook_close_submitted(request, local_site_name=None, + repository_id=None, + hosting_service_id=None): """Closes review requests as submitted automatically after a push.""" try: payload = json.loads(request.body) except KeyError as e: logging.error('There is no JSON payload in the POST request: %s', e, exc_info=1) - return HttpResponse(status=415) + return HttpResponse(status=400) except ValueError as e: logging.error('The payload is not in JSON format: %s', e, exc_info=1) - return HttpResponse(status=415) + return HttpResponse(status=400) server_url = get_server_url(request) - close_review_requests(payload, server_url) + review_request_id_to_commits_map = \ + close_review_requests(payload, server_url) + + if review_request_id_to_commits_map: + close_all_review_requests(review_request_id_to_commits_map, + local_site_name, repository_id, + hosting_service_id) + return HttpResponse() @@ -83,11 +95,11 @@ def close_review_requests(payload, server_url): # which SCM tool was used for the commit. That's why the only way # to close a review request through this hook is by adding the review # request id in the commit message. - review_id_to_commits_map = defaultdict(list) + review_request_id_to_commits_map = defaultdict(list) branch_name = payload.get('repository_path') if not branch_name: - return review_id_to_commits_map + return review_request_id_to_commits_map revisions = payload.get('revisions', []) @@ -100,7 +112,7 @@ def close_review_requests(payload, server_url): commit_message = revision.get('message') review_request_id = get_review_request_id(commit_message, server_url, None) - commit_entry = '%s (%s)' % (branch_name, revision_id) - review_id_to_commits_map[review_request_id].append(commit_entry) + review_request_id_to_commits_map[review_request_id].append( + '%s (%s)' % (branch_name, revision_id)) - close_all_review_requests(review_id_to_commits_map) + return review_request_id_to_commits_map diff --git a/reviewboard/hostingsvcs/hook_utils.py b/reviewboard/hostingsvcs/hook_utils.py index a296c4d7bc33c743cbe587de517df5018509d9c7..d1871385a7bfcd52b9c778d7a62675845bc8f599 100644 --- a/reviewboard/hostingsvcs/hook_utils.py +++ b/reviewboard/hostingsvcs/hook_utils.py @@ -5,13 +5,12 @@ import re from django.conf import settings from django.contrib.sites.models import Site -from django.core.urlresolvers import resolve, Resolver404 -from django.http import Http404 +from django.db.models import Q from django.utils import six from djblets.siteconfig.models import SiteConfiguration from reviewboard.reviews.models import ReviewRequest -from reviewboard.reviews.views import _find_review_request_object +from reviewboard.site.models import LocalSite from reviewboard.site.urlresolvers import local_site_reverse @@ -79,35 +78,66 @@ def close_review_request(review_request, review_request_id, description): review_request_id, review_request.status) -def close_all_review_requests(review_id_to_commits): +def close_all_review_requests(review_request_id_to_commits, local_site_name, + repository_id, hosting_service_id): """Closes each review request in the given dictionary as submitted. The provided dictionary should map a review request ID (int) to commits associated with that review request ID (list of strings). Commits that are not associated with any review requests have the key None. """ - for review_request_id in review_id_to_commits: - if not review_request_id: - logging.debug('No matching review request ID found for commits: ' + - ', '.join(review_id_to_commits[review_request_id])) - continue - + if local_site_name: try: - match = resolve('/r/%s/' % review_request_id) - except Resolver404, e: - logging.error('Could not resolve URL: %s', e) - continue + local_site = LocalSite.objects.get(name=local_site_name) + except LocalSite.DoesNotExist: + logging.error('close_all_review_requests: Local Site %s does ' + 'not exist.', + local_site_name) + return + else: + local_site = None - local_site = match.kwargs.get('local_site', None) - description = ('Pushed to ' + - ', '.join(review_id_to_commits[review_request_id])) + # Some of the entries we get may have 'None' keys, so filter them out. + review_request_ids = [ + review_request_id + for review_request_id in review_request_id_to_commits + if review_request_id is not None + ] - try: - review_request = \ - _find_review_request_object(review_request_id, local_site) - except Http404, e: - logging.error('Review request #%s does not exist.', - review_request_id) - continue - - close_review_request(review_request, review_request_id, description) + if not review_request_ids: + return + + # Look up all review requests that match the given repository, hosting + # service ID, and Local Site. + q = (Q(repository=repository_id) & + Q(repository__hosting_account__service_name=hosting_service_id)) + + if local_site: + q &= Q(local_id__in=review_request_ids) & Q(local_site=local_site) + else: + q &= Q(pk__in=review_request_ids) + + review_requests = list(ReviewRequest.objects.filter(q)) + + # Check if there are any listed that we couldn't find, and log them. + if len(review_request_ids) != len(review_requests): + id_to_review_request = dict(*[ + (review_request.display_id, review_request) + for review_request in review_requests + ]) + + for review_request_id in review_request_ids: + if review_request_id not in id_to_review_request: + logging.error('close_all_review_requests: Review request #%s ' + 'does not exist.', + review_request_id) + + # Close any review requests we did find. + for review_request in review_requests: + review_request_id = review_request.display_id + + close_review_request( + review_request, + review_request_id, + ('Pushed to ' + + ', '.join(review_request_id_to_commits[review_request_id]))) diff --git a/reviewboard/hostingsvcs/service.py b/reviewboard/hostingsvcs/service.py index 7bdf9dbaf8cd8c991244835b879d9b128fb152b5..fa0f6260946e4d01339dd8e6aaadf3190b2d3dc5 100644 --- a/reviewboard/hostingsvcs/service.py +++ b/reviewboard/hostingsvcs/service.py @@ -419,7 +419,8 @@ def _add_hosting_service_url_pattern(name, cls): if cls.repository_url_patterns: cls_urlpatterns = patterns( '', - url(r'^' + name + '/', include(cls.repository_url_patterns)) + url(r'^(?P' + name + ')/', + include(cls.repository_url_patterns)) ) _hostingsvcs_urlpatterns[name] = cls_urlpatterns hostingsvcs_urls.dynamic_urls.add_patterns(cls_urlpatterns) diff --git a/reviewboard/hostingsvcs/tests.py b/reviewboard/hostingsvcs/tests.py index ada4a6f3efe6013243862615cd7802049f32c90a..7836b96b66ba3b29c0b8ea19770e45c0efe0e4d9 100644 --- a/reviewboard/hostingsvcs/tests.py +++ b/reviewboard/hostingsvcs/tests.py @@ -5,11 +5,13 @@ from hashlib import md5 from textwrap import dedent from django.conf.urls import patterns, url +from django.core.urlresolvers import NoReverseMatch from django.http import HttpResponse from django.utils import six from django.utils.six.moves import cStringIO as StringIO from django.utils.six.moves.urllib.error import HTTPError from django.utils.six.moves.urllib.parse import urlparse +from djblets.testing.decorators import add_fixtures from kgb import SpyAgency from reviewboard.hostingsvcs.errors import RepositoryError @@ -18,10 +20,13 @@ from reviewboard.hostingsvcs.service import (get_hosting_service, HostingService, register_hosting_service, unregister_hosting_service) +from reviewboard.reviews.models import ReviewRequest from reviewboard.scmtools.core import Branch from reviewboard.scmtools.crypto_utils import encrypt_password from reviewboard.scmtools.errors import FileNotFoundError, SCMError from reviewboard.scmtools.models import Repository, Tool +from reviewboard.site.models import LocalSite +from reviewboard.site.urlresolvers import local_site_reverse from reviewboard.testing import TestCase @@ -65,7 +70,7 @@ class ServiceTests(SpyAgency, TestCase): return form - def _get_hosting_account(self, use_url=False): + def _get_hosting_account(self, use_url=False, local_site=None): if use_url: hosting_url = 'https://example.com' else: @@ -73,7 +78,8 @@ class ServiceTests(SpyAgency, TestCase): return HostingServiceAccount(service_name=self.service_name, username='myuser', - hosting_url=hosting_url) + hosting_url=hosting_url, + local_site=local_site) def _get_service(self): return self._get_hosting_account().service @@ -516,6 +522,145 @@ class BitbucketTests(ServiceTests): expected_found=False, expected_http_called=False) + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook(self): + """Testing BitBucket close_submitted hook""" + self._test_post_commit_hook() + + @add_fixtures(['test_site', 'test_users', 'test_scmtools']) + def test_close_submitted_hook_with_local_site(self): + """Testing BitBucket close_submitted hook with a Local Site""" + self._test_post_commit_hook( + LocalSite.objects.get(name=self.local_site_name)) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_repo(self): + """Testing BitBucket close_submitted hook with invalid repository""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'bitbucket-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'bitbucket', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_site(self): + """Testing BitBucket close_submitted hook with invalid Local Site""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'bitbucket-hooks-close-submitted', + local_site_name='badsite', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'bitbucket', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_service_id(self): + """Testing BitBucket close_submitted hook with invalid hosting + service ID + """ + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + # We'll test against GitHub's hooks for this test. + url = local_site_reverse( + 'github-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + def _test_post_commit_hook(self, local_site=None): + account = self._get_hosting_account(local_site=local_site) + account.save() + + repository = self.create_repository(hosting_account=account, + local_site=local_site) + + review_request = self.create_review_request(repository=repository, + local_site=local_site, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'bitbucket-hooks-close-submitted', + local_site=local_site, + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'bitbucket', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.SUBMITTED) + self.assertEqual(review_request.changedescs.count(), 1) + + changedesc = review_request.changedescs.get() + self.assertEqual(changedesc.text, 'Pushed to master (1c44b46)') + + def _post_commit_hook_payload(self, url, review_request): + self.client.post( + url, + data={ + 'payload': json.dumps({ + # NOTE: This payload only contains the content we make + # use of in the hook. + 'commits': [ + { + 'raw_node': '1c44b461cebe5874a857c51a4a13a84' + '9a4d1e52d', + 'branch': 'master', + 'message': 'This is my fancy commit\n' + '\n' + 'Reviewed at http://example.com%s' + % review_request.get_absolute_url(), + }, + ] + }), + }) + def _test_get_file(self, tool_name, revision, base_commit_id, expected_revision): def _http_get(service, url, *args, **kwargs): @@ -1299,6 +1444,142 @@ class GitHubTests(ServiceTests): SCMError, 'Not Found', lambda: service.get_change(repository, commit_sha)) + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook(self): + """Testing GitHub close_submitted hook""" + self._test_post_commit_hook() + + @add_fixtures(['test_site', 'test_users', 'test_scmtools']) + def test_close_submitted_hook_with_local_site(self): + """Testing GitHub close_submitted hook with a Local Site""" + self._test_post_commit_hook( + LocalSite.objects.get(name=self.local_site_name)) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_repo(self): + """Testing GitHub close_submitted hook with invalid repository""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'github-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_site(self): + """Testing GitHub close_submitted hook with invalid Local Site""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'github-hooks-close-submitted', + local_site_name='badsite', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_service_id(self): + """Testing GitHub close_submitted hook with invalid hosting service ID + """ + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + # We'll test against Bitbucket's hooks for this test. + url = local_site_reverse( + 'bitbucket-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'bitbucket', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + def _test_post_commit_hook(self, local_site=None): + account = self._get_hosting_account(local_site=local_site) + account.save() + + repository = self.create_repository(hosting_account=account, + local_site=local_site) + + review_request = self.create_review_request(repository=repository, + local_site=local_site, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'github-hooks-close-submitted', + local_site=local_site, + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'github', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.SUBMITTED) + self.assertEqual(review_request.changedescs.count(), 1) + + changedesc = review_request.changedescs.get() + self.assertEqual(changedesc.text, 'Pushed to master (1c44b46)') + + def _post_commit_hook_payload(self, url, review_request): + self.client.post( + url, + json.dumps({ + # NOTE: This payload only contains the content we make + # use of in the hook. + 'ref': 'refs/heads/master', + 'commits': [ + { + 'id': '1c44b461cebe5874a857c51a4a13a849a4d1e52d', + 'message': 'This is my fancy commit\n' + '\n' + 'Reviewed at http://example.com%s' + % review_request.get_absolute_url(), + }, + ] + }), + content_type='application/json') + def _test_check_repository(self, expected_user='myuser', **kwargs): def _http_get(service, url, *args, **kwargs): self.assertEqual( @@ -1588,6 +1869,142 @@ class GoogleCodeTests(ServiceTests): self.assertEqual(fields['mirror_path'], 'https://myproj.googlecode.com/svn') + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook(self): + """Testing Google Code close_submitted hook""" + self._test_post_commit_hook() + + @add_fixtures(['test_site', 'test_users', 'test_scmtools']) + def test_close_submitted_hook_with_local_site(self): + """Testing Google Code close_submitted hook with a Local Site""" + self._test_post_commit_hook( + LocalSite.objects.get(name=self.local_site_name)) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_repo(self): + """Testing Google Code close_submitted hook with invalid repository""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'googlecode-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'googlecode', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_site(self): + """Testing Google Code close_submitted hook with invalid Local Site""" + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'googlecode-hooks-close-submitted', + local_site_name='badsite', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'googlecode', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + @add_fixtures(['test_users', 'test_scmtools']) + def test_close_submitted_hook_with_invalid_service_id(self): + """Testing Google Code close_submitted hook with invalid hosting service ID + """ + repository = self.create_repository() + + review_request = self.create_review_request(repository=repository, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + # We'll test against Bitbucket's hooks for this test. + url = local_site_reverse( + 'bitbucket-hooks-close-submitted', + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'bitbucket', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + self.assertEqual(review_request.changedescs.count(), 0) + + def _test_post_commit_hook(self, local_site=None): + account = self._get_hosting_account(local_site=local_site) + account.save() + + repository = self.create_repository(hosting_account=account, + local_site=local_site) + + review_request = self.create_review_request(repository=repository, + local_site=local_site, + publish=True) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.PENDING_REVIEW) + + url = local_site_reverse( + 'googlecode-hooks-close-submitted', + local_site=local_site, + kwargs={ + 'repository_id': repository.pk, + 'hosting_service_id': 'googlecode', + }) + + self._post_commit_hook_payload(url, review_request) + + review_request = ReviewRequest.objects.get(pk=review_request.pk) + self.assertTrue(review_request.public) + self.assertEqual(review_request.status, review_request.SUBMITTED) + self.assertEqual(review_request.changedescs.count(), 1) + + changedesc = review_request.changedescs.get() + self.assertEqual(changedesc.text, 'Pushed to master (1c44b46)') + + def _post_commit_hook_payload(self, url, review_request): + self.client.post( + url, + json.dumps({ + # NOTE: This payload only contains the content we make + # use of in the hook. + 'repository_path': 'master', + 'revisions': [ + { + 'revision': '1c44b461cebe5874a857c51a4a13a849a4d1e52d', + 'message': 'This is my fancy commit\n' + '\n' + 'Reviewed at http://example.com%s' + % review_request.get_absolute_url(), + }, + ] + }), + content_type='application/json') + class RedmineTests(ServiceTests): """Unit tests for the Redmine hosting service.""" @@ -1962,56 +2379,86 @@ def hosting_service_url_test_view(request, repo_id): return HttpResponse(str(repo_id)) -class HostingServiceUrlPatternTests(TestCase): - """Unit tests for generating URL patterns.""" - test_url = '/repos/1/DummyService/hooks/pre-commit/' - +class HostingServiceRegistrationTests(TestCase): + """Unit tests for Hosting Service registration.""" class DummyService(HostingService): name = 'DummyService' + class DummyServiceWithURLs(HostingService): + name = 'DummyServiceWithURLs' + repository_url_patterns = patterns( '', - url(r'^hooks/pre-commit/$', - hosting_service_url_test_view) + + url(r'^hooks/pre-commit/$', hosting_service_url_test_view, + name='dummy-service-post-commit-hook'), ) - def test_url_registration(self): - """Testing the registration and unregistration of a hosting service""" - # Testing hosting service and URL registration - register_hosting_service('DummyService', self.DummyService) - response = self.client.get(self.test_url) - self.assertEqual(response.status_code, 200) + def tearDown(self): + super(HostingServiceRegistrationTests, self).tearDown() - # Once registered, should not be able to register again - self.assertRaises(KeyError, - register_hosting_service, - 'DummyService', - self.DummyService) + # Unregister the service, going back to a default state. It's okay + # if it fails. + # + # This will match whichever service we added for testing. + try: + unregister_hosting_service('dummy-service') + except KeyError: + pass + + def test_register_without_urls(self): + """Testing HostingService registration""" + register_hosting_service('dummy-service', self.DummyService) + + with self.assertRaises(KeyError): + register_hosting_service('dummy-service', self.DummyService) - # Testing unregistration of hosting service. Should not be - # able to resolve the URL - unregister_hosting_service('DummyService') - response = self.client.get(self.test_url) - self.assertEqual(response.status_code, 404) + def test_unregister(self): + """Testing HostingService unregistration""" + register_hosting_service('dummy-service', self.DummyService) + unregister_hosting_service('dummy-service') + + def test_registration_with_urls(self): + """Testing HostingService registration with URLs""" + register_hosting_service('dummy-service', self.DummyServiceWithURLs) + + self.assertEqual( + local_site_reverse( + 'dummy-service-post-commit-hook', + kwargs={ + 'repository_id': 1, + 'hosting_service_id': 'dummy-service', + }), + '/repos/1/dummy-service/hooks/pre-commit/') + + self.assertEqual( + local_site_reverse( + 'dummy-service-post-commit-hook', + local_site_name='test-site', + kwargs={ + 'repository_id': 1, + 'hosting_service_id': 'dummy-service', + }), + '/s/test-site/repos/1/dummy-service/hooks/pre-commit/') + + # Once registered, should not be able to register again + with self.assertRaises(KeyError): + register_hosting_service('dummy-service', + self.DummyServiceWithURLs) + + def test_unregistration_with_urls(self): + """Testing HostingService unregistration with URLs""" + register_hosting_service('dummy-service', self.DummyServiceWithURLs) + unregister_hosting_service('dummy-service') + + with self.assertRaises(NoReverseMatch): + local_site_reverse( + 'dummy-service-post-commit-hook', + kwargs={ + 'repository_id': 1, + 'hosting_service_id': 'dummy-service', + }), # Once unregistered, should not be able to unregister again - self.assertRaises(KeyError, - unregister_hosting_service, - 'DummyService') - - # Should not add repository_url_patterns if it is None. - # But should still register the hosting service - self.DummyService.repository_url_patterns = None - register_hosting_service('DummyService', self.DummyService) - self.assertRaises(KeyError, - register_hosting_service, - 'DummyService', - self.DummyService) - response = self.client.get(self.test_url) - self.assertEqual(response.status_code, 404) - - # Should be able to unregister successfully after the previous - # test. - unregister_hosting_service('DummyService') - response = self.client.get(self.test_url) - self.assertEqual(response.status_code, 404) + with self.assertRaises(KeyError): + unregister_hosting_service('dummy-service') diff --git a/reviewboard/hostingsvcs/urls.py b/reviewboard/hostingsvcs/urls.py index d722e29a32280d82390bead9e66137da87b94f19..9da403b7cddefc407e89cae1d9d6b7f7b8453b89 100755 --- a/reviewboard/hostingsvcs/urls.py +++ b/reviewboard/hostingsvcs/urls.py @@ -10,5 +10,5 @@ dynamic_urls = DynamicURLResolver() urlpatterns = patterns( '', - (r'^repos/(?P\d+)/', include(patterns('', dynamic_urls))), + (r'^repos/(?P\d+)/', include(patterns('', dynamic_urls))), ) diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index a6fe034de625e76fc8fc8909994bde19524382ba..33193874e8faea00e81e486fa403599cb326d56e 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -282,7 +282,8 @@ class TestCase(DjbletsTestCase): path=path, **kwargs) - def create_review_request(self, with_local_site=False, with_diffs=False, + def create_review_request(self, with_local_site=False, local_site=None, + with_diffs=False, summary='Test Summary', description='Test Description', testing_done='Testing', @@ -303,10 +304,13 @@ class TestCase(DjbletsTestCase): If publish is True, ReviewRequest.publish() will be called. """ - if with_local_site: - local_site = LocalSite.objects.get(name=self.local_site_name) - else: - local_site = None + if not local_site: + if with_local_site: + local_site = LocalSite.objects.get(name=self.local_site_name) + else: + local_site = None + + if not local_site: local_id = None if create_repository: diff --git a/reviewboard/urls.py b/reviewboard/urls.py index 85d5090c3ed58d5762de103bfafbd41ffe89d9f0..ec0d1ce099ac4efc3d63a4eeace515b5007c800b 100644 --- a/reviewboard/urls.py +++ b/reviewboard/urls.py @@ -92,6 +92,7 @@ localsite_urlpatterns = patterns( ) localsite_urlpatterns += datagrid_urlpatterns +localsite_urlpatterns += hostingsvcs_urlpatterns # Main includes @@ -105,4 +106,3 @@ urlpatterns += patterns( ) urlpatterns += localsite_urlpatterns -urlpatterns += hostingsvcs_urlpatterns