From 4c0dc6e5a1850e7a6a20826cb376666d511f03f7 Mon Sep 17 00:00:00 2001 From: ankur prabhu Date: Tue, 16 Apr 2024 00:42:27 +0530 Subject: [PATCH 1/3] optimizing query --- care/facility/api/serializers/facility.py | 8 ++------ care/facility/api/viewsets/facility.py | 18 ++++++++++++++++-- care/facility/tests/test_facility_api.py | 23 +++++++++++++++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/care/facility/api/serializers/facility.py b/care/facility/api/serializers/facility.py index 73779bdfd9..d7d9d2fb45 100644 --- a/care/facility/api/serializers/facility.py +++ b/care/facility/api/serializers/facility.py @@ -3,9 +3,7 @@ from rest_framework import serializers from care.facility.models import FACILITY_TYPES, Facility, FacilityLocalGovtBody -from care.facility.models.bed import Bed from care.facility.models.facility import FEATURE_CHOICES -from care.facility.models.patient import PatientRegistration from care.users.api.serializers.lsg import ( DistrictSerializer, LocalBodySerializer, @@ -52,12 +50,10 @@ class FacilityBasicInfoSerializer(serializers.ModelSerializer): bed_count = serializers.SerializerMethodField() def get_bed_count(self, facility): - return Bed.objects.filter(facility=facility).count() + return facility.bed_set.count() def get_patient_count(self, facility): - return PatientRegistration.objects.filter( - facility=facility, is_active=True - ).count() + return facility.patientregistration_set.count() def get_facility_type(self, facility): return { diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index 93b7e4bd2e..afb1a0a0ef 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -71,8 +71,15 @@ class FacilityViewSet( ): """Viewset for facility CRUD operations.""" - queryset = Facility.objects.all().select_related( - "ward", "local_body", "district", "state" + queryset = ( + Facility.objects.all() + .select_related( + "ward", + "local_body", + "district", + "state", + ) + .prefetch_related("patientregistration_set", "bed_set") ) permission_classes = ( IsAuthenticated, @@ -148,6 +155,13 @@ def list(self, request, *args, **kwargs): queryset, field_header_map=mapping, field_serializer_map=pretty_mapping ) + # for facility in self.get_queryset(): + # print(facility.__dict__) + # facility.patient_count = facility.patientregistration_set.filter( + # is_active=True + # ).count() + # facility.bed_count = facility.bed_set.count() + return super(FacilityViewSet, self).list(request, *args, **kwargs) @extend_schema(tags=["facility"]) diff --git a/care/facility/tests/test_facility_api.py b/care/facility/tests/test_facility_api.py index 60cdb08c36..29e12389ff 100644 --- a/care/facility/tests/test_facility_api.py +++ b/care/facility/tests/test_facility_api.py @@ -91,3 +91,26 @@ def test_delete_with_active_patients(self): self.client.force_authenticate(user=state_admin) response = self.client.delete(f"/api/v1/facility/{facility.external_id}/") self.assertIs(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_listing_db_queries(self): + """This test case is to test the get api endpoint for facility and to ensure that only one db query is made for fetching the list of facilities""" + facility_1 = self.create_facility( + self.super_user, self.district, self.local_body + ) + facility_2 = self.create_facility( + self.super_user, self.district, self.local_body + ) + facility_3 = self.create_facility( + self.super_user, self.district, self.local_body + ) + self.create_patient(self.district, facility_1) + self.create_patient(self.district, facility_2) + self.create_patient(self.district, facility_3) + + self.client.force_authenticate(user=self.super_user) + + # 3 for auth and 1 for fetching facilities + with self.assertNumQueries(4): + response = self.client.get("/api/v1/facility/") + + self.assertIs(response.status_code, status.HTTP_200_OK) From abab8d1e41d335461739b710f80c3e123d0be3ca Mon Sep 17 00:00:00 2001 From: ankur prabhu Date: Tue, 16 Apr 2024 00:46:13 +0530 Subject: [PATCH 2/3] optimizing query --- care/facility/api/viewsets/facility.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index afb1a0a0ef..38dc5071e2 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -155,13 +155,6 @@ def list(self, request, *args, **kwargs): queryset, field_header_map=mapping, field_serializer_map=pretty_mapping ) - # for facility in self.get_queryset(): - # print(facility.__dict__) - # facility.patient_count = facility.patientregistration_set.filter( - # is_active=True - # ).count() - # facility.bed_count = facility.bed_set.count() - return super(FacilityViewSet, self).list(request, *args, **kwargs) @extend_schema(tags=["facility"]) From 82c1582c80d45ace039b95f7bf4bf5682adff44c Mon Sep 17 00:00:00 2001 From: ankur prabhu Date: Sat, 20 Apr 2024 18:19:13 +0530 Subject: [PATCH 3/3] adiing suggested changes --- care/facility/api/serializers/facility.py | 12 ++++++++++-- care/facility/api/viewsets/facility.py | 22 +++++++++++++++++++++- care/facility/tests/test_facility_api.py | 3 +-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/care/facility/api/serializers/facility.py b/care/facility/api/serializers/facility.py index d7d9d2fb45..9ca652a7d1 100644 --- a/care/facility/api/serializers/facility.py +++ b/care/facility/api/serializers/facility.py @@ -3,7 +3,9 @@ from rest_framework import serializers from care.facility.models import FACILITY_TYPES, Facility, FacilityLocalGovtBody +from care.facility.models.bed import Bed from care.facility.models.facility import FEATURE_CHOICES +from care.facility.models.patient import PatientRegistration from care.users.api.serializers.lsg import ( DistrictSerializer, LocalBodySerializer, @@ -50,10 +52,16 @@ class FacilityBasicInfoSerializer(serializers.ModelSerializer): bed_count = serializers.SerializerMethodField() def get_bed_count(self, facility): - return facility.bed_set.count() + if hasattr(facility, "bed_count"): + return facility.bed_count + return Bed.objects.filter(facility=facility).count() def get_patient_count(self, facility): - return facility.patientregistration_set.count() + if hasattr(facility, "patientregistrations_count"): + return facility.patientregistrations_count + return PatientRegistration.objects.filter( + facility=facility, is_active=True + ).count() def get_facility_type(self, facility): return { diff --git a/care/facility/api/viewsets/facility.py b/care/facility/api/viewsets/facility.py index 38dc5071e2..67d6614bc6 100644 --- a/care/facility/api/viewsets/facility.py +++ b/care/facility/api/viewsets/facility.py @@ -1,4 +1,6 @@ from django.conf import settings +from django.db.models import Count, OuterRef, Subquery +from django.db.models.functions import Coalesce from django_filters import rest_framework as filters from djqscsv import render_to_csv_response from drf_spectacular.utils import extend_schema, extend_schema_view @@ -21,7 +23,9 @@ FacilityPatientStatsHistory, HospitalDoctors, ) +from care.facility.models.bed import Bed from care.facility.models.facility import FacilityUser +from care.facility.models.patient import PatientRegistration from care.users.models import User @@ -71,6 +75,19 @@ class FacilityViewSet( ): """Viewset for facility CRUD operations.""" + patient_registration_count_subquery = Subquery( + PatientRegistration.objects.filter(facility=OuterRef("pk")) + .values("facility") + .annotate(count=Count("id")) + .values("count") + ) + bed_count_subquery = Subquery( + Bed.objects.filter(facility=OuterRef("pk")) + .values("facility") + .annotate(count=Count("id")) + .values("count") + ) + queryset = ( Facility.objects.all() .select_related( @@ -79,7 +96,10 @@ class FacilityViewSet( "district", "state", ) - .prefetch_related("patientregistration_set", "bed_set") + .annotate( + patientregistrations_count=Coalesce(patient_registration_count_subquery, 0) + ) + .annotate(bed_count=Coalesce(bed_count_subquery, 0)) ) permission_classes = ( IsAuthenticated, diff --git a/care/facility/tests/test_facility_api.py b/care/facility/tests/test_facility_api.py index 29e12389ff..e6ae3c8777 100644 --- a/care/facility/tests/test_facility_api.py +++ b/care/facility/tests/test_facility_api.py @@ -109,8 +109,7 @@ def test_listing_db_queries(self): self.client.force_authenticate(user=self.super_user) - # 3 for auth and 1 for fetching facilities - with self.assertNumQueries(4): + with self.assertNumQueries(2): response = self.client.get("/api/v1/facility/") self.assertIs(response.status_code, status.HTTP_200_OK)