diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj index 54f28cc9d4138600ec9f2c75b0f1222881486a8b..5957d4f801d492a7145a352a3c9eb197017687bf 100644 --- a/src/metabase/api/card.clj +++ b/src/metabase/api/card.clj @@ -57,7 +57,7 @@ (defendpoint GET "/:id" "Get `Card` with ID." [id] - (->404 (sel :one Card :id id) + (->404 (Card id) read-check (hydrate :creator :can_read :can_write))) @@ -75,7 +75,7 @@ :name name :public_perms public_perms :visualization_settings visualization_settings)) - (sel :one Card :id id)) + (Card id)) (defendpoint DELETE "/:id" "Delete a `Card`." diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj index d2e22b51a31cb0cd560166ab8a7edf1fa4575c88..8edb69b6bd6778c231300846daff1ebd2ba5395f 100644 --- a/src/metabase/api/common.clj +++ b/src/metabase/api/common.clj @@ -8,6 +8,7 @@ [metabase.api.common.internal :refer :all] [metabase.db :refer :all] [metabase.db.internal :refer [entity->korma]] + [metabase.models.interface :as models] [metabase.util :as u] [metabase.util.password :as password]) (:import com.metabase.corvus.api.ApiException @@ -29,17 +30,6 @@ (atom nil)) ; default binding is just something that will return nil when dereferenced -;;; ## GENERAL HELPER FNS / MACROS - -;; TODO - move this to something like `metabase.util.debug` -(defmacro with-current-user - "Primarily for debugging purposes. Evaulates BODY as if `*current-user*` was the User with USER-ID." - [user-id & body] - `(binding [*current-user-id* ~user-id - *current-user* (delay (sel :one 'metabase.models.user/User :id ~user-id))] - ~@body)) - - ;;; ## CONDITIONAL RESPONSE FUNCTIONS / MACROS (defn check @@ -421,44 +411,26 @@ `(defroutes ~'routes ~@api-routes ~@additional-routes))) -;; ## NEW PERMISSIONS CHECKING MACROS -;; Since checking `@can_read`/`@can_write` is such a common pattern, these -;; macros eliminate a bit of the redundancy around doing so. -;; They support two forms: -;; -;; (read-check my-table) ; checks @(:can_read my-table) -;; (read-check Table 1) ; checks @(:can_read (sel :one Table :id 1)) -;; -;; * The first form is useful when you've already fetched an object (especially in threading forms such as `->404`). -;; * The second form takes care of fetching the object for you and is useful in cases where you won't need the object afterward -;; or want to combine the `sel` and permissions check statements into a single form. -;; -;; Both forms will throw a 404 if the object doesn't exist (saving you one more check!) and return the selected object. - -(defmacro read-check - "Checks that `@can_read` is true for this object." +(defn read-check + "Check whether we can read an existing OBJ, or ENTITY with ID." ([obj] - `(let-404 [{:keys [~'can_read] :as obj#} ~obj] - (check-403 @~'can_read) - obj#)) + (check-404 obj) + (check-403 (models/can-read? obj)) + obj) ([entity id] - (cond - ;; simple optimization : since @can-read is always true for a Database - ;; the read-check macro will just resolve to true in this simple case - ;; use `name` so we can match 'Database or 'metabase.models.database/Database - ;; - ;; TODO - it would be nice to generalize the read-checking pattern, and make it - ;; a separate multimethod or protocol so other models besides DB can write optimized - ;; implementations. Currently, we always fetch an *entire* object to do read checking, - ;; which is wasteful. - (= (name entity) "Database") `(comment "@(:can-read database) is always true.") ; put some non-constant value here so Eastwood doesn't complain about unused return values - :else `(read-check (sel :one ~entity :id ~id))))) - -(defmacro write-check - "Checks that `@can_write` is true for this object." + {:pre [(models/metabase-entity? entity) + (integer? id)]} + (if (satisfies? models/ICanReadWrite entity) + (read-check (entity id))))) + +(defn write-check + "Check whether we can write an existing OBJ, or ENTITY with ID." ([obj] - `(let-404 [{:keys [~'can_write] :as obj#} ~obj] - (check-403 @~'can_write) - obj#)) + (check-404 obj) + (check-403 (models/can-write? obj)) + obj) ([entity id] - `(write-check (sel :one ~entity :id ~id)))) + {:pre [(models/metabase-entity? entity) + (integer? id)]} + (if (satisfies? models/ICanReadWrite entity) (models/can-write? entity id) + (write-check (entity id))))) diff --git a/src/metabase/api/dash.clj b/src/metabase/api/dash.clj index 742cfb841dbf7e19bede9a22d035468db5f5deb5..a05a07715a72c1ae08052087fcd3df0ed28bbcfc 100644 --- a/src/metabase/api/dash.clj +++ b/src/metabase/api/dash.clj @@ -36,7 +36,7 @@ (defendpoint GET "/:id" "Get `Dashboard` with ID." [id] - (let-404 [db (-> (sel :one Dashboard :id id) + (let-404 [db (-> (Dashboard id) read-check (hydrate :creator [:ordered_cards [:card :creator]] :can_read :can_write))] {:dashboard db})) ; why is this returned with this {:dashboard} wrapper? @@ -50,7 +50,7 @@ :description description :name name :public_perms public_perms)) - (sel :one Dashboard :id id)) + (Dashboard id)) (defendpoint DELETE "/:id" "Delete a `Dashboard`." diff --git a/src/metabase/api/meta/db.clj b/src/metabase/api/meta/db.clj index 49c9a4c898c7e2d81fec5b78500c8b9de0ea3f5a..2f9d7b0445610260907b868c6464c109cacb87f4 100644 --- a/src/metabase/api/meta/db.clj +++ b/src/metabase/api/meta/db.clj @@ -64,7 +64,7 @@ (defendpoint GET "/:id" "Get `Database` with ID." [id] - (check-404 (sel :one Database :id id))) + (check-404 (Database id))) (defendpoint PUT "/:id" "Update a `Database`." @@ -75,7 +75,7 @@ :name name :engine engine :details details)) - (sel :one Database :id id)) + (Database id)) (defendpoint DELETE "/:id" "Delete a `Database`." @@ -126,7 +126,7 @@ (defendpoint POST "/:id/sync" "Update the metadata for this `Database`." [id] - (let-404 [db (sel :one Database :id id)] + (let-404 [db (Database id)] (write-check db) (future (driver/sync-database! db))) ; run sync-tables asynchronously {:status :ok}) diff --git a/src/metabase/api/meta/field.clj b/src/metabase/api/meta/field.clj index 36c6f6cbd7d278362999b35f4ce7734fbfa8acc4..eb37aa20e4e387b55f20050122bd62f6a133b84e 100644 --- a/src/metabase/api/meta/field.clj +++ b/src/metabase/api/meta/field.clj @@ -29,7 +29,7 @@ (defendpoint GET "/:id" "Get `Field` with ID." [id] - (->404 (sel :one Field :id id) + (->404 (Field id) read-check (hydrate [:table :db]))) @@ -43,12 +43,12 @@ :special_type special_type} ; but field_type and preview_display must be replaced (when field_type {:field_type field_type}) ; with new non-nil values (when (not (nil? preview_display)) {:preview_display preview_display})))) - (sel :one Field :id id)) + (Field id)) (defendpoint GET "/:id/summary" "Get the count and distinct count of `Field` with ID." [id] - (let-404 [field (sel :one Field :id id)] + (let-404 [field (Field id)] (read-check field) [[:count (metadata/field-count field)] [:distincts (metadata/field-distinct-count field)]])) @@ -79,7 +79,7 @@ "If `Field`'s special type is `category`/`city`/`state`/`country`, or its base type is `BooleanField`, return all distinct values of the field, and a map of human-readable values defined by the user." [id] - (let-404 [field (sel :one Field :id id)] + (let-404 [field (Field id)] (read-check field) (if-not (field-should-have-field-values? field) {:values {} :human_readable_values {}} @@ -91,7 +91,7 @@ or whose base type is `BooleanField`." [id :as {{:keys [fieldId values_map]} :body}] ; WTF is the reasoning behind client passing fieldId in POST params? {values_map [Required Dict]} - (let-404 [field (sel :one Field :id id)] + (let-404 [field (Field id)] (write-check field) (check (field-should-have-field-values? field) [400 "You can only update the mapped values of a Field whose 'special_type' is 'category'/'city'/'state'/'country' or whose 'base_type' is 'BooleanField'."]) diff --git a/src/metabase/api/meta/table.clj b/src/metabase/api/meta/table.clj index 552bac779f07c5f38133a6618c81def79d3059bd..7df3b7f232970f0548e74b3845db1bb6b54760cb 100644 --- a/src/metabase/api/meta/table.clj +++ b/src/metabase/api/meta/table.clj @@ -30,7 +30,7 @@ (defendpoint GET "/:id" "Get `Table` with ID." [id] - (->404 (sel :one Table :id id) + (->404 (Table id) read-check (hydrate :db :pk_field))) @@ -43,7 +43,7 @@ :entity_name entity_name :entity_type entity_type :description description)) - (sel :one Table :id id)) + (Table id)) (defendpoint GET "/:id/fields" "Get all `Fields` for `Table` with ID." @@ -60,7 +60,7 @@ will any of its corresponding values be returned. (This option is provided for use in the Admin Edit Metadata page)." [id include_sensitive_fields] {include_sensitive_fields String->Boolean} - (->404 (sel :one Table :id id) + (->404 (Table id) read-check (hydrate :db [:fields :target] :field_values) (update-in [:fields] (if include_sensitive_fields @@ -82,7 +82,7 @@ (defendpoint POST "/:id/sync" "Re-sync the metadata for this `Table`." [id] - (let-404 [table (sel :one Table :id id)] + (let-404 [table (Table id)] (write-check table) ;; run the task asynchronously (future (driver/sync-table! table))) @@ -107,7 +107,4 @@ new_order)) {:result "success"})) -;; TODO - GET /:id/segments -;; TODO - POST /:id/segments - (define-routes) diff --git a/src/metabase/api/notify.clj b/src/metabase/api/notify.clj index f155321920c087bac0a85f473b493fc560bbdb5f..c05576c27380f04658b43445a021761f470b2bbf 100644 --- a/src/metabase/api/notify.clj +++ b/src/metabase/api/notify.clj @@ -12,7 +12,7 @@ "Notification about a potential schema change to one of our `Databases`. Caller can optionally specify a `:table_id` or `:table_name` in the body to limit updates to a single `Table`." [id :as {{:keys [table_id table_name] :as body} :body}] - (let-404 [database (sel :one Database :id id)] + (let-404 [database (Database id)] (cond table_id (when-let [table (sel :one Table :db_id id :id (int table_id))] (future (driver/sync-table! table))) diff --git a/src/metabase/api/routes.clj b/src/metabase/api/routes.clj index 470f277d07800456920f616773fc04cbdb1a2c67..51413b17f1525c2b848db9df4e414cac2fad0173 100644 --- a/src/metabase/api/routes.clj +++ b/src/metabase/api/routes.clj @@ -15,18 +15,13 @@ [table :as table]) [metabase.middleware.auth :as auth])) -(defn- +apikey +(def ^:private +apikey "Wrap API-ROUTES so they may only be accessed with proper apikey credentials." - [api-routes] - (-> api-routes - auth/enforce-apikey)) + auth/enforce-api-key) -(defn- +auth +(def ^:private +auth "Wrap API-ROUTES so they may only be accessed with proper authentiaction credentials." - [api-routes] - (-> api-routes - auth/bind-current-user - auth/enforce-authentication)) + auth/enforce-authentication) (defroutes routes (context "/card" [] (+auth card/routes)) diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj index b6f3e2674a6dd802c1ec878d98f66d9505552eba..21eaa3fc21f1713006bd5a89f0bde3eb3511fc86 100644 --- a/src/metabase/api/user.clj +++ b/src/metabase/api/user.clj @@ -41,9 +41,9 @@ :last_name last_name :is_active true :is_superuser false) - (sel :one User :id (:id existing-user))) + (User (:id existing-user))) ;; account already exists and is active, so do nothing and just return the account - :else (sel :one User :id (:id existing-user))) + :else (User (:id existing-user))) (hydrate :user :organization)))) @@ -76,7 +76,7 @@ :is_superuser (if (:is_superuser @*current-user*) is_superuser nil))) - (sel :one User :id id)) + (User id)) (defendpoint PUT "/:id/password" @@ -88,7 +88,7 @@ (let-404 [user (sel :one [User :password_salt :password] :id id :is_active true)] (checkp (creds/bcrypt-verify (str (:password_salt user) old_password) (:password user)) "old_password" "Invalid password")) (set-user-password id password) - (sel :one User :id id)) + (User id)) (defendpoint DELETE "/:id" diff --git a/src/metabase/core.clj b/src/metabase/core.clj index 2e269bcd1f7b72c9dbc4d0f54eb4fc3e76748101..5584404dcffb9160459d590bf1c6aedc76bcd459 100644 --- a/src/metabase/core.clj +++ b/src/metabase/core.clj @@ -1,19 +1,9 @@ +;; -*- comment-column: 35; -*- (ns metabase.core (:gen-class) (:require [clojure.tools.logging :as log] [clojure.java.browse :refer [browse-url]] [colorize.core :as color] - [medley.core :as medley] - [metabase.config :as config] - [metabase.db :as db] - (metabase.middleware [auth :as auth] - [log-api-call :refer :all] - [format :refer :all]) - [metabase.models.setting :refer [defsetting]] - [metabase.models.user :refer [User]] - [metabase.routes :as routes] - [metabase.setup :as setup] - [metabase.task :as task] [ring.adapter.jetty :as ring-jetty] (ring.middleware [cookies :refer [wrap-cookies]] [gzip :refer [wrap-gzip]] @@ -21,7 +11,18 @@ wrap-json-body]] [keyword-params :refer [wrap-keyword-params]] [params :refer [wrap-params]] - [session :refer [wrap-session]]))) + [session :refer [wrap-session]]) + [medley.core :as medley] + (metabase [config :as config] + [db :as db] + [routes :as routes] + [setup :as setup] + [task :as task]) + (metabase.middleware [auth :as auth] + [log-api-call :refer :all] + [format :refer :all]) + (metabase.models [setting :refer [defsetting]] + [user :refer [User]]))) ;; ## CONFIG @@ -32,17 +33,19 @@ "The primary entry point to the HTTP server" (-> routes/routes (log-api-call :request :response) - format-response ; [METABASE] Do formatting before converting to JSON so serializer doesn't barf - (wrap-json-body ; extracts json POST body and makes it avaliable on request + format-response ; [METABASE] Do formatting before converting to JSON so serializer doesn't barf + (wrap-json-body ; extracts json POST body and makes it avaliable on request {:keywords? true}) - wrap-json-response ; middleware to automatically serialize suitable objects as JSON in responses - wrap-keyword-params ; converts string keys in :params to keyword keys - wrap-params ; parses GET and POST params as :query-params/:form-params and both as :params - auth/wrap-apikey ; looks for a Metabase API Key on the request and assocs as :metabase-apikey - auth/wrap-sessionid ; looks for a Metabase sessionid and assocs as :metabase-sessionid - wrap-cookies ; Parses cookies in the request map and assocs as :cookies - wrap-session ; reads in current HTTP session and sets :session/key - wrap-gzip)) ; GZIP response if client can handle it + wrap-json-response ; middleware to automatically serialize suitable objects as JSON in responses + wrap-keyword-params ; converts string keys in :params to keyword keys + wrap-params ; parses GET and POST params as :query-params/:form-params and both as :params + auth/bind-current-user ; Binds *current-user* and *current-user-id* if :metabase-user-id is non-nil + auth/wrap-current-user-id ; looks for :metabase-session-id and sets :metabase-user-id if Session ID is valid + auth/wrap-api-key ; looks for a Metabase API Key on the request and assocs as :metabase-api-key + auth/wrap-session-id ; looks for a Metabase Session ID and assoc as :metabase-session-id + wrap-cookies ; Parses cookies in the request map and assocs as :cookies + wrap-session ; reads in current HTTP session and sets :session/key + wrap-gzip)) ; GZIP response if client can handle it (defn- -init-create-setup-token "Create and set a new setup token, and open the setup URL on the user's system." @@ -57,10 +60,7 @@ setup-token)] (log/info (color/green "Please use the following url to setup your Metabase installation:\n\n" setup-url - "\n\n")) - ;; Attempt to browse URL on user's system; this will just fail silently if we can't do it - ;(browse-url setup-url) - )) + "\n\n")))) (defn init diff --git a/src/metabase/db.clj b/src/metabase/db.clj index 4b2016f04e1ec768cb79cee152411d1bbd4c9e1e..1543ca4a64fd560dc30fb40d578a6747cc1ce451 100644 --- a/src/metabase/db.clj +++ b/src/metabase/db.clj @@ -2,17 +2,17 @@ "Korma database definition and helper functions for interacting with the database." (:require [clojure.java.jdbc :as jdbc] [clojure.tools.logging :as log] - [clojure.string :as str] + (clojure [set :as set] + [string :as str]) [environ.core :refer [env]] - (korma [core :refer :all] - [db :refer :all]) + (korma [core :as k] + [db :as kdb]) [medley.core :as m] [metabase.config :as config] - [metabase.db.internal :refer :all :as i] - [metabase.util :as u])) - - -(declare post-select) + [metabase.db.internal :as i] + [metabase.models.interface :as models] + [metabase.util :as u]) + (:import com.metabase.corvus.migrations.LiquibaseMigrations)) ;; ## DB FILE, JDBC/KORMA DEFINITONS @@ -33,30 +33,30 @@ "Configure connection details for JDBC." [] (case (config/config-kw :mb-db-type) - :h2 {:subprotocol "h2" - :classname "org.h2.Driver" - :subname (db-file)} + :h2 {:subprotocol "h2" + :classname "org.h2.Driver" + :subname (db-file)} :postgres {:subprotocol "postgresql" - :classname "org.postgresql.Driver" - :subname (str "//" (config/config-str :mb-db-host) - ":" (config/config-str :mb-db-port) - "/" (config/config-str :mb-db-dbname)) - :user (config/config-str :mb-db-user) - :password (config/config-str :mb-db-pass)})) + :classname "org.postgresql.Driver" + :subname (str "//" (config/config-str :mb-db-host) + ":" (config/config-str :mb-db-port) + "/" (config/config-str :mb-db-dbname)) + :user (config/config-str :mb-db-user) + :password (config/config-str :mb-db-pass)})) (defn setup-korma-db "Configure connection details for Korma." [] (case (config/config-kw :mb-db-type) - :h2 (h2 {:db (db-file) - :naming {:keys str/lower-case - :fields str/upper-case}}) - :postgres (postgres {:db (config/config-str :mb-db-dbname) - :port (config/config-int :mb-db-port) - :user (config/config-str :mb-db-user) - :password (config/config-str :mb-db-pass) - :host (config/config-str :mb-db-host)}))) + :h2 (kdb/h2 {:db (db-file) + :naming {:keys str/lower-case + :fields str/upper-case}}) + :postgres (kdb/postgres {:db (config/config-str :mb-db-dbname) + :port (config/config-int :mb-db-port) + :user (config/config-str :mb-db-user) + :password (config/config-str :mb-db-pass) + :host (config/config-str :mb-db-host)}))) ;; ## CONNECTION @@ -81,9 +81,9 @@ [jdbc-db direction] (let [conn (jdbc/get-connection jdbc-db)] (case direction - :up (com.metabase.corvus.migrations.LiquibaseMigrations/setupDatabase conn) - :down (com.metabase.corvus.migrations.LiquibaseMigrations/teardownDatabase conn) - :print (com.metabase.corvus.migrations.LiquibaseMigrations/genSqlDatabase conn)))) + :up (LiquibaseMigrations/setupDatabase conn) + :down (LiquibaseMigrations/teardownDatabase conn) + :print (LiquibaseMigrations/genSqlDatabase conn)))) ;; ## SETUP-DB @@ -126,94 +126,17 @@ (log/info "Database Migrations Current ... CHECK") ;; Establish our 'default' Korma DB Connection - (default-connection (create-db korma-db)))) + (kdb/default-connection (kdb/create-db korma-db)))) (defn setup-db-if-needed [& args] (when-not @setup-db-has-been-called? (apply setup-db args))) -;; # UTILITY FUNCTIONS - -;; ## CAST-COLUMNS - -;; TODO - Doesn't Korma have similar `transformations` functionality? Investigate. - -(def ^:const ^:private type-fns - "A map of column type keywords to the functions that should be used to \"cast\" - them when going `:in` or `:out` of the database." - {:json {:in i/write-json - :out i/read-json} - :keyword {:in name - :out keyword}}) - -(defn types - "Tag columns in an entity definition with a type keyword. - This keyword will be used to automatically \"cast\" columns when they are present. - - ;; apply ((type-fns :json) :in) -- cheshire/generate-string -- to value of :details before inserting into DB - ;; apply ((type-fns :json) :out) -- read-json -- to value of :details when reading from DB - (defentity Database - (types {:details :json}))" - [entity types-map] - {:pre [(every? keyword? (keys types-map)) - (every? (partial contains? type-fns) (vals types-map))]} - (assoc entity ::types types-map)) - -(defn apply-type-fns - "Recursively apply a sequence of functions associated with COLUMN-TYPE-PAIRS to OBJ. - - COLUMN-TYPE-PAIRS should be the value of `(seq (::types korma-entity))`. - DIRECTION should be either `:in` or `:out`." - {:arglists '([direction column-type-pairs obj])} - [direction [[column column-type] & rest-pairs] obj] - (if-not column obj - (recur direction rest-pairs (if-not (column obj) obj - (update-in obj [column] (-> type-fns column-type direction)))))) - -;; TODO - It would be good to allow custom types by just inserting the `{:in fn :out fn}` inline with the -;; entity definition - -;; TODO - hydration-keys should be an entity function for the sake of prettiness - - -;; ## TIMESTAMPED - -(defn timestamped - "Mark ENTITY as having `:created_at` *and* `:updated_at` fields. - - (defentity Card - timestamped) - - * When a new object is created via `ins`, values for both fields will be generated. - * When an object is updated via `upd`, `:updated_at` will be updated." - [entity] - (assoc entity ::timestamped true)) - +;; # ---------------------------------------- UTILITY FUNCTIONS ---------------------------------------- ;; ## UPD -(defmulti pre-update - "Multimethod that is called by `upd` before DB operations happen. - A good place to set updated values for fields like `updated_at`, or serialize maps into JSON." - (fn [entity _] entity)) - -(defmethod pre-update :default [_ obj] - obj) ; default impl does no modifications to OBJ - -(defmulti post-update - "Multimethod that is called by `upd` after a SQL `UPDATE` *succeeds*. - (This gets called with whatever the output of `pre-update` was). - - A good place to schedule asynchronous tasks, such as creating a `FieldValues` object for a `Field` - when it is marked with `special_type` `:category`. - - The output of this function is ignored." - (fn [entity _] entity)) - -(defmethod post-update :default [_ _] ; default impl does nothing and returns nil - nil) - (defn upd "Wrapper around `korma.core/update` that updates a single row by its id value and automatically passes &rest KWARGS to `korma.core/set-fields`. @@ -224,15 +147,13 @@ [entity entity-id & {:as kwargs}] {:pre [(integer? entity-id)]} (let [obj (->> (assoc kwargs :id entity-id) - (pre-update entity) - (#(dissoc % :id)) - (apply-type-fns :in (seq (::types entity)))) - obj (cond-> obj - (::timestamped entity) (assoc :updated_at (u/new-sql-timestamp))) - result (-> (update entity (set-fields obj) (where {:id entity-id})) + (models/pre-update entity) + (models/internal-pre-update entity) + (#(dissoc % :id))) + result (-> (k/update entity (k/set-fields obj) (k/where {:id entity-id})) (> 0))] (when result - (post-update entity (assoc obj :id entity-id))) + (models/post-update entity (assoc obj :id entity-id))) result)) (defn upd-non-nil-keys @@ -248,28 +169,35 @@ "Wrapper around `korma.core/delete` that makes it easier to delete a row given a single PK value. Returns a `204 (No Content)` response dictionary." [entity & {:as kwargs}] - (delete entity (where kwargs)) + (k/delete entity (k/where kwargs)) {:status 204 :body nil}) ;; ## SEL -(defmulti post-select - "Called on the results from a call to `sel`. Default implementation doesn't do anything, but - you can provide custom implementations to do things like add hydrateable keys or remove sensitive fields." - (fn [entity _] entity)) - -;; Default implementation of post-select -(defmethod post-select :default [_ result] - result) - -(defmulti default-fields - "The default fields that should be used for ENTITY by calls to `sel` if none are specified." - identity) - -(defmethod default-fields :default [_] - nil) ; by default return nil, which we'll take to mean "everything" +(comment + :id->field `(let [[entity# field#] ~entity] + (->> (sel :many :fields [entity# field# :id] ~@forms) + (map (fn [{id# :id field-val# field#}] + {id# field-val#})) + (into {}))) + :field->id `(let [[entity# field#] ~entity] + (->> (sel :many :fields [entity# field# :id] ~@forms) + (map (fn [{id# :id field-val# field#}] + {field-val# id#})) + (into {}))) + :field->field `(let [[entity# field1# field2#] ~entity] + (->> (sel :many entity# ~@forms) + (map (fn [obj#] + {(field1# obj#) (field2# obj#)})) + (into {}))) + :field->obj `(let [[entity# field#] ~entity] + (->> (sel :many entity# ~@forms) + (map (fn [obj#] + {(field# obj#) obj#})) + (into {}))) + ) (defmacro sel "Wrapper for korma `select` that calls `post-select` on results and provides a few other conveniences. @@ -331,91 +259,19 @@ (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))" - {:arglists '([one-or-many option? entity & forms])} - [one-or-many & args] - {:pre [(contains? #{:one :many} one-or-many)]} - (if (= one-or-many :one) - `(first (sel :many ~@args (limit 1))) - (let [[option [entity & forms]] (u/optional keyword? args)] - (case option - :field `(let [[entity# field#] ~entity] - (map field# - (sel :many [entity# field#] ~@forms))) - :id `(sel :many :field [~entity :id] ~@forms) - :id->fields `(->> (sel :many :fields [~@entity :id] ~@forms) - (map (fn [{id# :id :as obj#}] - {id# obj#})) - (into {})) - :id->field `(let [[entity# field#] ~entity] - (->> (sel :many :fields [entity# field# :id] ~@forms) - (map (fn [{id# :id field-val# field#}] - {id# field-val#})) - (into {}))) - :field->id `(let [[entity# field#] ~entity] - (->> (sel :many :fields [entity# field# :id] ~@forms) - (map (fn [{id# :id field-val# field#}] - {field-val# id#})) - (into {}))) - :field->field `(let [[entity# field1# field2#] ~entity] - (->> (sel :many entity# ~@forms) - (map (fn [obj#] - {(field1# obj#) (field2# obj#)})) - (into {}))) - :field->obj `(let [[entity# field#] ~entity] - (->> (sel :many entity# ~@forms) - (map (fn [obj#] - {(field# obj#) obj#})) - (into {}))) - :fields `(let [[~'_ & fields# :as entity#] ~entity] - (map #(select-keys % fields#) - (sel :many entity# ~@forms))) - nil `(-sel-select ~entity ~@forms))))) - -(defmacro -sel-select - "Internal macro used by `sel` (don't call this directly). - Generates the korma `select` form." - [entity & forms] - (let [forms (sel-apply-kwargs forms)] ; convert kwargs like `:id 1` to korma `where` clause - `(let [[entity# field-keys#] (destructure-entity ~entity) ; pull out field-keys if passed entity vector like `[entity & field-keys]` - entity# (entity->korma entity#) ; entity## is the actual entity like `metabase.models.user/User` that we can dispatch on - entity-select-form# (-> entity# ; entity-select-form# is the tweaked version we'll pass to korma `select` - (assoc :fields (or field-keys# - (default-fields entity#))))] ; tell korma which fields to grab. If `field-keys` weren't passed in vector do lookup at runtime - (when (config/config-bool :mb-db-logging) - (log/debug "DB CALL: " (:name entity#) - (or (:fields entity-select-form#) "*") - ~@(mapv (fn [[form & args]] - `[~(name form) ~(apply str (interpose " " args))]) - forms))) - (->> (select entity-select-form# ~@forms) - (map (partial apply-type-fns :out (seq (::types entity#)))) - (map (partial post-select entity#)))))) ; map `post-select` over the results + {:arglists '([options? entity & forms])} + [& args] + (let [[option args] (u/optional keyword? args)] + `(~(if option + ;; if an option was specified, hand off to macro named metabase.db.internal/sel:OPTION + (symbol (format "metabase.db.internal/sel:%s" (name option))) + ;; otherwise just hand off to low-level sel* macro + 'metabase.db.internal/sel*) + ~@args))) ;; ## INS -(defmulti pre-insert - "Gets called by `ins` immediately before inserting a new object immediately before the korma `insert` call. - This provides an opportunity to do things like encode JSON or provide default values for certain fields. - - (pre-insert Query [_ query] - (let [defaults {:version 1}] - (merge defaults query))) ; set some default values" - (fn [entity _] entity)) - -(defmethod pre-insert :default [_ obj] - obj) ; default impl returns object as is - -(defmulti post-insert - "Gets called by `ins` after an object is inserted into the DB. (This object is fetched via `sel`). - A good place to do asynchronous tasks such as creating related objects. - Implementations should return the newly created object." - (fn [entity _] entity)) - -;; Default implementation returns object as-is -(defmethod post-insert :default [_ obj] - obj) - (defn ins "Wrapper around `korma.core/insert` that renames the `:scope_identity()` keyword in output to `:id` and automatically passes &rest KWARGS to `korma.core/values`. @@ -423,15 +279,11 @@ Returns newly created object by calling `sel`." [entity & {:as kwargs}] (let [vals (->> kwargs - (pre-insert entity) - (apply-type-fns :in (seq (::types entity)))) - vals (cond-> vals - (::timestamped entity) (assoc :created_at (u/new-sql-timestamp) - :updated_at (u/new-sql-timestamp))) - {:keys [id]} (-> (insert entity (values vals)) - (clojure.set/rename-keys {(keyword "scope_identity()") :id}))] - (->> (sel :one entity :id id) - (post-insert entity)))) + (models/pre-insert entity) + (models/internal-pre-insert entity)) + {:keys [id]} (-> (k/insert entity (k/values vals)) + (set/rename-keys {(keyword "scope_identity()") :id}))] + (models/post-insert entity (entity id)))) ;; ## EXISTS? @@ -441,39 +293,25 @@ (exists? User :id 100)" [entity & {:as kwargs}] - `(not (empty? (select (entity->korma ~entity) - (fields [:id]) - ~@(when (seq kwargs) - `[(where ~kwargs)]) - (limit 1))))) + `(boolean (seq (k/select (i/entity->korma ~entity) + (k/fields [:id]) + (k/where ~(if (seq kwargs) kwargs {})) + (k/limit 1))))) ;; ## CASADE-DELETE -(defmulti pre-cascade-delete - "Called by `cascade-delete` for each matching object that is about to be deleted. - Implementations should delete any objects related to this object by recursively - calling `cascade-delete`. - - (defmethod pre-cascade-delete Database [_ {database-id :id :as database}] - (cascade-delete Card :database_id database-id) - ...)" - (fn [entity _] - entity)) - -(defmethod pre-cascade-delete :default [_ instance] - instance) +(defn -cascade-delete [entity f] + (let [entity (i/entity->korma entity) + results (i/sel-exec entity f)] + (dorun (for [obj results] + (do (models/pre-cascade-delete entity obj) + (del entity :id (:id obj)))))) + {:status 204, :body nil}) -;; TODO - does this *really* need to be a macro? (defmacro cascade-delete "Do a cascading delete of object(s). For each matching object, the `pre-cascade-delete` multimethod is called, which should delete any objects related the object about to be deleted. Like `del`, this returns a 204/nil reponse so it can be used directly in an API endpoint." [entity & kwargs] - `(let [entity# (entity->korma ~entity) - instances# (sel :many entity# ~@kwargs)] - (dorun (map (fn [instance#] - (pre-cascade-delete entity# instance#) - (del entity# :id (:id instance#))) - instances#)) - {:status 204, :body nil})) + `(-cascade-delete ~entity (i/sel-fn ~@kwargs))) diff --git a/src/metabase/db/internal.clj b/src/metabase/db/internal.clj index d4f4bfe440f3527f5721cba9e0120ec6a1182895..2758e3d906c9847dbe7b436b7b7a5afac68bdd8b 100644 --- a/src/metabase/db/internal.clj +++ b/src/metabase/db/internal.clj @@ -1,8 +1,11 @@ (ns metabase.db.internal "Internal functions and macros used by the public-facing functions in `metabase.db`." - (:require [clojure.walk :as walk] - [cheshire.core :as cheshire] - [korma.core :refer [where]] + (:require [clojure.string :as s] + [clojure.tools.logging :as log] + [clojure.walk :as walk] + [korma.core :refer [where], :as k] + [metabase.config :as config] + [metabase.models.interface :as models] [metabase.util :as u])) (declare entity->korma) @@ -42,7 +45,7 @@ * Symbols like `'metabase.models.user/User` are handled the same way as strings." (memoize (fn -entity->korma [entity] - {:post [(= (type %) :korma.core/Entity)]} + {:post [(:metabase.models.interface/entity %)]} (cond (vector? entity) (-entity->korma (first entity)) (string? entity) (-entity->korma (symbol entity)) (symbol? entity) (try (eval entity) @@ -57,22 +60,105 @@ :else entity)))) -;; ## READ-JSON +;;; ## ---------------------------------------- SEL 2.0 FUNCTIONS ---------------------------------------- -(defn- read-json-str-or-clob - "If JSON-STRING is a JDBC Clob, convert to a String. Then call `json/read-str`." - [json-str] - (some-> (u/jdbc-clob->str json-str) - cheshire/parse-string)) +;;; Low-level sel implementation -(defn read-json - "Read JSON-STRING (or JDBC Clob) as JSON and keywordize keys." - [json-string] - (->> (read-json-str-or-clob json-string) - walk/keywordize-keys)) +(defmacro sel-fn [& forms] + (let [forms (sel-apply-kwargs forms) + entity-placeholder (gensym "ENTITY--")] + (loop [query `(k/select* ~entity-placeholder), [[f & args] & more] forms] + (cond + f (recur `(~f ~query ~@args) more) + (seq more) (recur query more) + :else `[(fn [~entity-placeholder] + ~query) ~(str query)])))) -(defn write-json - "If OBJ is not already a string, encode it as JSON." - [obj] - (if (string? obj) obj - (cheshire/generate-string obj))) +(defn sel-exec [entity [select-fn log-str]] + (let [[entity field-keys] (destructure-entity entity) + entity (entity->korma entity) + entity+fields (assoc entity :fields (or field-keys + (:metabase.models.interface/default-fields entity)))] + ;; Log if applicable + (future + (when (config/config-bool :mb-db-logging) + (log/debug "DB CALL: " (:name entity) + (or (:fields entity+fields) "*") + (s/replace log-str #"korma.core/" "")))) + + (->> (k/exec (select-fn entity+fields)) + (map (partial models/internal-post-select entity)) + (map (partial models/post-select entity))))) + +(defmacro sel* [entity & forms] + `(sel-exec ~entity (sel-fn ~@forms))) + +;;; :field + +(defmacro sel:field [[entity field] & forms] + `(let [field# ~field] + (map field# (sel* [~entity field#] ~@forms)))) + +;;; :id + +(defmacro sel:id [entity & forms] + `(sel:field [~entity :id] ~@forms)) + +;;; :fields + +(defn sel:fields* [fields results] + (for [result results] + (select-keys result fields))) + +(defmacro sel:fields [[entity & fields] & forms] + `(let [fields# ~(vec fields)] + (sel:fields* (set fields#) (sel* `[~~entity ~@fields#] ~@forms)))) + +;;; :id->fields + +(defn sel:id->fields* [fields results] + (->> results + (map (u/rpartial select-keys fields)) + (zipmap (map :id results)))) + +(defmacro sel:id->fields [[entity & fields] & forms] + `(let [fields# ~(conj (set fields) :id)] + (sel:id->fields* fields# (sel* `[~~entity ~@fields#] ~@forms)))) + +;;; :field->field + +(defn sel:field->field* [f1 f2 results] + (into {} (for [result results] + {(f1 result) (f2 result)}))) + +(defmacro sel:field->field [[entity f1 f2] & forms] + `(let [f1# ~f1 + f2# ~f2] + (sel:field->field* f1# f2# (sel* [~entity f1# f2#] ~@forms)))) + +;;; : id->field + +(defmacro sel:id->field [[entity field] & forms] + `(sel:field->field [~entity :id ~field] ~@forms)) + +;;; :field->id + +(defmacro sel:field->id [[entity field] & forms] + `(sel:field->field [~entity ~field :id] ~@forms)) + +;;; :field->obj + +(defn sel:field->obj* [field results] + (into {} (for [result results] + {(field result) result}))) + +(defmacro sel:field->obj [[entity field] & forms] + `(sel:field->obj* ~field (sel* ~entity ~@forms))) + +;;; :one & :many + +(defmacro sel:one [& args] + `(first (metabase.db/sel ~@args (k/limit 1)))) + +(defmacro sel:many [& args] + `(metabase.db/sel ~@args)) diff --git a/src/metabase/middleware/auth.clj b/src/metabase/middleware/auth.clj index 080a10fc3eecf837cfb84403c0a227f89dc8daf2..3e99d6eb9cb4ec201e44fc5bca2b46d70ba30356 100644 --- a/src/metabase/middleware/auth.clj +++ b/src/metabase/middleware/auth.clj @@ -1,6 +1,6 @@ (ns metabase.middleware.auth "Middleware for dealing with authentication and session management." - (:require [korma.core :as korma] + (:require [korma.core :as k] [metabase.config :as config] [metabase.db :refer [sel]] [metabase.api.common :refer [*current-user* *current-user-id*]] @@ -8,16 +8,16 @@ [user :refer [User current-user-fields]]))) -(def metabase-session-cookie "metabase.SESSION_ID") -(def metabase-session-header "x-metabase-session") -(def metabase-apikey-header "x-metabase-apikey") +(def ^:const metabase-session-cookie "metabase.SESSION_ID") +(def ^:const metabase-session-header "x-metabase-session") +(def ^:const metabase-api-key-header "x-metabase-apikey") -(def response-unauthentic {:status 401 :body "Unauthenticated"}) -(def response-forbidden {:status 403 :body "Forbidden"}) +(def ^:const response-unauthentic {:status 401 :body "Unauthenticated"}) +(def ^:const response-forbidden {:status 403 :body "Forbidden"}) -(defn wrap-sessionid - "Middleware that sets the :metabase-sessionid keyword on the request if a session id can be found. +(defn wrap-session-id + "Middleware that sets the `:metabase-session-id` keyword on the request if a session id can be found. We first check the request :cookies for `metabase.SESSION_ID`, then if no cookie is found we look in the http headers for `X-METABASE-SESSION`. If neither is found then then no keyword is bound to the request." @@ -25,32 +25,34 @@ (fn [{:keys [cookies headers] :as request}] (if-let [session-id (or (get-in cookies [metabase-session-cookie :value]) (headers metabase-session-header))] ;; alternatively we could always associate the keyword and just let it be nil if there is no value - (handler (assoc request :metabase-sessionid session-id)) + (handler (assoc request :metabase-session-id session-id)) (handler request)))) +(defn wrap-current-user-id + "Add `:metabase-user-id` to the request if a valid session token was passed." + [handler] + (fn [{:keys [metabase-session-id] :as request}] + ;; TODO - what kind of validations can we do on the sessionid to make sure it's safe to handle? str? alphanumeric? + (handler (or (when metabase-session-id + (when-let [session (first (k/select Session + ;; NOTE: we join with the User table and ensure user.is_active = true + (k/with User (k/where {:is_active true})) + (k/fields :created_at :user_id) + (k/where {:id metabase-session-id})))] + (let [session-age-ms (- (System/currentTimeMillis) (.getTime ^java.util.Date (get session :created_at (java.util.Date. 0))))] + ;; If the session exists and is not expired (max-session-age > session-age) then validation is good + (when (and session (> (config/config-int :max-session-age) (quot session-age-ms 60000))) + (assoc request :metabase-user-id (:user_id session)))))) + request)))) -(defn enforce-authentication - "Middleware that enforces authentication of the client, cancelling the request processing if auth fails. - - Authentication is determined by validating the :metabase-sessionid on the request against the db session list. - If the session is valid then we associate a :metabase-userid on the request and carry on, but if the validation - fails then we return an HTTP 401 response indicating that the client is not authentic. - NOTE: we are purposely not associating the full current user object here so that we can be modular." +(defn enforce-authentication + "Middleware that returns a 401 response if REQUEST has no associated `:metabase-user-id`." [handler] - (fn [{:keys [metabase-sessionid] :as request}] - ;; TODO - what kind of validations can we do on the sessionid to make sure it's safe to handle? str? alphanumeric? - (let [session (first (korma/select Session - ;; NOTE: we join with the User table and ensure user.is_active = true - (korma/with User (korma/where {:is_active true})) - (korma/fields :created_at :user_id) - (korma/where {:id metabase-sessionid}))) - session-age-ms (- (System/currentTimeMillis) (.getTime ^java.util.Date (get session :created_at (java.util.Date. 0))))] - ;; If the session exists and is not expired (max-session-age > session-age) then validation is good - (if (and session (> (config/config-int :max-session-age) (quot session-age-ms 60000))) - (handler (assoc request :metabase-userid (:user_id session))) - ;; default response is 401 - response-unauthentic)))) + (fn [{:keys [metabase-user-id] :as request}] + (if metabase-user-id + (handler request) + response-unauthentic))) (defmacro sel-current-user [current-user-id] @@ -65,35 +67,35 @@ *current-user* delay that returns current user (or nil) from DB" [handler] (fn [request] - (let [current-user-id (:metabase-userid request)] + (if-let [current-user-id (:metabase-user-id request)] (binding [*current-user-id* current-user-id - *current-user* (if-not current-user-id (atom nil) - (delay (sel-current-user current-user-id)))] - (handler request))))) + *current-user* (delay (sel-current-user current-user-id))] + (handler request)) + (handler request)))) -(defn wrap-apikey - "Middleware that sets the :metabase-apikey keyword on the request if a valid API Key can be found. +(defn wrap-api-key + "Middleware that sets the :metabase-api-key keyword on the request if a valid API Key can be found. We check the request headers for `X-METABASE-APIKEY` and if it's not found then then no keyword is bound to the request." [handler] (fn [{:keys [headers] :as request}] - (if-let [api-key (headers metabase-apikey-header)] - (handler (assoc request :metabase-apikey api-key)) + (if-let [api-key (headers metabase-api-key-header)] + (handler (assoc request :metabase-api-key api-key)) (handler request)))) -(defn enforce-apikey +(defn enforce-api-key "Middleware that enforces validation of the client via API Key, cancelling the request processing if the check fails. - Validation is handled by first checking for the presence of the :metabase-apikey on the request. If the api key + Validation is handled by first checking for the presence of the :metabase-api-key on the request. If the api key is available then we validate it by checking it against the configured :mb-api-key value set in our global config. - If the request :metabase-apikey matches the configured :mb-api-key value then the request continues, otherwise we + If the request :metabase-api-key matches the configured :mb-api-key value then the request continues, otherwise we reject the request and return a 403 Forbidden response." [handler] - (fn [{:keys [metabase-apikey] :as request}] - (if (= (config/config-str :mb-api-key) metabase-apikey) + (fn [{:keys [metabase-api-key] :as request}] + (if (= (config/config-str :mb-api-key) metabase-api-key) (handler request) ;; default response is 403 response-forbidden))) diff --git a/src/metabase/middleware/format.clj b/src/metabase/middleware/format.clj index f6a372d76a1db96f4aacf0508348eada9c6d7a57..673c11ad00a5e33262fe607b4811a664eb5c9799 100644 --- a/src/metabase/middleware/format.clj +++ b/src/metabase/middleware/format.clj @@ -3,6 +3,7 @@ (cheshire factory [generate :refer [add-encoder encode-str]]) [medley.core :refer [filter-vals map-vals]] + [metabase.models.interface :refer [api-serialize]] [metabase.util :as util])) (declare -format-response) @@ -45,11 +46,14 @@ [m] (filter-vals #(not (or (delay? %) (fn? %))) - m)) + ;; Convert typed maps such as metabase.models.database/DatabaseInstance to plain maps because empty, which is used internally by filter-vals, + ;; will fail otherwise + (into {} m))) (defn- -format-response [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 + (map? obj) (->> (api-serialize obj) + remove-fns-and-delays + (map-vals -format-response)) ; recurse over all vals in the map + (coll? obj) (map -format-response obj) ; recurse over all items in the collection :else obj)) diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 97ab1ff8de6d537f9511c5b070e670b647803224..6c7cab212ede0c778340b28342a7199c99e9f829 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -1,8 +1,8 @@ (ns metabase.models.card - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.api.common :refer [*current-user-id*]] [metabase.db :refer :all] - (metabase.models [common :refer :all] + (metabase.models [interface :refer :all] [user :refer [User]]))) (def ^:const display-types @@ -18,19 +18,25 @@ :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) - (types {:dataset_query :json - :display :keyword - :visualization_settings :json}) - timestamped - (assoc :hydration-keys #{:card})) - -(defmethod post-select Card [_ {:keys [creator_id] :as card}] - (-> (assoc card - :creator (delay (sel :one User :id creator_id))) - assoc-permissions-sets)) - -(defmethod pre-cascade-delete Card [_ {:keys [id]}] - (cascade-delete 'metabase.models.dashboard-card/DashboardCard :card_id id) - (cascade-delete 'metabase.models.card-favorite/CardFavorite :card_id id)) + [(table :report_card) + (hydration-keys card) + (types :dataset_query :json, :display :keyword, :visualization_settings :json) + timestamped] + + (post-select [_ {:keys [creator_id] :as card}] + (map->CardInstance (assoc card :creator (delay (User 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) diff --git a/src/metabase/models/card_favorite.clj b/src/metabase/models/card_favorite.clj index 42bd82ad488789cd6de6d11308d1e29b1aa899ec..5a38052192143f40234a96453ccd904191937a5b 100644 --- a/src/metabase/models/card_favorite.clj +++ b/src/metabase/models/card_favorite.clj @@ -1,14 +1,15 @@ (ns metabase.models.card-favorite - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] (metabase.models [card :refer [Card]] + [interface :refer :all] [user :refer [User]]))) (defentity CardFavorite - (table :report_cardfavorite) - timestamped) + [(table :report_cardfavorite) + timestamped] -(defmethod post-select CardFavorite [_ {:keys [card_id owner_id] :as card-favorite}] - (assoc card-favorite - :owner (delay (sel :one User :id owner_id)) - :card (delay (sel :one Card :id card_id)))) + (post-select [_ {:keys [card_id owner_id] :as card-favorite}] + (assoc card-favorite + :owner (delay (User owner_id)) + :card (delay (Card card_id))))) diff --git a/src/metabase/models/common.clj b/src/metabase/models/common.clj index 50a16eddf06e9590a45fc1e716bb47e35f2564ae..b9fcc6005abd8da945236d52b7aa28f06d03a1b6 100644 --- a/src/metabase/models/common.clj +++ b/src/metabase/models/common.clj @@ -3,7 +3,7 @@ [metabase.api.common :refer [*current-user* *current-user-id* check]] [metabase.util :as u])) -(def timezones +(def ^:const timezones ["GMT" "UTC" "US/Alaska" @@ -15,62 +15,16 @@ "US/Pacific" "America/Costa_Rica"]) -;;; ALLEN'S PERMISSIONS IMPLEMENTATION +(def ^:const perms-none 0) +(def ^:const perms-read 1) +(def ^:const perms-readwrite 2) -(def perms-none 0) -(def perms-read 1) -(def perms-readwrite 2) - -(def permissions +(def ^:const permissions [{:id perms-none :name "None"}, {:id perms-read :name "Read Only"}, {: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. diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index 4c769ca7628153a32ca3d1882c8374ecc830f880..bfc98e747951d914544cc4113842fdd80992fe5c 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -1,21 +1,32 @@ (ns metabase.models.dashboard - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] (metabase.models [common :refer :all] [dashboard-card :refer [DashboardCard]] + [interface :refer :all] [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) + [(table :report_dashboard) + timestamped] + + (post-select [_ {:keys [id creator_id description] :as dash}] + (-> dash + (assoc :creator (delay (User creator_id)) + :description (u/jdbc-clob->str description) + :ordered_cards (delay (sel :many DashboardCard :dashboard_id id (order :created_at :asc)))) + map->DashboardInstance)) -(defmethod post-select Dashboard [_ {:keys [id creator_id description] :as dash}] - (-> dash - (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)) + (pre-cascade-delete [_ {:keys [id]}] + (cascade-delete DashboardCard :dashboard_id id))) -(defmethod pre-cascade-delete Dashboard [_ {:keys [id]}] - (cascade-delete DashboardCard :dashboard_id id)) +(extend-ICanReadWrite DashboardEntity :read :public-perms, :write :public-perms) diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 8b336ec46b1d61e99accc7fbd14bdc6877d80c81..57fb7d21b5c26da252255a0d8212f957f734d49f 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -1,32 +1,22 @@ (ns metabase.models.dashboard-card - (:require [korma.core :refer :all] + (:require [clojure.set :as set] + [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] - (metabase.models [card :refer [Card]]))) + (metabase.models [card :refer [Card]] + [interface :refer :all]))) (defentity DashboardCard - (table :report_dashboardcard) - timestamped) + [(table :report_dashboardcard) + timestamped] -;; #### fields: -;; * `id` -;; * `created_at` -;; * `updated_at` -;; * `sizeX` -;; * `sizeY` -;; * `row` -;; * `col` -;; * `card_id` -;; * `dashboard_id` + (pre-insert [_ dashcard] + (let [defaults {:sizeX 2 + :sizeY 2}] + (merge defaults dashcard))) - -(defmethod post-select DashboardCard [_ {:keys [card_id dashboard_id] :as dashcard}] - (-> dashcard - (clojure.set/rename-keys {:sizex :sizeX ; mildly retarded: H2 columns are all uppercase, we're converting them - :sizey :sizeY}) ; to all downcase, and the Angular app expected mixed-case names here - (assoc :card (delay (sel :one Card :id card_id)) - :dashboard (delay (sel :one 'metabase.models.dashboard/Dashboard :id dashboard_id))))) - -(defmethod pre-insert DashboardCard [_ dashcard] - (let [defaults {:sizeX 2 - :sizeY 2}] - (merge defaults dashcard))) + (post-select [_ {:keys [card_id dashboard_id] :as dashcard}] + (-> dashcard + (set/rename-keys {:sizex :sizeX ; mildly retarded: H2 columns are all uppercase, we're converting them + :sizey :sizeY}) ; to all downcase, and the Angular app expected mixed-case names here + (assoc :card (delay (Card card_id)) + :dashboard (delay (sel :one 'metabase.models.dashboard/Dashboard :id dashboard_id)))))) diff --git a/src/metabase/models/dashboard_subscription.clj b/src/metabase/models/dashboard_subscription.clj deleted file mode 100644 index a791ae6a87e684cb0544bcfd6a27d1fb60497115..0000000000000000000000000000000000000000 --- a/src/metabase/models/dashboard_subscription.clj +++ /dev/null @@ -1,5 +0,0 @@ -(ns metabase.models.dashboard-subscription - (:require [korma.core :refer :all])) - -(defentity DashboardSubscription - (table :report_dashboardsubscription)) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index 680d57ed30f35331e376face7a5117f327791e33..1683ae7ed6024d269bb494cebbd57b9d031439af 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -1,45 +1,33 @@ (ns metabase.models.database - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.api.common :refer [*current-user*]] [metabase.db :refer :all] - [metabase.models.common :refer [assoc-permissions-sets]])) + [metabase.models.interface :refer :all])) +(defrecord DatabaseInstance [] + ;; preserve normal IFn behavior so things like ((sel :one Database) :id) work correctly + clojure.lang.IFn + (invoke [this k] + (get this k)) -(defentity Database - (table :metabase_database) - (types {:details :json - :engine :keyword}) - timestamped - (assoc :hydration-keys #{:database - :db})) + IModelInstanceApiSerialize + (api-serialize [this] + ;; If current user isn't an admin strip out DB details which may include things like password + (cond-> this + (not (:is_superuser @*current-user*)) (dissoc :details)))) -(defmethod post-select Database [_ db] - (assoc db - :can_read (delay true) - :can_write (delay (:is_superuser @*current-user*)))) +(extend-ICanReadWrite DatabaseInstance :read :always, :write :superuser) - {:created_at #inst "2015-06-30T19:51:45.294000000-00:00", - :engine :h2, - :id 3, - :details - {:db - "file:/Users/camsaul/metabase/target/Test_Database;AUTO_SERVER=TRUE;DB_CLOSE_DELAY=-1"}, - :updated_at #inst "2015-06-30T19:51:45.294000000-00:00", - :name "Test Database", - :organization_id nil, - :description nil} +(defentity Database + [(table :metabase_database) + (hydration-keys database db) + (types :details :json, :engine :keyword) + timestamped] -{:description nil, - :organization_id nil, - :name "Test Database", - :updated_at #inst "2015-06-30T19:51:45.294000000-00:00", - :details - {:db - "file:/Users/camsaul/metabase/target/Test_Database;AUTO_SERVER=TRUE;DB_CLOSE_DELAY=-1"}, - :id 3, - :engine "h2", - :created_at #inst "2015-06-30T19:51:45.294000000-00:00"} + (post-select [_ db] + (map->DatabaseInstance db)) + (pre-cascade-delete [_ {:keys [id] :as database}] + (cascade-delete 'metabase.models.table/Table :db_id id))) -(defmethod pre-cascade-delete Database [_ {:keys [id] :as database}] - (cascade-delete 'metabase.models.table/Table :db_id id)) +(extend-ICanReadWrite DatabaseEntity :read :always, :write :superuser) diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index ca0a44c8d20ebfaaef171d5d281af38ca9b08ae5..71272f40b162adcf6a66a22171eaec7a65f50174 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -1,14 +1,17 @@ (ns metabase.models.field - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.api.common :refer [check]] [metabase.db :refer :all] (metabase.models [common :as common] [database :refer [Database]] [field-values :refer [field-should-have-field-values? create-field-values create-field-values-if-needed]] + [foreign-key :refer [ForeignKey]] [hydrate :refer [hydrate]] - [foreign-key :refer [ForeignKey]]) + [interface :refer :all]) [metabase.util :as u])) +(declare field->fk-field) + (def ^:const special-types "Possible values for `Field` `:special_type`." #{:avatar @@ -71,15 +74,55 @@ :info ; Non-numerical value that is not meant to be used :sensitive}) ; A Fields that should *never* be shown *anywhere* +(defrecord FieldInstance [] + clojure.lang.IFn + (invoke [this k] + (get this k))) + +(extend-ICanReadWrite FieldInstance :read :always, :write :superuser) + + (defentity Field - (table :metabase_field) - timestamped - (types {:base_type :keyword - :field_type :keyword - :special_type :keyword}) - (assoc :hydration-keys #{:destination - :field - :origin})) + [(table :metabase_field) + (hydration-keys destination field origin) + (types :base_type :keyword, :field_type :keyword, :special_type :keyword) + timestamped] + + (pre-insert [_ field] + (let [defaults {:active true + :preview_display true + :field_type :info + :position 0}] + (merge defaults field))) + + (post-insert [_ field] + (when (field-should-have-field-values? field) + (future (create-field-values field))) + field) + + (post-update [this {:keys [id] :as field}] + ;; if base_type or special_type were affected then we should asynchronously create corresponding FieldValues objects if need be + (when (or (contains? field :base_type) + (contains? field :field_type) + (contains? field :special_type)) + (future (create-field-values-if-needed (sel :one [this :id :table_id :base_type :special_type :field_type] :id id))))) + + (post-select [_ {:keys [table_id] :as field}] + (map->FieldInstance + (u/assoc* field + :table (delay (sel :one 'metabase.models.table/Table :id table_id)) + :db (delay @(:db @(:table <>))) + :target (delay (field->fk-field field)) + :human_readable_name (when (name :field) + (delay (common/name->human-readable-name (:name field))))))) + + (pre-cascade-delete [_ {:keys [id]}] + (cascade-delete ForeignKey (where (or (= :origin_id id) + (= :destination_id id)))) + (cascade-delete 'metabase.models.field-values/FieldValues :field_id id))) + +(extend-ICanReadWrite FieldEntity :read :always, :write :superuser) + (defn field->fk-field "Attempts to follow a `ForeignKey` from the the given `Field` to a destination `Field`. @@ -88,38 +131,4 @@ [{:keys [id special_type] :as field}] (when (= :fk special_type) (let [dest-id (sel :one :field [ForeignKey :destination_id] :origin_id id)] - (sel :one Field :id dest-id)))) - -(defmethod post-select Field [_ {:keys [table_id] :as field}] - (u/assoc* field - :table (delay (sel :one 'metabase.models.table/Table :id table_id)) - :db (delay @(:db @(:table <>))) - :target (delay (field->fk-field field)) - :can_read (delay @(:can_read @(:table <>))) - :can_write (delay @(:can_write @(:table <>))) - :human_readable_name (when (name :field) - (delay (common/name->human-readable-name (:name field)))))) - -(defmethod pre-insert Field [_ field] - (let [defaults {:active true - :preview_display true - :field_type :info - :position 0}] - (merge defaults field))) - -(defmethod post-insert Field [_ field] - (when (field-should-have-field-values? field) - (future (create-field-values field))) - field) - -(defmethod post-update Field [_ {:keys [id] :as field}] - ;; if base_type or special_type were affected then we should asynchronously create corresponding FieldValues objects if need be - (when (or (contains? field :base_type) - (contains? field :field_type) - (contains? field :special_type)) - (future (create-field-values-if-needed (sel :one [Field :id :table_id :base_type :special_type :field_type] :id id))))) - -(defmethod pre-cascade-delete Field [_ {:keys [id]}] - (cascade-delete ForeignKey (where (or (= :origin_id id) - (= :destination_id id)))) - (cascade-delete 'metabase.models.field-values/FieldValues :field_id id)) + (Field dest-id)))) diff --git a/src/metabase/models/field_values.clj b/src/metabase/models/field_values.clj index 9d7ac98d4aab1e54bb4a3df0dc2afa61803207c3..e6c25f51ea83668e05a8ef0e07a98901d7dcdbe2 100644 --- a/src/metabase/models/field_values.clj +++ b/src/metabase/models/field_values.clj @@ -1,16 +1,19 @@ (ns metabase.models.field-values (:require [clojure.tools.logging :as log] - [korma.core :refer :all] - [metabase.db :refer :all] - [metabase.util :as u])) + [korma.core :refer :all, :exclude [defentity]] + (metabase [db :refer :all] + [util :as u]) + [metabase.models.interface :refer :all])) ;; ## Entity + DB Multimethods (defentity FieldValues - (table :metabase_fieldvalues) - timestamped - (types {:human_readable_values :json - :values :json})) + [(table :metabase_fieldvalues) + timestamped + (types :human_readable_values :json, :values :json)] + + (post-select [_ field-values] + (update-in field-values [:human_readable_values] #(or % {})))) ;; columns: ;; * :id @@ -20,10 +23,6 @@ ;; * :values (JSON-encoded array like ["table" "scalar" "pie"]) ;; * :human_readable_values (JSON-encoded map like {:table "Table" :scalar "Scalar"} -(defmethod post-select FieldValues [_ field-values] - (update-in field-values [:human_readable_values] #(or % {}))) ; return an empty map for :human_readable_values in cases where it is nil - - ;; ## `FieldValues` Helper Functions (defn field-should-have-field-values? diff --git a/src/metabase/models/foreign_key.clj b/src/metabase/models/foreign_key.clj index 9532345cd6bf5c77e7863d3d857846fc5d25a559..05b4ad25446fc001686e5831b80102b0e746e6a6 100644 --- a/src/metabase/models/foreign_key.clj +++ b/src/metabase/models/foreign_key.clj @@ -1,25 +1,25 @@ (ns metabase.models.foreign-key - (:require [korma.core :refer :all] - [metabase.db :refer :all])) + (:require [korma.core :refer :all, :exclude [defentity]] + [metabase.db :refer :all] + [metabase.models.interface :refer :all])) -(def relationships +(def ^:const relationships "Valid values for `ForeginKey.relationship`." #{:1t1 :Mt1 :MtM}) -(def relationship->name +(def ^:const relationship->name {:1t1 "One to One" :Mt1 "Many to One" :MtM "Many to Many"}) (defentity ForeignKey - (table :metabase_foreignkey) - timestamped - (types {:relationship :keyword})) + [(table :metabase_foreignkey) + (types :relationship :keyword) + timestamped] - -(defmethod post-select ForeignKey [_ {:keys [origin_id destination_id] :as fk}] - (assoc fk - :origin (delay (sel :one 'metabase.models.field/Field :id origin_id)) - :destination (delay (sel :one 'metabase.models.field/Field :id destination_id)))) + (post-select [_ {:keys [origin_id destination_id] :as fk}] + (assoc fk + :origin (delay (sel :one 'metabase.models.field/Field :id origin_id)) + :destination (delay (sel :one 'metabase.models.field/Field :id destination_id))))) diff --git a/src/metabase/models/hydrate.clj b/src/metabase/models/hydrate.clj index b5f81f4cc741af974601a5f22496f149cb6b8d75..740920171156a8f3073e3bea308a72c20c0c61c4 100644 --- a/src/metabase/models/hydrate.clj +++ b/src/metabase/models/hydrate.clj @@ -1,6 +1,7 @@ (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 @@ -25,16 +26,10 @@ **Batched Hydration** Hydration attempts to do a *batched hydration* where possible. - If the key being hydrated is defined as one of some entity's `:hydration-keys`, + If the key being hydrated is defined as one of some entity's `:metabase.models.interface/hydration-keys`, `hydrate` will do a batched `sel` if a corresponding key ending with `_id` is found in the objects being hydrated. - `defentity` threads the resulting map through its forms using `->`, so define - `:hydration-keys` with `assoc`: - - (defentity User - (assoc :hydration-keys #{:user})) - (hydrate [{:user_id 100}, {:user_id 101}] :user) Since `:user` is a hydration key for `User`, a single `sel` will used to @@ -131,14 +126,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? @@ -174,34 +177,6 @@ (assoc result dest-key obj)))) results)))) -;; #### Possible Improvements -;; TODO - It would be *nice* to extend this to work with one-to-many relationships. e.g. `Dashboard -> Cards` -;; -;; It could work like this: -;; -;; (defentity Card -;; (assoc :hydration-keys {:1t1 {:keys #{:card}} ; (hydrate obj :card) -> obj.card_id <-> Card.id -;; :1tM {:keys #{:cards} -;; :fks #{:table_id}}})) ; (hydrate table :cards) -> obj.id <-> Card.table_id -;; -;; (-> (sel :many Table ...) -;; (hydrate :cards)) -;; -;; 1. `:hydration-keys` can be reworked to differentiate between one-to-one hydrations and one-to-many hydrations -;; (not sure on the exact format yet) -;; -;; 2. one-to-many hydrations will additionally need to know what fields it has that can be used as Foreign Keys -;; - Could we reflect on the DB and add this info at runtime? -;; - Could we just use `belongs-to` / `has-one` / etc? (or an augmented version thereof) to specify foreign keys? -;; -;; 3. We can infer that `:table_id` is an FK to `Table` because `Table` has `:table` defined as a hydration key. -;; `:table <-> :table_id` -;; -;; 4. (This is the tricky part) -;; If we could somehow know that we are trying to hydrate `Tables`, we would know we could use `:id -> :table_id` -;; and could do a `(sel Card :table_id [in ids])` -;; - We could add a key like `:_type :Table` (?) to results so we know the type - ;; ### Helper Fns @@ -209,16 +184,15 @@ "Delay that returns map of `hydration-key` -> korma entity. e.g. `:user -> User`. - This is built pulling the `:hydration-keys` set from all korma entities." + This is built pulling the `::hydration-keys` set from all of our entities." (delay (->> (all-ns) (mapcat ns-publics) vals (map var-get) - (filter #(= (type %) :korma.core/Entity)) - (filter :hydration-keys) - (mapcat (fn [{:keys [hydration-keys] :as entity}] + (filter :metabase.models.interface/hydration-keys) + (mapcat (fn [{hydration-keys :metabase.models.interface/hydration-keys, :as entity}] (assert (and (set? hydration-keys) (every? keyword? hydration-keys)) - (str ":hydration-keys should be a set of keywords. In: " entity)) + (str "::hydration-keys should be a set of keywords. In: " entity)) (map (u/rpartial vector entity) hydration-keys))) (into {})))) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj new file mode 100644 index 0000000000000000000000000000000000000000..ac46ee92c838387dc30b58202125acc2ce34db6e --- /dev/null +++ b/src/metabase/models/interface.clj @@ -0,0 +1,234 @@ +(ns metabase.models.interface + (:require (clojure.tools [logging :as log] + [macro :refer [macrolet]]) + [clojure.walk :refer [macroexpand-all], :as walk] + [cheshire.core :as cheshire] + [korma.core :as k] + [medley.core :as m] + [metabase.config :as config] + [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 (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 :r read) + :can-write? (key->method :w write)}))) + + +;;; ## ---------------------------------------- ENTITIES ---------------------------------------- + +(defprotocol IEntity + (pre-insert [this instance] + "Gets called by `ins` immediately before inserting a new object immediately before the korma `insert` call. + This provides an opportunity to do things like encode JSON or provide default values for certain fields. + + (pre-insert [_ query] + (let [defaults {:version 1}] + (merge defaults query))) ; set some default values") + + (post-insert [this instance] + "Gets called by `ins` after an object is inserted into the DB. (This object is fetched via `sel`). + A good place to do asynchronous tasks such as creating related objects. + Implementations should return the newly created object.") + + (pre-update [this instance] + "Called by `upd` before DB operations happen. A good place to set updated values for fields like `updated_at`, or serialize maps into JSON.") + + (post-update [this instance] + "Called by `upd` after a SQL `UPDATE` *succeeds*. (This gets called with whatever the output of `pre-update` was). + + A good place to schedule asynchronous tasks, such as creating a `FieldValues` object for a `Field` + when it is marked with `special_type` `:category`. + + The output of this function is ignored.") + + (post-select [this instance] + "Called on the results from a call to `sel`. Default implementation doesn't do anything, but + you can provide custom implementations to do things like add hydrateable keys or remove sensitive fields.") + + (pre-cascade-delete [this instance] + "Called by `cascade-delete` for each matching object that is about to be deleted. + Implementations should delete any objects related to this object by recursively + calling `cascade-delete`. + + The output of this function is ignored. + + (pre-cascade-delete [_ {database-id :id :as database}] + (cascade-delete Card :database_id database-id) + ...)") + + (internal-pre-insert [this instance]) + (internal-pre-update [this instance]) + (internal-post-select [this instance])) + +(defn- identity-second [_ obj] obj) +(def ^:private constantly-nil (constantly nil)) + +(def ^:const ^:private default-entity-method-implementations + {:pre-insert #'identity-second + :post-insert #'identity-second + :pre-update #'identity-second + :post-update #'constantly-nil + :post-select #'identity-second + :pre-cascade-delete #'constantly-nil}) + +;; ## ---------------------------------------- READ-JSON ---------------------------------------- + +(defn- read-json-str-or-clob + "If JSON-STRING is a JDBC Clob, convert to a String. Then call `json/read-str`." + [json-str] + (some-> (u/jdbc-clob->str json-str) + cheshire/parse-string)) + +(defn- read-json + "Read JSON-STRING (or JDBC Clob) as JSON and keywordize keys." + [json-string] + (->> (read-json-str-or-clob json-string) + walk/keywordize-keys)) + +(defn- write-json + "If OBJ is not already a string, encode it as JSON." + [obj] + (if (string? obj) obj + (cheshire/generate-string obj))) + +(def ^:const ^:private type-fns + {:json {:in #'write-json + :out #'read-json} + :keyword {:in `name + :out `keyword}}) + +(defmacro apply-type-fns [obj-binding direction entity-map] + {:pre [(symbol? obj-binding) + (keyword? direction) + (map? entity-map)]} + (let [fns (m/map-vals #(direction (type-fns %)) (::types entity-map))] + (if-not (seq fns) obj-binding + `(cond-> ~obj-binding + ~@(mapcat (fn [[k f]] + [`(~k ~obj-binding) `(update-in [~k] ~f)]) + fns))))) + +(defn -invoke-entity + "Basically the same as `(sel :one Entity :id id)`." ; TODO - deduplicate with sel + [entity id] + (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))) + +(defn- update-created-at-updated-at [obj] + (let [ts (u/new-sql-timestamp)] + (assoc obj :created_at ts, :updated_at ts))) + +(defmacro macrolet-entity-map [entity & entity-forms] + `(macrolet [(~'default-fields [m# & fields#] `(assoc ~m# ::default-fields [~@(map keyword fields#)])) + (~'timestamped [m#] `(assoc ~m# ::timestamped true)) + (~'types [m# & {:as fields#}] `(assoc ~m# ::types ~fields#)) + (~'hydration-keys [m# & fields#] `(assoc ~m# ::hydration-keys #{~@(map keyword fields#)}))] + (-> (k/create-entity ~(name entity)) + ~@entity-forms))) + +(defmacro defentity + "Similar to korma `defentity`, but creates a new record type where you can specify protocol implementations." + [entity entity-forms & methods+specs] + {:pre [(vector? entity-forms)]} + (let [entity-symb (symbol (format "%sEntity" (name entity))) + internal-post-select-symb (symbol (format "internal-post-select-%s" (name entity))) + unevaled-entity-map (macroexpand-all `(macrolet-entity-map ~entity ~@entity-forms)) + entity-map (eval unevaled-entity-map) + [methods specs] (split-with list? methods+specs)] + `(do + (defrecord ~entity-symb [] + clojure.lang.IFn + (~'invoke [~'this ~'id] + (-invoke-entity ~'this ~'id)) + + ~@specs) + + (extend ~entity-symb + IEntity ~(merge default-entity-method-implementations + {:internal-pre-insert `(fn [~'_ obj#] + (-> obj# + (apply-type-fns :in ~entity-map) + ~@(when (::timestamped entity-map) + [update-created-at-updated-at]))) + :internal-pre-update `(fn [~'_ obj#] + (-> obj# + (apply-type-fns :in ~entity-map) + ~@(when (::timestamped entity-map) + [update-updated-at]))) + :internal-post-select `(fn [~'_ obj#] + (apply-type-fns obj# :out ~entity-map))} + (into {} + (for [[method-name & impl] methods] + {(keyword method-name) `(fn ~@impl)})))) + (def ~entity + (~(symbol (format "map->%sEntity" (name entity))) (assoc ~unevaled-entity-map ::entity true)))))) + +(defn metabase-entity? + "Is ENTITY a valid metabase model entity?" + [entity] + (::entity entity)) + + +;;; # ---------------------------------------- INSTANCE ---------------------------------------- + +(defprotocol IModelInstanceApiSerialize + (api-serialize [this] + "Called on all objects being written out by the API. Default implementations return THIS as-is, but models can provide + custom methods to strip sensitive data, from non-admins, etc.")) + +(extend Object + IModelInstanceApiSerialize {:api-serialize identity}) + +(extend nil + IModelInstanceApiSerialize {:api-serialize identity}) diff --git a/src/metabase/models/query_execution.clj b/src/metabase/models/query_execution.clj index 122c68b89a013d77e0d5ab81da649013eacc5e4b..0599fa0977c9691e97f5c7ddf111f1d739d3d6a5 100644 --- a/src/metabase/models/query_execution.clj +++ b/src/metabase/models/query_execution.clj @@ -1,32 +1,18 @@ (ns metabase.models.query-execution - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.api.common :refer [check]] [metabase.db :refer :all] (metabase.models [common :refer :all] - [database :refer [Database]]))) + [database :refer [Database]] + [interface :refer :all]))) (defentity QueryExecution - (table :query_queryexecution) - (types {:json_query :json - :result_data :json - :status :keyword})) + [(table :query_queryexecution) + (default-fields id uuid version json_query raw_query status started_at finished_at running_time error result_rows) + (types :json_query :json, :result_data :json, :status :keyword)] -;; default fields to return for `sel QueryExecution -;; specifically excludes stored data columns -(defmethod default-fields QueryExecution [_] - [:id - :uuid - :version - :json_query - :raw_query - :status - :started_at - :finished_at - :running_time - :error - :result_rows]) - -(defmethod post-select QueryExecution [_ {:keys [result_rows] :as query-execution}] - (assoc query-execution - :row_count (or result_rows 0))) ; sadly we have 2 ways to reference the row count :( + (post-select [_ {:keys [result_rows] :as query-execution}] + ;; sadly we have 2 ways to reference the row count :( + (assoc query-execution + :row_count (or result_rows 0)))) diff --git a/src/metabase/models/session.clj b/src/metabase/models/session.clj index 2b6a2e510685344e8de78d26e55068f72b1922f1..877890cdf09664e0b00992e6db4c63bfd0fa27aa 100644 --- a/src/metabase/models/session.clj +++ b/src/metabase/models/session.clj @@ -1,14 +1,15 @@ (ns metabase.models.session - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] (metabase.models [common :refer :all] + [interface :refer :all] [user :refer [User]]) [metabase.util :as u])) (defentity Session - (table :core_session) - (belongs-to User {:fk :user_id})) + [(table :core_session) + (belongs-to User {:fk :user_id})] -(defmethod pre-insert Session [_ session] - (let [defaults {:created_at (u/new-sql-timestamp)}] - (merge defaults session))) + (pre-insert [_ session] + (let [defaults {:created_at (u/new-sql-timestamp)}] + (merge defaults session)))) diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj index f6e0b55a2c63c31eb1c5685344c6dfb1aafae9f5..e96a2238a189f3eccbc33707bfa9abada8b4278a 100644 --- a/src/metabase/models/setting.clj +++ b/src/metabase/models/setting.clj @@ -3,8 +3,9 @@ (:require [clojure.core.match :refer [match]] [clojure.string :as s] [environ.core :as env] - [korma.core :refer :all :exclude [delete]] - [metabase.db :refer [sel del]])) + [korma.core :refer :all :exclude [defentity delete]] + [metabase.db :refer [sel del]] + [metabase.models.interface :refer :all])) ;; Settings are a fast + simple way to create a setting that can be set ;; from the SuperAdmin page. They are saved to the Database, but intelligently @@ -157,7 +158,7 @@ (atom nil)) (defentity Setting - (table :setting)) + [(table :setting)]) (defn- settings-list "Return a list of all Settings (as created with `defsetting`)." diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj index 44752d203576fb98b6c624ccadfc51d0f29f6f1b..f4a60161dd84d39b43de7fe5df03fa4edda845c9 100644 --- a/src/metabase/models/table.clj +++ b/src/metabase/models/table.clj @@ -1,43 +1,49 @@ (ns metabase.models.table - (:require [korma.core :refer :all] + (:require [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] (metabase.models [common :as common] [database :as db] [field :refer [Field]] - [field-values :refer [FieldValues]]) + [field-values :refer [FieldValues]] + [interface :refer :all]) [metabase.util :as u])) -(def entity-types +(def ^:const entity-types "Valid values for `Table.entity_type` (field may also be `nil`)." #{:person :event :photo :place}) -(defentity Table - (table :metabase_table) - timestamped - (types {:entity_type :keyword}) - (assoc :hydration-keys #{:table})) +(defrecord TableInstance [] + clojure.lang.IFn + (invoke [this k] + (get this k))) + +(extend-ICanReadWrite TableInstance :read :always, :write :superuser) +(defentity Table + [(table :metabase_table) + (hydration-keys table) + (types :entity_type :keyword) + timestamped] -; also missing :active and :pk_field -(defmethod post-select Table [_ {:keys [id db db_id description] :as table}] - (u/assoc* table - :db (or db (delay (sel :one db/Database :id db_id))) - :fields (delay (sel :many Field :table_id id :active true (order :position :ASC) (order :name :ASC))) - :field_values (delay - (let [field-ids (sel :many :field [Field :id] - :table_id id - :active true - :field_type [not= "sensitive"] - (order :position :asc) - (order :name :asc))] - (sel :many :field->field [FieldValues :field_id :values] :field_id [in field-ids]))) - :description (u/jdbc-clob->str description) - :pk_field (delay (:id (sel :one :fields [Field :id] :table_id id (where {:special_type "id"})))) - :can_read (delay @(:can_read @(:db <>))) - :can_write (delay @(:can_write @(:db <>))) - :human_readable_name (when (:name table) - (delay (common/name->human-readable-name (:name table)))))) + (post-select [_ {:keys [id db db_id description] :as table}] + (map->TableInstance + (u/assoc* table + :db (or db (delay (sel :one db/Database :id db_id))) + :fields (delay (sel :many Field :table_id id :active true (order :position :ASC) (order :name :ASC))) + :field_values (delay + (let [field-ids (sel :many :field [Field :id] + :table_id id + :active true + :field_type [not= "sensitive"] + (order :position :asc) + (order :name :asc))] + (sel :many :field->field [FieldValues :field_id :values] :field_id [in field-ids]))) + :description (u/jdbc-clob->str description) + :pk_field (delay (:id (sel :one :fields [Field :id] :table_id id (where {:special_type "id"})))) + :human_readable_name (when (:name table) + (delay (common/name->human-readable-name (:name table))))))) + (pre-cascade-delete [_ {:keys [id] :as table}] + (cascade-delete Field :table_id id))) -(defmethod pre-cascade-delete Table [_ {:keys [id] :as table}] - (cascade-delete Field :table_id id)) +(extend-ICanReadWrite TableEntity :read :always, :write :superuser) diff --git a/src/metabase/models/table_segment.clj b/src/metabase/models/table_segment.clj deleted file mode 100644 index 8a726b48fe5f14159252327956ce079ffe54856d..0000000000000000000000000000000000000000 --- a/src/metabase/models/table_segment.clj +++ /dev/null @@ -1,5 +0,0 @@ -(ns metabase.models.table-segment - (:require [korma.core :refer :all])) - -(defentity TableSegment - (table :metabase_tablesegment)) diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 2358f4b3ff352a2e0cdf6889266ff8376b870ef7..612aada390cb502664b9b97609b4636fc5821829 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -1,60 +1,51 @@ (ns metabase.models.user (:require [cemerick.friend.credentials :as creds] - [korma.core :refer :all] + [korma.core :refer :all, :exclude [defentity]] [metabase.db :refer :all] [metabase.email.messages :as email] + [metabase.models.interface :refer :all] [metabase.util :as u])) ;; ## Enity + DB Multimethods (defentity User - (table :core_user) - (assoc :hydration-keys #{:author :creator :user})) + [(table :core_user) + (default-fields id email date_joined first_name last_name last_login is_superuser) + (hydration-keys author creator user)] -;; fields to return for Users other `*than current-user*` -(defmethod default-fields User [_] - [:id - :email - :date_joined - :first_name - :last_name - :last_login - :is_superuser]) + (pre-insert [_ {:keys [email password] :as user}] + (assert (u/is-email? email)) + (assert (and (string? password) + (not (clojure.string/blank? password)))) + (assert (not (:password_salt user)) + "Don't try to pass an encrypted password to (ins User). Password encryption is handled by pre-insert.") + (let [salt (.toString (java.util.UUID/randomUUID)) + defaults {:date_joined (u/new-sql-timestamp) + :last_login (u/new-sql-timestamp) + :is_staff true + :is_active true + :is_superuser false}] + ;; always salt + encrypt the password before put new User in the DB + (merge defaults user {:password_salt salt + :password (creds/hash-bcrypt (str salt password))}))) -(def current-user-fields - "The fields we should return for `*current-user*` (used by `metabase.middleware.current-user`)" - (concat (default-fields User) - [:is_active - :is_staff])) ; but not `password` ! - -(defmethod post-select User [_ user] - (-> user - (assoc :common_name (str (:first_name user) " " (:last_name user))))) + (pre-update [_ {:keys [email] :as user}] + (when email + (assert (u/is-email? email))) + user) -(defmethod pre-insert User [_ {:keys [email password] :as user}] - (assert (u/is-email? email)) - (assert (and (string? password) - (not (clojure.string/blank? password)))) - (assert (not (:password_salt user)) - "Don't try to pass an encrypted password to (ins User). Password encryption is handled by pre-insert.") - (let [salt (.toString (java.util.UUID/randomUUID)) - defaults {:date_joined (u/new-sql-timestamp) - :last_login (u/new-sql-timestamp) - :is_staff true - :is_active true - :is_superuser false}] - ;; always salt + encrypt the password before put new User in the DB - (merge defaults user {:password_salt salt - :password (creds/hash-bcrypt (str salt password))}))) + (post-select [_ user] + (assoc user :common_name (str (:first_name user) " " (:last_name user)))) -(defmethod pre-update User [_ {:keys [email] :as user}] - (when email - (assert (u/is-email? email))) - user) + (pre-cascade-delete [_ {:keys [id]}] + (cascade-delete 'metabase.models.session/Session :user_id id))) -(defmethod pre-cascade-delete User [_ {:keys [id]}] - (cascade-delete 'metabase.models.session/Session :user_id id)) +(def ^:const current-user-fields + "The fields we should return for `*current-user*` (used by `metabase.middleware.current-user`)" + (concat (:metabase.models.interface/default-fields User) + [:is_active + :is_staff])) ; but not `password` ! ;; ## Related Functions diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj index 93ab8edbf0f68ebdd5f114f2c785f3f9760773d7..2b017fd5faf3075c9992337d87ff33046c0fdf2c 100644 --- a/test/metabase/api/card_test.clj +++ b/test/metabase/api/card_test.clj @@ -122,7 +122,7 @@ (expect-eval-actual-first nil (let [{:keys [id]} (post-card (random-name))] ((user->client :rasta) :delete 204 (format "card/%d" id)) - (sel :one Card :id id))) + (Card id))) ;; # CARD FAVORITE STUFF diff --git a/test/metabase/api/dash_test.clj b/test/metabase/api/dash_test.clj index 0f68968cd0ade016940446a6c742dfc4b8907fb0..f8434d9a2e4b362a812141367b07f273653477f0 100644 --- a/test/metabase/api/dash_test.clj +++ b/test/metabase/api/dash_test.clj @@ -44,8 +44,8 @@ {:description nil :can_read true :ordered_cards [] - :creator (-> (sel :one User :id (user->id :rasta)) - (select-keys [:email :first_name :last_login :is_superuser :id :last_name :date_joined :common_name])) + :creator (-> (User (user->id :rasta)) + (select-keys [:email :first_name :last_login :is_superuser :id :last_name :date_joined :common_name])) :can_write true :organization_id nil :name $ @@ -99,7 +99,7 @@ (expect-let [{:keys [id]} (create-dash (random-name))] nil (do ((user->client :rasta) :delete 204 (format "dash/%d" id)) - (sel :one Dashboard :id id))) + (Dashboard id))) ;; # DASHBOARD CARD ENDPOINTS @@ -115,7 +115,7 @@ {:sizeX 2 :card (match-$ card {:description nil - :creator (-> (sel :one User :id (user->id :rasta)) + :creator (-> (User (user->id :rasta)) (select-keys [:date_joined :last_name :id :is_superuser :last_login :first_name :email :common_name])) :organization_id nil :name $ diff --git a/test/metabase/api/meta/db_test.clj b/test/metabase/api/meta/db_test.clj index 736efb3dc7e89dd814ef10f2a6b7e0db4130237f..73c226390217ff0d2ba6a41470938f017cd34d08 100644 --- a/test/metabase/api/meta/db_test.clj +++ b/test/metabase/api/meta/db_test.clj @@ -40,18 +40,31 @@ ;; # DB LIFECYCLE ENDPOINTS ;; ## GET /api/meta/db/:id +;; regular users *should not* see DB details (expect (match-$ (db) - {:created_at $ - :engine "h2" - :id $ - :details $ - :updated_at $ - :name "Test Database" + {:created_at $ + :engine "h2" + :id $ + :updated_at $ + :name "Test Database" :organization_id nil - :description nil}) + :description nil}) ((user->client :rasta) :get 200 (format "meta/db/%d" (db-id)))) +;; superusers *should* see DB details +(expect + (match-$ (db) + {:created_at $ + :engine "h2" + :id $ + :details $ + :updated_at $ + :name "Test Database" + :organization_id nil + :description nil}) + ((user->client :crowberto) :get 200 (format "meta/db/%d" (db-id)))) + ;; ## POST /api/meta/db ;; Check that we can create a Database (let [db-name (random-name)] @@ -106,6 +119,7 @@ ;; ## GET /api/meta/db ;; Test that we can get all the DBs for an Org, ordered by name +;; Database details *should not* come back for Rasta since she's not a superuser (let [db-name (str "A" (random-name))] ; make sure this name comes before "Test Database" (expect-eval-actual-first (set (filter identity @@ -115,7 +129,6 @@ {:created_at $ :engine (name $engine) :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil @@ -125,7 +138,6 @@ {:created_at $ :engine "postgres" :id $ - :details {:host "localhost", :port 5432, :dbname "fakedb", :user "cam"} :updated_at $ :name $ :organization_id nil @@ -148,12 +160,12 @@ ;; These should come back in alphabetical order (expect (let [db-id (db-id)] - [(match-$ (sel :one Table :id (id :categories)) + [(match-$ (Table (id :categories)) {:description nil, :entity_type nil, :name "CATEGORIES", :rows 75, :updated_at $, :entity_name nil, :active true, :id $, :db_id db-id, :created_at $, :human_readable_name "Categories"}) - (match-$ (sel :one Table :id (id :checkins)) + (match-$ (Table (id :checkins)) {:description nil, :entity_type nil, :name "CHECKINS", :rows 1000, :updated_at $, :entity_name nil, :active true, :id $, :db_id db-id, :created_at $, :human_readable_name "Checkins"}) - (match-$ (sel :one Table :id (id :users)) + (match-$ (Table (id :users)) {:description nil, :entity_type nil, :name "USERS", :rows 15, :updated_at $, :entity_name nil, :active true, :id $, :db_id db-id, :created_at $, :human_readable_name "Users"}) - (match-$ (sel :one Table :id (id :venues)) + (match-$ (Table (id :venues)) {:description nil, :entity_type nil, :name "VENUES", :rows 100, :updated_at $, :entity_name nil, :active true, :id $, :db_id db-id, :created_at $, :human_readable_name "Venues"})]) ((user->client :rasta) :get 200 (format "meta/db/%d/tables" (db-id)))) diff --git a/test/metabase/api/meta/field_test.clj b/test/metabase/api/meta/field_test.clj index 09134c7bf6f42bd03dbfe8e86e78e80e39d8c5bb..15b8c860989489b6624851c6b1f309c26bf2b732 100644 --- a/test/metabase/api/meta/field_test.clj +++ b/test/metabase/api/meta/field_test.clj @@ -12,17 +12,16 @@ ;; ## GET /api/meta/field/:id (expect - (match-$ (sel :one Field :id (id :users :name)) + (match-$ (Field (id :users :name)) {:description nil :table_id (id :users) - :table (match-$ (sel :one Table :id (id :users)) + :table (match-$ (Table (id :users)) {:description nil :entity_type nil :db (match-$ (db) {:created_at $ :engine "h2" :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil @@ -58,7 +57,7 @@ ;; Check that we can update a Field ;; TODO - this should NOT be modifying a field from our test data, we should create new data to mess with (expect-eval-actual-first - (match-$ (let [field (sel :one Field :id (id :venues :latitude))] + (match-$ (let [field (Field (id :venues :latitude))] ;; this is sketchy. But return the Field back to its unmodified state so it won't affect other unit tests (upd Field (id :venues :latitude) :special_type "latitude") ;; match against the modified Field diff --git a/test/metabase/api/meta/table_test.clj b/test/metabase/api/meta/table_test.clj index 4bb229203a3cd289deae03e06a92e59efc19c2df..f53ad2dba3c7aff2b3f375b3649a3ebb65010b82 100644 --- a/test/metabase/api/meta/table_test.clj +++ b/test/metabase/api/meta/table_test.clj @@ -57,14 +57,13 @@ ;; ## GET /api/meta/table/:id (expect - (match-$ (sel :one Table :id (id :venues)) + (match-$ (Table (id :venues)) {:description nil :entity_type nil :db (match-$ (db) {:created_at $ :engine "h2" :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil @@ -81,7 +80,7 @@ ((user->client :rasta) :get 200 (format "meta/table/%d" (id :venues)))) ;; ## GET /api/meta/table/:id/fields -(expect [(match-$ (sel :one Field :id (id :categories :id)) +(expect [(match-$ (Field (id :categories :id)) {:description nil :table_id (id :categories) :special_type "id" @@ -95,7 +94,7 @@ :preview_display true :created_at $ :base_type "BigIntegerField"}) - (match-$ (sel :one Field :id (id :categories :name)) + (match-$ (Field (id :categories :name)) {:description nil :table_id (id :categories) :special_type "name" @@ -113,20 +112,19 @@ ;; ## GET /api/meta/table/:id/query_metadata (expect - (match-$ (sel :one Table :id (id :categories)) + (match-$ (Table (id :categories)) {:description nil :entity_type nil :db (match-$ (db) {:created_at $ :engine "h2" :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil :description nil}) :name "CATEGORIES" - :fields [(match-$ (sel :one Field :id (id :categories :id)) + :fields [(match-$ (Field (id :categories :id)) {:description nil :table_id (id :categories) :special_type "id" @@ -140,7 +138,7 @@ :preview_display true :created_at $ :base_type "BigIntegerField"}) - (match-$ (sel :one Field :id (id :categories :name)) + (match-$ (Field (id :categories :name)) {:description nil :table_id (id :categories) :special_type "name" @@ -186,20 +184,19 @@ ;;; GET api/meta/table/:id/query_metadata?include_sensitive_fields ;;; Make sure that getting the User table *does* include info about the password field, but not actual values themselves (expect - (match-$ (sel :one Table :id (id :users)) + (match-$ (Table (id :users)) {:description nil :entity_type nil :db (match-$ (db) {:created_at $ :engine "h2" :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil :description nil}) :name "USERS" - :fields [(match-$ (sel :one Field :id (id :users :id)) + :fields [(match-$ (Field (id :users :id)) {:description nil :table_id (id :users) :special_type "id" @@ -213,7 +210,7 @@ :preview_display true :created_at $ :base_type "BigIntegerField"}) - (match-$ (sel :one Field :id (id :users :last_login)) + (match-$ (Field (id :users :last_login)) {:description nil :table_id (id :users) :special_type "category" @@ -227,7 +224,7 @@ :preview_display true :created_at $ :base_type "DateTimeField"}) - (match-$ (sel :one Field :id (id :users :name)) + (match-$ (Field (id :users :name)) {:description nil :table_id (id :users) :special_type "category" @@ -286,20 +283,19 @@ ;;; GET api/meta/table/:id/query_metadata ;;; Make sure that getting the User table does *not* include password info (expect - (match-$ (sel :one Table :id (id :users)) + (match-$ (Table (id :users)) {:description nil :entity_type nil :db (match-$ (db) {:created_at $ :engine "h2" :id $ - :details $ :updated_at $ :name "Test Database" :organization_id nil :description nil}) :name "USERS" - :fields [(match-$ (sel :one Field :id (id :users :id)) + :fields [(match-$ (Field (id :users :id)) {:description nil :table_id (id :users) :special_type "id" @@ -313,7 +309,7 @@ :preview_display true :created_at $ :base_type "BigIntegerField"}) - (match-$ (sel :one Field :id (id :users :last_login)) + (match-$ (Field (id :users :last_login)) {:description nil :table_id (id :users) :special_type "category" @@ -327,7 +323,7 @@ :preview_display true :created_at $ :base_type "DateTimeField"}) - (match-$ (sel :one Field :id (id :users :name)) + (match-$ (Field (id :users :name)) {:description nil :table_id (id :users) :special_type "category" @@ -372,7 +368,7 @@ ;; ## PUT /api/meta/table/:id (expect-eval-actual-first - (match-$ (let [table (sel :one Table :id (id :users))] + (match-$ (let [table (Table (id :users))] ;; reset Table back to its original state (upd Table (id :users) :entity_name nil :entity_type nil :description nil) table) @@ -426,7 +422,7 @@ :special_type "fk" :created_at $ :updated_at $ - :table (match-$ (sel :one Table :id (id :checkins)) + :table (match-$ (Table (id :checkins)) {:description nil :entity_type nil :name "CHECKINS" @@ -444,8 +440,7 @@ :updated_at $, :id $, :engine "h2", - :created_at $ - :details $})})}) + :created_at $})})}) :destination (match-$ users-id-field {:id $ :table_id $ @@ -459,7 +454,7 @@ :special_type "id" :created_at $ :updated_at $ - :table (match-$ (sel :one Table :id (id :users)) + :table (match-$ (Table (id :users)) {:description nil :entity_type nil :name "USERS" diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj index 6fb6f084a76b35dc9df3a62b936aca467762f79e..5313b50a7ce6d89fa3fc55cb6027d2c4e97f8301 100644 --- a/test/metabase/api/session_test.clj +++ b/test/metabase/api/session_test.clj @@ -41,7 +41,7 @@ (let [{session_id :id} ((user->client :rasta) :post 200 "session" (user->credentials :rasta))] (assert session_id) ((user->client :rasta) :delete 204 "session" :session_id session_id) - (sel :one Session :id session_id))) + (Session session_id))) ;; ## POST /api/session/forgot_password diff --git a/test/metabase/driver/generic_sql_test.clj b/test/metabase/driver/generic_sql_test.clj index 350119284a9873238a347640c9ad42d45b5de994..a490a483f8c36621428461e2320a440245c4aa4a 100644 --- a/test/metabase/driver/generic_sql_test.clj +++ b/test/metabase/driver/generic_sql_test.clj @@ -22,7 +22,7 @@ (delay (korma-entity @users-table))) (def users-name-field - (delay (sel :one Field :id (id :users :name)))) + (delay (Field (id :users :name)))) ;; ACTIVE-TABLE-NAMES (expect diff --git a/test/metabase/driver/sync_test.clj b/test/metabase/driver/sync_test.clj index bdca4176f5f1374483a0eb1007750edbc1b58615..f0396e8b15ee76ca008ca6679438b9144a20b22a 100644 --- a/test/metabase/driver/sync_test.clj +++ b/test/metabase/driver/sync_test.clj @@ -23,7 +23,7 @@ (delay (korma-entity @users-table))) (def users-name-field - (delay (sel :one Field :id (id :users :name)))) + (delay (Field (id :users :name)))) ;; ## TEST PK SYNCING @@ -77,16 +77,16 @@ (upd Field field-id :special_type nil) (get-special-type-and-fk-exists?)) ;; Run sync-table and they should be set again - (let [table (sel :one Table :id (id :checkins))] + (let [table (Table (id :checkins))] (driver/sync-table! table) (get-special-type-and-fk-exists?))])) ;; ## Tests for DETERMINE-FK-TYPE ;; Since COUNT(category_id) > COUNT(DISTINCT(category_id)) the FK relationship should be Mt1 (expect :Mt1 - (sync/determine-fk-type (sel :one Field :id (id :venues :category_id)))) + (sync/determine-fk-type (Field (id :venues :category_id)))) ;; Since COUNT(id) == COUNT(DISTINCT(id)) the FK relationship should be 1t1 ;; (yes, ID isn't really a FK field, but determine-fk-type doesn't need to know that) (expect :1t1 - (sync/determine-fk-type (sel :one Field :id (id :venues :id)))) + (sync/determine-fk-type (Field (id :venues :id)))) diff --git a/test/metabase/middleware/auth_test.clj b/test/metabase/middleware/auth_test.clj index 871143d14a425766ed2ab26b17fec3b7b4e5dec7..58ac400655c86014fe3e3009d7c51339cfa6350f 100644 --- a/test/metabase/middleware/auth_test.clj +++ b/test/metabase/middleware/auth_test.clj @@ -1,188 +1,172 @@ (ns metabase.middleware.auth-test (:require [expectations :refer :all] - [korma.core :as korma] + [korma.core :as k] [ring.mock.request :as mock] [metabase.api.common :refer [*current-user-id* *current-user*]] [metabase.middleware.auth :refer :all] [metabase.models.session :refer [Session]] [metabase.test.data :refer :all] [metabase.test.data.users :refer :all] - [metabase.util :as util])) + [metabase.util :as u])) -;; =========================== TEST wrap-sessionid middleware =========================== +;; =========================== TEST wrap-session-id middleware =========================== ;; create a simple example of our middleware wrapped around a handler that simply returns the request ;; this works in this case because the only impact our middleware has is on the request -(def wrapped-handler - (wrap-sessionid (fn [req] req))) +(def ^:private wrapped-handler + (wrap-session-id identity)) -;; no sessionid in the request -(expect - {} +;; no session-id in the request +(expect nil (-> (wrapped-handler (mock/request :get "/anyurl") ) - (select-keys [:metabase-sessionid]))) + :metabase-session-id)) -;; extract sessionid from header -(expect - {:metabase-sessionid "foobar"} +;; extract session-id from header +(expect "foobar" (-> (wrapped-handler (mock/header (mock/request :get "/anyurl") metabase-session-header "foobar")) - (select-keys [:metabase-sessionid]))) + :metabase-session-id)) -;; extract sessionid from cookie -(expect - {:metabase-sessionid "cookie-session"} +;; extract session-id from cookie +(expect "cookie-session" (-> (wrapped-handler (assoc (mock/request :get "/anyurl") :cookies {metabase-session-cookie {:value "cookie-session"}})) - (select-keys [:metabase-sessionid]))) + :metabase-session-id)) -;; if both header and cookie sessionids exist, then we expect the cookie to take precedence -(expect - {:metabase-sessionid "cookie-session"} +;; if both header and cookie session-ids exist, then we expect the cookie to take precedence +(expect "cookie-session" (-> (wrapped-handler (-> (mock/header (mock/request :get "/anyurl") metabase-session-header "foobar") (assoc :cookies {metabase-session-cookie {:value "cookie-session"}}))) - (select-keys [:metabase-sessionid]))) + :metabase-session-id)) ;; =========================== TEST enforce-authentication middleware =========================== ;; create a simple example of our middleware wrapped around a handler that simply returns the request -(def auth-enforced-handler - (enforce-authentication (fn [req] req))) +(def ^:private auth-enforced-handler + (wrap-current-user-id (enforce-authentication identity))) -(defn request-with-sessionid - "Creates a mock Ring request with the given sessionid applied" - [sessionid] +(defn- request-with-session-id + "Creates a mock Ring request with the given session-id applied" + [session-id] (-> (mock/request :get "/anyurl") - (assoc :metabase-sessionid sessionid))) + (assoc :metabase-session-id session-id))) -;; no sessionid in the request -(expect - {:status (:status response-unauthentic) - :body (:body response-unauthentic)} +;; no session-id in the request +(expect response-unauthentic (auth-enforced-handler (mock/request :get "/anyurl"))) +(defn- random-session-id [] + {:post [(string? %)]} + (.toString (java.util.UUID/randomUUID))) -;; valid sessionid -(let [sessionid (.toString (java.util.UUID/randomUUID))] - (assert sessionid) - ;; validate that we are authenticated - (expect-let [res (korma/insert Session (korma/values {:id sessionid :user_id (user->id :rasta) :created_at (util/new-sql-timestamp)}))] - {:metabase-userid (user->id :rasta)} - (-> (auth-enforced-handler (request-with-sessionid sessionid)) - (select-keys [:metabase-userid])))) +;; valid session ID +(expect (user->id :rasta) + (let [session-id (random-session-id)] + (k/insert Session (k/values {:id session-id, :user_id (user->id :rasta), :created_at (u/new-sql-timestamp)})) + (-> (auth-enforced-handler (request-with-session-id session-id)) + :metabase-user-id))) -;; expired sessionid -(let [sessionid (.toString (java.util.UUID/randomUUID))] - (assert sessionid) - ;; create a new session (specifically created some time in the past so it's EXPIRED) - ;; should fail due to session expiration - (expect-let [res (korma/insert Session (korma/values {:id sessionid :user_id (user->id :rasta) :created_at (java.sql.Timestamp. 0)}))] - {:status (:status response-unauthentic) - :body (:body response-unauthentic)} - (auth-enforced-handler (request-with-sessionid sessionid)))) +;; expired session-id +;; create a new session (specifically created some time in the past so it's EXPIRED) +;; should fail due to session expiration +(expect response-unauthentic + (let [session-id (random-session-id)] + (k/insert Session (k/values {:id session-id, :user_id (user->id :rasta), :created_at (java.sql.Timestamp. 0)})) + (auth-enforced-handler (request-with-session-id session-id)))) -;; inactive user sessionid -(let [sessionid (.toString (java.util.UUID/randomUUID))] - (assert sessionid) - ;; create a new session (specifically created some time in the past so it's EXPIRED) - ;; should fail due to inactive user - ;; NOTE that :trashbird is our INACTIVE test user - (expect-let [res (korma/insert Session (korma/values {:id sessionid :user_id (user->id :trashbird) :created_at (util/new-sql-timestamp)}))] - {:status (:status response-unauthentic) - :body (:body response-unauthentic)} - (auth-enforced-handler (request-with-sessionid sessionid)))) +;; inactive user session-id +;; create a new session (specifically created some time in the past so it's EXPIRED) +;; should fail due to inactive user +;; NOTE that :trashbird is our INACTIVE test user +(expect response-unauthentic + (let [session-id (random-session-id)] + (k/insert Session (k/values {:id session-id, :user_id (user->id :trashbird), :created_at (u/new-sql-timestamp)})) + (auth-enforced-handler (request-with-session-id session-id)))) ;; =========================== TEST bind-current-user middleware =========================== ;; create a simple example of our middleware wrapped around a handler that simply returns our bound variables for users -(def user-bound-handler - (bind-current-user (fn [req] {:userid *current-user-id* - :user (select-keys @*current-user* [:id :email])}))) +(def ^:private user-bound-handler + (bind-current-user (fn [_] {:user-id *current-user-id* + :user (select-keys @*current-user* [:id :email])}))) - -(defn request-with-userid - "Creates a mock Ring request with the given userid applied" - [userid] +(defn- request-with-user-id + "Creates a mock Ring request with the given user-id applied" + [user-id] (-> (mock/request :get "/anyurl") - (assoc :metabase-userid userid))) + (assoc :metabase-user-id user-id))) ;; with valid user-id (expect - {:userid (user->id :rasta) - :user {:id (user->id :rasta) - :email (:email (fetch-user :rasta))}} - (user-bound-handler (request-with-userid (user->id :rasta)))) + {:user-id (user->id :rasta) + :user {:id (user->id :rasta) + :email (:email (fetch-user :rasta))}} + (user-bound-handler (request-with-user-id (user->id :rasta)))) ;; with invalid user-id (not sure how this could ever happen, but lets test it anyways) (expect - {:userid 0 - :user {}} - (user-bound-handler (request-with-userid 0))) + {:user-id 0 + :user {}} + (user-bound-handler (request-with-user-id 0))) -;; =========================== TEST wrap-apikey middleware =========================== +;; =========================== TEST wrap-api-key middleware =========================== ;; create a simple example of our middleware wrapped around a handler that simply returns the request ;; this works in this case because the only impact our middleware has is on the request -(def wrapped-apikey-handler - (wrap-apikey (fn [req] req))) +(def ^:private wrapped-api-key-handler + (wrap-api-key identity)) ;; no apikey in the request -(expect - {} - (-> (wrapped-apikey-handler (mock/request :get "/anyurl") ) - (select-keys [:metabase-sessionid]))) +(expect nil + (-> (wrapped-api-key-handler (mock/request :get "/anyurl") ) + :metabase-session-id)) ;; extract apikey from header -(expect - {:metabase-apikey "foobar"} - (-> (wrapped-apikey-handler (mock/header (mock/request :get "/anyurl") metabase-apikey-header "foobar")) - (select-keys [:metabase-apikey]))) +(expect "foobar" + (-> (wrapped-api-key-handler (mock/header (mock/request :get "/anyurl") metabase-api-key-header "foobar")) + :metabase-api-key)) -;; =========================== TEST enforce-apikey middleware =========================== +;; =========================== TEST enforce-api-key middleware =========================== ;; create a simple example of our middleware wrapped around a handler that simply returns the request -(def apikey-enforced-handler - (enforce-apikey (fn [req] {:success true}))) +(def ^:private api-key-enforced-handler + (enforce-api-key (constantly {:success true}))) -(defn request-with-apikey +(defn- request-with-api-key "Creates a mock Ring request with the given apikey applied" - [apikey] + [api-key] (-> (mock/request :get "/anyurl") - (assoc :metabase-apikey apikey))) + (assoc :metabase-api-key api-key))) ;; no apikey in the request, expect 403 -(expect - {:status (:status response-forbidden) - :body (:body response-forbidden)} - (apikey-enforced-handler (mock/request :get "/anyurl"))) +(expect response-forbidden + (api-key-enforced-handler (mock/request :get "/anyurl"))) ;; valid apikey, expect 200 (expect - {:success true} - (apikey-enforced-handler (request-with-apikey "test-api-key"))) + {:success true} + (api-key-enforced-handler (request-with-api-key "test-api-key"))) ;; invalid apikey, expect 403 -(expect - {:status (:status response-forbidden) - :body (:body response-forbidden)} - (apikey-enforced-handler (request-with-apikey "foobar"))) +(expect response-forbidden + (api-key-enforced-handler (request-with-api-key "foobar"))) diff --git a/test/metabase/models/common_test.clj b/test/metabase/models/common_test.clj deleted file mode 100644 index 2ac473653d073d8fbcfecf79d639211e6d24aa08..0000000000000000000000000000000000000000 --- a/test/metabase/models/common_test.clj +++ /dev/null @@ -1,26 +0,0 @@ -(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` diff --git a/test/metabase/task_test.clj b/test/metabase/task_test.clj index 5e1b817e39096e7fefbb0a007cfbf13927c8243f..a6e07ddfc41efc46dd2a18752b61960a31cba4bb 100644 --- a/test/metabase/task_test.clj +++ b/test/metabase/task_test.clj @@ -67,16 +67,16 @@ :restarted] [(do (stop-task-runner!) - (with-redefs [metabase.task/hourly-task-delay (constantly 100) + (with-redefs [metabase.task/hourly-task-delay (constantly 200) metabase.task/hourly-tasks-hook mock-hourly-tasks-hook] (add-hook! #'hourly-tasks-hook inc-task-test-atom-counter-by-system-hour) (reset! task-test-atom-counter 0) (start-task-runner!) [@task-test-atom-counter ; should be 0, since not enough time has elaspsed for the hook to be executed - (do (Thread/sleep 150) - @task-test-atom-counter) ; should have been called once (~50ms ago) - (do (Thread/sleep 200) + (do (Thread/sleep 300) + @task-test-atom-counter) ; should have been called once (~100ms ago) + (do (Thread/sleep 400) @task-test-atom-counter) ; should have been called two more times (do (stop-task-runner!) :stopped)])) diff --git a/test/metabase/test/data/datasets.clj b/test/metabase/test/data/datasets.clj index 4d4fda20b8fa298a101d4bfc74be9a678267735a..e45fa8fa4938469225f0b432dbd71dacfe59491f 100644 --- a/test/metabase/test/data/datasets.clj +++ b/test/metabase/test/data/datasets.clj @@ -109,7 +109,7 @@ (memoized-table-name->id (:id (db this)) (s/upper-case (name table-name)))) (table-name->table [this table-name] - (sel :one Table :id (table-name->id this (s/upper-case (name table-name))))) + (Table (table-name->id this (s/upper-case (name table-name))))) (field-name->id [this table-name field-name] (memoized-field-name->id (:id (db this)) (s/upper-case (name table-name)) (s/upper-case (name field-name)))) @@ -141,7 +141,7 @@ (memoized-table-name->id (:id (db this)) (name table-name))) (table-name->table [this table-name] - (sel :one Table :id (table-name->id this (name table-name)))) + (Table (table-name->id this (name table-name)))) (field-name->id [this table-name field-name] (memoized-field-name->id (:id (db this)) (name table-name) (name field-name))) diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj index fe648603da699e04c2cda905825265aa31231415..bd8f92f6e4dadbcb3ecc3424ede61e155f7bffd1 100644 --- a/test/metabase/test/data/users.clj +++ b/test/metabase/test/data/users.clj @@ -52,9 +52,9 @@ [& {:as kwargs}] (let [first-name (random-name) defaults {:first_name first-name - :last_name (random-name) - :email (.toLowerCase ^String (str first-name "@metabase.com")) - :password first-name}] + :last_name (random-name) + :email (.toLowerCase ^String (str first-name "@metabase.com")) + :password first-name}] (->> (merge defaults kwargs) (m/mapply ins User)))) diff --git a/test/metabase/test_setup.clj b/test/metabase/test_setup.clj index d42a83338890bfe5de4d11ccf2a014926ccc39f5..5c7a54e20dca4cc3bfda987f99a014c99fa4b57f 100644 --- a/test/metabase/test_setup.clj +++ b/test/metabase/test_setup.clj @@ -75,7 +75,7 @@ (datasets/load-data! dataset) ;; Check that dataset is loaded and working - (assert (db/sel :one Table :id (datasets/table-name->id dataset :venues)) + (assert (Table (datasets/table-name->id dataset :venues)) (format "Loading test dataset %s failed: could not find 'venues' Table!" dataset-name))))) (defn test-startup