Skip to content

Commit 60c896b

Browse files
committed
eli-391 reverting caching of campaign config
1 parent f348260 commit 60c896b

File tree

5 files changed

+4
-320
lines changed

5 files changed

+4
-320
lines changed
Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,14 @@
11
import json
2-
import logging
3-
import os
42
from collections.abc import Generator
53
from typing import Annotated, NewType
64

75
from botocore.client import BaseClient
86
from wireup import Inject, service
97

10-
from eligibility_signposting_api.common.cache_manager import (
11-
CAMPAIGN_CONFIGS_CACHE_KEY,
12-
cache_manager,
13-
)
148
from eligibility_signposting_api.model.campaign_config import CampaignConfig, Rules
159

1610
BucketName = NewType("BucketName", str)
1711

18-
logger = logging.getLogger(__name__)
19-
2012

2113
@service
2214
class CampaignRepo:
@@ -34,47 +26,8 @@ def __init__(
3426
self.bucket_name = bucket_name
3527

3628
def get_campaign_configs(self) -> Generator[CampaignConfig]:
37-
"""Get campaign configurations, using cached data if available.
38-
39-
Campaign rules are loaded once per Lambda container and cached globally
40-
to improve performance by avoiding repeated S3 reads, unless caching is disabled for testing.
41-
"""
42-
# Check if caching is disabled for tests
43-
if os.getenv("DISABLE_CAMPAIGN_CACHE", "").lower() in ("true", "1", "yes"):
44-
logger.debug("Campaign caching disabled for testing")
45-
yield from self._load_campaign_configs_from_s3()
46-
return
47-
48-
cached_configs = cache_manager.get(CAMPAIGN_CONFIGS_CACHE_KEY)
49-
50-
if cached_configs is None:
51-
logger.info("Loading campaign configurations from S3 and caching for container reuse")
52-
configs = self._load_campaign_configs_from_s3()
53-
cache_manager.set(CAMPAIGN_CONFIGS_CACHE_KEY, configs)
54-
logger.info("Cached campaign configurations", extra={"campaign_count": len(configs)})
55-
yield from configs
56-
else:
57-
logger.debug("Using cached campaign configurations")
58-
yield from cached_configs # type: ignore[misc]
59-
60-
def _load_campaign_configs_from_s3(self) -> list[CampaignConfig]:
61-
"""Load campaign configurations from S3."""
62-
configs = []
6329
campaign_objects = self.s3_client.list_objects(Bucket=self.bucket_name)
64-
6530
for campaign_object in campaign_objects["Contents"]:
6631
response = self.s3_client.get_object(Bucket=self.bucket_name, Key=f"{campaign_object['Key']}")
6732
body = response["Body"].read()
68-
config = Rules.model_validate(json.loads(body)).campaign_config
69-
configs.append(config)
70-
71-
return configs
72-
73-
def clear_campaign_cache(self) -> None:
74-
"""Clear the campaign configurations cache.
75-
76-
This forces the next call to get_campaign_configs() to reload from S3.
77-
Useful for testing or when you need to refresh the data.
78-
"""
79-
cache_manager.clear(CAMPAIGN_CONFIGS_CACHE_KEY)
80-
logger.info("Campaign configurations cache cleared")
33+
yield Rules.model_validate(json.loads(body)).campaign_config

tests/integration/conftest.py

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
from httpx import RequestError
1818
from yarl import URL
1919

20-
from eligibility_signposting_api.common.cache_manager import cache_manager
2120
from eligibility_signposting_api.model import eligibility_status
2221
from eligibility_signposting_api.model.campaign_config import (
2322
CampaignConfig,
@@ -206,7 +205,6 @@ def flask_function(lambda_client: BaseClient, iam_role: str, lambda_zip: Path) -
206205
"FIREHOSE_ENDPOINT": os.getenv("LOCALSTACK_INTERNAL_ENDPOINT", "http://localstack:4566/"),
207206
"AWS_REGION": AWS_REGION,
208207
"LOG_LEVEL": "DEBUG",
209-
"DISABLE_CAMPAIGN_CACHE": "true", # Disable caching for integration tests
210208
}
211209
},
212210
)
@@ -479,34 +477,6 @@ def firehose_delivery_stream(firehose_client: BaseClient, audit_bucket: BucketNa
479477
)
480478

