✨ Improve internal state validation on db module

backend/src/app/db.clj

@@ -17,7 +17,6 @@
    [app.db.sql :as sql]
    [app.metrics :as mtx]
    [app.util.json :as json]
-   [app.util.migrations :as mg]
    [app.util.time :as dt]
    [clojure.java.io :as io]
    [clojure.spec.alpha :as s]
@@ -32,7 +31,6 @@
    io.whitfin.siphash.SipHasherContainer
    java.io.InputStream
    java.io.OutputStream
-   java.lang.AutoCloseable
    java.sql.Connection
    java.sql.Savepoint
    org.postgresql.PGConnection
@@ -50,12 +48,9 @@
 ;; Initialization
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-(declare apply-migrations!)
-
 (s/def ::connection-timeout ::us/integer)
 (s/def ::max-size ::us/integer)
 (s/def ::min-size ::us/integer)
-(s/def ::migrations map?)
 (s/def ::name keyword?)
 (s/def ::password ::us/string)
 (s/def ::uri ::us/not-empty-string)
@@ -64,26 +59,26 @@
 (s/def ::read-only? ::us/boolean)
 
 (s/def ::pool-options
-  (s/keys :opt-un [::uri ::name
-                   ::min-size
-                   ::max-size
-                   ::connection-timeout
-                   ::validation-timeout
-                   ::migrations
-                   ::username
-                   ::password
-                   ::mtx/metrics
-                   ::read-only?]))
+  (s/keys :req [::uri]
+          :opt [::name
+                ::min-size
+                ::max-size
+                ::connection-timeout
+                ::validation-timeout
+                ::username
+                ::password
+                ::mtx/metrics
+                ::read-only?]))
 
 (def defaults
-  {:name :main
-   :min-size 0
-   :max-size 60
-   :connection-timeout 10000
-   :validation-timeout 10000
-   :idle-timeout 120000 ; 2min
-   :max-lifetime 1800000 ; 30m
-   :read-only? false})
+  {::name :main
+   ::min-size 0
+   ::max-size 60
+   ::connection-timeout 10000
+   ::validation-timeout 10000
+   ::idle-timeout 120000 ; 2min
+   ::max-lifetime 1800000 ; 30m
+   ::read-only? false})
 
 (defmethod ig/prep-key ::pool
   [_ cfg]
@@ -93,39 +88,22 @@
 (defmethod ig/pre-init-spec ::pool [_] ::pool-options)
 
 (defmethod ig/init-key ::pool
-  [_ {:keys [migrations read-only? uri] :as cfg}]
-  (if uri
-    (let [pool (create-pool cfg)]
-      (l/info :hint "initialize connection pool"
-              :name (d/name (:name cfg))
-              :uri uri
-              :read-only read-only?
-              :with-credentials (and (contains? cfg :username)
-                                     (contains? cfg :password))
-              :min-size (:min-size cfg)
-              :max-size (:max-size cfg))
-      (when-not read-only?
-        (some->> (seq migrations) (apply-migrations! pool)))
-      pool)
-
-    (do
-      (l/warn :hint "unable to initialize pool, missing url"
-              :name (d/name (:name cfg))
-              :read-only read-only?)
-      nil)))
+  [_ {:keys [::uri ::read-only?] :as cfg}]
+  (l/info :hint "initialize connection pool"
+          :name (d/name (::name cfg))
+          :uri uri
+          :read-only read-only?
+          :with-credentials (and (contains? cfg ::username)
+                                 (contains? cfg ::password))
+          :min-size (::min-size cfg)
+          :max-size (::max-size cfg))
+  (create-pool cfg))
 
 (defmethod ig/halt-key! ::pool
   [_ pool]
   (when pool
     (.close ^HikariDataSource pool)))
 
-(defn- apply-migrations!
-  [pool migrations]
-  (with-open [conn ^AutoCloseable (open pool)]
-    (mg/setup! conn)
-    (doseq [[name steps] migrations]
-      (mg/migrate! conn {:name (d/name name) :steps steps}))))
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; API & Impl
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -135,19 +113,19 @@
        "SET idle_in_transaction_session_timeout = 300000;"))
 
 (defn- create-datasource-config
-  [{:keys [metrics uri] :as cfg}]
+  [{:keys [::mtx/metrics ::uri] :as cfg}]
   (let [config (HikariConfig.)]
     (doto config
       (.setJdbcUrl           (str "jdbc:" uri))
-      (.setPoolName          (d/name (:name cfg)))
+      (.setPoolName          (d/name (::name cfg)))
       (.setAutoCommit true)
-      (.setReadOnly          (:read-only? cfg))
-      (.setConnectionTimeout (:connection-timeout cfg))
-      (.setValidationTimeout (:validation-timeout cfg))
-      (.setIdleTimeout       (:idle-timeout cfg))
-      (.setMaxLifetime       (:max-lifetime cfg))
-      (.setMinimumIdle       (:min-size cfg))
-      (.setMaximumPoolSize   (:max-size cfg))
+      (.setReadOnly          (::read-only? cfg))
+      (.setConnectionTimeout (::connection-timeout cfg))
+      (.setValidationTimeout (::validation-timeout cfg))
+      (.setIdleTimeout       (::idle-timeout cfg))
+      (.setMaxLifetime       (::max-lifetime cfg))
+      (.setMinimumIdle       (::min-size cfg))
+      (.setMaximumPoolSize   (::max-size cfg))
       (.setConnectionInitSql initsql)
       (.setInitializationFailTimeout -1))
 
@@ -157,8 +135,8 @@
            (PrometheusMetricsTrackerFactory.)
            (.setMetricsTrackerFactory config)))
 
-    (some->> ^String (:username cfg) (.setUsername config))
-    (some->> ^String (:password cfg) (.setPassword config))
+    (some->> ^String (::username cfg) (.setUsername config))
+    (some->> ^String (::password cfg) (.setPassword config))
 
     config))
 
@@ -167,6 +145,7 @@
   (instance? javax.sql.DataSource v))
 
 (s/def ::pool pool?)
+(s/def ::nilable-pool (s/nilable ::pool))
 (s/def ::conn-or-pool some?)
 
 (defn closed?

backend/src/app/main.clj

@@ -161,15 +161,13 @@
 
 (def system-config
   {::db/pool
-   {:uri        (cf/get :database-uri)
-    :username   (cf/get :database-username)
-    :password   (cf/get :database-password)
-    :read-only  (cf/get :database-readonly false)
-    :metrics    (ig/ref ::mtx/metrics)
-    :migrations (ig/ref :app.migrations/all)
-    :name       :main
-    :min-size   (cf/get :database-min-pool-size 0)
-    :max-size   (cf/get :database-max-pool-size 60)}
+   {::db/uri        (cf/get :database-uri)
+    ::db/username   (cf/get :database-username)
+    ::db/password   (cf/get :database-password)
+    ::db/read-only? (cf/get :database-readonly false)
+    ::db/min-size   (cf/get :database-min-pool-size 0)
+    ::db/max-size   (cf/get :database-max-pool-size 60)
+    ::mtx/metrics   (ig/ref ::mtx/metrics)}
 
    ;; Default thread pool for IO operations
    ::wrk/executor
@@ -184,7 +182,7 @@
     ::wrk/executor (ig/ref ::wrk/executor)}
 
    :app.migrations/migrations
-   {}
+   {::db/pool (ig/ref ::db/pool)}
 
    ::mtx/metrics
    {:default default-metrics}
@@ -192,9 +190,6 @@
    ::mtx/routes
    {::mtx/metrics (ig/ref ::mtx/metrics)}
 
-   :app.migrations/all
-   {:main (ig/ref :app.migrations/migrations)}
-
    ::rds/redis
    {::rds/uri     (cf/get :redis-uri)
     ::mtx/metrics (ig/ref ::mtx/metrics)}
@@ -426,8 +421,12 @@
    {::http.client/client (ig/ref ::http.client/client)}
 
    :app.setup/props
-   {:pool (ig/ref ::db/pool)
-    :key  (cf/get :secret-key)}
+   {::db/pool    (ig/ref ::db/pool)
+    ::key        (cf/get :secret-key)
+
+    ;; NOTE: this dependency is only necessary for proper initialization ordering, props
+    ;; module requires the migrations to run before initialize.
+    ::migrations (ig/ref :app.migrations/migrations)}
 
    ::audit/collector
    {::db/pool           (ig/ref ::db/pool)

backend/src/app/migrations.clj

@@ -6,8 +6,12 @@
 
 (ns app.migrations
   (:require
+   [app.common.data.macros :as dm]
+   [app.common.logging :as l]
+   [app.db :as db]
    [app.migrations.clj.migration-0023 :as mg0023]
    [app.util.migrations :as mg]
+   [clojure.spec.alpha :as s]
    [integrant.core :as ig]))
 
 (def migrations
@@ -313,5 +317,19 @@
 
   ])
 
+(defn- apply-migrations!
+  [pool migrations]
+  ;; (app.common.pprint/pprint migrations)
+  (dm/with-open [conn (db/open pool)]
+    (mg/setup! conn)
+    (mg/migrate! conn {:name "main" :steps migrations})))
 
-(defmethod ig/init-key ::migrations [_ _] migrations)
+(defmethod ig/pre-init-spec ::migrations
+  [_]
+  (s/keys :req [::db/pool]))
+
+(defmethod ig/init-key ::migrations
+  [module {:keys [::db/pool]}]
+  (when-not (db/read-only? pool)
+    (l/info :hint "running migrations" :module module)
+    (some->> (seq migrations) (apply-migrations! pool))))

backend/src/app/setup.clj

@@ -50,14 +50,16 @@
                       :cause cause))))
         instance-id)))
 
+(s/def ::main/key ::us/string)
 (s/def ::main/props
   (s/map-of ::us/keyword some?))
 
 (defmethod ig/pre-init-spec ::props [_]
-  (s/keys :req-un [::db/pool]))
+  (s/keys :req [::db/pool]
+          :opt [::main/key]))
 
 (defmethod ig/init-key ::props
-  [_ {:keys [pool key] :as cfg}]
+  [_ {:keys [::db/pool ::main/key] :as cfg}]
   (db/with-atomic [conn pool]
     (db/xact-lock! conn 0)
     (when-not key

backend/src/app/util/migrations.clj

@@ -13,7 +13,7 @@
 
 (s/def ::name string?)
 (s/def ::step (s/keys :req-un [::name ::fn]))
-(s/def ::steps (s/every ::step :kind vector?))
+(s/def ::steps (s/every ::step))
 (s/def ::migrations
   (s/keys :req-un [::name ::steps]))