Change how to find classes, now 100% test coverage

Use the context class loader of the current thread. Clojure internals seem to use this approach, and it allows for testing the impl factory functions.

Author: Alexander Taggart

CHANGELOG.md

@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 - Add support for testing logs in `clojure.tools.logging.test`
 
+### Changed
+- Now passes the context classloader of current thread to `Class/forName` when
+  determining whether logging implementation classes are available on the
+  classpath. This was done to allow testing of the various `impl/*-factory`
+  functions, and seems consistent with clojure internals.
+
 ## [0.4.1] - 2018-05-07
 ### Fixed
 - Fix inadvertent reflection when using log4j2.

project.clj

@@ -6,7 +6,7 @@
   :source-paths ["src/main/clojure"]
   :test-paths ["src/test/clojure"]
   :dependencies [[org.clojure/clojure "1.3.0"]]
-  :aliases {"cloverage" ["with-profile" "dev,cloverage" "cloverage"]}
+  :aliases {"cloverage" ["with-profile" "dev,cloverage" "cloverage" "--fail-threshold" "100"]}
   :profiles {:cloverage {:plugins [[lein-cloverage "1.0.9"]]}
              :dev       {:dependencies [[org.clojure/clojure "1.8.0"]
                                         [org.clojure/test.check "0.9.0"]

src/main/clojure/clojure/tools/logging/impl.clj

@@ -44,12 +44,22 @@
     (name [_] "disabled")
     (get-logger [_ _] disabled-logger)))
 
+(defn class-found?
+  "Returns true if the Class associated with the given classname can be found
+   using the context ClassLoader for the current thread."
+  [name]
+  (try
+    (Class/forName name true (.. Thread currentThread getContextClassLoader))
+    true
+    (catch ClassNotFoundException _
+      false)))
+
+
 (defn slf4j-factory
   "Returns a SLF4J-based implementation of the LoggerFactory protocol, or nil if
   not available."
   []
-  (try
-    (Class/forName "org.slf4j.Logger")
+  (when (class-found? "org.slf4j.Logger")
     (eval
       `(do
         (extend org.slf4j.Logger
@@ -88,15 +98,13 @@
           (name [_#]
             "org.slf4j")
           (get-logger [_# logger-ns#]
-            (org.slf4j.LoggerFactory/getLogger ^String (str logger-ns#))))))
-    (catch Exception e nil)))
+            (org.slf4j.LoggerFactory/getLogger ^String (str logger-ns#))))))))
 
 (defn cl-factory
   "Returns a Commons Logging-based implementation of the LoggerFactory protocol, or
   nil if not available."
   []
-  (try
-    (Class/forName "org.apache.commons.logging.Log")
+  (when (class-found? "org.apache.commons.logging.Log")
     (eval
       `(do
          (extend org.apache.commons.logging.Log
@@ -134,15 +142,13 @@
            (name [_#]
              "org.apache.commons.logging")
            (get-logger [_# logger-ns#]
-             (org.apache.commons.logging.LogFactory/getLog (str logger-ns#))))))
-    (catch Exception e nil)))
+             (org.apache.commons.logging.LogFactory/getLog (str logger-ns#))))))))
 
 (defn log4j-factory
   "Returns a Log4j-based implementation of the LoggerFactory protocol, or nil if
   not available."
   []
-  (try
-    (Class/forName "org.apache.log4j.Logger")
+  (when (class-found? "org.apache.log4j.Logger")
     (eval
       `(let [levels# {:trace org.apache.log4j.Level/TRACE
                       :debug org.apache.log4j.Level/DEBUG
@@ -165,15 +171,13 @@
            (name [_#]
              "org.apache.log4j")
            (get-logger [_# logger-ns#]
-             (org.apache.log4j.Logger/getLogger ^String (str logger-ns#))))))
-    (catch Exception e nil)))
+             (org.apache.log4j.Logger/getLogger ^String (str logger-ns#))))))))
 
 (defn log4j2-factory
   "Returns a Log4j2-based implementation of the LoggerFactory protocol, or nil if
   not available."
   []
-  (try
-    (Class/forName "org.apache.logging.log4j.Logger")
+  (when (class-found? "org.apache.logging.log4j.Logger")
     (eval
       `(let [levels# {:trace org.apache.logging.log4j.Level/TRACE
                       :debug org.apache.logging.log4j.Level/DEBUG
@@ -202,15 +206,13 @@
            (name [_#]
              "org.apache.logging.log4j")
            (get-logger [_# logger-ns#]
-             (org.apache.logging.log4j.LogManager/getLogger ^String (str logger-ns#))))))
-    (catch Exception e nil)))
+             (org.apache.logging.log4j.LogManager/getLogger ^String (str logger-ns#))))))))
 
 (defn jul-factory
   "Returns a java.util.logging-based implementation of the LoggerFactory protocol,
   or nil if not available."
   []
-  (try
-    (Class/forName "java.util.logging.Logger")
+  (when (class-found? "java.util.logging.Logger")
     (eval
       `(let [levels# {:trace java.util.logging.Level/FINEST
                       :debug java.util.logging.Level/FINE
@@ -234,8 +236,7 @@
            (name [_#]
              "java.util.logging")
            (get-logger [_# logger-ns#]
-             (java.util.logging.Logger/getLogger (str logger-ns#))))))
-    (catch Exception e nil)))
+             (java.util.logging.Logger/getLogger (str logger-ns#))))))))
 
 (defn find-factory
   "Returns the first non-nil value from slf4j-factory, cl-factory,

src/test/clojure/clojure/tools/logging/test_impl.clj

@@ -2,6 +2,109 @@
   (:use clojure.test)
   (:require [clojure.tools.logging.impl :as impl]))
 
+
+(deftest test-disabled-logger-factory
+  (is (= "disabled" (impl/name impl/disabled-logger-factory)))
+  (is (identical? impl/disabled-logger (impl/get-logger impl/disabled-logger-factory nil))))
+
+(deftest test-disabled-logger
+  (is (= false (impl/enabled? impl/disabled-logger :fatal))))
+
+(defn blacklist-loader [blacklist loader]
+  (proxy [ClassLoader] []
+    (loadClass [name]
+      (if (contains? (set blacklist) name)
+        (throw (ClassNotFoundException. name))
+        (.loadClass loader name)))))
+
+(deftest test-blacklist-loader
+  (let [white            "org.slf4j.Logger"
+        black-1          "java.util.logging.Logger"
+        black-2          "org.apache.log4j.Logger"
+        orig-loader      (.. Thread currentThread getContextClassLoader)
+        blacklist-loader (blacklist-loader #{black-1 black-2} orig-loader)]
+    (Class/forName white true orig-loader)
+    (Class/forName black-1 true orig-loader)
+    (Class/forName black-2 true orig-loader)
+    (Class/forName white true blacklist-loader)
+    (is (thrown? ClassNotFoundException
+           (Class/forName black-1 true blacklist-loader)))
+    (is (thrown? ClassNotFoundException
+           (Class/forName black-2 true blacklist-loader)))))
+
+(defmacro with-blacklist-loader [forbidden-set & body]
+  `(let [forbidden-set# ~forbidden-set
+         orig-loader# (.. Thread currentThread getContextClassLoader)
+         this-loader# (blacklist-loader forbidden-set# orig-loader#)]
+     (try
+       (.. Thread currentThread (setContextClassLoader this-loader#))
+       ~@body
+       (finally
+         (.. Thread currentThread (setContextClassLoader orig-loader#))))))
+
+(defn thread-loader []
+  (.. Thread currentThread getContextClassLoader))
+
+(deftest test-with-blacklist-loader
+  (with-blacklist-loader #{"org.apache.log4j.Logger"}
+    (Class/forName "org.slf4j.Logger" true (thread-loader))
+    (is (thrown? ClassNotFoundException
+          (Class/forName "org.apache.log4j.Logger" true (thread-loader)))))
+  (Class/forName "org.apache.log4j.Logger" true (thread-loader)))
+
+
+(deftest test-class-found?
+  (is (= true (impl/class-found? "org.apache.log4j.Logger")))
+  (is (= true (impl/class-found? "org.slf4j.Logger")))
+  (with-blacklist-loader #{"org.apache.log4j.Logger"}
+    (is (= true (impl/class-found? "org.slf4j.Logger")))
+    (is (= false (impl/class-found? "org.apache.log4j.Logger")))))
+
+
+(deftest test-find-factory
+  (testing "has all loggers"
+    (are [classname] (impl/class-found? classname)
+         "org.slf4j.Logger"
+         "org.apache.commons.logging.Log"
+         "org.apache.logging.log4j.Logger"
+         "org.apache.log4j.Logger"
+         "java.util.logging.Logger"))
+  
+  (testing "finds slf4j"
+    (with-blacklist-loader []
+      (is (= "org.slf4j" (impl/name (impl/find-factory))))))
+  
+  (testing "finds cl"
+    (with-blacklist-loader ["org.slf4j.Logger"]
+      (is (= "org.apache.commons.logging" (impl/name (impl/find-factory))))))
+  
+  (testing "finds log4j2"
+    (with-blacklist-loader ["org.slf4j.Logger"
+                            "org.apache.commons.logging.Log"]
+      (is (= "org.apache.logging.log4j" (impl/name (impl/find-factory))))))
+  
+  (testing "finds log4j"
+    (with-blacklist-loader ["org.slf4j.Logger"
+                            "org.apache.commons.logging.Log"
+                            "org.apache.logging.log4j.Logger"]
+      (is (= "org.apache.log4j" (impl/name (impl/find-factory))))))
+  
+  (testing "finds jul"
+    (with-blacklist-loader ["org.slf4j.Logger"
+                            "org.apache.commons.logging.Log"
+                            "org.apache.logging.log4j.Logger"
+                            "org.apache.log4j.Logger"]
+      (is (= "java.util.logging" (impl/name (impl/find-factory))))))
+  
+  (testing "finds none"
+    (with-blacklist-loader ["org.slf4j.Logger"
+                            "org.apache.commons.logging.Log"
+                            "org.apache.logging.log4j.Logger"
+                            "org.apache.log4j.Logger"
+                            "java.util.logging.Logger"]
+      (is (thrown? RuntimeException (impl/find-factory))))))
+
+
 (let [[factory level] (try
                         [(impl/log4j2-factory) (eval 'org.apache.logging.log4j.Level/FATAL)]
                         (catch UnsupportedClassVersionError _