New Quartz ClassLoadHelper & ConnectionProvider record type impls

project.clj

@@ -149,15 +149,13 @@
     [[lein-environ "1.1.0"]]                                          ; easy access to environment variables
 
     :env      {:mb-run-mode "dev"}
-    :jvm-opts ["-Dlogfile.path=target/log"]
-    ;; Log appender class needs to be compiled for log4j to use it. Same with the Quartz class load helper
-    :aot      [metabase.task.DynamicClassLoadHelper]}
+    :jvm-opts ["-Dlogfile.path=target/log"]}
 
    :ci
    {:jvm-opts ["-Xmx2500m"]}
 
    :install
-   {:aot [metabase.task.DynamicClassLoadHelper]}
+   {}
 
    :install-for-building-drivers
    {:auto-clean true

resources/quartz.properties

@@ -5,7 +5,6 @@ org.quartz.threadPool.threadCount = 10
 
 # Don't phone home
 org.quartz.scheduler.skipUpdateCheck: true
-org.quartz.scheduler.classLoadHelper.class=metabase.task.DynamicClassLoadHelper
 
 # Use the JDBC backend so we can cluster when running multiple instances!
 # See http://www.quartz-scheduler.org/documentation/quartz-2.x/configuration/ConfigJDBCJobStoreClustering
@@ -18,8 +17,6 @@ org.quartz.jobStore.dataSource=db
 
 org.quartz.jobStore.isClustered = true
 
-org.quartz.dataSource.db.validationQuery=SELECT 1
-
 # By default, Quartz will fire triggers up to a minute late without considering them to be misfired; if it cannot fire
 # anything within that period for one reason or another (such as all threads in the thread pool being tied up), the
 # trigger is considered misfired. Threshold is in milliseconds.

src/metabase/task.clj

@@ -7,14 +7,17 @@
   function which accepts zero arguments. This function is dynamically resolved and called exactly once when the
   application goes through normal startup procedures. Inside this function you can do any work needed and add your
   task to the scheduler as usual via `schedule-task!`."
-  (:require [clojure.string :as str]
+  (:require [clojure.java.jdbc :as jdbc]
+            [clojure.string :as str]
             [clojure.tools.logging :as log]
             [clojurewerkz.quartzite.scheduler :as qs]
             [metabase
              [db :as mdb]
              [util :as u]]
+            [metabase.plugins.classloader :as classloader]
             [metabase.util.i18n :refer [trs]]
-            [schema.core :as s])
+            [schema.core :as s]
+            [toucan.db :as db])
   (:import [org.quartz JobDetail JobKey Scheduler Trigger TriggerKey]))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -24,22 +27,6 @@
 (defonce ^:private quartz-scheduler
   (atom nil))
 
