code cleanup

project.clj

@@ -53,6 +53,7 @@
                  [com.taoensso/nippy "2.13.0"]                        ; Fast serialization (i.e., GZIP) library for Clojure
                  [compojure "1.5.2"]                                  ; HTTP Routing library built on Ring
                  [crypto-random "1.2.0"]                              ; library for generating cryptographically secure random bytes and strings
+                 [dk.ative/docjure "1.11.0"]                          ; Excel export
                  [environ "1.1.0"]                                    ; easy environment management
                  [hiccup "1.0.5"]                                     ; HTML templating
                  [honeysql "0.8.2"]                                   ; Transform Clojure data structures to SQL
@@ -78,8 +79,7 @@
                  [ring/ring-json "0.4.0"]                             ; Ring middleware for reading/writing JSON automatically
                  [stencil "0.5.0"]                                    ; Mustache templates for Clojure
                  [toucan "1.0.2"                                      ; Model layer, hydration, and DB utilities
-                  :exclusions [honeysql]]
-                 [dk.ative/docjure "1.11.0"]]                         ; Excel export
+                  :exclusions [honeysql]]]
   :repositories [["bintray" "https://dl.bintray.com/crate/crate"]]    ; Repo for Crate JDBC driver
   :plugins [[lein-environ "1.1.0"]                                    ; easy access to environment variables
             [lein-ring "0.11.0"                                       ; start the HTTP server with 'lein ring server'

src/metabase/api/card.clj

@@ -414,17 +414,18 @@
   (binding [cache/*ignore-cached-results* ignore_cache]
     (run-query-for-card card-id, :parameters parameters)))
 
-(api/defendpoint POST "/:card-id/query/:export-format-name"
+(api/defendpoint POST "/:card-id/query/:export-format"
   "Run the query associated with a Card, and return its results as a file in the specified format. Note that this expects the parameters as serialized JSON in the 'parameters' parameter"
-  [card-id export-format-name parameters]
-  {parameters (s/maybe su/JSONString)}
+  [card-id export-format parameters]
+  {parameters (s/maybe su/JSONString)
+   export-format dataset-api/export-format-schema}
   (binding [cache/*ignore-cached-results* true]
     (dataset-api/as-format
-       export-format-name
+       export-format
        (run-query-for-card card-id
          :parameters  (json/parse-string parameters keyword)
          :constraints nil
-         :context     :download))))
+         :context     (dataset-api/export-format-context export-format)))))
 
 ;;; ------------------------------------------------------------ Sharing is Caring ------------------------------------------------------------
 

src/metabase/api/dataset.clj

@@ -2,6 +2,7 @@
   "/api/dataset endpoints."
   (:require [cheshire.core :as json]
             [clojure.data.csv :as csv]
+            [clojure.string :as string]
             [compojure.core :refer [POST]]
             [dk.ative.docjure.spreadsheet :as spreadsheet]
             [metabase
@@ -12,7 +13,8 @@
              [database :refer [Database]]
              [query :as query]]
             [metabase.query-processor.util :as qputil]
-            [metabase.util.schema :as su]))
+            [metabase.util.schema :as su]
+            [schema.core :as s]))
 
 (def ^:private ^:const max-results-bare-rows
   "Maximum number of rows to return specifically on :rows type queries via the API."
@@ -45,13 +47,13 @@
                 (query/average-execution-time-ms (qputil/query-hash (assoc query :constraints default-query-constraints)))
                 0)})
 
-(defn ^:private export-to-csv
+(defn- export-to-csv
   [columns rows]
   (with-out-str
     ;; turn keywords into strings, otherwise we get colons in our output
     (csv/write-csv *out* (into [(mapv name columns)] rows))))
 
-(defn ^:private export-to-xlsx
+(defn- export-to-xlsx
   [columns rows]
   (let [wb (spreadsheet/create-workbook "Query result" (conj rows (mapv name columns)))
         ;; note: byte array streams don't need to be closed
@@ -59,39 +61,59 @@
     (spreadsheet/save-workbook! out wb)
     (java.io.ByteArrayInputStream. (.toByteArray out))))
 
-(defn ^:private export-to-json
+(defn- export-to-json
   [columns rows]
   (for [row rows]
     (zipmap columns row)))
 
 (def ^:private export-formats
-  {"csv"  {:export-fn export-to-csv,  :content-type "text/csv",                                                          :ext "csv"},
-   "xlsx" {:export-fn export-to-xlsx, :content-type "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", :ext "xlsx"},
-   "json" {:export-fn export-to-json, :content-type "applicaton/json",                                                   :ext "json"}})
+  {"csv"  {:export-fn    export-to-csv
+           :content-type "text/csv"
+           :ext          "csv"
+           :context      :csv-download},
+   "xlsx" {:export-fn    export-to-xlsx
+           :content-type "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
+           :ext          "xlsx"
+           :context      :xlsx-download},
+   "json" {:export-fn    export-to-json
+           :content-type "applicaton/json"
+           :ext          "json"
+           :context      :json-download}})
+
+(def export-format-schema (apply s/enum (keys export-formats)))
+
+(defn export-format-context [export-format]
+  (if-let [export-conf (export-formats export-format)]
+    (:context export-conf)))
 
 (defn as-format
   "Return a response containing the RESULTS of a query in the specified format."
-  {:arglists '([export-format-name results])}
-  [export-format-name {{:keys [columns rows]} :data, :keys [status], :as response}]
-  (let-404 [export-format (export-formats export-format-name)]
+  {:arglists '([export-format results])}
+  [export-format {{:keys [columns rows]} :data, :keys [status], :as response}]
+  (api/let-404 [export-conf (export-formats export-format)]
     (if (= status :completed)
       ;; successful query, send file
       {:status  200
-       :body ((:export-fn export-format) columns rows)
-       :headers {"Content-Type" (str (:content-type export-format) "; charset=utf-8")
-                 "Content-Disposition" (str "attachment; filename=\"query_result_" (u/date->iso-8601) "." (:ext export-format) "\"")}}
+       :body ((:export-fn export-conf) columns rows)
+       :headers {"Content-Type" (str (:content-type export-conf) "; charset=utf-8")
+                 "Content-Disposition" (str "attachment; filename=\"query_result_" (u/date->iso-8601) "." (:ext export-conf) "\"")}}
       ;; failed query, send error message
       {:status 500
        :body   (:error response)})))
 
-(def ^:private export-format-name-regex (re-pattern (str "(" (string/join "|" (keys export-formats)) ")")))
+(def ^:private export-format-regex (re-pattern (str "(" (string/join "|" (keys export-formats)) ")")))
 
-(defendpoint POST ["/:export-format-name", :export-format-name export-format-name-regex]
+(api/defendpoint POST ["/:export-format", :export-format export-format-regex]
   "Execute a query and download the result data as a file in the specified format."
-  [export-format-name query]
-  {query su/JSONString}
+  [export-format query]
+  {query su/JSONString
+   export-format export-format-schema}
   (let [query (json/parse-string query keyword)]
     (api/read-check Database (:database query))
-    (as-format export-format-name (qp/dataset-query (dissoc query :constraints) {:executed-by api/*current-user-id*, :context :download}))))
+    (as-format
+      export-format
+      (qp/dataset-query
+        (dissoc query :constraints)
+        {:executed-by api/*current-user-id*, :context (export-format-context export-format)}))))
 
 (api/define-routes)

src/metabase/api/embed.clj

@@ -276,10 +276,11 @@
   (run-query-for-unsigned-token (eu/unsign token) query-params))
 
 
-(api/defendpoint GET "/card/:token/query/:export-format-name"
+(api/defendpoint GET "/card/:token/query/:export-format"
   "Like `GET /api/embed/card/query`, but returns the results as a file in the specified format."
-  [token export-format-name & query-params]
-  (dataset-api/as-format export-format-name (run-query-for-unsigned-token (eu/unsign token) query-params, :constraints nil)))
+  [token export-format & query-params]
+  {export-format dataset-api/export-format-schema}
+  (dataset-api/as-format export-format (run-query-for-unsigned-token (eu/unsign token) query-params, :constraints nil)))
 
 
 ;;; ------------------------------------------------------------ /api/embed/dashboard endpoints ------------------------------------------------------------

src/metabase/api/public.clj

@@ -113,11 +113,12 @@
   {parameters (s/maybe su/JSONString)}
   (run-query-for-card-with-public-uuid uuid parameters))
 
-(api/defendpoint GET "/card/:uuid/query/:export-format-name"
+(api/defendpoint GET "/card/:uuid/query/:export-format"
   "Fetch a publically-accessible Card and return query results in the specified format. Does not require auth credentials. Public sharing must be enabled."
-  [uuid export-format-name parameters]
-  {parameters (s/maybe su/JSONString)}
-  (dataset-api/as-format export-format-name (run-query-for-card-with-public-uuid uuid parameters, :constraints nil)))
+  [uuid export-format parameters]
+  {parameters (s/maybe su/JSONString)
+   export-format dataset-api/export-format-schema}
+  (dataset-api/as-format export-format (run-query-for-card-with-public-uuid uuid parameters, :constraints nil)))
 
 ;;; ------------------------------------------------------------ Public Dashboards ------------------------------------------------------------
 

src/metabase/models/query_execution.clj

@@ -14,16 +14,18 @@
 (def Context
   "Schema for valid values of QueryExecution `:context`."
   (s/enum :ad-hoc
-          :download
+          :csv-download
           :dashboard
           :embedded-dashboard
           :embedded-question
+          :json-download
           :map-tiles
           :metabot
           :public-dashboard
           :public-question
           :pulse
-          :question))
+          :question
+          :xlsx-download))
 
 (defn- pre-insert [{context :context, :as query-execution}]
   (u/prog1 query-execution

src/metabase/routes.clj

@@ -34,15 +34,15 @@
 (def ^:private embed  (partial entrypoint "embed"  :embeddable))
 
 (defroutes ^:private public-routes
-  (GET ["/question/:uuid.:export-format-name" :uuid u/uuid-regex]
-       [uuid export-format-name]
-       (resp/redirect (format "/api/public/card/%s/query/%s" uuid export-format-name)))
+  (GET ["/question/:uuid.:export-format" :uuid u/uuid-regex]
+       [uuid export-format]
+       (resp/redirect (format "/api/public/card/%s/query/%s" uuid export-format)))
   (GET "*" [] public))
 
 (defroutes ^:private embed-routes
-  (GET "/question/:token.:export-format-name"
-       [token export-format-name]
-       (resp/redirect (format "/api/embed/card/%s/query/%s" token export-format-name)))
+  (GET "/question/:token.:export-format"
+       [token export-format]
+       (resp/redirect (format "/api/embed/card/%s/query/%s" token export-format)))
   (GET "*" [] embed))
 
 ;; Redirect naughty users who try to visit a page other than setup if setup is not yet complete