From patchwork Wed Apr 11 16:13:34 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 897273 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 40Lpxy27FTz9s27 for ; Thu, 12 Apr 2018 02:13:58 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Xf9Q6Ogp"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 40Lpxy0Y5QzF1RK for ; Thu, 12 Apr 2018 02:13:58 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Xf9Q6Ogp"; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (mailfrom) smtp.mailfrom=that.guru (client-ip=23.83.214.32; helo=catfish.maple.relay.mailchannels.net; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Xf9Q6Ogp"; dkim-atps=neutral Received: from catfish.maple.relay.mailchannels.net (catfish.maple.relay.mailchannels.net [23.83.214.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 40Lpxs3cYrzF0dY for ; Thu, 12 Apr 2018 02:13:52 +1000 (AEST) X-Sender-Id: 5xi41l16bi|x-authuser|stephen@that.guru Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 9A856282AB2 for ; Wed, 11 Apr 2018 16:13:47 +0000 (UTC) Received: from one.mxroute.com (unknown [100.96.13.37]) (Authenticated sender: 5xi41l16bi) by relay.mailchannels.net (Postfix) with ESMTPA id DDEA42825ED for ; Wed, 11 Apr 2018 16:13:46 +0000 (UTC) X-Sender-Id: 5xi41l16bi|x-authuser|stephen@that.guru Received: from one.mxroute.com (one-outgoing.mxroute.com [100.96.13.1]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.14.1); Wed, 11 Apr 2018 16:13:47 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: 5xi41l16bi|x-authuser|stephen@that.guru X-MailChannels-Auth-Id: 5xi41l16bi X-Abaft-Towering: 4b8f329c35daea47_1523463227396_4231550284 X-MC-Loop-Signature: 1523463227396:1961171533 X-MC-Ingress-Time: 1523463227395 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=HhJtEl07qiNOkayegb/v8mevRpq8baVJOsULOHkRMT4=; b=Xf9Q6Ogp8pQ+B3q1hjtuKHff9p ShSeKwBThPDUjVi5emowE1c6AkmiBqTuRb8pm/iMfx4awVCinidEKofrTHWj2UhdR1u6PUAnhVCPA 8L2EK2GAJ5L5N9riwcwmSoGjFSxdGJQoMcwlV+1H3ySB3xvj5bBbj03QdczVH06VN7IN5ukGSviir 8poOlwWtDDq4oongtXNAos+YxxUCPzMQIe5VXmQ0Nq/f5iOrDMPTGMRIh7iV5nWuehx39S2c6zORx 7yG/vea7Xr6MFwtzsrU7RfJVpgNhA/8bL6A3X/Q15ZV72Wpyd6oIiS7oN1XPCIwrU0tUqN6XjxCjg J2IV6WRg==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 3/7] REST: Use ModelMultipleChoiceField Date: Wed, 11 Apr 2018 17:13:34 +0100 Message-Id: <20180411161338.420-4-stephen@that.guru> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20180411161338.420-1-stephen@that.guru> References: <20180411161338.420-1-stephen@that.guru> X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" We use a modified version of this that allows us to query on multiple fields. Signed-off-by: Stephen Finucane Fixes: #156 --- patchwork/api/filters.py | 103 ++++++++++++--------- patchwork/tests/api/test_patch.py | 15 ++- .../improved-rest-filtering-bf68399270a9b245.yaml | 10 +- 3 files changed, 82 insertions(+), 46 deletions(-) diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py index 030f9af3..b30499d0 100644 --- a/patchwork/api/filters.py +++ b/patchwork/api/filters.py @@ -19,10 +19,11 @@ from django.contrib.auth.models import User from django.core.exceptions import ValidationError +from django.db.models import Q from django_filters import FilterSet from django_filters import IsoDateTimeFilter -from django_filters import ModelChoiceFilter -from django.forms import ModelChoiceField +from django_filters import ModelMultipleChoiceFilter +from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField from patchwork.models import Bundle from patchwork.models import Check @@ -37,79 +38,96 @@ from patchwork.models import State # custom fields, filters -class ModelMultiChoiceField(ModelChoiceField): - - def _get_filters(self, value): - raise NotImplementedError - - def to_python(self, value): - if value in self.empty_values: - return None +class ModelMultipleChoiceField(BaseMultipleChoiceField): + def _get_filter(self, value): try: - filters = {'pk': int(value)} + return 'pk', int(value) except ValueError: - filters = {self.alternate_lookup: value} - + return self.alternate_lookup, value + + def _check_values(self, value): + """ + Given a list of possible PK values, returns a QuerySet of the + corresponding objects. Raises a ValidationError if a given value is + invalid (not a valid PK, not in the queryset, etc.) + """ + # deduplicate given values to avoid creating many querysets or + # requiring the database backend deduplicate efficiently. try: - value = self.queryset.get(**filters) - except (ValueError, TypeError, self.queryset.model.DoesNotExist): - raise ValidationError(self.error_messages['invalid_choice'], - code='invalid_choice') - return value + value = frozenset(value) + except TypeError: + # list of lists isn't hashable, for example + raise ValidationError( + self.error_messages['list'], + code='list', + ) + + q_objects = Q() + + for pk in value: + key, val = self._get_filter(pk) + + try: + # NOTE(stephenfin): In contrast to the Django implementation + # of this, we check to ensure each specified key exists and + # fail if not. If we don't this, we can end up doing nothing + # for the filtering which, to me, seems very confusing + self.queryset.get(**{key: val}) + except (ValueError, TypeError, self.queryset.model.DoesNotExist): + raise ValidationError( + self.error_messages['invalid_pk_value'], + code='invalid_pk_value', + params={'pk': val}, + ) + q_objects |= Q(**{key: val}) -class ProjectChoiceField(ModelMultiChoiceField): + qs = self.queryset.filter(q_objects) + + return qs + + +class ProjectChoiceField(ModelMultipleChoiceField): alternate_lookup = 'linkname__iexact' -class ProjectFilter(ModelChoiceFilter): +class ProjectFilter(ModelMultipleChoiceFilter): field_class = ProjectChoiceField -class PersonChoiceField(ModelMultiChoiceField): +class PersonChoiceField(ModelMultipleChoiceField): alternate_lookup = 'email__iexact' -class PersonFilter(ModelChoiceFilter): +class PersonFilter(ModelMultipleChoiceFilter): field_class = PersonChoiceField -class StateChoiceField(ModelChoiceField): +class StateChoiceField(ModelMultipleChoiceField): - def prepare_value(self, value): - if hasattr(value, '_meta'): - return value.slug - else: - return super(StateChoiceField, self).prepare_value(value) - - def to_python(self, value): - if value in self.empty_values: - return None + def _get_filter(self, value): try: - value = ' '.join(value.split('-')) - value = self.queryset.get(name__iexact=value) - except (ValueError, TypeError, self.queryset.model.DoesNotExist): - raise ValidationError(self.error_messages['invalid_choice'], - code='invalid_choice') - return value + return 'pk', int(value) + except ValueError: + return 'name__iexact', ' '.join(value.split('-')) -class StateFilter(ModelChoiceFilter): +class StateFilter(ModelMultipleChoiceFilter): field_class = StateChoiceField -class UserChoiceField(ModelMultiChoiceField): +class UserChoiceField(ModelMultipleChoiceField): alternate_lookup = 'username__iexact' -class UserFilter(ModelChoiceFilter): +class UserFilter(ModelMultipleChoiceFilter): field_class = UserChoiceField @@ -125,8 +143,7 @@ class TimestampMixin(FilterSet): class ProjectMixin(FilterSet): - project = ProjectFilter(to_field_name='linkname', - queryset=Project.objects.all()) + project = ProjectFilter(queryset=Project.objects.all()) class SeriesFilter(ProjectMixin, TimestampMixin, FilterSet): diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py index 909c1eb4..373ee0d5 100644 --- a/patchwork/tests/api/test_patch.py +++ b/patchwork/tests/api/test_patch.py @@ -71,8 +71,8 @@ class TestPatchAPI(APITestCase): self.assertEqual(0, len(resp.data)) person_obj = create_person(email='test@example.com') - state_obj = create_state(name='Under Review') project_obj = create_project(linkname='myproject') + state_obj = create_state(name='Under Review') patch_obj = create_patch(state=state_obj, project=project_obj, submitter=person_obj) @@ -117,6 +117,19 @@ class TestPatchAPI(APITestCase): 'submitter': 'test@example.org'}) self.assertEqual(0, len(resp.data)) + state_obj_b = create_state(name='New') + create_patch(state=state_obj_b) + state_obj_c = create_state(name='RFC') + create_patch(state=state_obj_c) + + resp = self.client.get(self.api_url()) + self.assertEqual(3, len(resp.data)) + resp = self.client.get(self.api_url(), [('state', 'under-review')]) + self.assertEqual(1, len(resp.data)) + resp = self.client.get(self.api_url(), [('state', 'under-review'), + ('state', 'new')]) + self.assertEqual(2, len(resp.data)) + def test_detail(self): """Validate we can get a specific patch.""" patch = create_patch( diff --git a/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml b/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml index b1d12eb6..170e9621 100644 --- a/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml +++ b/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml @@ -8,9 +8,15 @@ api: $ curl /covers/?submitter=stephen@that.guru - | - Bundles can be filtered by owner and checks by user using username. For - example: + Bundles can be filtered by owner, patches by delegate and checks by user + using username. For example: .. code-block:: shell $ curl /bundles/?owner=stephenfin + - | + Filters can now be specified multiple times. For example: + + .. code-block:: shell + + $ curl /patches/?state=under-review&state=rfc