Skip to content
Snippets Groups Projects
Commit 86f4e632 authored by Cam Saül's avatar Cam Saül
Browse files

Code cleanup :shower:

parent fa4f26ac
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/card.clj
+ 7
− 4
View file @ 86f4e632
......@@ -324,7 +324,10 @@
;;; ------------------------------------------------------------ Running a Query ------------------------------------------------------------
(defn- run-query-for-card [card-id parameters constraints]
(defn run-query-for-card
"Run the query for Card with PARAMETERS and CONSTRAINTS, and return results in the usual format."
[card-id & {:keys [parameters constraints]
:or {constraints dataset-api/default-query-constraints}}]
{:pre [(u/maybe? sequential? parameters)]}
(let [card (read-check Card card-id)
query (assoc (:dataset_query card)
......@@ -337,19 +340,19 @@
(defendpoint POST "/:card-id/query"
"Run the query associated with a Card."
[card-id :as {{:keys [parameters]} :body}]
(run-query-for-card card-id parameters dataset-api/query-constraints))
(run-query-for-card card-id, :parameters parameters))
(defendpoint POST "/:card-id/query/csv"
"Run the query associated with a Card, and return its results as CSV. Note that this expects the parameters as serialized JSON in the 'parameters' parameter"
[card-id parameters]
{parameters (s/maybe su/JSONString)}
(dataset-api/as-csv (run-query-for-card card-id (json/parse-string parameters keyword) nil)))
(dataset-api/as-csv (run-query-for-card card-id, :parameters (json/parse-string parameters keyword), :constraints nil)))
(defendpoint POST "/:card-id/query/json"
"Run the query associated with a Card, and return its results as JSON. Note that this expects the parameters as serialized JSON in the 'parameters' parameter"
[card-id parameters]
{parameters (s/maybe su/JSONString)}
(dataset-api/as-json (run-query-for-card card-id (json/parse-string parameters keyword) nil)))
(dataset-api/as-json (run-query-for-card card-id, :parameters (json/parse-string parameters keyword), :constraints nil)))
(define-routes)
src/metabase/api/common.clj
+ 6
− 3
View file @ 86f4e632
......@@ -64,9 +64,11 @@
(recur (first rest-args) (second rest-args) (drop 2 rest-args))))))
(defn check-exists?
"Check that object with ID exists in the DB, or throw a 404."
[entity id]
(check-404 (db/exists? entity, :id id)))
"Check that object with ID (or other key/values) exists in the DB, or throw a 404."
([entity id]
(check-exists? entity :id id))
([entity k v & more]
(check-404 (apply db/exists? entity k v more))))
(defn check-superuser
"Check that `*current-user*` is a superuser or throw a 403. This doesn't require a DB call."
......@@ -210,6 +212,7 @@
;;; ------------------------------------------------------------ DEFENDPOINT AND RELATED FUNCTIONS ------------------------------------------------------------
;; TODO - several of the things `defendpoint` does could and should just be done by custom Ring middleware instead
(defmacro defendpoint
"Define an API function.
This automatically does several things:
......
src/metabase/api/dataset.clj
+ 3
− 3
View file @ 86f4e632
......@@ -21,7 +21,7 @@
"General maximum number of rows to return from an API query."
10000)
(def ^:const query-constraints
(def ^:const default-query-constraints
"Default map of constraints that we apply on dataset queries executed by the api."
{:max-results max-results
:max-results-bare-rows max-results-bare-rows})
......@@ -31,7 +31,7 @@
[:as {{:keys [database] :as body} :body}]
(read-check Database database)
;; add sensible constraints for results limits on our query
(let [query (assoc body :constraints query-constraints)]
(let [query (assoc body :constraints default-query-constraints)]
(qp/dataset-query query {:executed-by *current-user-id*})))
(defendpoint POST "/duration"
......@@ -39,7 +39,7 @@
[:as {{:keys [database] :as query} :body}]
(read-check Database database)
;; add sensible constraints for results limits on our query
(let [query (assoc query :constraints query-constraints)
(let [query (assoc query :constraints default-query-constraints)
running-times (db/select-field :running_time QueryExecution
:query_hash (hash query)
{:order-by [[:started_at :desc]]
......
src/metabase/public_settings.clj
+ 13
− 13
View file @ 86f4e632
......@@ -68,20 +68,20 @@
(defn public-settings
"Return a simple map of key/value pairs which represent the public settings (`MetabaseBootstrap`) for the front-end application."
[]
{:ga_code "UA-60817802-1"
:password_complexity password/active-password-complexity
:timezones common/timezones
:version config/mb-version-info
:engines ((resolve 'metabase.driver/available-drivers))
:setup_token ((resolve 'metabase.setup/token-value))
{:admin_email (admin-email)
:anon_tracking_enabled (anon-tracking-enabled)
:site_name (site-name)
:custom_geojson (setting/get :custom-geojson)
:email_configured ((resolve 'metabase.email/email-configured?))
:admin_email (admin-email)
:report_timezone (setting/get :report-timezone)
:timezone_short (short-timezone-name (setting/get :report-timezone))
:has_sample_dataset (db/exists? 'Database, :is_sample true)
:engines ((resolve 'metabase.driver/available-drivers))
:ga_code "UA-60817802-1"
:google_auth_client_id (setting/get :google-auth-client-id)
:has_sample_dataset (db/exists? 'Database, :is_sample true)
:map_tile_server_url (map-tile-server-url)
:custom_geojson (setting/get :custom-geojson)
:types (types/types->parents)})
:password_complexity password/active-password-complexity
:report_timezone (setting/get :report-timezone)
:setup_token ((resolve 'metabase.setup/token-value))
:site_name (site-name)
:timezone_short (short-timezone-name (setting/get :report-timezone))
:timezones common/timezones
:types (types/types->parents)
:version config/mb-version-info})
src/metabase/query_processor.clj
+ 86
− 68
View file @ 86f4e632
......@@ -508,65 +508,17 @@
;;; | DATASET-QUERY PUBLIC API |
;;; +----------------------------------------------------------------------------------------------------+
(declare query-fail query-complete save-query-execution!)
(defn- assert-valid-query-result [query-result]
(when-not (contains? query-result :status)
(throw (Exception. "invalid response from database driver. no :status provided")))
(when (= :failed (:status query-result))
(log/error (u/pprint-to-str 'red query-result))
(throw (Exception. (str (get query-result :error "general error"))))))
(defn dataset-query
"Process and run a json based dataset query and return results.
Takes 2 arguments:
1. the json query as a dictionary
2. query execution options specified in a dictionary
Depending on the database specified in the query this function will delegate to a driver specific implementation.
For the purposes of tracking we record each call to this function as a QueryExecution in the database.
Possible caller-options include:
:executed-by [int] (User ID of caller)
:card-id [int] (ID of Card associated with this execution)"
{:arglists '([query options])}
[query {:keys [executed-by card-id]}]
{:pre [(integer? executed-by) (u/maybe? integer? card-id)]}
(let [query-uuid (str (java.util.UUID/randomUUID))
query-hash (hash query)
query-execution {:uuid query-uuid
:executor_id executed-by
:json_query query
:query_hash query-hash
:version 0
:status :starting
:error ""
:started_at (u/new-sql-timestamp)
:finished_at (u/new-sql-timestamp)
:running_time 0
:result_rows 0
:result_file ""
:result_data "{}"
:raw_query ""
:additional_info ""
:start_time_millis (System/currentTimeMillis)}
query (assoc query :info {:executed-by executed-by
:card-id card-id
:uuid query-uuid
:query-hash query-hash
:query-type (if (mbql-query? query) "MBQL" "native")})]
(try
(let [result (process-query query)]
(assert-valid-query-result result)
(query-complete query-execution result))
(catch Throwable e
(log/error (u/format-color 'red "Query failure: %s\n%s" (.getMessage e) (u/pprint-to-str (u/filtered-stacktrace e))))
(query-fail query-execution (.getMessage e))))))
(defn- save-query-execution!
"Save (or update) a `QueryExecution`."
[{:keys [id], :as query-execution}]
(if id
;; execution has already been saved, so update it
(u/prog1 query-execution
(db/update! QueryExecution id query-execution))
;; first time saving execution, so insert it
(db/insert! QueryExecution query-execution)))
(defn- query-fail
(defn- save-and-return-failed-query!
"Save QueryExecution state and construct a failed query response"
[query-execution error-message]
(let [updates {:status :failed
......@@ -586,7 +538,7 @@
:cols []
:columns []}))))
(defn- query-complete
(defn- save-and-return-successful-query!
"Save QueryExecution state and construct a completed (successful) query response"
[query-execution query-result]
;; record our query execution and format response
......@@ -602,12 +554,78 @@
(dissoc :error :raw_query :result_rows :version)
(merge query-result)))
(defn- save-query-execution!
"Save (or update) a `QueryExecution`."
[{:keys [id], :as query-execution}]
(if id
;; execution has already been saved, so update it
(u/prog1 query-execution
(db/update! QueryExecution id query-execution))
;; first time saving execution, so insert it
(db/insert! QueryExecution query-execution)))
(defn- assert-query-status-successful
"Make sure QUERY-RESULT `:status` is something other than `nil`or `:failed`, or throw an Exception."
[query-result]
(when-not (contains? query-result :status)
(throw (Exception. "invalid response from database driver. no :status provided")))
(when (= :failed (:status query-result))
(log/error (u/pprint-to-str 'red query-result))
(throw (Exception. (str (get query-result :error "general error"))))))
(def ^:dynamic ^Boolean *allow-queries-with-no-executor-id*
"Should we allow running queries (via `dataset-query`) without specifying the `executed-by` User ID?
By default this is `false`, but this constraint can be disabled for running queries not executed by a specific user
(e.g., public Cards)."
false)
(defn- query-execution-info
"Return the info for the `QueryExecution` entry for this QUERY."
[{{:keys [uuid executed-by query-hash]} :info, :as query}]
{:uuid (or uuid (throw (Exception. "Missing query UUID!")))
:executor_id executed-by
:json_query (dissoc query :info)
:query_hash (or query-hash (throw (Exception. "Missing query hash!")))
:version 0
:error ""
:started_at (u/new-sql-timestamp)
:finished_at (u/new-sql-timestamp)
:running_time 0
:result_rows 0
:result_file ""
:result_data "{}"
:raw_query ""
:additional_info ""
:start_time_millis (System/currentTimeMillis)})
(defn- run-and-save-query!
"Run QUERY and save appropriate `QueryExecution` info, and then return results (or an error message) in the usual format."
[query]
(let [query-execution (query-execution-info query)]
(try
(let [result (process-query query)]
(assert-query-status-successful result)
(save-and-return-successful-query! query-execution result))
(catch Throwable e
(log/error (u/format-color 'red "Query failure: %s\n%s" (.getMessage e) (u/pprint-to-str (u/filtered-stacktrace e))))
(save-and-return-failed-query! query-execution (.getMessage e))))))
(defn dataset-query
"Process and run a json based dataset query and return results.
Takes 2 arguments:
1. the json query as a dictionary
2. query execution options specified in a dictionary
Depending on the database specified in the query this function will delegate to a driver specific implementation.
For the purposes of tracking we record each call to this function as a QueryExecution in the database.
Possible caller-options include:
:executed-by [int] (User ID of caller)
:card-id [int] (ID of Card associated with this execution)"
{:arglists '([query options])}
[query {:keys [executed-by card-id]}]
{:pre [(or (integer? executed-by)
*allow-queries-with-no-executor-id*)
(u/maybe? integer? card-id)]}
(let [query-uuid (str (java.util.UUID/randomUUID))
query-hash (hash query)
query (assoc query :info {:executed-by executed-by
:card-id card-id
:uuid query-uuid
:query-hash query-hash
:query-type (if (mbql-query? query) "MBQL" "native")})]
(run-and-save-query! query)))
test/metabase/api/dataset_test.clj
+ 4
− 4
View file @ 86f4e632
......@@ -2,7 +2,7 @@
"Unit tests for /api/dataset endpoints."
(:require [clojure.string :as s]
[expectations :refer :all]
[metabase.api.dataset :refer [query-constraints]]
[metabase.api.dataset :refer [default-query-constraints]]
[metabase.db :as db]
(metabase.models [card :refer [Card]]
[query-execution :refer [QueryExecution]])
......@@ -65,7 +65,7 @@
(ql/aggregation (ql/count))))
(assoc :type "query")
(assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints))
(assoc :constraints default-query-constraints))
:started_at true
:finished_at true
:running_time true}
......@@ -81,7 +81,7 @@
(ql/aggregation (ql/count))))
(assoc :type "query")
(assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints))
(assoc :constraints default-query-constraints))
:started_at true
:finished_at true
:running_time true
......@@ -108,7 +108,7 @@
:json_query {:database (id)
:type "native"
:native {:query "foobar"}
:constraints query-constraints}
:constraints default-query-constraints}
:started_at true
:finished_at true
:running_time true}]
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment