From patchwork Mon Aug 23 14:58:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Raxel Gutierrez X-Patchwork-Id: 1519751 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=112.213.38.117; helo=lists.ozlabs.org; envelope-from=patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=vrPPQfLz; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Gtb4H3SC9z9sRf for ; Tue, 24 Aug 2021 00:59:27 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Gtb4H2RM0z2y6F for ; Tue, 24 Aug 2021 00:59:27 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=vrPPQfLz; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=flex--raxel.bounces.google.com (client-ip=2607:f8b0:4864:20::f4a; helo=mail-qv1-xf4a.google.com; envelope-from=3rrcjyqukcbssbyfmhpphmf.dpnqbudixpslmjtut.p0mbct.psh@flex--raxel.bounces.google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20161025 header.b=vrPPQfLz; dkim-atps=neutral Received: from mail-qv1-xf4a.google.com (mail-qv1-xf4a.google.com [IPv6:2607:f8b0:4864:20::f4a]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Gtb3j6PsHz2xlF for ; Tue, 24 Aug 2021 00:58:57 +1000 (AEST) Received: by mail-qv1-xf4a.google.com with SMTP id u8-20020a0cec880000b029035825559ec4so12454809qvo.22 for ; Mon, 23 Aug 2021 07:58:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=YX3B3pJBZ1if66Y8YtqsNze+oUD6adZatt9/s+wqXvw=; b=vrPPQfLz6J9eNC/Ta4b2Ss8cNHhHmveatEnahKNG/26qt1RQ7iD/J8gZSN1UYv+/Ej +dl9+mmgOpZJi95veKHFH+QJOdZ2jWs9s2b8l4Oc5kxZACee4B/8AFaSO5iJ0yJumfMl MEwyHxarg5iKOws5zk/F6Xq/ZFM32xhuTfsWrgTwrTFp+RxRbWuLUDKnyG8jqjWicDyL 1z9Xp/87ShTp0xMHBKirOWggM0R3A1WxcFZvmImogjXwWFz48TTJqp4yVuZuKySOTtN5 shNQnAODWZuQMW1olSsDEYl6FgmWfUVizL9EZI4L062fRoRw7gmkI5B+Dk9dGheORDrz CY6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=YX3B3pJBZ1if66Y8YtqsNze+oUD6adZatt9/s+wqXvw=; b=Mf/FoHUYLl8pr7i0XKTcSTYHjBBgguYGrr8AFaAzH4ucnUxZ/vxJv3wm6yWiLIkP9f 5DExQ98MxS42bhbwgtGl3p+py6cz5uKAtEF0UPBegyJM8wlPplvpB51dsd3j6cCQqALh CHDlnzqEDPvrpVyv6nwRxf6BG202/4Pk0KSC+AjAH8gjFOtj/ZBcywm/llH1blg3oEHt JSBV6G2bWkqJ+8Skcy4hx2W99wfKNbnV9TCJmsdcqtkJRBlYli81SageWd7B6FK3Qi/M o8myeTCp1Qp/eQgsogzCUgZ6YYy+L2onbRKRsxxDVcJTSGIKQDNEaevCjwOiweMSeWOn jvmQ== X-Gm-Message-State: AOAM532WKJvLBX0UDTCtBBX48GOZSHOWX9VwSRzs+yhhXj8cdNXK1/P2 qEOFngZZnHC42ZAKJT0/2vp3hNpVUjzAJNGAePP+4cIjEyTHe3SQW/MgIbUonILtI2xMQEIuaK5 y4Fs26XksH5951yrLLsd0bChqI1SjtdSd/HWg7rsjyhkqoXBG311QkFCt6tfOruh4 X-Google-Smtp-Source: ABdhPJysbO+yrosy4gDMQCdZ9VpUm7rVABHhQtm20jaLPELwTF/0fha0Q5IyMB7OvHh+YdoIodGbwhEvDg== X-Received: from raxel-pw.c.googlers.com ([fda3:e722:ac3:cc00:14:4d90:c0a8:2fda]) (user=raxel job=sendgmr) by 2002:a05:6214:1382:: with SMTP id g2mr33202226qvz.14.1629730734555; Mon, 23 Aug 2021 07:58:54 -0700 (PDT) Date: Mon, 23 Aug 2021 14:58:46 +0000 In-Reply-To: <20210823145847.3944896-1-raxel@google.com> Message-Id: <20210823145847.3944896-4-raxel@google.com> Mime-Version: 1.0 References: <20210823145847.3944896-1-raxel@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [RFC PATCH 3/4] api: modularize patch relations handling From: Raxel Gutierrez To: patchwork@lists.ozlabs.org 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" Move function that handles updates for patch relations for REST API calls outside of DRF serialized so that it can be reused by the patch relations table through form submission. The function is used in an upcoming patch that deals with the manual modification of the patch relations of a given patch. Signed-off-by: Raxel Gutierrez --- patchwork/api/patch.py | 161 +++++++++++++++++++++-------------------- 1 file changed, 84 insertions(+), 77 deletions(-) diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index a97a8820..ff7640b8 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -70,6 +70,87 @@ class PatchConflict(APIException): ) +def check_user_maintains_all(user, patches): + maintains = user.maintainer_projects.all() + if any(s.project not in maintains for s in patches): + detail = ( + 'At least one patch is part of a project you are not ' + 'maintaining.' + ) + raise PermissionDenied(detail=detail) + return True + + +def update_patch_relations(user, patch, data): + # Validation rules + # ---------------- + # + # Permissions: to change a relation: + # for all patches in the relation, current and proposed, + # the user must be maintainer of the patch's project + # Note that this has a ratchet effect: if you add a cross-project + # relation, only you or another maintainer across both projects can + # modify that relationship in _any way_. + # + # Break before Make: a patch must be explicitly removed from a + # relation before being added to another + # + # No Read-Modify-Write for deletion: + # to delete a patch from a relation, clear _its_ related patch, + # don't modify one of the patches that are to remain. + # + # (As a consequence of those two, operations are additive: + # if 1 is in a relation with [1,2,3], then + # patching 1 with related=[2,4] gives related=[1,2,3,4]) + + # Permissions: + # Must be maintainer of: + # - current patch + check_user_maintains_all(user, [patch]) + # - all patches currently in relation + # - all patches proposed to be in relation + errors = [] + related_patches = data.pop('related') + patches = set(related_patches) if related_patches else set() + if patch.related is not None: + patches = patches.union(patch.related.patches.all()) + check_user_maintains_all(user, patches) + + # handle deletion + if not related_patches: + # do not allow relations with a single patch + if patch.related and patch.related.patches.count() == 2: + patch.related.delete() + patch.related = None + patch.save() + return errors + + # break before make + relations = {patch.related for patch in patches if patch.related} + if len(relations) > 1: + return [PatchConflict()] + if relations: + relation = relations.pop() + else: + relation = None + if relation and patch.related is not None: + if patch.related != relation: + return [PatchConflict()] + + # apply + if relation is None: + relation = PatchRelation() + relation.save() + + for rp in patches: + rp.related = relation + rp.save() + patch.related = relation + patch.save() + + return errors + + class PatchListSerializer(BaseHyperlinkedModelSerializer): web_url = SerializerMethodField() @@ -184,87 +265,13 @@ class PatchDetailSerializer(PatchListSerializer): return super(PatchDetailSerializer, self).update( instance, validated_data) - related = validated_data.pop('related') - - # Validation rules - # ---------------- - # - # Permissions: to change a relation: - # for all patches in the relation, current and proposed, - # the user must be maintainer of the patch's project - # Note that this has a ratchet effect: if you add a cross-project - # relation, only you or another maintainer across both projects can - # modify that relationship in _any way_. - # - # Break before Make: a patch must be explicitly removed from a - # relation before being added to another - # - # No Read-Modify-Write for deletion: - # to delete a patch from a relation, clear _its_ related patch, - # don't modify one of the patches that are to remain. - # - # (As a consequence of those two, operations are additive: - # if 1 is in a relation with [1,2,3], then - # patching 1 with related=[2,4] gives related=[1,2,3,4]) - - # Permissions: - # Because we're in a serializer, not a view, this is a bit clunky user = self.context['request'].user.profile - # Must be maintainer of: - # - current patch - self.check_user_maintains_all(user, [instance]) - # - all patches currently in relation - # - all patches proposed to be in relation - patches = set(related['patches']) if related else {} - if instance.related is not None: - patches = patches.union(instance.related.patches.all()) - self.check_user_maintains_all(user, patches) - - # handle deletion - if not related['patches']: - # do not allow relations with a single patch - if instance.related and instance.related.patches.count() == 2: - instance.related.delete() - instance.related = None - return super(PatchDetailSerializer, self).update( - instance, validated_data) - - # break before make - relations = {patch.related for patch in patches if patch.related} - if len(relations) > 1: - raise PatchConflict() - if relations: - relation = relations.pop() - else: - relation = None - if relation and instance.related is not None: - if instance.related != relation: - raise PatchConflict() - - # apply - if relation is None: - relation = PatchRelation() - relation.save() - for patch in patches: - patch.related = relation - patch.save() - instance.related = relation - instance.save() - + errors = update_patch_relations(user, instance, validated_data) + if errors: + raise errors.pop() return super(PatchDetailSerializer, self).update( instance, validated_data) - @staticmethod - def check_user_maintains_all(user, patches): - maintains = user.maintainer_projects.all() - if any(s.project not in maintains for s in patches): - detail = ( - 'At least one patch is part of a project you are not ' - 'maintaining.' - ) - raise PermissionDenied(detail=detail) - return True - class Meta: model = Patch fields = PatchListSerializer.Meta.fields + (