Don't delete "off" persisted_info records (#39272)

* Don't delete "off" persisted_info records

If they are off, they are "eligible" models for persistence that we do
not want to persist. And to remember that use choice to not persist them
we need to keep the record around. This was handled previously, but
#39219 was a bit too blunt. We
had a bunch of records in a "creating" state that we weren't removing,
so i deleted them regardless of state. But we want to make sure that the
"off" ones are not deleted.

On stats, a bunch of ones that were set to off are now slated to be
persisted after the prune job deleted the persisted_info records.

* unused local. whoops

enterprise/backend/test/metabase_enterprise/advanced_config/caching_test.clj

@@ -1,5 +1,6 @@
 (ns metabase-enterprise.advanced-config.caching-test
   (:require
+   [clojure.set :as set]
    [clojure.test :refer :all]
    [java-time.api :as t]
    [metabase.models :refer [Card Dashboard Database PersistedInfo TaskHistory]]
@@ -74,9 +75,9 @@
   (mt/with-model-cleanup [TaskHistory]
     (testing "with :cache-granular-controls enabled, don't refresh any tables in an 'off' or 'deletable' state"
       (mt/with-premium-features #{:cache-granular-controls}
-        (with-temp-persist-models [db creating]
+        (with-temp-persist-models [db creating poff pdeletable]
           (testing "Calls refresh on each persisted-info row"
-            (let [card-ids (atom #{})
+            (let [card-ids       (atom #{})
                   test-refresher (reify task.persist-refresh/Refresher
                                    (refresh! [_ _database _definition card]
                                      (swap! card-ids conj (:id card))
@@ -85,12 +86,25 @@
               (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher)
               (testing "Doesn't refresh models that have state='off' or 'deletable' if :cache-granular-controls feature flag is enabled"
                 (is (= #{(u/the-id creating)} @card-ids)))
-              (is (partial= {:task "persist-refresh"
+              (is (partial= {:task         "persist-refresh"
                              :task_details {:success 1 :error 0}}
                             (t2/select-one TaskHistory
                                            :db_id (u/the-id db)
                                            :task "persist-refresh"
-                                           {:order-by [[:id :desc]]}))))))))
+                                           {:order-by [[:id :desc]]})))))
+          (testing "Deletes backing tables of models that have state='off'"
+            (let [unpersisted-ids (atom #{})
+                  test-refresher  (reify task.persist-refresh/Refresher
+                                    (unpersist! [_ _database persisted-info]
+                                      (swap! unpersisted-ids conj (:id persisted-info))))
+                  deleted?        (fn [{id :id}]
+                                    (not (t2/exists? :model/PersistedInfo :id id)))]
+              (#'task.persist-refresh/prune-all-deletable! test-refresher)
+              (is (set/subset? (set [(:id pdeletable) (:id poff)])
+                               @unpersisted-ids))
+              (is (deleted? pdeletable))
+              (testing "But does not delete the persisted_info record for \"off\" models"
+                (is (not (deleted? poff)))))))))
     (testing "with :cache-granular-controls disabled, refresh tables in an 'off' state, but not 'deletable'"
         (mt/with-premium-features #{}
           (with-temp-persist-models [db creating off]

src/metabase/task/persist_refresh.clj

@@ -133,7 +133,8 @@
                                          (log/info (trs "Unpersisting model with card-id {0}" (:card_id persisted-info)))
                                          (try
                                            (unpersist! refresher database persisted-info)
-                                           (t2/delete! PersistedInfo :id (:id persisted-info))
+                                           (when-not (= "off" current-state)
+                                             (t2/delete! PersistedInfo :id (:id persisted-info)))
                                            (update stats :success inc)
                                            (catch Exception e
                                              (log/info e (trs "Error unpersisting model with card-id {0}" (:card_id persisted-info)))