Merge pull request #299 from NHSDigital/feature/eja-eli-391-lambda-performance-improvements

Feature/eja eli 391 lambda performance improvements

Author: eddalmond1

src/eligibility_signposting_api/app.py

@@ -10,6 +10,7 @@
 from mangum.types import LambdaContext, LambdaEvent
 
 from eligibility_signposting_api import audit, repos, services
+from eligibility_signposting_api.common.cache_manager import FLASK_APP_CACHE_KEY, cache_manager
 from eligibility_signposting_api.common.error_handler import handle_exception
 from eligibility_signposting_api.common.request_validator import validate_request_params
 from eligibility_signposting_api.config.config import config
@@ -31,13 +32,27 @@ def main() -> None:  # pragma: no cover
     app.run(debug=config()["log_level"] == logging.DEBUG)
 
 
+def get_or_create_app() -> Flask:
+    """Get the global Flask app instance, creating it if it doesn't exist.
+
+    This ensures the Flask app is initialized only once per Lambda container,
+    improving performance by avoiding repeated initialization.
+    """
+    app = cache_manager.get(FLASK_APP_CACHE_KEY)
+    if app is None:
+        app = create_app()
+        cache_manager.set(FLASK_APP_CACHE_KEY, app)
+        logger.info("Flask app initialized and cached for container reuse")
+    return app  # type: ignore[return-value]
+
+
 @add_lambda_request_id_to_logger()
 @tracing_setup()
 @log_request_ids_from_headers()
 @validate_request_params()
 def lambda_handler(event: LambdaEvent, context: LambdaContext) -> dict[str, Any]:  # pragma: no cover
     """Run the Flask app as an AWS Lambda."""
-    app = create_app()
+    app = get_or_create_app()
     app.debug = config()["log_level"] == logging.DEBUG
     handler = Mangum(WsgiToAsgi(app), lifespan="off")
     handler.config["text_mime_types"].append("application/fhir+json")

src/eligibility_signposting_api/common/cache_manager.py

@@ -0,0 +1,72 @@
+"""Cache management utilities for Lambda container optimization."""
+
+import logging
+from typing import TypeVar
+
+logger = logging.getLogger(__name__)
+
+T = TypeVar("T")
+
+
+class CacheManager:
+    """Thread-safe cache manager for Lambda container optimization."""
+
+    def __init__(self) -> None:
+        self._caches: dict[str, object] = {}
+        self._logger = logging.getLogger(__name__)
+
+    def get(self, cache_key: str) -> object | None:
+        """Get a value from the cache."""
+        value = self._caches.get(cache_key)
+        if value is not None:
+            self._logger.debug("Cache hit", extra={"cache_key": cache_key})
+        else:
+            self._logger.debug("Cache miss", extra={"cache_key": cache_key})
+        return value
+
+    def set(self, cache_key: str, value: object) -> None:
+        """Set a value in the cache."""
+        self._caches[cache_key] = value
+        self._logger.debug("Cache updated", extra={"cache_key": cache_key})
+
+    def clear(self, cache_key: str) -> bool:
+        """Clear a specific cache entry. Returns True if entry existed."""
+        if cache_key in self._caches:
+            del self._caches[cache_key]
+            self._logger.info("Cache entry cleared", extra={"cache_key": cache_key})
+            return True
+        return False
+
+    def clear_all(self) -> None:
+        """Clear all cached data."""
+        self._caches.clear()
+        self._logger.info("All caches cleared")
+
+    def get_cache_info(self) -> dict[str, int]:
+        """Get information about current cache state."""
+        info = {}
+        for key, value in self._caches.items():
+            try:
+                info[key] = len(value) if hasattr(value, "__len__") else 1  # type: ignore[arg-type]
+            except TypeError:
+                info[key] = 1
+        return info
+
+    def has(self, cache_key: str) -> bool:
+        """Check if a cache key exists."""
+        return cache_key in self._caches
+
+    def size(self) -> int:
+        """Get the number of cached items."""
+        return len(self._caches)
+
+
+# Global cache manager instance
+_cache_manager = CacheManager()
+
+# Export the global cache manager for direct use
+cache_manager = _cache_manager
+
+# Cache keys constants
+FLASK_APP_CACHE_KEY = "flask_app"
+CAMPAIGN_CONFIGS_CACHE_KEY = "campaign_configs"

tests/integration/test_performance_optimizations.py

