define- helper macros should generate unique vars with named based …

…on model (#109) * `define-` helper macros should generate unique vars with named based on model * Sort namespaces * Revert MariaDB driver bump * Switch protocol back to mysql:// * Sort namespaces
camsaul · Feb 3, 2023 · 4f51679 · 4f51679
1 parent 999e636
commit 4f51679
Show file tree

Hide file tree

Showing 19 changed files with 247 additions and 30 deletions.
diff --git a/deps.edn b/deps.edn
@@ -23,7 +23,11 @@
     org.clojure/java.classpath           {:mvn/version "1.0.0"}
     org.clojure/math.combinatorics       {:mvn/version "0.1.6"}
     org.clojure/tools.namespace          {:mvn/version "1.3.0"}
-    org.mariadb.jdbc/mariadb-java-client {:mvn/version "3.1.2"}
+    ;; Don't upgrade to 3.x yet. It only returns the first generated key when inserting multiple rows. So keep using v2
+    ;; until we decide how to work around it or they fix it. See
+    ;;
+    ;; https://mariadb.com/kb/en/about-mariadb-connector-j/#generated-keys-limitation
+    org.mariadb.jdbc/mariadb-java-client {:mvn/version "2.7.8"}
     org.postgresql/postgresql            {:mvn/version "42.5.2"}
     pjstadig/humane-test-output          {:mvn/version "0.11.0"}}
 

diff --git a/src/toucan2/jdbc/read.clj b/src/toucan2/jdbc/read.clj
@@ -46,7 +46,7 @@
       (log/debugf :results
                   "Column %s %s is of JDBC type %s, native type %s"
                   i
-                  (str (.getTableName rsmeta i) \. (.getColumnLabel rsmeta i))
+                  (str (some->> (.getTableName rsmeta i) not-empty (str \.)) (.getColumnLabel rsmeta i))
                   (symbol "java.sql.Types" (type-name col-type))
                   (.getColumnTypeName rsmeta i))
       [(protocols/dispatch-value conn) (protocols/dispatch-value model) col-type])))

diff --git a/src/toucan2/query_execution_backend/jdbc.clj b/src/toucan2/query_execution_backend/jdbc.clj
@@ -79,6 +79,7 @@
 (when (class-exists? "org.postgresql.jdbc.PgConnection")
   (require 'toucan2.query-execution-backend.jdbc.postgres))
 
-(when (or (class-exists? "com.mysql.cj.MysqlConnection")
-          (class-exists? "org.mariadb.jdbc.MariaDbConnection"))
+(when (some class-exists? ["org.mariadb.jdbc.Connection"
+                           "org.mariadb.jdbc.MariaDbConnection"
+                           "com.mysql.cj.MysqlConnection"])
   (require 'toucan2.query-execution-backend.jdbc.mysql-mariadb))
diff --git a/src/toucan2/query_execution_backend/jdbc/mysql_mariadb.clj b/src/toucan2/query_execution_backend/jdbc/mysql_mariadb.clj
@@ -16,15 +16,13 @@
 
 ;;; TODO -- need the MySQL class here too.
 
-(when-let [mariadb-connection-class (try
-                                      (Class/forName "org.mariadb.jdbc.MariaDbConnection")
-                                      (catch Throwable _))]
-  (derive mariadb-connection-class ::connection))
-
-(when-let [mysql-connection-class (try
-                                    (Class/forName "com.mysql.cj.MysqlConnection")
-                                    (catch Throwable _))]
-  (derive mysql-connection-class ::connection))
+(doseq [^String connection-class-name ["org.mariadb.jdbc.Connection"
+                                       "org.mariadb.jdbc.MariaDbConnection"
+                                       "com.mysql.cj.MysqlConnection"]]
+  (when-let [connection-class (try
+                                (Class/forName connection-class-name)
+                                (catch Throwable _))]
+    (derive connection-class ::connection)))
 
 (m/defmethod jdbc.read/read-column-thunk [#_conn  ::connection
                                           #_model :default

diff --git a/src/toucan2/tools/before_insert.clj b/src/toucan2/tools/before_insert.clj
@@ -57,9 +57,9 @@
 (defmacro define-before-insert
   {:style/indent :defn}
   [model [instance-binding] & body]
-  `(let [model# ~model]
-     (u/maybe-derive model# ::before-insert)
-     (m/defmethod before-insert model#
+  `(do
+     (u/maybe-derive ~model ::before-insert)
+     (m/defmethod before-insert ~model
        [~'&model ~instance-binding]
        (cond->> (do ~@body)
          ~'next-method (~'next-method ~'&model)))))

diff --git a/src/toucan2/tools/before_update.clj b/src/toucan2/tools/before_update.clj
@@ -120,9 +120,9 @@
          new-args-maps)))))
 
 (defmacro define-before-update [model [instance-binding] & body]
-  `(let [model# ~model]
-     (u/maybe-derive model# ::before-update)
-     (m/defmethod before-update model#
+  `(do
+     (u/maybe-derive ~model ::before-update)
+     (m/defmethod before-update ~model
        [~'&model ~instance-binding]
        (cond->> (do ~@body)
          ~'next-method

diff --git a/src/toucan2/tools/default_fields.clj b/src/toucan2/tools/default_fields.clj
@@ -113,9 +113,9 @@
                   [:toucan.result-type/instances :toucan2.tools.after/model])
 
 (defmacro define-default-fields {:style/indent :defn} [model & body]
-  `(let [model# ~model]
-     (u/maybe-derive model# ::default-fields)
-     (m/defmethod default-fields model# [~'&model] ~@body)))
+  `(do
+     (u/maybe-derive ~model ::default-fields)
+     (m/defmethod default-fields ~model [~'&model] ~@body)))
 
 (s/fdef define-default-fields
   :args (s/cat :model some?

diff --git a/src/toucan2/tools/transformed.clj b/src/toucan2/tools/transformed.clj
@@ -414,9 +414,9 @@
   ```"
   {:style/indent 1}
   [model column->direction->fn]
-  `(let [model# ~model]
-     (u/maybe-derive model# ::transformed.model)
-     (m/defmethod transforms model#
+  `(do
+     (u/maybe-derive ~model ::transformed.model)
+     (m/defmethod transforms ~model
        [~'model]
        ~column->direction->fn)))
 

diff --git a/test/toucan2/test.clj b/test/toucan2/test.clj
@@ -184,7 +184,7 @@
 
 (defmethod default-test-db-url :mariadb
   [_db-type]
-  "jdbc:mariadb://localhost:3306/metabase_test?user=root")
+  "jdbc:mysql://localhost:3306/metabase_test?user=root")
 
 (defmethod default-test-db-url :h2
   [_db-type]

diff --git a/test/toucan2/tools/after_insert_test.clj b/test/toucan2/tools/after_insert_test.clj
@@ -1,6 +1,8 @@
 (ns toucan2.tools.after-insert-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.insert :as insert]
    [toucan2.instance :as instance]
    [toucan2.protocols :as protocols]
@@ -253,3 +255,26 @@
                        :created-at (LocalDateTime/parse "2017-01-01T00:00")
                        :updated-at (LocalDateTime/parse "2017-01-01T00:00")}]
                      @*venues-awaiting-moderation*)))))))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-after-insert should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "each-row-fn-primary-method"))
+                form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'each-row-fn-primary-method-toucan-query-type-insert-*-model-1
+             (generated-name `(after-insert/define-after-insert :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'each-row-fn-primary-method-toucan-query-type-insert-*-model-2
+             (generated-name `(after-insert/define-after-insert :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/after_select_test.clj b/test/toucan2/tools/after_select_test.clj
@@ -2,6 +2,7 @@
   (:require
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.insert :as insert]
    [toucan2.instance :as instance]
    [toucan2.protocols :as protocols]
@@ -144,3 +145,26 @@
       (testing `protocols/changes
         (is (= nil
                (protocols/changes person)))))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-after-select should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "after-select"))
+                form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'after-select-primary-method-model-1
+             (generated-name `(after-select/define-after-select :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'after-select-primary-method-model-2
+             (generated-name `(after-select/define-after-select :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/after_update_test.clj b/test/toucan2/tools/after_update_test.clj
@@ -1,6 +1,8 @@
 (ns toucan2.tools.after-update-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.instance :as instance]
    [toucan2.select :as select]
    [toucan2.test :as test]
@@ -150,3 +152,26 @@
                                         :updated-at (LocalDateTime/parse "2017-01-01T00:00")})]
                    (select/select [model :id :name :updated-at]
                                   {:order-by [[:id :asc]]})))))))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-after-update should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "each-row-fn-primary-method-toucan-query-type-update"))
+                form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'each-row-fn-primary-method-toucan-query-type-update-*-model-1
+             (generated-name `(after-update/define-after-update :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'each-row-fn-primary-method-toucan-query-type-update-*-model-2
+             (generated-name `(after-update/define-after-update :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/before_delete_test.clj b/test/toucan2/tools/before_delete_test.clj
@@ -1,6 +1,8 @@
 (ns toucan2.tools.before-delete-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.delete :as delete]
    [toucan2.execute :as execute]
    [toucan2.instance :as instance]
@@ -174,3 +176,26 @@
                @*deleted-venues*))
         (is (= nil
                (select/select-fn-set :id ::test/venues)))))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-before-delete should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "before-delete"))
+                form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'before-delete-primary-method-model-1
+             (generated-name `(before-delete/define-before-delete :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'before-delete-primary-method-model-2
+             (generated-name `(before-delete/define-before-delete :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/before_insert_test.clj b/test/toucan2/tools/before_insert_test.clj
@@ -3,6 +3,7 @@
    [clojure.edn :as edn]
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.insert :as insert]
    [toucan2.instance :as instance]
    [toucan2.select :as select]
@@ -196,3 +197,26 @@
                           :h2                  (OffsetDateTime/parse "2022-12-31T17:26:00-08:00")
                           (:postgres :mariadb) (OffsetDateTime/parse "2023-01-01T01:26Z"))}
            (select/select-one ::people.default-values 5)))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-before-insert should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "before-insert-primary-method"))
+                form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'before-insert-primary-method-model-1
+             (generated-name `(before-insert/define-before-insert :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'before-insert-primary-method-model-2
+             (generated-name `(before-insert/define-before-insert :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/before_select_test.clj b/test/toucan2/tools/before_select_test.clj
@@ -1,6 +1,8 @@
 (ns toucan2.tools.before-select-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [methodical.core :as m]
    [toucan2.instance :as instance]
    [toucan2.select :as select]
@@ -62,3 +64,25 @@
              (select/select-one [::people.increment-id :id :name] :id 1)))
       (is (= [{:id 1}]
              @*select-calls*)))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-before-select should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "before-select")) form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'before-select-primary-method-model-1
