dox + API tests

src/metabase/api/setting.clj

@@ -11,7 +11,14 @@
   (write-check Org org)
   (setting/all-with-descriptions org))
 
+;; ## Get a single setting
+(defendpoint GET "/:key" [key org]
+  (require-params key org)
+  (write-check Org org)
+  (setting/get org (keyword key)))
+
 ;; ## Create/update a setting
+;; We could write a separate `PUT` endpoint but that's just needless duplication IMO
 (defendpoint POST "/" [:as {{:keys [org key value]} :body}]
   (require-params key value org)
   (write-check Org org)

src/metabase/api/user.clj

@@ -23,7 +23,7 @@
 (defendpoint GET "/:id" [id]
   ;; user must be getting their own details OR they must be a superuser to proceed
   (check-403 (or (= id *current-user-id*) (:is_superuser @*current-user*)))
-  (sel :one User :id id))
+  (check-404 (sel :one User :id id)))
 
 
 (defendpoint PUT "/:id" [id :as {{:keys [email] :as body} :body}]

src/metabase/core.clj

@@ -7,6 +7,7 @@
             (metabase.middleware [auth :as auth]
                                  [log-api-call :refer :all]
                                  [format :refer :all])
+            [metabase.models.user :refer [User]]
             [metabase.routes :as routes]
             [metabase.util :as util]
             [ring.adapter.jetty :as ring-jetty]
@@ -44,8 +45,9 @@
   (db/setup-db :auto-migrate (config/config-bool :mb-db-automigrate))
 
   ;; this is a temporary need until we finalize the code for bootstrapping the first user
-  (when-not (db/exists? metabase.models.user/User :id 1)
-    (db/ins metabase.models.user/User :email "admin@admin.com" :first_name "admin" :last_name "admin" :password "admin" :is_superuser true))
+  (when-not (or (db/exists? User :id 1)
+                (db/exists? User :email "admin@admin.com")) ; possible for User 1 to have been deleted
+    (db/ins User :email "admin@admin.com" :first_name "admin" :last_name "admin" :password "admin" :is_superuser true))
 
   (log/info "Metabase Initialization COMPLETE")
   true)
@@ -72,4 +74,4 @@
                              :max-idle-time (config/config-int :mb-jetty-maxidletime)}
                          (medley/filter-vals identity))]
       (log/info "Launching Embedded Jetty Webserver with config:\n" (with-out-str (clojure.pprint/pprint jetty-config)))
-      (ring-jetty/run-jetty app jetty-config))))
\ No newline at end of file
+      (ring-jetty/run-jetty app jetty-config))))

src/metabase/middleware/format.clj

@@ -25,17 +25,13 @@
                      (not (delay? %)))
                m))
 
-(defn- type-key
-  [obj]
-  (cond (map? obj) :map
-        (coll? obj) :coll
-        (= (type obj) org.h2.jdbc.JdbcClob) :jdbc-clob
-        :else :obj))
+(defn- clob? [obj]
+  (= (type obj) org.h2.jdbc.JdbcClob))
 
 (defn- -format-response [obj]
-  (case (type-key obj)
-    :obj obj
-    :map (->> (remove-fns-and-delays obj)
-              (map-vals -format-response))       ; recurse over all vals in the map
-    :coll (map -format-response obj)             ; recurse over all items in the collection
-    :jdbc-clob (util/jdbc-clob->str obj)))
+  (cond
+    (map? obj)  (->> (remove-fns-and-delays obj)   ; recurse over all vals in the map
+                    (map-vals -format-response))
+    (coll? obj) (map -format-response obj)        ; recurse over all items in the collection
+    (clob? obj) (util/jdbc-clob->str obj)
+    :else       obj))

src/metabase/models/setting.clj

@@ -4,6 +4,34 @@
             [korma.core :refer :all :exclude [delete]]
             [metabase.db :refer [sel del]]))
 
