diff --git a/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj b/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj index b73f4ced27b4f0dfb892b418f4eeaa811400e6d4..c4b5e8652d8d305925ed873f4f1179757cfe5ece 100644 --- a/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj +++ b/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj @@ -154,37 +154,37 @@ :model "database"))) (testing "making a query will cache it" - (is (=? {:cached false :data some?} + (is (=? {:cached nil :data some?} (run-query! card1-id))) - (is (=? {:cached true :data some?} + (is (=? {:cached some? :data some?} (run-query! card1-id))) - (is (=? {:cached true :data some?} + (is (=? {:cached some? :data some?} (run-query! card2-id)))) (testing "invalidation drops cache only for affected card" (is (=? {:count 1} (invalidate! 200 :question card2-id :include :overrides))) - (is (=? {:cached true :data some?} + (is (=? {:cached some? :data some?} (run-query! card1-id))) - (is (=? {:cached false :data some?} + (is (=? {:cached nil :data some?} (run-query! card2-id)))) (testing "but invalidating a whole config drops cache for any affected card" (doseq [card-id [card1-id card2-id]] (is (=? {:count 1} (invalidate! 200 :database (mt/id)))) - (is (=? {:cached false :data some?} + (is (=? {:cached nil :data some?} (run-query! card-id {:ignore_cache true}))))) (testing "when invalidating database config directly, dashboard-related queries are still cached" (is (=? {:count 1} (invalidate! 200 :database (mt/id)))) - (is (=? {:cached true :data some?} + (is (=? {:cached some? :data some?} (run-query! card1-id {:dashboard_id (:id dash)})))) (testing "but with overrides - will go through every card and mark cache invalidated" ;; not a concrete number here since (mt/id) can have a bit more than 2 cards we've currently defined (is (=? {:count pos-int?} (invalidate! 200 :include :overrides :database (mt/id)))) - (is (=? {:cached false :data some?} + (is (=? {:cached nil :data some?} (run-query! card1-id {:dashboard_id (:id dash)})))))))))) diff --git a/frontend/src/metabase/query_builder/components/view/QuestionLastUpdated.jsx b/frontend/src/metabase/query_builder/components/view/QuestionLastUpdated.jsx index c9a75f5c522f43e3c11b389d219392f206b1206a..408ac4afcc624fc94b8e42426244e4e131417a6f 100644 --- a/frontend/src/metabase/query_builder/components/view/QuestionLastUpdated.jsx +++ b/frontend/src/metabase/query_builder/components/view/QuestionLastUpdated.jsx @@ -11,7 +11,7 @@ export default function QuestionLastUpdated({ result, ...props }) { return result ? ( <SectionRoot {...props}> <Icon name="clock" className={CS.mr1} /> - {t`Updated ${moment(result.updated_at).fromNow()}`} + {t`Updated ${moment(result.cached).fromNow()}`} </SectionRoot> ) : null; } diff --git a/src/metabase/query_processor/middleware/cache.clj b/src/metabase/query_processor/middleware/cache.clj index 2987cb10f0c7f1ee6112d3436de4f83cbc4e43af..88ed12962696bde2a01649cc52b008a04526806d 100644 --- a/src/metabase/query_processor/middleware/cache.clj +++ b/src/metabase/query_processor/middleware/cache.clj @@ -220,7 +220,8 @@ (save-results-xform start-time-ns metadata query-hash cache-strategy (rff metadata))))))))) (defn- is-cacheable? {:arglists '([query])} [{:keys [cache-strategy]}] - (some? cache-strategy)) + (and (some? cache-strategy) + (not= (:type cache-strategy) :nocache))) (defn maybe-return-cached-results "Middleware for caching results of a query if applicable. diff --git a/src/metabase/query_processor/middleware/process_userland_query.clj b/src/metabase/query_processor/middleware/process_userland_query.clj index ea1b391d8fa1e19375a580f28ac306c03558ef0a..76b211eec4527f4db7845668b3ef623205f2578b 100644 --- a/src/metabase/query_processor/middleware/process_userland_query.clj +++ b/src/metabase/query_processor/middleware/process_userland_query.clj @@ -94,7 +94,7 @@ add-running-time (dissoc :error :hash :executor_id :action_id :is_sandboxed :card_id :dashboard_id :pulse_id :result_rows :native)) (dissoc result :cache/details) - {:cached (boolean (:cached cache)) + {:cached (when (:cached cache) (:updated_at cache)) :status :completed :average_execution_time (when (:cached cache) (query/average-execution-time-ms query-hash))})) diff --git a/test/metabase/query_processor/middleware/cache_test.clj b/test/metabase/query_processor/middleware/cache_test.clj index 30771c9425b3ce743358cd4319ab39e18c0dd597..5493e9d0fba5e00470f7def789a460713ffbb000 100644 --- a/test/metabase/query_processor/middleware/cache_test.clj +++ b/test/metabase/query_processor/middleware/cache_test.clj @@ -28,7 +28,8 @@ [metabase.util :as u] [metabase.util.log :as log] [pretty.core :as pretty] - [toucan2.core :as t2])) + [toucan2.core :as t2]) + (:import [java.time ZonedDateTime])) (set! *warn-on-reflection* true) @@ -159,10 +160,11 @@ (boolean (#'cache/is-cacheable? (merge {:cache-strategy (ttl-strategy), :query :abc} query-kvs)))) (deftest is-cacheable-test - (testing "something is-cacheable? if it includes `:cache-strategy`" + (testing "something is-cacheable? if it includes `:cache-strategy` and it is not `:nocache`" (with-mock-cache [] - (doseq [cache-strategy [(ttl-strategy) nil] - :let [expected (boolean cache-strategy)]] + (doseq [[cache-strategy expected] {(ttl-strategy) true + {:type :nocache} false + nil false}] (testing (format "cache strategy = %s" (pr-str cache-strategy)) (is (= expected (boolean (#'cache/is-cacheable? {:cache-strategy cache-strategy}))))))))) @@ -349,9 +351,10 @@ (is (= 1 @save-execution-metadata-count)) (is (= 1 @update-avg-execution-count)) (let [avg-execution-time (query/average-execution-time-ms q-hash)] - (is (number? avg-execution-time)) + (is (pos? avg-execution-time)) ;; rerun query getting cached results - (is (:cached (qp/process-query (qp/userland-query query)))) + (is (instance? ZonedDateTime + (:cached (qp/process-query (qp/userland-query query))))) (mt/wait-for-result save-chan) (is (= 2 @save-execution-metadata-count) "Saving execution times of a cache lookup") diff --git a/test/metabase/query_processor/middleware/process_userland_query_test.clj b/test/metabase/query_processor/middleware/process_userland_query_test.clj index edb50c7f39346964a5ad6a3f8b5ee930c483212e..b1237f8c52331b6d8cf8fde1d761b4889ef3cf13 100644 --- a/test/metabase/query_processor/middleware/process_userland_query_test.clj +++ b/test/metabase/query_processor/middleware/process_userland_query_test.clj @@ -83,7 +83,7 @@ :average_execution_time nil :context nil :running_time int? - :cached false} + :cached nil} (process-userland-query query)) "Result should have query execution info") (is (=? {:hash "58af781ea2ba252ce3131462bdc7c54bc57538ed965d55beec62928ce8b32635"