From 35d58c13dd73117a6ebbdedb7709bd8490d772dc Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:10:45 +0000 Subject: [PATCH 1/3] eli 620 - reading consumer_mapping_config.json by name --- .../config/constants.py | 1 + .../repos/consumer_mapping_repo.py | 30 ++++++++++++------- .../unit/repos/test_consumer_mapping_repo.py | 6 +--- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/eligibility_signposting_api/config/constants.py b/src/eligibility_signposting_api/config/constants.py index 6deb0b4ec..947d50cb0 100644 --- a/src/eligibility_signposting_api/config/constants.py +++ b/src/eligibility_signposting_api/config/constants.py @@ -5,3 +5,4 @@ NHS_NUMBER_HEADER = "nhs-login-nhs-number" CONSUMER_ID = "nhsd-application-id" # "Nhsd-Application-Id" ALLOWED_CONDITIONS = Literal["COVID", "FLU", "MMR", "RSV"] +CONSUMER_MAPPING_FILE_NAME = "consumer_mapping_config.json" diff --git a/src/eligibility_signposting_api/repos/consumer_mapping_repo.py b/src/eligibility_signposting_api/repos/consumer_mapping_repo.py index 122291aac..83c50d3d5 100644 --- a/src/eligibility_signposting_api/repos/consumer_mapping_repo.py +++ b/src/eligibility_signposting_api/repos/consumer_mapping_repo.py @@ -1,12 +1,17 @@ import json +import logging from typing import Annotated, NewType from botocore.client import BaseClient +from botocore.exceptions import ClientError from wireup import Inject, service +from eligibility_signposting_api.config.constants import CONSUMER_MAPPING_FILE_NAME from eligibility_signposting_api.model.campaign_config import CampaignID from eligibility_signposting_api.model.consumer_mapping import ConsumerId, ConsumerMapping +logger = logging.getLogger(__name__) + BucketName = NewType("BucketName", str) @@ -24,18 +29,21 @@ def __init__( self.bucket_name = bucket_name def get_permitted_campaign_ids(self, consumer_id: ConsumerId) -> list[CampaignID] | None: - objects = self.s3_client.list_objects(Bucket=self.bucket_name).get("Contents") - - if not objects: - return None + try: + response = self.s3_client.get_object(Bucket=self.bucket_name, Key=CONSUMER_MAPPING_FILE_NAME) + body = response["Body"].read() - consumer_mappings_obj = objects[0] - response = self.s3_client.get_object(Bucket=self.bucket_name, Key=consumer_mappings_obj["Key"]) - body = response["Body"].read() + mapping_result = ConsumerMapping.model_validate(json.loads(body)).get(consumer_id) - mapping_result = ConsumerMapping.model_validate(json.loads(body)).get(consumer_id) + if mapping_result is None: + return None - if mapping_result is None: - return None + return [item.campaign_config_id for item in mapping_result] - return [item.campaign_config_id for item in mapping_result] + except ClientError as e: + if e.response["Error"]["Code"] == "NoSuchKey": + return None + logger.exception( + "Error while reading consumer mapping config file : %s", CONSUMER_MAPPING_FILE_NAME, exc_info=e + ) + raise diff --git a/tests/unit/repos/test_consumer_mapping_repo.py b/tests/unit/repos/test_consumer_mapping_repo.py index 1c6d60e8d..668d17a7d 100644 --- a/tests/unit/repos/test_consumer_mapping_repo.py +++ b/tests/unit/repos/test_consumer_mapping_repo.py @@ -31,8 +31,6 @@ def test_get_permitted_campaign_ids_success(self, repo, mock_s3_client): ] } - mock_s3_client.list_objects.return_value = {"Contents": [{"Key": "mappings.json"}]} - body_json = json.dumps(mapping_data).encode("utf-8") mock_s3_client.get_object.return_value = {"Body": MagicMock(read=lambda: body_json)} @@ -41,8 +39,7 @@ def test_get_permitted_campaign_ids_success(self, repo, mock_s3_client): # Then assert result == expected_campaign_ids - mock_s3_client.list_objects.assert_called_once_with(Bucket="test-bucket") - mock_s3_client.get_object.assert_called_once_with(Bucket="test-bucket", Key="mappings.json") + mock_s3_client.get_object.assert_called_once_with(Bucket="test-bucket", Key="consumer_mapping_config.json") def test_get_permitted_campaign_ids_returns_none_when_missing(self, repo, mock_s3_client): """ @@ -51,7 +48,6 @@ def test_get_permitted_campaign_ids_returns_none_when_missing(self, repo, mock_s """ valid_schema_data = {"other-user": [{"CampaignConfigID": "camp-1", "Description": "Some description"}]} - mock_s3_client.list_objects.return_value = {"Contents": [{"Key": "mappings.json"}]} body_json = json.dumps(valid_schema_data).encode("utf-8") mock_s3_client.get_object.return_value = {"Body": MagicMock(read=lambda: body_json)} From cfa264d8e41251360ea0dfa919bb134207eb831c Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:12:16 +0000 Subject: [PATCH 2/3] eli 620 - fix integration tests --- tests/integration/conftest.py | 36 +++++++++---------- .../in_process/test_eligibility_endpoint.py | 2 +- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 9ba0968d3..8d6c0f369 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -1360,7 +1360,7 @@ def create_and_put_consumer_mapping_in_s3( consumer_mapping_data = consumer_mapping.model_dump(by_alias=True) s3_client.put_object( Bucket=consumer_mapping_bucket, - Key="consumer_mapping.json", + Key="consumer_mapping_config.json", Body=json.dumps(consumer_mapping_data), ContentType="application/json", ) @@ -1378,7 +1378,7 @@ def consumer_to_active_campaign_having_invalid_tokens_mapping( campaign_config_with_invalid_tokens, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture(scope="class") @@ -1392,7 +1392,7 @@ def consumer_to_active_campaign_having_tokens_mapping( campaign_config_with_tokens, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture(scope="class") @@ -1406,7 +1406,7 @@ def consumer_to_active_rsv_campaign_mapping( rsv_campaign_config, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture(scope="class") @@ -1420,7 +1420,7 @@ def consumer_to_active_campaign_having_and_rule_mapping( campaign_config_with_and_rule, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1434,7 +1434,7 @@ def consumer_to_active_campaign_missing_descriptions_and_rule_text_mapping( campaign_config_with_missing_descriptions_missing_rule_text, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1448,7 +1448,7 @@ def consumer_to_active_campaign_having_rules_with_rule_code_mapping( campaign_config_with_rules_having_rule_code, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1462,7 +1462,7 @@ def consumer_to_active_campaign_having_rules_with_rule_mapper_mapping( campaign_config_with_rules_having_rule_mapper, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1476,7 +1476,7 @@ def consumer_to_active_campaign_having_only_virtual_cohort_mapping( campaign_config_with_virtual_cohort, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1490,7 +1490,7 @@ def consumer_to_active_campaign_config_with_derived_values_mapping( campaign_config_with_derived_values, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1504,7 +1504,7 @@ def consumer_to_active_campaign_config_with_derived_values_formatted_mapping( campaign_config_with_derived_values_formatted, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1518,7 +1518,7 @@ def consumer_to_active_campaign_config_with_multiple_add_days_mapping( campaign_config_with_multiple_add_days, consumer_id, consumer_mapping_bucket, s3_client ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1536,12 +1536,12 @@ def consumer_to_campaign_having_inactive_iteration_mapping( s3_client.put_object( Bucket=consumer_mapping_bucket, - Key="consumer_mapping.json", + Key="consumer_mapping_config.json", Body=json.dumps(mapping.model_dump(by_alias=True)), ContentType="application/json", ) yield mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture(scope="class") @@ -1559,12 +1559,12 @@ def consumer_to_multiple_campaign_configs_mapping( s3_client.put_object( Bucket=consumer_mapping_bucket, - Key="consumer_mapping.json", + Key="consumer_mapping_config.json", Body=json.dumps(mapping.model_dump(by_alias=True)), ContentType="application/json", ) yield mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") @pytest.fixture @@ -1575,12 +1575,12 @@ def consumer_mappings( consumer_mapping_data = consumer_mapping.model_dump(by_alias=True) s3_client.put_object( Bucket=consumer_mapping_bucket, - Key="consumer_mapping.json", + Key="consumer_mapping_config.json", Body=json.dumps(consumer_mapping_data), ContentType="application/json", ) yield consumer_mapping - s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping.json") + s3_client.delete_object(Bucket=consumer_mapping_bucket, Key="consumer_mapping_config.json") # If you put StubSecretRepo in a separate module, import it instead diff --git a/tests/integration/in_process/test_eligibility_endpoint.py b/tests/integration/in_process/test_eligibility_endpoint.py index 7e25c2813..28989d2be 100644 --- a/tests/integration/in_process/test_eligibility_endpoint.py +++ b/tests/integration/in_process/test_eligibility_endpoint.py @@ -1274,7 +1274,7 @@ def test_if_campaign_having_best_status_is_chosen_if_there_exists_multiple_campa # Consumer Mapping Data s3_client.put_object( Bucket=consumer_mapping_bucket, - Key="consumer_mapping.json", + Key="consumer_mapping_config.json", Body=json.dumps( { consumer_id: [ From cf98858ac7f8492fb6f0353a0f335dd6615e554e Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Tue, 3 Feb 2026 12:16:50 +0000 Subject: [PATCH 3/3] eli 620 - few more tests for mapping repo --- .../repos/consumer_mapping_repo.py | 4 +--- .../unit/repos/test_consumer_mapping_repo.py | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/eligibility_signposting_api/repos/consumer_mapping_repo.py b/src/eligibility_signposting_api/repos/consumer_mapping_repo.py index 83c50d3d5..487b9ca00 100644 --- a/src/eligibility_signposting_api/repos/consumer_mapping_repo.py +++ b/src/eligibility_signposting_api/repos/consumer_mapping_repo.py @@ -43,7 +43,5 @@ def get_permitted_campaign_ids(self, consumer_id: ConsumerId) -> list[CampaignID except ClientError as e: if e.response["Error"]["Code"] == "NoSuchKey": return None - logger.exception( - "Error while reading consumer mapping config file : %s", CONSUMER_MAPPING_FILE_NAME, exc_info=e - ) + logger.exception("Error while reading consumer mapping config file : %s", CONSUMER_MAPPING_FILE_NAME) raise diff --git a/tests/unit/repos/test_consumer_mapping_repo.py b/tests/unit/repos/test_consumer_mapping_repo.py index 668d17a7d..13a5fd015 100644 --- a/tests/unit/repos/test_consumer_mapping_repo.py +++ b/tests/unit/repos/test_consumer_mapping_repo.py @@ -2,6 +2,7 @@ from unittest.mock import MagicMock import pytest +from botocore.exceptions import ClientError from eligibility_signposting_api.model.consumer_mapping import ConsumerId from eligibility_signposting_api.repos.consumer_mapping_repo import BucketName, ConsumerMappingRepo @@ -56,3 +57,25 @@ def test_get_permitted_campaign_ids_returns_none_when_missing(self, repo, mock_s # Then assert result is None + + def test_get_permitted_campaign_ids_returns_none_when_file_not_in_s3(self, repo, mock_s3_client): + # Given: S3 returns a NoSuchKey error + error_response = {"Error": {"Code": "NoSuchKey", "Message": "Not Found"}} + mock_s3_client.get_object.side_effect = ClientError(error_response, "GetObject") + + # When + result = repo.get_permitted_campaign_ids(ConsumerId("any-user")) + + # Then + assert result is None + + def test_get_permitted_campaign_ids_raises_client_error(self, repo, mock_s3_client): + # Given: An S3 error that is NOT 'NoSuchKey' (e.g, Access denied error) + error_response = {"Error": {"Code": "AccessDenied", "Message": "Access Denied"}} + mock_s3_client.get_object.side_effect = ClientError(error_response, "GetObject") + + # When / Then: Verify the error is re-raised + with pytest.raises(ClientError) as exc_info: + repo.get_permitted_campaign_ids(ConsumerId("any-user")) + + assert exc_info.value.response["Error"]["Code"] == "AccessDenied"