diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index a3c19a00c5ca1b8fcb15aefc9688d5462dc0fd5a..29a174a44356f115446220f51324ef2a18eb9c86 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -668,7 +668,7 @@
metabase.search.config search.config
metabase.search.filter search.filter
metabase.search.in-place.filter search.in-place.filter
- metabase.search.util search.util
+ metabase.search.in-place.util search.util
metabase.search.in-place.scoring scoring
metabase.server.middleware.auth mw.auth
metabase.server.middleware.browser-cookie mw.browser-cookie
diff --git a/src/metabase/models/table.clj b/src/metabase/models/table.clj
index ea0844e1c079c3bf4c812e7efb7fe52568438b02..e11b9c9e635ce0a0e62c99b6d731b8cb89365fdb 100644
--- a/src/metabase/models/table.clj
+++ b/src/metabase/models/table.clj
@@ -311,7 +311,7 @@
:view-count true
:created-at true
:updated-at true}
- :search-terms [:name :description :display_name]
+ :search-terms [:name :display_name :description]
:render-terms {:initial-sync-status true
:table-id :id
:table-description :description
diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj
index 4a967a0001ee9a23b01e22d1f37d25d0dfbb9c96..316f37a4a3a98d16c5afd79a07ec2352fccc2850 100644
--- a/src/metabase/public_settings.clj
+++ b/src/metabase/public_settings.clj
@@ -1052,7 +1052,6 @@ See [fonts](../configuring-metabase/fonts.md).")
(defsetting experimental-search-weight-overrides
(deferred-tru "Used to override weights used for search ranking")
:visibility :internal
- :cache? false
:encryption :no
:export? false
:default nil
diff --git a/src/metabase/search/filter.clj b/src/metabase/search/filter.clj
index a0bdc477d174904765a88d2d001d12e9063a5c90..cc9c731cad13fe64a9fb9a2d9e94598e03d9ae6f 100644
--- a/src/metabase/search/filter.clj
+++ b/src/metabase/search/filter.clj
@@ -8,7 +8,8 @@
[metabase.search.permissions :as search.permissions]
[metabase.search.spec :as search.spec]
[metabase.util.date-2 :as u.date]
- [metabase.util.i18n :refer [tru]])
+ [metabase.util.i18n :refer [tru]]
+ [toucan2.core :as t2])
(:import
(java.time LocalDate)))
@@ -77,6 +78,31 @@
(defmethod where-clause* ::list [_ k v] [:in k v])
+(defn personal-collections-where-clause
+ "Build a clause limiting the entries to those (not) within or within personal collections, if relevant.
+ WARNING: this method queries the appdb, and its approach will get very slow when there are many users!"
+ [{filter-type :filter-items-in-personal-collection} collection-id-col]
+ (when filter-type
+ (let [parent-ids (t2/select-pks-vec :model/Collection :personal_owner_id [:not= nil])
+ child-patterns (for [id parent-ids] (format "/%d/%%" id))]
+ (case filter-type
+ "only"
+ `[:or
+ ;; top level personal collections
+ [:and [:not= :collection.personal_owner_id nil] [:= :collection.location "/"]]
+ ;; their sub-collections
+ ~@(for [p child-patterns] [:like :collection.location p])]
+
+ "exclude"
+ `[:or
+ ;; not in a collection
+ [:= ~collection-id-col nil]
+ [:and
+ ;; neither in a top-level personal collection
+ [:= :collection.personal_owner_id nil]
+ ;; nor within one of their sub-collections
+ ~@(for [p child-patterns] [:not-like :collection.location p])]]))))
+
(defn with-filters
"Return a HoneySQL clause corresponding to all the optional search filters."
[search-context qry]
diff --git a/src/metabase/search/in_place/filter.clj b/src/metabase/search/in_place/filter.clj
index 8ad689f5695b6d37e84fd4a15e2746fd4e3ddc8b..3ad8b997f5033420212f2591d2ee64a36a3294d4 100644
--- a/src/metabase/search/in_place/filter.clj
+++ b/src/metabase/search/in_place/filter.clj
@@ -19,8 +19,8 @@
[metabase.search.config
:as search.config
:refer [SearchableModel SearchContext]]
+ [metabase.search.in-place.util :as search.util]
[metabase.search.permissions :as search.permissions]
- [metabase.search.util :as search.util]
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [tru]]
[metabase.util.malli :as mu])
diff --git a/src/metabase/search/in_place/scoring.clj b/src/metabase/search/in_place/scoring.clj
index 934c15becb1023b70953edba4f831590233b90cf..27d93e252734fdd756ded39f88ba52f4f0f11a69 100644
--- a/src/metabase/search/in_place/scoring.clj
+++ b/src/metabase/search/in_place/scoring.clj
@@ -115,7 +115,7 @@
[java-time.api :as t]
[metabase.public-settings.premium-features :refer [defenterprise]]
[metabase.search.config :as search.config]
- [metabase.search.util :as search.util]
+ [metabase.search.in-place.util :as search.util]
[metabase.util :as u]))
(defn- matches?
diff --git a/src/metabase/search/util.clj b/src/metabase/search/in_place/util.clj
similarity index 98%
rename from src/metabase/search/util.clj
rename to src/metabase/search/in_place/util.clj
index a1954f0d8b8e721137ad278b7990ce8d95b447fe..9358160dda879fa2cdf86c1187224f03a94c0516 100644
--- a/src/metabase/search/util.clj
+++ b/src/metabase/search/in_place/util.clj
@@ -1,4 +1,4 @@
-(ns metabase.search.util
+(ns metabase.search.in-place.util
(:require
[clojure.core.memoize :as memoize]
[clojure.string :as str]
diff --git a/src/metabase/search/legacy.clj b/src/metabase/search/legacy.clj
index 85736dca42ffdb2bd0ece9df11cce7ee64fd1e5f..20c1d3380113ae94c4706a2926dc65e755496967 100644
--- a/src/metabase/search/legacy.clj
+++ b/src/metabase/search/legacy.clj
@@ -11,10 +11,11 @@
[metabase.search.config
:as search.config
:refer [SearchContext SearchableModel]]
+ [metabase.search.filter :as search.filter]
[metabase.search.in-place.filter :as search.in-place.filter]
[metabase.search.in-place.scoring :as scoring]
+ [metabase.search.in-place.util :as search.util]
[metabase.search.permissions :as search.permissions]
- [metabase.search.util :as search.util]
[metabase.util :as u]
[metabase.util.honey-sql-2 :as h2x]
[metabase.util.log :as log]
@@ -151,6 +152,24 @@
(sql.helpers/where query [:= id :database_id])
query))
+(mu/defn add-collection-join-and-where-clauses
+ "Add a `WHERE` clause to the query to only return Collections the Current User has access to; join against Collection,
+ so we can return its `:name`."
+ [honeysql-query :- :map
+ model :- [:maybe :string]
+ search-ctx :- SearchContext]
+ (let [collection-id-col (case model
+ "collection" :collection.id
+ "search-index" :search_index.collection_id
+ :collection_id)
+ permitted-clause (search.permissions/permitted-collections-clause search-ctx collection-id-col)
+ personal-clause (search.filter/personal-collections-where-clause search-ctx collection-id-col)]
+ (cond-> honeysql-query
+ ;; add a JOIN against Collection *unless* the source table is already Collection
+ (not= model "collection") (sql.helpers/left-join [:collection :collection] [:= collection-id-col :collection.id])
+ true (sql.helpers/where permitted-clause)
+ personal-clause (sql.helpers/where personal-clause))))
+
(mu/defn- 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.
@@ -418,7 +437,7 @@
[:and
[:= :bookmark.card_id :card.id]
[:= :bookmark.user_id (:current-user-id search-ctx)]])
- (search.permissions/add-collection-join-and-where-clauses "card" search-ctx)
+ (add-collection-join-and-where-clauses "card" search-ctx)
(add-card-db-id-clause (:table-db-id search-ctx))
(with-last-editing-info "card")
(with-moderated-status "card")))
@@ -430,7 +449,7 @@
[:= :model.id :action.model_id])
(sql.helpers/left-join :query_action
[:= :query_action.action_id :action.id])
- (search.permissions/add-collection-join-and-where-clauses model search-ctx)))
+ (add-collection-join-and-where-clauses model search-ctx)))
(defmethod search-query-for-model "card"
[_model search-ctx]
@@ -455,7 +474,7 @@
[:and
[:= :bookmark.collection_id :collection.id]
[:= :bookmark.user_id (:current-user-id search-ctx)]])
- (search.permissions/add-collection-join-and-where-clauses model search-ctx)))
+ (add-collection-join-and-where-clauses model search-ctx)))
(defmethod search-query-for-model "database"
[model search-ctx]
@@ -469,7 +488,7 @@
[:= :bookmark.dashboard_id :dashboard.id]
[:= :bookmark.user_id (:current-user-id search-ctx)]])
(with-moderated-status "dashboard")
- (search.permissions/add-collection-join-and-where-clauses model search-ctx)
+ (add-collection-join-and-where-clauses model search-ctx)
(with-last-editing-info "dashboard")))
(defn- add-model-index-permissions-clause
diff --git a/src/metabase/search/permissions.clj b/src/metabase/search/permissions.clj
index f2408e8802a66f48d8117057a749a09d23117216..23b11134b825a86fab3ef08de05484c971c32755 100644
--- a/src/metabase/search/permissions.clj
+++ b/src/metabase/search/permissions.clj
@@ -1,12 +1,10 @@
(ns metabase.search.permissions
(:require
- [honey.sql.helpers :as sql.helpers]
[metabase.models.collection :as collection]
[metabase.models.permissions :as perms]
[metabase.public-settings.premium-features :as premium-features]
[metabase.search.config :refer [SearchContext]]
- [metabase.util.malli :as mu]
- [toucan2.core :as t2]))
+ [metabase.util.malli :as mu]))
(defn- assert-current-user! [missing-param]
(assert @@(requiring-resolve 'metabase.api.common/*current-user*)
@@ -31,54 +29,15 @@
[search-ctx]
(or (impersonated-user? search-ctx) (sandboxed-user? search-ctx)))
-(mu/defn add-collection-join-and-where-clauses
- "Add a `WHERE` clause to the query to only return Collections the Current User has access to; join against Collection,
- so we can return its `:name`."
- [honeysql-query :- :map
- model :- [:maybe :string]
- {:keys [filter-items-in-personal-collection
- archived
- current-user-id
- is-superuser?]} :- SearchContext]
- (let [collection-id-col (case model
- "collection" :collection.id
- "search-index" :search_index.collection_id
- :collection_id)
- collection-filter-clause (collection/visible-collection-filter-clause
- collection-id-col
- {:include-archived-items :all
- :include-trash-collection? true
- :permission-level (if archived
- :write
- :read)}
- {:current-user-id current-user-id
- :is-superuser? is-superuser?})]
- (cond-> honeysql-query
- true
- (sql.helpers/where collection-filter-clause (perms/audit-namespace-clause :collection.namespace nil))
- ;; add a JOIN against Collection *unless* the source table is already Collection
- (not= model "collection")
- (sql.helpers/left-join [:collection :collection]
- [:= collection-id-col :collection.id])
-
- ;; TODO This is not really about permissions, it should really be handled in search.filter
- (some? filter-items-in-personal-collection)
- (sql.helpers/where
- (case filter-items-in-personal-collection
- "only"
- (concat [:or]
- ;; sub personal collections
- (for [id (t2/select-pks-set :model/Collection :personal_owner_id [:not= nil])]
- [:like :collection.location (format "/%d/%%" id)])
- ;; top level personal collections
- [[:and
- [:= :collection.location "/"]
- [:not= :collection.personal_owner_id nil]]])
-
- "exclude"
- (conj [:or]
- (into
- [:and [:= :collection.personal_owner_id nil]]
- (for [id (t2/select-pks-set :model/Collection :personal_owner_id [:not= nil])]
- [:not-like :collection.location (format "/%d/%%" id)]))
- [:= collection-id-col nil]))))))
+(mu/defn permitted-collections-clause
+ "Build the WHERE clause corresponding to which collections the given user has access to."
+ [{:keys [archived current-user-id is-superuser?]} :- SearchContext collection-id-col :- :keyword]
+ [:and
+ (collection/visible-collection-filter-clause
+ collection-id-col
+ {:include-archived-items :all
+ :include-trash-collection? true
+ :permission-level (if archived :write :read)}
+ {:current-user-id current-user-id
+ :is-superuser? is-superuser?})
+ (perms/audit-namespace-clause :collection.namespace nil)])
diff --git a/src/metabase/search/postgres/core.clj b/src/metabase/search/postgres/core.clj
index fc9e65d6ac267465ce0176fb37b77a8dd5e7bdb9..5e188656019df764ed28fcd59a1179ca60e8858b 100644
--- a/src/metabase/search/postgres/core.clj
+++ b/src/metabase/search/postgres/core.clj
@@ -81,14 +81,26 @@
(update :updated_at parse-datetime)
(update :last_edited_at parse-datetime)))
+(defn add-collection-join-and-where-clauses
+ "Add a `WHERE` clause to the query to only return Collections the Current User has access to; join against Collection,
+ so we can return its `:name`."
+ [search-ctx qry]
+ (let [collection-id-col :search_index.collection_id
+ permitted-clause (search.permissions/permitted-collections-clause search-ctx collection-id-col)
+ personal-clause (search.filter/personal-collections-where-clause search-ctx collection-id-col)]
+ (cond-> qry
+ true (sql.helpers/left-join [:collection :collection] [:= collection-id-col :collection.id])
+ true (sql.helpers/where permitted-clause)
+ personal-clause (sql.helpers/where personal-clause))))
+
(defn- fulltext
"Search purely using the index."
[search-term & {:as search-ctx}]
(when-not @#'search.index/initialized?
(throw (ex-info "Search index is not initialized. Use [[init!]] to ensure it exists."
{:search-engine :postgres})))
- (->> (let [base-query (search.index/search-query search-term search-ctx [:legacy_input])]
- (search.permissions/add-collection-join-and-where-clauses base-query "search-index" search-ctx))
+ (->> (search.index/search-query search-term search-ctx [:legacy_input])
+ (add-collection-join-and-where-clauses search-ctx)
(search.scoring/with-scores search-ctx)
(search.filter/with-filters search-ctx)
(t2/query)
@@ -114,8 +126,8 @@
[search-ctx]
;; We ignore any current models filter
(let [search-ctx (assoc search-ctx :models search.config/all-models)]
- (->> (-> (search.index/search-query (:search-string search-ctx) search-ctx [[[:distinct :model] :model]])
- (search.permissions/add-collection-join-and-where-clauses "search-index" search-ctx))
+ (->> (search.index/search-query (:search-string search-ctx) search-ctx [[[:distinct :model] :model]])
+ (add-collection-join-and-where-clauses search-ctx)
(search.filter/with-filters search-ctx)
t2/query
(into #{} (map :model)))))
diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj
index d469ef73a25d3c24bae37ebc43ca777efa93234c..90c62acde4590f022339ddba0959f4db131219a7 100644
--- a/test/metabase/api/search_test.clj
+++ b/test/metabase/api/search_test.clj
@@ -313,7 +313,7 @@
(when (search/supports-index?)
(testing "It can use an alternate search engine"
(with-search-items-in-root-collection "test"
- (let [resp (search-request :crowberto :q "test" :search_engine "fulltext")]
+ (let [resp (search-request :crowberto :q "test" :search_engine "fulltext" :limit 1)]
;; The index is not populated here, so there's not much interesting to assert.
(is (= "search.engine/fulltext" (:engine resp))))))))
diff --git a/test/metabase/search/util_test.clj b/test/metabase/search/in_place/util_test.clj
similarity index 95%
rename from test/metabase/search/util_test.clj
rename to test/metabase/search/in_place/util_test.clj
index 980f14354ae5e9e404851e3b2712f4bfc01d06c7..5f4f1fe8b7cf5c7f52ee4652764f6168f5108039 100644
--- a/test/metabase/search/util_test.clj
+++ b/test/metabase/search/in_place/util_test.clj
@@ -1,7 +1,7 @@
-(ns metabase.search.util-test
+(ns metabase.search.in-place.util-test
(:require
[clojure.test :refer :all]
- [metabase.search.util :as search.util]))
+ [metabase.search.in-place.util :as search.util]))
(deftest ^:parallel tokenize-test
(testing "basic tokenization"