Improve performance of Organization endpoint (#395)

* adding org view and orgname index

* renaming primary_name to name for organization

* updating model as well

* remove print statement

* fixing failing e2e

* fix merge conflicts

Author: spopelka-dsac

backend/npdfhir/filters/organization_filter_set.py

@@ -3,7 +3,7 @@
 from django_filters import rest_framework as filters
 
 from ..mappings import addressUseMapping
-from ..models import Organization
+from ..models import OrganizationView
 from ..utils import parse_identifier_query
 
 
@@ -38,7 +38,7 @@ class OrganizationFilterSet(filters.FilterSet):
     )
 
     class Meta:
-        model = Organization
+        model = OrganizationView
         fields = [
             "name",
             "identifier",
@@ -51,13 +51,8 @@ class Meta:
         ]
 
     def filter_name(self, queryset, name, value):
-        return (
-            queryset.annotate(
-                search=SearchVector("organizationtoname__name"),
-            )
-            .filter(search=SearchQuery(value, search_type="plain"))
-            .distinct()
-        )
+        query = SearchQuery(f"{value.upper()}", search_type="websearch")
+        return queryset.filter(organization__organizationtoname__search_vector=query)
 
     def filter_identifier(self, queryset, name, value):
         from uuid import UUID
@@ -68,12 +63,12 @@ def filter_identifier(self, queryset, name, value):
         if system:  # specific identifier search requested
             if system.upper() == "NPI":
                 try:
-                    queries = Q(clinicalorganization__npi__npi=int(identifier_id))
+                    queries = Q(organization__clinicalorganization__npi__npi=int(identifier_id))
                 except (ValueError, TypeError):
                     pass  # TODO: implement validationerror to show users that NPI must be an int
         else:  # general identifier search requested
             try:
-                queries |= Q(clinicalorganization__npi__npi=int(identifier_id))
+                queries |= Q(organization__clinicalorganization__npi__npi=int(identifier_id))
             except (ValueError, TypeError):
                 pass
 
@@ -83,46 +78,52 @@ def filter_identifier(self, queryset, name, value):
             except (ValueError, TypeError):
                 pass
 
-            queries |= Q(clinicalorganization__organizationtootherid__other_id=identifier_id)
+            queries |= Q(
+                organization__clinicalorganization__organizationtootherid__other_id=identifier_id
+            )
 
         return queryset.filter(queries).distinct()
 
     def filter_organization_type(self, queryset, name, value):
         return queryset.annotate(
             search=SearchVector(
-                "clinicalorganization__organizationtotaxonomy__nucc_code__display_name"
+                "organization__clinicalorganization__organizationtotaxonomy__nucc_code__display_name"
             )
         ).filter(search=value)
 
     def filter_address(self, queryset, name, value):
         return queryset.annotate(
             search=SearchVector(
-                "organizationtoaddress__address__address_us__delivery_line_1",
-                "organizationtoaddress__address__address_us__delivery_line_2",
-                "organizationtoaddress__address__address_us__city_name",
-                "organizationtoaddress__address__address_us__state_code__abbreviation",
-                "organizationtoaddress__address__address_us__zipcode",
+                "organization__organizationtoaddress__address__address_us__delivery_line_1",
+                "organization__organizationtoaddress__address__address_us__delivery_line_2",
+                "organization__organizationtoaddress__address__address_us__city_name",
+                "organization__organizationtoaddress__address__address_us__state_code__abbreviation",
+                "organization__organizationtoaddress__address__address_us__zipcode",
             )
         ).filter(search=SearchQuery(value, search_type="websearch"))
 
     def filter_address_city(self, queryset, name, value):
         return queryset.annotate(
-            search=SearchVector("organizationtoaddress__address__address_us__city_name")
+            search=SearchVector(
+                "organization__organizationtoaddress__address__address_us__city_name"
+            )
         ).filter(search=value)
 
     def filter_address_state(self, queryset, name, value):
         return queryset.annotate(
             search=SearchVector(
-                "organizationtoaddress__address__address_us__state_code__abbreviation"
+                "organization__organizationtoaddress__address__address_us__state_code__abbreviation"
             )
         ).filter(search=value)
 
     def filter_address_postalcode(self, queryset, name, value):
