Merge pull request #71 from metabase/improvements

sel now handles "fully.qualified/Names", and fixes some issues with

src/metabase/api/common.clj

@@ -3,7 +3,7 @@
   (:require [compojure.core :refer [defroutes]]
             [medley.core :refer :all]
             [metabase.api.common.internal :refer :all]
-            [metabase.db :refer [sel-fn]])
+            [metabase.db :refer :all])
   (:import com.metabase.corvus.api.ApiException))
 
 ;;; ## DYNAMIC VARIABLES
@@ -34,8 +34,9 @@
    Case will be `nil`, `:default`, or `:admin`."
   [org-id & body]
   `(let [org-id# ~org-id]                                ; make sure org-id gets evaluated before get to `case`
-     (case (when @*current-user*
-             ((:perms-for-org @*current-user*) org-id#))
+     (case (when *current-user-id*
+             (when-let [{:keys [~'admin]} (sel :one ["metabase.models.org-perm/OrgPerm" :admin] :user_id *current-user-id* :organization_id org-id#)]
+               (if ~'admin :admin :default)))
        ~@body)))
 
 

src/metabase/api/meta/db.clj

@@ -64,8 +64,8 @@
   (sel :many Table :db_id id (order :name)))
 
 (defendpoint POST "/:id/sync" [id]
-  (let-404 [db (sel :one Database :id id)]   ; run sync-tables asynchronously.
-    (future (pgsync/sync-tables db)))        ; TODO - this only works for Postgres right now (since that's the only driver we have)
+  (let-404 [db (sel :one Database :id id)]   ; TODO - run sync-tables asynchronously
+           (pgsync/sync-tables db))          ; TODO - this only works for Postgres right now (since that's the only driver we have)
   {:status :ok})
 
 (define-routes)

src/metabase/db.clj

@@ -130,15 +130,21 @@
 
     (sel :many [OrgPerm :admin :id] :user_id 1) -> return admin and id of OrgPerms whose user_id is 1
 
+  ENTITY may optionally be a fully-qualified string name of an entity; in this case, the symbol's namespace
+  will be required and the symbol itself resolved at runtime. This is sometimes neccesary to avoid circular
+  dependencies. This is slower, however, due to added runtime overhead.
+
+    (sel :one \"metabase.models.table/Table\" :id 1) ; require/resolve metabase.models.table/Table. then sel Table 1
+
   FORMS may be either keyword args, which will be added to a korma `where` clause, or other korma
    clauses such as `order`, which are passed directly.
 
     (sel :many Table :db_id 1)                    -> (select User (where {:id 1}))
     (sel :many Table :db_id 1 (order :name :ASC)) -> (select User (where {:id 1}) (order :name ASC))"
   [one-or-many entity & forms]
+  {:pre [(contains? #{:one :many} one-or-many)]}
   `(->> (-sel-select ~entity ~@forms)
-        (map (partial post-select ~(if (vector? entity) (first entity)
-                                       entity)))
+        (map (partial post-select (entity->korma ~entity)))
         ~(case one-or-many
            :one 'first
            :many 'identity)))
@@ -153,27 +159,15 @@
                    (sel-apply-fields field-keys))]
     `(do (when *log-db-calls*
            (println "DB CALL: " ~(str entity) " " ~(str forms)))
-         (select ~entity ~@forms))))
+         (select (entity->korma ~entity) ~@forms))))
 
 (defmacro sel-fn
-  "Returns a memoized fn that calls `sel`.
-
-   ENTITY may optionally be a fully-qualified string name of an entity; in this case, the symbol's namespace
-   will be required and the symbol itself resolved at runtime. This is sometimes neccesary to avoid circular
-   dependencies in the model files. This is slower, however, due to added runtime overhead.
-
-     (sel :one Table :id 1)                           ; returns fn that will sel Table 1 when called
-     (sel :one \"metabase.models.table/Table\" :id 1) ; returns fn that will require/resolve metabase.models.table/Table. then sel Table 1"
+  "Returns a memoized fn that calls `sel`."
   [one-or-many entity & forms]
   `(memoize
     (fn []
-      ~@(if (string? entity)
-          `((require '~(-> ^String entity (.split "/") first symbol)) ; require the namespace
-            (let [entity# (symbol ~entity)]
-              (eval `(sel ~~one-or-many ~entity# ~~@forms))))
-          `((sel ~one-or-many ~entity ~@forms))))))
+      (sel ~one-or-many ~entity ~@forms))))
 
-;;
 (defmacro exists?
   "Easy way to see if something exists in the db.
    TODO: How can we disable the `post-select` functionality for this call.

src/metabase/db/internal.clj

@@ -36,3 +36,20 @@
   [field-keys forms]
   (if-not (empty? field-keys) (conj forms `(fields ~@field-keys))
           forms))
+
+(defn entity->korma
+  "Convert an ENTITY argument to `sel`/`sel-fn` into the form we should pass to korma `select` and to various multi-methods such as
+   `post-select`.
+
+    *  If entity is a vector like `[User :name]`, only keeps the first arg (`User`)
+    *  Converts fully-qualified entity name strings like `\"metabase.models.user/User\"` to the corresponding entity
+       and requires their namespace if needed."
+  [entity]
+  {:post [(= (type %) :korma.core/Entity)]}
+  (if (vector? entity) (entity->korma (first entity))
+      (if (string? entity) (let [ns (-> (.split ^String entity "/")
+                                        first
+                                        symbol)]
+                             (require ns)
+                             (eval (symbol entity)))
+          entity)))