diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj
index 0504ee587e03bd18bb53f53383cf9577d7c325b4..b1357fa8faa6944f1dfc7f11045612db50df1bf1 100644
--- a/src/metabase/api/dataset.clj
+++ b/src/metabase/api/dataset.clj
@@ -3,9 +3,12 @@
(:require [clojure.data.csv :as csv]
[compojure.core :refer [GET POST]]
[metabase.api.common :refer :all]
+ [metabase.db :as db]
[metabase.driver :as driver]
[metabase.driver.query-processor :refer [structured-query?]]
+ [metabase.models.card :refer [Card]]
[metabase.models.database :refer [Database]]
+ [metabase.models.hydrate :refer [hydrate]]
[metabase.util :as u]))
(def ^:const api-max-results-bare-rows
@@ -16,14 +19,17 @@
"General maximum number of rows to return from an API query."
10000)
+(def ^:const dataset-query-api-constraints
+ "Default map of constraints that we apply on dataset queries executed by the api."
+ {:max-results api-max-results
+ :max-results-bare-rows api-max-results-bare-rows})
(defendpoint POST "/"
"Execute an MQL query and retrieve the results as JSON."
[:as {{:keys [database] :as body} :body}]
(read-check Database database)
;; add sensible constraints for results limits on our query
- (let [query (assoc body :constraints {:max-results api-max-results
- :max-results-bare-rows api-max-results-bare-rows})]
+ (let [query (assoc body :constraints dataset-query-api-constraints)]
(driver/dataset-query query {:executed_by *current-user-id*})))
@@ -45,4 +51,19 @@
{:status 500
:body (:error response)})))
+(defendpoint GET "/card/:id"
+ "Execute the MQL query for a given `Card` and retrieve both the `Card` and the execution results as JSON.
+ This is a convenience endpoint which simplifies the normal 2 api calls to fetch the `Card` then execute its query."
+ [id]
+ (let-404 [{:keys [dataset_query] :as card} (db/sel :one Card :id id)]
+ (read-check card)
+ (read-check Database (:database dataset_query))
+ ;; add sensible constraints for results limits on our query
+ ;; TODO: it would be nice to associate the card :id with the query execution tracking
+ (let [query (assoc dataset_query :constraints dataset-query-api-constraints)
+ options {:executed_by *current-user-id*}]
+ {:card (hydrate card :creator)
+ :result (driver/dataset-query query options)})))
+
+
(define-routes)
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index d81dfe2234547a246c9859eb35e4aed24f08ebea..ac625ad8bd1db56fede7fad29c79ea101d3356ff 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -441,6 +441,7 @@
(dissoc :start_time_millis)
(merge updates)
(save-query-execution)
+ (dissoc :raw_query :result_rows :version)
;; this is just for the response for clien
(assoc :error error-message
:row_count 0
@@ -460,7 +461,7 @@
(dissoc :start_time_millis)
(save-query-execution)
;; at this point we've saved and we just need to massage things into our final response format
- (select-keys [:id :uuid])
+ (dissoc :error :raw_query :result_rows :version)
(merge query-result)))
(defn save-query-execution
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index b91f6635fcd489fe12d2d270c302c7f73ff68b53..06d810ca8fe2fc51a0008c75d5310e5019a6bfb6 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -1,48 +1,186 @@
(ns metabase.api.dataset-test
"Unit tests for /api/dataset endpoints."
(:require [expectations :refer :all]
- [korma.core :as k]
+ [metabase.api.dataset :refer [dataset-query-api-constraints]]
[metabase.db :refer :all]
[metabase.driver.query-processor.expand :as ql]
+ [metabase.models.card :refer [Card]]
[metabase.models.query-execution :refer [QueryExecution]]
- [metabase.test.data :refer :all]
[metabase.test.data.users :refer :all]
- [metabase.test.util :refer [match-$ expect-eval-actual-first]]))
+ [metabase.test.data :refer :all]
+ [metabase.test.util :as tu]))
+
+
+(defn user-details [user]
+ (tu/match-$ user
+ {:email $
+ :date_joined $
+ :first_name $
+ :last_name $
+ :last_login $
+ :is_superuser $
+ :common_name $}))
+
+(defn remove-ids-and-boolean-timestamps [m]
+ (let [f (fn [v]
+ (cond
+ (map? v) (remove-ids-and-boolean-timestamps v)
+ (coll? v) (mapv remove-ids-and-boolean-timestamps v)
+ :else v))]
+ (into {} (for [[k v] m]
+ (when-not (or (= :id k)
+ (.endsWith (name k) "_id"))
+ (if (or (= :created_at k)
+ (= :updated_at k))
+ [k (not (nil? v))]
+ [k (f v)]))))))
+
+(defn format-response [m]
+ (into {} (for [[k v] m]
+ (if (contains? #{:id :uuid :started_at :finished_at :running_time} k)
+ [k (boolean v)]
+ [k v]))))
;;; ## POST /api/meta/dataset
;; Just a basic sanity check to make sure Query Processor endpoint is still working correctly.
-(expect-eval-actual-first
- (match-$ (sel :one :fields [QueryExecution :id :uuid] (k/order :id :desc))
- {:data {:rows [[1000]]
+(expect
+ ;; the first result is directly from the api call
+ ;; the second result is checking our QueryExection log to ensure it captured the right details
+ [{:data {:rows [[1000]]
:columns ["count"]
:cols [{:base_type "IntegerField", :special_type "number", :name "count", :display_name "count", :id nil, :table_id nil,
:description nil, :target nil, :extra_info {}}]}
- :row_count 1
- :status "completed"
- :id $
- :uuid $})
- ((user->client :rasta) :post 200 "dataset" (ql/wrap-inner-query
- (query checkins
- (ql/aggregation (ql/count))))))
+ :row_count 1
+ :status "completed"
+ :id true
+ :uuid true
+ :json_query (-> (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ (assoc :type "query")
+ (assoc-in [:query :aggregation] {:aggregation-type "count"})
+ (assoc :constraints dataset-query-api-constraints))
+ :started_at true
+ :finished_at true
+ :running_time true}
+ {:row_count 1
+ :result_rows 1
+ :status :completed
+ :error ""
+ :id true
+ :uuid true
+ :raw_query ""
+ :json_query (-> (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ (assoc :type "query")
+ (assoc-in [:query :aggregation] {:aggregation-type "count"})
+ (assoc :constraints dataset-query-api-constraints))
+ :started_at true
+ :finished_at true
+ :running_time true
+ :version 0}]
+ (let [result ((user->client :rasta) :post 200 "dataset" (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count)))))]
+ [(format-response result)
+ (format-response (sel :one QueryExecution :uuid (:uuid result)))]))
;; Even if a query fails we still expect a 200 response from the api
-(expect-eval-actual-first
- (match-$ (sel :one QueryExecution (k/order :id :desc))
- {:data {:rows []
- :cols []
- :columns []}
- :error "Syntax error in SQL statement \"FOOBAR[*] \"; expected \"FROM, {\""
- :raw_query ""
- :row_count 0
- :result_rows 0
- :status "failed"
- :version 0
- :json_query $
- :started_at $
- :finished_at $
- :running_time $
- :id $
- :uuid $})
- ((user->client :rasta) :post 200 "dataset" {:database (id)
- :type "native"
- :native {:query "foobar"}}))
+(expect
+ ;; the first result is directly from the api call
+ ;; the second result is checking our QueryExection log to ensure it captured the right details
+ (let [output {:data {:rows []
+ :columns []
+ :cols []}
+ :row_count 0
+ :status "failed"
+ :error "Syntax error in SQL statement \"FOOBAR[*] \"; expected \"FROM, {\""
+ :id true
+ :uuid true
+ :json_query {:database (id)
+ :type "native"
+ :native {:query "foobar"}
+ :constraints dataset-query-api-constraints}
+ :started_at true
+ :finished_at true
+ :running_time true}]
+ [output
+ (-> output
+ (dissoc :data)
+ (assoc :status :failed
+ :version 0
+ :raw_query ""
+ :result_rows 0))])
+ (let [result ((user->client :rasta) :post 200 "dataset" {:database (id)
+ :type "native"
+ :native {:query "foobar"}})]
+ [(format-response result)
+ (format-response (sel :one QueryExecution :uuid (:uuid result)))]))
+
+
+;; GET /api/dataset/card/:id
+(expect
+ ;; the first result is directly from the api call
+ ;; the second result is checking our QueryExection log to ensure it captured the right details
+ [{:card {:name "Dataset Test Card"
+ :description nil
+ :public_perms 0
+ :creator (user-details (fetch-user :rasta))
+ :display "scalar"
+ :query_type "query"
+ :dataset_query (-> (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ (assoc :type "query")
+ (assoc-in [:query :aggregation] {:aggregation-type "count"}))
+ :visualization_settings {}
+ :created_at true
+ :updated_at true}
+ :result {:data {:rows [[1000]]
+ :columns ["count"]
+ :cols [{:base_type "IntegerField", :special_type "number", :name "count", :display_name "count", :id nil, :table_id nil,
+ :description nil, :target nil, :extra_info {}}]}
+ :row_count 1
+ :status "completed"
+ :id true
+ :uuid true
+ :json_query (-> (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ (assoc :type "query")
+ (assoc-in [:query :aggregation] {:aggregation-type "count"})
+ (assoc :constraints dataset-query-api-constraints))
+ :started_at true
+ :finished_at true
+ :running_time true}}
+ {:row_count 1
+ :result_rows 1
+ :status :completed
+ :error ""
+ :id true
+ :uuid true
+ :raw_query ""
+ :json_query (-> (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ (assoc :type "query")
+ (assoc-in [:query :aggregation] {:aggregation-type "count"})
+ (assoc :constraints dataset-query-api-constraints))
+ :started_at true
+ :finished_at true
+ :running_time true
+ :version 0}]
+ (tu/with-temp Card [{card-id :id} {:name "Dataset Test Card"
+ :creator_id (user->id :rasta)
+ :public_perms 0
+ :display "scalar"
+ :dataset_query (ql/wrap-inner-query
+ (query checkins
+ (ql/aggregation (ql/count))))
+ :visualization_settings {}}]
+ (let [result ((user->client :rasta) :get 200 (format "dataset/card/%d" card-id))]
+ [(-> result
+ (update :card remove-ids-and-boolean-timestamps)
+ (update :result format-response))
+ (format-response (sel :one QueryExecution :uuid (get-in result [:result :uuid])))])))