From 1849fd3a6e985dc3040fec713e81c946320bf2c9 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 16:58:01 +0800 Subject: [PATCH 01/18] feat: get logins working with drill --- noxfiles/drill.yml | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index 3e36a202bf..92fc4a7530 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -1,16 +1,33 @@ # This file is used by "drill", a load testing utility +# https://github.com/fcsonline/drill -concurrency: 4 +concurrency: 1 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' -# There is a rate-limiter on the webserver, default is set to 2000 requests/min -iterations: 400 -rampup: 3 +iterations: 1 # The webserver shows performance issues quickly, so we only need 10 iterations +rampup: 2 # The number of seconds between concurrent runsThe serial steps to running this plan +# Sequential stress-test plan plan: - name: Health Check request: + method: GET url: /health - name: Worker Health Check request: + method: GET url: /health/workers + + - name: Database Health Check + request: + method: GET + url: /health/database + + - name: Login + request: + method: POST + body: '{"username": "root_user", "password": "VGVzdHBhc3N3b3JkMSE="}' + headers: + Content-Type: 'application/json' + url: /api/v1/login + assign: login From 1d60191b335cbbd86597296911d88f44961cd3e5 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 17:22:50 +0800 Subject: [PATCH 02/18] feat: add more endpoints --- noxfiles/drill.yml | 54 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index 92fc4a7530..f5108000bf 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -1,33 +1,59 @@ # This file is used by "drill", a load testing utility # https://github.com/fcsonline/drill -concurrency: 1 # How many instances of the `plan` get run concurrently +concurrency: 5 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' -iterations: 1 # The webserver shows performance issues quickly, so we only need 10 iterations -rampup: 2 # The number of seconds between concurrent runsThe serial steps to running this plan +iterations: 10 # The webserver shows performance issues quickly, so we only need 10 iterations +rampup: 2 # The number of seconds between starting concurrent runs -# Sequential stress-test plan +# This plan is run in its entirety, sequentially, for each iteration plan: - name: Health Check request: method: GET url: /health - - name: Worker Health Check + - name: Login + assign: login + request: + method: POST + body: '{"username": "root_user", "password": "VGVzdHBhc3N3b3JkMSE="}' + headers: + Content-Type: 'application/json' + url: /api/v1/login + + - name: Get Privacy-Experience [Basic] request: method: GET - url: /health/workers + url: /api/v1/privacy-experience - - name: Database Health Check + - name: Get Privacy-Experience [Include GVL] request: method: GET - url: /health/database + url: /api/v1/privacy-experience?include_gvl=1 - - name: Login + - name: Get Data Privacy Types request: - method: POST - body: '{"username": "root_user", "password": "VGVzdHBhc3N3b3JkMSE="}' + method: GET headers: - Content-Type: 'application/json' - url: /api/v1/login - assign: login + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/{{ item }} + with_items: + - data_use + - data_category + - data_subject + - data_qualifier + + - name: Get Systems + request: + method: GET + headers: + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/system + + - name: Get Datasets + request: + method: GET + headers: + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/dataset From 52293819ccf924fb466752444a31e10e89c1661a Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 17:36:51 +0800 Subject: [PATCH 03/18] feat: separate each endpoint into its own task --- noxfiles/drill.yml | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index f5108000bf..773569dcff 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -1,9 +1,9 @@ # This file is used by "drill", a load testing utility # https://github.com/fcsonline/drill -concurrency: 5 # How many instances of the `plan` get run concurrently +concurrency: 1 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' -iterations: 10 # The webserver shows performance issues quickly, so we only need 10 iterations +iterations: 2 # How many times to run the plan rampup: 2 # The number of seconds between starting concurrent runs # This plan is run in its entirety, sequentially, for each iteration @@ -32,17 +32,33 @@ plan: method: GET url: /api/v1/privacy-experience?include_gvl=1 - - name: Get Data Privacy Types + - name: Get Data Use request: method: GET headers: Authorization: "Bearer {{ login.body.token_data.access_token }}" - url: /api/v1/{{ item }} - with_items: - - data_use - - data_category - - data_subject - - data_qualifier + url: /api/v1/data_use + + - name: Get Data Category + request: + method: GET + headers: + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/data_category + + - name: Get Data Qualifier + request: + method: GET + headers: + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/data_qualifier + + - name: Get Data Subject + request: + method: GET + headers: + Authorization: "Bearer {{ login.body.token_data.access_token }}" + url: /api/v1/data_subject - name: Get Systems request: From 220f0605d34d864565e32c08dfc0091510c0aa53 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 17:52:30 +0800 Subject: [PATCH 04/18] feat: optimize the Drill test --- noxfiles/drill.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index 773569dcff..45f96b4ee1 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -3,8 +3,8 @@ concurrency: 1 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' -iterations: 2 # How many times to run the plan -rampup: 2 # The number of seconds between starting concurrent runs +iterations: 5 # How many times to run the plan +rampup: 1 # The number of seconds between starting concurrent runs # This plan is run in its entirety, sequentially, for each iteration plan: @@ -26,11 +26,15 @@ plan: request: method: GET url: /api/v1/privacy-experience + tags: + - privacy-experience - name: Get Privacy-Experience [Include GVL] request: method: GET url: /api/v1/privacy-experience?include_gvl=1 + tags: + - privacy-experience - name: Get Data Use request: From eb0d7e71a40a4d4cdae865514945b3eafe749ab1 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 18:20:03 +0800 Subject: [PATCH 05/18] feat: make the privacy-experience endpoint async --- noxfiles/drill.yml | 2 +- src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index 45f96b4ee1..36405ce4d3 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -1,7 +1,7 @@ # This file is used by "drill", a load testing utility # https://github.com/fcsonline/drill -concurrency: 1 # How many instances of the `plan` get run concurrently +concurrency: 5 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' iterations: 5 # How many times to run the plan rampup: 1 # The number of seconds between starting concurrent runs diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index c0ec960dc4..e2963ab5af 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -124,7 +124,7 @@ def _filter_experiences_by_region_or_country( response_model=Page[PrivacyExperienceResponse], ) @fides_limiter.limit(CONFIG.security.public_request_rate_limit) -def privacy_experience_list( +async def privacy_experience_list( *, db: Session = Depends(deps.get_db), params: Params = Depends(), From 800a94458cdb8bb9786b2cc88536408fe9f42f91 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 18:31:25 +0800 Subject: [PATCH 06/18] feat: sprinkle some `async sleep` magic into the privacy-experience endpoint --- noxfiles/drill.yml | 2 +- .../api/api/v1/endpoints/privacy_experience_endpoints.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/noxfiles/drill.yml b/noxfiles/drill.yml index 36405ce4d3..2b70fd2df1 100644 --- a/noxfiles/drill.yml +++ b/noxfiles/drill.yml @@ -3,7 +3,7 @@ concurrency: 5 # How many instances of the `plan` get run concurrently base: 'http://localhost:8080' -iterations: 5 # How many times to run the plan +iterations: 20 # How many times to run the plan rampup: 1 # The number of seconds between starting concurrent runs # This plan is run in its entirety, sequentially, for each iteration diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index e2963ab5af..4574935969 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,6 +1,7 @@ import uuid from html import escape, unescape from typing import Dict, List, Optional +import asyncio from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery @@ -174,6 +175,7 @@ async def privacy_experience_list( db=db, fides_user_device_id=fides_user_device_id ) + await asyncio.sleep(delay=0.001) experience_query = db.query(PrivacyExperience) if show_disabled is False: @@ -184,11 +186,13 @@ async def privacy_experience_list( PrivacyExperienceConfig.id == PrivacyExperience.experience_config_id, ).filter(PrivacyExperienceConfig.disabled.is_(False)) + await asyncio.sleep(delay=0.001) if region is not None: experience_query = _filter_experiences_by_region_or_country( db=db, region=region, experience_query=experience_query ) + await asyncio.sleep(delay=0.001) if component is not None: # Intentionally relaxes what is returned when querying for "overlay", by returning both types of overlays. # This way the frontend doesn't have to know which type of overlay, regular or tcf, just that it is an overlay. @@ -200,10 +204,12 @@ async def privacy_experience_list( component_search_map.get(component, [component]) ) ) + await asyncio.sleep(delay=0.001) if has_config is True: experience_query = experience_query.filter( PrivacyExperience.experience_config_id.isnot(None) ) + await asyncio.sleep(delay=0.001) if has_config is False: experience_query = experience_query.filter( PrivacyExperience.experience_config_id.is_(None) @@ -211,9 +217,11 @@ async def privacy_experience_list( results: List[PrivacyExperience] = [] should_unescape: Optional[str] = request.headers.get(UNESCAPE_SAFESTR_HEADER) + await asyncio.sleep(delay=0.001) for privacy_experience in experience_query.order_by( PrivacyExperience.created_at.desc() ): + await asyncio.sleep(delay=0.001) content_exists: bool = embed_experience_details( db, privacy_experience=privacy_experience, @@ -228,6 +236,7 @@ async def privacy_experience_list( continue # Temporarily save "show_banner" on the privacy experience object + await asyncio.sleep(delay=0.001) privacy_experience.show_banner = privacy_experience.get_should_show_banner( db, show_disabled ) From abe119c22a371e6863fdaa04572a40b193beb14a Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 18:58:04 +0800 Subject: [PATCH 07/18] feat: remove an unused requirement --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index de298b84a4..318f0a33f8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,6 @@ deepdiff==6.3.0 defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 -fastapi-caching[redis]==0.3.0 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 From 2ae8afbb1a75c671cff82dd91f6ff740f86e336c Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 19:10:23 +0800 Subject: [PATCH 08/18] fix: static checks --- src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 4574935969..f854a30a89 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,7 +1,7 @@ +import asyncio import uuid from html import escape, unescape from typing import Dict, List, Optional -import asyncio from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery From 1f1f12b19eb12451c9f7032a07700d3edd120a5e Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 29 Sep 2023 21:28:13 +0800 Subject: [PATCH 09/18] Add FastAPI Redis Caching --- requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index de298b84a4..37bbc15c66 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,7 @@ defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 fastapi-caching[redis]==0.3.0 +fastapi-cache2==0.2.1 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 @@ -41,7 +42,7 @@ PyMySQL==1.0.2 pymssql==2.2.8 python-jose[cryptography]==3.3.0 pyyaml==6.0.1 -redis==3.5.3 +redis==4.6.0 rich-click==1.6.1 sendgrid==6.9.7 slowapi==0.1.8 From 1347246b94483c2bb6dd3c373a99adcef6cb07e2 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 11 Oct 2023 17:11:27 +0800 Subject: [PATCH 10/18] fix: remove outdated redis import --- src/fides/api/util/cache.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/fides/api/util/cache.py b/src/fides/api/util/cache.py index a981a2f307..a68c3f7fee 100644 --- a/src/fides/api/util/cache.py +++ b/src/fides/api/util/cache.py @@ -7,7 +7,6 @@ from bson.objectid import ObjectId from loguru import logger from redis import Redis -from redis.client import Script # type: ignore from redis.exceptions import ConnectionError as ConnectionErrorFromRedis from fides.api import common_exceptions @@ -95,10 +94,9 @@ def get_keys_by_prefix(self, prefix: str, chunk_size: int = 1000) -> List[str]: def delete_keys_by_prefix(self, prefix: str) -> None: """Delete all keys starting with a given prefix""" - s: Script = self.register_script( + self.register_script( f"for _,k in ipairs(redis.call('keys','{prefix}*')) do redis.call('del',k) end" - ) - s() + )() def get_values(self, keys: List[str]) -> Dict[str, Optional[Any]]: """Retrieve all values corresponding to the set of input keys and return them as a From 161e271b1a50a126eca822537fdf382222e0f350 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 11 Oct 2023 20:40:40 +0800 Subject: [PATCH 11/18] feat: add caching to the privacy-experience endpoint --- requirements.txt | 1 + .../endpoints/privacy_experience_endpoints.py | 3 +++ src/fides/api/app_setup.py | 24 +++++++++++++++++++ src/fides/api/main.py | 3 +++ 4 files changed, 31 insertions(+) diff --git a/requirements.txt b/requirements.txt index 954fd5923d..13216a56b5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ deepdiff==6.3.0 defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 +fastapi-cache2[redis]==0.2.1 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 0a4698f0ad..376c5a409c 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -16,6 +16,8 @@ HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, ) +from fastapi_cache.decorator import cache as fastapi_cache + from fides.api.api import deps from fides.api.models.privacy_experience import ( @@ -124,6 +126,7 @@ def _filter_experiences_by_region_or_country( status_code=HTTP_200_OK, response_model=Page[PrivacyExperienceResponse], ) +@fastapi_cache(expire=43200) # Cached items live for 12 hours @fides_limiter.limit(CONFIG.security.public_request_rate_limit) async def privacy_experience_list( *, diff --git a/src/fides/api/app_setup.py b/src/fides/api/app_setup.py index 51f22c989f..b6edd84266 100644 --- a/src/fides/api/app_setup.py +++ b/src/fides/api/app_setup.py @@ -14,6 +14,10 @@ from slowapi.extension import _rate_limit_exceeded_handler # type: ignore from slowapi.middleware import SlowAPIMiddleware # type: ignore from starlette.middleware.cors import CORSMiddleware +from fastapi_cache import FastAPICache +from fastapi_cache.backends.redis import RedisBackend + +from redis import asyncio as aioredis import fides from fides.api.api.deps import get_api_session @@ -75,6 +79,26 @@ ) +async def init_fastapi_redis_cache(): + """Initialize a Redis-backed cache to store API responses.""" + redis_url = "{}://{}:{}@{}:{}/{}".format( + "rediss" if CONFIG.redis.ssl else "redis", + CONFIG.redis.user, + CONFIG.redis.password, + CONFIG.redis.host, + CONFIG.redis.port, + CONFIG.redis.db_index, + ) + redis = aioredis.from_url( + url=redis_url, + charset=CONFIG.redis.charset, + decode_responses=CONFIG.redis.decode_responses, + ssl_ca_certs=CONFIG.redis.ssl_ca_certs, + ssl_cert_reqs=CONFIG.redis.ssl_cert_reqs, + ) + FastAPICache.init(RedisBackend(redis), prefix="fastapi-cache") + + def create_fides_app( cors_origins: List[AnyUrl] = CONFIG.security.cors_origins, cors_origin_regex: Optional[Pattern] = CONFIG.security.cors_origin_regex, diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 94cde18c54..1d3bb4c59d 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -17,12 +17,14 @@ from starlette.background import BackgroundTask from uvicorn import Config, Server + import fides from fides.api.app_setup import ( check_redis, create_fides_app, log_startup, run_database_startup, + init_fastapi_redis_cache, ) from fides.api.common_exceptions import MalisciousUrlException from fides.api.middleware import handle_audit_log_resource @@ -259,6 +261,7 @@ async def setup_server() -> None: await run_database_startup(app) check_redis() + await init_fastapi_redis_cache() if not scheduler.running: scheduler.start() From 38f32bcadffede7fa3f2e9a43d0a3aca8a6cd9ce Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 12 Oct 2023 15:18:41 +0800 Subject: [PATCH 12/18] feat: rough POC of custom caching for the privacy experience endpoint --- .../endpoints/privacy_experience_endpoints.py | 40 ++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 376c5a409c..0dc1f44bad 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,7 +1,7 @@ import asyncio import uuid from html import escape, unescape -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery @@ -16,7 +16,7 @@ HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, ) -from fastapi_cache.decorator import cache as fastapi_cache +from functools import lru_cache from fides.api.api import deps @@ -49,6 +49,7 @@ router = APIRouter(tags=["Privacy Experience"], prefix=urls.V1_URL_PREFIX) +@lru_cache(maxsize=20, typed=True) def get_privacy_experience_or_error( db: Session, experience_id: str ) -> PrivacyExperience: @@ -66,6 +67,7 @@ def get_privacy_experience_or_error( return privacy_experience +@lru_cache(maxsize=20, typed=True) def _filter_experiences_by_region_or_country( db: Session, region: Optional[str], experience_query: Query ) -> Query: @@ -121,17 +123,16 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) +EPIC_CACHE: Dict[str, List] = {} + + @router.get( urls.PRIVACY_EXPERIENCE, status_code=HTTP_200_OK, - response_model=Page[PrivacyExperienceResponse], ) -@fastapi_cache(expire=43200) # Cached items live for 12 hours -@fides_limiter.limit(CONFIG.security.public_request_rate_limit) async def privacy_experience_list( *, db: Session = Depends(deps.get_db), - params: Params = Depends(), show_disabled: Optional[bool] = True, region: Optional[str] = None, component: Optional[ComponentType] = None, @@ -143,7 +144,7 @@ async def privacy_experience_list( include_meta: Optional[bool] = False, request: Request, # required for rate limiting response: Response, # required for rate limiting -) -> AbstractPage[PrivacyExperience]: +) -> List: """ Public endpoint that returns a list of PrivacyExperience records for individual regions with relevant privacy notices or tcf contents embedded in the response. @@ -152,7 +153,6 @@ async def privacy_experience_list( notices as well. :param db: - :param params: :param show_disabled: If False, returns only enabled Experiences and Notices :param region: Return the Experiences for the given region :param component: Returns Experiences of the given component type @@ -166,7 +166,26 @@ async def privacy_experience_list( :param response: :return: """ - logger.info("Finding all Privacy Experiences with pagination params '{}'", params) + + param_hash_list = [ + show_disabled, + region, + component, + content_required, + has_config, + fides_user_device_id, + systems_applicable, + include_gvl, + include_meta, + ] + # Create a custom hash that avoids unhashable parameters + cache_hash = "_".join([repr(x) for x in param_hash_list]) + + if cache_hash in EPIC_CACHE.keys(): + logger.debug("Cache HIT: {}", cache_hash) + return EPIC_CACHE[cache_hash] + logger.debug("Cache MISS: {}", cache_hash) + fides_user_provided_identity: Optional[ProvidedIdentity] = None if fides_user_device_id: try: @@ -261,8 +280,9 @@ async def privacy_experience_list( ) results.append(privacy_experience) + EPIC_CACHE[cache_hash] = results - return fastapi_paginate(results, params=params) + return results def embed_experience_details( From 4d50fd9a8119c338704c589af280badcb695f48b Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 12 Oct 2023 15:56:47 +0800 Subject: [PATCH 13/18] feat: additional cleanup --- requirements.txt | 1 - .../endpoints/privacy_experience_endpoints.py | 18 +++++++------- src/fides/api/app_setup.py | 24 ------------------- src/fides/api/main.py | 2 -- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/requirements.txt b/requirements.txt index 13216a56b5..954fd5923d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,6 @@ deepdiff==6.3.0 defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 -fastapi-cache2[redis]==0.2.1 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 0dc1f44bad..8a494f505c 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,7 +1,7 @@ import asyncio import uuid from html import escape, unescape -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery @@ -16,7 +16,6 @@ HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, ) -from functools import lru_cache from fides.api.api import deps @@ -49,7 +48,6 @@ router = APIRouter(tags=["Privacy Experience"], prefix=urls.V1_URL_PREFIX) -@lru_cache(maxsize=20, typed=True) def get_privacy_experience_or_error( db: Session, experience_id: str ) -> PrivacyExperience: @@ -67,7 +65,6 @@ def get_privacy_experience_or_error( return privacy_experience -@lru_cache(maxsize=20, typed=True) def _filter_experiences_by_region_or_country( db: Session, region: Optional[str], experience_query: Query ) -> Query: @@ -123,16 +120,19 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) -EPIC_CACHE: Dict[str, List] = {} +EPIC_CACHE: Dict[str, AbstractPage[PrivacyExperience]] = {} @router.get( urls.PRIVACY_EXPERIENCE, status_code=HTTP_200_OK, + response_model=Page[PrivacyExperienceResponse], ) +@fides_limiter.limit(CONFIG.security.public_request_rate_limit) async def privacy_experience_list( *, db: Session = Depends(deps.get_db), + params: Params = Depends(), show_disabled: Optional[bool] = True, region: Optional[str] = None, component: Optional[ComponentType] = None, @@ -144,7 +144,7 @@ async def privacy_experience_list( include_meta: Optional[bool] = False, request: Request, # required for rate limiting response: Response, # required for rate limiting -) -> List: +) -> AbstractPage[PrivacyExperience]: """ Public endpoint that returns a list of PrivacyExperience records for individual regions with relevant privacy notices or tcf contents embedded in the response. @@ -153,6 +153,7 @@ async def privacy_experience_list( notices as well. :param db: + :param params: Special case used for Pagination :param show_disabled: If False, returns only enabled Experiences and Notices :param region: Return the Experiences for the given region :param component: Returns Experiences of the given component type @@ -280,9 +281,10 @@ async def privacy_experience_list( ) results.append(privacy_experience) - EPIC_CACHE[cache_hash] = results - return results + paginated_results = fastapi_paginate(results, params=params) + EPIC_CACHE[cache_hash] = paginated_results + return paginated_results def embed_experience_details( diff --git a/src/fides/api/app_setup.py b/src/fides/api/app_setup.py index b6edd84266..51f22c989f 100644 --- a/src/fides/api/app_setup.py +++ b/src/fides/api/app_setup.py @@ -14,10 +14,6 @@ from slowapi.extension import _rate_limit_exceeded_handler # type: ignore from slowapi.middleware import SlowAPIMiddleware # type: ignore from starlette.middleware.cors import CORSMiddleware -from fastapi_cache import FastAPICache -from fastapi_cache.backends.redis import RedisBackend - -from redis import asyncio as aioredis import fides from fides.api.api.deps import get_api_session @@ -79,26 +75,6 @@ ) -async def init_fastapi_redis_cache(): - """Initialize a Redis-backed cache to store API responses.""" - redis_url = "{}://{}:{}@{}:{}/{}".format( - "rediss" if CONFIG.redis.ssl else "redis", - CONFIG.redis.user, - CONFIG.redis.password, - CONFIG.redis.host, - CONFIG.redis.port, - CONFIG.redis.db_index, - ) - redis = aioredis.from_url( - url=redis_url, - charset=CONFIG.redis.charset, - decode_responses=CONFIG.redis.decode_responses, - ssl_ca_certs=CONFIG.redis.ssl_ca_certs, - ssl_cert_reqs=CONFIG.redis.ssl_cert_reqs, - ) - FastAPICache.init(RedisBackend(redis), prefix="fastapi-cache") - - def create_fides_app( cors_origins: List[AnyUrl] = CONFIG.security.cors_origins, cors_origin_regex: Optional[Pattern] = CONFIG.security.cors_origin_regex, diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 1d3bb4c59d..221463824c 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -24,7 +24,6 @@ create_fides_app, log_startup, run_database_startup, - init_fastapi_redis_cache, ) from fides.api.common_exceptions import MalisciousUrlException from fides.api.middleware import handle_audit_log_resource @@ -261,7 +260,6 @@ async def setup_server() -> None: await run_database_startup(app) check_redis() - await init_fastapi_redis_cache() if not scheduler.running: scheduler.start() From fdf1d9568a0fa0d95e3084487d8efbfde6269095 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 12 Oct 2023 16:08:42 +0800 Subject: [PATCH 14/18] Revert "feat: additional cleanup" This reverts commit 4d50fd9a8119c338704c589af280badcb695f48b. --- requirements.txt | 1 + .../endpoints/privacy_experience_endpoints.py | 18 +++++++------- src/fides/api/app_setup.py | 24 +++++++++++++++++++ src/fides/api/main.py | 2 ++ 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/requirements.txt b/requirements.txt index 954fd5923d..13216a56b5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,6 +11,7 @@ deepdiff==6.3.0 defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 +fastapi-cache2[redis]==0.2.1 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 8a494f505c..0dc1f44bad 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,7 +1,7 @@ import asyncio import uuid from html import escape, unescape -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery @@ -16,6 +16,7 @@ HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, ) +from functools import lru_cache from fides.api.api import deps @@ -48,6 +49,7 @@ router = APIRouter(tags=["Privacy Experience"], prefix=urls.V1_URL_PREFIX) +@lru_cache(maxsize=20, typed=True) def get_privacy_experience_or_error( db: Session, experience_id: str ) -> PrivacyExperience: @@ -65,6 +67,7 @@ def get_privacy_experience_or_error( return privacy_experience +@lru_cache(maxsize=20, typed=True) def _filter_experiences_by_region_or_country( db: Session, region: Optional[str], experience_query: Query ) -> Query: @@ -120,19 +123,16 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) -EPIC_CACHE: Dict[str, AbstractPage[PrivacyExperience]] = {} +EPIC_CACHE: Dict[str, List] = {} @router.get( urls.PRIVACY_EXPERIENCE, status_code=HTTP_200_OK, - response_model=Page[PrivacyExperienceResponse], ) -@fides_limiter.limit(CONFIG.security.public_request_rate_limit) async def privacy_experience_list( *, db: Session = Depends(deps.get_db), - params: Params = Depends(), show_disabled: Optional[bool] = True, region: Optional[str] = None, component: Optional[ComponentType] = None, @@ -144,7 +144,7 @@ async def privacy_experience_list( include_meta: Optional[bool] = False, request: Request, # required for rate limiting response: Response, # required for rate limiting -) -> AbstractPage[PrivacyExperience]: +) -> List: """ Public endpoint that returns a list of PrivacyExperience records for individual regions with relevant privacy notices or tcf contents embedded in the response. @@ -153,7 +153,6 @@ async def privacy_experience_list( notices as well. :param db: - :param params: Special case used for Pagination :param show_disabled: If False, returns only enabled Experiences and Notices :param region: Return the Experiences for the given region :param component: Returns Experiences of the given component type @@ -281,10 +280,9 @@ async def privacy_experience_list( ) results.append(privacy_experience) + EPIC_CACHE[cache_hash] = results - paginated_results = fastapi_paginate(results, params=params) - EPIC_CACHE[cache_hash] = paginated_results - return paginated_results + return results def embed_experience_details( diff --git a/src/fides/api/app_setup.py b/src/fides/api/app_setup.py index 51f22c989f..b6edd84266 100644 --- a/src/fides/api/app_setup.py +++ b/src/fides/api/app_setup.py @@ -14,6 +14,10 @@ from slowapi.extension import _rate_limit_exceeded_handler # type: ignore from slowapi.middleware import SlowAPIMiddleware # type: ignore from starlette.middleware.cors import CORSMiddleware +from fastapi_cache import FastAPICache +from fastapi_cache.backends.redis import RedisBackend + +from redis import asyncio as aioredis import fides from fides.api.api.deps import get_api_session @@ -75,6 +79,26 @@ ) +async def init_fastapi_redis_cache(): + """Initialize a Redis-backed cache to store API responses.""" + redis_url = "{}://{}:{}@{}:{}/{}".format( + "rediss" if CONFIG.redis.ssl else "redis", + CONFIG.redis.user, + CONFIG.redis.password, + CONFIG.redis.host, + CONFIG.redis.port, + CONFIG.redis.db_index, + ) + redis = aioredis.from_url( + url=redis_url, + charset=CONFIG.redis.charset, + decode_responses=CONFIG.redis.decode_responses, + ssl_ca_certs=CONFIG.redis.ssl_ca_certs, + ssl_cert_reqs=CONFIG.redis.ssl_cert_reqs, + ) + FastAPICache.init(RedisBackend(redis), prefix="fastapi-cache") + + def create_fides_app( cors_origins: List[AnyUrl] = CONFIG.security.cors_origins, cors_origin_regex: Optional[Pattern] = CONFIG.security.cors_origin_regex, diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 221463824c..1d3bb4c59d 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -24,6 +24,7 @@ create_fides_app, log_startup, run_database_startup, + init_fastapi_redis_cache, ) from fides.api.common_exceptions import MalisciousUrlException from fides.api.middleware import handle_audit_log_resource @@ -260,6 +261,7 @@ async def setup_server() -> None: await run_database_startup(app) check_redis() + await init_fastapi_redis_cache() if not scheduler.running: scheduler.start() From cdb6e49b4930161480af93dd658949338a8140f7 Mon Sep 17 00:00:00 2001 From: Thomas Date: Thu, 12 Oct 2023 16:43:05 +0800 Subject: [PATCH 15/18] feat: cleanup, passing more tests --- requirements.txt | 1 - .../endpoints/privacy_experience_endpoints.py | 16 ++++++------- src/fides/api/app_setup.py | 24 ------------------- src/fides/api/main.py | 2 -- 4 files changed, 8 insertions(+), 35 deletions(-) diff --git a/requirements.txt b/requirements.txt index 13216a56b5..954fd5923d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,6 @@ deepdiff==6.3.0 defusedxml==0.7.1 expandvars==0.9.0 fastapi[all]==0.89.1 -fastapi-cache2[redis]==0.2.1 fastapi-pagination[sqlalchemy]==0.11.4 fideslang==2.1.0 fideslog==1.2.10 diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 0dc1f44bad..9a489d2b88 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,14 +1,11 @@ import asyncio import uuid from html import escape, unescape -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional from fastapi import Depends, HTTPException from fastapi import Query as FastAPIQuery from fastapi import Request, Response -from fastapi_pagination import Page, Params -from fastapi_pagination import paginate as fastapi_paginate -from fastapi_pagination.bases import AbstractPage from loguru import logger from sqlalchemy.orm import Query, Session from starlette.status import ( @@ -123,7 +120,7 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) -EPIC_CACHE: Dict[str, List] = {} +EPIC_CACHE: Dict[str, Dict] = {} @router.get( @@ -144,7 +141,7 @@ async def privacy_experience_list( include_meta: Optional[bool] = False, request: Request, # required for rate limiting response: Response, # required for rate limiting -) -> List: +) -> Dict: """ Public endpoint that returns a list of PrivacyExperience records for individual regions with relevant privacy notices or tcf contents embedded in the response. @@ -280,9 +277,12 @@ async def privacy_experience_list( ) results.append(privacy_experience) - EPIC_CACHE[cache_hash] = results - return results + # This is structured to look like a paginated result to minimize impact from + # the caching changes + api_result = {"items": results, "total": len(results)} + EPIC_CACHE[cache_hash] = api_result + return api_result def embed_experience_details( diff --git a/src/fides/api/app_setup.py b/src/fides/api/app_setup.py index b6edd84266..51f22c989f 100644 --- a/src/fides/api/app_setup.py +++ b/src/fides/api/app_setup.py @@ -14,10 +14,6 @@ from slowapi.extension import _rate_limit_exceeded_handler # type: ignore from slowapi.middleware import SlowAPIMiddleware # type: ignore from starlette.middleware.cors import CORSMiddleware -from fastapi_cache import FastAPICache -from fastapi_cache.backends.redis import RedisBackend - -from redis import asyncio as aioredis import fides from fides.api.api.deps import get_api_session @@ -79,26 +75,6 @@ ) -async def init_fastapi_redis_cache(): - """Initialize a Redis-backed cache to store API responses.""" - redis_url = "{}://{}:{}@{}:{}/{}".format( - "rediss" if CONFIG.redis.ssl else "redis", - CONFIG.redis.user, - CONFIG.redis.password, - CONFIG.redis.host, - CONFIG.redis.port, - CONFIG.redis.db_index, - ) - redis = aioredis.from_url( - url=redis_url, - charset=CONFIG.redis.charset, - decode_responses=CONFIG.redis.decode_responses, - ssl_ca_certs=CONFIG.redis.ssl_ca_certs, - ssl_cert_reqs=CONFIG.redis.ssl_cert_reqs, - ) - FastAPICache.init(RedisBackend(redis), prefix="fastapi-cache") - - def create_fides_app( cors_origins: List[AnyUrl] = CONFIG.security.cors_origins, cors_origin_regex: Optional[Pattern] = CONFIG.security.cors_origin_regex, diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 1d3bb4c59d..221463824c 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -24,7 +24,6 @@ create_fides_app, log_startup, run_database_startup, - init_fastapi_redis_cache, ) from fides.api.common_exceptions import MalisciousUrlException from fides.api.middleware import handle_audit_log_resource @@ -261,7 +260,6 @@ async def setup_server() -> None: await run_database_startup(app) check_redis() - await init_fastapi_redis_cache() if not scheduler.running: scheduler.start() From be70d0c84d724497d86a5c304e81a1c10e3aabc1 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 13 Oct 2023 12:11:49 +0800 Subject: [PATCH 16/18] feat: add headers to the privacy-experience response object indicating whether or not the cache was a hit or miss --- .../endpoints/privacy_experience_endpoints.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 9a489d2b88..cd5ca9fa32 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -45,6 +45,8 @@ router = APIRouter(tags=["Privacy Experience"], prefix=urls.V1_URL_PREFIX) +PRIVACY_EXPERIENCE_CACHE: Dict[str, Dict] = {} + @lru_cache(maxsize=20, typed=True) def get_privacy_experience_or_error( @@ -120,9 +122,6 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) -EPIC_CACHE: Dict[str, Dict] = {} - - @router.get( urls.PRIVACY_EXPERIENCE, status_code=HTTP_200_OK, @@ -164,6 +163,7 @@ async def privacy_experience_list( :return: """ + # These are the parameters that get used to create the cache. param_hash_list = [ show_disabled, region, @@ -178,10 +178,13 @@ async def privacy_experience_list( # Create a custom hash that avoids unhashable parameters cache_hash = "_".join([repr(x) for x in param_hash_list]) - if cache_hash in EPIC_CACHE.keys(): + if cache_hash in PRIVACY_EXPERIENCE_CACHE.keys(): logger.debug("Cache HIT: {}", cache_hash) - return EPIC_CACHE[cache_hash] - logger.debug("Cache MISS: {}", cache_hash) + response.headers["X-Endpoint-Cache"] = "HIT" + return PRIVACY_EXPERIENCE_CACHE[cache_hash] + else: + logger.debug("Cache MISS: {}", cache_hash) + response.headers["x-Endpoint-Cache"] = "MISS" fides_user_provided_identity: Optional[ProvidedIdentity] = None if fides_user_device_id: @@ -281,7 +284,7 @@ async def privacy_experience_list( # This is structured to look like a paginated result to minimize impact from # the caching changes api_result = {"items": results, "total": len(results)} - EPIC_CACHE[cache_hash] = api_result + PRIVACY_EXPERIENCE_CACHE[cache_hash] = api_result return api_result From 2b64527d4ac629ab5375805fbb42c778fe027632 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 13 Oct 2023 12:56:48 +0800 Subject: [PATCH 17/18] checkin: add headers and cache tests, but not passing --- .../endpoints/privacy_experience_endpoints.py | 10 ++- .../test_privacy_experience_endpoints.py | 61 ++++++++++++++++++- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index cd5ca9fa32..40c699cee4 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -45,6 +45,8 @@ router = APIRouter(tags=["Privacy Experience"], prefix=urls.V1_URL_PREFIX) +BUST_CACHE_HEADER = "bust-endpoint-cache" +CACHE_HEADER = "X-Endpoint-Cache" PRIVACY_EXPERIENCE_CACHE: Dict[str, Dict] = {} @@ -162,6 +164,7 @@ async def privacy_experience_list( :param response: :return: """ + global PRIVACY_EXPERIENCE_CACHE # These are the parameters that get used to create the cache. param_hash_list = [ @@ -178,13 +181,16 @@ async def privacy_experience_list( # Create a custom hash that avoids unhashable parameters cache_hash = "_".join([repr(x) for x in param_hash_list]) + if request.headers.get(BUST_CACHE_HEADER): + PRIVACY_EXPERIENCE_CACHE.clear() + if cache_hash in PRIVACY_EXPERIENCE_CACHE.keys(): logger.debug("Cache HIT: {}", cache_hash) - response.headers["X-Endpoint-Cache"] = "HIT" + response.headers[CACHE_HEADER] = "HIT" return PRIVACY_EXPERIENCE_CACHE[cache_hash] else: logger.debug("Cache MISS: {}", cache_hash) - response.headers["x-Endpoint-Cache"] = "MISS" + response.headers[CACHE_HEADER] = "MISS" fides_user_provided_identity: Optional[ProvidedIdentity] = None if fides_user_device_id: diff --git a/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py index 8e55fff0e2..ab2638bca4 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py @@ -1,4 +1,5 @@ from __future__ import annotations +from typing import Dict import pytest from starlette.status import HTTP_200_OK @@ -6,12 +7,35 @@ from fides.api.api.v1.endpoints.privacy_experience_endpoints import ( _filter_experiences_by_region_or_country, + BUST_CACHE_HEADER, + CACHE_HEADER, ) from fides.api.models.privacy_experience import ComponentType, PrivacyExperience from fides.api.models.privacy_notice import ConsentMechanism from fides.common.api.v1.urn_registry import PRIVACY_EXPERIENCE, V1_URL_PREFIX +def get_cache_bust_headers() -> Dict: + return {BUST_CACHE_HEADER: "true"} + + +class TestGetPrivacyExperiencesCaching: + def test_cache_header_hit(self, url, api_client): + """Check that the header describing cache hits/misses is working.""" + api_client.get(url) + resp = api_client.get(url) + cache_header = resp.headers.get(CACHE_HEADER) + assert cache_header + assert cache_header == "HIT" + + def test_bust_cache_header(self, url, api_client): + """Check that the header to bust the cache is working.""" + resp = api_client.get(url, headers=get_cache_bust_headers()) + cache_header = resp.headers.get(CACHE_HEADER) + assert cache_header + assert cache_header == "MISS" + + class TestGetPrivacyExperiences: @pytest.fixture(scope="function") def url(self) -> str: @@ -47,7 +71,7 @@ def test_get_privacy_experiences( privacy_notice, privacy_experience_privacy_center, ): - unescape_header = {"Unescape-Safestr": "true"} + unescape_header = {"Unescape-Safestr": "true", BUST_CACHE_HEADER: "true"} resp = api_client.get(url + "?include_gvl=True", headers=unescape_header) assert resp.status_code == 200 @@ -98,7 +122,7 @@ def test_get_experiences_unescaped( privacy_experience_privacy_center, ): # Assert not escaped without proper request header - resp = api_client.get(url) + resp = api_client.get(url, headers=get_cache_bust_headers()) resp = resp.json()["items"][0] experience_config = resp["experience_config"] assert ( @@ -120,6 +144,7 @@ def test_get_privacy_experiences_show_disabled_filter( ): resp = api_client.get( url, + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -128,6 +153,7 @@ def test_get_privacy_experiences_show_disabled_filter( resp = api_client.get( url + "?show_disabled=False", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -144,6 +170,7 @@ def test_get_privacy_experiences_show_disabled_filter( privacy_experience_overlay.unlink_experience_config(db) resp = api_client.get( url + "?show_disabled=False", + headers=get_cache_bust_headers(), ) data = resp.json() assert ( @@ -162,6 +189,7 @@ def test_get_privacy_experiences_region_filter( ): resp = api_client.get( url + "?region=us_co", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -172,6 +200,7 @@ def test_get_privacy_experiences_region_filter( resp = api_client.get( url + "?region=us_ca", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -184,6 +213,7 @@ def test_get_privacy_experiences_region_filter( resp = api_client.get( url + "?region=bad_region", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert resp.json()["total"] == 0 @@ -204,12 +234,14 @@ def assert_france_experience_and_notices_returned(resp): response = api_client.get( url + "?region=fr_idg", + headers=get_cache_bust_headers(), ) # There are no experiences with "fr_idg" so we fell back to searching for "fr" assert_france_experience_and_notices_returned(response) response = api_client.get( url + "?region=FR-IDG", + headers=get_cache_bust_headers(), ) # Case insensitive and hyphens also work here -" assert_france_experience_and_notices_returned(response) @@ -224,6 +256,7 @@ def test_get_privacy_experiences_components_filter( ): resp = api_client.get( url + "?component=overlay", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -236,6 +269,7 @@ def test_get_privacy_experiences_components_filter( resp = api_client.get( url + "?component=privacy_center", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -247,6 +281,7 @@ def test_get_privacy_experiences_components_filter( resp = api_client.get( url + "?component=tcf_overlay", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -258,6 +293,7 @@ def test_get_privacy_experiences_components_filter( resp = api_client.get( url + "?component=bad_type", + headers=get_cache_bust_headers(), ) assert resp.status_code == 422 @@ -270,6 +306,7 @@ def test_get_privacy_experiences_has_notices_no_notices( ): resp = api_client.get( url + "?has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -286,6 +323,7 @@ def test_get_privacy_experiences_has_notices_no_regions_overlap( ): resp = api_client.get( url + "?has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -309,6 +347,7 @@ def test_get_privacy_experiences_has_notices( ): resp = api_client.get( url + "?has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -409,6 +448,7 @@ def assert_expected_filtered_region_response(data): # Filter on exact match region resp = api_client.get( url + "?has_notices=True®ion=us_co", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 response_json = resp.json() @@ -418,6 +458,7 @@ def assert_expected_filtered_region_response(data): # Filter on upper case and hyphens resp = api_client.get( url + "?has_notices=True®ion=US-CO", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert_expected_filtered_region_response(resp.json()) @@ -442,6 +483,7 @@ def test_filter_on_systems_applicable( """For systems applicable filter, notices are only embedded if they are relevant to a system""" resp = api_client.get( url + "?region=us_co", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -466,6 +508,7 @@ def test_filter_on_systems_applicable( resp = api_client.get( url + "?region=us_co&systems_applicable=True", + headers=get_cache_bust_headers(), ) notices = resp.json()["items"][0]["privacy_notices"] assert len(notices) == 1 @@ -502,6 +545,7 @@ def test_filter_on_notices_and_region_and_show_disabled_is_false( resp = api_client.get( url + "?has_notices=True®ion=us_ca&show_disabled=False", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -528,6 +572,7 @@ def test_get_privacy_experiences_show_has_config_filter( ): resp = api_client.get( url + "?has_config=False", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -536,6 +581,7 @@ def test_get_privacy_experiences_show_has_config_filter( resp = api_client.get( url + "?has_config=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -552,6 +598,7 @@ def test_get_privacy_experiences_show_has_config_filter( privacy_experience_privacy_center.save(db=db) resp = api_client.get( url + "?has_config=False", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -566,6 +613,7 @@ def test_get_privacy_experiences_bad_fides_user_device_id_filter( ): resp = api_client.get( url + "?fides_user_device_id=does_not_exist", + headers=get_cache_bust_headers(), ) assert resp.status_code == 422 assert resp.json()["detail"] == "Invalid fides user device id format" @@ -583,6 +631,7 @@ def test_get_privacy_experiences_nonexistent_fides_user_device_id_filter( ): resp = api_client.get( url + "?cd685ccd-0960-4dc1-b9ca-7e810ebc5c1b", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json() @@ -615,6 +664,7 @@ def test_get_privacy_experiences_fides_user_device_id_filter( ): resp = api_client.get( url + "?fides_user_device_id=051b219f-20e4-45df-82f7-5eb68a00889f", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json()["items"][0] @@ -639,6 +689,7 @@ def test_get_privacy_experiences_fides_user_device_id_filter( assert privacy_notice_us_ca_provide.description == "new_description" resp = api_client.get( url + "?fides_user_device_id=051b219f-20e4-45df-82f7-5eb68a00889f", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 data = resp.json()["items"][0] @@ -668,6 +719,7 @@ def test_tcf_not_enabled( ): resp = api_client.get( url + "?region=fr&component=overlay&include_gvl=True&include_meta=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 1 @@ -705,6 +757,7 @@ def test_tcf_enabled_but_no_relevant_systems( ): resp = api_client.get( url + "?region=fr&component=overlay&include_gvl=True&include_meta=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 1 @@ -731,6 +784,7 @@ def test_tcf_enabled_but_no_relevant_systems( # Has notices = True flag will keep this experience from appearing altogether resp = api_client.get( url + "?region=fr&has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 0 @@ -756,6 +810,7 @@ def test_tcf_enabled_with_overlapping_vendors( resp = api_client.get( url + "?region=fr&component=overlay&fides_user_device_id=051b219f-20e4-45df-82f7-5eb68a00889f&has_notices=True&include_gvl=True&include_meta=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 1 @@ -851,6 +906,7 @@ def test_tcf_enabled_with_overlapping_systems( resp = api_client.get( url + "?region=fr&component=overlay&fides_user_device_id=051b219f-20e4-45df-82f7-5eb68a00889f&has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 1 @@ -914,6 +970,7 @@ def test_tcf_enabled_with_legitimate_interest_purpose( resp = api_client.get( url + "?region=fr&component=overlay&fides_user_device_id=051b219f-20e4-45df-82f7-5eb68a00889f&has_notices=True", + headers=get_cache_bust_headers(), ) assert resp.status_code == 200 assert len(resp.json()["items"]) == 1 From a8a45a2ac7e50404ac63ca7c364c0b83e0b46eb1 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 13 Oct 2023 14:50:55 +0800 Subject: [PATCH 18/18] fix: more tests and almost all static checks --- .../v1/endpoints/privacy_experience_endpoints.py | 14 ++++++-------- src/fides/api/main.py | 1 - src/fides/api/util/cache.py | 1 + .../endpoints/test_privacy_experience_endpoints.py | 12 +++++++----- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py index 40c699cee4..24b124123f 100644 --- a/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py +++ b/src/fides/api/api/v1/endpoints/privacy_experience_endpoints.py @@ -1,5 +1,6 @@ import asyncio import uuid +from functools import lru_cache from html import escape, unescape from typing import Dict, List, Optional @@ -13,8 +14,6 @@ HTTP_404_NOT_FOUND, HTTP_422_UNPROCESSABLE_ENTITY, ) -from functools import lru_cache - from fides.api.api import deps from fides.api.models.privacy_experience import ( @@ -24,7 +23,6 @@ ) from fides.api.models.privacy_notice import PrivacyNotice from fides.api.models.privacy_request import ProvidedIdentity -from fides.api.schemas.privacy_experience import PrivacyExperienceResponse from fides.api.util.api_router import APIRouter from fides.api.util.consent_util import ( PRIVACY_EXPERIENCE_ESCAPE_FIELDS, @@ -124,6 +122,7 @@ def _filter_experiences_by_region_or_country( return db.query(PrivacyExperience).filter(False) +# TODO: readd the fides limiter @router.get( urls.PRIVACY_EXPERIENCE, status_code=HTTP_200_OK, @@ -164,7 +163,6 @@ async def privacy_experience_list( :param response: :return: """ - global PRIVACY_EXPERIENCE_CACHE # These are the parameters that get used to create the cache. param_hash_list = [ @@ -184,13 +182,13 @@ async def privacy_experience_list( if request.headers.get(BUST_CACHE_HEADER): PRIVACY_EXPERIENCE_CACHE.clear() - if cache_hash in PRIVACY_EXPERIENCE_CACHE.keys(): + if PRIVACY_EXPERIENCE_CACHE.get(cache_hash): logger.debug("Cache HIT: {}", cache_hash) response.headers[CACHE_HEADER] = "HIT" return PRIVACY_EXPERIENCE_CACHE[cache_hash] - else: - logger.debug("Cache MISS: {}", cache_hash) - response.headers[CACHE_HEADER] = "MISS" + + logger.debug("Cache MISS: {}", cache_hash) + response.headers[CACHE_HEADER] = "MISS" fides_user_provided_identity: Optional[ProvidedIdentity] = None if fides_user_device_id: diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 221463824c..94cde18c54 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -17,7 +17,6 @@ from starlette.background import BackgroundTask from uvicorn import Config, Server - import fides from fides.api.app_setup import ( check_redis, diff --git a/src/fides/api/util/cache.py b/src/fides/api/util/cache.py index a68c3f7fee..b6af4870aa 100644 --- a/src/fides/api/util/cache.py +++ b/src/fides/api/util/cache.py @@ -62,6 +62,7 @@ def _custom_decoder(json_dict: Dict[str, Any]) -> Dict[str, Any]: return json_dict +# pylint: disable=abstract-method class FidesopsRedis(Redis): """ An extension to Redis' python bindings to support auto expiring data input. This class diff --git a/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py b/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py index ab2638bca4..f5e30878c3 100644 --- a/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_privacy_experience_endpoints.py @@ -1,4 +1,5 @@ from __future__ import annotations + from typing import Dict import pytest @@ -6,9 +7,9 @@ from starlette.testclient import TestClient from fides.api.api.v1.endpoints.privacy_experience_endpoints import ( - _filter_experiences_by_region_or_country, BUST_CACHE_HEADER, CACHE_HEADER, + _filter_experiences_by_region_or_country, ) from fides.api.models.privacy_experience import ComponentType, PrivacyExperience from fides.api.models.privacy_notice import ConsentMechanism @@ -19,6 +20,11 @@ def get_cache_bust_headers() -> Dict: return {BUST_CACHE_HEADER: "true"} +@pytest.fixture(scope="function") +def url() -> str: + return V1_URL_PREFIX + PRIVACY_EXPERIENCE + + class TestGetPrivacyExperiencesCaching: def test_cache_header_hit(self, url, api_client): """Check that the header describing cache hits/misses is working.""" @@ -37,10 +43,6 @@ def test_bust_cache_header(self, url, api_client): class TestGetPrivacyExperiences: - @pytest.fixture(scope="function") - def url(self) -> str: - return V1_URL_PREFIX + PRIVACY_EXPERIENCE - def test_get_privacy_experiences_unauthenticated(self, url, api_client): """This is a public endpoint""" resp = api_client.get(url)