diff --git a/frontend/test/metabase/scenarios/admin/settings/cache.cy.spec.js b/frontend/test/metabase/scenarios/admin/settings/cache.cy.spec.js index 05a8cc91176d5efac3e72d699c625e5e0d8583a7..50ccbabc2cd28d2433011e1283113a870284f244 100644 --- a/frontend/test/metabase/scenarios/admin/settings/cache.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/settings/cache.cy.spec.js @@ -15,7 +15,7 @@ describe("scenarios > admin > settings > cache", () => { cy.signInAsAdmin(); }); - describe.skip("issue 18458", () => { + describe("issue 18458", () => { beforeEach(() => { cy.visit("/admin/settings/caching"); diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj index c572b9513da68264341856466bc8f661a977e548..2d7fdc5002143751a96436d72878178702f62a9f 100644 --- a/src/metabase/api/card.clj +++ b/src/metabase/api/card.clj @@ -668,11 +668,15 @@ ttl-seconds)))) (defn- ttl-hierarchy + "Returns the cache ttl (in seconds), by first checking whether there is a stored value for the database, + dashboard, or card (in that order of increasing preference), and if all of those don't exist, then the + `query-magic-ttl`, which is based on average execution time." [card dashboard database query] (when (public-settings/enable-query-caching) (let [ttls (map :cache_ttl [card dashboard database]) most-granular-ttl (first (filter some? ttls))] - (or most-granular-ttl + (or (when most-granular-ttl ; stored TTLs are in hours; convert to seconds + (* most-granular-ttl 3600)) (query-magic-ttl query))))) (defn query-for-card @@ -687,9 +691,7 @@ :middleware middleware)) dashboard (db/select-one [Dashboard :cache_ttl] :id (:dashboard-id ids)) database (db/select-one [Database :cache_ttl] :id (:database_id card)) - ttl (ttl-hierarchy card dashboard database query) - ;; Stored TTL's are in hours: query wants seconds - ttl-secs (if (nil? ttl) nil (* 3600 ttl))] + ttl-secs (ttl-hierarchy card dashboard database query)] (assoc query :cache-ttl ttl-secs))) (defn run-query-for-card-async diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj index 3a7b52a5c6fa09f6fa40c617fe4fbca5b128c1a2..a18736f9df139b902b3ee502b5c53d2d5a11452d 100644 --- a/test/metabase/api/card_test.clj +++ b/test/metabase/api/card_test.clj @@ -16,6 +16,7 @@ [metabase.models.moderation-review :as moderation-review] [metabase.models.permissions :as perms] [metabase.models.permissions-group :as perms-group] + [metabase.models.query :as query] [metabase.models.revision :as revision :refer [Revision]] [metabase.models.user :refer [User]] [metabase.public-settings :as public-settings] @@ -1367,6 +1368,13 @@ (deftest query-cache-ttl-hierarchy-test (mt/discard-setting-changes [enable-query-caching] (public-settings/enable-query-caching true) + (testing "query-magic-ttl converts to seconds correctly" + (mt/with-temporary-setting-values [query-caching-ttl-ratio 2] + ;; fake average execution time (in millis) + (with-redefs [query/average-execution-time-ms (constantly 4000)] + (mt/with-temp Card [card] + ;; the magic multiplier should be ttl-ratio times avg execution time + (is (= (* 2 4) (:cache-ttl (card-api/query-for-card card {} {} {})))))))) (testing "card ttl only" (mt/with-temp* [Card [card {:cache_ttl 1337}]] (is (= (* 3600 1337) (:cache-ttl (card-api/query-for-card card {} {} {}))))))