From 586e1b0bb5fe992b74b408337eb38c142fcba7ac Mon Sep 17 00:00:00 2001 From: codeEmpress1 Date: Wed, 19 Nov 2025 10:35:00 +0300 Subject: [PATCH 1/4] add cache utility --- cache/README.md | 23 +++++++++++++++++++++++ cache/cache_utility.py | 8 ++++++++ cache/docker-compose.yaml | 23 +++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 cache/README.md create mode 100644 cache/cache_utility.py create mode 100644 cache/docker-compose.yaml diff --git a/cache/README.md b/cache/README.md new file mode 100644 index 0000000000..403dd41dc3 --- /dev/null +++ b/cache/README.md @@ -0,0 +1,23 @@ +# Local Redis for Development + +This setup provides a local Redis instance for development and testing purposes. + +## Getting Started + +1. Ensure Docker is installed. +2. ```cd cache``` and run ```docker compose run redis-cli``` + +3. Then you can interact with the redis server. + To check all saved keys use: + ``` + KEYS * + ``` +4. To exit the interactive promt, run + ``` + exit + ``` +5. To stop the container, run: + ``` + docker compose down + ``` +For more Redis CLI commands, check the docs at https://redis.io/learn/howtos/quick-start/cheat-sheet \ No newline at end of file diff --git a/cache/cache_utility.py b/cache/cache_utility.py new file mode 100644 index 0000000000..fbee65dbad --- /dev/null +++ b/cache/cache_utility.py @@ -0,0 +1,8 @@ +from canonicalwebteam.stores_web_redis.utility import RedisCache +from webapp.config import APP_NAME + +redis_cache = RedisCache( + namespace=APP_NAME, + maxsize=1000, + ttl=300, +) diff --git a/cache/docker-compose.yaml b/cache/docker-compose.yaml new file mode 100644 index 0000000000..57f8a50a55 --- /dev/null +++ b/cache/docker-compose.yaml @@ -0,0 +1,23 @@ +services: + redis: + image: redis:7 + ports: + - "6379:6379" + volumes: + - redis-data:/data + command: ["redis-server", "--appendonly", "yes"] + healthcheck: + test: ["CMD", "redis-cli", "-h", "localhost", "-p", "6379", "ping"] + interval: 5s + timeout: 2s + retries: 5 + + redis-cli: + image: redis:7 + entrypoint: ["redis-cli", "-h", "redis", "-p", "6379"] + depends_on: + redis: + condition: service_healthy + +volumes: + redis-data: From 23f5f61262b92be9b1a4c43fb61625f38a7f6fd6 Mon Sep 17 00:00:00 2001 From: codeEmpress1 Date: Wed, 19 Nov 2025 10:37:30 +0300 Subject: [PATCH 2/4] add cache to snap listing page --- webapp/endpoints/publisher/listing.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/webapp/endpoints/publisher/listing.py b/webapp/endpoints/publisher/listing.py index 72514d2e9f..65d1e9b1e4 100644 --- a/webapp/endpoints/publisher/listing.py +++ b/webapp/endpoints/publisher/listing.py @@ -18,14 +18,26 @@ from webapp.store.logic import ( get_categories, ) +from cache.cache_utility import redis_cache dashboard = Dashboard(api_session) device_gateway = DeviceGW("snap", api_session) +def get_snap_info_key(snap_name): + user_id = flask.session.get( + "publisher", {} + ).get("identity_url", "").split("+id")[1] + + return f"{user_id}:snap_info:{snap_name}" @login_required def get_listing_data(snap_name): - snap_details = dashboard.get_snap_info(flask.session, snap_name) + snap_info_key = get_snap_info_key(snap_name) + snap_details = redis_cache.get(snap_info_key, expected_type=dict) + + if not snap_details: + snap_details = dashboard.get_snap_info(flask.session, snap_name) + redis_cache.set(snap_info_key, snap_details, ttl=3600) details_metrics_enabled = snap_details["public_metrics_enabled"] details_blacklist = snap_details.get("public_metrics_blacklist", []) @@ -184,6 +196,9 @@ def post_listing_data(snap_name): dashboard.snap_screenshots( flask.session, snap_id, images_json, images_files ) + # Invalidate cache after successful screenshot update + snap_info_key = get_snap_info_key(snap_name) + redis_cache.delete(snap_info_key) except StoreApiResponseErrorList as api_response_error_list: if api_response_error_list.status_code != 404: error_list = error_list + api_response_error_list.errors @@ -198,6 +213,9 @@ def post_listing_data(snap_name): try: dashboard.snap_metadata(flask.session, snap_id, body_json) + # Invalidate cache after successful metadata update + snap_info_key = get_snap_info_key(snap_name) + redis_cache.delete(snap_info_key) except StoreApiResponseErrorList as api_response_error_list: if api_response_error_list.status_code != 404: error_list = error_list + api_response_error_list.errors From 7d6b582868d749e14a567692c1386a1c4efe0a0d Mon Sep 17 00:00:00 2001 From: codeEmpress1 Date: Wed, 19 Nov 2025 13:40:16 +0300 Subject: [PATCH 3/4] update tests --- tests/endpoints/endpoint_testing.py | 10 + tests/publisher/snaps/tests_listing.py | 129 ++++++++++++ tests/publisher/snaps/tests_post_listing.py | 222 ++++++++++++++++++++ webapp/endpoints/publisher/listing.py | 10 +- 4 files changed, 366 insertions(+), 5 deletions(-) diff --git a/tests/endpoints/endpoint_testing.py b/tests/endpoints/endpoint_testing.py index cd52bd079a..1a765b2b55 100644 --- a/tests/endpoints/endpoint_testing.py +++ b/tests/endpoints/endpoint_testing.py @@ -3,6 +3,7 @@ from webapp.app import create_app from webapp.authentication import get_publishergw_authorization_header +from cache.cache_utility import redis_cache class TestEndpoints(TestCase): @@ -24,6 +25,15 @@ def _log_in(self, client): return get_publishergw_authorization_header(test_macaroon) def setUp(self): + # Clear cache before each test + if redis_cache.redis_available: + try: + redis_cache.client.flushdb() + except Exception: + pass + else: + redis_cache.fallback.clear() + self.app = create_app(testing=True) self.client = self.app.test_client() self._log_in(self.client) diff --git a/tests/publisher/snaps/tests_listing.py b/tests/publisher/snaps/tests_listing.py index a099cb1a3b..096de2461f 100644 --- a/tests/publisher/snaps/tests_listing.py +++ b/tests/publisher/snaps/tests_listing.py @@ -1,9 +1,19 @@ import responses from tests.publisher.endpoint_testing import BaseTestCases +from cache.cache_utility import redis_cache class ListingPageNotAuth(BaseTestCases.EndpointLoggedOut): def setUp(self): + # Clear cache before each test + if redis_cache.redis_available: + try: + redis_cache.client.flushdb() + except Exception: + pass + else: + redis_cache.fallback.clear() + snap_name = "test-snap" endpoint_url = "/api/{}/listing".format(snap_name) @@ -12,6 +22,15 @@ def setUp(self): class GetListingPage(BaseTestCases.EndpointLoggedInErrorHandling): def setUp(self): + # Clear cache before each test + if redis_cache.redis_available: + try: + redis_cache.client.flushdb() + except Exception: + pass + else: + redis_cache.fallback.clear() + snap_name = "test-snap" api_url = "https://dashboard.snapcraft.io/dev/api/snaps/info/{}" @@ -275,3 +294,113 @@ def test_failed_categories_api(self): self.check_call_by_api_url(responses.calls) assert response.status_code == 200 + + @responses.activate + def test_cache_hit_on_second_request(self): + """Test that second GET request uses cache instead of API""" + payload = { + "snap_id": "id", + "snap_name": self.snap_name, + "title": "Snap title", + "summary": "This is a summary", + "description": "This is a description", + "media": [], + "publisher": { + "display-name": "The publisher", + "username": "toto", + }, + "private": True, + "channel_maps_list": [{"map": [{"info": "info"}]}], + "contact": "contact adress", + "website": "website_url", + "public_metrics_enabled": True, + "public_metrics_blacklist": False, + "license": "License", + "video_urls": [], + "categories": {"items": []}, + "status": "published", + "update_metadata_on_release": True, + "links": {"website": ["https://example.com"]}, + } + + responses.add(responses.GET, self.api_url, json=payload, status=200) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + # First request should hit API + response1 = self.client.get(self.endpoint_url) + assert response1.status_code == 200 + + snap_info_calls_before = [ + call + for call in responses.calls + if self.api_url in call.request.url + ] + assert len(snap_info_calls_before) == 1 + + # Second request should use cache (no additional snap info API calls) + response2 = self.client.get(self.endpoint_url) + assert response2.status_code == 200 + snap_info_calls_after = [ + call + for call in responses.calls + if self.api_url in call.request.url + ] + # Should have same number of calls (cache hit) + assert len(snap_info_calls_after) == len(snap_info_calls_before) + + @responses.activate + def test_cache_stores_data_correctly(self): + """Test that cached data matches API response""" + payload = { + "snap_id": "cached-id", + "snap_name": self.snap_name, + "title": "Cached Snap Title", + "summary": "This is a cached summary", + "description": "This is a cached description", + "media": [], + "publisher": { + "display-name": "Cached Publisher", + "username": "cached", + }, + "private": False, + "channel_maps_list": [{"map": [{"info": "info"}]}], + "contact": "cached@example.com", + "website": "https://cached.example.com", + "public_metrics_enabled": True, + "public_metrics_blacklist": False, + "license": "MIT", + "video_urls": [], + "categories": {"items": []}, + "status": "published", + "update_metadata_on_release": False, + "links": {"website": ["https://cached.example.com"]}, + } + + responses.add(responses.GET, self.api_url, json=payload, status=200) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + # First request + response1 = self.client.get(self.endpoint_url) + assert response1.status_code == 200 + data1 = response1.get_json() + + # Second request (from cache) + response2 = self.client.get(self.endpoint_url) + assert response2.status_code == 200 + data2 = response2.get_json() + + # Verify data matches + assert data1["data"]["snap_id"] == data2["data"]["snap_id"] + assert data1["data"]["title"] == data2["data"]["title"] + assert data1["data"]["summary"] == data2["data"]["summary"] + assert data1["data"]["description"] == data2["data"]["description"] diff --git a/tests/publisher/snaps/tests_post_listing.py b/tests/publisher/snaps/tests_post_listing.py index 7c794a30d0..6989fc95cd 100644 --- a/tests/publisher/snaps/tests_post_listing.py +++ b/tests/publisher/snaps/tests_post_listing.py @@ -2,10 +2,20 @@ import responses from tests.publisher.endpoint_testing import BaseTestCases +from cache.cache_utility import redis_cache class PostListingPageNotAuth(BaseTestCases.EndpointLoggedOut): def setUp(self): + # Clear cache before each test + if redis_cache.redis_available: + try: + redis_cache.client.flushdb() + except Exception: + pass + else: + redis_cache.fallback.clear() + snap_name = "test-snap" endpoint_url = "/api/{}/listing".format(snap_name) @@ -18,6 +28,15 @@ def setUp(self): class PostMetadataListingPage(BaseTestCases.EndpointLoggedIn): def setUp(self): + # Clear cache before each test + if redis_cache.redis_available: + try: + redis_cache.client.flushdb() + except Exception: + pass + else: + redis_cache.fallback.clear() + self.snap_id = "complexId" snap_name = "test-snap" @@ -102,3 +121,206 @@ def test_update_description_with_carriage_return(self): ) assert response.status_code == 200 + + @responses.activate + def test_cache_invalidation_after_metadata_update(self): + """Test cache invalidation after successful metadata update""" + snap_name = "test-snap" + + get_url = f"/api/{snap_name}/listing" + get_api_url = ( + f"https://dashboard.snapcraft.io/dev/api/snaps/info/" + f"{snap_name}" + ) + + get_payload = { + "snap_id": self.snap_id, + "snap_name": snap_name, + "title": "Original Title", + "summary": "Original summary", + "description": "Original description", + "media": [], + "publisher": { + "display-name": "Publisher", + "username": "user", + }, + "private": True, + "channel_maps_list": [{"map": [{"info": "info"}]}], + "contact": "contact@example.com", + "website": "https://example.com", + "public_metrics_enabled": True, + "public_metrics_blacklist": False, + "license": "MIT", + "video_urls": [], + "categories": {"items": []}, + "status": "published", + "update_metadata_on_release": True, + "links": {"website": ["https://example.com"]}, + } + + responses.add(responses.GET, get_api_url, json=get_payload, status=200) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + get_response = self.client.get(get_url) + assert get_response.status_code == 200 + + snap_info_calls_before = [ + call for call in responses.calls if get_api_url in call.request.url + ] + initial_snap_info_calls = len(snap_info_calls_before) + + # Second GET should use cache (no additional snap info API calls) + get_response_cached = self.client.get(get_url) + assert get_response_cached.status_code == 200 + snap_info_calls_cached = [ + call for call in responses.calls if get_api_url in call.request.url + ] + assert len(snap_info_calls_cached) == initial_snap_info_calls + + # Update metadata via POST (should invalidate cache) + responses.add(responses.PUT, self.api_url, json={}, status=200) + + changes = {"description": "Updated description"} + post_response = self.client.post( + self.endpoint_url, + data={ + "changes": json.dumps(changes), + "snap_id": self.snap_id, + }, + ) + assert post_response.status_code == 200 + + # Another GET request, should hit API + updated_payload = get_payload.copy() + updated_payload["description"] = "Updated description" + responses.add( + responses.GET, get_api_url, json=updated_payload, status=200 + ) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + get_response2 = self.client.get(get_url) + assert get_response2.status_code == 200 + + # Verify snap info API was called again after POST + snap_info_calls_after = [ + call for call in responses.calls if get_api_url in call.request.url + ] + assert len(snap_info_calls_after) == len(snap_info_calls_before) + 1 + + @responses.activate + def test_cache_invalidation_after_screenshot_update(self): + """Test cache invalidation after updating with images field""" + snap_name = "test-snap" + + get_url = f"/api/{snap_name}/listing" + get_api_url = ( + f"https://dashboard.snapcraft.io/dev/api/snaps/info/" + f"{snap_name}" + ) + + get_payload = { + "snap_id": self.snap_id, + "snap_name": snap_name, + "title": "Test Snap", + "summary": "Summary", + "description": "Description", + "media": [], + "publisher": { + "display-name": "Publisher", + "username": "user", + }, + "private": True, + "channel_maps_list": [{"map": [{"info": "info"}]}], + "contact": "contact@example.com", + "website": "https://example.com", + "public_metrics_enabled": True, + "public_metrics_blacklist": False, + "license": "MIT", + "video_urls": [], + "categories": {"items": []}, + "status": "published", + "update_metadata_on_release": True, + "links": {"website": ["https://example.com"]}, + } + + responses.add(responses.GET, get_api_url, json=get_payload, status=200) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + get_response = self.client.get(get_url) + assert get_response.status_code == 200 + + snap_info_calls_before = [ + call for call in responses.calls if get_api_url in call.request.url + ] + initial_snap_info_calls = len(snap_info_calls_before) + + # Second GET should hit cache + get_response_cached = self.client.get(get_url) + assert get_response_cached.status_code == 200 + snap_info_calls_cached = [ + call for call in responses.calls if get_api_url in call.request.url + ] + assert len(snap_info_calls_cached) == initial_snap_info_calls + + # update should trigger cache invalidation + screenshot_api_url = ( + f"https://dashboard.snapcraft.io/dev/api/snaps/" + f"{self.snap_id}/binary-metadata" + ) + + responses.add( + responses.GET, + screenshot_api_url, + json={"screenshot_urls": []}, + status=200, + ) + + responses.add( + responses.PUT, + screenshot_api_url, + json={}, + status=200, + ) + + changes = {"images": []} + post_response = self.client.post( + self.endpoint_url, + data={ + "changes": json.dumps(changes), + "snap_id": self.snap_id, + }, + ) + assert post_response.status_code == 200 + + responses.add(responses.GET, get_api_url, json=get_payload, status=200) + responses.add( + responses.GET, + "https://api.snapcraft.io/v2/snaps/categories?type=shared", + json=[], + status=200, + ) + + get_response3 = self.client.get(get_url) + assert get_response3.status_code == 200 + + # Verify snap info API was called again after POST + snap_info_calls_after = [ + call for call in responses.calls if get_api_url in call.request.url + ] + # Should have one more call than before (cache was invalidated) + assert len(snap_info_calls_after) == initial_snap_info_calls + 1 diff --git a/webapp/endpoints/publisher/listing.py b/webapp/endpoints/publisher/listing.py index 65d1e9b1e4..4d943a4897 100644 --- a/webapp/endpoints/publisher/listing.py +++ b/webapp/endpoints/publisher/listing.py @@ -23,18 +23,18 @@ dashboard = Dashboard(api_session) device_gateway = DeviceGW("snap", api_session) + def get_snap_info_key(snap_name): - user_id = flask.session.get( - "publisher", {} - ).get("identity_url", "").split("+id")[1] + user_id = flask.session.get("publisher", {}).get("identity_url", "") + cache_key = f"snapcraft:dashboard:snap_info:{snap_name}:{user_id}" + return cache_key - return f"{user_id}:snap_info:{snap_name}" @login_required def get_listing_data(snap_name): snap_info_key = get_snap_info_key(snap_name) snap_details = redis_cache.get(snap_info_key, expected_type=dict) - + if not snap_details: snap_details = dashboard.get_snap_info(flask.session, snap_name) redis_cache.set(snap_info_key, snap_details, ttl=3600) From f1ae166dd53c95c67a03e5140a04161f2f1875e6 Mon Sep 17 00:00:00 2001 From: codeEmpress1 Date: Wed, 19 Nov 2025 13:49:00 +0300 Subject: [PATCH 4/4] remove user-id from key --- webapp/endpoints/publisher/listing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/endpoints/publisher/listing.py b/webapp/endpoints/publisher/listing.py index 4d943a4897..119e8d54a1 100644 --- a/webapp/endpoints/publisher/listing.py +++ b/webapp/endpoints/publisher/listing.py @@ -25,8 +25,7 @@ def get_snap_info_key(snap_name): - user_id = flask.session.get("publisher", {}).get("identity_url", "") - cache_key = f"snapcraft:dashboard:snap_info:{snap_name}:{user_id}" + cache_key = f"snap_info:{snap_name}" return cache_key