Merge pull request #141 from metabase/more_query_api_tests

More query api tests

src/metabase/api/query.clj

@@ -39,6 +39,7 @@
 (defn query-clone
   "Create a new query by cloning an existing query.  Returns a 403 if user doesn't have acces to read query."
   [query-id]
+  {:pre [(integer? query-id)]}
   (let-400 [{:keys [name] :as query} (sel :one Query :id query-id)]
     (read-check query)
     (->> (-> query
@@ -97,17 +98,18 @@
 
 
 (defendpoint POST "/:id" [id]
-  (let-404 [query (sel :one Query :id id)]
-           (read-check query)
-           (let [dataset-query {:type "native"
-                                :database (:database_id query)
-                                :native {:query (get-in query [:details :sql])
-                                         :timezone (get-in query [:details :timezone])}}
-                 options {:executed_by *current-user-id*
-                          :saved_query query
-                          :synchronously false
-                          :cache_result true}]
-             (driver/dataset-query dataset-query options))))
+  (let-404 [{database-id :database_id
+             {:keys [sql timezone]} :details :as query} (sel :one Query :id id)]
+    (read-check query)
+    (let [dataset-query {:type "native"
+                         :database database-id
+                         :native {:query sql
+                                  :timezone timezone}}
+          options {:executed_by *current-user-id*
+                   :saved_query query
+                   :synchronously false
+                   :cache_result true}]
+      (driver/dataset-query dataset-query options))))
 
 
 (defendpoint GET "/:id/results" [id]

src/metabase/models/query.clj

@@ -14,7 +14,7 @@
   (table :query_query))
 
 
-;; default fields to return for `sel Query
+;; default fields to return for `sel` Query
 (defmethod default-fields Query [_]
   [:id
    :created_at
@@ -50,5 +50,5 @@
               :database (delay
                          (check database_id 500 "Can't get database: Query doesn't have a :database_id.")
                          (sel :one Database :id database_id))
-              :organization_id (delay (:organization_id ((:database <>)))))
+              :organization_id (delay (:organization_id @(:database <>))))
       assoc-permissions-sets))

test/metabase/api/query_test.clj

@@ -3,17 +3,19 @@
   (:require [expectations :refer :all]
             [korma.core :refer :all]
             [metabase.db :refer :all]
-            (metabase.models [query :refer [Query]])
+            (metabase.models [query :refer [Query]]
+                             [query-execution :refer [QueryExecution]])
             [metabase.test.util :refer [match-$ random-name expect-eval-actual-first]]
             [metabase.test-data :refer :all]))
 
 ;; ## Helper Fns
 
-(defn create-query []
-  ((user->client :rasta) :post 200 "query" {:database (:id @test-db)
-                                            :sql "SELECT COUNT(*) FROM VENUES;"}))
+(defn create-query [& {:as kwargs}]
+  ((user->client :rasta) :post 200 "query" (merge {:database (:id @test-db)
+                                                   :sql "SELECT COUNT(*) FROM VENUES;"}
+                                                  kwargs)))
 
-;; ## POST /api/query
+;; ## POST /api/query (create)
 ;; Check that we can save a Query
 (expect-eval-actual-first
     (match-$ (sel :one Query (order :id :DESC))
@@ -30,7 +32,8 @@
        :created_at $})
   (create-query))
 
-;; ## GET /api/query
+
+;; ## GET /api/query/:id
 ;; Check that we can fetch details for a Query
 (expect-eval-actual-first
     (match-$ (sel :one Query (order :id :DESC))
@@ -68,16 +71,90 @@
   (let [{id :id} (create-query)]
     ((user->client :rasta) :get 200 (format "query/%d" id))))
 
-;; ## PUT /api/query
+
+;; ## PUT /api/query/:id
 ;; Check that we can update a Query
 (expect-eval-actual-first
-    ["My Awesome Query"
-     "My Awesome Query 2"]
+    [{:name "My Awesome Query"
+      :version 2}
+     {:name "My Awesome Query 2"
+      :version 3}]
   (let [{:keys [id name database_id]} (create-query)
-        get-query-name (fn [] (sel :one :field [Query :name] :id id))]
+        get-query-name-and-version (fn [] (sel :one :fields [Query :name :version] :id id))]
     [(do ((user->client :rasta) :put 200 (format "query/%d" id) {:name "My Awesome Query"
                                                                  :database {:id database_id}})
-         (get-query-name))
+         (get-query-name-and-version))
      (do ((user->client :rasta) :put 200 (format "query/%d" id) {:name "My Awesome Query 2"
                                                                  :database {:id database_id}})
-         (get-query-name))]))
+         (get-query-name-and-version))]))
+
+
+;; ## DELETE /api/query/:id
+;; Check that we can delete a Query
+(let [query-name (random-name)
+      get-query-name (fn [] (sel :one :field [Query :name] :name query-name))]
+  (expect-eval-actual-first
+      [query-name
+       nil]
+    (let [{id :id} (create-query :name query-name)]
+      [(get-query-name)
+       (do ((user->client :rasta) :delete 204 (format "query/%d" id))
+           (get-query-name))])))
+
+;; ## POST /api/query (clone)
+;; Can we clone a Query?
+(let [query-name (random-name)]
+  (expect-eval-actual-first
+      (match-$ (sel :one Query :name (format "%s CLONED" query-name))
+        {:database_id (:id @test-db)
+         :name $
+         :type "rawsql"
+         :creator_id (user->id :crowberto)
+         :updated_at $
+         :details {:timezone nil
+                   :sql "SELECT COUNT(*) FROM VENUES;"}
+         :id $
+         :version 1
+         :public_perms 0
+         :created_at $})
+    ;; Clone Query with a different User than the one that created it
+    (let [{id :id} (create-query :name query-name)]
+      ((user->client :crowberto) :post 200 "query" {:clone id}))))
+
+
+;; ## POST /api/query/:id & GET /api/query/:id/results
+;; Can we execute a Query (i.e., create a new QueryExecution) ?
+(expect-eval-actual-first
+    (let [{query-id :id :as query} (sel :one Query (order :id :DESC))
+          query-execution (sel :one QueryExecution :query_id query-id (order :id :DESC))]
+      [(match-$ query-execution
+         {:id $
+          :uuid $
+          :query_id query-id
+          :version 1
+          :status "starting"
+          :started_at $})
+       [(match-$ query-execution
+           {:query_id query-id
+            :raw_query ""
+            :result_rows 1
+            :finished_at $
+            :started_at $
+            :json_query {:native {:timezone nil
+                                  :query "SELECT COUNT(*) FROM VENUES;"}
+                         :database (:id @test-db)
+                         :type "native"}
+            :status "completed"
+            :id $
+            :uuid $
+            :row_count 1
+            :running_time $
+            :version 1})]])
+  (let [{id :id} (create-query)]
+    [;; POST /query/:id should create a new QueryExecution
+     ((user->client :rasta) :post 200 (format "query/%d" id))
+     ;; GET /query/:id/results should return array of QueryExecutions for the Query (e.g., the one we just created)
+     (do
+       ;; wait 100ms for QueryExecution to complete. If it takes longer than that, it's probably brokesies
+       (Thread/sleep 100)
+       ((user->client :rasta) :get 200 (format "query/%d/results" id)))]))