-        return queryset.filter(organizationtoaddress__address__address_us__zipcode=value)
+        return queryset.filter(
+            organization__organizationtoaddress__address__address_us__zipcode=value
+        )
 
     def filter_address_use(self, queryset, name, value):
         if value in addressUseMapping.keys():
             value = addressUseMapping.toNPD(value)
         else:
             value = -1
-        return queryset.filter(organizationtoaddress__address_use_id=value)
+        return queryset.filter(organization__organizationtoaddress__address_use_id=value)

backend/npdfhir/management/commands/seedsystem.py

@@ -10,6 +10,8 @@
 from npdfhir.tests.fixtures.organization import create_organization
 from npdfhir.tests.fixtures.practitioner import create_practitioner
 
+from npdfhir.models import OrganizationView
+
 
 class Command(BaseCommand):
     help = "Create test data for end-to-end specs"
@@ -128,4 +130,5 @@ def handle(self, *args, **options):
             self.stdout.write(f"created Endpoint: {self.to_json(id=endpoint.id)}")
 
         self.generate_sample_organizations(25)
+        OrganizationView.refresh_materialized_view()
         self.generate_sample_practitioners(25)

backend/npdfhir/models.py

@@ -1,4 +1,4 @@
-from django.db import models
+from django.db import connection, models
 from django.contrib.gis.db import models as geomodels
 from django.contrib.postgres.search import SearchVectorField
 
@@ -601,11 +601,17 @@ class OrganizationToName(models.Model):
     organization = models.ForeignKey(Organization, models.DO_NOTHING)
     name = models.CharField(max_length=1000)
     is_primary = models.BooleanField(blank=True, null=True)
+    search_vector = SearchVectorField(blank=True, null=True)
 
     class Meta:
         managed = False
         db_table = "organization_to_name"
 
+    def _do_insert(self, manager, using, fields, update_pk, raw):
+        # Prevents the model from attempting to insert values into the generated search_vector field
+        fields = [f for f in fields if f.attname != "search_vector"]
+        return super()._do_insert(manager, using, fields, update_pk, raw)
+
 
 class OrganizationToOtherId(models.Model):
     pk = models.CompositePrimaryKey("npi", "other_id", "other_id_type_id", "issuer", "state_code")
@@ -644,6 +650,26 @@ class Meta:
         db_table = "organization_to_taxonomy"
 
 
+class OrganizationView(models.Model):
+    organization = models.OneToOneField(
+        Organization, models.DO_NOTHING, primary_key=True, db_column="id"
+    )
+    authorized_official = models.ForeignKey(Individual, models.DO_NOTHING, blank=True, null=True)
+    ein = models.ForeignKey(LegalEntity, models.DO_NOTHING, blank=True, null=True)
+    parent = models.ForeignKey("self", models.DO_NOTHING, blank=True, null=True)
+    # the sorting field from organization_to_name
+    name = models.CharField(max_length=1000)
+
+    @classmethod
+    def refresh_materialized_view(cls):
+        with connection.cursor() as cursor:
+            cursor.execute(f"REFRESH MATERIALIZED VIEW {cls._meta.db_table};")
+
+    class Meta:
+        managed = False
+        db_table = "organization_view"
+
+
 class OtherIdType(models.Model):
     value = models.CharField(max_length=50, blank=True, null=True)
 

backend/npdfhir/serializers.py

@@ -311,6 +311,7 @@ class Meta:
 
     def to_representation(self, instance):
         request = self.context.get("request")
+        instance = instance.organization
         representation = super().to_representation(instance)
 
         organization = FHIROrganization()
@@ -417,7 +418,7 @@ def to_representation(self, instance):
             if len(names) > 1:
                 aliases = names[1:]
         if aliases:
-            organization.alias = [n['name'] for n in aliases]
+            organization.alias = [n["name"] for n in aliases]
 
         if instance.parent_id is not None:
             organization.partOf = genReference(

backend/npdfhir/tests/test_organization.py

@@ -1,7 +1,7 @@
 from django.urls import reverse
 from rest_framework import status
 
-from ..models import Organization, OtherIdType
+from ..models import Organization, OtherIdType, OrganizationView
 from .api_test_case import APITestCase
 from .fixtures.organization import create_legal_entity, create_organization
 from .fixtures.location import create_location
@@ -90,13 +90,17 @@ def setUpTestData(cls):
         cls.org_cumberland = create_organization(name="Cumberland")
         cls.orgs.append(cls.org_cumberland)
 
+        OrganizationView.refresh_materialized_view()
+
         return super().setUpTestData()
 
     def setUp(self):
         super().setUp()
         self.org_without_authorized_official = Organization.objects.create(
-            id="26708690-19d6-499e-b481-cebe05b98f08", authorized_official_id=None
+            id="26708690-19d6-499e-b481-cebe05b98f08",
+            authorized_official_id=None,
         )
+        OrganizationView.refresh_materialized_view()
 
     # Basic tests
     def test_list_default(self):
@@ -137,7 +141,7 @@ def test_list_in_default_order(self):
 
     def test_list_in_descending_order(self):
         url = reverse("fhir-organization-list")
-        response = self.client.get(url, {"_sort": "-primary_name"})
+        response = self.client.get(url, {"_sort": "-name"})
         assert_fhir_response(self, response)
 
         # Extract names

backend/npdfhir/tests/test_practitioner_role.py

@@ -13,13 +13,7 @@
     # extract_resource_ids,
 )
 
