Skip to content
Snippets Groups Projects
Unverified Commit 0e17e3b5 authored by Ngoc Khuat's avatar Ngoc Khuat Committed by GitHub
Browse files

Redshift and Oracle test sync: do not use hack (#40052)

parent bc886925
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
modules/drivers/oracle/test/metabase/test/data/oracle.clj
+ 13
− 30
View file @ 0e17e3b5
(ns metabase.test.data.oracle
(:require
[clojure.java.jdbc :as jdbc]
[clojure.set :as set]
[clojure.string :as str]
[clojure.test :refer :all]
[honey.sql :as sql]
[medley.core :as m]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.test.data.interface :as tx]
[metabase.test.data.sql :as sql.tx]
......@@ -38,13 +36,15 @@
;; Session password is only used when creating session user, not anywhere else
(defn- connection-details []
(let [details* {:host (tx/db-test-env-var-or-throw :oracle :host "localhost")
:port (Integer/parseInt (tx/db-test-env-var-or-throw :oracle :port "1521"))
:user (tx/db-test-env-var :oracle :user)
:password (tx/db-test-env-var :oracle :password)
:sid (tx/db-test-env-var :oracle :sid)
:service-name (tx/db-test-env-var :oracle :service-name (when-not (tx/db-test-env-var :oracle :sid) "XEPDB1"))
:ssl (tx/db-test-env-var :oracle :ssl false)}
(let [details* {:host (tx/db-test-env-var-or-throw :oracle :host "localhost")
:port (Integer/parseInt (tx/db-test-env-var-or-throw :oracle :port "1521"))
:user (tx/db-test-env-var :oracle :user)
:password (tx/db-test-env-var :oracle :password)
:sid (tx/db-test-env-var :oracle :sid)
:service-name (tx/db-test-env-var :oracle :service-name (when-not (tx/db-test-env-var :oracle :sid) "XEPDB1"))
:ssl (tx/db-test-env-var :oracle :ssl false)
:schema-filters-type "inclusion"
:schema-filters-patterns session-schema}
ssl-keys [:ssl-use-truststore :ssl-truststore-options :ssl-truststore-path :ssl-truststore-value
:ssl-truststore-password-value
:ssl-use-keystore :ssl-keystore-options :ssl-keystore-path :ssl-keystore-value
......@@ -60,6 +60,10 @@
(and (nil? (:password details*)) (not (:ssl-use-keystore details*)))
(assoc :password "password"))))
(defn- dbspec [& _]
(let [conn-details (connection-details)]
(sql-jdbc.conn/connection-details->spec :oracle conn-details)))
(defmethod tx/dbdef->connection-details :oracle [& _]
(connection-details))
......@@ -201,27 +205,6 @@
(update 0 (partial driver/prettify-native-form :oracle))
(update 0 str/split-lines))))))
(defn- dbspec [& _]
(let [conn-details (connection-details)]
(sql-jdbc.conn/connection-details->spec :oracle conn-details)))
(defn- non-session-schemas
"Return a set of the names of schemas (users) that are not meant for use in this test session (i.e., ones that should
be ignored). (This is used as part of the implementation of `excluded-schemas` for the Oracle driver during tests.)"
[]
(set (map :username (jdbc/query (dbspec) ["SELECT username FROM dba_users WHERE username <> ?" session-schema]))))
(defonce ^:private original-excluded-schemas
(get-method sql-jdbc.sync/excluded-schemas :oracle))
(defmethod sql-jdbc.sync/excluded-schemas :oracle
[driver]
(set/union
(original-excluded-schemas driver)
;; This is similar hack we do for Redshift, see the explanation there we just want to ignore all the test
;; "session schemas" that don't match the current test
(non-session-schemas)))
;;; Clear out the session schema before and after tests run
;; TL;DR Oracle schema == Oracle user. Create new user for session-schema
......
modules/drivers/redshift/test/metabase/driver/redshift_test.clj
+ 46
− 49
View file @ 0e17e3b5
......@@ -295,58 +295,55 @@
random-schema
temp-username)
(try
(binding [redshift.test/*use-original-filtered-syncable-schemas-impl?* true]
(t2.with-temp/with-temp [Database db {:engine :redshift, :details (assoc db-det :user temp-username :password user-pw)}]
(sql-jdbc.execute/do-with-connection-with-options
:redshift
db
nil
(fn [^java.sql.Connection conn]
(let [schemas (reduce conj
#{}
(sql-jdbc.sync/filtered-syncable-schemas :redshift
conn
(.getMetaData conn)
nil
nil))]
(testing "filtered-syncable-schemas for the user should contain the newly created random schema"
(is (contains? schemas random-schema)))
(testing "should not contain the current session-schema name (since that was never granted)"
(is (not (contains? schemas (redshift.test/unique-session-schema))))))))))
(finally
(execute! (str "REVOKE USAGE ON SCHEMA %s FROM %s;%n"
"DROP USER IF EXISTS %s;%n"
"DROP SCHEMA IF EXISTS %s;%n")
random-schema
temp-username
temp-username
random-schema)))))
(t2.with-temp/with-temp [Database db {:engine :redshift, :details (assoc db-det :user temp-username :password user-pw)}]
(sql-jdbc.execute/do-with-connection-with-options
:redshift
db
nil
(fn [^java.sql.Connection conn]
(let [schemas (reduce conj
#{}
(sql-jdbc.sync/filtered-syncable-schemas :redshift
conn
(.getMetaData conn)
nil
nil))]
(testing "filtered-syncable-schemas for the user should contain the newly created random schema"
(is (contains? schemas random-schema)))
(testing "should not contain the current session-schema name (since that was never granted)"
(is (not (contains? schemas (redshift.test/unique-session-schema)))))))))
(finally
(execute! (str "REVOKE USAGE ON SCHEMA %s FROM %s;%n"
"DROP USER IF EXISTS %s;%n"
"DROP SCHEMA IF EXISTS %s;%n")
random-schema
temp-username
temp-username
random-schema)))))
(testing "Should filter out non-existent schemas (for which nobody has permissions)"
(let [fake-schema-name (u/qualified-name ::fake-schema)]
(binding [redshift.test/*use-original-filtered-syncable-schemas-impl?* true]
;; override `all-schemas` so it returns our fake schema in addition to the real ones.
(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]])))]
(sql-jdbc.execute/do-with-connection-with-options
:redshift
(mt/db)
nil
(fn [^java.sql.Connection conn]
(letfn [(schemas []
(reduce
conj
#{}
(sql-jdbc.sync/filtered-syncable-schemas :redshift conn (.getMetaData conn) nil nil)))]
(testing "if schemas-with-usage-permissions is disabled, the ::fake-schema should come back"
(with-redefs [redshift/reducible-schemas-with-usage-permissions (fn [_ reducible]
reducible)]
(is (contains? (schemas) fake-schema-name))))
(testing "normally, ::fake-schema should be filtered out (because it does not exist)"
(is (not (contains? (schemas) fake-schema-name)))))))))))))
(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]])))]
(sql-jdbc.execute/do-with-connection-with-options
:redshift
(mt/db)
nil
(fn [^java.sql.Connection conn]
(letfn [(schemas []
(reduce
conj
#{}
(sql-jdbc.sync/filtered-syncable-schemas :redshift conn (.getMetaData conn) nil nil)))]
(testing "if schemas-with-usage-permissions is disabled, the ::fake-schema should come back"
(with-redefs [redshift/reducible-schemas-with-usage-permissions (fn [_ reducible]
reducible)]
(is (contains? (schemas) fake-schema-name))))
(testing "normally, ::fake-schema should be filtered out (because it does not exist)"
(is (not (contains? (schemas) fake-schema-name))))))))))))
(deftest sync-materialized-views-test
(mt/test-driver :redshift
......
modules/drivers/redshift/test/metabase/test/data/redshift.clj
+ 10
− 26
View file @ 0e17e3b5
......@@ -4,7 +4,6 @@
[java-time.api :as t]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.test-util.unique-prefix :as sql.tu.unique-prefix]
[metabase.test.data.interface :as tx]
[metabase.test.data.sql :as sql.tx]
......@@ -39,20 +38,22 @@
[_ _]
(throw (UnsupportedOperationException. "Redshift does not have a TIME data type.")))
(defn unique-session-schema []
(str (sql.tu.unique-prefix/unique-prefix) "schema"))
(def db-connection-details
(delay {:host (tx/db-test-env-var-or-throw :redshift :host)
:port (Integer/parseInt (tx/db-test-env-var-or-throw :redshift :port "5439"))
:db (tx/db-test-env-var-or-throw :redshift :db)
:user (tx/db-test-env-var-or-throw :redshift :user)
:password (tx/db-test-env-var-or-throw :redshift :password)}))
(delay {:host (tx/db-test-env-var-or-throw :redshift :host)
:port (Integer/parseInt (tx/db-test-env-var-or-throw :redshift :port "5439"))
:db (tx/db-test-env-var-or-throw :redshift :db)
:user (tx/db-test-env-var-or-throw :redshift :user)
:password (tx/db-test-env-var-or-throw :redshift :password)
:schema-filters-type "inclusion"
:schema-filters-patterns (str "spectrum," (unique-session-schema))}))
(defmethod tx/dbdef->connection-details :redshift
[& _]
@db-connection-details)
(defn unique-session-schema []
(str (sql.tu.unique-prefix/unique-prefix) "schema"))
(defmethod sql.tx/create-db-sql :redshift [& _] nil)
(defmethod sql.tx/drop-db-if-exists-sql :redshift [& _] nil)
......@@ -191,20 +192,3 @@
(sql-jdbc.conn/connection-details->spec driver @db-connection-details)
{:write? true}
delete-session-schema!))
(defonce ^:private ^{:arglists '([driver connection metadata _ _])}
original-filtered-syncable-schemas
(get-method sql-jdbc.sync/filtered-syncable-schemas :redshift))
(def ^:dynamic *use-original-filtered-syncable-schemas-impl?*
"Whether to use the actual prod impl for `filtered-syncable-schemas` rather than the special test one that only syncs
the test schema."
false)
;; replace the impl the `metabase.driver.redshift`. Only sync the current test schema and the external "spectrum"
;; schema used for a specific test.
(defmethod sql-jdbc.sync/filtered-syncable-schemas :redshift
[driver conn metadata schema-inclusion-filters schema-exclusion-filters]
(if *use-original-filtered-syncable-schemas-impl?*
(original-filtered-syncable-schemas driver conn metadata schema-inclusion-filters schema-exclusion-filters)
#{(unique-session-schema) "spectrum"}))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment