diff --git a/reviewboard/testing/testcase.py b/reviewboard/testing/testcase.py index 0599b5b7104c60b6146026368b02dfb5d831ca7f..a0ef951f465fe0ade969672fe4cadf115b6f8ac4 100644 --- a/reviewboard/testing/testcase.py +++ b/reviewboard/testing/testcase.py @@ -21,7 +21,8 @@ from reviewboard.accounts.models import ReviewRequestVisit from reviewboard.admin.siteconfig import load_site_config from reviewboard.attachments.models import FileAttachment from reviewboard.diffviewer.differ import DiffCompatVersion -from reviewboard.diffviewer.models import DiffSet, DiffSetHistory, FileDiff +from reviewboard.diffviewer.models import (DiffCommit, DiffSet, DiffSetHistory, + FileDiff) from reviewboard.notifications.models import WebHookTarget from reviewboard.oauth.models import Application from reviewboard.reviews.models import (Comment, @@ -178,6 +179,112 @@ class TestCase(FixturesCompilerMixin, DjbletsTestCase): return file_attachment + def create_diffcommit(self, + repository=None, + diffset=None, + commit_id='r1', + parent_id='r0', + diff_contents=DEFAULT_GIT_FILEDIFF_DATA, + parent_diff_contents=None, + author_name='Author', + author_email='author@example.com', + author_date=None, + commit_message='Commit message', + committer_name='Committer', + committer_email='committer@example.com', + committer_date=None, + **kwargs): + """Create a DiffCommit for testing. + + Args: + repository (reviewboard.scmtools.models.Repository, optional): + The repository the commit is associated with. + + diffset (reviewboard.diffviewer.models.diffset.DiffSet, optional): + The parent diffset. + + commit_id (unicode, optional): + The commit ID. + + parent_id (unicode, optional): + The commit ID of the parent commit. + + diff_contents (bytes, optional): + The contents of the diff. + + parent_diff_contents (bytes, optional): + The contents of the parent diff, if any. + + author_name (unicode, optional): + The name of the commit's author. + + author_email (unicode, optional): + The e-mail address of the commit's author. + + author_date (datetime.datetime, optional): + The date the commit was authored. + + commit_message (unicode, optional): + The commit message. + + committer_name (unicode, optional): + The name of the committer, if any. + + committer_email (unicode, optional): + The e-mail address of the committer, if any. + + committer_date (datetime.datetime, optional): + The date the commit was committed, if any. + + **kwargs (dict): + Keyword arguments to be passed to the + :py:class:`~reviewboard.diffviewer.models.diff_commit.DiffCommit` + initializer. + + Returns: + reviewboard.diffviewer.models.diff_commit.DiffCommit: + The resulting DiffCommit. + """ + assert isinstance(diff_contents, bytes) + + if author_date is None: + author_date = timezone.now() + + if not committer_date and committer_name and committer_email: + committer_date = author_date + + if ((not committer_name and committer_email) or + (committer_name and not committer_email)): + raise ValueError( + 'Either both or neither of committer_name and committer_email ' + 'must be provided.') + + if parent_diff_contents: + assert isinstance(parent_diff_contents, bytes) + parent_diff_file_name = 'parent_diff' + else: + parent_diff_file_name = None + + return DiffCommit.objects.create_from_data( + repository=repository, + diff_file_name='diff', + diff_file_contents=diff_contents, + parent_diff_file_name=parent_diff_file_name, + parent_diff_file_contents=parent_diff_contents, + diffset=diffset, + commit_id=commit_id, + parent_id=parent_id, + author_name=author_name, + author_email=author_email, + author_date=author_date, + commit_message=commit_message, + request=None, + committer_name=committer_name, + committer_email=committer_email, + committer_date=committer_date, + check_existence=False, + **kwargs) + def create_diffset(self, review_request=None, revision=1, repository=None, draft=False, name='diffset'): """Creates a DiffSet for testing. diff --git a/reviewboard/webapi/resources/draft_filediff.py b/reviewboard/webapi/resources/draft_filediff.py index e8fb943669e99e11e605c58a0bc0bbb9635a0a5f..1cd2e75353a5e0db3d87acf5c1fa90df667e9f61 100644 --- a/reviewboard/webapi/resources/draft_filediff.py +++ b/reviewboard/webapi/resources/draft_filediff.py @@ -39,13 +39,28 @@ class DraftFileDiffResource(FileDiffResource): resources.draft_patched_file, ] - def get_queryset(self, request, diff_revision, *args, **kwargs): + def get_diffset_query(self, request, *args, **kwargs): + """Return a QuerySet specific to the review request draft diffs. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + django.db.models.query.QuerySet: + A QuerySet limited to the FileDiffs associated with the review + request draft. + """ draft = resources.review_request_draft.get_object( request, *args, **kwargs) - return self.model.objects.filter( - diffset__review_request_draft=draft, - diffset__revision=diff_revision) + return self.model.objects.filter(diffset__review_request_draft=draft) def has_access_permissions(self, request, filediff, *args, **kwargs): draft = resources.review_request_draft.get_object( diff --git a/reviewboard/webapi/resources/filediff.py b/reviewboard/webapi/resources/filediff.py index d1a1ae1f239fb8b29f77dbb0874341b700b41eb4..fae229d17a72e212a07386965aad50a1b1262aa7 100644 --- a/reviewboard/webapi/resources/filediff.py +++ b/reviewboard/webapi/resources/filediff.py @@ -148,21 +148,87 @@ class FileDiffResource(WebAPIResource): def get_last_modified(self, request, obj, *args, **kwargs): return obj.diffset.timestamp - def get_queryset(self, request, review_request_id, diff_revision, - local_site_name=None, *args, **kwargs): + def get_queryset(self, request, diff_revision, is_list=False, *args, + **kwargs): + """Return a queryset for FileDiff models. + + If the ``commit-id`` query parameter is given, results will be limited + to FileDiffs belonging to that commit. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + diff_revision (unicode): + Limit results to the DiffSet with the given revision. + + is_list (bool, optional): + Whether or not we are computing the queryset for the list + resource. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + django.db.models.query.QuerySet: + The computed queryset. + """ + qs = ( + self.get_diffset_query( + request=request, + is_list=is_list, + *args, + **kwargs) + .filter(diffset__revision=diff_revision) + ) + + if is_list: + commit_id = request.GET.get('commit-id', '').strip() + + if commit_id: + qs = qs.filter(commit__commit_id=commit_id) + + return qs + + def get_diffset_query(self, request, review_request_id, + local_site_name=None, *args, **kwargs): + """Return a QuerySet for all diffs on the review request. + + Args: + request (django.http.HttpRequest): + The HTTP request from the client. + + review_request_id (int): + The review request ID. + + local_site_name (unicode, optional): + The name of the current LocalSite, if any. + + *args (tuple): + Additional positional arguments. + + **kwargs (dict): + Additional keyword arguments. + + Returns: + django.db.models.query.QuerySet: + Return a QuerySet limited to the FileDiffs associated with the + specified review request. + """ if local_site_name: review_request = resources.review_request.get_object( request, review_request_id=review_request_id, - diff_revision=diff_revision, local_site_name=local_site_name, *args, **kwargs) review_request_id = review_request.pk return self.model.objects.filter( - diffset__history__review_request=review_request_id, - diffset__revision=diff_revision) + diffset__history__review_request=review_request_id) def has_access_permissions(self, request, filediff, *args, **kwargs): review_request = resources.review_request.get_object( diff --git a/reviewboard/webapi/tests/test_draft_filediff.py b/reviewboard/webapi/tests/test_draft_filediff.py index de6c4541cd4ee473a10ea63feaab1de9d9974569..29848180fda30c85a4ab4ed43c515a112165ba7e 100644 --- a/reviewboard/webapi/tests/test_draft_filediff.py +++ b/reviewboard/webapi/tests/test_draft_filediff.py @@ -4,6 +4,7 @@ import os from django.utils import six from djblets.webapi.errors import INVALID_FORM_DATA +from djblets.webapi.testing.decorators import webapi_test_template from reviewboard import scmtools from reviewboard.attachments.models import FileAttachment @@ -75,6 +76,61 @@ class ResourceListTests(BaseWebAPITestCase): get_draft_filediff_list_url(diffset, review_request), expected_status=403) + @webapi_test_template + def test_commit_filter(self): + """Testing the GET ?commit-id= API filters draft FileDiffs to the + requested commit + """ + repository = self.create_repository() + review_request = self.create_review_request(repository=repository, + submitter=self.user) + diffset = self.create_diffset(review_request=review_request, + draft=True, + repository=repository) + commit = self.create_diffcommit(diffset=diffset, + repository=repository) + + rsp = self.api_get( + '%s?commit-id=%s' + % (get_draft_filediff_list_url(diffset, review_request), + commit.commit_id), + expected_status=200, + expected_mimetype=filediff_list_mimetype) + + self.assertIn('stat', rsp) + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('files', rsp) + self.assertEqual(rsp['total_results'], 1) + + item_rsp = rsp['files'][0] + filediff = FileDiff.objects.get(pk=item_rsp['id']) + self.compare_item(item_rsp, filediff) + + @webapi_test_template + def test_commit_filter_no_results(self): + """Testing the GET ?commit-id= API with no results""" + repository = self.create_repository() + review_request = self.create_review_request(repository=repository, + submitter=self.user) + diffset = self.create_diffset(review_request=review_request, + draft=True, + repository=repository) + commit = self.create_diffcommit(diffset=diffset, + repository=repository) + + rsp = self.api_get( + '%s?commit-id=%s' + % (get_draft_filediff_list_url(diffset, review_request), + commit.parent_id), + expected_status=200, + expected_mimetype=filediff_list_mimetype) + + self.assertIn('stat', rsp) + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('files', rsp) + self.assertEqual(rsp['files'], []) + self.assertEqual(rsp['total_results'], 0) + @six.add_metaclass(BasicTestsMetaclass) class ResourceItemTests(ExtraDataItemMixin, BaseWebAPITestCase): diff --git a/reviewboard/webapi/tests/test_filediff.py b/reviewboard/webapi/tests/test_filediff.py index 5a74b7df796f2903f94a9b26ac83990930e8ef69..8a034255a0c4324406d4561038d9bb60fd44dd14 100644 --- a/reviewboard/webapi/tests/test_filediff.py +++ b/reviewboard/webapi/tests/test_filediff.py @@ -1,7 +1,9 @@ from __future__ import unicode_literals from django.utils import six +from djblets.webapi.testing.decorators import webapi_test_template +from reviewboard.diffviewer.models import FileDiff from reviewboard.webapi.resources import resources from reviewboard.webapi.tests.base import BaseWebAPITestCase from reviewboard.webapi.tests.mimetypes import (filediff_item_mimetype, @@ -76,6 +78,59 @@ class ResourceListTests(ReviewRequestChildListMixin, BaseWebAPITestCase): return (get_filediff_list_url(diffset, review_request), filediff_list_mimetype) + @webapi_test_template + def test_commit_filter(self): + """Testing the GET ?commit-id= API filters FileDiffs to the + requested commit + """ + repository = self.create_repository() + review_request = self.create_review_request(repository=repository, + submitter=self.user) + diffset = self.create_diffset(review_request=review_request, + repository=repository) + commit = self.create_diffcommit(diffset=diffset, + repository=repository) + + rsp = self.api_get( + '%s?commit-id=%s' + % (get_filediff_list_url(diffset, review_request), + commit.commit_id), + expected_status=200, + expected_mimetype=filediff_list_mimetype) + + self.assertIn('stat', rsp) + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('files', rsp) + self.assertEqual(rsp['total_results'], 1) + + item_rsp = rsp['files'][0] + filediff = FileDiff.objects.get(pk=item_rsp['id']) + self.compare_item(item_rsp, filediff) + + @webapi_test_template + def test_commit_filter_no_results(self): + """Testing the GET ?commit-id= API with no results""" + repository = self.create_repository() + review_request = self.create_review_request(repository=repository, + submitter=self.user) + diffset = self.create_diffset(review_request=review_request, + repository=repository) + commit = self.create_diffcommit(diffset=diffset, + repository=repository) + + rsp = self.api_get( + '%s?commit-id=%s' + % (get_filediff_list_url(diffset, review_request), + commit.parent_id), + expected_status=200, + expected_mimetype=filediff_list_mimetype) + + self.assertIn('stat', rsp) + self.assertEqual(rsp['stat'], 'ok') + self.assertIn('files', rsp) + self.assertEqual(rsp['files'], []) + self.assertEqual(rsp['total_results'], 0) + @six.add_metaclass(BasicTestsMetaclass) class ResourceItemTests(ExtraDataItemMixin, ReviewRequestChildItemMixin,