Symbol model resolution should use requiring-resolve; namespaced sy…

…mbols should use symbol namespace
camsaul · Jan 18, 2023 · 873cb78 · 873cb78
1 parent 88c569e
commit 873cb78
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 8 deletions.
diff --git a/toucan1/deps.edn b/toucan1/deps.edn
@@ -3,7 +3,7 @@
 
  :aliases
  {:dev
-  {:extra-paths ["test"]}
+  {:extra-paths ["test" "../test"]}
 
   ;; clojure -M:check
   :check

diff --git a/toucan1/src/toucan/models.clj b/toucan1/src/toucan/models.clj
@@ -54,11 +54,16 @@
   the [[toucan.models/root-namespace]]. This behavior should be considered deprecated -- just use model keywords
   directly."
   [symb]
-  (let [model-ns (model-symb->ns symb)]
-    @(try (ns-resolve model-ns symb)
-          (catch Throwable _
-            (require model-ns)
-            (ns-resolve model-ns symb)))))
+  (let [qualified-symb (if (namespace symb)
+                         symb
+                         (let [model-ns (model-symb->ns symb)]
+                           (symbol (name model-ns) (name symb))))]
+    (try
+      (some-> (requiring-resolve qualified-symb) var-get)
+      (catch Throwable e
+        (throw (ex-info (format "Error resolving model %s from symbol: %s" symb (ex-message e))
+                        {:symbol    symb
+                         :attempted qualified-symb}))))))
 
 (defn resolve-model
   "Deprecated: use [[toucan2.model/resolve-model]] to resolve models instead. (The Toucan 2 version doesn't support

diff --git a/toucan1/test/toucan/models_test.clj b/toucan1/test/toucan/models_test.clj
@@ -31,21 +31,36 @@
   test-setup/do-with-quoted-snake-disabled
   test-setup/do-with-default-quoting-style)
 
+#_{:clj-kondo/ignore [:unused-private-var]}
+(def ^:private MyCategory :toucan.test-models.category/Category)
+
 (deftest ^:parallel resolve-model-test
   (are [x] (= :toucan.test-models.category/Category
               (t1.models/resolve-model x))
     Category
     'Category
     [Category]
-    ['Category])
+    ['Category]
+    'toucan.models-test/MyCategory)
   (is (thrown-with-msg?
        clojure.lang.ExceptionInfo
        #"Invalid model: 2"
        (t1.models/resolve-model 2)))
   (is (thrown-with-msg?
        clojure.lang.ExceptionInfo
        #"Invalid model: :some-other-keyword"
-       (t1.models/resolve-model :some-other-keyword))))
+       (t1.models/resolve-model :some-other-keyword)))
+  (testing "Error when model cannot be resolved from the root namespace"
+    (is (thrown-with-msg?
+         clojure.lang.ExceptionInfo
+         #"Error resolving model MyCategory from symbol: Could not locate toucan/test_models/my_category__init\.class"
+         (t1.models/resolve-model 'MyCategory)))
+    (try
+      (t1.models/resolve-model 'MyCategory)
+      (catch Throwable e
+        (is (= '{:symbol    MyCategory
+                 :attempted toucan.test-models.my-category/MyCategory}
+               (ex-data e)))))))
 
 (deftest ^:parallel properties-test
   (is (= {:toucan.test-models.venue/timestamped? true}