Skip to content

Commit e760344

Browse files
authored
[PRMP-1550] Improve GetReportByODS lambda performance
1 parent 8eaf18f commit e760344

File tree

2 files changed

+40
-20
lines changed

2 files changed

+40
-20
lines changed

lambdas/services/ods_report_service.py

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from services.base.dynamo_service import DynamoDBService
1515
from services.base.s3_service import S3Service
1616
from utils.audit_logging_setup import LoggingService
17+
from utils.common_query_filters import NotDeleted
1718
from utils.dynamo_query_filter_builder import DynamoQueryFilterBuilder
1819
from utils.lambda_exceptions import OdsReportException
1920
from utils.request_context import request_context
@@ -37,7 +38,7 @@ def get_nhs_numbers_by_ods(
3738
is_upload_to_s3_needed: bool = False,
3839
file_type_output: FileType = FileType.CSV,
3940
):
40-
results = self.scan_table_with_filter(ods_code)
41+
results = self.query_table_by_index(ods_code)
4142
nhs_numbers = {
4243
item.get(DocumentReferenceMetadataFields.NHS_NUMBER.value)
4344
for item in results
@@ -98,27 +99,41 @@ def build_filter_expression(ods_codes: list[str]):
9899
)
99100

100101
def query_table_by_index(self, ods_code: str):
102+
ods_codes = [ods_code]
103+
authorization_user = getattr(request_context, "authorization", {})
104+
if (
105+
authorization_user
106+
and authorization_user.get("repository_role") == RepositoryRole.PCSE.value
107+
):
108+
ods_codes = [
109+
PatientOdsInactiveStatus.SUSPENDED,
110+
PatientOdsInactiveStatus.DECEASED,
111+
]
101112
results = []
102-
103-
response = self.dynamo_service.query_table_by_index(
104-
table_name=self.table_name,
105-
index_name="OdsCodeIndex",
106-
search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value,
107-
search_condition=ods_code,
108-
)
109-
results += response["Items"]
110-
111-
if not response["Items"]:
112-
logger.info("No records found for ODS code {}".format(ods_code))
113-
while "LastEvaluatedKey" in response:
113+
for ods_code in ods_codes:
114114
response = self.dynamo_service.query_table_by_index(
115115
table_name=self.table_name,
116116
index_name="OdsCodeIndex",
117-
exclusive_start_key=response["LastEvaluatedKey"],
118117
search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value,
119118
search_condition=ods_code,
119+
query_filter=NotDeleted,
120120
)
121121
results += response["Items"]
122+
123+
while "LastEvaluatedKey" in response:
124+
response = self.dynamo_service.query_table_by_index(
125+
table_name=self.table_name,
126+
index_name="OdsCodeIndex",
127+
exclusive_start_key=response["LastEvaluatedKey"],
128+
search_key=DocumentReferenceMetadataFields.CURRENT_GP_ODS.value,
129+
search_condition=ods_code,
130+
query_filter=NotDeleted,
131+
)
132+
results += response["Items"]
133+
134+
if not results:
135+
logger.info("No records found for ODS code {}".format(ods_code))
136+
raise OdsReportException(404, LambdaError.NoDataFound)
122137
return results
123138

124139
def create_and_save_ods_report(

lambdas/tests/unit/services/test_ods_report_service.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ def mock_scan_table_with_filter(mocker, ods_report_service):
4141
return mocker.patch.object(ods_report_service, "scan_table_with_filter")
4242

4343

44+
@pytest.fixture
45+
def mock_query_table_by_index(mocker, ods_report_service):
46+
return mocker.patch.object(ods_report_service, "query_table_by_index")
47+
48+
4449
@pytest.fixture
4550
def mock_dynamo_service_scan_table(mocker, ods_report_service):
4651
return mocker.patch.object(ods_report_service.dynamo_service, "scan_whole_table")
@@ -67,32 +72,32 @@ def mock_get_pre_signed_url(mocker, ods_report_service):
6772

6873

6974
def test_get_nhs_numbers_by_ods(
70-
ods_report_service, mock_scan_table_with_filter, mock_create_and_save_ods_report
75+
ods_report_service, mock_query_table_by_index, mock_create_and_save_ods_report
7176
):
72-
mock_scan_table_with_filter.return_value = [
77+
mock_query_table_by_index.return_value = [
7378
{DocumentReferenceMetadataFields.NHS_NUMBER.value: "NHS123"},
7479
{DocumentReferenceMetadataFields.NHS_NUMBER.value: "NHS456"},
7580
]
7681

7782
ods_report_service.get_nhs_numbers_by_ods("ODS123")
7883

79-
mock_scan_table_with_filter.assert_called_once_with("ODS123")
84+
mock_query_table_by_index.assert_called_once_with("ODS123")
8085
mock_create_and_save_ods_report.assert_called_once_with(
8186
"ODS123", {"NHS123", "NHS456"}, False, False, "csv"
8287
)
8388

8489

8590
def test_get_nhs_numbers_by_ods_with_temp_folder(
86-
ods_report_service, mock_scan_table_with_filter, mock_create_and_save_ods_report
91+
ods_report_service, mock_query_table_by_index, mock_create_and_save_ods_report
8792
):
88-
mock_scan_table_with_filter.return_value = [
93+
mock_query_table_by_index.return_value = [
8994
{DocumentReferenceMetadataFields.NHS_NUMBER.value: "NHS123"},
9095
{DocumentReferenceMetadataFields.NHS_NUMBER.value: "NHS456"},
9196
]
9297

9398
ods_report_service.get_nhs_numbers_by_ods("ODS123", is_upload_to_s3_needed=True)
9499

95-
mock_scan_table_with_filter.assert_called_once_with("ODS123")
100+
mock_query_table_by_index.assert_called_once_with("ODS123")
96101
mock_create_and_save_ods_report.assert_called_once_with(
97102
"ODS123", {"NHS123", "NHS456"}, False, True, "csv"
98103
)

0 commit comments

Comments
 (0)