+             (generated-name `(before-select/define-before-select :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'before-select-primary-method-model-2
+             (generated-name `(before-select/define-before-select :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/before_update_test.clj b/test/toucan2/tools/before_update_test.clj
@@ -2,6 +2,7 @@
   (:require
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [methodical.core :as m]
    [toucan2.execute :as execute]
    [toucan2.instance :as instance]
@@ -444,3 +445,25 @@
                 :name       "CAM 2.0"
                 :created-at (OffsetDateTime/parse "2019-01-11T23:56Z")}
                (select/select-one ::test/people 2)))))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-before-update should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "before-update")) form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'before-update-primary-method-model-1
+             (generated-name `(before-update/define-before-update :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'before-update-primary-method-model-2
+             (generated-name `(before-update/define-before-update :model-2
+                                [~'venue]
+                                ~'venue)))))))
diff --git a/test/toucan2/tools/default_fields_test.clj b/test/toucan2/tools/default_fields_test.clj
@@ -2,6 +2,7 @@
   (:require
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [clojure.walk :as walk]
    [toucan2.insert :as insert]
    [toucan2.instance :as instance]
    [toucan2.pipeline :as pipeline]
@@ -185,3 +186,25 @@
                         (select/select-one ::venues.default-fields query))
           {select [:id :name :updated-at], :where [:= :id 1]}
           ::named-query.select-venues-override-default-fields)))))
+
+(deftest ^:parallel macroexpansion-test
+  (testing "define-default-fields should define vars with different names based on the model."
+    (letfn [(generated-name* [form]
+              (cond
+                (sequential? form)
+                (some generated-name* form)
+
+                (and (symbol? form)
+                     (str/starts-with? (name form) "default-fields")) form))
+            (generated-name [form]
+              (let [expanded (walk/macroexpand-all form)]
+                (or (generated-name* expanded)
+                    ['no-match expanded])))]
+      (is (= 'default-fields-primary-method-model-1
+             (generated-name `(default-fields/define-default-fields :model-1
+                                [~'venue]
+                                ~'venue))))
+      (is (= 'default-fields-primary-method-model-2
+             (generated-name `(default-fields/define-default-fields :model-2
+                                [~'venue]
+                                ~'venue)))))))