diff --git a/resources/log4j2.xml b/resources/log4j2.xml index f71bc3a827a85acfc6c1637f5a5f8dbcfd50a73c..cc2636dd4c97de6a6f7ed51150d1f10e3a8e7642 100644 --- a/resources/log4j2.xml +++ b/resources/log4j2.xml @@ -23,6 +23,7 @@ <Loggers> <Logger name="metabase" level="INFO"/> + <Logger name="metabase-enterprise" level="INFO"/> <Logger name="metabase.plugins" level="DEBUG"/> <Logger name="metabase.middleware" level="DEBUG"/> <Logger name="metabase.query-processor.async" level="DEBUG"/> diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index b8c0fae938bd789019992c4757017ba07fa19c93..6a9fc3a0c8377c9e9f9aed09c974f3d25a0b6099 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -7722,3 +7722,25 @@ databaseChangeLog: tableName: core_user columnName: email newDataType: varchar_ignorecase(254) + + - changeSet: + id: 273 + author: camsaul + comment: Added 0.37.1 + changes: + - addDefaultValue: + tableName: core_user + columnName: is_superuser + columnDataType: boolean + defaultValueBoolean: false + + - changeSet: + id: 274 + author: camsaul + comment: Added 0.37.1 + changes: + - addDefaultValue: + tableName: core_user + columnName: is_active + columnDataType: boolean + defaultValueBoolean: true diff --git a/src/metabase/models/collection/graph.clj b/src/metabase/models/collection/graph.clj index 338ce8a5f54bd8f728ce80555e188dffe4594cc4..18abbcc90a3abdc267f2fbdf24d08174927f7d08 100644 --- a/src/metabase/models/collection/graph.clj +++ b/src/metabase/models/collection/graph.clj @@ -1,6 +1,5 @@ (ns metabase.models.collection.graph (:require [clojure.data :as data] - [honeysql.helpers :as h] [metabase.api.common :as api :refer [*current-user-id*]] [metabase.models [collection :as collection :refer [Collection]] @@ -8,7 +7,9 @@ [permissions :as perms :refer [Permissions]] [permissions-group :refer [PermissionsGroup]]] [metabase.util :as u] - [metabase.util.schema :as su] + [metabase.util + [honeysql-extensions :as hx] + [schema :as su]] [schema.core :as s] [toucan.db :as db])) @@ -57,16 +58,13 @@ Collections (i.e., things that you can set Permissions for, and that should go in the graph.)" [collection-namespace :- (s/maybe su/KeywordOrString)] (let [personal-collection-ids (db/select-ids Collection :personal_owner_id [:not= nil]) - honeysql-form (cond-> {:select [[:id :id]] - :from [Collection] - :where [:= :namespace (u/qualified-name collection-namespace)]} - (seq personal-collection-ids) - (h/merge-where [:not-in :id (set personal-collection-ids)])) - honeysql-form (reduce - (fn [honeysql-form collection-id] - (h/merge-where honeysql-form [:not [:like :location (format "/%d/%%" collection-id)]])) - honeysql-form - personal-collection-ids)] + honeysql-form {:select [[:id :id]] + :from [Collection] + :where (into [:and + [:= :namespace (u/qualified-name collection-namespace)] + [:= :personal_owner_id nil]] + (for [collection-id personal-collection-ids] + [:not [:like :location (hx/literal (format "/%d/%%" collection-id))]]))}] (set (map :id (db/query honeysql-form))))) (s/defn graph :- PermissionsGraph diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj index 4d2bda5f79b99bc3cce57683f795860eeaae16cf..edc518a8718ce7526350f8ac3dc438aaad412bfa 100644 --- a/src/metabase/util/honeysql_extensions.clj +++ b/src/metabase/util/honeysql_extensions.clj @@ -186,8 +186,8 @@ (def ^{:arglists '([& exprs])} concat "SQL `concat` function." (partial hsql/call :concat)) ;; Etc (Dev Stuff) -(alter-meta! #'honeysql.core/format assoc :style/indent 1) -(alter-meta! #'honeysql.core/call assoc :style/indent 1) +(alter-meta! #'honeysql.core/format assoc :style/indent :defn) +(alter-meta! #'honeysql.core/call assoc :style/indent :defn) (require 'honeysql.types) (extend-protocol PrettyPrintable diff --git a/test/metabase/models/collection/graph_test.clj b/test/metabase/models/collection/graph_test.clj index 17cb9a66ef3125bc7e4bc0e54c9df9db5b3731ae..f9c6cfcc140ca2e9ee9eb715cd6076a8481024c5 100644 --- a/test/metabase/models/collection/graph_test.clj +++ b/test/metabase/models/collection/graph_test.clj @@ -1,7 +1,9 @@ (ns metabase.models.collection.graph-test - (:require [clojure.test :refer :all] + (:require [clojure.java.jdbc :as jdbc] + [clojure.test :refer :all] [medley.core :as m] [metabase + [models :refer [User]] [test :as mt] [util :as u]] [metabase.api.common :refer [*current-user-id*]] @@ -14,7 +16,8 @@ [metabase.test.fixtures :as fixtures] [metabase.util.schema :as su] [schema.core :as s] - [toucan.db :as db])) + [toucan.db :as db] + [toucan.util.test :as tt])) (use-fixtures :once (fixtures/initialize :db :test-users :test-users-personal-collections)) @@ -415,3 +418,38 @@ :after {(keyword (str group-id)) {:root (s/eq "write")}} s/Keyword s/Any} (db/select-one CollectionRevision {:order-by [[:id :desc]]}))))))))))) + +(defn- do-with-n-temp-users-with-personal-collections! [num-users thunk] + (mt/with-model-cleanup [User Collection] + ;; insert all the users + (let [user-ids (jdbc/execute! + (db/connection) + (db/honeysql->sql + {:insert-into User + :values (repeatedly num-users #(assoc (tt/with-temp-defaults User) :date_joined :%now))})) + max-id (:max-id (db/select-one [User [:%max.id :max-id]])) + ;; determine the range of IDs we inserted -- MySQL doesn't support INSERT INTO ... RETURNING like Postgres + ;; so this is the fastest way to do this + user-ids (range (inc (- max-id num-users)) (inc max-id))] + (assert (= (count user-ids) num-users)) + ;; insert the Collections + (jdbc/execute! + (db/connection) + (db/honeysql->sql + {:insert-into Collection + :values (for [user-id user-ids + :let [collection (tt/with-temp-defaults Collection)]] + (assoc collection + :personal_owner_id user-id + :slug "my_collection"))}))) + ;; now run the thunk + (thunk))) + +(defmacro ^:private with-n-temp-users-with-personal-collections [num-users & body] + `(do-with-n-temp-users-with-personal-collections! ~num-users (fn [] ~@body))) + +(deftest mega-graph-test + (testing "A truly insane amount of Personal Collections shouldn't cause a Stack Overflow (#13211)" + (with-n-temp-users-with-personal-collections 2000 + (is (>= (db/count Collection :personal_owner_id [:not= nil]) 2000)) + (is (map? (graph/graph)))))) diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index aba9e2a1c282bcd2f6cd412f544d65e1490d875b..741b651af5c11739feb3c77afc7e738a6f6f14aa 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -561,7 +561,7 @@ ;; might not have an old max ID if this is the first time the macro is used in this test run. :let [old-max-id (or (get model->old-max-id model) 0)]] - (db/delete! model :id [:> old-max-id])))))) + (db/simple-delete! model :id [:> old-max-id])))))) (defmacro with-model-cleanup "Execute `body`, then delete any *new* rows created for each model in `models`. Calls `delete!`, so if the model has