~ remove search-filters-for-model

src/metabase/api/search.clj

@@ -15,7 +15,7 @@
   "Get the set of models that a search query will return"
   [q archived table-db-id created_at created_by last_edited_at last_edited_by
    filter_items_in_personal_collection search_native_query verified]
-  {archived            [:maybe ms/BooleanValue]
+  {archived            [:maybe true?]
    table-db-id         [:maybe ms/PositiveInt]
    created_at          [:maybe ms/NonBlankString]
    created_by          [:maybe (ms/QueryVectorOf ms/PositiveInt)]

src/metabase/driver/sql_jdbc.clj

@@ -126,7 +126,7 @@
   [_driver _semantic_type expr]
   (h2x/->timestamp expr))
 
-(defn- create-table!-sql
+(defn create-table!-sql
   [driver table-name column-definitions & {:keys [primary-key]}]
   (first (sql/format {:create-table (keyword table-name)
                       :with-columns (cond-> (mapv (fn [[name type-spec]]

src/metabase/models.clj

@@ -38,6 +38,7 @@
    [metabase.models.query-execution :as query-execution]
    [metabase.models.query-field :as query-field]
    [metabase.models.revision :as revision]
+   [metabase.models.searchable :as searchable]
    [metabase.models.secret :as secret]
    [metabase.models.segment :as segment]
    [metabase.models.session :as session]
@@ -92,6 +93,7 @@
          query-execution/keep-me
          query-field/keep-me
          revision/keep-me
+         searchable/keep-me
          secret/keep-me
          segment/keep-me
          session/keep-me

src/metabase/models/card.clj

@@ -45,6 +45,7 @@
     :refer [defenterprise]]
    [metabase.query-processor.async :as qp.async]
    [metabase.query-processor.util :as qp.util]
+   [metabase.search.impl :as search.impl]
    [metabase.server.middleware.session :as mw.session]
    [metabase.shared.util.i18n :refer [trs]]
    [metabase.util :as u]
@@ -550,6 +551,13 @@
   (public-settings/remove-public-uuid-if-public-sharing-is-disabled
    (dissoc card :dataset_query_metrics_v2_migration_backup)))
 
+;; Maybe this should be after-delete, but that doesn't exist for toucan yet
+(t2/define-before-delete :model/Card
+  [card]
+  (search.impl/index {:change-type  :update
+                      :id           (:id card)
+                      :search-model "card"}))
+
 (t2/define-before-insert :model/Card
   [card]
   (-> card
@@ -566,7 +574,11 @@
       (log/info "Card references Fields in params:" field-ids)
       (field-values/update-field-values-for-on-demand-dbs! field-ids))
     (parameter-card/upsert-or-delete-from-parameters! "card" (:id card) (:parameters card))
-    (query-field/update-query-fields-for-card! card)))
+    (query-field/update-query-fields-for-card! card)
+    (search.impl/index {:change-type  :insert
+                        :id           (:id card)
+                        :search-model "card"
+                        :values       card})))
 
 (t2/define-before-update :model/Card
   [{:keys [verified-result-metadata?] :as card}]
@@ -594,7 +606,11 @@
   [card]
   (u/prog1 card
     (when (contains? (t2/changes card) :dataset_query)
-      (query-field/update-query-fields-for-card! card))))
+      (query-field/update-query-fields-for-card! card))
+    (search.impl/index {:change-type  :update
+                        :id           (:id card)
+                        :search-model "card"
+                        :values       card})))
 
 ;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests
 (t2/define-before-delete :model/Card

src/metabase/models/searchable.clj

@@ -0,0 +1,10 @@
+(ns metabase.models.searchable
+  (:require
+   [methodical.core :as methodical]
+   [toucan2.core :as t2]))
+
+(set! *warn-on-reflection* true)
+
+(methodical/defmethod t2/table-name :model/Searchable [_model] :search)
+
+(derive :model/Searchable :metabase/model)

src/metabase/search/config.clj

@@ -81,7 +81,7 @@
   [model]
   (-> model model-to-db-model :alias))
 
-(mu/defn column-with-model-alias :- keyword?
+(mu/defn column-with-alias :- keyword?
   "Given a column and a model name, Return a keyword representing the column with the model alias prepended.
 
   (column-with-model-alias \"card\" :id) => :card.id)"
@@ -99,7 +99,6 @@
    ;;
    ;; required
    ;;
-   [:archived?          [:maybe :boolean]]
    [:current-user-id    pos-int?]
    [:current-user-perms [:set perms.u/PathSchema]]
    [:model-ancestors?   :boolean]
@@ -108,6 +107,7 @@
    ;;
    ;; optional
    ;;
+   [:archived                            {:optional true} true?]
    [:created-at                          {:optional true} ms/NonBlankString]
    [:created-by                          {:optional true} [:set {:min 1} ms/PositiveInt]]
    [:filter-items-in-personal-collection {:optional true} [:enum "only" "exclude"]]
@@ -135,6 +135,8 @@
   `UNION` two columns of two different types.)"
   (ordered-map/ordered-map
    ;; returned for all models. Important to be first for changing model for dataset
+   :query_type          :text
+   :type                :text
    :model               :text
    :id                  :integer
    :name                :text
@@ -186,64 +188,38 @@
   "All of the result components that by default are displayed by the frontend."
   #{:name :display_name :collection_name :description})
 
+(defmulti native-query-columns
+  "The native query columns that will be searched if the `:search-native-query` option is true."
+  {:arglists '([model])}
+  (fn [model] model))
+
+(defmethod native-query-columns :default   [_] [])
+(defmethod native-query-columns "card"     [_] [:dataset_query])
+(defmethod native-query-columns "dataset"  [_] (native-query-columns "card"))
+(defmethod native-query-columns "metric"   [_] (native-query-columns "card"))
+(defmethod native-query-columns "action"   [_] [:dataset_query])
+
 (defmulti searchable-columns-for-model
   "The columns that will be searched for the query."
   {:arglists '([model])}
   (fn [model] model))
 
-(defmethod searchable-columns-for-model :default
-  [_]
-  [:name])
+(defmethod searchable-columns-for-model :default         [_] [:name])
+(defmethod searchable-columns-for-model "action"         [_] [:name :description])
+(defmethod searchable-columns-for-model "card"           [_] [:name :description])
+(defmethod searchable-columns-for-model "dataset"        [_] (searchable-columns-for-model "card"))
+(defmethod searchable-columns-for-model "metric"         [_] (searchable-columns-for-model "card"))
+(defmethod searchable-columns-for-model "dashboard"      [_] [:name :description])
+(defmethod searchable-columns-for-model "page"           [_] (searchable-columns-for-model "dashboard"))
+(defmethod searchable-columns-for-model "database"       [_] [:name :description])
+(defmethod searchable-columns-for-model "table"          [_] [:name :description :display_name])
+(defmethod searchable-columns-for-model "indexed-entity" [_] [:name])
 
-(defmethod searchable-columns-for-model "action"
-  [_]
-  [:name
-   :dataset_query
-   :description])
-
-(defmethod searchable-columns-for-model "card"
-  [_]
-  [:name
-   :dataset_query
-   :description])
-
-(defmethod searchable-columns-for-model "dataset"
-  [_]
-  (searchable-columns-for-model "card"))
-
-(defmethod searchable-columns-for-model "metric"
-  [_]
-  (searchable-columns-for-model "card"))
-
-(defmethod searchable-columns-for-model "dashboard"
-  [_]
-  [:name
-   :description])
-
-(defmethod searchable-columns-for-model "page"
-  [_]
-  (searchable-columns-for-model "dashboard"))
-
-(defmethod searchable-columns-for-model "database"
-  [_]
-  [:name
-   :description])
-
-(defmethod searchable-columns-for-model "table"
-  [_]
-  [:name
-   :display_name
-   :description])
-
-(defmethod searchable-columns-for-model "indexed-entity"
-  [_]
-  [:name])
-
-(def ^:private default-columns
+(def default-columns
   "Columns returned for all models."
   [:id :name :description :archived :created_at :updated_at])
 
-(def ^:private bookmark-col
+(def bookmark-col
   "Case statement to return boolean values of `:bookmark` for Card, Collection and Dashboard."
   [[:case [:not= :bookmark.id nil] true :else false] :bookmark])
 

src/metabase/search/filter.clj

@@ -13,149 +13,73 @@
    [clojure.set :as set]
    [clojure.string :as str]
    [honey.sql.helpers :as sql.helpers]
-   [metabase.audit :as audit]
    [metabase.driver.common.parameters.dates :as params.dates]
    [metabase.public-settings.premium-features :as premium-features]
-   [metabase.search.config
-    :as search.config
-    :refer [SearchableModel SearchContext]]
-   [metabase.search.util :as search.util]
+   [metabase.search.config :as search.config :refer [SearchableModel SearchContext]]
    [metabase.util.date-2 :as u.date]
    [metabase.util.i18n :refer [tru]]
    [metabase.util.malli :as mu])
   (:import
    (java.time LocalDate)))
 
-(def ^:private true-clause [:inline [:= 1 1]])
-(def ^:private false-clause [:inline [:= 0 1]])
-
-;; ------------------------------------------------------------------------------------------------;;
-;;                                         Required Filters                                         ;
-;; ------------------------------------------------------------------------------------------------;;
-
-(defmulti ^:private archived-clause
-  "Clause to filter by the archived status of the entity."
-  {:arglists '([model archived?])}
-  (fn [model _] model))
-
-(defmethod archived-clause :default
-  [model archived?]
-  [:= (search.config/column-with-model-alias model :archived) archived?])
-
-;; Databases can't be archived
-(defmethod archived-clause "database"
-  [_model archived?]
-  (if archived?
-    false-clause
-    true-clause))
-
-(defmethod archived-clause "indexed-entity"
-  [_model archived?]
-  (if-not archived?
-    true-clause
-    false-clause))
-
-;; Table has an `:active` flag, but no `:archived` flag; never return inactive Tables
-(defmethod archived-clause "table"
-  [model archived?]
-  (if archived?
-    false-clause ; No tables should appear in archive searches
-    [:and
-     [:= (search.config/column-with-model-alias model :active) true]
-     [:= (search.config/column-with-model-alias model :visibility_type) nil]]))
-
-(defn- sandboxed-or-impersonated-user? []
-  ;; TODO FIXME -- search actually currently still requires [[metabase.api.common/*current-user*]] to be bound,
-  ;; because [[metabase.public-settings.premium-features/sandboxed-or-impersonated-user?]] requires it to be bound.
-  ;; Since it's part of the search context it would be nice if we could run search without having to bind that stuff at
-  ;; all.
-  (assert @@(requiring-resolve 'metabase.api.common/*current-user*)
-          "metabase.api.common/*current-user* must be bound in order to use search for an indexed entity")
-  (premium-features/sandboxed-or-impersonated-user?))
-
-(mu/defn ^:private search-string-clause-for-model
-  [model                :- SearchableModel
-   search-context       :- SearchContext
-   search-native-query? :- [:maybe :boolean]]
-  (when-let [query (:search-string search-context)]
-    (into
-     [:or]
-     (for [column           (cond->> (search.config/searchable-columns-for-model model)
-                              (not search-native-query?)
-                              (remove #{:dataset_query})
-
-                              true
-                              (map #(search.config/column-with-model-alias model %)))
-           wildcarded-token (->> (search.util/normalize query)
-                                 search.util/tokenize
-                                 (map search.util/wildcard-match))]
-       (cond
-         (and (= model "indexed-entity") (sandboxed-or-impersonated-user?))
-         [:= 0 1]
-
-         (and (#{"card" "dataset"} model) (= column (search.config/column-with-model-alias model :dataset_query)))
-         [:and
-          [:= (search.config/column-with-model-alias model :query_type) "native"]
-          [:like [:lower column] wildcarded-token]]
-
-         (and (#{"action"} model)
-              (= column (search.config/column-with-model-alias model :dataset_query)))
-         [:like [:lower :query_action.dataset_query] wildcarded-token]
-
-         :else
-         [:like [:lower column] wildcarded-token])))))
+(def false-clause
+  "A clause which is always false. Useful for when you want to return no results."
+  [:inline [:= 0 1]])
 
 ;; ------------------------------------------------------------------------------------------------;;
 ;;                                         Optional filters                                        ;;
 ;; ------------------------------------------------------------------------------------------------;;
 
-(defmulti ^:private build-optional-filter-query
+(defmulti ^:private build-filter
   "Build the query to filter by `filter`.
-  Dispatch with an array of [filter model-name]."
+  Dispatch with an array of [filter model-name].
+  `filter-value` may be nil if the filter is not in the search context."
   {:arglists '([model filter query filter-value])}
   (fn [filter model _query _filter-value]
     [filter model]))
 
-(defmethod build-optional-filter-query :default
-  [filter model _query _filter-value]
-  (throw (ex-info (format "%s filter for %s is not supported" filter model) {:filter filter :model model})))
+(defmethod build-filter :default
+  [filter model query filter-value]
+  (when (some? filter-value)
+    (throw (ex-info (format "%s filter for %s is not supported" filter model) {:filter filter :model model})))
+  query)
 
-;; Created by filters
-(defn- default-created-by-filter-clause
-  [model creator-ids]
-  (if (= 1 (count creator-ids))
-    [:= (search.config/column-with-model-alias model :creator_id) (first creator-ids)]
-    [:in (search.config/column-with-model-alias model :creator_id) creator-ids]))
+;; These models have an `archive` column and they can be archived
+(doseq [model ["dashboard" "metric" "segment" "card" "dataset" "collection" "action"]]
+  (defmethod build-filter [:archived model]
+    [_filter _model query archived]
+    (sql.helpers/where query [:= :archived (boolean archived)])))
+
+; created-by
 
 (doseq [model ["card" "dataset" "metric" "dashboard" "action"]]
-  (defmethod build-optional-filter-query [:created-by model]
+  (defmethod build-filter [:created-by model]
     [_filter model query creator-ids]
-    (sql.helpers/where query (default-created-by-filter-clause model creator-ids))))
+    (cond-> query (seq creator-ids)
+      (sql.helpers/where (if (= 1 (count creator-ids))
+                           [:= (search.config/column-with-alias model :creator_id) (first creator-ids)]
+                           [:in (search.config/column-with-alias model :creator_id) creator-ids])))))
 
-;; Verified filters
+;; verified filters
 
-(defmethod build-optional-filter-query [:verified "card"]
-  [_filter model query verified]
-  (assert (true? verified) "filter for non-verified cards is not supported")
-  (if (premium-features/has-feature? :content-verification)
-    (-> query
-        (sql.helpers/join :moderation_review
-                          [:= :moderation_review.moderated_item_id
-                           (search.config/column-with-model-alias model :id)])
-        (sql.helpers/where [:= :moderation_review.status "verified"]
-                           [:= :moderation_review.moderated_item_type "card"]
-                           [:= :moderation_review.most_recent true]))
-    (sql.helpers/where query false-clause)))
+(doseq [model ["card" "dataset" "metric"]]
+  (defmethod build-filter [:verified model]
+    [_filter model query verified]
+    (if (nil? verified)
+      query
+      (do
+        (assert (true? verified) "filter for non-verified cards is not supported")
+        (if (premium-features/has-feature? :content-verification)
+          (-> query
+              (sql.helpers/join :moderation_review
+                                [:= :moderation_review.moderated_item_id
+                                 (search.config/column-with-alias model :id)])
+              (sql.helpers/where [:= :moderation_review.status "verified"]
+                                 [:= :moderation_review.moderated_item_type "card"]
+                                 [:= :moderation_review.most_recent true]))
+          (sql.helpers/where query false-clause))))))
 
-(defmethod build-optional-filter-query [:verified "dataset"]
-  [filter _model query verified]
-  (build-optional-filter-query filter "card" query verified))
-
-(defmethod build-optional-filter-query [:verified "metric"]
-  [filter _model query verified]
-  (build-optional-filter-query filter "card" query verified))
-
-;; Created at filters
+;; created-at filters
 
 (defn- date-range-filter-clause
   [dt-col dt-val]
@@ -182,13 +106,15 @@
      [:and [:>= dt-col start] [:< dt-col end]])))
 
 (doseq [model ["collection" "database" "table" "dashboard" "card" "dataset" "metric" "action"]]
-  (defmethod build-optional-filter-query [:created-at model]
+  (defmethod build-filter [:created-at model]
     [_filter model query created-at]
-    (sql.helpers/where query (date-range-filter-clause
-                              (search.config/column-with-model-alias model :created_at)
-                              created-at))))
+    (cond-> query
+      created-at
+      (sql.helpers/where (date-range-filter-clause
+                          (search.config/column-with-alias model :created_at)
+                          created-at)))))
 
-;; Last edited by filter
+;; last-edited-by filter
 
 (defn- joined-with-table?
   "Check if  the query have a join with `table`.
@@ -212,41 +138,56 @@
     (str/capitalize model)))
 
 (doseq [model ["dashboard" "card" "dataset" "metric"]]
-  (defmethod build-optional-filter-query [:last-edited-by model]
+  (defmethod build-filter [:last-edited-by model]
     [_filter model query editor-ids]
-    (cond-> query
-      ;; both last-edited-by and last-edited-at join with revision, so we should be careful not to join twice
-      (not (joined-with-table? query :join :revision))
-      (-> (sql.helpers/join :revision [:= :revision.model_id (search.config/column-with-model-alias model :id)])
-          (sql.helpers/where [:= :revision.most_recent true]
-                             [:= :revision.model (search.config/search-model->revision-model model)]))
-      (= 1 (count editor-ids))
-      (sql.helpers/where [:= :revision.user_id (first editor-ids)])
+    (if (empty? editor-ids)
+      query
+      (cond-> query
+        ;; both last-edited-by and last-edited-at join with revision, so we should be careful not to join twice
+        (not (joined-with-table? query :join :revision))
+        (-> (sql.helpers/join :revision [:= :revision.model_id (search.config/column-with-alias model :id)])
+            (sql.helpers/where [:= :revision.most_recent true]
+                               [:= :revision.model (search.config/search-model->revision-model model)]))
+        (= 1 (count editor-ids))
+        (sql.helpers/where [:= :revision.user_id (first editor-ids)])
 
-      (> (count editor-ids) 1)
-      (sql.helpers/where [:in :revision.user_id editor-ids]))))
+        (> (count editor-ids) 1)
+        (sql.helpers/where [:in :revision.user_id editor-ids])))))
+
+;; last-edited-at filter
 
 (doseq [model ["dashboard" "card" "dataset" "metric"]]
-  (defmethod build-optional-filter-query [:last-edited-at model]
+  (defmethod build-filter [:last-edited-at model]
     [_filter model query last-edited-at]
-    (cond-> query
-      ;; both last-edited-by and last-edited-at join with revision, so we should be careful not to join twice
-      (not (joined-with-table? query :join :revision))
-      (-> (sql.helpers/join :revision [:= :revision.model_id (search.config/column-with-model-alias model :id)])
-          (sql.helpers/where [:= :revision.most_recent true]
-                             [:= :revision.model (search.config/search-model->revision-model model)]))
-      true
-      ;; on UI we showed the the last edit info from revision.timestamp
-      ;; not the model.updated_at column
-      ;; to be consistent we use revision.timestamp to do the filtering
-      (sql.helpers/where (date-range-filter-clause :revision.timestamp last-edited-at)))))
+    (if-not last-edited-at
+      query
+      (cond-> query
+        ;; both last-edited-by and last-edited-at join with revision, so we should be careful not to join twice
+        (not (joined-with-table? query :join :revision))
+        (-> (sql.helpers/join :revision [:= :revision.model_id (search.config/column-with-alias model :id)])
+            (sql.helpers/where [:= :revision.most_recent true]
+                               [:= :revision.model (search.config/search-model->revision-model model)]))
+        true
+        ;; on UI we showed the the last edit info from revision.timestamp
+        ;; not the model.updated_at column
+        ;; to be consistent we use revision.timestamp to do the filtering
+        (sql.helpers/where (date-range-filter-clause :revision.timestamp last-edited-at))))))
 
 ;; TODO: once we record revision for actions, we should update this to use the same approach with dashboard/card
-(defmethod build-optional-filter-query [:last-edited-at "action"]
+(defmethod build-filter [:last-edited-at "action"]
   [_filter model query last-edited-at]
-  (sql.helpers/where query (date-range-filter-clause
-                              (search.config/column-with-model-alias model :updated_at)
-                              last-edited-at)))
+  (cond-> query
+    last-edited-at
+    (sql.helpers/where (date-range-filter-clause
+                        (search.config/column-with-alias model :updated_at)
+                        last-edited-at))))
+
+(defn- optional-filters->supported-models []
+  (->> (dissoc (methods build-filter) :default)
+       keys
+       (reduce (fn [acc [filter model]]
+                 (update acc filter set/union #{model}))
+               {})))
 
 (defn- feature->supported-models
   "Return A map of filter to its support models.
@@ -256,17 +197,11 @@
   This is function instead of a def so that optional-filter-clause can be defined anywhere in the codebase."
   []
   (merge
-   ;; models support search-native-query if dataset_query is one of the searchable columns
-   {:search-native-query (->> (dissoc (methods search.config/searchable-columns-for-model) :default)
-                              (filter (fn [[k v]]
-                                        (contains? (set (v k)) :dataset_query)))
+   ;; models support search-native-query if the model has a native query column
+   {:search-native-query (->> (dissoc (methods search.config/native-query-columns) :default)
                               (map first)
                               set)}
-   (->> (dissoc (methods build-optional-filter-query) :default)
-        keys
-        (reduce (fn [acc [filter model]]
-                  (update acc filter set/union #{model}))
-                {}))))
+   (optional-filters->supported-models)))
 
 ;; ------------------------------------------------------------------------------------------------;;
 ;;                                        Public functions                                         ;;
@@ -277,58 +212,33 @@
 
   If the context has optional filters, the models will be restricted for the set of supported models only."
   [search-context :- SearchContext]
-  (let [{:keys [created-at
+  (let [{:keys [archived
+                created-at
                 created-by
                 last-edited-at
                 last-edited-by
                 models
                 search-native-query
                 verified]}        search-context
-        feature->supported-models (feature->supported-models)]
+        feature->models (feature->supported-models)]
     (cond-> models
-      (some? created-at)          (set/intersection (:created-at feature->supported-models))
-      (some? created-by)          (set/intersection (:created-by feature->supported-models))
-      (some? last-edited-at)      (set/intersection (:last-edited-at feature->supported-models))
-      (some? last-edited-by)      (set/intersection (:last-edited-by feature->supported-models))
-      (true? search-native-query) (set/intersection (:search-native-query feature->supported-models))
-      (true? verified)            (set/intersection (:verified feature->supported-models)))))
+      (some? archived)            (set/intersection (:archived feature->models))
+      (some? created-at)          (set/intersection (:created-at feature->models))
+      (some? created-by)          (set/intersection (:created-by feature->models))
+      (some? last-edited-at)      (set/intersection (:last-edited-at feature->models))
+      (some? last-edited-by)      (set/intersection (:last-edited-by feature->models))
+      (true? verified)            (set/intersection (:verified feature->models))
+      (true? search-native-query) (set/intersection (:search-native-query feature->models)))))
 
 (mu/defn build-filters :- :map
   "Build the search filters for a model."
-  [honeysql-query :- :map
+  [query          :- :map
    model          :- SearchableModel
    search-context :- SearchContext]
-  (let [{:keys [archived?
-                created-at
-                created-by
-                last-edited-at
-                last-edited-by
-                search-string
-                search-native-query
-                verified]}    search-context]
-    (cond-> honeysql-query
-      (not (str/blank? search-string))
-      (sql.helpers/where (search-string-clause-for-model model search-context search-native-query))
-
-      (some? archived?)
-      (sql.helpers/where (archived-clause model archived?))
-
-      ;; build optional filters
-      (some? created-at)
-      (#(build-optional-filter-query :created-at model % created-at))
-
-      (some? created-by)
-      (#(build-optional-filter-query :created-by model % created-by))
-
-      (some? last-edited-at)
-      (#(build-optional-filter-query :last-edited-at model % last-edited-at))
-
-      (some? last-edited-by)
-      (#(build-optional-filter-query :last-edited-by model % last-edited-by))
-
-      (some? verified)
-      (#(build-optional-filter-query :verified model % verified))
-
-      (= "table" model)
-      (sql.helpers/where
-       [:not [:= (search.config/column-with-model-alias "table" :db_id) audit/audit-db-id]]))))
+  ;; TODO: fix the rest of the filters
+  (let [filters [:archived] #_(keys (optional-filters->supported-models))]
+    (reduce (fn [q filter]
+              (let [v (get search-context filter)]
+                (build-filter filter model q v)))
+            query
+            filters)))

src/metabase/search/impl.clj

@@ -1,10 +1,16 @@
 (ns metabase.search.impl
   (:require
    [cheshire.core :as json]
+   [clojure.set :as set]
+   [clojure.string :as str]
+   [honey.sql :as sql]
    [honey.sql.helpers :as sql.helpers]
    [medley.core :as m]
+   [metabase.audit :as audit]
    [metabase.db :as mdb]
    [metabase.db.query :as mdb.query]
+   [metabase.driver :as driver]
+   [metabase.driver.sql.query-processor :as sql.qp]
    [metabase.legacy-mbql.normalize :as mbql.normalize]
    [metabase.lib.core :as lib]
    [metabase.models.collection :as collection]
@@ -64,7 +70,7 @@
 
       ;; This is a column reference, need to add the table alias to the column
       maybe-aliased-col
-      (search.config/column-with-model-alias model maybe-aliased-col)
+      (search.config/column-with-alias model maybe-aliased-col)
 
       ;; This entity is missing the column, project a null for that column value. For Postgres and H2, cast it to the
       ;; correct type, e.g.
@@ -92,10 +98,30 @@
   (let [{:keys [db-model alias]} (get search.config/model-to-db-model model)]
     [[(t2/table-name db-model) alias]]))
 
+(defn- wildcarded-tokens [s]
+  (->> s
+       search.util/normalize
+       search.util/tokenize
+       (map search.util/wildcard-match)))
+
+(mu/defn ^:private search-string-clause
+  [model
+   search-ctx :- SearchContext]
+  (when (not (str/blank? (:search-string search-ctx)))
+    (into
+     [:or]
+     (for [wildcarded-token (wildcarded-tokens (:search-string search-ctx))
+           column (cond-> (search.config/searchable-columns-for-model model)
+                    (:search-native-query search-ctx)
+                    (concat (search.config/native-query-columns model)))]
+       [:like
+        [:lower column]
+        wildcarded-token]))))
+
 (mu/defn ^:private base-query-for-model :- [:map {:closed true}
                                             [:select :any]
                                             [:from :any]
-                                            [:where :any]
+                                            [:where {:optional true} :any]
                                             [:join {:optional true} :any]
                                             [:left-join {:optional true} :any]]
   "Create a HoneySQL query map with `:select`, `:from`, and `:where` clauses for `model`, suitable for the `UNION ALL`
@@ -103,6 +129,7 @@
   [model :- SearchableModel context :- SearchContext]
   (-> {:select (select-clause-for-model model)
        :from   (from-clause-for-model model)}
+      (sql.helpers/where (search-string-clause model context))
       (search.filter/build-filters model context)))
 
 (mu/defn add-collection-join-and-where-clauses
@@ -178,8 +205,8 @@
     query))
 
 (mu/defn ^:private replace-select :- :map
-  "Replace a select from query that has alias is `target-alias` with [`with` `target-alias`] column, throw an error if
-  can't find the target select.
+  "Replace a select from query that has alias is `target-alias` with [`with` `target-alias`] column.
+   If the target-alias cannot be found, add the select.
 
   This works with the assumption that `query` contains a list of select from [[select-clause-for-model]],
   and some of them are dummy column casted to the correct type.
@@ -197,9 +224,7 @@
         with-select [with target-alias]]
     (if (some? idx)
       (assoc query :select (m/replace-nth idx with-select selects))
-      (throw (ex-info "Failed to replace selector" {:status-code  400
-                                                    :target-alias target-alias
-                                                    :with         with})))))
+      (sql.helpers/select query [with target-alias]))))
 
 (mu/defn ^:private with-last-editing-info :- :map
   [query :- :map
@@ -208,9 +233,9 @@
       (replace-select :last_editor_id :r.user_id)
       (replace-select :last_edited_at :r.timestamp)
       (sql.helpers/left-join [:revision :r]
-                             [:and [:= :r.model_id (search.config/column-with-model-alias model :id)]
+                             [:and [:= :r.model_id (search.config/column-with-alias model :id)]
                               [:= :r.most_recent true]
-                              [:= :r.model (search.config/search-model->revision-model model)]])))
+                              [:= :r.model [:inline (search.config/search-model->revision-model model)]]])))
 
 (mu/defn ^:private with-moderated-status :- :map
   [query :- :map
@@ -219,19 +244,245 @@
       (replace-select :moderated_status :mr.status)
       (sql.helpers/left-join [:moderation_review :mr]
                              [:and
-                              [:= :mr.moderated_item_type "card"]
-                              [:= :mr.moderated_item_id (search.config/column-with-model-alias model :id)]
+                              [:= :mr.moderated_item_type [:inline "card"]]
+                              [:= :mr.moderated_item_id (search.config/column-with-alias model :id)]
                               [:= :mr.most_recent true]])))
 
-(defmulti ^:private search-query-for-model
-  {:arglists '([model search-context])}
-  (fn [model _] model))
+;; ----------------- NEW IMPLEMENTATION BEGINS -------------------
+
+;; -------------- Model-specific code -----------------
+
+;; TODO: make :query and :columns separate multimethods?
+(defmulti ^:private searchable-data-query-for-model
+  {:arglists '([model])}
+  (fn [model] model))
+
+(defmethod searchable-data-query-for-model "card"
+  [model]
+  {:query (-> {:select (concat [[[:inline "card"] :search_model]]
+                               (map #(search.config/column-with-alias model %)
+                                    (conj search.config/default-columns
+                                          :type ;; rename this to card_type?
+                                          :query_type
+                                          :collection_id
+                                          :trashed_from_collection_id
+                                          :collection_position
+                                          [:dataset_query :searchable_native_query]
+                                          :display
+                                          :creator_id)))
+               :from   (from-clause-for-model model)}
+              ;; TODO: add all columns from filters
+              #_(search.filter/build-filters model context)
+              ;; TODO: collection perms clause
+              #_(add-collection-join-and-where-clauses "card" search-ctx)
+              (with-last-editing-info "card")
+              (with-moderated-status "card"))
+   :columns [:type
+             :query_type
+             :search_model
+             :description
+             :archived
+             :collection_position
+             :trashed_from_collection_id
+             :collection_id
+             :name
+             :creator_id
+             :updated_at
+             :moderated_status
+             :dataset_query
+             :last_editor_id
+             :id
+             :last_edited_at
+             :display
+             :created_at]})
+
+(defmethod searchable-data-query-for-model "table"
+  [model]
+  {:query (let [table-column #(search.config/column-with-alias model %)]
+            (-> (-> {:select [[[:inline "table"] :search_model]
+                              [:table.id :id]
+                              [:table.name :name]
+                              [:table.created_at :created_at]
+                              [:table.display_name :display_name]
+                              [:table.description :description]
+                              [:table.updated_at :updated_at]
+                              [:table.initial_sync_status :initial_sync_status]
+                              [:table.id :table_id]
+                              [:table.db_id :database_id]
+                              [:table.schema :table_schema]
+                              [:table.name :table_name]
+                              [:table.description :table_description]
+                              [:metabase_database.name :database_name]]
+                     :from   (from-clause-for-model model)}
+              ;; TODO
+                    #_(search.filter/build-filters model context))
+                (sql.helpers/where [:and
+                                    [:not [:= :table.db_id [:inline audit/audit-db-id]]]
+                                    [:= :table.active true]
+                                    [:= :table.visibility_type nil]])
+                (sql.helpers/left-join :metabase_database [:= (table-column :db_id) :metabase_database.id])))
+   :columns [:search_model
+             :id
+             :name
+             :created_at
+             :display_name
+             :description
+             :updated_at
+             :initial_sync_status
+             :table_id
+             :database_id
+             :table_schema
+             :table_name
+             :table_description
+             :database_name]})
+
+(defn searchable-data-query-for-instance [model id]
+  (let [query (:query (searchable-data-query-for-model model))]
+    (sql.helpers/where query [:= :id id])))
+
+(def searchable-models ["card"]) ; just card for now
+
+;; -------------- Backend-specific code -----------------
+
+;; union all the queries for each model, selecting all columns from each query
+(def models ["table" "card"])
+(def search-table-name "search")
+
+;; TODO: move this to metabase.driver
+(defmulti ^:private search-type->database-type
+  "Given a search type, return the database type for that column. Applies to postgres for now"
+  {:arglists '([driver])}
+  driver/dispatch-on-initialized-driver
+  :hierarchy #'driver/hierarchy)
+
+;; TODO: implement for MySQL and H2
+(defmethod search-type->database-type :postgres
+  [_driver search-type]
+  (case search-type
+    :text      "TEXT"
+    :integer   "INTEGER"
+    :boolean   "BOOLEAN"
+    :timestamp "TIMESTAMP"
+    :json      "JSON"
+    :decimal   "DECIMAL"
+    :float     "DOUBLE"
+    :time      "TIME"))
+
+;; TODO: implement this for each search backend
+(defn reindex []
+  ;; 1. Create the search table with the appropriate columns
+  (let [driver  :postgres
+        columns (->> models
+                     (map #(set (:columns (searchable-data-query-for-model %))))
+                     (reduce set/union)
+                     sort)
+        columns+types (for [column columns
+                            :let [type (get (assoc search.config/all-search-columns :search_model :text)
+                                            column)]]
+                        [column (keyword (search-type->database-type driver type))])]
+    (t2/query {:drop-table [:if-exists (keyword search-table-name)]})
+    (t2/query {:create-table (keyword search-table-name)
+               :with-columns columns+types})
+  ;; 2. For each model, insert the data into the search table
+  (doseq [model models]
+    (let [{:keys [query columns]} (searchable-data-query-for-model model)
+          ->sql #(first (mdb.query/compile %))]
+      (t2/query (str (->sql {:insert-into (keyword search-table-name)
+                             :columns columns})
+                     "\n"
+                     (->sql {:select columns
+                             :from   [[query :alias]]})))))))
+
+(comment
+  (reindex)
+  ;; TODO: make this a test checking that there are no query parameters in each query
+  (assert (every? nil? (for [model ["card" "table"]]
+                         (let [{:keys [query]} (searchable-data-query-for-model model)
+                               format-sql #(sql/format % :quoted true, :dialect (sql.qp/quote-style driver))]
+                           (second (format-sql query)))))
+          )
+  )
+
+;; TODO: implement this for each search backend
+(defn index [& changes]
+  (for [{:keys [change-type search-model id] :as change} changes]
+    (case change-type
+      :insert
+      (let [search-data (t2/query (searchable-data-query-for-instance search-model id))]
+        (t2/insert! :model/Searchable search-data))
+      :delete
+      (t2/delete! :model/Searchable :id id :search_model search-model)
+      :update
+      (let [search-data (t2/query (searchable-data-query-for-instance search-model id))]
+        (t2/update! :model/Searchable
+                    :id (:id change)
+                    :search_model (:search-model change)
+                    search-data)))))
+
+;; TODO: make this a multimethod to be implemented by each backend that compiles the query.
+(defn execute-search [search-ctx]
+  (let [compile (fn [q]
+                  (->  {:select :*
+                        :from   (keyword search-table-name)}
+                       ;; TODO: extract search string clause and apply it across all models to allow for full-text search
+                       (sql.helpers/where (search-string-clause q search-ctx))
+                       ;; TODO: update build-filters to only query from the search table, and not use joins
+                       (search.filter/build-filters q search-ctx)
+                       ;; TODO: implement collection perms
+                       #_(add-collection-join-and-where-clauses "card" search-ctx)
+                       ;; TODO: implement table-db-id fitler
+                       #_(add-card-db-id-clause (:table-db-id search-ctx))))
+        specific-query (compile search-ctx)]
+    (map #(into {} %) (t2/query specific-query))))
+
+(comment
+  ;; 1. reindex
+  (reindex)
+  ;; 2. execute search:
+  (let [search-ctx {:search-string      "orders"
+                    :models             search.config/all-models
+                    :model-ancestors?   false
+                    :current-user-id    1
+                    :current-user-perms #{"/"}}]
+    (execute-search search-ctx))
+  ;; => ({:description nil,
+  ;;      :archived false,
+  ;;      :collection_position nil,
+  ;;      :table_id nil,
+  ;;      :trashed_from_collection_id nil,
+  ;;      :database_id nil,
+  ;;      :collection_id nil,
+  ;;      :database_name nil,
+  ;;      :query_type "query",
+  ;;      :name "Orders q",
+  ;;      :type "question",
+  ;;      :table_schema nil,
+  ;;      :search_model "card",
+  ;;      :creator_id 3,
+  ;;      :updated_at #t "2024-06-11T00:50:50.820882",
+  ;;      :moderated_status nil,
+  ;;      :dataset_query "{\"database\":2,\"type\":\"query\",\"query\":{\"source-table\":12}}",
+  ;;      :last_editor_id 3,
+  ;;      :id 237,
+  ;;      :last_edited_at #t "2024-06-11T00:50:50.865469",
+  ;;      :table_description nil,
+  ;;      :display "table",
+  ;;      :initial_sync_status nil,
+  ;;      :table_name nil,
+  ;;      :display_name nil,
+  ;;      :created_at #t "2024-06-11T00:50:50.820882"})
+
+  )
+
+;; ----------------- NEW IMPLEMENTATION ENDS -------------------
 
 (mu/defn ^:private shared-card-impl
-  [model      :- :metabase.models.card/type
+  [type       :- :metabase.models.card/type
    search-ctx :- SearchContext]
   (-> (base-query-for-model "card" search-ctx)
-      (sql.helpers/where [:= :card.type (name model)])
+      (sql.helpers/where (when (:search-native-query search-ctx)
+                           [:= (search.config/column-with-alias "card" :query_type) [:inline "native"]]))
+      (sql.helpers/where [:= :card.type (name type)])
       (sql.helpers/left-join [:card_bookmark :bookmark]
                              [:and
                               [:= :bookmark.card_id :card.id]
@@ -241,6 +492,10 @@
       (with-last-editing-info "card")
       (with-moderated-status "card")))
 
+(defmulti ^:private search-query-for-model
+  {:arglists '([model search-context])}
+  (fn [model _] model))
+
 (defmethod search-query-for-model "action"
   [model search-ctx]
   (-> (base-query-for-model model search-ctx)
@@ -316,6 +571,15 @@
          query
          collection-perm-clause)))))
 
+(defn- sandboxed-or-impersonated-user? []
+  ;; TODO FIXME -- search actually currently still requires [[metabase.api.common/*current-user*]] to be bound,
+  ;; because [[metabase.public-settings.premium-features/sandboxed-or-impersonated-user?]] requires it to be bound.
+  ;; Since it's part of the search context it would be nice if we could run search without having to bind that stuff at
+  ;; all.
+  (assert @@(requiring-resolve 'metabase.api.common/*current-user*)
+          "metabase.api.common/*current-user* must be bound in order to use search for an indexed entity")
+  (premium-features/sandboxed-or-impersonated-user?))
+
 (defmethod search-query-for-model "indexed-entity"
   [model {:keys [current-user-perms] :as search-ctx}]
   (-> (base-query-for-model model search-ctx)
@@ -323,6 +587,7 @@
                              [:= :model-index.id :model-index-value.model_index_id])
       (sql.helpers/left-join [:report_card :model] [:= :model-index.model_id :model.id])
       (sql.helpers/left-join [:collection :collection] [:= :model.collection_id :collection.id])
+      (sql.helpers/where (when (sandboxed-or-impersonated-user?) search.filter/false-clause))
       (add-model-index-permissions-clause current-user-perms)))
 
 (defmethod search-query-for-model "segment"
@@ -330,14 +595,6 @@
   (-> (base-query-for-model model search-ctx)
       (sql.helpers/left-join [:metabase_table :table] [:= :segment.table_id :table.id])))
 
-(defmethod search-query-for-model "table"
-  [model {:keys [current-user-perms table-db-id], :as search-ctx}]
-  (when (seq current-user-perms)
-
-    (-> (base-query-for-model model search-ctx)
-        (add-table-db-id-clause table-db-id)
-        (sql.helpers/left-join :metabase_database [:= :table.db_id :metabase_database.id]))))
-
 (defn order-clause
   "CASE expression that lets the results be ordered by whether they're an exact (non-fuzzy) match or not"
   [query]
@@ -378,7 +635,7 @@
 
 (defmethod check-permissions-for-model :default
   [search-ctx instance]
-  (if (:archived? search-ctx)
+  (if (:archived search-ctx)
     (can-write? instance)
     ;; We filter what we can (ie. everything that is in a collection) out already when querying
     true))
@@ -410,19 +667,19 @@
 
 (defmethod check-permissions-for-model :metric
   [search-ctx instance]
-  (if (:archived? search-ctx)
+  (if (:archived search-ctx)
     (can-write? instance)
     (can-read? instance)))
 
 (defmethod check-permissions-for-model :segment
   [search-ctx instance]
-  (if (:archived? search-ctx)
+  (if (:archived search-ctx)
     (can-write? instance)
     (can-read? instance)))
 
 (defmethod check-permissions-for-model :database
   [search-ctx instance]
-  (if (:archived? search-ctx)
+  (if (:archived search-ctx)
     (can-write? instance)
     (can-read? instance)))
 
@@ -620,7 +877,7 @@
 
                             (map #(update % :pk_ref json/parse-string))
                             (map add-can-write)
-                            (map (partial scoring/score-and-result (:search-string search-ctx)))
+                            (map (partial scoring/score-and-result (:search-string search-ctx) (:search-native-query search-ctx)))
 
                             (filter #(pos? (:score %))))
         total-results       (cond->> (scoring/top-results reducible-results search.config/max-filtered-results xf)
@@ -690,20 +947,20 @@
      [:content-verification :official-collections]
      (deferred-tru "Content Management or Official Collections")))
   (let [models (if (string? models) [models] models)
-        ctx    (cond-> {:archived?          (boolean archived)
-                        :current-user-id current-user-id
+        ctx    (cond-> {:current-user-id current-user-id
                         :current-user-perms current-user-perms
                         :model-ancestors?   (boolean model-ancestors?)
                         :models             models
                         :search-string      search-string}
+                 (some? archived)                            (assoc :archived archived)
                  (some? created-at)                          (assoc :created-at created-at)
                  (seq created-by)                            (assoc :created-by created-by)
                  (some? filter-items-in-personal-collection) (assoc :filter-items-in-personal-collection filter-items-in-personal-collection)
-                 (some? last-edited-at)                     (assoc :last-edited-at last-edited-at)
-                 (seq last-edited-by)                       (assoc :last-edited-by last-edited-by)
-                 (some? table-db-id)                        (assoc :table-db-id table-db-id)
-                 (some? limit)                              (assoc :limit-int limit)
-                 (some? offset)                             (assoc :offset-int offset)
-                 (some? search-native-query)                (assoc :search-native-query search-native-query)
-                 (some? verified)                           (assoc :verified verified))]
+                 (some? last-edited-at)                      (assoc :last-edited-at last-edited-at)
+                 (seq last-edited-by)                        (assoc :last-edited-by last-edited-by)
+                 (some? table-db-id)                         (assoc :table-db-id table-db-id)
+                 (some? limit)                               (assoc :limit-int limit)
+                 (some? offset)                              (assoc :offset-int offset)
+                 (some? search-native-query)                 (assoc :search-native-query search-native-query)
+                 (some? verified)                            (assoc :verified verified))]
     (assoc ctx :models (search.filter/search-context->applicable-models ctx))))

src/metabase/search/scoring.clj

@@ -156,9 +156,12 @@
 (defn- text-scores-with
   "Scores a search result. Returns a vector of score maps, each containing `:weight`, `:score`, and other info about
   the text match, if there is one. If there is no match, the score is 0."
-  [weighted-scorers query-tokens search-result]
+  [search-native-query weighted-scorers query-tokens search-result]
   ;; TODO is pmap over search-result worth it?
-  (let [scores (for [column      (search.config/searchable-columns-for-model (:model search-result))
+  (let [scores (for [column      (cond-> (search.config/searchable-columns-for-model (:model search-result))
+                                   search-native-query
+                                   (concat
+                                    (search.config/native-query-columns (:model search-result))))
                      {:keys [scorer name weight]
                       :as   _ws} weighted-scorers
                      :let        [matched-text (-> search-result
@@ -256,9 +259,10 @@
      (count model->sort-position)))
 
 (defn- text-scores-with-match
-  [raw-search-string result]
+  [raw-search-string search-native-query result]
   (if (seq raw-search-string)
-    (text-scores-with match-based-scorers
+    (text-scores-with search-native-query
+                      match-based-scorers
                       (search.util/tokenize (search.util/normalize raw-search-string))
                       result)
     [{:score 0 :weight 0}]))
@@ -353,9 +357,9 @@
 
 (defn score-and-result
   "Returns a map with the normalized, combined score from relevant-scores as `:score` and `:result`."
-  [raw-search-string result]
+  [raw-search-string search-native-query result]
   (let [text-matches     (-> raw-search-string
-                             (text-scores-with-match result)
+                             (text-scores-with-match search-native-query result)
                              (force-weight text-scores-weight))
         all-scores       (into (vec (score-result result)) text-matches)
         relevant-scores  (remove #(= 0 (:score %)) all-scores)

test/metabase/api/search_test.clj

@@ -170,12 +170,15 @@
                                                      {:db_id db-id})
 
                    QueryAction _qa (query-action action-id)
-                   Card        card           (coll-data-map "card %s card" coll)
+                   Card        card           (assoc (coll-data-map "card %s card" coll)
+                                                     :query_type :native)
                    Card        dataset        (assoc (coll-data-map "dataset %s dataset" coll)
-                                                     :type :model)
+                                                     :type :model
+                                                     :query_type :native)
                    Dashboard   dashboard      (coll-data-map "dashboard %s dashboard" coll)
                    Card        metric         (assoc (coll-data-map "metric %s metric" coll)
-                                                     :type :metric)
+                                                     :type :metric
+                                                     :query_type :native)
                    Segment     segment        (data-map "segment %s segment")]
       (f {:action     action
           :collection coll
@@ -1263,12 +1266,12 @@
 (deftest search-native-query-test
   (let [search-term "search-native-query"]
     (mt/with-temp
-      [:model/Card {mbql-card :id}             {:name search-term}
-       :model/Card {native-card-in-name :id}   {:name search-term}
-       :model/Card {native-card-in-query :id}  {:dataset_query (mt/native-query {:query (format "select %s" search-term)})}
-       :model/Card {mbql-model :id}            {:name search-term :type :model}
-       :model/Card {native-model-in-name :id}  {:name search-term :type :model}
-       :model/Card {native-model-in-query :id} {:dataset_query (mt/native-query {:query (format "select %s" search-term)}) :type :model}]
+      [:model/Card {mbql-card :id}             {:query_type :query,  :name search-term}
+       :model/Card {native-card-in-name :id}   {:query_type :native, :name search-term}
+       :model/Card {native-card-in-query :id}  {:query_type :native, :dataset_query (mt/native-query {:query (format "select %s" search-term)})}
+       :model/Card {mbql-model :id}            {:query_type :query,  :name search-term, :type :model}
+       :model/Card {native-model-in-name :id}  {:query_type :native, :name search-term, :type :model}
+       :model/Card {native-model-in-query :id} {:query_type :native, :dataset_query (mt/native-query {:query (format "select %s" search-term)}), :type :model}]
       (mt/with-actions
         [_                         {:type :model :dataset_query (mt/mbql-query venues)}
          {http-action :action-id}  {:type :http :name search-term}
@@ -1283,11 +1286,8 @@
                       :data
                       (map (juxt :model :id))
                       set))))
-
         (testing "if search-native-query is true, search both dataset_query and the name"
-          (is (= #{["card" mbql-card]
-                   ["card" native-card-in-name]
-                   ["dataset" mbql-model]
+          (is (= #{["card" native-card-in-name]
                    ["dataset" native-model-in-name]
                    ["action" http-action]
 
@@ -1349,16 +1349,16 @@
                (set (mt/user-http-request :crowberto :get 200 "search/models" :table-db-id (mt/id)))))))))
 
 (deftest models-archived-string-test
-  (testing "search/models request includes `archived-string` param"
+  (testing "search/models request can include the `archived` param"
     (with-search-items-in-root-collection "Available models"
       (mt/with-temp [Card   {model-id :id} action-model-params
                      Action _              (archived {:name     "test action"
                                                       :type     :query
                                                       :model_id model-id})]
-        (testing "`archived-string` is 'false'"
+        (testing "`archived` is not present"
           (is (= #{"dashboard" "table" "dataset" "segment" "collection" "database" "action" "metric" "card"}
-                 (set (mt/user-http-request :crowberto :get 200 "search/models" :archived "false")))))
-        (testing "`archived-string` is 'true'"
+                 (set (mt/user-http-request :crowberto :get 200 "search/models")))))
+        (testing "`archived` is 'true'"
           (is (= #{"action"}
                  (set (mt/user-http-request :crowberto :get 200 "search/models" :archived "true")))))))))
 

test/metabase/search/filter_test.clj

@@ -6,9 +6,8 @@
    [metabase.search.filter :as search.filter]
    [metabase.test :as mt]))
 
-(def ^:private default-search-ctx
-  {:search-string       nil
-   :archived?           false
+(def default-search-ctx
+  {:search-string      nil
    :models             search.config/all-models
    :model-ancestors?   false
    :current-user-id    1
@@ -23,10 +22,10 @@
       (is (= #{}
              (search.filter/search-context->applicable-models
               (assoc default-search-ctx :models #{}))))
-      (is (= search.config/all-models
+      (is (= #{"dashboard" "dataset" "segment" "collection" "action" "metric" "card"}
              (search.filter/search-context->applicable-models
               (merge default-search-ctx
-                     {:archived? true})))))))
+                     {:archived true})))))))
 
 (deftest ^:parallel ->applicable-models-test-2
   (testing "optional filters will return intersection of support models and provided models\n"
@@ -129,14 +128,7 @@
   (testing "archived filters"
     (is (= [:= :card.archived false]
            (:where (search.filter/build-filters
-                    base-search-query "card" default-search-ctx))))
-
-    (is (= [:and
-            [:= :table.active true]
-            [:= :table.visibility_type nil]
-            [:not [:= :table.db_id audit/audit-db-id]]]
-           (:where (search.filter/build-filters
-                    base-search-query "table"  default-search-ctx))))))
+                    base-search-query "card" default-search-ctx))))))
 
 (deftest ^:parallel build-table-filter-always-ignores-audit-tables
   (is (contains?
@@ -363,76 +355,3 @@
           "database"
           (merge default-search-ctx
                  {:created-by #{1}}))))))
-
-(deftest build-filters-indexed-entity-test
-  (testing "users that are not sandboxed or impersonated can search for indexed entity"
-    (with-redefs [search.filter/sandboxed-or-impersonated-user? (constantly false)]
-      (is (= [:and
-              [:or [:like [:lower :model-index-value.name] "%foo%"]]
-              [:inline [:= 1 1]]]
-             (:where (search.filter/build-filters
-                      base-search-query
-                      "indexed-entity"
-                      (merge default-search-ctx {:search-string "foo"}))))))))
-
-(deftest build-filters-indexed-entity-test-2
-  (testing "otherwise search result is empty"
-    (with-redefs [search.filter/sandboxed-or-impersonated-user? (constantly true)]
-      (is (= [:and
-              [:or [:= 0 1]]
-              [:inline [:= 1 1]]]
-             (:where (search.filter/build-filters
-                      base-search-query
-                      "indexed-entity"
-                      (merge default-search-ctx {:search-string "foo"}))))))))
-
-(deftest ^:parallel build-filters-search-native-query
-  (doseq [model ["dataset" "card"]]
-    (testing model
-      (testing "do not search for native query by default"
-        (is (= [:and
-                [:or [:like [:lower :card.name] "%foo%"] [:like [:lower :card.description] "%foo%"]]
-                [:= :card.archived false]]
-               (:where (search.filter/build-filters
-                        base-search-query
-                        model
-                        (merge default-search-ctx {:search-string "foo"})))))))))
-
-(deftest ^:parallel build-filters-search-native-query-2
-  (doseq [model ["dataset" "card"]]
-    (testing model
-      (testing "search in both name, description and dataset_query if is enabled"
-        (is (= [:and [:or
-                      [:like [:lower :card.name] "%foo%"]
-                      [:and [:= :card.query_type "native"] [:like [:lower :card.dataset_query] "%foo%"]]
-                      [:like [:lower :card.description] "%foo%"]]
-                [:= :card.archived false]]
-               (:where (search.filter/build-filters
-                        base-search-query
-                        model
-                        (merge default-search-ctx {:search-string "foo" :search-native-query true})))))))))
-
-(deftest ^:parallel build-filters-search-native-query-3
-  (testing "action"
-    (testing "do not search for native query by default"
-      (is (= [:and
-              [:or [:like [:lower :action.name] "%foo%"] [:like [:lower :action.description] "%foo%"]]
-              [:= :action.archived false]]
-             (:where (search.filter/build-filters
-                      base-search-query
-                      "action"
-                      (merge default-search-ctx {:search-string "foo"}))))))))
-
-(deftest ^:parallel build-filters-search-native-query-4
-  (testing "action"
-    (testing "search in both name, description and dataset_query if is enabled"
-      (is (= [:and
-              [:or
-               [:like [:lower :action.name] "%foo%"]
-               [:like [:lower :query_action.dataset_query] "%foo%"]
-               [:like [:lower :action.description] "%foo%"]]
-              [:= :action.archived false]]
-             (:where (search.filter/build-filters
-                      base-search-query
-                      "action"
-                      (merge default-search-ctx {:search-string "foo" :search-native-query true}))))))))

test/metabase/search/impl_test.clj

@@ -7,7 +7,9 @@
    [clojure.test :refer :all]
    [java-time.api :as t]
    [metabase.api.common :as api]
+   [metabase.audit :as audit]
    [metabase.search.config :as search.config]
+   [metabase.search.filter-test :as search.filter-test]
    [metabase.search.impl :as search.impl]
    [metabase.test :as mt]
    [toucan2.core :as t2]
@@ -58,7 +60,6 @@
       (mt/with-current-user (mt/user->id :crowberto)
         (let [do-search (fn []
                           (search.impl/search {:search-string      search-string
-                                               :archived?          false
                                                :models             search.config/all-models
                                                :current-user-id    (mt/user->id :crowberto)
                                                :current-user-perms #{"/"}
@@ -131,7 +132,6 @@
                                 (is (= expected
                                        (->> (search.impl/search (search.impl/search-context
                                                                  {:search-string      search-term
-                                                                  :archived           false
                                                                   :models             search.config/all-models
                                                                   :created-at         created-at
                                                                   :current-user-id    (mt/user->id :crowberto)
@@ -216,7 +216,6 @@
                                 (is (= expected
                                        (->> (search.impl/search (search.impl/search-context
                                                                  {:search-string      search-term
-                                                                  :archived           false
                                                                   :models             search.config/all-models
                                                                   :last-edited-at     last-edited-at
                                                                   :current-user-id    (mt/user->id :crowberto)
@@ -264,3 +263,69 @@
     (is (nil? (-> {:name "dash" :model "dashboard" :all-scores {} :relevant-scores {}}
                   search.impl/serialize
                   :dataset_query)))))
+
+(deftest ^:parallel active-table-search-test
+  (testing "Only active, visible, and non audit DB tables should be included"
+    (is (= [:and
+            [:not [:= :table.db_id audit/audit-db-id]]
+            [:= :table.active true]
+            [:= :table.visibility_type nil]]
+           (:where (#'search.impl/search-query-for-model "table" search.filter-test/default-search-ctx))))))
+
+(deftest ^:parallel indexed-entity-sandboxed-or-impersonated-user-test
+  (testing "Only active, visible, and non audit DB tables should be included"
+    (let [search-query (fn []
+                         (#'search.impl/search-query-for-model
+                          "indexed-entity"
+                          (assoc search.filter-test/default-search-ctx :search-string "foo")))]
+      (mt/with-dynamic-redefs [search.impl/sandboxed-or-impersonated-user? (constantly false)]
+        (is (= [:or [:like [:lower :model-index-value.name] "%foo%"]]
+               (:where (search-query)))))
+      (mt/with-dynamic-redefs [search.impl/sandboxed-or-impersonated-user? (constantly true)]
+        (is (= [:and
+                [:or [:like [:lower :model-index-value.name] "%foo%"]]
+                [:inline [:= 0 1]]]
+               (:where (search-query))))))))
+
+(deftest ^:parallel search-native-query-test
+  (doseq [model ["dataset" "card"]]
+    (testing model
+      (testing "do not search for native query by default"
+        (let [where-clause (:where (#'search.impl/search-query-for-model
+                                    model
+                                    (assoc search.filter-test/default-search-ctx :search-string "foo")))]
+          (is (= :and (first where-clause)))
+          (is (contains? (set where-clause)
+                         [:or [:like [:lower :card.name] "%foo%"] [:like [:lower :card.description] "%foo%"]]))))
+       (testing "search in both name, description and dataset_query if is enabled"
+         (let [where-clause (:where (#'search.impl/search-query-for-model
+                                     model
+                                     (assoc search.filter-test/default-search-ctx :search-string "foo" :search-native-query true)))]
+           (is (= :and (first where-clause)))
+           (is (contains? (set where-clause)
+                          [:or
+                           [:like [:lower :card.name] "%foo%"]
+                           [:like [:lower :card.description] "%foo%"]
+                           [:like [:lower :dataset_query] "%foo%"]])))))))
+
+(deftest ^:parallel search-native-query-test-2
+  (testing "action"
+    (testing "do not search for native query by default"
+      (let [where-clause (:where (#'search.impl/search-query-for-model
+                                  "action"
+                                  (assoc search.filter-test/default-search-ctx :search-string "foo")))]
+        (is (= :and (first where-clause)))
+        (is (contains? (set where-clause)
+                       [:or
+                        [:like [:lower :action.name] "%foo%"]
+                        [:like [:lower :action.description] "%foo%"]]))))
+    (testing "search in both name, description and dataset_query if is enabled"
+      (let [where-clause (:where (#'search.impl/search-query-for-model
+                                  "action"
+                                  (assoc search.filter-test/default-search-ctx :search-string "foo" :search-native-query true)))]
+        (is (= :and (first where-clause)))
+        (is (contains? (set where-clause)
+                       [:or
+                        [:like [:lower :action.name] "%foo%"]
+                        [:like [:lower :action.description] "%foo%"]
+                        [:like [:lower :query_action.dataset_query] "%foo%"]]))))))

test/metabase/search/scoring_test.clj

@@ -16,7 +16,7 @@
   [scorer]
   (comp :score
         first
-        (partial #'scoring/text-scores-with [{:weight 1 :scorer scorer}])))
+        (partial #'scoring/text-scores-with nil [{:weight 1 :scorer scorer}])))
 
 (deftest ^:parallel consecutivity-scorer-test
   (let [score (scorer->score #'scoring/consecutivity-scorer)]
@@ -243,7 +243,7 @@
                        d])              ; middling text match, no other signal
            (->> labeled-results
                 vals
-                (map (partial scoring/score-and-result search-string))
+                (map (partial scoring/score-and-result search-string nil))
                 (sort-by :score)
                 reverse
                 (map :result)
@@ -258,7 +258,7 @@
     (is (= (map :name [b c a])
            (->> labeled-results
                 vals
-                (map (partial scoring/score-and-result search-string))
+                (map (partial scoring/score-and-result search-string nil))
                 (sort-by :score)
                 reverse
                 (map :result)
@@ -270,7 +270,7 @@
                   (fn [_]
                     [{:weight 100 :score 0 :name "Some score type"}
                      {:weight 100 :score 0 :name "Some other score type"}])]
-      (is (= 0 (:score (scoring/score-and-result "" {:name "racing yo" :model "card"})))))))
+      (is (= 0 (:score (scoring/score-and-result "" nil {:name "racing yo" :model "card"})))))))
 
 
 (deftest ^:parallel force-weight-test