diff --git a/deps.edn b/deps.edn index 5e8f64237ed9545006f129ad6ea78adf9e3d754e..f9832452b2a223664627db6490a86d10b1821fac 100644 --- a/deps.edn +++ b/deps.edn @@ -95,7 +95,7 @@ org.bouncycastle/bcpkix-jdk18on org.bouncycastle/bcprov-jdk18on]} metabase/throttle {:mvn/version "1.0.2"} ; Tools for throttling access to API endpoints and other code pathways - metosin/malli {:mvn/version "0.14.0"} ; Data-driven Schemas for Clojure/Script and babashka + metosin/malli {:mvn/version "0.16.3"} ; Data-driven Schemas for Clojure/Script and babashka nano-id/nano-id {:mvn/version "1.1.0"} ; NanoID generator for generating entity_ids net.cgrand/macrovich {:mvn/version "0.2.2"} ; utils for writing macros for both Clojure & ClojureScript net.clojars.wkok/openai-clojure {:mvn/version "0.16.0" diff --git a/src/metabase/analyze/schema.clj b/src/metabase/analyze/schema.clj index d727d8f79aacfb3d5e738825fcc31b45bb76d66f..574da20d2132d66fc4a5f53da6297a4f1288f6c1 100644 --- a/src/metabase/analyze/schema.clj +++ b/src/metabase/analyze/schema.clj @@ -1,17 +1,10 @@ (ns metabase.analyze.schema "Schemas used by the analyze code." (:require - [clojure.string :as str] [metabase.util.malli.registry :as mr] [metabase.util.malli.schema :as ms])) -(mr/def ::no-kebab-case-keys - [:fn - {:error/message "Map should not contain any kebab-case keys"} - (fn [m] - (every? (fn [k] - (not (str/includes? k "-"))) - (keys m)))]) +(mr/def ::no-kebab-case-keys (ms/MapWithNoKebabKeys)) (mr/def ::Table [:and diff --git a/src/metabase/lib/metadata/protocols.cljc b/src/metabase/lib/metadata/protocols.cljc index 95ff2d51446fd6b9e9e0a756852b97cd23bec1c1..a90493c20d5142e71dad2f70e4d87c8341a941b9 100644 --- a/src/metabase/lib/metadata/protocols.cljc +++ b/src/metabase/lib/metadata/protocols.cljc @@ -61,7 +61,8 @@ (defn metadata-provider? "Whether `x` is a valid [[MetadataProvider]]." [x] - (satisfies? MetadataProvider x)) + #?(:clj (extends? MetadataProvider (class x)) + :cljs (satisfies? MetadataProvider x))) (mr/def ::metadata-provider "Schema for something that satisfies the [[metabase.lib.metadata.protocols/MetadataProvider]] protocol." @@ -158,7 +159,8 @@ (defn cached-metadata-provider? "Whether `x` is a valid [[CachedMetadataProvider]]." [x] - (satisfies? CachedMetadataProvider x)) + #?(:clj (extends? CachedMetadataProvider (class x)) + :cljs (satisfies? CachedMetadataProvider x))) (mr/def ::cached-metadata-provider [:fn diff --git a/src/metabase/lib/schema.cljc b/src/metabase/lib/schema.cljc index f07bc9e65b7cb3237467a3ae36ae409967895685..ce2c6a4c1f6b97ab915f2a13bb637ee0ff75eb2b 100644 --- a/src/metabase/lib/schema.cljc +++ b/src/metabase/lib/schema.cljc @@ -106,10 +106,12 @@ "For ref validation purposes we should ignore `:joins` and any namespaced keys that might be used to record additional info e.g. `:lib/metadata`." [stage] - (select-keys stage (into [] - (comp (filter simple-keyword?) - (remove (partial = :joins))) - (keys stage)))) + (reduce-kv (fn [acc k _] + (if (or (qualified-keyword? k) + (= k :joins)) + (dissoc acc k) + acc)) + stage stage)) (defn- expression-ref-errors-for-stage [stage] (let [expression-names (into #{} (map (comp :lib/expression-name second)) (:expressions stage))] diff --git a/src/metabase/lib/schema/util.cljc b/src/metabase/lib/schema/util.cljc index 4eae642f594809d5c4b6d5dda3593862db730f3f..ef4163e4561fac2b7743fb60dd0eef417e4f650f 100644 --- a/src/metabase/lib/schema/util.cljc +++ b/src/metabase/lib/schema/util.cljc @@ -52,11 +52,6 @@ (str "Duplicate :lib/uuid " (pr-str (find-duplicate-uuid value))))} #'unique-uuids?]) -(defn remove-namespaced-keys - "Remove all the namespaced keys from a map." - [m] - (into {} (remove (fn [[k _v]] (qualified-keyword? k))) m)) - (defn distinct-refs? "Is a sequence of `refs` distinct for the purposes of appearing in `:fields` or `:breakouts` (ignoring keys that aren't important such as namespaced keys and type info)?" @@ -66,7 +61,13 @@ (apply distinct? (for [ref refs] - (lib.options/update-options ref (fn [options] - (-> options - remove-namespaced-keys - (dissoc :base-type :effective-type)))))))) + (let [options (lib.options/options ref)] + (lib.options/with-options ref + ;; Using reduce-kv to remove namespaced keys and some other keys to perform the comparison. + (reduce-kv (fn [acc k _] + (if (or (qualified-keyword? k) + (= k :base-type) + (= k :effective-type)) + (dissoc acc k) + acc)) + options options))))))) diff --git a/src/metabase/sync/interface.clj b/src/metabase/sync/interface.clj index ee1205465c84aad1cc9657223f03c35b7e7371b3..d3a850d41955a8c2721995a9091184362745c037 100644 --- a/src/metabase/sync/interface.clj +++ b/src/metabase/sync/interface.clj @@ -1,7 +1,6 @@ (ns metabase.sync.interface "Schemas and constants used by the sync code." (:require - [clojure.string :as str] [malli.util :as mut] [metabase.lib.schema.common :as lib.schema.common] [metabase.util.malli.registry :as mr] @@ -114,13 +113,7 @@ ;; out from the ns declaration when running `cljr-clean-ns`. Plus as a bonus in the future we could add additional ;; validations to these, e.g. requiring that a Field have a base_type -(mr/def ::no-kebab-case-keys - [:fn - {:error/message "Map should not contain any kebab-case keys"} - (fn [m] - (every? (fn [k] - (not (str/includes? k "-"))) - (keys m)))]) +(mr/def ::no-kebab-case-keys (ms/MapWithNoKebabKeys)) (mr/def ::DatabaseInstance [:and diff --git a/src/metabase/util/malli/registry.cljc b/src/metabase/util/malli/registry.cljc index fb593605f2e732aafd08788fb9104ca79b42138a..b97c940230e0a6b4be626255101e6f81567af37d 100644 --- a/src/metabase/util/malli/registry.cljc +++ b/src/metabase/util/malli/registry.cljc @@ -17,7 +17,7 @@ You generally shouldn't use this outside of this namespace unless you have a really good reason to do so! Make sure you used namespaced keys if you are using it elsewhere." [k schema value-thunk] - (or (get-in @cache [k schema]) + (or (get (get @cache k) schema) ; get-in is terribly inefficient (let [v (value-thunk)] (swap! cache assoc-in [k schema] v) v))) diff --git a/src/metabase/util/malli/schema.clj b/src/metabase/util/malli/schema.clj index 6a20ab3a1af2b9a507d8c713014d061bb9c75311..cdd8f29103f9d05720bc6df4c10ed076ecb67bd4 100644 --- a/src/metabase/util/malli/schema.clj +++ b/src/metabase/util/malli/schema.clj @@ -5,6 +5,7 @@ " (:require [cheshire.core :as json] + [clojure.string :as str] [malli.core :as mc] [metabase.legacy-mbql.normalize :as mbql.normalize] [metabase.legacy-mbql.schema :as mbql.s] @@ -386,3 +387,20 @@ "Helper for creating a schema that coerces single-value to a vector. Useful for coercing query parameters." [schema] [:vector {:decode/string (fn [x] (cond (vector? x) x x [x]))} schema]) + +(defn MapWithNoKebabKeys + "Helper for creating a schema to check if a map doesn't contain kebab case keys." + [] + [:fn + {:error/message "Map should not contain any kebab-case keys"} + (fn [m] + ;; reduce-kv is more efficient that iterating over (keys m). But we have to extract the underlying map from + ;; Toucan2 Instance because it doesn't implement IKVReduce (yet). + (let [m (if (instance? toucan2.instance.Instance m) + (.m ^toucan2.instance.Instance m) + m)] + (reduce-kv (fn [_ k _] + (if (str/includes? k "-") + (reduced false) + true)) + true m)))]) diff --git a/test/metabase/api/alert_test.clj b/test/metabase/api/alert_test.clj index 61dd8ea95dcc115aeaeb4ef395981e6ceb8a99ba..f176c48d13e183e92fd121c6fc2d96e6e9d20e3e 100644 --- a/test/metabase/api/alert_test.clj +++ b/test/metabase/api/alert_test.clj @@ -210,7 +210,7 @@ (deftest post-alert-test (is (= {:errors {:alert_condition "enum of rows, goal"} - :specific-errors {:alert_condition ["should be either rows or goal, received: \"not rows\""]}} + :specific-errors {:alert_condition ["should be either \"rows\" or \"goal\", received: \"not rows\""]}} (mt/user-http-request :rasta :post 400 "alert" {:alert_condition "not rows" :card "foobar"}))) @@ -419,7 +419,7 @@ (deftest put-alert-test-2 (is (= {:errors {:alert_condition "nullable enum of rows, goal"}, - :specific-errors {:alert_condition ["should be either rows or goal, received: \"not rows\""]}} + :specific-errors {:alert_condition ["should be either \"rows\" or \"goal\", received: \"not rows\""]}} (mt/user-http-request :rasta :put 400 "alert/1" {:alert_condition "not rows"}))) diff --git a/test/metabase/api/api_key_test.clj b/test/metabase/api/api_key_test.clj index 2a00748c44323bf4140235fdd7ecb6347fbb6935..d7995e0d7fd370953775134e5148fc4be1cd0052 100644 --- a/test/metabase/api/api_key_test.clj +++ b/test/metabase/api/api_key_test.clj @@ -85,7 +85,7 @@ [:group :name])))) (testing "A non-empty name is required" (is (= {:errors {:name "value must be a non-blank string."} - :specific-errors {:name ["should be at least 1 characters, received: \"\"" "non-blank string, received: \"\""]}} + :specific-errors {:name ["should be at least 1 character, received: \"\"" "non-blank string, received: \"\""]}} (mt/user-http-request :crowberto :post 400 "api-key" {:group_id group-id :name ""})))) diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj index a9b6b64aaf5674fae350a98f1867b8d507932a5f..d0ce40e3ce0eba02d2a710311f87da50787dce19 100644 --- a/test/metabase/api/dashboard_test.clj +++ b/test/metabase/api/dashboard_test.clj @@ -873,7 +873,7 @@ (t2/select-one-fn :width Dashboard :id (u/the-id dashboard))))) (testing "values that are not 'fixed' or 'full' error." - (is (= "should be either fixed or full, received: 1200" + (is (= "should be either \"fixed\" or \"full\", received: 1200" (-> (mt/user-http-request :rasta :put 400 (str "dashboard/" (u/the-id dashboard)) {:width 1200}) :specific-errors :dash-updates diff --git a/test/metabase/api/database_test.clj b/test/metabase/api/database_test.clj index c1d32db5cc9f56fc514e7934a7fb586c8a683fc8..f979ed428ace20dd31a7df88249b0ad70faa27f8 100644 --- a/test/metabase/api/database_test.clj +++ b/test/metabase/api/database_test.clj @@ -192,7 +192,7 @@ (testing "GET /api/database/:id" (testing "Invalid `?include` should return an error" (is (= {:errors {:include "nullable enum of tables, tables.fields"}, - :specific-errors {:include ["should be either tables or tables.fields, received: \"schemas\""]}} + :specific-errors {:include ["should be either \"tables\" or \"tables.fields\", received: \"schemas\""]}} (mt/user-http-request :lucky :get 400 (format "database/%d?include=schemas" (mt/id)))))))) (deftest get-database-legacy-no-self-service-test diff --git a/test/metabase/lib/schema/expression/temporal_test.cljc b/test/metabase/lib/schema/expression/temporal_test.cljc index e31603515f2cd6936bdf16a648b3c78f6ed34b17..94f4129429c4303947038975d6d9ca5b30196d67 100644 --- a/test/metabase/lib/schema/expression/temporal_test.cljc +++ b/test/metabase/lib/schema/expression/temporal_test.cljc @@ -75,7 +75,7 @@ (me/humanize (mc/explain ::temporal/timezone-id input))) "US/Pacific" nil "US/Specific" ["invalid timezone ID: \"US/Specific\"" "timezone offset string literal"] - "" ["should be at least 1 characters" "non-blank string" "invalid timezone ID: \"\"" "timezone offset string literal"] + "" ["should be at least 1 character" "non-blank string" "invalid timezone ID: \"\"" "timezone offset string literal"] " " ["non-blank string" "invalid timezone ID: \" \"" "timezone offset string literal"] nil ["should be a string" "non-blank string" "invalid timezone ID: nil" "timezone offset string literal"]))