@@ -0,0 +1,113 @@
+"""
+Test to verify the performance optimization caching is working correctly.
+"""
+
+from eligibility_signposting_api.app import get_or_create_app
+from eligibility_signposting_api.common.cache_manager import (
+    FLASK_APP_CACHE_KEY,
+    cache_manager,
+)
+
+
+class TestPerformanceOptimizations:
+    """Tests to verify caching optimizations work correctly."""
+
+    def test_flask_app_caching(self):
+        """Test that Flask app is cached and reused."""
+        # Clear all caches first
+        cache_manager.clear_all()
+
+        # First call should create and cache the app
+        app1 = get_or_create_app()
+        cache_info_after_first = cache_manager.get_cache_info()
+
+        # Second call should reuse the cached app
+        app2 = get_or_create_app()
+        cache_info_after_second = cache_manager.get_cache_info()
+
+        # Verify same instance is returned
+        assert app1 is app2, "Flask app should be reused from cache"
+
+        # Verify cache contains the Flask app
+        assert FLASK_APP_CACHE_KEY in cache_info_after_first
+        assert FLASK_APP_CACHE_KEY in cache_info_after_second
+
+        # Cache info should be the same after second call (no new caching)
+        assert cache_info_after_first == cache_info_after_second
+
+    def test_cache_clearing_works(self):
+        """Test that cache clearing functionality works."""
+        # Set up some cached data
+        app1 = get_or_create_app()
+        cache_info_before = cache_manager.get_cache_info()
+
+        # Verify cache has data
+        assert len(cache_info_before) > 0, "Cache should contain Flask app"
+
+        # Clear all caches
+        cache_manager.clear_all()
+        cache_info_after = cache_manager.get_cache_info()
+
+        # Verify cache is empty
+        assert len(cache_info_after) == 0, "Cache should be empty after clearing"
+
+        # New app should be different instance
+        app2 = get_or_create_app()
+        assert app1 is not app2, "New Flask app should be created after cache clear"
+
+    def test_automatic_cache_clearing_in_tests(self):
+        """Test that the automatic cache clearing fixture works."""
+        # This test relies on the clear_performance_caches fixture
+        # which should clear caches before each test class
+
+        # We can't guarantee completely empty cache because of test setup,
+        # but we can verify the cache clearing mechanism exists
+
+        # Create some cached data
+        app = get_or_create_app()
+        assert app is not None
+
+        # Verify Flask app is now cached
+        cache_info_after = cache_manager.get_cache_info()
+        assert FLASK_APP_CACHE_KEY in cache_info_after
+
+    def test_clear_cache_specific_key(self):
+        """Test clearing a specific cache key and logging."""
+        # Set up test data
+        test_key = "test_cache_key"
+        test_value = "test_value"
+        cache_manager.set(test_key, test_value)
+
+        # Verify key exists
+        cache_info_before = cache_manager.get_cache_info()
+        assert test_key in cache_info_before
+
+        # Clear specific key (this covers lines 28-29)
+        result = cache_manager.clear(test_key)
+        assert result is True  # Should return True for existing key
+
+        # Verify key is removed
+        cache_info_after = cache_manager.get_cache_info()
+        assert test_key not in cache_info_after
+
+        # Clearing non-existent key should not cause error
+        result = cache_manager.clear("non_existent_key")
+        assert result is False  # Should return False for non-existent key
+
+    def test_cache_info_with_non_len_objects(self):
+        """Test get_cache_info with objects that don't have __len__ method."""
+
+        # Set up object that raises TypeError on len() (covers lines 44-45)
+        class NoLenObject:
+            def __len__(self):
+                msg = "This object doesn't support len()"
+                raise TypeError(msg)
+
+        test_key = "no_len_object"
+        no_len_obj = NoLenObject()
+        cache_manager.set(test_key, no_len_obj)
+
+        # This should handle the TypeError gracefully
+        cache_info = cache_manager.get_cache_info()
+        assert test_key in cache_info
+        assert cache_info[test_key] == 1  # Should default to 1

tests/unit/common/test_cache_manager.py

