diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj index 6266696070d5f4362d9c4cad02dedd37f0aa9fae..65954e5f40f1a20440a2217fc92ec6aa924ad9c1 100644 --- a/src/metabase/api/common.clj +++ b/src/metabase/api/common.clj @@ -26,7 +26,7 @@ "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-fn :one "metabase.models.user/User" :id ~user-id))) ] + *current-user* (delay (sel :one 'metabase.models.user/User :id ~user-id))] ~@body)) (defn current-user-perms-for-org diff --git a/src/metabase/db.clj b/src/metabase/db.clj index f0f63324c724b0250402d41bd840fa7108444ad7..23851daeb7f5e238d2ffceffca16edf01f1eb631 100644 --- a/src/metabase/db.clj +++ b/src/metabase/db.clj @@ -201,13 +201,6 @@ (->> (select entity-select-form# ~@forms) (map (partial post-select entity#)))))) ; map `post-select` over the results -(defmacro sel-fn - "Returns a memoized fn that calls `sel`." - [one-or-many entity & forms] - `(memoize - (fn [] - (sel ~one-or-many ~entity ~@forms)))) - ;; ## INS diff --git a/src/metabase/db/internal.clj b/src/metabase/db/internal.clj index 953bd6f267f0abfe9a85f49a4ae0ad63efd7741f..4250f5e4dbb3781781497132155789876d4fa9f6 100644 --- a/src/metabase/db/internal.clj +++ b/src/metabase/db/internal.clj @@ -30,12 +30,13 @@ [(first entity) (vec (rest entity))])) (def entity->korma - "Convert an ENTITY argument to `sel`/`sel-fn` into the form we should pass to korma `select` and to various multi-methods such as + "Convert an ENTITY argument to `sel` into the form we should pass to korma `select` and to various multi-methods such as `post-select`. * If entity is a vector like `[User :name]`, only keeps the first arg (`User`) * Converts fully-qualified entity name strings like `\"metabase.models.user/User\"` to the corresponding entity - and requires their namespace if needed." + and requires their namespace if needed. + * Symbols like `'metabase.models.user/User` are handled the same way as strings." (memoize (fn -entity->korma [entity] {:post [(= (type %) :korma.core/Entity)]} diff --git a/src/metabase/driver/generic_sql/metadata.clj b/src/metabase/driver/generic_sql/metadata.clj index 9674a44c56f1fd60c3792585c7a0edd97babec5d..6abd7822029840a5ab0d306643c23e83bf5fcb89 100644 --- a/src/metabase/driver/generic_sql/metadata.clj +++ b/src/metabase/driver/generic_sql/metadata.clj @@ -6,9 +6,9 @@ [{:keys [db table name]}] (-> ((:native-query @db) (format "SELECT COUNT(\"%s\".\"%s\") AS count FROM \"%s\"" - (:name (table)) + (:name @table) name - (:name (table)))) + (:name @table))) first :count)) @@ -16,8 +16,8 @@ [{:keys [db table name]}] (-> ((:native-query @db) (format "SELECT COUNT(DISTINCT \"%s\".\"%s\") AS count FROM \"%s\"" - (:name (table)) + (:name @table) name - (:name (table)))) + (:name @table))) first :count)) diff --git a/src/metabase/driver/generic_sql/sync.clj b/src/metabase/driver/generic_sql/sync.clj index c214301bb32205635c0f9fb70a45090a81085534..65a6b20800b67a5e93ac7e10063d76292e045c70 100644 --- a/src/metabase/driver/generic_sql/sync.clj +++ b/src/metabase/driver/generic_sql/sync.clj @@ -47,13 +47,13 @@ (fn [_] ; by wrapping the entire sync operation in this we can reuse the same connection throughout (->> @table-names (pmap (fn [table-name] - (binding [*entity-overrides* {:transforms [#(assoc % :db (constantly database))]}] ; add a korma transform to Table that will assoc :db on results. - (let [table (or (sel :one Table :db_id id :name table-name) ; Table's post-select only sets :db if it's not already set. - (ins Table ; This way, we can reuse a single `database` instead of creating - :db_id id ; a few dozen duplicate instances of it. - :name table-name ; We can re-use one korma connection pool instead of - :active true))] ; creating dozens of them, which was causing issues with too - (update-table-row-count table) ; many open connections. + (binding [*entity-overrides* {:transforms [#(assoc % :db (delay database))]}] ; add a korma transform to Table that will assoc :db on results. + (let [table (or (sel :one Table :db_id id :name table-name) ; Table's post-select only sets :db if it's not already set. + (ins Table ; This way, we can reuse a single `database` instead of creating + :db_id id ; a few dozen duplicate instances of it. + :name table-name ; We can re-use one korma connection pool instead of + :active true))] ; creating dozens of them, which was causing issues with too + (update-table-row-count table) ; many open connections. (sync-fields table) (log/debug "Synced" table-name))))) dorun))))) diff --git a/src/metabase/models/annotation.clj b/src/metabase/models/annotation.clj index 34dbd168e73a7d1bdb9451d39312714651eacbaf..fd7fbc750d4a08d4305fc3295a7419036561b09a 100644 --- a/src/metabase/models/annotation.clj +++ b/src/metabase/models/annotation.clj @@ -2,9 +2,9 @@ (:require [korma.core :refer :all] [metabase.db :refer :all] (metabase.models [common :refer :all] - [hydrate :refer [realize-json]] - [org :refer [Org]] - [user :refer [User]]) + [hydrate :refer [realize-json]] + [org :refer [Org]] + [user :refer [User]]) [metabase.util :as util])) @@ -28,6 +28,6 @@ (defmethod post-select Annotation [_ {:keys [organization_id author_id] :as annotation}] (-> annotation - ;; TODO - would probably be nice to associate a function which pulls the object the annotation points to - (assoc :author (sel-fn :one User :id author_id) - :organization (sel-fn :one Org :id organization_id)))) + ;; TODO - would probably be nice to associate a function which pulls the object the annotation points to + (assoc :author (delay (sel :one User :id author_id)) + :organization (delay (sel :one Org :id organization_id))))) diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index b6fa13dd6d5e42aaf4568384844775d2e646e155..4597c0806f2781130644988a0154fc1a2bfbc05a 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -28,6 +28,6 @@ (defmethod post-select Card [_ {:keys [organization_id creator_id] :as card}] (-> card (realize-json :dataset_query :visualization_settings) - (assoc :creator (sel-fn :one User :id creator_id) - :organization (sel-fn :one Org :id organization_id)) + (assoc :creator (delay (sel :one User :id creator_id)) + :organization (delay (sel :one Org :id organization_id))) assoc-permissions-sets)) diff --git a/src/metabase/models/card_favorite.clj b/src/metabase/models/card_favorite.clj index 1fc66ee257e711f23b025c0e1463779c8f7cc1a1..fff8d266442675e5fb0f8481f8d45ac9f12bcfae 100644 --- a/src/metabase/models/card_favorite.clj +++ b/src/metabase/models/card_favorite.clj @@ -10,8 +10,8 @@ (defmethod post-select CardFavorite [_ {:keys [card_id owner_id] :as card-favorite}] (assoc card-favorite - :owner (sel-fn :one User :id owner_id) - :card (sel-fn :one Card :id card_id))) + :owner (delay (sel :one User :id owner_id)) + :card (delay (sel :one Card :id card_id)))) (defmethod pre-insert CardFavorite [_ card-favorite] (let [defaults {:created_at (new-sql-timestamp) diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index 25794c378358f73a838697c7ed40b2f488764717..6061092495787db18504533b182e46f085a7aca1 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -21,10 +21,10 @@ (defmethod post-select Dashboard [_ {:keys [id creator_id organization_id description] :as dash}] (-> dash - (assoc :creator (sel-fn :one User :id creator_id) - :description (util/jdbc-clob->str description) - :organization (sel-fn :one Org :id organization_id) - :ordered_cards (sel-fn :many DashboardCard :dashboard_id id)) + (assoc :creator (delay (sel :one User :id creator_id)) + :description (util/jdbc-clob->str description) + :organization (delay (sel :one Org :id organization_id)) + :ordered_cards (delay (sel :many DashboardCard :dashboard_id id))) assoc-permissions-sets)) ; TODO - ordered_cards diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 600c59a443c695e19af188cc55360ababad4ef88..7ec304a3997dbbcb3a21af1f1ee7a3156144a433 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -23,8 +23,8 @@ (-> 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 (sel-fn :one Card :id card_id) - :dashboard (sel-fn :one "metabase.models.dashboard/Dashboard" :id dashboard_id)))) + (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 {:created_at (util/new-sql-timestamp) diff --git a/src/metabase/models/database.clj b/src/metabase/models/database.clj index 02a09312aa0995637f30feb42db60edbff61f5aa..9dfa609be920668f8d3c2fa488362b4c822311bb 100644 --- a/src/metabase/models/database.clj +++ b/src/metabase/models/database.clj @@ -59,7 +59,7 @@ (defmethod post-select Database [_ {:keys [organization_id] :as db}] (-> db (realize-json :details) ; TODO wouldn't we want to actually strip this info instead of returning it? - (assoc* :organization (sel-fn :one Org :id organization_id) + (assoc* :organization (delay (sel :one Org :id organization_id)) :can_read (delay (org-can-read organization_id)) :can_write (delay (org-can-write organization_id)) :connection-details (delay (conn/connection-details <>)) @@ -79,6 +79,10 @@ (cond-> (assoc database :updated_at (new-sql-timestamp)) details (assoc :details (json/write-str details)))) +(defmethod pre-cascade-delete Database [_ {:keys [id] :as database}] + (cascade-delete 'metabase.models.table/Table :db_id id) + (cascade-delete 'metabase.models.query/Query :database_id id)) + (defn databases-for-org "Selects the ID and NAME for all databases available to the given org-id." [org-id] diff --git a/src/metabase/models/emailreport.clj b/src/metabase/models/emailreport.clj index b08f154cc7300d5c0e6aa17ff4fefaecc5940e4d..182e84b21eae5650cb92ace905842bae6696be1b 100644 --- a/src/metabase/models/emailreport.clj +++ b/src/metabase/models/emailreport.clj @@ -53,23 +53,20 @@ (defmethod pre-update EmailReport [_ {:keys [version dataset_query schedule] :as report}] (assoc report - :updated_at (util/new-sql-timestamp) + :updated_at (util/new-sql-timestamp) :dataset_query (json/write-str dataset_query) - :schedule (json/write-str schedule) - :version (+ 1 version))) + :schedule (json/write-str schedule) + :version (inc version))) (defmethod post-select EmailReport [_ {:keys [id creator_id organization_id] :as report}] (-> report (realize-json :dataset_query :schedule) (util/assoc* - :creator (delay - (check creator_id 500 "Can't get creator: Query doesn't have a :creator_id.") - (sel :one User :id creator_id)) - :organization (delay - (check organization_id 500 "Can't get database: Query doesn't have a :database_id.") - (sel :one Org :id organization_id)) - :recipients (delay - (sel :many User - (where {:id [in (subselect EmailReportRecipients (fields :user_id) (where {:emailreport_id id}))]})))) + :creator (delay (check creator_id 500 "Can't get creator: Query doesn't have a :creator_id.") + (sel :one User :id creator_id)) + :organization (delay (check organization_id 500 "Can't get database: Query doesn't have a :database_id.") + (sel :one Org :id organization_id)) + :recipients (delay (sel :many User + (where {:id [in (subselect EmailReportRecipients (fields :user_id) (where {:emailreport_id id}))]})))) assoc-permissions-sets)) diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index eb6613c37e09ce91a58286c32fc1db8a20f65721..b251a700b376125ba4ab785a2a84862dd6726b11 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -68,11 +68,11 @@ (defmethod post-select Field [_ {:keys [table_id] :as field}] (util/assoc* field - :table (sel-fn :one "metabase.models.table/Table" :id table_id) - :db (delay ((:db ((:table <>))))) - :can_read (delay @(:can_read ((:table <>)))) - :can_write (delay @(:can_write ((:table <>)))) - :count (delay (field-count <>)) + :table (delay (sel :one 'metabase.models.table/Table :id table_id)) + :db (delay @(:db @(:table <>))) + :can_read (delay @(:can_read @(:table <>))) + :can_write (delay @(:can_write @(:table <>))) + :count (delay (field-count <>)) :distinct-count (delay (field-distinct-count <>)))) (defmethod pre-insert Field [_ field] @@ -84,6 +84,6 @@ :position 0}] (let [{:keys [field_type base_type special_type] :as field} (merge defaults field)] (assoc field - :base_type (name base_type) + :base_type (name base_type) :special_type (when special_type (name special_type)) - :field_type (name field_type))))) + :field_type (name field_type))))) diff --git a/src/metabase/models/org_perm.clj b/src/metabase/models/org_perm.clj index bc87ce8668e56a94d7ba605cf18de85c3fc4d6b6..3cd2b9ce8251858bcf9ea40b693eacad07179b25 100644 --- a/src/metabase/models/org_perm.clj +++ b/src/metabase/models/org_perm.clj @@ -10,8 +10,8 @@ (defmethod post-select OrgPerm [_ {:keys [organization_id user_id] :as org-perm}] (assoc org-perm - :organization (sel-fn :one Org :id organization_id) - :user (sel-fn :one "metabase.models.user/User" :id user_id))) + :organization (delay (sel :one Org :id organization_id)) + :user (delay (sel :one 'metabase.models.user/User :id user_id)))) (defn grant-org-perm diff --git a/src/metabase/models/query.clj b/src/metabase/models/query.clj index 68ff6f397ac0e9dfca39ae97046a440d9289665d..311350c731268fed2bed206fccfb95f9765a849d 100644 --- a/src/metabase/models/query.clj +++ b/src/metabase/models/query.clj @@ -52,3 +52,6 @@ (sel :one Database :id database_id)) :organization_id (delay (:organization_id @(:database <>)))) assoc-permissions-sets)) + +(defmethod pre-cascade-delete Query [_ {:keys [id] :as query}] + (cascade-delete 'metabase.models.query-execution/QueryExecution :query_id id)) diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj index a96365dc2df2f508f1b3db580407e247eb5b59dd..60751f2daaf207b5f22094cfa4840a53253ff652 100644 --- a/src/metabase/models/table.clj +++ b/src/metabase/models/table.clj @@ -18,7 +18,7 @@ [{:keys [name db]}] {:table name :pk :id - :db @(:korma-db (db))}) + :db @(:korma-db @db)}) (defn korma-count [{:keys [korma-entity]}] (-> @korma-entity @@ -37,14 +37,17 @@ (defmethod post-select Table [_ {:keys [id db db_id name] :as table}] (util/assoc* table - :db (or db (sel-fn :one db/Database :id db_id)) ; Check to see if `:db` is already set. In some cases we add a korma transform fn to `Table` - :fields (sel-fn :many Field :table_id id) ; and assoc :db if the DB has already been fetched, so we can re-use its DB connections. - :jdbc-columns (delay (jdbc-columns ((:db <>)) name)) - :can_read (delay @(:can_read ((:db <>)))) - :can_write (delay @(:can_write ((:db <>)))) + :db (or db (delay (sel :one db/Database :id db_id))) ; Check to see if `:db` is already set. In some cases we add a korma transform fn to `Table` + :fields (delay (sel :many Field :table_id id)) ; and assoc :db if the DB has already been fetched, so we can re-use its DB connections. + :jdbc-columns (delay (jdbc-columns @(:db <>) name)) + :can_read (delay @(:can_read @(:db <>))) + :can_write (delay @(:can_write @(:db <>))) :korma-entity (delay (korma-entity <>)))) (defmethod pre-insert Table [_ table] (assoc table :created_at (util/new-sql-timestamp) :updated_at (util/new-sql-timestamp))) + +(defmethod pre-cascade-delete Table [_ {:keys [id] :as table}] + (cascade-delete Field :table_id id)) diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj index 11248420534aba5d9ac39e1f5811ca5f583a6235..d4ce0e1241da4374c20b990a4cff66cbb9c59329 100644 --- a/src/metabase/models/user.clj +++ b/src/metabase/models/user.clj @@ -28,9 +28,10 @@ (defn user-perms-for-org "Return the permissions level User with USER-ID has for Org with ORG-ID. - nil -> no permissions - :default -> default permissions - :admin -> admin permissions" + + nil -> no permissions + :default -> default permissions + :admin -> admin permissions" [user-id org-id] (when-let [{superuser? :is_superuser} (sel :one [User :is_superuser] :id user-id)] (if superuser? :admin @@ -39,9 +40,9 @@ (defmethod post-select User [_ {:keys [id] :as user}] (-> user - (assoc :org_perms (sel-fn :many OrgPerm :user_id id) + (assoc :org_perms (delay (sel :many OrgPerm :user_id id)) :perms-for-org (memoize (partial user-perms-for-org id)) - :common_name (str (:first_name user) " " (:last_name user))))) + :common_name (str (:first_name user) " " (:last_name user))))) (defmethod pre-insert User [_ {:keys [email password] :as user}] (assert (util/is-email? email)) diff --git a/test/metabase/test_data.clj b/test/metabase/test_data.clj index 82eba96d058a650af0cfdf7f0ccf2a3f1731dc19..d42af92e19b550e89eb85c897397e8a7375054cd 100644 --- a/test/metabase/test_data.clj +++ b/test/metabase/test_data.clj @@ -164,12 +164,12 @@ :private true} tables (delay + @test-db ; force lazy evaluation of Test DB (binding [*log-db-calls* false] - (letfn [(table-kw->table-id [table-kw] - (->> (-> table-kw name .toUpperCase) - (sel :one [Table :id] :db_id @db-id :name) - :id))] - (map-table-kws table-kw->table-id))))) + (map-table-kws (fn [table-kw] + (->> (-> table-kw name .toUpperCase) + (sel :one [Table :id] :db_id @db-id :name) + :id)))))) (def ^{:doc "A map of Table name keywords -> map of Field name keywords -> Field IDs. @@ -180,16 +180,16 @@ :private true} table-fields (delay + @test-db ; force lazy evaluation of Test DB (binding [*log-db-calls* false] - (letfn [(table-kw->fields [table-kw] - (->> (sel :many [Field :name :id] :table_id (@tables table-kw)) - (map (fn [{:keys [^String name id]}] - {:pre [(string? name) - (integer? id) - (not (zero? id))]} - {(keyword (.toLowerCase name)) id})) - (reduce merge {})))] - (map-table-kws table-kw->fields))))) + (map-table-kws (fn [table-kw] + (->> (sel :many [Field :name :id] :table_id (@tables table-kw)) + (map (fn [{:keys [^String name id]}] + {:pre [(string? name) + (integer? id) + (not (zero? id))]} + {(keyword (.toLowerCase name)) id})) + (reduce merge {}))))))) ;; ## Users diff --git a/test/metabase/test_data/load.clj b/test/metabase/test_data/load.clj index 586fdeec3d7a8e1c7e2eed3680fb686e3d001f88..6ce19645fa1a72c216c53253c805ec2f51c3fe61 100644 --- a/test/metabase/test_data/load.clj +++ b/test/metabase/test_data/load.clj @@ -59,13 +59,8 @@ "Drop the test `Database` and `Fields`/`Tables` associated with it." [] (when-let [{:keys [id]} (sel :one [Database :id] :name db-name)] - (let [table-ids (->> (sel :many [Table :id] :db_id id) - (map :id) - set)] - (delete Field (where {:table_id [in table-ids]})) - (delete Table (where {:id [in table-ids]})) - (del Database :id id) - (recur)))) ; recurse and delete any other DBs named db-name in case we somehow created more than one + (cascade-delete Database :id id) + (recur))) ; recurse and delete any other DBs named db-name in case we somehow created more than one ;; # INTERNAL IMPLENTATION DETAILS