can_read and can_write are now hydrated via methods

src/metabase/models/card.clj

@@ -2,8 +2,7 @@
   (:require [korma.core :refer :all, :exclude [defentity]]
             [metabase.api.common :refer [*current-user-id*]]
             [metabase.db :refer :all]
-            (metabase.models [common :refer :all]
-                             [interface :refer :all]
+            (metabase.models [interface :refer :all]
                              [user :refer [User]])))
 
 (def ^:const display-types
@@ -19,6 +18,14 @@
     :table
     :timeseries})
 
+(defrecord CardInstance []
+  clojure.lang.IFn
+  (invoke [this k]
+    (get this k)))
+
+(extend-ICanReadWrite CardInstance :read :public-perms, :write :public-perms)
+
+
 (defentity Card
   [(table :report_card)
    (hydration-keys card)
@@ -26,10 +33,10 @@
    timestamped]
 
   (post-select [_ {:keys [creator_id] :as card}]
-    (-> (assoc card
-               :creator (delay (sel :one User :id creator_id)))
-        assoc-permissions-sets))
+    (map->CardInstance (assoc card :creator (delay (sel :one User :id creator_id)))))
 
   (pre-cascade-delete [_ {:keys [id]}]
     (cascade-delete 'metabase.models.dashboard-card/DashboardCard :card_id id)
     (cascade-delete 'metabase.models.card-favorite/CardFavorite :card_id id)))
+
+(extend-ICanReadWrite CardEntity :read :public-perms, :write :public-perms)

src/metabase/models/common.clj

@@ -15,8 +15,6 @@
    "US/Pacific"
    "America/Costa_Rica"])
 
-;;; ALLEN'S PERMISSIONS IMPLEMENTATION
-
 (def ^:const perms-none 0)
 (def ^:const perms-read 1)
 (def ^:const perms-readwrite 2)
@@ -27,50 +25,6 @@
    {:id perms-readwrite :name "Read & Write"}])
 
 
-;;; CAM'S PERMISSIONS IMPL
-;; (TODO - need to use one or the other)
-
-(defn public-permissions
-  "Return the set of public permissions for some object with key `:public_perms`. Possible permissions are `:read` and `:write`."
-  [{:keys [public_perms]}]
-  (check public_perms 500 "Can't check public permissions: object doesn't have :public_perms.")
-  ({0 #{}
-    1 #{:read}
-    2 #{:read :write}} public_perms))
-
-(defn user-permissions
-  "Return the set of current user's permissions for some object with keys `:creator_id` and `:public_perms`."
-  [{:keys [creator_id public_perms] :as obj}]
-  (check creator_id      500 "Can't check user permissions: object doesn't have :creator_id."
-         public_perms    500 "Can't check user permissions: object doesn't have :public_perms.")
-  (cond (:is_superuser *current-user*)   #{:read :write}    ; superusers have full access to everything
-        (= creator_id *current-user-id*) #{:read :write}    ; if user created OBJ they have all permissions
-        (<= perms-read public_perms)     #{:read}           ; if the object is public then everyone gets :read
-        :else                            #{}))              ; default is user has no permissions a.k.a private
-
-(defn user-can?
-  "Check if *current-user* has a given PERMISSION for OBJ.
-   PERMISSION should be either `:read` or `:write`."
-  [permission obj]
-  (contains? @(:user-permissions-set obj) permission))
-
-(defn assoc-permissions-sets
-  "Associates the following delays with OBJ:
-
-   *  `:public-permissions-set`
-   *  `:user-permissions-set`
-   *  `:can_read`
-   *  `:can_write`
-
-  Note that these delays depend upon the presence of `creator_id`, and `public_perms` fields in OBJ."
-  [obj]
-  (u/assoc* obj
-    :public-permissions-set (delay (public-permissions <>))
-    :user-permissions-set   (delay (user-permissions <>))
-    :can_read               (delay (user-can? :read <>))
-    :can_write              (delay (user-can? :write <>))))
-
-
 (defn name->human-readable-name
   "Convert a string NAME of some object like a `Table` or `Field` to one more friendly to humans.
 

src/metabase/models/dashboard.clj

@@ -7,6 +7,14 @@
                              [user :refer [User]])
             [metabase.util :as u]))
 
+(defrecord DashboardInstance []
+  clojure.lang.IFn
+  (invoke [this k]
+    (get this k)))
+
+(extend-ICanReadWrite DashboardInstance :read :public-perms, :write :public-perms)
+
+
 (defentity Dashboard
   [(table :report_dashboard)
    timestamped]
@@ -16,7 +24,9 @@
         (assoc :creator       (delay (sel :one User :id creator_id))
                :description   (u/jdbc-clob->str description)
                :ordered_cards (delay (sel :many DashboardCard :dashboard_id id (order :created_at :asc))))
-        assoc-permissions-sets))
+        map->DashboardInstance))
 
   (pre-cascade-delete [_ {:keys [id]}]
     (cascade-delete DashboardCard :dashboard_id id)))
+
+(extend-ICanReadWrite DashboardEntity :read :public-perms, :write :public-perms)

src/metabase/models/database.clj

