Implement equality for MLv2 MetadataProviders (#38413)

* MLv2 MetadataProviders should know if they are equal

* PR feedback

src/metabase/lib/metadata/cached_provider.cljc

@@ -9,6 +9,8 @@
    [metabase.util.malli :as mu]
    #?@(:clj ([pretty.core :as pretty]))))
 
+#?(:clj (set! *warn-on-reflection* true))
+
 (defn- get-in-cache [cache ks]
   (when-some [cached-value (get-in @cache ks)]
     (when-not (= cached-value ::nil)
@@ -109,6 +111,12 @@
   (bulk-metadata [_this metadata-type ids]
     (bulk-metadata cache metadata-provider metadata-type ids))
 
+  #?(:clj Object :cljs IEquiv)
+  (#?(:clj equals :cljs -equiv) [_this another]
+    (and (instance? CachedProxyMetadataProvider another)
+         (= metadata-provider
+            (.-metadata-provider ^CachedProxyMetadataProvider another))))
+
   #?@(:clj
       [pretty/PrettyPrintable
        (pretty [_this]

src/metabase/lib/metadata/composed_provider.cljc

@@ -22,38 +22,46 @@
          (m/distinct-by :id))
         metadata-providers))
 
+(deftype ComposedMetadataProvider [metadata-providers]
+  metadata.protocols/MetadataProvider
+  (database [_this]              (some metadata.protocols/database metadata-providers))
+  (table    [_this table-id]     (object-for-id metadata.protocols/table   table-id     metadata-providers))
+  (field    [_this field-id]     (object-for-id metadata.protocols/field   field-id     metadata-providers))
+  (card     [_this card-id]      (object-for-id metadata.protocols/card    card-id      metadata-providers))
+  (metric   [_this metric-id]    (object-for-id metadata.protocols/metric  metric-id    metadata-providers))
+  (segment  [_this segment-id]   (object-for-id metadata.protocols/segment segment-id   metadata-providers))
+  (setting  [_this setting-name] (object-for-id metadata.protocols/setting setting-name metadata-providers))
+  (tables   [_this]              (m/distinct-by :id (mapcat metadata.protocols/tables metadata-providers)))
+  (fields   [_this table-id]     (objects-for-table-id metadata.protocols/fields   table-id metadata-providers))
+  (metrics  [_this table-id]     (objects-for-table-id metadata.protocols/metrics  table-id metadata-providers))
+  (segments [_this table-id]     (objects-for-table-id metadata.protocols/segments table-id metadata-providers))
+
+  metadata.protocols/CachedMetadataProvider
+  (cached-database [_this]
+    (some metadata.protocols/cached-database
+          (cached-providers metadata-providers)))
+  (cached-metadata [_this metadata-type id]
+    (some #(metadata.protocols/cached-metadata % metadata-type id)
+          (cached-providers metadata-providers)))
+  (store-database! [_this database-metadata]
+    (when-first [provider (cached-providers metadata-providers)]
+      (metadata.protocols/store-database! provider database-metadata)))
+  (store-metadata! [_this metadata-type id metadata]
+    (when-first [provider (cached-providers metadata-providers)]
+      (metadata.protocols/store-metadata! provider metadata-type id metadata)))
+
+  #?(:clj Object :cljs IEquiv)
+  (#?(:clj equals :cljs -equiv) [_this another]
+    (and (instance? ComposedMetadataProvider another)
+         (= metadata-providers
+            (.-metadata-providers ^ComposedMetadataProvider another))))
+
+  clojure.core.protocols/Datafiable
+  (datafy [_this]
+    (cons `composed-metadata-provider (map datafy/datafy metadata-providers))))
+
 (defn composed-metadata-provider
   "A metadata provider composed of several different `metadata-providers`. Methods try each constituent provider in
   turn from left to right until one returns a truthy result."
   [& metadata-providers]
-  (reify
-    metadata.protocols/MetadataProvider
-    (database [_this]              (some metadata.protocols/database metadata-providers))
-    (table    [_this table-id]     (object-for-id metadata.protocols/table   table-id     metadata-providers))
-    (field    [_this field-id]     (object-for-id metadata.protocols/field   field-id     metadata-providers))
-    (card     [_this card-id]      (object-for-id metadata.protocols/card    card-id      metadata-providers))
-    (metric   [_this metric-id]    (object-for-id metadata.protocols/metric  metric-id    metadata-providers))
-    (segment  [_this segment-id]   (object-for-id metadata.protocols/segment segment-id   metadata-providers))
-    (setting  [_this setting-name] (object-for-id metadata.protocols/setting setting-name metadata-providers))
-    (tables   [_this]              (m/distinct-by :id (mapcat metadata.protocols/tables metadata-providers)))
-    (fields   [_this table-id]     (objects-for-table-id metadata.protocols/fields   table-id metadata-providers))
-    (metrics  [_this table-id]     (objects-for-table-id metadata.protocols/metrics  table-id metadata-providers))
-    (segments [_this table-id]     (objects-for-table-id metadata.protocols/segments table-id metadata-providers))
-
-    metadata.protocols/CachedMetadataProvider
-    (cached-database [_this]
-      (some metadata.protocols/cached-database
-            (cached-providers metadata-providers)))
-    (cached-metadata [_this metadata-type id]
-      (some #(metadata.protocols/cached-metadata % metadata-type id)
-            (cached-providers metadata-providers)))
-    (store-database! [_this database-metadata]
-      (when-first [provider (cached-providers metadata-providers)]
-        (metadata.protocols/store-database! provider database-metadata)))
-    (store-metadata! [_this metadata-type id metadata]
-      (when-first [provider (cached-providers metadata-providers)]
-        (metadata.protocols/store-metadata! provider metadata-type id metadata)))
-
-    clojure.core.protocols/Datafiable
-    (datafy [_this]
-      (cons `composed-metadata-provider (map datafy/datafy metadata-providers)))))
+  (->ComposedMetadataProvider metadata-providers))

src/metabase/lib/metadata/jvm.clj

@@ -394,7 +394,12 @@
 
   pretty/PrettyPrintable
   (pretty [_this]
-    (list `->UncachedApplicationDatabaseMetadataProvider database-id)))
+    (list `->UncachedApplicationDatabaseMetadataProvider database-id))
+
+  Object
+  (equals [_this another]
+    (and (instance? UncachedApplicationDatabaseMetadataProvider another)
+         (= database-id (.database-id ^UncachedApplicationDatabaseMetadataProvider another)))))
 
 (mu/defn application-database-metadata-provider :- lib.metadata/MetadataProvider
   "An implementation of [[metabase.lib.metadata.protocols/MetadataProvider]] for the application database.

test/metabase/lib/metadata/cached_provider_test.cljc

@@ -52,3 +52,7 @@
                (lib.metadata.protocols/bulk-metadata provider :metadata/table #{1 2})))
         (is (= 2
                @fetch-count))))))
