Fork me on GitHub
Slack Archive

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

2023-09-12

Channels

#sql
<
2023-09-12
>
jumar21:09:07

Is anybody using liquibase for their db migrations? Do you also use custom migrations written in Clojure? If so, how do you handle them - do you use gen-class and AOT-compile your migrations, even in the dev mode? That's our current approach but it's cumbersome (especially when I'm porting leiningen-based project to deps.edn). I tried to use deftype instead but it throws ClassNotFoundException More details in the thread.

jumar21:09:31

(deftype CustomTaskChange []
  liquibase.change.custom.CustomTaskChange
  (getConfirmationMessage [this] "something")
  (setFileOpener [this resourceAccessor] nil)
  (setUp [this] nil)
  (^ValidationErrors validate [this ^Database database] (ValidationErrors.))
  (^void execute [this ^Database database] (println "Dummy transaction execution...."))
  liquibase.change.custom.CustomTaskRollback (^void rollback [this ^Database database] nil))
;; => myapp.database.migrations.mig2808.CustomTaskChange

;; defining the migration in an XML file like this:
<databaseChangeLog
        xmlns=""
        xmlns:xsi=""
        xsi:schemaLocation=" ">
    <changeSet id="2080_01">
        <customChange class="myapp.database.migrations.Mig2808" />
    </changeSet>
</databaseChangeLog>



;; then  triggering the migrations

1. Caused by java.lang.ClassNotFoundException
   myapp.database.migrations.Mig2808.CustomTaskChange

    URLClassLoader.java:  445  java.net.URLClassLoader/findClass
   DynamicClassLoader.java:   69  clojure.lang.DynamicClassLoader/findClass
          ClassLoader.java:  592  java.lang.ClassLoader/loadClass
   DynamicClassLoader.java:   77  clojure.lang.DynamicClassLoader/loadClass
          ClassLoader.java:  525  java.lang.ClassLoader/loadClass
                Class.java:   -2  java.lang.Class/forName0
                Class.java:  467  java.lang.Class/forName
  CustomChangeWrapper.java:   79  liquibase.change.custom.CustomChangeWrapper/setClass
  CustomChangeWrapper.java:  298  liquibase.change.custom.CustomChangeWrapper/load
            ChangeSet.java:  535  liquibase.changelog.ChangeSet/toChange
            ChangeSet.java:  460  liquibase.changelog.ChangeSet/handleChildNode
            ChangeSet.java:  384  liquibase.changelog.ChangeSet/load
    DatabaseChangeLog.java:  761  liquibase.changelog.DatabaseChangeLog/createChangeSet
...

seancorfield22:09:00

I don't know how much you've edited the copy'n'paste above but these don't match: 

cacsremoteservice.database.migrations.mig2808.CustomTaskChange

        <customChange class="myapp.database.migrations.Mig2808" />

1. Caused by java.lang.ClassNotFoundException
   myapp.database.migrations.Mig2808.CustomTaskChange

jumar04:09:43

@U04V70XH6 I wasn't diligent enough when editing the example. But in real life, those names do match.

jumar04:09:39

@U11BV7MTK thanks for the link! I don't fully understand the implementation but it seems that it's simply using defrecord , similarly to how I used deftype. Are there any more tricks to make it work with Liquibase classloading?

jumar04:09:40

So I tried to copy those metabase macros to my project: 

(ns cacsremoteservice.database.migrations.custom-migrations
  "Most of this code was copied from metabase: "
  (:require
   [clojure.java.jdbc :as jdbc]
   [taoensso.timbre :as log]))

(defmacro define-reversible-migration
  "Define a reversible custom migration. Both the forward and reverse migrations are defined using the same structure,
  similar to the bodies of multi-arity Clojure functions.

  Example:
clj (define-reversible-migration ExampleMigrationName tx (migration-body tx) (reverse-migration-body tx))) 
"
  [name tx-symbol migration-body reverse-migration-body]
  `(defrecord ~name []
     liquibase.change.custom.CustomTaskChange
     (execute [_# database#]
       (jdbc/with-db-transaction [~tx-symbol {:connection (.getUnderlyingConnection (.getConnection database#))}]
         ~migration-body))
     (getConfirmationMessage [_#]
       (str "Custom migration: " ~name))
     (setUp [_#])
     (validate [_# _database#]
       (liquibase.exception.ValidationErrors.))
     (setFileOpener [_# _resourceAccessor#])

     liquibase.change.custom.CustomTaskRollback
     (rollback [_# database#]
       (jdbc/with-db-transaction [~tx-symbol {:connection (.getUnderlyingConnection (.getConnection database#))}]
         ~reverse-migration-body))))

(defn no-op
  "No-op logging rollback function"
  [n]
  (log/info "No rollback for: " n))

(defmacro define-migration
  "Define a custom migration without a reverse migration."
  [name tx-symbol & migration-body]
  `(define-reversible-migration ~name ~tx-symbol (do ~@migration-body) (no-op ~(str name))))
