Skip to content
Snippets Groups Projects
Unverified Commit 508b9931 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Fix NPE in schema pattern matching fns in sync (#38161)

* Fix NPE in schema pattern matching fns in sync

* Require PG
parent bdfaf72f
No related branches found
No related tags found
No related merge requests found
......@@ -40,11 +40,16 @@
:else
(let [inclusion? exclusion-blank?
pattern (schema-pattern->re-pattern (if inclusion? inclusion-patterns exclusion-patterns))]
(fn [s]
(let [m (.matcher pattern s)
matches? (.matches m)]
(if inclusion? matches? (not matches?))))))))
pattern (schema-pattern->re-pattern (if inclusion? inclusion-patterns exclusion-patterns))
match-fn (fn match-fn [^String s]
(when s
(let [m (.matcher pattern s)]
(.matches m))))]
;; for inclusion patterns, never match a `nil` schema; for exclusion patterns, always consider a `nil` schema to
;; be ok
(if inclusion?
match-fn
(complement match-fn))))))
(def ^:private schema-patterns->filter-fn (memoize schema-patterns->filter-fn*))
......@@ -56,12 +61,13 @@
(let [{prop-name :name} (driver.u/find-schema-filters-prop (driver.u/database->driver database))]
(db-details->schema-filter-patterns prop-name database)))
([prop-nm {db-details :details :as _database}]
(let [schema-filter-type (get db-details (keyword (str prop-nm "-type")))
schema-filter-patterns (get db-details (keyword (str prop-nm "-patterns")))]
(case schema-filter-type
"exclusion" [nil schema-filter-patterns]
"inclusion" [schema-filter-patterns nil]
[nil nil]))))
(when prop-nm
(let [schema-filter-type (get db-details (keyword (str prop-nm "-type")))
schema-filter-patterns (get db-details (keyword (str prop-nm "-patterns")))]
(case schema-filter-type
"exclusion" [nil schema-filter-patterns]
"inclusion" [schema-filter-patterns nil]
[nil nil])))))
(defn include-schema?
"Returns true if the given `schema-name` should be included/synced, considering the given `inclusion-patterns` and
......
(ns metabase.driver.sync-test
(:require
[clojure.test :as t]
[clojure.test :refer :all]
[metabase.driver.postgres]
[metabase.driver.sync :as driver.s])
(:import
(clojure.lang ExceptionInfo)))
(t/deftest ^:parallel schema-filter-test
(comment metabase.driver.postgres/keep-me) ; this is used by [[schema-filter-NPE-test]] below
(deftest ^:parallel schema-filter-test
(doseq [[test-kind expect-match? schema-name inclusion-filters exclusion-filters]
[["nil filters" true "foo" nil nil]
["blank filters" true "foo" "" ""]
......@@ -20,10 +23,23 @@
["exclusion filter with commas and wildcards (include)" true "foo" "" "ba*,fob"]
["exclusion filter with commas and wildcards (exclude)" false "foo" "" "bar,baz,fo*"]
["multiple inclusion with whitespace trimming" true "bar" " foo , bar \n , \nbaz " ""]]]
(t/testing (str "include-schema? works as expected for " test-kind)
(t/is (= expect-match? (driver.s/include-schema? inclusion-filters exclusion-filters schema-name))))
(t/testing "include-schema? throws an exception if both patterns are specified"
(t/is (thrown-with-msg?
ExceptionInfo
#"Inclusion and exclusion patterns cannot both be specified"
(driver.s/include-schema? "foo" "bar" "whatever"))))))
(testing (str "include-schema? works as expected for " test-kind)
(is (= expect-match? (driver.s/include-schema? inclusion-filters exclusion-filters schema-name))))
(testing "include-schema? throws an exception if both patterns are specified"
(is (thrown-with-msg?
ExceptionInfo
#"Inclusion and exclusion patterns cannot both be specified"
(driver.s/include-schema? "foo" "bar" "whatever"))))))
(deftest ^:parallel schema-filter-NPE-test
(testing "Schema filter function should not NPE if you pass in a `nil` schema (#38156)"
(testing "inclusion -- don't include nil schemas"
(let [db {:details {:schema-filters-type "inclusion"
:schema-filters-patterns "x"}
:engine :postgres}]
(is (not (driver.s/include-schema? db nil)))))
(testing "exclusion -- don't exclude nil schemas"
(let [db {:details {:schema-filters-type "exclusion"
:schema-filters-patterns "x"}
:engine :postgres}]
(is (driver.s/include-schema? db nil))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment