diff --git a/src/metabase/task.clj b/src/metabase/task.clj index 839fbe7b72d85df38e30392a30114b8dfc9a38f9..fb67762cb3ba1dd4655078844a8c93da04a49948 100644 --- a/src/metabase/task.clj +++ b/src/metabase/task.clj @@ -22,7 +22,7 @@ [metabase.util.malli :as mu] [metabase.util.malli.schema :as ms]) (:import - (org.quartz CronTrigger JobDetail JobKey Scheduler Trigger TriggerKey))) + (org.quartz CronTrigger JobDetail JobKey JobPersistenceException Scheduler Trigger TriggerKey))) (set! *warn-on-reflection* true) @@ -141,6 +141,18 @@ (when (= (mdb/db-type) :postgres) (System/setProperty "org.quartz.jobStore.driverDelegateClass" "org.quartz.impl.jdbcjobstore.PostgreSQLDelegate"))) +(defn- delete-jobs-with-no-class! + "Delete any jobs that have been scheduled but whose class is no longer available." + [] + (when-let [scheduler (scheduler)] + (doseq [job-key (.getJobKeys scheduler nil)] + (try + (qs/get-job scheduler job-key) + (catch JobPersistenceException e + (when (instance? ClassNotFoundException (.getCause e)) + (log/infof "Deleting job %s due to class not found" (.getName ^JobKey job-key)) + (qs/delete-job scheduler job-key))))))) + (defn- init-scheduler! "Initialize our Quartzite scheduler which allows jobs to be submitted and triggers to scheduled. Puts scheduler in standby mode. Call [[start-scheduler!]] to begin running scheduled tasks." @@ -153,6 +165,7 @@ (find-and-load-task-namespaces!) (qs/standby new-scheduler) (log/info "Task scheduler initialized into standby mode.") + (delete-jobs-with-no-class!) (init-tasks!))))) ;;; this is a function mostly to facilitate testing. diff --git a/test/metabase/task_test.clj b/test/metabase/task_test.clj index b528af326fa49887ee51e56561a486d8de8b306a..76fbea1734de298438e862552739d7e29850a26c 100644 --- a/test/metabase/task_test.clj +++ b/test/metabase/task_test.clj @@ -5,11 +5,14 @@ [clojurewerkz.quartzite.schedule.cron :as cron] [clojurewerkz.quartzite.scheduler :as qs] [clojurewerkz.quartzite.triggers :as triggers] + [metabase.db.connection :as mdb.connection] [metabase.task :as task] [metabase.test :as mt] [metabase.test.fixtures :as fixtures] [metabase.test.util :as tu] - [metabase.util.malli.schema :as ms]) + [metabase.util :as u] + [metabase.util.malli.schema :as ms] + [toucan2.core :as t2]) (:import (org.quartz CronTrigger JobDetail))) @@ -121,3 +124,34 @@ (mt/with-temp-env-var-value! ["MB_DISABLE_SCHEDULER" "FALSE"] (task/start-scheduler!) (is (qs/started? (#'task/scheduler)))))))) + +(defn- capitalize-if-mysql [s] + (cond-> (name s) + (= :mysql (mdb.connection/db-type)) + u/upper-case-en + true keyword)) + +(deftest start-scheduler-will-cleanup-jobs-without-class-test + ;; we can't use the temp scheduler in this test because the temp scheduler use an in-memory jobstore + ;; and we need update the job class in the database to trigger the cleanup + (let [scheduler-initialized? (some? (#'task/scheduler))] + (try + (when-not scheduler-initialized? + (task/start-scheduler!)) + (task/schedule-task! (job) (trigger-1)) + (testing "make sure the job is in the database before we start the scheduler" + (is (t2/exists? (capitalize-if-mysql :qrtz_job_details) (capitalize-if-mysql :job_name) "metabase.task-test.job"))) + + ;; update the job class to a non-existent class + (t2/update! (capitalize-if-mysql :qrtz_job_details) (capitalize-if-mysql :job_name) "metabase.task-test.job" + {(capitalize-if-mysql :job_class_name) "NOT_A_REAL_CLASS"}) + ;; stop the scheduler then restart so [[task/delete-jobs-with-no-class!]] is triggered + (task/stop-scheduler!) + (task/start-scheduler!) + (testing "the job should be removed from the database when the scheduler starts" + (is (not (t2/exists? (capitalize-if-mysql :qrtz_job_details) (capitalize-if-mysql :job_name) "metabase.task-test.job")))) + (finally + ;; restore the state of scheduler before we start the test + (if scheduler-initialized? + (task/start-scheduler!) + (task/stop-scheduler!))))))