perf: Optimize validation

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

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

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))]

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)))))))

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

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)))

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)))])