From fa5690d71ff034a89a6cf222ef7bc4ca6d80fcfb Mon Sep 17 00:00:00 2001 From: Arsenii Kharlanow Date: Mon, 5 Aug 2024 19:51:22 +0200 Subject: [PATCH 1/5] Reduce DB updates by traits without changes --- api/environments/identities/traits/views.py | 62 +++++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index 201512643e65..2067583bbec2 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -245,39 +245,65 @@ def bulk_create(self, request): if not request.environment.trait_persistence_allowed(request): raise BadRequest("Unable to set traits with client key.") - # endpoint allows users to delete existing traits by sending null values - # for the trait value so we need to filter those out here + identities = {trait["identity"]["identifier"] for trait in request.data} + + existing_traits = Trait.objects.filter( + identity__identifier__in=identities, + identity__environment=request.environment, + ) + + # Map to easily access existing traits + existing_traits_map = { + (trait.identity.identifier, trait.trait_key): trait + for trait in existing_traits + } + traits = [] delete_filter_query = Q() for trait in request.data: + trait_key = trait.get("trait_key") + identifier = trait["identity"]["identifier"] + if trait.get("trait_value") is None: delete_filter_query = delete_filter_query | Q( - trait_key=trait.get("trait_key"), - identity__identifier=trait["identity"]["identifier"], + trait_key=trait_key, + identity__identifier=identifier, identity__environment=request.environment, ) - else: + continue + + existing_trait = existing_traits_map.get((identifier, trait_key)) + if ( + not existing_trait + or existing_trait.trait_value != trait["trait_value"] + ): traits.append(trait) if delete_filter_query: Trait.objects.filter(delete_filter_query).delete() - serializer = self.get_serializer(data=traits, many=True) - serializer.is_valid(raise_exception=True) - serializer.save() - - if settings.EDGE_API_URL and request.environment.project.enable_dynamo_db: - forward_trait_requests.delay( - args=( - request.method, - dict(request.headers), - request.environment.project.id, - request.data, + if len(traits) > 0: + serializer = self.get_serializer(data=traits, many=True) + serializer.is_valid(raise_exception=True) + serializer.save() + + if ( + settings.EDGE_API_URL + and request.environment.project.enable_dynamo_db + ): + forward_trait_requests.delay( + args=( + request.method, + dict(request.headers), + request.environment.project.id, + traits, + ) ) - ) - return Response(serializer.data, status=200) + return Response(serializer.data if traits else [], status=200) + + return Response(traits, status=200) except (TypeError, AttributeError) as excinfo: logger.error("Invalid request data: %s" % str(excinfo)) From 1cc7a2ba237eb7213bd345167c0296471dd6c737 Mon Sep 17 00:00:00 2001 From: Arsenii Kharlanow Date: Fri, 9 Aug 2024 15:48:27 +0200 Subject: [PATCH 2/5] Refactoring of update traits logic --- api/environments/identities/traits/views.py | 119 +++++++++++--------- 1 file changed, 66 insertions(+), 53 deletions(-) diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index 2067583bbec2..b4fa9aaab551 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -238,6 +238,58 @@ def increment_value(self, request): return Response(serializer.data, status=200) + def _update_traits(self, request): + + identities = {trait["identity"]["identifier"] for trait in request.data} + + existing_traits = Trait.objects.filter( + identity__identifier__in=identities, + identity__environment=request.environment, + ) + + # Map to easily access existing traits + existing_traits_map = { + (trait.identity.identifier, trait.trait_key): trait + for trait in existing_traits + } + + updated_traits = [] + delete_filter_query = Q() + + for trait in request.data: + trait_key = trait.get("trait_key") + identifier = trait["identity"]["identifier"] + + if trait.get("trait_value") is None: + delete_filter_query = delete_filter_query | Q( + trait_key=trait_key, + identity__identifier=identifier, + identity__environment=request.environment, + ) + continue + + existing_trait = existing_traits_map.get((identifier, trait_key)) + if not existing_trait or existing_trait.trait_value != trait["trait_value"]: + updated_traits.append(trait) + + if delete_filter_query: + Trait.objects.filter(delete_filter_query).delete() + + if len(updated_traits) > 0: + serializer = self.get_serializer(data=updated_traits, many=True) + serializer.is_valid(raise_exception=True) + serializer.save() + + if settings.EDGE_API_URL and request.environment.project.enable_dynamo_db: + forward_trait_requests.delay( + args=( + request.method, + dict(request.headers), + request.environment.project.id, + updated_traits, + ) + ) + @swagger_auto_schema(request_body=SDKCreateUpdateTraitSerializer(many=True)) @action(detail=False, methods=["PUT"], url_path="bulk") def bulk_create(self, request): @@ -245,65 +297,26 @@ def bulk_create(self, request): if not request.environment.trait_persistence_allowed(request): raise BadRequest("Unable to set traits with client key.") + # endpoint allows users to delete existing traits by sending null values + # for the trait value so we need to filter those out here + + self._update_traits(request) + identities = {trait["identity"]["identifier"] for trait in request.data} - existing_traits = Trait.objects.filter( + all_traits = Trait.objects.filter( identity__identifier__in=identities, identity__environment=request.environment, ) - # Map to easily access existing traits - existing_traits_map = { - (trait.identity.identifier, trait.trait_key): trait - for trait in existing_traits - } - - traits = [] - delete_filter_query = Q() - - for trait in request.data: - trait_key = trait.get("trait_key") - identifier = trait["identity"]["identifier"] - - if trait.get("trait_value") is None: - delete_filter_query = delete_filter_query | Q( - trait_key=trait_key, - identity__identifier=identifier, - identity__environment=request.environment, - ) - continue - - existing_trait = existing_traits_map.get((identifier, trait_key)) - if ( - not existing_trait - or existing_trait.trait_value != trait["trait_value"] - ): - traits.append(trait) - - if delete_filter_query: - Trait.objects.filter(delete_filter_query).delete() - - if len(traits) > 0: - serializer = self.get_serializer(data=traits, many=True) - serializer.is_valid(raise_exception=True) - serializer.save() - - if ( - settings.EDGE_API_URL - and request.environment.project.enable_dynamo_db - ): - forward_trait_requests.delay( - args=( - request.method, - dict(request.headers), - request.environment.project.id, - traits, - ) - ) - - return Response(serializer.data if traits else [], status=200) - - return Response(traits, status=200) + return Response( + [ + *SDKCreateUpdateTraitSerializer( + instance=all_traits, many=True + ).data, + ], + status=status.HTTP_200_OK, + ) except (TypeError, AttributeError) as excinfo: logger.error("Invalid request data: %s" % str(excinfo)) From e30e08a62fdb935f8c0ee9b6bd5a58abb323bbeb Mon Sep 17 00:00:00 2001 From: Arsenii Kharlanow Date: Mon, 26 Aug 2024 15:57:52 +0200 Subject: [PATCH 3/5] Refactoring after code review --- api/environments/identities/traits/views.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index b4fa9aaab551..e335793843d2 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -260,6 +260,8 @@ def _update_traits(self, request): trait_key = trait.get("trait_key") identifier = trait["identity"]["identifier"] + # endpoint allows users to delete existing traits by sending null values + # for the trait value so we need to filter those out here if trait.get("trait_value") is None: delete_filter_query = delete_filter_query | Q( trait_key=trait_key, @@ -297,9 +299,6 @@ def bulk_create(self, request): if not request.environment.trait_persistence_allowed(request): raise BadRequest("Unable to set traits with client key.") - # endpoint allows users to delete existing traits by sending null values - # for the trait value so we need to filter those out here - self._update_traits(request) identities = {trait["identity"]["identifier"] for trait in request.data} @@ -310,12 +309,7 @@ def bulk_create(self, request): ) return Response( - [ - *SDKCreateUpdateTraitSerializer( - instance=all_traits, many=True - ).data, - ], - status=status.HTTP_200_OK, + SDKCreateUpdateTraitSerializer(instance=all_traits, many=True).data, ) except (TypeError, AttributeError) as excinfo: From 62f1695bb88e7f93ba6cfe638765d2e0a634d58c Mon Sep 17 00:00:00 2001 From: Arsenii Kharlanow Date: Sat, 21 Sep 2024 00:57:51 +0200 Subject: [PATCH 4/5] Refactoring after code review --- api/environments/identities/traits/views.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index e335793843d2..c3c54e79d3a5 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -1,8 +1,11 @@ +from typing import Set, Any + from django.conf import settings from django.core.exceptions import BadRequest from django.db.models import Q from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema +from rest_framework.request import Request from rest_framework import mixins, status, viewsets from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated @@ -238,7 +241,7 @@ def increment_value(self, request): return Response(serializer.data, status=200) - def _update_traits(self, request): + def _update_traits(self, request: Request) -> Set[str]: identities = {trait["identity"]["identifier"] for trait in request.data} @@ -292,6 +295,8 @@ def _update_traits(self, request): ) ) + return identities + @swagger_auto_schema(request_body=SDKCreateUpdateTraitSerializer(many=True)) @action(detail=False, methods=["PUT"], url_path="bulk") def bulk_create(self, request): @@ -299,9 +304,7 @@ def bulk_create(self, request): if not request.environment.trait_persistence_allowed(request): raise BadRequest("Unable to set traits with client key.") - self._update_traits(request) - - identities = {trait["identity"]["identifier"] for trait in request.data} + identities = self._update_traits(request) all_traits = Trait.objects.filter( identity__identifier__in=identities, From 39d82cd5e4ab2570450e0c4afb990cd308ac7828 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 22:58:32 +0000 Subject: [PATCH 5/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/environments/identities/traits/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index c3c54e79d3a5..a295c260ac1d 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -1,14 +1,14 @@ -from typing import Set, Any +from typing import Any, Set from django.conf import settings from django.core.exceptions import BadRequest from django.db.models import Q from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema -from rest_framework.request import Request from rest_framework import mixins, status, viewsets from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated +from rest_framework.request import Request from rest_framework.response import Response from edge_api.identities.edge_request_forwarder import (