-from ..models import (
-    OrganizationToName,
-    Provider,
-    Location,
-    IndividualToName,
-    Organization
-)
+from ..models import OrganizationToName, Provider, IndividualToName, Organization, OrganizationView
 
 from .fixtures.location import create_location
 from .fixtures.practitioner import (
@@ -155,6 +149,8 @@ def setUpTestData(cls):
 
         cls.roles_with_params.append(pr)
 
+        OrganizationView.refresh_materialized_view()
+
         return super().setUpTestData()
 
     # Basic tests
@@ -563,17 +559,18 @@ def test_list_filter_by_address_city(self):
         bundle = response.data["results"]
 
         for entry in bundle["entry"]:
-            location_id = entry["resource"]["location"][0]["reference"].split("/")[-1]
             self.assertIn("resource", entry)
             location_entry = entry["resource"]
 
             self.assertEqual(location_entry["resourceType"], "PractitionerRole")
             self.assertIn("id", location_entry)
             self.assertIn("active", location_entry)
 
-            location_obj = Location.objects.get(pk=location_id)
-
-            self.assertEqual(city_search, location_obj.address.address_us.city_name)
+            for entry in bundle["entry"]:
+                self.assertEqual(1, len(entry["resource"]["location"]))
+                location_url = entry["resource"]["location"][0]["reference"]
+                returned_location = self.client.get(location_url).data
+                self.assertEqual(city_search, returned_location["address"]["city"])
 
     def test_list_filter_by_address_state(self):
         state_search = "CA"
@@ -585,17 +582,18 @@ def test_list_filter_by_address_state(self):
         bundle = response.data["results"]
 
         for entry in bundle["entry"]:
-            location_id = entry["resource"]["location"][0]["reference"].split("/")[-1]
             self.assertIn("resource", entry)
             location_entry = entry["resource"]
 
             self.assertEqual(location_entry["resourceType"], "PractitionerRole")
             self.assertIn("id", location_entry)
             self.assertIn("active", location_entry)
 
-            location_obj = Location.objects.get(pk=location_id)
-
-            self.assertEqual(state_search, location_obj.address.address_us.state_code.abbreviation)
+            for entry in bundle["entry"]:
+                self.assertEqual(1, len(entry["resource"]["location"]))
+                location_url = entry["resource"]["location"][0]["reference"]
+                returned_location = self.client.get(location_url).data
+                self.assertEqual(state_search, returned_location["address"]["state"])
 
     def test_list_filter_by_address_zip(self):
         zip_search = "90001"
@@ -607,17 +605,18 @@ def test_list_filter_by_address_zip(self):
         bundle = response.data["results"]
 
         for entry in bundle["entry"]:
-            location_id = entry["resource"]["location"][0]["reference"].split("/")[-1]
             self.assertIn("resource", entry)
             location_entry = entry["resource"]
 
             self.assertEqual(location_entry["resourceType"], "PractitionerRole")
             self.assertIn("id", location_entry)
             self.assertIn("active", location_entry)
 
-            location_obj = Location.objects.get(pk=location_id)
-
-            self.assertEqual(zip_search, location_obj.address.address_us.zipcode)
+            for entry in bundle["entry"]:
+                self.assertEqual(1, len(entry["resource"]["location"]))
+                location_url = entry["resource"]["location"][0]["reference"]
+                returned_location = self.client.get(location_url).data
+                self.assertEqual(zip_search, returned_location["address"]["postalCode"])
 
     def test_list_filter_by_address_zip_leading_zero(self):
         zip_search = "05555"
@@ -629,39 +628,45 @@ def test_list_filter_by_address_zip_leading_zero(self):
         bundle = response.data["results"]
 
         for entry in bundle["entry"]:
-            location_id = entry["resource"]["location"][0]["reference"].split("/")[-1]
             self.assertIn("resource", entry)
             location_entry = entry["resource"]
 
             self.assertEqual(location_entry["resourceType"], "PractitionerRole")
             self.assertIn("id", location_entry)
             self.assertIn("active", location_entry)
 
-            location_obj = Location.objects.get(pk=location_id)
+            for entry in bundle["entry"]:
+                self.assertEqual(1, len(entry["resource"]["location"]))
+                location_url = entry["resource"]["location"][0]["reference"]
+                returned_location = self.client.get(location_url).data
+                self.assertEqual(zip_search, returned_location["address"]["postalCode"])
 
-            self.assertEqual(zip_search, location_obj.address.address_us.zipcode)
-    
     def test_list_filter_by_address_general_zip_leading_zero(self):
-        zip_search = "404 Great Amazing Avenue San Diego CA 05555"
+        address_line_1 = "404 Great Amazing Avenue"
+        city = "San Diego"
+        state = "CA"
+        zip_code = "05555"
+        address_search = " ".join([address_line_1, city, state, zip_code])
         url = reverse("fhir-practitionerrole-list")
-        response = self.client.get(url, {"location_address": zip_search})
+        response = self.client.get(url, {"location_address": address_search})
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         assert_has_results(self, response)
 
         bundle = response.data["results"]
 
         for entry in bundle["entry"]:
-            location_id = entry["resource"]["location"][0]["reference"].split("/")[-1]
             self.assertIn("resource", entry)
             location_entry = entry["resource"]
 
             self.assertEqual(location_entry["resourceType"], "PractitionerRole")
             self.assertIn("id", location_entry)
             self.assertIn("active", location_entry)
 
-            location_obj = Location.objects.get(pk=location_id)
-
-            self.assertEqual(zip_search.split()[-1], location_obj.address.address_us.zipcode)
-            self.assertEqual("404 Great Amazing Avenue", location_obj.address.address_us.delivery_line_1)
-            self.assertEqual("San Diego", location_obj.address.address_us.city_name)
-            self.assertEqual("CA", location_obj.address.address_us.state_code.abbreviation)
+            for entry in bundle["entry"]:
+                self.assertEqual(1, len(entry["resource"]["location"]))
+                location_url = entry["resource"]["location"][0]["reference"]
+                returned_location = self.client.get(location_url).data
+                self.assertEqual(zip_code, returned_location["address"]["postalCode"])
+                self.assertEqual(state, returned_location["address"]["state"])
+                self.assertEqual(city, returned_location["address"]["city"])
+                self.assertIn(address_line_1, returned_location["address"]["line"])