🍒 Manual backport of metabase#34996 (metabase#35429)

Add drop_entity_ids CLI command, to drop all entity_ids

This is useful for migrating from serdes v1 to v2.

Fixes metabase#34871.

enterprise/backend/src/metabase_enterprise/serialization/cmd.clj

@@ -3,10 +3,10 @@
   (:require
    [metabase-enterprise.serialization.dump :as dump]
    [metabase-enterprise.serialization.load :as load]
+   [metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids]
    [metabase-enterprise.serialization.v2.extract :as v2.extract]
    [metabase-enterprise.serialization.v2.ingest :as v2.ingest]
    [metabase-enterprise.serialization.v2.load :as v2.load]
-   [metabase-enterprise.serialization.v2.seed-entity-ids :as v2.seed-entity-ids]
    [metabase-enterprise.serialization.v2.storage :as v2.storage]
    [metabase.db :as mdb]
    [metabase.models.card :refer [Card]]
@@ -220,4 +220,16 @@
 
   Returns truthy if all entity IDs were added successfully, or falsey if any errors were encountered."
   []
-  (v2.seed-entity-ids/seed-entity-ids!))
+  (v2.entity-ids/seed-entity-ids!))
+
+(defn drop-entity-ids
+  "Drop entity IDs for all instances of serializable models.
+
+  This is needed for some cases of migrating from v1 to v2 serdes. v1 doesn't dump `entity_id`, so they may have been
+  randomly generated independently in both instances. Then when v2 serdes is used to export and import, the randomly
+  generated IDs don't match and the entities get duplicated. Dropping `entity_id` from both instances first will force
+  them to be regenerated based on the hashes, so they should match up if the receiving instance is a copy of the sender.
+
+  Returns truthy if all entity IDs have been dropped, or falsey if any errors were encountered."
+  []
+  (v2.entity-ids/drop-entity-ids!))

enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj → enterprise/backend/src/metabase_enterprise/serialization/v2/entity_ids.clj

@@ -1,4 +1,4 @@
-(ns metabase-enterprise.serialization.v2.seed-entity-ids
+(ns metabase-enterprise.serialization.v2.entity-ids
   (:require
    [clojure.string :as str]
    [metabase.db :as mdb]
@@ -116,3 +116,28 @@
                                {:update-count 0, :error-count 0}
                                (entity-id-models))]
     (zero? error-count)))
+
+(defn- drop-entity-ids-for-model! [model]
+  (log/infof "Dropping Entity IDs for model %s" (name model))
+  (try
+    (let [update-count (t2/update! model {:entity_id nil})]
+      (when (pos? update-count)
+        (log/infof "Updated %d %s instance(s) successfully." update-count (name model)))
+      {:update-count update-count})
+    (catch Throwable e
+      (log/errorf e "Error dropping entity ID: %s" (ex-message e))
+      {:error-count 1})))
+
+(defn drop-entity-ids!
+  "Delete entity IDs for any models that have them. See #34871.
+
+  Returns truthy if all entity IDs were removed successfully, and falsey if there were any errors."
+  []
+  (log/info "Dropping Entity IDs")
+  (mdb/setup-db!)
+  (let [{:keys [error-count]} (transduce
+                                (map drop-entity-ids-for-model!)
+                                (completing (partial merge-with +))
+                                {:update-count 0, :error-count 0}
+                                (entity-id-models))]
+    (zero? error-count)))

enterprise/backend/test/metabase_enterprise/models/entity_id_test.clj

@@ -7,8 +7,9 @@
   (:require
    [clojure.test :refer :all]
    [metabase-enterprise.serialization.v2.backfill-ids :as serdes.backfill]
-   [metabase-enterprise.serialization.v2.seed-entity-ids :as v2.seed-entity-ids]
+   [metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids]
    [metabase.db.data-migrations]
+   #_{:clj-kondo/ignore [:deprecated-namespace]}
    [metabase.models]
    [metabase.models.revision-test]
    [metabase.models.serialization :as serdes]))
