Fix collections graph stack overflow (#13618)

* Fix collections/graph stack overflow

* Test fixes 

 [ci postgres] [ci mysql]

* Show engine in Task trouble shooting logs (#13608)

* Show engine in Task trouble shooting logs

in an effort to get better feedback when people raise issues about
long running sync processes, include the database engine in the logs.

Also wrapped `th` in a `tr` inside the `thead` to remove react warning

* Const intead of let on the db_id_to_engine

* Swap db id for db name

* Move comment out of jsx

* fixup! Merge branch 'release-x.37.x' into fix-collections-graph-stack-overflow

Co-authored-by: dpsutton <dan@dpsutton.com>

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"/>

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

src/metabase/models/collection/graph.clj

 (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

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

test/metabase/models/collection/graph_test.clj

 (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))))))

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