@@ -0,0 +1,150 @@
+"""Unit tests for cache_manager module."""
+
+from eligibility_signposting_api.common.cache_manager import (
+    FLASK_APP_CACHE_KEY,
+    cache_manager,
+)
+
+
+class TestCacheManager:
+    """Test the cache manager functionality."""
+
+    def setup_method(self):
+        """Clean up cache before each test."""
+        cache_manager.clear_all()
+
+    def test_set_and_get_cache(self):
+        """Test basic cache set and get operations."""
+        test_key = "test_key"
+        test_value = "test_value"
+
+        # Set cache
+        cache_manager.set(test_key, test_value)
+
+        # Get cache
+        result = cache_manager.get(test_key)
+        assert result == test_value
+
+    def test_get_cache_nonexistent_key(self):
+        """Test getting a non-existent cache key returns None."""
+        result = cache_manager.get("non_existent_key")
+        assert result is None
+
+    def test_clear_cache_existing_key(self):
+        """Test clearing an existing cache key."""
+        test_key = "test_key"
+        test_value = "test_value"
+
+        cache_manager.set(test_key, test_value)
+        assert cache_manager.get(test_key) == test_value
+
+        # Clear cache
+        result = cache_manager.clear(test_key)
+        assert result is True  # Should return True if key existed
+        assert cache_manager.get(test_key) is None
+
+    def test_clear_cache_nonexistent_key(self):
+        """Test clearing a non-existent cache key."""
+        # This should not raise an exception
+        result = cache_manager.clear("non_existent_key")
+        assert result is False  # Should return False if key didn't exist
+
+    def test_clear_all_caches(self):
+        """Test clearing all caches."""
+        cache_manager.set("key1", "value1")
+        cache_manager.set("key2", "value2")
+        cache_manager.set("key3", "value3")
+
+        # Verify keys exist
+        assert cache_manager.get("key1") == "value1"
+        assert cache_manager.get("key2") == "value2"
+        assert cache_manager.get("key3") == "value3"
+
+        # Clear all caches
+        cache_manager.clear_all()
+
+        # Verify all keys are gone
+        assert cache_manager.get("key1") is None
+        assert cache_manager.get("key2") is None
+        assert cache_manager.get("key3") is None
+
+    def test_get_cache_info_with_len_objects(self):
+        """Test cache info with objects that have __len__."""
+        test_list = [1, 2, 3, 4, 5]
+        test_dict = {"a": 1, "b": 2, "c": 3}
+
+        cache_manager.set("list_key", test_list)
+        cache_manager.set("dict_key", test_dict)
+
+        cache_info = cache_manager.get_cache_info()
+        assert cache_info["list_key"] == len(test_list)
+        assert cache_info["dict_key"] == len(test_dict)
+
+    def test_get_cache_info_with_non_len_objects(self):
+        """Test cache info with objects that don't have __len__."""
+        test_int = 42
+        test_str = "test"
+        expected_int_size = 1  # Default for non-len objects
+
+        cache_manager.set("int_key", test_int)
+        cache_manager.set("str_key", test_str)
+
+        cache_info = cache_manager.get_cache_info()
+        assert cache_info["int_key"] == expected_int_size  # Default for non-len objects
+        assert cache_info["str_key"] == len(test_str)
+
+    def test_flask_app_cache_key_constant(self):
+        """Test that the Flask app cache key constant is available."""
+        assert FLASK_APP_CACHE_KEY == "flask_app"
+
+    def test_cache_overwrites_existing_value(self):
+        """Test that setting a cache key overwrites existing value."""
+        cache_manager.set("key", "original_value")
+        assert cache_manager.get("key") == "original_value"
+
+        cache_manager.set("key", "new_value")
+        assert cache_manager.get("key") == "new_value"
+
+    def test_cache_info_empty_cache(self):
+        """Test cache info when cache is empty."""
+        cache_info = cache_manager.get_cache_info()
+        assert cache_info == {}
+
+    def test_cache_handles_none_values(self):
+        """Test that cache can handle None values."""
+        cache_manager.set("none_key", None)
+        result = cache_manager.get("none_key")
+        assert result is None
+
+        # Verify the key actually exists (not just returning None for missing key)
+        assert cache_manager.has("none_key") is True
+
+    def test_has_method(self):
+        """Test the has() method."""
+        assert cache_manager.has("nonexistent") is False
+
+        cache_manager.set("test_key", "test_value")
+        assert cache_manager.has("test_key") is True
+
+        cache_manager.clear("test_key")
+        assert cache_manager.has("test_key") is False
+
+    def test_size_method(self):
+        """Test the size() method."""
+        empty_cache_size = 0
+        single_item_size = 1
+        two_items_size = 2
+
+        assert cache_manager.size() == empty_cache_size
+
+        cache_manager.set("key1", "value1")
+        assert cache_manager.size() == single_item_size
+
+        cache_manager.set("key2", "value2")
+        assert cache_manager.size() == two_items_size
+
+        cache_manager.clear("key1")
+        assert cache_manager.size() == single_item_size
+
+        cache_manager.clear_all()
+        assert cache_manager.size() == empty_cache_size