Cache external source ids (#919)

* release v1.0.4

* Select person_id from patient table instead of person table.

* Add _get_external_source_id() method and cache results

* Add test for new method

* black

* remove unused dependency

phdi/linkage/mpi.py

@@ -1,4 +1,4 @@
-from typing import List, Dict
+from typing import List, Dict, Literal
 from sqlalchemy import Select, and_, func, literal_column, select, text
 from sqlalchemy.dialects.postgresql import aggregate_order_by
 from phdi.linkage.core import BaseMPIConnectorClient
@@ -7,6 +7,7 @@
 from phdi.fhir.utils import extract_value_with_resource_path
 import uuid
 import copy
+from functools import cache
 
 
 class DIBBsMPIConnectorClient(BaseMPIConnectorClient):
@@ -533,18 +534,9 @@ def _insert_external_person_id(
         if person_id is None or external_person_id is None:  # pragma: no cover
             raise ValueError("person_id and external_person_id must be provided.")
 
-        external_source_id_query = select(self.dal.EXTERNAL_SOURCE_TABLE).where(
-            text(
-                f"{self.dal.EXTERNAL_SOURCE_TABLE.name}.external_source_name"
-                + " = 'IRIS'"
-            )
-        )
-        external_source_record = self.dal.select_results(
-            external_source_id_query, False
-        )
-        if len(external_source_record) > 0:
-            external_source_id = external_source_record[0][0]
+        external_source_id = self._get_external_source_id("IRIS")
 
+        if external_source_id is not None:
             query = select(self.dal.EXTERNAL_PERSON_TABLE).where(
                 text(
                     f"{self.dal.EXTERNAL_PERSON_TABLE.name}.external_person_id"
@@ -566,6 +558,30 @@ def _insert_external_person_id(
                     self.dal.EXTERNAL_PERSON_TABLE, [new_external_person_record], False
                 )
 
+    @cache
+    def _get_external_source_id(self, external_source_name: str) -> Literal[str, None]:
+        """
+        Gets the external source id for the external source name provided.
+        :param external_source_name: The external source name.
+        :return: The external source id if found, otherwise None.
+        """
+
+        external_source_id_query = select(self.dal.EXTERNAL_SOURCE_TABLE).where(
+            text(
+                f"{self.dal.EXTERNAL_SOURCE_TABLE.name}.external_source_name"
+                + f" = '{external_source_name}'"
+            )
+        )
+        external_source_record = self.dal.select_results(
+            external_source_id_query, False
+        )
+
+        external_source_id = None
+        if len(external_source_record) > 0:
+            external_source_id = external_source_record[0][0]
+
+        return external_source_id
+
     def _generate_dict_record_from_results(
         self, results_list: List[list]
     ) -> List[dict]:

phdi/linkage/new_tables.ddl

@@ -3,111 +3,98 @@ BEGIN;
 CREATE EXTENSION IF NOT EXISTS "uuid-ossp";
 
 CREATE TABLE IF NOT EXISTS person (
-    person_id   UUID DEFAULT uuid_generate_v4 (),
+    person_id UUID DEFAULT uuid_generate_v4 (),
     PRIMARY KEY (person_id)
 );
 
 CREATE TABLE IF NOT EXISTS patient (
-    patient_id  UUID DEFAULT uuid_generate_v4 (),
-    person_id   UUID,
-    dob         DATE,
-    sex         VARCHAR(25),    
-    race        VARCHAR(100),
-    ethnicity   VARCHAR(100),
+    patient_id UUID DEFAULT uuid_generate_v4 (),
+    person_id UUID,
+    dob DATE,
+    sex VARCHAR(7),
+    race VARCHAR(100),
+    ethnicity VARCHAR(100),
     PRIMARY KEY (patient_id),
-    CONSTRAINT fk_patient_to_person
-        FOREIGN KEY(person_id)
-            REFERENCES person(person_id)
+    CONSTRAINT fk_patient_to_person FOREIGN KEY(person_id) REFERENCES person(person_id)
 );
 
 CREATE TABLE IF NOT EXISTS name (
-    name_id         UUID DEFAULT uuid_generate_v4 (),
-    patient_id      UUID,
-    last_name       VARCHAR(255),
-    type            VARCHAR(100),
+    name_id UUID DEFAULT uuid_generate_v4 (),
+    patient_id UUID,
+    last_name VARCHAR(255),
+    type VARCHAR(100),
     PRIMARY KEY (name_id),
-    CONSTRAINT fk_name_to_patient
-        FOREIGN KEY(patient_id)
-            REFERENCES patient(patient_id)
+    CONSTRAINT fk_name_to_patient FOREIGN KEY(patient_id) REFERENCES patient(patient_id)
 );
 
 CREATE TABLE IF NOT EXISTS given_name (
-    given_name_id       UUID DEFAULT uuid_generate_v4 (), 
-    name_id             UUID,
-    given_name          VARCHAR(255),
-    given_name_index    INTEGER,
+    given_name_id UUID DEFAULT uuid_generate_v4 (),
+    name_id UUID,
+    given_name VARCHAR(255),
+    given_name_index INTEGER,
     PRIMARY KEY (given_name_id),
-    CONSTRAINT fk_given_to_name
-        FOREIGN KEY(name_id)
-            REFERENCES name(name_id)
+    CONSTRAINT fk_given_to_name FOREIGN KEY(name_id) REFERENCES name(name_id)
 );
 
 CREATE TABLE IF NOT EXISTS identifier (
-    identifier_id   UUID DEFAULT uuid_generate_v4 (), 
-    patient_id      UUID,
+    identifier_id UUID DEFAULT uuid_generate_v4 (),
+    patient_id UUID,
     patient_identifier VARCHAR(255),
-    type_code       VARCHAR(255),
-    type_display    VARCHAR(255),
-    type_system     VARCHAR(255),
+    type_code VARCHAR(255),
+    type_display VARCHAR(255),
+    type_system VARCHAR(255),
     PRIMARY KEY (identifier_id),
-    CONSTRAINT fk_ident_to_patient
-        FOREIGN KEY(patient_id)
-            REFERENCES patient(patient_id)
+    CONSTRAINT fk_ident_to_patient FOREIGN KEY(patient_id) REFERENCES patient(patient_id)
 );
 
 CREATE TABLE IF NOT EXISTS phone_number (
-    phone_id   UUID DEFAULT uuid_generate_v4 (),
-    patient_id      UUID,
-    phone_number    VARCHAR(20),
-    type            VARCHAR(100),
-    start_date      TIMESTAMP,
-    end_date        TIMESTAMP,
+    phone_id UUID DEFAULT uuid_generate_v4 (),
+    patient_id UUID,
+    phone_number VARCHAR(20),
+    type VARCHAR(100),
+    start_date TIMESTAMP,
+    end_date TIMESTAMP,
     PRIMARY KEY (phone_id),
-    CONSTRAINT fk_phone_to_patient
-        FOREIGN KEY(patient_id)
-            REFERENCES patient(patient_id)
+    CONSTRAINT fk_phone_to_patient FOREIGN KEY(patient_id) REFERENCES patient(patient_id)
 );
 
 CREATE TABLE IF NOT EXISTS address (
-    address_id      UUID DEFAULT uuid_generate_v4 (),
-    patient_id      UUID,
-    type            VARCHAR(100),
-    line_1          VARCHAR(100),
-    line_2          VARCHAR(100),
-    city            VARCHAR(255),
-    zip_code        VARCHAR(10),
-    state           VARCHAR(100),
-    country         VARCHAR(255),
-    latitude        DECIMAL,
-    longitude       DECIMAL,
-    start_date      TIMESTAMP,
-    end_date        TIMESTAMP,
+    address_id UUID DEFAULT uuid_generate_v4 (),
+    patient_id UUID,
+    type VARCHAR(100),
+    line_1 VARCHAR(100),
+    line_2 VARCHAR(100),
+    city VARCHAR(255),
+    zip_code VARCHAR(10),
+    state VARCHAR(100),
+    country VARCHAR(255),
+    latitude DECIMAL,
+    longitude DECIMAL,
+    start_date TIMESTAMP,
+    end_date TIMESTAMP,
     PRIMARY KEY (address_id),
-    CONSTRAINT fk_addr_to_patient
-        FOREIGN KEY(patient_id)
-            REFERENCES patient(patient_id)
+    CONSTRAINT fk_addr_to_patient FOREIGN KEY(patient_id) REFERENCES patient(patient_id)
 );
 
 CREATE TABLE IF NOT EXISTS external_source (
-    external_source_id          UUID DEFAULT uuid_generate_v4 (),
-    external_source_name        VARCHAR(255),
-    external_source_description VARCHAR(255), 
+    external_source_id UUID DEFAULT uuid_generate_v4 (),
+    external_source_name VARCHAR(255),
+    external_source_description VARCHAR(255),
     PRIMARY KEY (external_source_id)
 );
 
 CREATE TABLE IF NOT EXISTS external_person (
-    external_id         UUID DEFAULT uuid_generate_v4 (),
-    person_id           UUID,
-    external_person_id  VARCHAR(255),
-    external_source_id  UUID,
+    external_id UUID DEFAULT uuid_generate_v4 (),
+    person_id UUID,
+    external_person_id VARCHAR(255),
+    external_source_id UUID,
     PRIMARY KEY (external_id),
-    CONSTRAINT fk_ext_person_to_person
-        FOREIGN KEY(person_id)
-            REFERENCES person(person_id),
-    CONSTRAINT fk_ext_person_to_source
-        FOREIGN KEY(external_source_id)
-            REFERENCES external_source(external_source_id)
+    CONSTRAINT fk_ext_person_to_person FOREIGN KEY(person_id) REFERENCES person(person_id),
+    CONSTRAINT fk_ext_person_to_source FOREIGN KEY(external_source_id) REFERENCES external_source(external_source_id)
 );
 
+COMMIT;
 
-COMMIT;
+INSERT INTO external_source (external_source_id, external_source_name, external_source_description)
+VALUES ('a0eebc99-9c0b-4ef8-bb6d-6bb9bd380b79','IRIS','LACDPH Surveillance System');
+COMMIT;

tests/linkage/test_mpi.py

@@ -5,7 +5,7 @@
 import re
 import pytest
 import uuid
-from sqlalchemy import Select, select, text, insert
+from sqlalchemy import Select, select, text
 from phdi.linkage.mpi import DIBBsMPIConnectorClient
 from phdi.linkage.dal import DataAccessLayer
 
@@ -359,16 +359,6 @@ def test_insert_matched_patient():
     _clean_up(MPI.dal)
 
     MPI = _init_db()
-    # Insert fake IRIS external source id for testing
-    external_source_id = uuid.uuid4()
-    stmt = insert(MPI.dal.EXTERNAL_SOURCE_TABLE).values(
-        external_source_id=external_source_id,
-        external_source_name="IRIS",
-        external_source_description="Fake surveillance system",
-    )
-    with MPI.dal.engine.connect() as conn:
-        result = conn.execute(stmt)
-        conn.commit()
 
     external_person_id = "EXT-1233456"
     result = MPI.insert_matched_patient(
@@ -390,7 +380,7 @@ def test_insert_matched_patient():
     assert len(person_rec) == 2
     assert len(EXTERNAL_PERSON_rec) == 2
     assert EXTERNAL_PERSON_rec[1][2] == external_person_id
-    assert EXTERNAL_PERSON_rec[1][3] == external_source_id
+    assert EXTERNAL_PERSON_rec[1][3].__str__() == "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380b79"
     assert len(patient_rec) == 2
     assert len(name_rec) == 2
     assert len(given_name_rec) == 3
@@ -402,16 +392,6 @@ def test_insert_matched_patient():
 
     # Test for missing external_person_id
     MPI = _init_db()
-    # Insert fake IRIS external source id for testing
-    external_source_id = uuid.uuid4()
-    stmt = insert(MPI.dal.EXTERNAL_SOURCE_TABLE).values(
-        external_source_id=external_source_id,
-        external_source_name="IRIS",
-        external_source_description="Fake surveillance system",
-    )
-    with MPI.dal.engine.connect() as conn:
-        result = conn.execute(stmt)
-        conn.commit()
 
     result = MPI.insert_matched_patient(
         patient_resource=patient_resource,
@@ -678,3 +658,22 @@ def test_get_mpi_records():
         records_for_insert["patient"][0]["patient_id"]
         == records_for_insert["address"][0]["patient_id"]
     )
+
+    _clean_up(MPI.dal)
+
+
+def test_get_external_source_id():
+    MPI = _init_db()
+
+    # Success
+    external_source_id = MPI._get_external_source_id("IRIS")
+    assert isinstance(external_source_id, uuid.UUID)
+
+    # Failure
+    external_source_id = MPI._get_external_source_id("Not a source")
+    assert external_source_id is None
+
+    # Confirm cache is working
+    MPI._get_external_source_id("IRIS")
+    assert MPI._get_external_source_id.cache_info().hits == 1
+    _clean_up(MPI.dal)