New `key-by` util function; eliminate some `select` variations

src/metabase/api/card.clj

@@ -21,7 +21,7 @@
   [cards]
   (let [card-labels          (db/select [CardLabel :card_id :label_id])
         label-id->label      (when (seq card-labels)
-                               (db/select-id->object Label :id [:in (map :label_id card-labels)]))
+                               (u/key-by :id (db/select Label :id [:in (map :label_id card-labels)])))
         card-id->card-labels (group-by :card_id card-labels)]
     (for [card cards]
       (assoc card :labels (for [card-label (card-id->card-labels (:id card))] ; TODO - do these need to be sorted ?
@@ -68,7 +68,7 @@
    Make sure cards are returned in the same order as CARD-IDS`; `[in card-ids]` won't preserve the order."
   [card-ids]
   {:pre [(every? integer? card-ids)]}
-  (let [card-id->card (db/select-id->object Card, :id [:in (set card-ids)], :archived false)]
+  (let [card-id->card (u/key-by :id (db/select Card, :id [:in (set card-ids)], :archived false))]
     (filter identity (map card-id->card card-ids))))
 
 (defn- cards:recent

src/metabase/api/database.clj

@@ -55,7 +55,7 @@
   (let [dbs (db/select Database {:order-by [:%lower.name]})]
     (if-not include_tables
       dbs
-      (let [db-id->tables (db/select-field->objects :db_id Table, :active true)]
+      (let [db-id->tables (group-by :db_id (db/select Table, :active true))]
         (for [db dbs]
           (assoc db :tables (sort-by :name (get db-id->tables (:id db) []))))))))
 

src/metabase/db.clj

@@ -601,34 +601,6 @@
   [entity & options]
   (apply select-field :id entity options))
 
-(defn select-field->object
-  "Select objects from the database, and return them as a map of FIELD to the objects themselves.
-
-     (db/select-field->object :name 'Database) -> {\"Sample Dataset\" {...}, \"test-data\" {...}}"
-  {:style/indent 2}
-  [field entity & options]
-  {:pre [(keyword? field)]}
-  (into {} (for [result (apply select entity options)]
-             {(field result) result})))
-
-(defn select-field->objects
-  "Select objects from the database, and return a map of distinct values of FIELD to objects having that value.
-
-     ;; get a map of database ID -> tables with that database ID
-     (db/select-field->objects :db_id 'Table) -> 1 [...], 2 [...]}"
-  {:style/indent 2}
-  [field entity & options]
-  {:pre [(keyword? field)]}
-  (group-by field (apply select entity options)))
-
-(defn select-id->object
-  "Select objects from the database, and return them as a map of their `:id` to the objects themselves.
-
-     (db/select-id->object 'Database) -> {1 {...}, 2 {...}}"
-  {:style/indent 1}
-  [entity & options]
-  (apply select-field->object :id entity options))
-
 (defn select-field->field
   "Select fields K and V from objects in the database, and return them as a map from K to V.
 

src/metabase/models/hydrate.clj

@@ -181,7 +181,7 @@
                                :when  (not (get result dest-key))]
                            (source-key result)))
          objs       (when (seq ids)
-                      (db/select-id->object entity, :id [:in ids]))]
+                      (u/key-by :id (db/select entity, :id [:in ids])))]
      (for [{source-id source-key :as result} results]
        (if (get result dest-key)
          result

src/metabase/query_processor/annotate.clj

@@ -219,8 +219,8 @@
   ;; Fetch the destination Fields referenced by the ForeignKeys
   ([fields fk-ids id->dest-id]
    (when (seq id->dest-id)
-     (fk-field->dest-fn fields fk-ids id->dest-id (db/select-id->object [Field :id :name :display_name :table_id :description :base_type :special_type :visibility_type]
-                                                    :id [:in (vals id->dest-id)]))))
+     (fk-field->dest-fn fields fk-ids id->dest-id (u/key-by :id (db/select [Field :id :name :display_name :table_id :description :base_type :special_type :visibility_type]
+                                                                  :id [:in (vals id->dest-id)])))))
   ;; Return a function that will return the corresponding destination Field for a given Field
   ([fields fk-ids id->dest-id dest-id->field]
    (fn [{:keys [id]}]

src/metabase/query_processor/resolve.clj

@@ -183,8 +183,8 @@
         ;; If there are no more Field IDs to resolve we're done.
         expanded-query-dict
         ;; Otherwise fetch + resolve the Fields in question
-        (let [fields (->> (db/select-id->object [field/Field :name :display_name :base_type :special_type :visibility_type :table_id :parent_id :description :id]
-                            :id [:in field-ids])
+        (let [fields (->> (u/key-by :id (db/select [field/Field :name :display_name :base_type :special_type :visibility_type :table_id :parent_id :description :id]
+                                          :id [:in field-ids]))
                           (m/map-vals rename-mb-field-keys)
                           (m/map-vals #(assoc % :parent (when-let [parent-id (:parent-id %)]
                                                           (map->FieldPlaceholder {:field-id parent-id})))))]
@@ -236,8 +236,8 @@
   [{{source-table-id :source-table} :query, :keys [table-ids fk-field-ids], :as expanded-query-dict}]
   {:pre [(integer? source-table-id)]}
   (let [table-ids       (conj table-ids source-table-id)
-        table-id->table (db/select-id->object [Table :schema :name :id]
-                          :id [:in table-ids])
+        table-id->table (u/key-by :id (db/select [Table :schema :name :id]
+                                        :id [:in table-ids]))
         join-tables     (vals (dissoc table-id->table source-table-id))]
     (as-> expanded-query-dict <>
       (assoc-in <> [:query :source-table] (or (table-id->table source-table-id)

src/metabase/sync_database/sync.clj

@@ -73,7 +73,7 @@
   [{table-id :id, raw-table-id :raw_table_id}]
   (let [active-raw-columns   (raw-table/active-columns {:id raw-table-id})
         active-column-ids    (set (map :id active-raw-columns))
-        raw-column-id->field (db/select-field->object :raw_column_id Field, :table_id table-id, :visibility_type [:not= "retired"], :parent_id nil)]
+        raw-column-id->field (u/key-by :raw_column_id (db/select Field, :table_id table-id, :visibility_type [:not= "retired"], :parent_id nil))]
     ;; retire any fields which were disabled in the schema (including child nested fields)
     (doseq [[raw-column-id {field-id :id}] raw-column-id->field]
       (when-not (contains? active-column-ids raw-column-id)
@@ -214,7 +214,7 @@
   (retire-tables! database)
 
   (let [raw-tables          (raw-table/active-tables database-id)
-        raw-table-id->table (db/select-field->object :raw_table_id Table, :db_id database-id, :active true)]
+        raw-table-id->table (u/key-by :raw_table_id (db/select Table, :db_id database-id, :active true))]
     (create-and-update-tables! database raw-table-id->table raw-tables)
     (set-fk-relationships! database)
     (maybe-sync-metabase-metadata-table! database raw-tables)))

src/metabase/sync_database/sync_dynamic.clj

@@ -19,7 +19,7 @@
    All field-defs provided are assumed to be children of the given FIELD."
   [{parent-id :id, table-id :table_id, :as parent-field} nested-field-defs]
   ;; NOTE: remember that we never retire any fields in dynamic-schema tables
-  (let [existing-field-name->field (db/select-field->object :name field/Field, :parent_id parent-id)]
+  (let [existing-field-name->field (u/key-by :name (db/select field/Field, :parent_id parent-id))]
     (u/prog1 (set/difference (set (map :name nested-field-defs)) (set (keys existing-field-name->field)))
       (when (seq <>)
         (log/debug (u/format-color 'blue "Found new nested fields for field '%s': %s" (:name parent-field) <>))))
@@ -43,7 +43,7 @@
   {:pre [(integer? table-id)
          (coll? field-defs)
          (every? map? field-defs)]}
-  (let [field-name->field (db/select-field->object :name field/Field, :table_id table-id, :parent_id nil)]
+  (let [field-name->field (u/key-by :name (db/select field/Field, :table_id table-id, :parent_id nil))]
     ;; NOTE: with dynamic schemas we never disable fields
     ;; create/update the fields
     (doseq [{field-name :name, :keys [nested-fields], :as field-def} field-defs]
@@ -87,7 +87,7 @@
   (sync/retire-tables! database)
 
   (let [raw-tables          (raw-table/active-tables database-id)
-        raw-table-id->table (db/select-field->object :raw_table_id table/Table, :db_id database-id, :active true)]
+        raw-table-id->table (u/key-by :raw_table_id (db/select table/Table, :db_id database-id, :active true))]
     ;; create/update tables (and their fields)
     ;; NOTE: we make sure to skip the _metabase_metadata table here.  it's not a normal table.
     (doseq [{raw-table-id :id, :as raw-tbl} (filter #(not= "_metabase_metadata" (s/lower-case (:name %))) raw-tables)]

src/metabase/util.clj

@@ -611,7 +611,7 @@
   "Execute F, a function that takes no arguments, and return the results.
    If F fails with an exception, retry F up to NUM-RETRIES times until it succeeds.
 
-    Consider using the `auto-retry` macro instead of calling this function directly."
+   Consider using the `auto-retry` macro instead of calling this function directly."
   {:style/indent 1}
   [num-retries f]
   (if (<= num-retries 0)
@@ -634,3 +634,14 @@
   [x]
   (or (string? x)
       (keyword? x)))
+
+(defn key-by
+  "Convert a sequential COLL to a map of `(f item)` -> `item`.
+   This is similar to `group-by`, but the resultant map's values are single items from COLL rather than sequences of items.
+   (Because only a single item is kept for each value of `f`,  items producing duplicate values will be discarded).
+
+     (key-by :id [{:id 1, :name :a} {:id 2, :name :b}]) -> {1 {:id 1, :name :a}, 2 {:id 2, :name :b}}"
+  {:style/indent 1}
+  [f coll]
+  (into {} (for [item coll]
+             {(f item) item})))

test/metabase/util_test.clj

@@ -102,3 +102,11 @@
   (cond-as-> 100 <>
     (even? <>) (inc <>)
     (odd? <>)  (inc <>)))
+
+
+;;; TESTS FOR key-by
+(expect
+  {1 {:id 1, :name "Rasta"}
+   2 {:id 2, :name "Lucky"}}
+  (key-by :id [{:id 1, :name "Rasta"}
+               {:id 2, :name "Lucky"}]))