From dceca1d3f0931d43efe9a3b90ca54c6d9f346b3e Mon Sep 17 00:00:00 2001
From: Jesse Yang <jesse.yang@airbnb.com>
Date: Tue, 6 Oct 2020 09:43:43 -0700
Subject: [PATCH] fix: enable consistent etag across workers and force no-cache
 for dashboards (#11137)

* fix: enable consistent etag across workers

* Use CACHE_DEFAULT_TIMEOUT instead of 0

* Change timeout to 0 and set expires header even for no-cache

* Reduce number of if branches to appease Pylint

* Fix mypy error
---
 superset/utils/decorators.py | 34 +++++++++++++++++++++-------------
 superset/views/core.py       |  8 ++++++++
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/superset/utils/decorators.py b/superset/utils/decorators.py
index ae4c5726d8ee7..d85bf41812818 100644
--- a/superset/utils/decorators.py
+++ b/superset/utils/decorators.py
@@ -51,6 +51,7 @@ def etag_cache(
     check_perms: Callable[..., Any],
     get_last_modified: Optional[Callable[..., Any]] = None,
     skip: Optional[Callable[..., Any]] = None,
+    must_revalidate: Optional[bool] = False,
 ) -> Callable[..., Any]:
     """
     A decorator for caching views and handling etag conditional requests.
@@ -78,6 +79,8 @@ def wrapper(*args: Any, **kwargs: Any) -> ETagResponseMixin:
                 return f(*args, **kwargs)
 
             response = None
+            last_modified = get_last_modified and get_last_modified(*args, **kwargs)
+
             if cache:
                 try:
                     # build the cache key from the function arguments and any
@@ -94,32 +97,37 @@ def wrapper(*args: Any, **kwargs: Any) -> ETagResponseMixin:
                         raise
                     logger.exception("Exception possibly due to cache backend.")
 
-            # if cache is stale?
-            if get_last_modified:
-                content_changed_time = get_last_modified(*args, **kwargs)
+                # if cache is stale?
                 if (
                     response
+                    and last_modified
                     and response.last_modified
-                    and response.last_modified.timestamp()
-                    < content_changed_time.timestamp()
+                    and response.last_modified < last_modified
                 ):
                     response = None
-            else:
-                # if caller didn't provide content's last_modified time, assume
-                # its cache won't be stale.
-                content_changed_time = datetime.utcnow()
 
-            # if no response was cached, compute it using the wrapped function
             if response is None:
+                # if no response was cached, compute it using the wrapped function
                 response = f(*args, **kwargs)
 
-                # add headers for caching: Last Modified, Expires and ETag
-                response.cache_control.public = True
-                response.last_modified = content_changed_time
+                # set expiration headers:
+                #   Last-Modified, Expires, Cache-Control, ETag
+                response.last_modified = last_modified or datetime.utcnow()
                 expiration = max_age if max_age != 0 else FAR_FUTURE
                 response.expires = response.last_modified + timedelta(
                     seconds=expiration
                 )
+
+                # when needed, instruct the browser to always revalidate cache
+                if must_revalidate:
+                    # `Cache-Control: no-cache` asks the browser to always store
+                    # the cache, but also must validate it with the server.
+                    response.cache_control.no_cache = True
+                else:
+                    # `Cache-Control: Public` asks the browser to always store
+                    # the cache.
+                    response.cache_control.public = True
+
                 response.add_etag()
 
                 # if we have a cache, store the response from the request
diff --git a/superset/views/core.py b/superset/views/core.py
index 36668cf082539..9d9caafc4107c 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -171,6 +171,13 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
 
     logger = logging.getLogger(__name__)
 
+    def __repr__(self) -> str:
+        """Determinate string representation of the view instance for etag_cache."""
+        return "Superset.views.core.Superset@v{}{}".format(
+            self.appbuilder.app.config["VERSION_STRING"],
+            self.appbuilder.app.config["VERSION_SHA"],
+        )
+
     @has_access_api
     @expose("/datasources/")
     def datasources(self) -> FlaskResponse:
@@ -1605,6 +1612,7 @@ def publish(  # pylint: disable=no-self-use
         skip=lambda _self, dashboard_id_or_slug: not is_feature_enabled(
             "ENABLE_DASHBOARD_ETAG_HEADER"
         ),
+        must_revalidate=True,
     )
     @expose("/dashboard/<dashboard_id_or_slug>/")
     def dashboard(  # pylint: disable=too-many-locals