[ELI-619] - formatting and removing monkey patch

Author: TOEL2

src/eligibility_signposting_api/config/constants.py

@@ -1,5 +1,5 @@
-from typing import Literal
 import os
+from typing import Literal
 
 URL_PREFIX = "patient-check"
 RULE_STOP_DEFAULT = False

src/eligibility_signposting_api/repos/campaign_repo.py

@@ -1,7 +1,5 @@
 import json
 import logging
-import os
-import time
 from collections.abc import Generator
 from typing import Annotated, NewType
 
@@ -10,15 +8,16 @@
 from cachetools import TTLCache
 from wireup import Inject, service
 
-from eligibility_signposting_api.model.campaign_config import CampaignConfig, Rules
 from eligibility_signposting_api.config.constants import CACHE_TTL_SECONDS
+from eligibility_signposting_api.model.campaign_config import CampaignConfig, Rules
 
 BucketName = NewType("BucketName", str)
 
 logger = logging.getLogger(__name__)
 
 campaign_config_cache: TTLCache[str, list[CampaignConfig]] = TTLCache(maxsize=1, ttl=CACHE_TTL_SECONDS)
 
+
 @service
 class CampaignRepo:
     """Repository class for Campaign Rules, which we can use to calculate a person's eligibility for vaccination.
@@ -34,7 +33,7 @@ def __init__(
         self.s3_client = s3_client
         self.bucket_name = bucket_name
 
-    def get_campaign_configs(self, consumer_id: str) -> Generator[CampaignConfig, None, None]:
+    def get_campaign_configs(self, consumer_id: str) -> Generator[CampaignConfig]:
         bypass = "test-" in consumer_id
         cache_key = "all_campaigns"
         cached = None if bypass else campaign_config_cache.get(cache_key)
@@ -71,8 +70,6 @@ def _load_campaign_configs_from_s3(self) -> list[CampaignConfig]:
                         Key=f"{campaign_object['Key']}",
                     )
                     body = response["Body"].read()
-                    campaign_configs.append(
-                        Rules.model_validate(json.loads(body)).campaign_config
-                    )
+                    campaign_configs.append(Rules.model_validate(json.loads(body)).campaign_config)
 
         return campaign_configs

src/eligibility_signposting_api/services/eligibility_services.py

@@ -50,9 +50,7 @@ def get_eligibility_status(
             except NotFoundError as e:
                 raise UnknownPersonError from e
             else:
-                campaign_configs: list[CampaignConfig] = list(
-                    self.campaign_repo.get_campaign_configs(consumer_id)
-                )
+                campaign_configs: list[CampaignConfig] = list(self.campaign_repo.get_campaign_configs(consumer_id))
                 permitted_campaign_configs = self.__collect_permitted_campaign_configs(
                     campaign_configs, ConsumerId(consumer_id)
                 )

tests/integration/conftest.py

@@ -107,6 +107,7 @@ def moto_server(request: pytest.FixtureRequest) -> URL:
 def clear_cache():
     campaign_config_cache.clear()
 
+
 def is_responsive(url: URL) -> bool:
     try:
         response = httpx.get(str(url))

tests/integration/lambda/test_app_running_as_lambda.py

@@ -726,72 +726,3 @@ def test_status_end_point(invoke_with_mock_apigw_request):
             )
         ),
     )
-
-# Runnable if we go to tests/docker-compose.mock_aws.yml and add ENVIRONMENT=dev
-# shows that in a non-local environment we can bypass cache
-
-# def test_cache_bypass(  # noqa: PLR0913
-#     lambda_client: BaseClient,  # noqa:ARG001
-#     persisted_person: NHSNumber,
-#     rsv_campaign_config: CampaignConfig,
-#     consumer_to_active_rsv_campaign_mapping: ConsumerMapping,  # noqa: ARG001
-#     consumer_id: ConsumerId,
-#     s3_client: BaseClient,
-#     audit_bucket: BucketName,
-#     invoke_with_mock_apigw_request,
-#     lambda_logs: Callable[[], list[str]],
-#     secretsmanager_client: BaseClient,  # noqa:ARG001
-# ):
-#     # Given
-#     invoke_path = f"/patient-check/{persisted_person}"
-#     headers = {
-#         "nhs-login-nhs-number": str(persisted_person),
-#         "x_request_id": "x_request_id",
-#         "x_correlation_id": "x_correlation_id",
-#         "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods",
-#         "nhsd-application-id": "nhsd-application-id",
-#         "NHSE-Product-ID": consumer_id,
-#     }
-#     params = {"includeActions": "Y"}
-#
-#     objects = s3_client.list_objects_v2(Bucket="test-rules-bucket").get("Contents", [])
-#     assert_that(objects, is_not(equal_to([])))
-#     config_key = objects[0]["Key"]
-#     original = s3_client.get_object(Bucket="test-rules-bucket", Key=config_key)
-#     original_payload = json.loads(original["Body"].read())
-#     print(original_payload)
-#
-#     # When
-#     response = invoke_with_mock_apigw_request(path=invoke_path, headers=headers, params=params)
-#
-#     # Then
-#     assert_that(
-#         response,
-#         is_response().with_status_code(HTTPStatus.OK).and_body(is_json_that(has_key("processedSuggestions"))),
-#     )
-#
-#     original_payload["CampaignConfig"]["Target"] = "RSV_CHANGED_FOR_BYPASS_TEST"
-#     s3_client.put_object(
-#         Bucket="test-rules-bucket",
-#         Key=config_key,
-#         Body=json.dumps(original_payload),
-#         ContentType="application/json",
-#     )
-#
-#     # Second request without bypass header should still use cached config
-#     response_2 = invoke_with_mock_apigw_request(invoke_path, headers)
-#     assert_that(
-#         response_2,
-#         is_response().with_status_code(HTTPStatus.OK).and_body(is_json_that(has_key("processedSuggestions"))),
-#     )
-#
-#     # Third request with bypass header should re-read S3 and reflect the change
-#     bypass_headers = {
-#         **headers,
-#         "X-Bypass-Campaign-Config-Cache": "true",
-#     }
-#     response_3 = invoke_with_mock_apigw_request(invoke_path, bypass_headers)
-#     assert_that(
-#         response_3,
-#         is_response().with_status_code(500),
-#     )

tests/unit/repos/test_campaign_repo.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from eligibility_signposting_api.repos.campaign_repo import CampaignRepo, BucketName, campaign_config_cache
+from eligibility_signposting_api.repos.campaign_repo import BucketName, CampaignRepo, campaign_config_cache
 from tests.fixtures.builders.model.rule import CampaignConfigFactory
 
 
@@ -31,14 +31,10 @@ def repo(self, mock_s3_client):
     @pytest.fixture
     def rules_payload(self):
         campaign_config = CampaignConfigFactory.build()
-        return {
-            "campaign_config": campaign_config.model_dump(mode="json")
-        }
+        return {"campaign_config": campaign_config.model_dump(mode="json")}
 
     def test_get_campaign_configs_loads_from_s3(self, repo, mock_s3_client, rules_payload):
-        mock_s3_client.list_objects.return_value = {
-            "Contents": [{"Key": "rsv.json"}]
-        }
+        mock_s3_client.list_objects.return_value = {"Contents": [{"Key": "rsv.json"}]}
         mock_s3_client.get_object.return_value = make_s3_body(rules_payload)
 
         result = list(repo.get_campaign_configs("consumer_id"))
@@ -56,20 +52,11 @@ def test_get_campaign_configs_uses_cache_within_ttl(
         self,
         repo,
         mock_s3_client,
-        monkeypatch,
     ):
-        repo._cache_ttl_seconds = 60
-
         first_config = CampaignConfigFactory.build(version=1)
 
-        mock_s3_client.list_objects.return_value = {
-            "Contents": [{"Key": "rsv.json"}]
-        }
-        mock_s3_client.get_object.return_value = make_s3_body(
-            {"campaign_config": first_config.model_dump(mode="json")}
-        )
-
-        monkeypatch.setattr("time.time", lambda: 1000.0)
+        mock_s3_client.list_objects.return_value = {"Contents": [{"Key": "rsv.json"}]}
+        mock_s3_client.get_object.return_value = make_s3_body({"campaign_config": first_config.model_dump(mode="json")})
 
         first = list(repo.get_campaign_configs("consumer_id"))
         second = list(repo.get_campaign_configs("consumer_id"))
@@ -83,28 +70,19 @@ def test_get_campaign_configs_refreshes_after_ttl_expiry(
         self,
         repo,
         mock_s3_client,
-        monkeypatch,
     ):
-        repo._cache_ttl_seconds = 60
-
         first_config = CampaignConfigFactory.build(version=1)
         second_config = CampaignConfigFactory.build(version=2)
 
-        mock_s3_client.list_objects.return_value = {
-            "Contents": [{"Key": "rsv.json"}]
-        }
+        mock_s3_client.list_objects.return_value = {"Contents": [{"Key": "rsv.json"}]}
         mock_s3_client.get_object.side_effect = [
             make_s3_body({"campaign_config": first_config.model_dump(mode="json")}),
             make_s3_body({"campaign_config": second_config.model_dump(mode="json")}),
         ]
 
-        current_time = {"value": 1000.0}
-        monkeypatch.setattr("time.time", lambda: current_time["value"])
-
         first = list(repo.get_campaign_configs("consumer_id"))
-        current_time["value"] = 1030.0
         second = list(repo.get_campaign_configs("consumer_id"))
-        current_time["value"] = 1061.0
+        campaign_config_cache.clear()
         third = list(repo.get_campaign_configs("test-consumer-1"))
 
         assert first[0].version == 1