-;; whenever the value of `quartz-scheduler` changes:
-;;
-;; 1.  shut down the old scheduler, if there was one
-;; 2.  start the new scheduler, if there is one
-(add-watch
- quartz-scheduler
- ::quartz-scheduler-watcher
- (fn [_ _ old-scheduler new-scheduler]
-   (when-not (identical? old-scheduler new-scheduler)
-     (when old-scheduler
-       (log/debug (trs "Stopping Quartz Scheduler {0}" old-scheduler))
-       (qs/shutdown old-scheduler))
-     (when new-scheduler
-       (log/debug (trs "Starting Quartz Scheduler {0}" new-scheduler))
-       (qs/start new-scheduler)))))
-
 (defn- scheduler
   "Fetch the instance of our Quartz scheduler. Call this function rather than dereffing the atom directly because there
   are a few places (e.g., in tests) where we swap the instance out."
@@ -88,6 +75,70 @@
         (log/error e (trs "Error initializing task {0}" k))))))
 
 
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; |                                      Quartz Scheduler Connection Provider                                      |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+;; Custom `ConnectionProvider` implementation that uses our application DB connection pool to provide connections.
+
+(defrecord ^:private ConnectionProvider []
+  org.quartz.utils.ConnectionProvider
+  (getConnection [_]
+    ;; get a connection from our application DB connection pool. Quartz will close it (i.e., return it to the pool)
+    ;; when it's done
+    (jdbc/get-connection (db/connection)))
+  (shutdown [_]))
+
+(when-not *compile-files*
+  (System/setProperty "org.quartz.dataSource.db.connectionProvider.class" (.getName ConnectionProvider)))
+
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; |                                       Quartz Scheduler Class Load Helper                                       |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+;; Custom `ClassLoadHelper` implementation that makes sure to require the namespaces that tasks live in (to make sure
+;; record types are loaded) and that uses our canonical ClassLoader.
+
+(defn- task-class-name->namespace-str
+  "Determine the namespace we need to load for one of our tasks.
+
+    (task-class-name->namespace-str \"metabase.task.upgrade_checks.CheckForNewVersions\")
+    ;; -> \"metabase.task.upgrade-checks\""
+  [class-name]
+  (-> class-name
+      (str/replace \_ \-)
+      (str/replace #"\.\w+$" "")))
+
+(defn- require-task-namespace
+  "Since Metabase tasks are defined in Clojure-land we need to make sure we `require` the namespaces where they are
+  defined before we try to load the task classes."
+  [class-name]
+  ;; call `the-classloader` to force side-effects of making it the current thread context classloader
+  (classloader/the-classloader)
+  ;; only try to `require` metabase.task classes; don't do this for other stuff that gets shuffled thru here like
+  ;; Quartz classes
+  (when (str/starts-with? class-name "metabase.task.")
+    (require (symbol (task-class-name->namespace-str class-name)))))
+
+(defn- load-class ^Class [^String class-name]
+  (require-task-namespace class-name)
+  (.loadClass (classloader/the-classloader) class-name))
+
+(defrecord ^:private ClassLoadHelper []
+  org.quartz.spi.ClassLoadHelper
+  (initialize [_])
+  (getClassLoader [_]
+    (classloader/the-classloader))
+  (loadClass [_ class-name]
+    (load-class class-name))
+  (loadClass [_ class-name _]
+    (load-class class-name)))
+
+(when-not *compile-files*
+  (System/setProperty "org.quartz.scheduler.classLoadHelper.class" (.getName ClassLoadHelper)))
+
+
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                          STARTING/STOPPING SCHEDULER                                           |
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -97,38 +148,25 @@
   connection properties ahead of time, we'll need to set these at runtime rather than Setting them in the
   `quartz.properties` file.)"
   []
-  (let [{:keys [classname user password subname subprotocol type]} (mdb/jdbc-details)]
-    ;; If we're using a Postgres application DB the driverDelegateClass has to be the Postgres-specific one rather
-    ;; than the Standard JDBC one we define in `quartz.properties`
-    (when (= type :postgres)
-      (System/setProperty "org.quartz.jobStore.driverDelegateClass" "org.quartz.impl.jdbcjobstore.PostgreSQLDelegate"))
-    ;; set other properties like URL, user, and password so Quartz knows how to connect
-    (doseq [[k, ^String v] {:driver   classname
-                            :URL      (format "jdbc:%s:%s" subprotocol subname)
-                            :user     user
-                            :password password}]
-      (when v
-        (System/setProperty (str "org.quartz.dataSource.db." (name k)) v)))))
-
-(def ^:private start-scheduler-lock (Object.))
+  (when (= (mdb/db-type) :postgres)
+    (System/setProperty "org.quartz.jobStore.driverDelegateClass" "org.quartz.impl.jdbcjobstore.PostgreSQLDelegate")))
 
 (defn start-scheduler!
   "Start our Quartzite scheduler which allows jobs to be submitted and triggers to begin executing."
   []
   (when-not @quartz-scheduler
-    (locking start-scheduler-lock
-      (when-not @quartz-scheduler
-        (set-jdbc-backend-properties!)
-        ;; keep a reference to our scheduler
-        (reset! quartz-scheduler (qs/initialize))
-        ;; look for job/trigger definitions
+    (set-jdbc-backend-properties!)
+    (let [new-scheduler (qs/initialize)]
+      (when (compare-and-set! quartz-scheduler nil new-scheduler)
+        (qs/start new-scheduler)
         (find-and-load-tasks!)))))
 
 (defn stop-scheduler!
   "Stop our Quartzite scheduler and shutdown any running executions."
   []
-  ;; setting `quartz-scheduler` to nil will cause it to shut down via the watcher on it
-  (reset! quartz-scheduler nil))
+  (let [[old-scheduler] (reset-vals! quartz-scheduler nil)]
+    (when old-scheduler
+      (qs/shutdown old-scheduler))))
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+

src/metabase/task/DynamicClassLoadHelper.clj

-(ns metabase.task.DynamicClassLoadHelper
-  "This is needed to get the JDBC backend for Quartz working, or something like that. See
-  http://clojurequartz.info/articles/durable_quartz_stores.html for details."
-  (:gen-class
-   :extends clojure.lang.DynamicClassLoader
-   :exposes-methods {loadClass superLoadClass}
-   :implements [org.quartz.spi.ClassLoadHelper])
-  (:require [clojure.string :as str]))
-
-;; docstrings are copies of the ones for the corresponding methods of the ClassLoadHelper interface
-
-(defn -initialize
-  "void initialize()
-
-  Called to give the ClassLoadHelper a chance to initialize itself, including the opportunity to \"steal\" the class
-  loader off of the calling thread, which is the thread that is initializing Quartz."
-  [_])
-
-(defn- task-class-name->namespace-str
-  "Determine the namespace we need to load for one of our tasks.
-
-    (task-class-name->namespace-str \"metabase.task.upgrade_checks.CheckForNewVersions\")
-    ;; -> \"metabase.task.upgrade-checks\""
-  [class-name]
-  (-> class-name
-      (str/replace \_ \-)
-      (str/replace #"\.\w+$" "")))
-
-(defn- require-task-namespace
-  "Since Metabase tasks are defined in Clojure-land we need to make sure we `require` the namespaces where they are
-  defined before we try to load the task classes."
-  [class-name]
-  ;; only try to `require` metabase.task classes; don't do this for other stuff that gets shuffled thru here like
-  ;; Quartz classes
-  (when (str/starts-with? class-name "metabase.task.")
-    (require (symbol (task-class-name->namespace-str class-name)))))
-
-(defn -loadClass
-  "Class loadClass(String className)
-
-  Return the class with the given name."
-  ([^metabase.task.DynamicClassLoadHelper this, ^String class-name]
-   (require-task-namespace class-name)
-   (.superLoadClass this class-name true)) ; loadClass(String name, boolean resolve)
-  ([^metabase.task.DynamicClassLoadHelper this, ^String class-name, _]
-   (require-task-namespace class-name)
-   (.superLoadClass this class-name true)))
-
-(defn -getClassLoader
-  "ClassLoader getClassLoader()
-
-  Enable sharing of the class-loader with 3rd party"
-  [this]
-  this)

test/metabase/task/DynamicClassLoadHelper_test.clj

-(ns metabase.task.DynamicClassLoadHelper-test
-  (:require [expectations :refer :all]
-            [metabase.task.DynamicClassLoadHelper :as DynamicClassLoadHelper]))
-
-(expect
-  "metabase.task.upgrade-checks"
-  (#'DynamicClassLoadHelper/task-class-name->namespace-str "metabase.task.upgrade_checks.CheckForNewVersions"))

test/metabase/task_test.clj

+(ns metabase.task-test
+  (:require [expectations :refer [expect]]
+            [metabase.task :as task]))
+
+(expect
+  "metabase.task.upgrade-checks"
+  (#'task/task-class-name->namespace-str "metabase.task.upgrade_checks.CheckForNewVersions"))