481479

482-
@pytest.fixture(scope="class", autouse=True)
483-
def clear_performance_caches():
484-
"""Clear all performance caches before each test class.
485-
486-
This ensures test isolation when using the performance-optimized
487-
Flask app and campaign configuration caching.
488-
"""
489-
cache_manager.clear_all()
490-
yield
491-
# Optionally clear again after tests complete
492-
cache_manager.clear_all()
493-
494-
495-
@pytest.fixture
496-
def clear_campaign_cache_for_test():
497-
"""Clear campaign cache before each test function that needs fresh campaign data.
498-
499-
This fixture should be used by tests that create specific campaign configurations
500-
and expect them to be loaded fresh from S3, not from cache.
501-
"""
502-
from eligibility_signposting_api.common.cache_manager import CAMPAIGN_CONFIGS_CACHE_KEY
503-
504-
cache_manager.clear(CAMPAIGN_CONFIGS_CACHE_KEY)
505-
yield
506-
# Optionally clear again after test completes
507-
cache_manager.clear(CAMPAIGN_CONFIGS_CACHE_KEY)
508-
509-
510480
@pytest.fixture(scope="class")
511481
def campaign_config(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]:
512482
campaign: CampaignConfig = rule.CampaignConfigFactory.build(

tests/integration/in_process/test_eligibility_endpoint.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,6 @@ def test_actionable_with_and_rule(
242242
client: FlaskClient,
243243
persisted_person: NHSNumber,
244244
campaign_config_with_and_rule: CampaignConfig, # noqa: ARG002
245-
clear_campaign_cache_for_test, # noqa: ARG002
246245
):
247246
# Given
248247

tests/integration/repo/test_campaign_repo.py

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ def test_get_campaign_config(s3_client: BaseClient, rules_bucket: BucketName, ca
2626
# Given
2727
repo = CampaignRepo(s3_client, rules_bucket)
2828

29-
# Ensure we start with a fresh cache for this test
30-
repo.clear_campaign_cache()
31-
3229
# When
33-
actual = list(repo.get_campaign_configs()) # Then
30+
actual = list(repo.get_campaign_configs())
31+
32+
# Then
3433
assert_that(
3534
actual,
3635
has_item(
@@ -51,35 +50,3 @@ def test_get_campaign_config(s3_client: BaseClient, rules_bucket: BucketName, ca
5150
)
5251
),
5352
)
54-
55-
56-
def test_get_campaign_config_caching_behavior(
57-
s3_client: BaseClient,
58-
rules_bucket: BucketName,
59-
campaign_config: CampaignConfig, # noqa: ARG001
60-
):
61-
"""Test that campaign configurations are properly cached and cache clearing works."""
62-
# Given
63-
repo = CampaignRepo(s3_client, rules_bucket)
64-
65-
# Ensure we start with a fresh cache
66-
repo.clear_campaign_cache()
67-
68-
# When - first call should load from S3
69-
first_result = list(repo.get_campaign_configs())
70-
71-
# When - second call should use cache (this tests the cache hit path)
72-
second_result = list(repo.get_campaign_configs())
73-
74-
# Then - both results should be identical
75-
assert len(first_result) == len(second_result)
76-
assert first_result[0].id == second_result[0].id
77-
assert first_result[0].name == second_result[0].name
78-
79-
# When - clear cache and call again
80-
repo.clear_campaign_cache()
81-
third_result = list(repo.get_campaign_configs())
82-
83-
# Then - should still get the same data (reloaded from S3)
84-
assert len(third_result) == len(first_result)
85-
assert third_result[0].id == first_result[0].id

tests/unit/repos/test_campaign_repo.py

Lines changed: 0 additions & 205 deletions
This file was deleted.

0 commit comments

Comments
 (0)