From 2e328a254ca97964e209f354416d4151d7030822 Mon Sep 17 00:00:00 2001
From: Cal Herries <39073188+calherries@users.noreply.github.com>
Date: Wed, 31 May 2023 12:56:32 +0300
Subject: [PATCH] Apply schema filters in Admin > Uploads > Select a schema
dropdown (#31029)
* Apply inclusion and exclusion patterns to `driver/syncable-schemas` for sql-jdbc drivers
* Fix kondo
* Fix postgres test
* Remove unnecessary test DB names
---
src/metabase/driver/sql_jdbc.clj | 7 +++-
test/metabase/driver/h2_test.clj | 2 +-
test/metabase/driver/mysql_test.clj | 2 +-
test/metabase/driver/postgres_test.clj | 4 +--
test/metabase/driver/sql_jdbc_test.clj | 46 ++++++++++++++++++++++++--
5 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/src/metabase/driver/sql_jdbc.clj b/src/metabase/driver/sql_jdbc.clj
index 04f35b7d4da..dae5b5f77b4 100644
--- a/src/metabase/driver/sql_jdbc.clj
+++ b/src/metabase/driver/sql_jdbc.clj
@@ -10,6 +10,8 @@
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync.interface]
[metabase.driver.sql.query-processor :as sql.qp]
+ [metabase.driver.sync :as driver.s]
+ [metabase.driver.util :as driver.u]
[metabase.query-processor.writeback :as qp.writeback]
[metabase.util.honeysql-extensions :as hx]))
@@ -145,4 +147,7 @@
(defmethod driver/syncable-schemas :sql-jdbc
[driver database]
(with-open [conn ^java.sql.Connection (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec database))]
- (into #{} (sql-jdbc.sync.interface/filtered-syncable-schemas driver conn (.getMetaData conn) nil nil))))
+ (let [schema-filter-prop (driver.u/find-schema-filters-prop driver)
+ [inclusion-patterns
+ exclusion-patterns] (driver.s/db-details->schema-filter-patterns (:name schema-filter-prop) database)]
+ (into #{} (sql-jdbc.sync.interface/filtered-syncable-schemas driver conn (.getMetaData conn) inclusion-patterns exclusion-patterns)))))
diff --git a/test/metabase/driver/h2_test.clj b/test/metabase/driver/h2_test.clj
index 7e7e79ee18d..d649ba5ca8e 100644
--- a/test/metabase/driver/h2_test.clj
+++ b/test/metabase/driver/h2_test.clj
@@ -287,7 +287,7 @@
(testing "`syncable-schemas` should return schemas that should be synced"
(mt/with-empty-db
(is (= #{"PUBLIC"}
- (driver/syncable-schemas driver/*driver* (mt/id))))))))
+ (driver/syncable-schemas driver/*driver* (mt/db))))))))
(deftest syncable-audit-db-test
(mt/test-driver :h2
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
index 72251cb8df1..22b2dca11e9 100644
--- a/test/metabase/driver/mysql_test.clj
+++ b/test/metabase/driver/mysql_test.clj
@@ -630,4 +630,4 @@
(testing "`syncable-schemas` should return an empty set because mysql doesn't support schemas"
(mt/with-empty-db
(is (= #{}
- (driver/syncable-schemas driver/*driver* (mt/id))))))))
+ (driver/syncable-schemas driver/*driver* (mt/db))))))))
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 82ebe1b9bb4..013c54a190a 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -1172,7 +1172,7 @@
(testing "`syncable-schemas` should return schemas that should be synced"
(mt/with-empty-db
(is (= #{"public"}
- (driver/syncable-schemas driver/*driver* (mt/id))))))
+ (driver/syncable-schemas driver/*driver* (mt/db))))))
(testing "metabase_cache schemas should be excluded"
(mt/dataset test-data
(mt/with-persistence-enabled [persist-models!]
@@ -1185,4 +1185,4 @@
(is (some (partial re-matches #"metabase_cache(.*)")
(map :schema_name (jdbc/query conn-spec "SELECT schema_name from INFORMATION_SCHEMA.SCHEMATA;"))))
(is (nil? (some (partial re-matches #"metabase_cache(.*)")
- (driver/syncable-schemas driver/*driver* (mt/id))))))))))))
+ (driver/syncable-schemas driver/*driver* (mt/db))))))))))))
diff --git a/test/metabase/driver/sql_jdbc_test.clj b/test/metabase/driver/sql_jdbc_test.clj
index d34ce4cba2f..a47e05e9921 100644
--- a/test/metabase/driver/sql_jdbc_test.clj
+++ b/test/metabase/driver/sql_jdbc_test.clj
@@ -1,14 +1,16 @@
(ns metabase.driver.sql-jdbc-test
(:require
+ [clojure.set :as set]
[clojure.test :refer :all]
[metabase.db.metadata-queries :as metadata-queries]
[metabase.driver :as driver]
+ [metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
[metabase.driver.sql-jdbc.test-util :as sql-jdbc.tu]
[metabase.driver.util :as driver.u]
- [metabase.models.field :refer [Field]]
- [metabase.models.table :as table :refer [Table]]
+ [metabase.models :refer [Database Field Table]]
[metabase.query-processor :as qp]
[metabase.test :as mt]
+ [metabase.util :as u]
[toucan2.core :as t2]))
(set! *warn-on-reflection* true)
@@ -219,3 +221,43 @@
(is (= 2
(mt/$ids users
(spliced-count-of :users [:= $last_login_time "09:30"]))))))))))
+
+(defn- find-schema-filters-prop [driver]
+ (first (filter (fn [conn-prop]
+ (= :schema-filters (keyword (:type conn-prop))))
+ (driver/connection-properties driver))))
+
+(deftest syncable-schemas-with-schema-filters-test
+ (mt/test-driver (set (for [driver (set/intersection (sql-jdbc.tu/sql-jdbc-drivers)
+ (mt/normal-drivers-with-feature :actions))
+ :when (driver.u/find-schema-filters-prop driver)]
+ driver))
+ (let [fake-schema-name (u/qualified-name ::fake-schema)]
+ (with-redefs [sql-jdbc.describe-database/all-schemas (let [orig sql-jdbc.describe-database/all-schemas]
+ (fn [metadata]
+ (eduction
+ cat
+ [(orig metadata) [fake-schema-name]])))]
+ (is (= #{"public" fake-schema-name}
+ (driver/syncable-schemas driver/*driver* (mt/db))))
+ (let [driver (driver.u/database->driver (mt/db))
+ schema-filter-prop (find-schema-filters-prop driver)
+ filter-type-prop (keyword (str (:name schema-filter-prop) "-type"))
+ patterns-type-prop (keyword (str (:name schema-filter-prop) "-patterns"))]
+ (testing "syncable-schemas works as expected"
+ (testing " with an inclusion filter"
+ (mt/with-temp Database [db-filtered {:engine driver
+ :details (-> (mt/db)
+ :details
+ (assoc filter-type-prop "inclusion"
+ patterns-type-prop "public"))}]
+ (is (= #{"public"}
+ (driver/syncable-schemas driver/*driver* db-filtered)))))
+ (testing " with an exclusion filter"
+ (mt/with-temp Database [db-filtered {:engine driver
+ :details (-> (mt/db)
+ :details
+ (assoc filter-type-prop "exclusion"
+ patterns-type-prop "public"))}]
+ (is (= #{fake-schema-name}
+ (driver/syncable-schemas driver/*driver* db-filtered)))))))))))
--
GitLab