Merge pull request #287 from dpsutton/changes-for-metabase-53

Update for metabase 53/master

Author: slvrtrn

.github/workflows/check.yml

@@ -12,7 +12,7 @@ on:
 env:
   # Temporarily using a fork to disable a few failing tests
   METABASE_REPOSITORY: slvrtrn/metabase
-  METABASE_VERSION: 0.52.x-ch
+  METABASE_VERSION: 0.53.x-ch
 
 jobs:
   check-local-current-version:

test/metabase/driver/clickhouse_impersonation_test.clj

@@ -8,7 +8,7 @@
             [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
             [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
             [metabase.query-processor.store :as qp.store]
-            [metabase.sync :as sync]
+            [metabase.sync.core :as sync]
             [metabase.test :as mt]
             [metabase.test.data.clickhouse :as ctd]
             [metabase.util :as u]
@@ -156,26 +156,25 @@
        (t2.with-temp/with-temp [:model/Database db cluster-details]
          (mt/with-db db (sync/sync-database! db)
 
-           (defn- check-impersonation!
-             [roles expected]
-             (advanced-perms.api.tu/with-impersonations!
-               {:impersonations [{:db-id (mt/id) :attribute "impersonation_attr"}]
-                :attributes     {"impersonation_attr" roles}}
-               (is (= expected
-                      (-> {:query select-query}
-                          mt/native-query
-                          mt/process-query
-                          mt/rows)))))
+           (letfn [(check-impersonation! [roles expected]
+                      (advanced-perms.api.tu/with-impersonations!
+                        {:impersonations [{:db-id (mt/id) :attribute "impersonation_attr"}]
+                         :attributes     {"impersonation_attr" roles}}
+                        (is (= expected
+                               (-> {:query select-query}
+                                   mt/native-query
+                                   mt/process-query
+                                   mt/rows)))))]
 
-           (is (= [["a"] ["b"] ["c"]]
-                  (-> {:query select-query}
-                      mt/native-query
-                      mt/process-query
-                      mt/rows)))
+             (is (= [["a"] ["b"] ["c"]]
+                    (-> {:query select-query}
+                        mt/native-query
+                        mt/process-query
+                        mt/rows)))
 
-           (check-impersonation! "row_a" [["a"]])
-           (check-impersonation! "row_b" [["b"]])
-           (check-impersonation! "row_c" [["c"]])
-           (check-impersonation! "row_a,row_c" [["a"] ["c"]])
-           (check-impersonation! "row_b,row_c" [["b"] ["c"]])
-           (check-impersonation! "row_a,row_b,row_c" [["a"] ["b"] ["c"]])))))))
+             (check-impersonation! "row_a" [["a"]])
+             (check-impersonation! "row_b" [["b"]])
+             (check-impersonation! "row_c" [["c"]])
+             (check-impersonation! "row_a,row_c" [["a"] ["c"]])
+             (check-impersonation! "row_b,row_c" [["b"] ["c"]])
+             (check-impersonation! "row_a,row_b,row_c" [["a"] ["b"] ["c"]]))))))))

test/metabase/driver/clickhouse_test.clj

@@ -39,9 +39,9 @@
             (driver/db-default-timezone :clickhouse spec))))))
 
 (deftest ^:parallel clickhouse-connection-string
-  (mt/with-dynamic-redefs [;; This function's implementation requires the connection details to actually connect to the
-                           ;; database, which is orthogonal to the purpose of this test.
-                           clickhouse/cloud? (constantly false)]
+  (mt/with-dynamic-fn-redefs [ ;; This function's implementation requires the connection details to actually connect to the
+                              ;; database, which is orthogonal to the purpose of this test.
+                              clickhouse/cloud? (constantly false)]
     (testing "connection with no additional options"
       (is (= ctd/default-connection-params
              (sql-jdbc.conn/connection-details->spec
@@ -72,9 +72,9 @@
               {:dbname nil}))))))
 
 (deftest ^:parallel clickhouse-connection-string-select-sequential-consistency
-  (mt/with-dynamic-redefs [;; This function's implementation requires the connection details to actually
-                           ;; connect to the database, which is orthogonal to the purpose of this test.
-                           clickhouse/cloud? (constantly true)]
+  (mt/with-dynamic-fn-redefs [ ;; This function's implementation requires the connection details to actually
+                              ;; connect to the database, which is orthogonal to the purpose of this test.
+                              clickhouse/cloud? (constantly true)]
     (testing "connection with no additional options"
       (is (= (assoc ctd/default-connection-params :select_sequential_consistency true)
              (sql-jdbc.conn/connection-details->spec

test/metabase/test/data/clickhouse.clj

@@ -1,17 +1,19 @@
 (ns metabase.test.data.clickhouse
   "Code for creating / destroying a ClickHouse database from a `DatabaseDefinition`."
   (:require
+   [clojure.java.io :as io]
    [clojure.java.jdbc :as jdbc]
    [clojure.string :as str]
    [clojure.test :refer :all]
    [metabase.db.query :as mdb.query]
+   [metabase.driver :as driver]
    [metabase.driver.ddl.interface :as ddl.i]
    [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
    [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
    [metabase.driver.sql.util :as sql.u]
    [metabase.lib.schema.common :as lib.schema.common]
    [metabase.query-processor.test-util :as qp.test]
-   [metabase.sync.sync-metadata :as sync-metadata]
+   [metabase.sync.core :as sync]
    [metabase.test.data.interface :as tx]
    [metabase.test.data.sql :as sql.tx]
    [metabase.test.data.sql-jdbc :as sql-jdbc.tx]
@@ -24,6 +26,11 @@
 
 (sql-jdbc.tx/add-test-extensions! :clickhouse)
 
+(defmethod driver/database-supports? [:clickhouse :metabase.driver.sql-jdbc.sync.describe-table-test/describe-view-fields]
+  [_driver _feature _db] true)
+(defmethod driver/database-supports? [:clickhouse :metabase.driver.sql-jdbc.sync.describe-table-test/describe-materialized-view-fields]
+  [_driver _feature _db] false)
+
 (def default-connection-params
   {:classname "com.clickhouse.jdbc.ClickHouseDriver"
    :subprotocol "clickhouse"
@@ -178,7 +185,8 @@
       ;; (println "### Executing create-test-db! with details:" details)
       (jdbc/with-db-connection
         [spec (sql-jdbc.conn/connection-details->spec :clickhouse (merge {:engine :clickhouse} details))]
-        (let [statements (as-> (slurp "modules/drivers/clickhouse/test/metabase/test/data/datasets.sql") s
+        (let [raw-statements (slurp (io/resource "metabase/test/data/clickhouse_datasets.sql"))
+              statements (as-> raw-statements s
                            (str/split s #";")
                            (map str/trim s)
                            (filter seq s))]
@@ -216,7 +224,7 @@
     [:model/Database database
      {:engine :clickhouse
       :details (tx/dbdef->connection-details :clickhouse :db {:database-name "metabase_test"})}]
-    (sync-metadata/sync-db-metadata! database)
+    (sync/sync-db-metadata! database)
     (f database)))
 
 (defmethod tx/dataset-already-loaded? :clickhouse