@@ -74,7 +75,7 @@
     :model/ConnectionImpersonation})
 
 (deftest ^:parallel comprehensive-entity-id-test
-  (doseq [model (->> (v2.seed-entity-ids/toucan-models)
+  (doseq [model (->> (v2.entity-ids/toucan-models)
                      (remove (fn [model]
                                (not= (namespace model) "model")))
                      (remove entities-not-exported)
@@ -85,7 +86,7 @@
       (is (true? (serdes.backfill/has-entity-id? model))))))
 
 (deftest ^:parallel comprehensive-identity-hash-test
-  (doseq [model (->> (v2.seed-entity-ids/toucan-models)
+  (doseq [model (->> (v2.entity-ids/toucan-models)
                      (remove (fn [model]
                                (not= (namespace model) "model")))
                      (remove entities-not-exported))]

enterprise/backend/test/metabase_enterprise/serialization/v2/seed_entity_ids_test.clj → enterprise/backend/test/metabase_enterprise/serialization/v2/entity_ids_test.clj

@@ -1,9 +1,8 @@
-(ns metabase-enterprise.serialization.v2.seed-entity-ids-test
+(ns metabase-enterprise.serialization.v2.entity-ids-test
   (:require
    [clojure.string :as str]
    [clojure.test :refer :all]
-   [metabase-enterprise.serialization.v2.seed-entity-ids
-    :as v2.seed-entity-ids]
+   [metabase-enterprise.serialization.v2.entity-ids :as v2.entity-ids]
    [metabase.models :refer [Collection]]
    [toucan2.core :as t2]
    [toucan2.tools.with-temp :as t2.with-temp])
@@ -14,7 +13,7 @@
 
 (deftest seed-entity-ids-test
   (testing "Sanity check: should succeed before we go around testing specific situations"
-    (is (true? (v2.seed-entity-ids/seed-entity-ids!))))
+    (is (true? (v2.entity-ids/seed-entity-ids!))))
   (testing "With a temp Collection with no entity ID"
     (let [now (LocalDateTime/of 2022 9 1 12 34 56)]
       (t2.with-temp/with-temp [Collection c {:name       "No Entity ID Collection"
@@ -28,7 +27,7 @@
                  (entity-id)))
           (testing "Should return truthy on success"
             (is (= true
-                   (v2.seed-entity-ids/seed-entity-ids!))))
+                   (v2.entity-ids/seed-entity-ids!))))
           (is (= "998b109c"
                  (entity-id))))
         (testing "Error: duplicate entity IDs"
@@ -43,6 +42,20 @@
                      (entity-id)))
               (testing "Should return falsey on error"
                 (is (= false
-                       (v2.seed-entity-ids/seed-entity-ids!))))
+                       (v2.entity-ids/seed-entity-ids!))))
               (is (= nil
                      (entity-id))))))))))
+
+(deftest drop-entity-ids-test
+  (testing "With a temp Collection with an entity ID"
+    (let [now (LocalDateTime/of 2022 9 1 12 34 56)]
+      (t2.with-temp/with-temp [Collection c {:name       "No Entity ID Collection"
+                                             :slug       "no_entity_id_collection"
+                                             :created_at now}]
+        (letfn [(entity-id []
+                  (some-> (t2/select-one-fn :entity_id Collection :id (:id c)) str/trim))]
+          (is (some? (entity-id)))
+          (testing "Should return truthy on success"
+            (is (= true
+                   (v2.entity-ids/drop-entity-ids!))))
+          (is (nil? (entity-id))))))))

src/metabase/cmd.clj

@@ -222,6 +222,13 @@
   (when-not (call-enterprise 'metabase-enterprise.serialization.cmd/seed-entity-ids)
     (throw (Exception. "Error encountered while seeding entity IDs"))))
 
+(defn ^:command drop-entity-ids
+  "Drop entity IDs for instances of serializable models. Useful for migrating from v1 serialization (x.46 and earlier)
+  to v2 (x.47+)."
+  []
+  (when-not (call-enterprise 'metabase-enterprise.serialization.cmd/drop-entity-ids)
+    (throw (Exception. "Error encountered while dropping entity IDs"))))
+
 (defn ^:command rotate-encryption-key
   "Rotate the encryption key of a metabase database. The MB_ENCRYPTION_SECRET_KEY environment variable has to be set to
   the current key, and the parameter `new-key` has to be the new key. `new-key` has to be at least 16 chars."

test/metabase/cmd_test.clj

@@ -10,7 +10,7 @@
 
 (deftest ^:parallel error-message-test
   (is (= ["Unrecognized command: 'a-command-that-does-not-exist'"
-          "Valid commands: version, help, import, dump, profile, api-documentation, load, seed-entity-ids, dump-to-h2, environment-variables-documentation, migrate, driver-methods, load-from-h2, export, rotate-encryption-key, reset-password"]
+          "Valid commands: version, help, drop-entity-ids, import, dump, profile, api-documentation, load, seed-entity-ids, dump-to-h2, environment-variables-documentation, migrate, driver-methods, load-from-h2, export, rotate-encryption-key, reset-password"]
          (#'cmd/validate "a-command-that-does-not-exist" [])))
   (is (= ["The 'rotate-encryption-key' command requires the following arguments: [new-key], but received: []."]
          (#'cmd/validate "rotate-encryption-key" [])))