From 030d322d247d1a4136485bf043f1e3519fe39c78 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Mon, 27 Dec 2021 13:43:04 -0300 Subject: [PATCH 1/6] fix: Returns 404 instead of 500 for unknown dashboard filter state keys --- superset/dashboards/filter_state/commands/get.py | 7 ++++--- .../integration_tests/dashboards/filter_state/api_tests.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index 3087389cc5a2e..6f571cf91bf9a 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -30,7 +30,8 @@ def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str entry: Entry = cache_manager.filter_state_cache.get( cache_key(resource_id, key) ) - if refresh_timeout: - cache_manager.filter_state_cache.set(key, entry) - return entry["value"] + if entry: + if refresh_timeout: + cache_manager.filter_state_cache.set(key, entry) + return entry["value"] return None diff --git a/tests/integration_tests/dashboards/filter_state/api_tests.py b/tests/integration_tests/dashboards/filter_state/api_tests.py index bd0e2e7d97753..00e88624909f8 100644 --- a/tests/integration_tests/dashboards/filter_state/api_tests.py +++ b/tests/integration_tests/dashboards/filter_state/api_tests.py @@ -146,9 +146,9 @@ def test_put_not_owner(client, dashboard_id: int): assert resp.status_code == 403 -def test_get_key_not_found(client): +def test_get_key_not_found(client, dashboard_id: int): login(client, "admin") - resp = client.get("unknown-key") + resp = client.get(f"api/v1/dashboard/{dashboard_id}/filter_state/unknown-key/") assert resp.status_code == 404 From 3e3c7bba1ef46cc737fb26e9dbf6c5d69839e945 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Mon, 10 Jan 2022 09:03:49 -0300 Subject: [PATCH 2/6] Reduces hierarchies of if-expression --- superset/dashboards/filter_state/commands/get.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index 6f571cf91bf9a..d81e29d7c2ade 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -26,12 +26,9 @@ class GetFilterStateCommand(GetKeyValueCommand): def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str]: dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id)) - if dashboard: - entry: Entry = cache_manager.filter_state_cache.get( - cache_key(resource_id, key) - ) - if entry: - if refresh_timeout: - cache_manager.filter_state_cache.set(key, entry) - return entry["value"] - return None + entry: Entry = cache_manager.filter_state_cache.get( + cache_key(resource_id, key) + ) or {} + if dashboard and entry and refresh_timeout: + cache_manager.filter_state_cache.set(key, entry) + return entry.get("value") From 52b8b5b963c8a5a1f4e48332c98530b613fb58bd Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Mon, 10 Jan 2022 11:25:46 -0300 Subject: [PATCH 3/6] Removes unnecessary check Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> --- superset/dashboards/filter_state/commands/get.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index d81e29d7c2ade..cbddd43693e4c 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -29,6 +29,6 @@ def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str entry: Entry = cache_manager.filter_state_cache.get( cache_key(resource_id, key) ) or {} - if dashboard and entry and refresh_timeout: + if entry and refresh_timeout: cache_manager.filter_state_cache.set(key, entry) return entry.get("value") From 68e47aaf77162b4beeac0ff32b6be6b2898c3f55 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Mon, 10 Jan 2022 11:31:45 -0300 Subject: [PATCH 4/6] Removes unused variable --- superset/dashboards/filter_state/commands/get.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index cbddd43693e4c..8aa62295012a4 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -25,7 +25,7 @@ class GetFilterStateCommand(GetKeyValueCommand): def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str]: - dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id)) + DashboardDAO.get_by_id_or_slug(str(resource_id)) entry: Entry = cache_manager.filter_state_cache.get( cache_key(resource_id, key) ) or {} From 5c7bd90f91a96a7885d92ae25e3d4b9f329979c4 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Mon, 10 Jan 2022 11:45:45 -0300 Subject: [PATCH 5/6] Fixes type error --- superset/dashboards/filter_state/commands/get.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index 8aa62295012a4..a7eb765b76706 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -26,9 +26,7 @@ class GetFilterStateCommand(GetKeyValueCommand): def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str]: DashboardDAO.get_by_id_or_slug(str(resource_id)) - entry: Entry = cache_manager.filter_state_cache.get( - cache_key(resource_id, key) - ) or {} + entry = cache_manager.filter_state_cache.get(cache_key(resource_id, key)) or {} if entry and refresh_timeout: cache_manager.filter_state_cache.set(key, entry) return entry.get("value") From 13d61b101b809e580643495bd46a1ea7480d242a Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Mon, 10 Jan 2022 11:52:25 -0300 Subject: [PATCH 6/6] Removes unused import --- superset/dashboards/filter_state/commands/get.py | 1 - 1 file changed, 1 deletion(-) diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index a7eb765b76706..bb3e95aaa615c 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/superset/dashboards/filter_state/commands/get.py @@ -17,7 +17,6 @@ from typing import Optional from superset.dashboards.dao import DashboardDAO -from superset.dashboards.filter_state.commands.entry import Entry from superset.extensions import cache_manager from superset.key_value.commands.get import GetKeyValueCommand from superset.key_value.utils import cache_key