+
+(deftest ^:parallel equality-test
+  (is (= (lib.metadata.cached-provider/cached-metadata-provider nil)
+         (lib.metadata.cached-provider/cached-metadata-provider nil))))

test/metabase/lib/metadata/composed_provider_test.cljc

@@ -25,3 +25,7 @@
               (lib.metadata/field
                metadata-provider
                (meta/id :people :birth-date)))))))
+
+(deftest ^:parallel equality-test
+  (is (= (lib/composed-metadata-provider meta/metadata-provider)
+         (lib/composed-metadata-provider meta/metadata-provider))))

test/metabase/lib/metadata/jvm_test.clj

@@ -195,3 +195,7 @@
             (lib.metadata/card
              (lib.metadata.jvm/application-database-metadata-provider (mt/id))
              card-id)))))
+
+(deftest ^:parallel equality-test
+  (is (= (lib.metadata.jvm/application-database-metadata-provider (mt/id))
+         (lib.metadata.jvm/application-database-metadata-provider (mt/id)))))

test/metabase/lib/test_metadata/graph_provider.cljc

 (ns metabase.lib.test-metadata.graph-provider
   (:require
    [clojure.core.protocols]
+   [clojure.test :refer [deftest is]]
    [medley.core :as m]
    [metabase.lib.metadata.protocols :as lib.metadata.protocols]
    #?@(:clj
@@ -73,9 +74,19 @@
   (datafy [_this]
     (list `->SimpleGraphMetadataProvider metadata-graph))
 
+  #?(:clj Object :cljs IEquiv)
+  (#?(:clj equals :cljs -equiv) [_this another]
+    (and (instance? SimpleGraphMetadataProvider another)
+         (= metadata-graph
+            (#?(:clj .metadata-graph :cljs .-metadata-graph) ^SimpleGraphMetadataProvider another))))
+
   #?@(:clj
       [pretty/PrettyPrintable
        (pretty [_this]
-         (if (identical? metadata-graph @(requiring-resolve 'metabase.lib.test-metadata/metadata))
-           'metabase.lib.test-metadata/metadata-provider
-           (list `->SimpleGraphMetadataProvider metadata-graph)))]))
+               (if (identical? metadata-graph @(requiring-resolve 'metabase.lib.test-metadata/metadata))
+                 'metabase.lib.test-metadata/metadata-provider
+                 (list `->SimpleGraphMetadataProvider metadata-graph)))]))
+
+(deftest ^:parallel equality-test
+  (is (= (->SimpleGraphMetadataProvider {})
+         (->SimpleGraphMetadataProvider {}))))

test/metabase/lib/test_util/metadata_providers/mock.cljc

 (ns metabase.lib.test-util.metadata-providers.mock
   (:require
    [clojure.core.protocols]
+   [clojure.test :refer [deftest is]]
    [medley.core :as m]
    [metabase.lib.core :as lib]
    [metabase.lib.metadata :as lib.metadata]
    [metabase.lib.metadata.protocols :as metadata.protocols]
+   [metabase.lib.test-metadata :as meta]
    [metabase.util.malli :as mu]))
 
 (defn- with-optional-lib-type
@@ -27,6 +29,47 @@
    [:segments {:optional true} [:maybe [:sequential (with-optional-lib-type lib.metadata/SegmentMetadata :metadata/segment)]]]
    [:settings {:optional true} [:maybe [:map-of :keyword any?]]]])
 
+(deftype MockMetadataProvider [metadata]
+  metadata.protocols/MetadataProvider
+  (database [_this]            (some-> (:database metadata)
+                                       (assoc :lib/type :metadata/database)
+                                       (dissoc :tables)))
+  (table    [_this table-id]   (some-> (m/find-first #(= (:id %) table-id) (:tables metadata))
+                                       (assoc :lib/type :metadata/table)
+                                       (dissoc :fields)))
+  (field    [_this field-id]   (some-> (m/find-first #(= (:id %) field-id) (:fields metadata))
+                                       (assoc :lib/type :metadata/column)))
+  (card     [_this card-id]    (some-> (m/find-first #(= (:id %) card-id) (:cards metadata))
+                                       (assoc :lib/type :metadata/card)))
+  (metric   [_this metric-id]  (some-> (m/find-first #(= (:id %) metric-id) (:metrics metadata))
+                                       (assoc :lib/type :metadata/metric)))
+  (segment  [_this segment-id] (some-> (m/find-first #(= (:id %) segment-id) (:segments metadata))
+                                       (assoc :lib/type :metadata/segment)))
+  (tables   [_this]            (for [table (:tables metadata)]
+                                 (-> (assoc table :lib/type :metadata/table)
+                                     (dissoc :fields))))
+  (fields   [_this table-id]   (for [field (:fields metadata)
+                                     :when (= (:table-id field) table-id)]
+                                 (assoc field :lib/type :metadata/column)))
+  (metrics  [_this table-id]   (for [metric (:metrics metadata)
+                                     :when  (= (:table-id metric) table-id)]
+                                 (assoc metric :lib/type :metadata/metric)))
+  (segments [_this table-id]   (for [segment (:segments metadata)
+                                     :when   (= (:table-id segment) table-id)]
+                                 (assoc segment :lib/type :metadata/segment)))
+
+  (setting [_this setting]     (get-in metadata [:settings (keyword setting)]))
+
+  #?(:clj Object :cljs IEquiv)
+  (#?(:clj equals :cljs -equiv) [_this another]
+    (and (instance? MockMetadataProvider another)
+         (= metadata
+            (#?(:clj .metadata :cljs .-metadata) ^MockMetadataProvider another))))
+
+  clojure.core.protocols/Datafiable
+  (datafy [_this]
+    (list `mock-metadata-provider metadata)))
+
 (mu/defn mock-metadata-provider :- lib.metadata/MetadataProvider
   "Create a mock metadata provider to facilitate writing tests. All keys except `:database` should be a sequence of maps
   e.g.
@@ -42,43 +85,19 @@
     (lib.tu/mock-metadata-provider parent-metadata-provider {...})
     =>
     (lib/composed-metadata-provider (lib.tu/mock-metadata-provider {...}) parent-metadata-provider)"
-  ([{:keys [database tables fields cards metrics segments settings] :as m} :- MockMetadata]
-   (reify
-     metadata.protocols/MetadataProvider
-     (database [_this]            (some-> database
-                                          (assoc :lib/type :metadata/database)
-                                          (dissoc :tables)))
-     (table    [_this table-id]   (some-> (m/find-first #(= (:id %) table-id) tables)
-                                          (assoc :lib/type :metadata/table)
-                                          (dissoc :fields)))
-     (field    [_this field-id]   (some-> (m/find-first #(= (:id %) field-id) fields)
-                                          (assoc :lib/type :metadata/column)))
-     (card     [_this card-id]    (some-> (m/find-first #(= (:id %) card-id) cards)
-                                          (assoc :lib/type :metadata/card)))
-     (metric   [_this metric-id]  (some-> (m/find-first #(= (:id %) metric-id) metrics)
-                                          (assoc :lib/type :metadata/metric)))
-     (segment  [_this segment-id] (some-> (m/find-first #(= (:id %) segment-id) segments)
-                                          (assoc :lib/type :metadata/segment)))
-     (tables   [_this]            (for [table tables]
-                                    (-> (assoc table :lib/type :metadata/table)
-                                        (dissoc :fields))))
-     (fields   [_this table-id]   (for [field fields
-                                        :when (= (:table-id field) table-id)]
-                                    (assoc field :lib/type :metadata/column)))
-     (metrics  [_this table-id]   (for [metric metrics
-                                        :when  (= (:table-id metric) table-id)]
-                                    (assoc metric :lib/type :metadata/metric)))
-     (segments [_this table-id]   (for [segment segments
-                                        :when   (= (:table-id segment) table-id)]
-                                    (assoc segment :lib/type :metadata/segment)))
-
-     (setting [_this setting]     (get settings (keyword setting)))
-
-     clojure.core.protocols/Datafiable
-     (datafy [_this]
-       (list `mock-metadata-provider m))))
+  ([m :- MockMetadata]
+   (->MockMetadataProvider m))
 
   ([parent-metadata-provider mock-metadata]
    (lib/composed-metadata-provider
     (mock-metadata-provider mock-metadata)
     parent-metadata-provider)))
+
+(deftest ^:parallel equality-test
+  (let [time-field (assoc (meta/field-metadata :people :birth-date)
+                          :base-type      :type/Time
+                          :effective-type :type/Time)]
+    (is (= (mock-metadata-provider
+            {:fields [time-field]})
+           (mock-metadata-provider
+            {:fields [time-field]})))))