From patchwork Fri Sep 30 16:19:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1684923 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=qv0Nu8bA; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4MfFnQ0pXXz1ypH for ; Sat, 1 Oct 2022 02:20:10 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4MfFnQ0byqz3c7S for ; Sat, 1 Oct 2022 02:20:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=qv0Nu8bA; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=136.175.108.112; helo=mail-108-mta112.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=qv0Nu8bA; dkim-atps=neutral Received: from mail-108-mta112.mxroute.com (mail-108-mta112.mxroute.com [136.175.108.112]) (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 4MfFmx5yb9z3cCY for ; Sat, 1 Oct 2022 02:19:45 +1000 (AEST) Received: from mail-111-mta2.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta112.mxroute.com (ZoneMTA) with ESMTPSA id 1838f3222150002b7a.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Fri, 30 Sep 2022 16:19:30 +0000 X-Zone-Loop: 67c0264af5b244b8b605f8d0dbccb54ec27c0ea75f22 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=x; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: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=NndQUqRVXJDGoxQrP4gwVEvjq8WuPXk5cPQlI0rz/8s=; b=qv0Nu8bAqIXEuoueU+wYZLlrvP hzlknEL7OBk2yA7e6+l+gSeOvGtKl5cQEI5lwkKEnKdj6RI/hDMz9K8z6moVtY1PDI8WK+Ytx9mKA FgGxUjD83qWfL0lcJ7MNwdUrhBIJr5iOSL0Kt6dQ66faf9VRKPfcKy9wF4+j/c2Wx/x81mRP48H02 AIcGbDdoG2BEw9Pable/wp14+vVgJjVvtG3RVyix8W4v0uNdtKRXk2adzp7CEPo9y0TSkDWKBl3yx mIKUn1e2TxJ7ZB70EHPeo0BhMkZpcPpiaBjbsptQA9QHja12d9vJpk8BqE/xjCtNebVFYWL5ZG1IB 9g0xDFVA==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 07/10] REST: De-duplicate handling of nested resource URLs Date: Fri, 30 Sep 2022 17:19:18 +0100 Message-Id: <20220930161921.266633-7-stephen@that.guru> In-Reply-To: <20220930161921.266633-1-stephen@that.guru> References: <20220930161921.266633-1-stephen@that.guru> MIME-Version: 1.0 X-Authenticated-Id: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" These were all doing the same thing. Make things more generic. We also speed up test (inadvertently) by using the 'patch_id' attribute of the 'Check' model rather than 'patch.id', thus avoiding the JOIN. Signed-off-by: Stephen Finucane --- patchwork/api/base.py | 52 +++++++++---------------------- patchwork/api/check.py | 10 ++++-- patchwork/api/embedded.py | 28 +++++++++++++---- patchwork/tests/api/test_event.py | 2 +- 4 files changed, 45 insertions(+), 47 deletions(-) diff --git patchwork/api/base.py patchwork/api/base.py index 3ed4182c..0f5c44a2 100644 --- patchwork/api/base.py +++ patchwork/api/base.py @@ -9,8 +9,8 @@ from django.conf import settings from django.shortcuts import get_object_or_404 from rest_framework import permissions from rest_framework.pagination import PageNumberPagination +from rest_framework.relations import HyperlinkedIdentityField from rest_framework.response import Response -from rest_framework.serializers import HyperlinkedIdentityField from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.utils.urls import replace_query_param @@ -122,52 +122,28 @@ class MultipleFieldLookupMixin(object): return get_object_or_404(queryset, **filter_kwargs) -class CheckHyperlinkedIdentityField(HyperlinkedIdentityField): - def get_url(self, obj, view_name, request, format): - # Unsaved objects will not yet have a valid URL. - if obj.pk is None: - return None - - return self.reverse( - view_name, - kwargs={ - 'patch_id': obj.patch.id, - 'check_id': obj.id, - }, - request=request, - format=format, - ) +class NestedHyperlinkedIdentityField(HyperlinkedIdentityField): + """A variant of HyperlinkedIdentityField that supports nested resources.""" + def __init__(self, view_name, lookup_field_mapping, **kwargs): + self.lookup_field_mapping = lookup_field_mapping + super().__init__(view_name, **kwargs) -class CoverCommentHyperlinkedIdentityField(HyperlinkedIdentityField): def get_url(self, obj, view_name, request, format): # Unsaved objects will not yet have a valid URL. - if obj.pk is None: + if hasattr(obj, 'pk') and obj.pk in (None, ''): return None - return self.reverse( - view_name, - kwargs={ - 'cover_id': obj.cover.id, - 'comment_id': obj.id, - }, - request=request, - format=format, - ) - - -class PatchCommentHyperlinkedIdentityField(HyperlinkedIdentityField): - def get_url(self, obj, view_name, request, format): - # Unsaved objects will not yet have a valid URL. - if obj.pk is None: - return None + kwargs = {} + for ( + lookup_url_kwarg, + lookup_field, + ) in self.lookup_field_mapping.items(): + kwargs[lookup_url_kwarg] = getattr(obj, lookup_field) return self.reverse( view_name, - kwargs={ - 'patch_id': obj.patch.id, - 'comment_id': obj.id, - }, + kwargs=kwargs, request=request, format=format, ) diff --git patchwork/api/check.py patchwork/api/check.py index c28d89f7..f5461fc6 100644 --- patchwork/api/check.py +++ patchwork/api/check.py @@ -14,8 +14,8 @@ from rest_framework.serializers import HiddenField from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.serializers import ValidationError -from patchwork.api.base import CheckHyperlinkedIdentityField from patchwork.api.base import MultipleFieldLookupMixin +from patchwork.api.base import NestedHyperlinkedIdentityField from patchwork.api.base import CurrentPatchDefault from patchwork.api.embedded import UserSerializer from patchwork.api.filters import CheckFilterSet @@ -25,7 +25,13 @@ from patchwork.models import Patch class CheckSerializer(HyperlinkedModelSerializer): - url = CheckHyperlinkedIdentityField('api-check-detail') + url = NestedHyperlinkedIdentityField( + 'api-check-detail', + lookup_field_mapping={ + 'patch_id': 'patch_id', + 'check_id': 'id', + }, + ) patch = HiddenField(default=CurrentPatchDefault()) user = UserSerializer(default=CurrentUserDefault()) diff --git patchwork/api/embedded.py patchwork/api/embedded.py index 485ed6f7..7105da08 100644 --- patchwork/api/embedded.py +++ patchwork/api/embedded.py @@ -16,9 +16,7 @@ from rest_framework.serializers import PrimaryKeyRelatedField from rest_framework.serializers import SerializerMethodField from patchwork.api.base import BaseHyperlinkedModelSerializer -from patchwork.api.base import CheckHyperlinkedIdentityField -from patchwork.api.base import CoverCommentHyperlinkedIdentityField -from patchwork.api.base import PatchCommentHyperlinkedIdentityField +from patchwork.api.base import NestedHyperlinkedIdentityField from patchwork import models @@ -82,7 +80,13 @@ class WebURLMixin(BaseHyperlinkedModelSerializer): class CheckSerializer(SerializedRelatedField): class _Serializer(BaseHyperlinkedModelSerializer): - url = CheckHyperlinkedIdentityField('api-check-detail') + url = NestedHyperlinkedIdentityField( + 'api-check-detail', + lookup_field_mapping={ + 'patch_id': 'patch_id', + 'check_id': 'id', + }, + ) def to_representation(self, instance): data = super(CheckSerializer._Serializer, self).to_representation( @@ -130,7 +134,13 @@ class CoverSerializer(SerializedRelatedField): class CoverCommentSerializer(SerializedRelatedField): class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): - url = CoverCommentHyperlinkedIdentityField('api-cover-comment-detail') + url = NestedHyperlinkedIdentityField( + 'api-cover-comment-detail', + lookup_field_mapping={ + 'cover_id': 'cover_id', + 'comment_id': 'id', + }, + ) class Meta: model = models.CoverComment @@ -182,7 +192,13 @@ class PatchSerializer(SerializedRelatedField): class PatchCommentSerializer(SerializedRelatedField): class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer): - url = PatchCommentHyperlinkedIdentityField('api-patch-comment-detail') + url = NestedHyperlinkedIdentityField( + 'api-patch-comment-detail', + lookup_field_mapping={ + 'patch_id': 'patch_id', + 'comment_id': 'id', + }, + ) class Meta: model = models.PatchComment diff --git patchwork/tests/api/test_event.py patchwork/tests/api/test_event.py index 7ca09c2e..1a0d811d 100644 --- patchwork/tests/api/test_event.py +++ patchwork/tests/api/test_event.py @@ -200,7 +200,7 @@ class TestEventAPI(APITestCase): for _ in range(3): self._create_events() - with self.assertNumQueries(33): + with self.assertNumQueries(30): self.client.get(self.api_url()) def test_order_by_date_default(self):