diff --git a/reviewboard/extensions/tests.py b/reviewboard/extensions/tests.py index 4bdb29b0eb17a0e6b0389d9153c680743f198a69..b56e8d4dd08908d7d8e9361fc93c99260232825c 100644 --- a/reviewboard/extensions/tests.py +++ b/reviewboard/extensions/tests.py @@ -37,6 +37,7 @@ from reviewboard.extensions.hooks import (AdminWidgetHook, WebAPICapabilitiesHook) from reviewboard.hostingsvcs.service import (get_hosting_service, HostingService) +from reviewboard.scmtools.errors import FileNotFoundError from reviewboard.testing.testcase import TestCase from reviewboard.reviews.models.review_request import ReviewRequest from reviewboard.reviews.fields import (BaseReviewRequestField, @@ -368,6 +369,73 @@ class HookTests(TestCase): class TestService(HostingService): name = 'test-service' + def get_file(self, repository, path, revision, *args, **kwargs): + """Return the specified file from the repository. + + If the given file path is ``/invalid-path``, the file will be assumed + to not exist and + :py:exc:`reviewboard.scmtools.errors.FileNotFoundError` will be raised. + + Args: + repository (reviewboard.scmtools.models.Repository): + The repository the file belongs to. + + path (unicode): + The file path. + + revision (unicode): + The file revision. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + unicode: The file data. + + Raises: + reviewboard.scmtools.errors.FileNotFoundError: + Raised if the file does not exist. + """ + if path == '/invalid-path': + raise FileNotFoundError(path, revision) + + return super(TestService, self).get_file(repository, path, revision, + *args, **kwargs) + + def get_file_exists(self, repository, path, revision, *args, **kwargs): + """Return the specified file from the repository. + + If the given file path is ``/invalid-path``, the file will + be assumed to not exist. + + Args: + repository (reviewboard.scmtools.models.Repository): + The repository the file belongs to. + + path (unicode): + The file path. + + revision (unicode): + The file revision. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + bool: Whether or not the file exists. + """ + if path == '/invalid-path': + return False + + return super(TestService, self).get_file_exists( + repository, path, revision, *args, **kwargs) + class HostingServiceHookTests(TestCase): """Testing HostingServiceHook.""" diff --git a/reviewboard/reviews/tests/test_views.py b/reviewboard/reviews/tests/test_views.py index 8176a0ab369c0a7e7f44e36364a8b7c4b6312d3d..d448751ed4165428008c433de6bcbb28955be8cb 100644 --- a/reviewboard/reviews/tests/test_views.py +++ b/reviewboard/reviews/tests/test_views.py @@ -11,6 +11,10 @@ from django.core.urlresolvers import reverse from djblets.siteconfig.models import SiteConfiguration from reviewboard.attachments.models import FileAttachment +from reviewboard.extensions.tests import TestService +from reviewboard.hostingsvcs.service import (register_hosting_service, + unregister_hosting_service) +from reviewboard.hostingsvcs.models import HostingServiceAccount from reviewboard.reviews.models import (Comment, Review, ReviewRequest, ReviewRequestDraft, Screenshot) from reviewboard.site.urlresolvers import local_site_reverse @@ -1137,6 +1141,64 @@ class ViewTests(TestCase): return None +class DownloadFileTests(TestCase): + """Tests for the download_*_file views.""" + + fixtures = ['test_users', 'test_scmtools'] + + @classmethod + def setUpClass(cls): + super(DownloadFileTests, cls).setUpClass() + + register_hosting_service(TestService.name, TestService) + + @classmethod + def tearDownClass(cls): + super(DownloadFileTests, cls).tearDownClass() + + unregister_hosting_service(TestService.name) + + def setUp(self): + super(DownloadFileTests, self).setUp() + + self.account = HostingServiceAccount.objects.create( + service_name=TestService.name, + hosting_url='http://example.com/', + username='foo') + + self.repository = self.create_repository(hosting_account=self.account) + self.review_request = self.create_review_request( + repository=self.repository, publish=True) + self.diffset = self.create_diffset(review_request=self.review_request) + self.filediff = self.create_filediff(self.diffset, + source_file='/invalid-path', + dest_file='/invalid-path') + + def testing_download_orig_file_404(self): + """Testing download_orig_file when the file cannot be found upstream""" + rsp = self.client.get( + local_site_reverse('download-orig-file', kwargs={ + 'review_request_id': self.review_request.display_id, + 'revision': self.diffset.revision, + 'filediff_id': self.filediff.pk, + })) + + self.assertEquals(rsp.status_code, 404) + + def testing_download_modified_file_404(self): + """Testing download_modified_file when the file cannot be found + upstream + """ + rsp = self.client.get( + local_site_reverse('download-modified-file', kwargs={ + 'review_request_id': self.review_request.display_id, + 'revision': self.diffset.revision, + 'filediff_id': self.filediff.pk, + })) + + self.assertEquals(rsp.status_code, 404) + + class UserInfoboxTests(TestCase): def test_unicode(self): """Testing user_infobox with a user with non-ascii characters""" diff --git a/reviewboard/reviews/views.py b/reviewboard/reviews/views.py index 6c2d98757112d72c8671e13ce61b5149e6995a96..ce9462dcf7f7772f3ae35d312baf20d97c1910b6 100644 --- a/reviewboard/reviews/views.py +++ b/reviewboard/reviews/views.py @@ -59,6 +59,7 @@ from reviewboard.reviews.models import (BaseComment, Comment, ReviewRequest, Review, Screenshot, ScreenshotComment) from reviewboard.reviews.ui.base import FileAttachmentReviewUI +from reviewboard.scmtools.errors import FileNotFoundError from reviewboard.scmtools.models import Repository from reviewboard.site.decorators import check_local_site_access from reviewboard.site.urlresolvers import local_site_reverse @@ -1745,7 +1746,14 @@ def _download_diff_file(modified, request, review_request_id, revision, diffset = _query_for_diff(review_request, request.user, revision, draft) filediff = get_object_or_404(diffset.files, pk=filediff_id) encoding_list = diffset.repository.get_encoding_list() - data = get_original_file(filediff, request, encoding_list) + + try: + data = get_original_file(filediff, request, encoding_list) + except FileNotFoundError: + logging.exception( + 'Could not retrieve file "%s" (revision %s) for filediff ID %s', + filediff.dest_detail, revision, filediff_id) + raise Http404 if modified: data = get_patched_file(data, filediff, request)