@@ -2,8 +2,7 @@
   (:require [korma.core :refer :all, :exclude [defentity]]
             [metabase.api.common :refer [*current-user*]]
             [metabase.db :refer :all]
-            (metabase.models [common :refer [assoc-permissions-sets]]
-                             [interface :refer :all])))
+            [metabase.models.interface :refer :all]))
 
 (defrecord DatabaseInstance []
   ;; preserve normal IFn behavior so things like ((sel :one Database) :id) work correctly

src/metabase/models/hydrate.clj

 (ns metabase.models.hydrate
   "Functions for deserializing and hydrating fields in objects fetched from the DB."
   (:require [metabase.db :refer [sel]]
+            [metabase.models.interface :as models]
             [metabase.util :as u]))
 
 (declare batched-hydrate
@@ -131,14 +132,22 @@
   (if (can-batched-hydrate? results k) (batched-hydrate results k)
       (simple-hydrate results k)))
 
+(def ^:private hydration-k->method
+  "Methods that can be used to hydrate corresponding keys."
+  {:can_read  #(models/can-read? %)
+   :can_write #(models/can-write? %)})
+
 (defn- simple-hydrate
   "Hydrate keyword K in results by dereferencing corresponding delays when applicable."
   [results k]
   {:pre [(keyword? k)]}
   (map (fn [result]
          (let [v (k result)]
-           (if-not (delay? v) result      ; if v isn't a delay it's either already hydrated or nil.
-                   (assoc result k @v)))) ; don't barf on nil; just no-op
+           (cond
+             (delay? v)                    (assoc result k @v)                               ; hydrate delay if possible
+             (and (not v)
+                  (hydration-k->method k)) (assoc result k ((hydration-k->method k) result)) ; otherwise if no value exists look for a method we can use for hydration
+             :else                         result)))                                         ; otherwise don't barf, v may already be hydrated
        results))
 
 (defn- already-hydrated?

src/metabase/models/interface.clj

@@ -8,20 +8,50 @@
             metabase.db.internal
             [metabase.util :as u]))
 
+;;; ## ---------------------------------------- PERMISSIONS CHECKING ----------------------------------------
+
 (defprotocol ICanReadWrite
   (can-read?  [obj] [entity ^Integer id])
   (can-write? [obj] [entity ^Integer id]))
 
+(defn- superuser? [& _]
+  (:is_superuser @@(resolve 'metabase.api.common/*current-user*)))
+
+(defn- owner?
+  ([{:keys [creator_id], :as obj}]
+   (assert creator_id "Can't check user permissions: object doesn't have :creator_id.")
+   (or (superuser?)
+       (= creator_id @(resolve 'metabase.api.common/*current-user-id*))))
+  ([entity id]
+   (or (superuser?)
+       (owner? (entity id)))))
+
+(defn- has-public-perms? [rw]
+  (let [perms (delay (require 'metabase.models.common)
+                     (case rw
+                       :r @(resolve 'metabase.models.common/perms-read)
+                       :w @(resolve 'metabase.models.common/perms-readwrite)))]
+    (fn -has-public-perms?
+      ([{:keys [public_perms], :as obj}]
+       (assert public_perms "Can't check user permissions: object doesn't have :public_perms.")
+       (or (owner? obj)
+           (>= public_perms @perms)))
+      ([entity id]
+       (or (owner? entity id)
+           (-has-public-perms? (entity id)))))))
+
 (defn extend-ICanReadWrite
   "Add standard implementations of `can-read?` and `can-write?` to KLASS."
   [klass & {:keys [read write]}]
-  (let [key->method #(case %
-                       :always    (constantly true)
-                       :superuser (fn [& args]
-                                    (:is_superuser @@(resolve 'metabase.api.common/*current-user*))))]
+  (let [key->method (fn [rw v]
+                      (case v
+                        :always       (constantly true)
+                        :public-perms (has-public-perms? rw)
+                        :owner        #'owner?
+                        :superuser    #'superuser?))]
     (extend klass
-      ICanReadWrite {:can-read?  (key->method read)
-                     :can-write? (key->method write)})))
+      ICanReadWrite {:can-read?  (key->method :r read)
+                     :can-write? (key->method :w write)})))
 
 
 ;;; ## ---------------------------------------- ENTITIES ----------------------------------------
@@ -102,15 +132,16 @@
 (defn -invoke-entity
   "Basically the same as `(sel :one Entity :id id)`." ; TODO - deduplicate with sel
   [entity id]
-  (when (metabase.config/config-bool :mb-db-logging)
-    (clojure.tools.logging/debug
-     "DB CALL: " (:name entity) id))
-  (let [[obj] (k/select (assoc entity :fields (::default-fields entity))
-                        (k/where {:id id})
-                        (k/limit 1))]
-    (some->> obj
-             (internal-post-select entity)
-             (post-select entity))))
+  (when id
+    (when (metabase.config/config-bool :mb-db-logging)
+      (clojure.tools.logging/debug
+       "DB CALL: " (:name entity) id))
+    (let [[obj] (k/select (assoc entity :fields (::default-fields entity))
+                          (k/where {:id id})
+                          (k/limit 1))]
+      (some->> obj
+               (internal-post-select entity)
+               (post-select entity)))))
 
 (defn- update-updated-at [obj]
   (assoc obj :updated_at (u/new-sql-timestamp)))

test/metabase/models/common_test.clj

-(ns metabase.models.common-test
-  (:require [expectations :refer :all]
-            [metabase.api.common :refer [*current-user-id*]]
-            [metabase.models.common :refer :all]))
-
-;;; tests for PUBLIC-PERMISSIONS
-
-(expect #{}
-  (public-permissions {:public_perms 0}))
-
-(expect #{:read}
-  (public-permissions {:public_perms 1}))
-
-(expect #{:read :write}
-  (public-permissions {:public_perms 2}))
-
-
-;;; tests for USER-PERMISSIONS
-
-;; creator can read + write
-(expect #{:read :write}
-  (binding [*current-user-id* 100]
-    (user-permissions {:creator_id 100
-                       :public_perms 0})))
-
-;; TODO - write tests for the rest of the `user-permissions`