+;; Settings are a fast + simple way to create an Org-specific setting that can be
+;; set from the admin page. They are saved to the Database, but intelligently
+;; cached internally for super-fast lookups.
+;;
+;; Define a new Setting with `defsetting`:
+;;
+;;    (defsetting mandrill-api-key "API key for Mandrill")
+;;
+;; The setting and docstr will then be auto-magically accessible from the admin page.
+;;
+;; The var created with `defsetting` can be used as a getter/setter, or you can
+;; use `get`/`set`/`delete`:
+;;
+;;     (require '[metabase.models.setting :as setting])
+;;
+;;     (setting/get org-id :mandrill-api-key)
+;;     (mandrill-api-key org-id)
+;;
+;;     (setting/set org-id :mandrill-api-key "MY NEW API KEY")
+;;     (mandrill-api-key org-id "MY NEW API KEY")
+;;
+;;     (setting/delete org-id :mandrill-api-key)
+;;     (mandrill-api-key org-id nil)
+;;
+;; Get a map of all Settings for an Org:
+;;
+;;    (setting/all org-id)
+
 (declare Setting
          cached-setting-values
          restore-cache-if-needed
@@ -96,7 +124,7 @@
        (reduce merge {})))
 
 (defn all-with-descriptions
-  "Return a combined list of all `Settings` and values for `Org`, if they exist."
+  "Return a combined list of all `Settings` (including description) and values for `Org`, if they exist."
   [org-id]
   (let [settings-for-org (all org-id)]
     (->> (settings-list)

test/metabase/api/setting_test.clj

+(ns metabase.api.setting-test
+  (:require [expectations :refer :all]
+            (metabase.models [setting :as setting]
+                             [setting-test :refer [set-settings
+                                                   setting-exists?
+                                                   test-setting-1
+                                                   test-setting-2]])
+            [metabase.test-data :refer :all]
+            [metabase.test.util :refer :all]))
+
+;; ## Helper Fns
+(defn fetch-all-settings  []
+  (filter (fn [{k :key}]
+            (re-find #"^test-setting-\d$" (name k)))
+          ((user->client :rasta) :get 200 "setting" :org @org-id)))
+
+(defn fetch-setting [setting-name]
+  ((user->client :rasta) :get 200 (format "setting/%s" (name setting-name)) :org @org-id))
+
+;; ## GET /api/setting
+;; Check that we can fetch all Settings for Org
+(expect-eval-actual-first
+    [{:key "test-setting-1", :value nil,     :description "Test setting - this only shows up in dev (1)"}
+     {:key "test-setting-2", :value "FANCY", :description "Test setting - this only shows up in dev (2)"}
+     {:key "test-setting-3", :value nil,     :description "Test setting - this only shows up in dev (3)"}]
+    (do (set-settings nil "FANCY" nil)
+        (fetch-all-settings)))
+
+;; Check that a non-admin can't read settings
+(expect "You don't have permissions to do that."
+  ((user->client :lucky) :get 403 "setting" :org @org-id))
+
+
+;; ## GET /api/setting/:key
+;; Test that we can fetch a single setting
+(expect-eval-actual-first
+    "OK!"
+    (do (test-setting-2 @org-id "OK!")
+        (fetch-setting :test-setting-2)))
+
+
+;; ## POST /api/setting
+(expect-eval-actual-first
+    ["NICE!"
+     "NICE!"]
+    (do ((user->client :rasta) :post 200 "setting" {:org @org-id
+                                                    :key "test-setting-1"
+                                                    :value "NICE!"})
+        [(test-setting-1 @org-id)
+         (fetch-setting :test-setting-1)]))
+
+;; ## Check non-admin can't set a Setting
+(expect "You don't have permissions to do that."
+  ((user->client :lucky) :post 403 "setting" {:org @org-id
+                                              :key "test-setting-1"
+                                              :value "NICE!"}))
+
+;; ## DELETE /api/setting/:key
+(expect-eval-actual-first
+    [nil
+     nil
+     false]
+  (do ((user->client :rasta) :delete 204 "setting/test-setting-1" :org @org-id)
+      [(test-setting-1 @org-id)
+       (fetch-setting :test-setting-1)
+       (setting-exists? :test-setting-1)]))

test/metabase/http_client.clj

@@ -99,7 +99,8 @@
              clojure.walk/keywordize-keys
              auto-deserialize-dates)
          (catch Exception _
-           body))))
+           (if (clojure.string/blank? body) nil
+               body)))))
 
 (defn authenticate [{:keys [email password] :as credentials}]
   {:pre [(string? email)

test/metabase/models/setting_test.clj

@@ -15,7 +15,7 @@
 
 ;; ## HELPER FUNCTIONS
 
-(defn fetch-setting
+(defn db-fetch-setting
   "Fetch `Setting` value from the DB to verify things work as we expect."
   [setting-name]
   (sel :one :field [Setting :value] :key (name setting-name) :organization_id @org-id))
@@ -23,6 +23,11 @@
 (defn setting-exists? [setting-name]
   (exists? Setting :key (name setting-name) :organization_id @org-id))
 
+(defn set-settings [setting-1-value setting-2-value setting-3-value]
+  (test-setting-1 @org-id setting-1-value)
+  (test-setting-2 @org-id setting-2-value)
+  (test-setting-3 @org-id setting-3-value))
+
 
 ;; ## GETTERS
 ;; Test defsetting getter fn
@@ -43,7 +48,7 @@
   [(test-setting-2 @org-id)
    (do (test-setting-2 @org-id "FANCY NEW VALUE <3")
        (test-setting-2 @org-id))
-   (fetch-setting :test-setting-2)])
+   (db-fetch-setting :test-setting-2)])
 
 ;; Test `set` function
 (expect-eval-actual-first
@@ -53,7 +58,7 @@
   [(test-setting-3 @org-id)
    (do (setting/set @org-id :test-setting-3 "WHAT A NICE VALUE <3")
        (test-setting-3 @org-id))
-   (fetch-setting :test-setting-3)])
+   (db-fetch-setting :test-setting-3)])
 
 
 ;; ## DELETE
@@ -87,11 +92,9 @@
 
 ;; all
 (expect-eval-actual-first
-    {:test-setting-1 "GREAT!"
-     :test-setting-2 "TOUCANS"}
-  (do (test-setting-1 @org-id nil)
-      (test-setting-2 @org-id "GREAT!")
-      (test-setting-3 @org-id "TOUCANS")
+    {:test-setting-2 "BIRDS<3"
+     :test-setting-3 "TOUCANS"}
+  (do (set-settings nil "BIRDS<3" "TOUCANS")
       (m/filter-keys #(re-find #"^test-setting-\d$" (name %)) ; filter out any non-test settings
                      (setting/all @org-id))))
 
@@ -100,9 +103,7 @@
     [{:key :test-setting-1, :value nil,  :description "Test setting - this only shows up in dev (1)"}
      {:key :test-setting-2, :value "S2", :description "Test setting - this only shows up in dev (2)"}
      {:key :test-setting-3, :value "S3", :description "Test setting - this only shows up in dev (3)"}]
-  (do (test-setting-1 @org-id nil)
-      (test-setting-2 @org-id "S2")
-      (test-setting-3 @org-id "S3")
+  (do (set-settings nil "S2" "S3")
       (filter (fn [{k :key}]
                 (re-find #"^test-setting-\d$" (name k)))
               (setting/all-with-descriptions @org-id))))