Fix #39138 again (#40936)

* Fix #39138 again

Fixes #40934

Got unfixed somehow in #40578. seems like some file renames caught it off guard

But the long and short is that its possible for a persisted model to end
up in the state "refreshing" (possible an instance restart during
refreshing). The refreshing job doesn't look for these so they become
effectively invisible.

Since the job to refresh them will only run one at a time cluster wide,
any jobs that are in the "refreshing" state when the refresher begins to
refresh are necessarilly stuck (no one else could currently be
refreshing them). So we can just add them to the queue of models to
refresh.

```clojure
(jobs/defjob ^{org.quartz.DisallowConcurrentExecution true ;; <----
               :doc "Refresh persisted tables job"}
  PersistenceRefresh [job-context]
  (refresh-job-fn! job-context))
```

* Ensure ee/oss pathways are taken in tests

Annoying little footgun here. CI does not run with an ee token, so in
order to ensure that test pathway goes through ee version of a
defenterprise we _must_ use the `mt/with-premium-features
{:cache-granular-controls}`,  but we also want to ensure that it goes
through the oss version.

So two options, a `doseq` on both features (empty set and the feature
that triggers this). But want a test in the enterprise folder as well to
ensure.

The real trickiness comes from running tests at a repl and CLI. My REPL
always has an ee token in it. My command line always lacks that as
well. So want to be explicit about the token features in effect at test
time.

That's why I'm essentially duplicating tests (ee in ee folder, oss in
regular pathway)

enterprise/backend/src/metabase_enterprise/cache/config.clj

@@ -5,7 +5,8 @@
   "States of `persisted_info` records which can be refreshed."
   :feature :cache-granular-controls
   []
-  #{"creating" "persisted" "error"})
+  ;; meant to be the same as the oss version except that "off" is deleteable rather than refreshable
+  #{"refreshing" "creating" "persisted" "error"})
 
 (defenterprise prunable-states
   "States of `persisted_info` records which can be pruned."

enterprise/backend/test/metabase_enterprise/cache/config_test.clj

@@ -47,6 +47,36 @@
   `(do-with-persist-models
     (fn [{:keys [~@bindings]}] ~@body)))
 
+(deftest refreshing-models-are-refreshed
+  ;; models might get stuck in "refreshing" mode and need to be "kicked". This is actually the case in the OSS
+  ;; version, but when [[metabase-enterprise.cache.config/refreshable-states]] was extracted, "refreshing" was not
+  ;; carried over.
+  ;;
+  ;; This affects mostly models that were refreshing when an instance was restarted.  both OSS and enterprise should
+  ;; behave this way. Don't know how to exercise both in the same jvm, but will let CI sort it out.
+  (let [two-hours-ago (t/minus (t/local-date-time) (t/hours 2))]
+    (mt/with-temp
+      [Database db {:settings {:persist-models-enabled true}}
+       Card          refreshing  {:type :model, :database_id (u/the-id db)}
+       PersistedInfo prefreshing {:card_id         (u/the-id refreshing)
+                                  :database_id     (u/the-id db)
+                                  :state           "refreshing"
+                                  :state_change_at two-hours-ago}]
+      (let [card-ids       (atom #{})
+            test-refresher (reify task.persist-refresh/Refresher
+                             (refresh! [_ _database _definition card]
+                               (swap! card-ids conj (:id card))
+                               {:state :success})
+                             (unpersist! [_ _database _persisted-info]))
+            current-state! (fn [] (t2/select-one-fn :state :model/PersistedInfo (u/the-id prefreshing)))]
+        ;; ensure ee path is taken
+        (mt/with-premium-features #{:cache-granular-controls}
+          (is (= "refreshing" (current-state!)))
+          (#'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 refreshing)} @card-ids)))
+          (is (= "persisted" (current-state!))))))))
+
 (deftest model-caching-granular-controls-test
   (mt/with-model-cleanup [TaskHistory]
     (testing "with :cache-granular-controls enabled, don't refresh any tables in an 'off' or 'deletable' state"

src/metabase/models/persisted_info.clj

@@ -85,6 +85,7 @@
    refreshable with the feature flag disabled."
   metabase-enterprise.cache.config
   []
+  ;; meant to be the same as the enterprise version except that "off" is not honored and is refreshed
   #{"refreshing" "creating" "persisted" "error" "off"})
 
 (defenterprise prunable-states

test/metabase/task/persist_refresh_test.clj

@@ -124,18 +124,20 @@
                                {:state :success})
                              (unpersist! [_ _database _persisted-info]))
             original-update! t2/update!]
-        (testing "If saving the `persisted` (or `error`) state fails..."
-          (with-redefs [t2/update! (fn [model id update]
-                                     (when (= "persisted" (:state update))
-                                       (throw (ex-info "simulated error" {})))
-                                     (original-update! model id update))]
-            (is (thrown-with-msg? clojure.lang.ExceptionInfo #"simulated error"
-                                  (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher))))
-          (testing "the PersistedInfo is left in the `refreshing` state"
-            (is (= "refreshing" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info)))))
-          (testing "but a subsequent refresh run will refresh the table"
-            (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher)
-            (is (= "persisted" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info))))))))))
+        ;; ensure no EE features
+        (mt/with-premium-features #{}
+          (testing "If saving the `persisted` (or `error`) state fails..."
+            (with-redefs [t2/update! (fn [model id update]
+                                       (when (= "persisted" (:state update))
+                                         (throw (ex-info "simulated error" {})))
+                                       (original-update! model id update))]
+              (is (thrown-with-msg? clojure.lang.ExceptionInfo #"simulated error"
+                                    (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher))))
+            (testing "the PersistedInfo is left in the `refreshing` state"
+              (is (= "refreshing" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info)))))
+            (testing "but a subsequent refresh run will refresh the table"
+              (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher)
+              (is (= "persisted" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info)))))))))))
 
 (deftest refresh-tables!'-test
   (mt/with-model-cleanup [TaskHistory]