This is an automated email from the ASF dual-hosted git repository. erikrit pushed a commit to branch erik-ritter--dashboard-etag-caching in repository https://gitbox.apache.org/repos/asf/superset.git
commit fd6bf7811ac3b1a41e9bc2a706c9c02bd0e0f280 Author: erik_ritter <[email protected]> AuthorDate: Mon Apr 26 10:55:49 2021 -0700 feat: Add etag caching to dashboard APIs --- superset/dashboards/api.py | 95 ++++++++++++++++++++++++++++++---------------- superset/dashboards/dao.py | 16 ++++++++ superset/utils/cache.py | 33 ++++++++++++++-- 3 files changed, 109 insertions(+), 35 deletions(-) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 64bfd82..853154d 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -72,6 +72,7 @@ from superset.dashboards.schemas import ( from superset.extensions import event_logger from superset.models.dashboard import Dashboard from superset.tasks.thumbnails import cache_dashboard_thumbnail +from superset.utils.cache import etag_cache from superset.utils.screenshots import DashboardScreenshot from superset.utils.urls import get_url_path from superset.views.base import generate_download_headers @@ -210,6 +211,16 @@ class DashboardRestApi(BaseSupersetModelRestApi): self.include_route_methods = self.include_route_methods | {"thumbnail"} super().__init__() + @etag_cache( + get_last_modified=lambda _self, id_or_slug: DashboardDAO.get_dashboard_changed_on( + id_or_slug + ), + max_age=0, + raise_for_access=lambda _self, id_or_slug: DashboardDAO.get_by_id_or_slug( + id_or_slug + ), + skip=lambda _self, id_or_slug: not is_feature_enabled("DASHBOARD_CACHE"), + ) @expose("/<id_or_slug>", methods=["GET"]) @protect() @safe @@ -257,6 +268,16 @@ class DashboardRestApi(BaseSupersetModelRestApi): except DashboardNotFoundError: return self.response_404() + @etag_cache( + get_last_modified=lambda _self, id_or_slug: DashboardDAO.get_dashboard_changed_on( + id_or_slug + ), + max_age=0, + raise_for_access=lambda _self, id_or_slug: DashboardDAO.get_by_id_or_slug( + id_or_slug + ), + skip=lambda _self, id_or_slug: not is_feature_enabled("DASHBOARD_CACHE"), + ) @expose("/<id_or_slug>/datasets", methods=["GET"]) @protect() @safe @@ -267,38 +288,38 @@ class DashboardRestApi(BaseSupersetModelRestApi): ) def get_datasets(self, id_or_slug: str) -> Response: """Gets a dashboard's datasets - --- - get: - description: >- - Returns a list of a dashboard's datasets. Each dataset includes only - the information necessary to render the dashboard's charts. - parameters: - - in: path - schema: - type: string - name: id_or_slug - description: Either the id of the dashboard, or its slug - responses: - 200: - description: Dashboard dataset definitions - content: - application/json: - schema: - type: object - properties: - result: - type: array - items: - $ref: '#/components/schemas/DashboardDatasetSchema' - 302: - description: Redirects to the current digest - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 404: - $ref: '#/components/responses/404' - """ + --- + get: + description: >- + Returns a list of a dashboard's datasets. Each dataset includes only + the information necessary to render the dashboard's charts. + parameters: + - in: path + schema: + type: string + name: id_or_slug + description: Either the id of the dashboard, or its slug + responses: + 200: + description: Dashboard dataset definitions + content: + application/json: + schema: + type: object + properties: + result: + type: array + items: + $ref: '#/components/schemas/DashboardDatasetSchema' + 302: + description: Redirects to the current digest + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + """ try: datasets = DashboardDAO.get_datasets_for_dashboard(id_or_slug) result = [ @@ -308,6 +329,16 @@ class DashboardRestApi(BaseSupersetModelRestApi): except DashboardNotFoundError: return self.response_404() + @etag_cache( + get_last_modified=lambda _self, id_or_slug: DashboardDAO.get_dashboard_changed_on( + id_or_slug + ), + max_age=0, + raise_for_access=lambda _self, id_or_slug: DashboardDAO.get_by_id_or_slug( + id_or_slug + ), + skip=lambda _self, id_or_slug: not is_feature_enabled("DASHBOARD_CACHE"), + ) @expose("/<id_or_slug>/charts", methods=["GET"]) @protect() @safe diff --git a/superset/dashboards/dao.py b/superset/dashboards/dao.py index 4a8a314..5118ddd 100644 --- a/superset/dashboards/dao.py +++ b/superset/dashboards/dao.py @@ -16,6 +16,7 @@ # under the License. import json import logging +from datetime import datetime from typing import Any, Dict, List, Optional from sqlalchemy.exc import SQLAlchemyError @@ -55,6 +56,21 @@ class DashboardDAO(BaseDAO): return DashboardDAO.get_by_id_or_slug(id_or_slug).slices @staticmethod + def get_dashboard_changed_on(id_or_slug: str) -> datetime: + """ + Get latest changed datetime for a dashboard. The change could be a dashboard + metadata change, or a change to one of its dependent slices. + + :param id_or_slug: The ID or slug of the dashboard. + :returns: The datetime the dashboard was last changed. + """ + dashboard = DashboardDAO.get_by_id_or_slug(id_or_slug) + dashboard_changed_on = dashboard.changed_on + slices_changed_on = max([slc.changed_on for slc in dashboard.slices]) + # drop microseconds in datetime to match with last_modified header + return max(dashboard_changed_on, slices_changed_on).replace(microsecond=0) + + @staticmethod def validate_slug_uniqueness(slug: str) -> bool: if not slug: return True diff --git a/superset/utils/cache.py b/superset/utils/cache.py index 0abd76a..576900b 100644 --- a/superset/utils/cache.py +++ b/superset/utils/cache.py @@ -126,7 +126,11 @@ def memoized_func( def etag_cache( - cache: Cache = cache_manager.cache, max_age: Optional[Union[int, float]] = None, + cache: Cache = cache_manager.cache, + get_last_modified: Optional[Callable[..., datetime]] = None, + max_age: Optional[Union[int, float]] = None, + raise_for_access: Optional[Callable[..., Any]] = None, + skip: Optional[Callable[..., bool]] = None, ) -> Callable[..., Any]: """ A decorator for caching views and handling etag conditional requests. @@ -146,10 +150,19 @@ def etag_cache( def decorator(f: Callable[..., Any]) -> Callable[..., Any]: @wraps(f) def wrapper(*args: Any, **kwargs: Any) -> ETagResponseMixin: + # Check if the user can access the resource + if raise_for_access: + try: + raise_for_access(*args, **kwargs) + except Exception: # pylint: disable=broad-except + # If there's no access, bypass the cache and let the function + # handle the response. + return f(*args, **kwargs) + # for POST requests we can't set cache headers, use the response # cache nor use conditional requests; this will still use the # dataframe cache in `superset/viz.py`, though. - if request.method == "POST": + if request.method == "POST" or (skip and skip(*args, **kwargs)): return f(*args, **kwargs) response = None @@ -168,13 +181,27 @@ def etag_cache( raise logger.exception("Exception possibly due to cache backend.") + # Check if the cache is stale. Default the content_changed_time to now + # if we don't know when it was last modified. + content_changed_time = datetime.utcnow() + if get_last_modified: + content_changed_time = get_last_modified(*args, **kwargs) + if ( + response + and response.last_modified + and response.last_modified.timestamp() + < content_changed_time.timestamp() + ): + # Bypass the cache if the response is stale + response = None + # if no response was cached, compute it using the wrapped function if response is None: response = f(*args, **kwargs) # add headers for caching: Last Modified, Expires and ETag response.cache_control.public = True - response.last_modified = datetime.utcnow() + response.last_modified = content_changed_time expiration = max_age or ONE_YEAR # max_age=0 also means far future response.expires = response.last_modified + timedelta( seconds=expiration