Then use use it to define those migrations in seperate namespaces, e.g. 
(ns cacsremoteservice.database.migrations.mig1122-01
  (:require
   [cacsremoteservice.database.migrations.custom-migrations :as custom-migrations]))

(defn execute [db-spec]
  ...)

(custom-migrations/define-migration MyFirstMigration tx
  (execute tx))
;; => cacsremoteservice.database.migrations.mig1122_01.MyFirstMigration
And reference it in the xml file 
<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<databaseChangeLog
        xmlns=""
        xmlns:xsi=""
        xsi:schemaLocation=" ">
    <changeSet id="1122_01">
      <customChange class="cacsremoteservice.database.migrations.Mig1122_01.MyFirstMigration" />
    </changeSet>
</databaseChangeLog>
But that is still failing with ClassNotFoundException as before 
1. Caused by java.lang.ClassNotFoundException
   cacsremoteservice.database.migrations.Mig1122_01.MyFirstMigration

       URLClassLoader.java:  445  java.net.URLClassLoader/findClass
   DynamicClassLoader.java:   69  clojure.lang.DynamicClassLoader/findClass
          ClassLoader.java:  592  java.lang.ClassLoader/loadClass
   DynamicClassLoader.java:   77  clojure.lang.DynamicClassLoader/loadClass
          ClassLoader.java:  525  java.lang.ClassLoader/loadClass
                Class.java:   -2  java.lang.Class/forName0
                Class.java:  467  java.lang.Class/forName
  CustomChangeWrapper.java:   79  liquibase.change.custom.CustomChangeWrapper/setClass
  CustomChangeWrapper.java:  298  liquibase.change.custom.CustomChangeWrapper/load
            ChangeSet.java:  535  liquibase.changelog.ChangeSet/toChange
            ChangeSet.java:  460  liquibase.changelog.ChangeSet/handleChildNode
            ChangeSet.java:  384  liquibase.changelog.ChangeSet/load
    DatabaseChangeLog.java:  761  liquibase.changelog.DatabaseChangeLog/createChangeSet

dpsutton04:09:34

i think that should be solved by these lines 

(defn- load-class ^Class [^String class-name]
  (Class/forName class-name true (classloader/the-classloader)))

(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)))
this is how those classes are found by the liquibase classloader

jumar04:09:17

I got confused by seeing "quartz" there - what's the connection to Liquibase?

jumar04:09:40

Note: I don't have quartz on the classpath...

dpsutton04:09:07

ah right. that’s for the task runner. let me check

dpsutton04:09:52

here’s where we first introduced it: https://github.com/metabase/metabase/pull/28175/files . My memory is coming back a bit. We’re running the migrations from our main thread which has the clojure dynamic class loader so it can see these. Are you running your migrations from some other thread without this classloader? And are you sure the namespaces with the custom migrations is loaded before attempting to run the migrations?

jumar04:09:46

Regarding loading - something I'm fixing right now but at the moment, I was experimenting with this in the REPL by first evaluating the migration definition and then re-running migrations - that should make sure that they are loaded first. The migrations should also run in the same/main thread but I'm not sure what liquibase is doing under the hood...

jumar05:09:27

I'm thinking that this might be it: https://github.com/metabase/metabase/blob/77c64754c02eb0854182b96a0c6e6b96fa3b6b2c/src/metabase/db/liquibase.clj#L63 We construct the Liquibase object just like this 

(Liquibase. "liquibase/migrations/master.xml" (ClassLoaderResourceAccessor.) jdbc-connection)
But even changing that to this doesn't help 
(Liquibase. "liquibase/migrations/master.xml" (ClassLoaderResourceAccessor. (.getContextClassLoader (Thread/currentThread))) jdbc-connection)

jumar05:09:36

Ah, ok. So that does help. I forgot to update the classname in the xml file after I moved stuff around.

jumar05:09:20

Now I need to sort out tricky cyclic dependencies between those migrations and other db code 😮 But that's another thing. Thanks @U11BV7MTK for the help!

💪 1
jumar05:09:17

Just a tangent: is there a reason to prefer defrecord over deftype?

dpsutton06:09:13

deftype is probably more correct. i don’t think there was any reason for defrecord. probably just for printing at the repl and i was lazy

jumar08:09:31

I've managed to run this successufully in the REPL. However, when I try to run tests (`lein test` or via deps.edn alias and eftest) I'm again getting ClassNotFoundException-s 

liquibase.exception.ChangeLogParseException: liquibase.parser.core.ParsedNodeException: liquibase.exception.CustomChangeException: java.lang.ClassNotFoundException: cacsremoteservice.database.migrations.custom_migrations.Mig1122_01
  liquibase.parser.core.ParsedNodeException: liquibase.exception.CustomChangeException: java.lang.ClassNotFoundException: cacsremoteservice.database.migrations.custom_migrations.Mig1122_01
  liquibase.exception.CustomChangeException: java.lang.ClassNotFoundException: cacsremoteservice.database.migrations.custom_migrations.Mig1122_01
           java.lang.ClassNotFoundException: cacsremoteservice.database.migrations.custom_migrations.Mig1122_01
         jdk.internal.loader.BuiltinClassLoader.loadClass       BuiltinClassLoader.java:  641
jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass             ClassLoaders.java:  188
                          java.lang.ClassLoader.loadClass              ClassLoader.java:  525
                                 java.lang.Class.forName0                     Class.java
                                  java.lang.Class.forName                    Class.java:  467
     liquibase.change.custom.CustomChangeWrapper.setClass      CustomChangeWrapper.java:   79
         liquibase.change.custom.CustomChangeWrapper.load      CustomChangeWrapper.java:  298
                   liquibase.changelog.ChangeSet.toChange                ChangeSet.java:  535
...
                               liquibase.Liquibase.update                Liquibase.java:  207
         cacsremoteservice.database.migrations/migrate/fn                migrations.clj:   34
                        clojure.java.jdbc/db-transaction*                      jdbc.clj:  807
                        clojure.java.jdbc/db-transaction*                      jdbc.clj:  852
                        clojure.java.jdbc/db-transaction*                      jdbc.clj:  789
            cacsremoteservice.database.migrations/migrate                migrations.clj:   19
...
I do require my custom migrations transitively in the db-fixture namespace. 
(:require
   :reload-all
   :verbose
   [cacsremoteservice.database.db :as db]
   [cacsremoteservice.database.migrations :as db-migrations]
...

jumar08:09:38

In desperation, I even added :reload-all but no difference...

dpsutton08:09:17

My suggestion would be to find where you call the migration code. And right before that call class on the deftype of your custom migration. Then assert that classforname can find it. And then call your migration code

👍 1
jumar12:09:33

I tried your suggestion and it all looked well. Until I started logging the classloader too 

;; in the REPL
current classloader:  #object[clojure.lang.DynamicClassLoader 0xd8d9f44 "clojure.lang.DynamicClassLoader@d8d9f44"]

;; when run via `lein test` 
current classloader:  #object[jdk.internal.loader.ClassLoaders$AppClassLoader 0x531d72ca "jdk.internal.loader.ClassLoaders$AppClassLoader@531d72ca"]
So it's AppClassLoader which doesn't know about classes loaded by Clojure's DynamicClassLoader. I set up Liquibase using (.getContextClassLoader (Thread/currentThread) 
(Liquibase. "liquibase/migrations/master.xml"
                                ;; - inspired by 
                                (ClassLoaderResourceAccessor. (.getContextClassLoader (Thread/currentThread))) jdbc-connection)

jumar12:09:22

I found a related discussion here: https://groups.google.com/g/clojure/c/PdVEeU1NbAQ I cannot quite understand all the details but I guess this is the reason: > When you aot compile you generate a class file on disk that is visible to the system classloader. > • When the reference to the Foo class is compiled, it is compiled as a call something like clojure.lang.RT.classForName("Foo"); > ◦ RT.classForName uses different class loaders depend on this that an the other, and in this case it is using the system classloader (the AppClassLoader) when using lein trampoline. > • When you loaded your clojure code it ran through the compiler which generated bytecode which is loaded via clojure's DynamicClassLoader. > • The way RT.classForName determines which classloader to use is something like: > ◦ if there is a DynamicClassloader available from the compiler, use it, otherwise use the system classloader. > ◦ Once you finish loading code the DynamicClassloader the compiler uses is, uh, for lack of a better word, popped, so it isn't available to RT.classForName. > • So given that and the error we can deduce that lein trampoline runs in t[w]o phases: a code loading phase, and then an execution phase. > ◦ Which is a perfectly reasonable thing to do and easy to do with the command line options to clojure.main. > • I suspect adding an import of the Foo class would work around this issue, but I strongly recommend you consider not aot compiling instead. > ◦ AOT compilation is a source of weird behavior and just isn't worth it. The difference in my case is that I'm actually trying to avoid AOT compilation! With AOT, all works, it seems because that the on-disk class file becomes available to the AppClassLoader. The key thing anyway, is that when the tests execute via lein test there's AppClassLoader instance in use which doesn't know anything about my deftype-based classes.

jumar13:09:54

I tried to capture the classloader of one of the migration classes and pass that to Liquibase, but it doesn't seem to work. For some reason, it's still seems to be using AppClassLoader 

(log/info "current thread classloader: " (.getContextClassLoader (Thread/currentThread)))
    (log/info "Clojure's classloader" (.getClassLoader (class custom-migrations/->Mig1122_01)))
    (let [jdbc-connection (JdbcConnection. (:connection connection))
          liquibase (Liquibase. "liquibase/migrations/master.xml"
                                (ClassLoaderResourceAccessor. (.getClassLoader (class custom-migrations/->Mig1122_01))) jdbc-connection)]
lein test
...
current thread classloader:  #object[jdk.internal.loader.ClassLoaders$AppClassLoader 0x531d72ca "jdk.internal.loader.ClassLoaders$AppClassLoader@531d72ca"]
Clojure's classloader #object[clojure.lang.DynamicClassLoader 0x4bdd094b "clojure.lang.DynamicClassLoader@4bdd094b"]
...
Caused by: java.lang.ClassNotFoundException: ...custom_migrations.Mig1122_01
 at jdk.internal.loader.BuiltinClassLoader.loadClass (BuiltinClassLoader.java:641)
    jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass (ClassLoaders.java:188)
    java.lang.ClassLoader.loadClass (ClassLoader.java:525)
    java.lang.Class.forName0 (Class.java:-2)
    java.lang.Class.forName (Class.java:467)
    liquibase.change.custom.CustomChangeWrapper.setClass (CustomChangeWrapper.java:79)
    liquibase.change.custom.CustomChangeWrapper.load (CustomChangeWrapper.java:298)
...
    liquibase.Liquibase.update (Liquibase.java:207)
    ...migrations$migrate$fn__79059.invoke (migrations.clj:35)

jumar07:09:01

I was able to finally solve this problem! Nothing worked except using setting Thread's context class loader before calling liquibase 

(let
...
          _ (.setContextClassLoader (Thread/currentThread) clojure-classloader)
          liquibase (Liquibase. "liquibase/migrations/master.xml"
                                ;; ideally passing clojure-classloader here would be enough, but it doesn't work (see comment above)
                                (ClassLoaderResourceAccessor. clojure-classloader) jdbc-connection)]
   
      (try
        (.update liquibase (Contexts.))
        (finally
          ;; restore the original classloader
          (.setContextClassLoader (Thread/currentThread) original-classloader))))))

🎉 1
emccue18:09:09

we used clj-liquibase for awhile

emccue18:09:18

wrote edn files for migrations and whatnot

jumar07:09:01

I was able to finally solve this problem! Nothing worked except using setting Thread's context class loader before calling liquibase 

(let
...
          _ (.setContextClassLoader (Thread/currentThread) clojure-classloader)
          liquibase (Liquibase. "liquibase/migrations/master.xml"
                                ;; ideally passing clojure-classloader here would be enough, but it doesn't work (see comment above)
                                (ClassLoaderResourceAccessor. clojure-classloader) jdbc-connection)]
   
      (try
        (.update liquibase (Contexts.))
        (finally
          ;; restore the original classloader
          (.setContextClassLoader (Thread/currentThread) original-classloader))))))

🎉 1