From 3a5b034cef045f6201cf3c1f786db8985d9c2d47 Mon Sep 17 00:00:00 2001
From: "Richard A. Sitze" <rsitze@apache.org>
Date: Sat, 19 Oct 2002 17:25:18 +0000
Subject: [PATCH] - code cleanup, refactoring, and corrected a few undiscoved
 bugs.. - Bugzilla 13157 - Log4j takes undue precedence over Log override.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk@138927 13f79535-47bb-0310-9956-ffa450edef68
---
 .../apache/commons/logging/LogFactory.java    |  83 ++++++-----
 .../commons/logging/impl/LogFactoryImpl.java  | 141 ++++++++++--------
 2 files changed, 123 insertions(+), 101 deletions(-)

diff --git a/src/java/org/apache/commons/logging/LogFactory.java b/src/java/org/apache/commons/logging/LogFactory.java
index ee2b9d0..5a81469 100644
--- a/src/java/org/apache/commons/logging/LogFactory.java
+++ b/src/java/org/apache/commons/logging/LogFactory.java
@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//logging/src/java/org/apache/commons/logging/LogFactory.java,v 1.13 2002/10/17 23:00:04 rsitze Exp $
- * $Revision: 1.13 $
- * $Date: 2002/10/17 23:00:04 $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//logging/src/java/org/apache/commons/logging/LogFactory.java,v 1.14 2002/10/19 17:25:18 rsitze Exp $
+ * $Revision: 1.14 $
+ * $Date: 2002/10/19 17:25:18 $
  *
  * ====================================================================
  *
@@ -79,14 +79,15 @@ import java.util.Properties;
  * <p>Factory for creating {@link Log} instances, with discovery and
  * configuration features similar to that employed by standard Java APIs
  * such as JAXP.</p>
- *
+ * 
  * <p><strong>IMPLEMENTATION NOTE</strong> - This implementation is heavily
  * based on the SAXParserFactory and DocumentBuilderFactory implementations
  * (corresponding to the JAXP pluggability APIs) found in Apache Xerces.</p>
  *
  * @author Craig R. McClanahan
  * @author Costin Manolache
- * @version $Revision: 1.13 $ $Date: 2002/10/17 23:00:04 $
+ * @author Richard A. Sitze
+ * @version $Revision: 1.14 $ $Date: 2002/10/19 17:25:18 $
  */
 
 public abstract class LogFactory {
@@ -102,7 +103,6 @@ public abstract class LogFactory {
     public static final String FACTORY_PROPERTY =
         "org.apache.commons.logging.LogFactory";
 
-
     /**
      * The fully qualified class name of the fallback <code>LogFactory</code>
      * implementation class to use, if no other can be found.
@@ -110,7 +110,6 @@ public abstract class LogFactory {
     public static final String FACTORY_DEFAULT =
         "org.apache.commons.logging.impl.LogFactoryImpl";
 
-
     /**
      * The name of the properties file to search for.
      */
@@ -239,9 +238,10 @@ public abstract class LogFactory {
      * <ul>
      * <li>The <code>org.apache.commons.logging.LogFactory</code> system
      *     property.</li>
+     * <li>The JDK 1.3 Service Discovery mechanism</li>
      * <li>Use the properties file <code>commons-logging.properties</code>
      *     file, if found in the class path of this class.  The configuration
-     *     file is in standard <code>java.util.Propertis</code> format and
+     *     file is in standard <code>java.util.Properties</code> format and
      *     contains the fully qualified name of the implementation class
      *     with the key being the system property defined above.</li>
      * <li>Fall back to a default implementation class
@@ -272,6 +272,25 @@ public abstract class LogFactory {
         if (factory != null)
             return factory;
 
+
+        // Load properties file..
+        // will be used one way or another in the end.
+
+        Properties props=null;
+        try {
+            InputStream stream = (contextClassLoader == null
+                                 ? ClassLoader.getSystemResourceAsStream( FACTORY_PROPERTIES )
+                                 : contextClassLoader.getResourceAsStream( FACTORY_PROPERTIES ));
+            if (stream != null) {
+                props = new Properties();
+                props.load(stream);
+                stream.close();
+            }
+        } catch (IOException e) {
+        } catch (SecurityException e) {
+        }
+
+        
         // First, try the system property
         try {
             String factoryClass = System.getProperty(FACTORY_PROPERTY);
@@ -282,6 +301,7 @@ public abstract class LogFactory {
             ;  // ignore
         }
 
+
         // Second, try to find a service by using the JDK1.3 jar
         // discovery mechanism. This will allow users to plug a logger
         // by just placing it in the lib/ directory of the webapp ( or in
@@ -319,8 +339,6 @@ public abstract class LogFactory {
         }
 
 
-        Properties props=null;
-        
         // Third try a properties file. 
         // If the properties file exists, it'll be read and the properties
         // used. IMHO ( costin ) System property and JDK1.3 jar service
@@ -329,28 +347,16 @@ public abstract class LogFactory {
         // the webapp, even if a default logger is set at JVM level by a
         // system property )
 
-        try {
-            InputStream stream = (contextClassLoader == null
-                                 ? ClassLoader.getSystemResourceAsStream( FACTORY_PROPERTIES )
-                                 : contextClassLoader.getResourceAsStream( FACTORY_PROPERTIES ));
-            if (stream != null) {
-                props = new Properties();
-                props.load(stream);
-                stream.close();
-                String factoryClass = props.getProperty(FACTORY_PROPERTY);
-                if( factory==null ) {
-                    if (factoryClass == null) {
-                        factoryClass = FACTORY_DEFAULT;
-                    }
-                    factory = newFactory(factoryClass, contextClassLoader);
-                }
+        if (factory == null  &&  props != null) {
+            String factoryClass = props.getProperty(FACTORY_PROPERTY);
+            if (factoryClass != null) {
+                factory = newFactory(factoryClass, contextClassLoader);
             }
-            // the properties will be set at the end.
-        } catch (IOException e) {
-        } catch (SecurityException e) {
         }
 
+
         // Fourth, try the fallback implementation class
+
         if (factory == null) {
             factory = newFactory(FACTORY_DEFAULT, LogFactory.class.getClassLoader());
         }
@@ -360,14 +366,14 @@ public abstract class LogFactory {
              * Always cache using context class loader..
              */
             cacheFactory(contextClassLoader, factory);
-        }
 
-        if( props!=null ) {
-            Enumeration names = props.propertyNames();
-            while (names.hasMoreElements()) {
-                String name = (String) names.nextElement();
-                String value = props.getProperty(name);
-                factory.setAttribute(name, value);
+            if( props!=null ) {
+                Enumeration names = props.propertyNames();
+                while (names.hasMoreElements()) {
+                    String name = (String) names.nextElement();
+                    String value = props.getProperty(name);
+                    factory.setAttribute(name, value);
+                }
             }
         }
         
@@ -534,7 +540,6 @@ public abstract class LogFactory {
     {
         
         try {
-            Class clazz = null;
             if (classLoader != null) {
                 try {
                     // first the given class loader param (thread class loader)
@@ -545,6 +550,12 @@ public abstract class LogFactory {
                         throw ex;
                     }
                     // ignore exception, continue
+                } catch (NoClassDefFoundError e) {
+                    if (classLoader == LogFactory.class.getClassLoader()) {
+                        // Nothing more to try, onwards.
+                        throw ex;
+                    }
+                    // ignore exception, continue
                 }
             }
 
diff --git a/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java b/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
index fd02c17..b98e061 100644
--- a/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
+++ b/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//logging/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java,v 1.16 2002/09/27 02:16:44 rsitze Exp $
- * $Revision: 1.16 $
- * $Date: 2002/09/27 02:16:44 $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//logging/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java,v 1.17 2002/10/19 17:25:04 rsitze Exp $
+ * $Revision: 1.17 $
+ * $Date: 2002/10/19 17:25:04 $
  *
  * ====================================================================
  *
@@ -106,7 +106,7 @@ import org.apache.commons.logging.LogFactory;
  *
  * @author Rod Waldhoff
  * @author Craig R. McClanahan
- * @version $Revision: 1.16 $ $Date: 2002/09/27 02:16:44 $
+ * @version $Revision: 1.17 $ $Date: 2002/10/19 17:25:04 $
  */
 
 public class LogFactoryImpl extends LogFactory {
@@ -152,6 +152,9 @@ public class LogFactoryImpl extends LogFactory {
         "org.apache.commons.logging.log";
 
 
+    private static final String LOG4JLOGIMPL =
+        "org.apache.commons.logging.impl.Log4JCategoryLog".intern();
+
     // ----------------------------------------------------- Instance Variables
 
 
@@ -168,6 +171,11 @@ public class LogFactoryImpl extends LogFactory {
     protected Hashtable instances = new Hashtable();
 
 
+    /**
+     * Name of the class implementing the Log interface.
+     */
+    private String logClassName;
+
     /**
      * The one-argument constructor of the
      * {@link org.apache.commons.logging.Log}
@@ -213,8 +221,8 @@ public class LogFactoryImpl extends LogFactory {
     public Object getAttribute(String name) {
         if( proxyFactory != null )
             return proxyFactory.getAttribute( name );
-        return (attributes.get(name));
 
+        return attributes.get(name);
     }
 
 
@@ -236,8 +244,7 @@ public class LogFactoryImpl extends LogFactory {
         for (int i = 0; i < results.length; i++) {
             results[i] = (String) names.elementAt(i);
         }
-        return (results);
-
+        return results;
     }
 
 
@@ -250,14 +257,11 @@ public class LogFactoryImpl extends LogFactory {
      * @exception LogConfigurationException if a suitable <code>Log</code>
      *  instance cannot be returned
      */
-    public Log getInstance(Class clazz)
-        throws LogConfigurationException
-    {
+    public Log getInstance(Class clazz) throws LogConfigurationException {
         if( proxyFactory != null )
             return proxyFactory.getInstance(clazz);
 
-        return (getInstance(clazz.getName()));
-
+        return getInstance(clazz.getName());
     }
 
 
@@ -278,9 +282,7 @@ public class LogFactoryImpl extends LogFactory {
      * @exception LogConfigurationException if a suitable <code>Log</code>
      *  instance cannot be returned
      */
-    public Log getInstance(String name)
-        throws LogConfigurationException
-    {
+    public Log getInstance(String name) throws LogConfigurationException {
         if( proxyFactory != null )
             return proxyFactory.getInstance(name);
 
@@ -289,8 +291,7 @@ public class LogFactoryImpl extends LogFactory {
             instance = newInstance(name);
             instances.put(name, instance);
         }
-        return (instance);
-
+        return instance;
     }
 
 
@@ -307,7 +308,6 @@ public class LogFactoryImpl extends LogFactory {
             proxyFactory.release();
 
         instances.clear();
-
     }
 
 
@@ -320,7 +320,6 @@ public class LogFactoryImpl extends LogFactory {
     public void removeAttribute(String name) {
         if( proxyFactory != null )
             proxyFactory.removeAttribute(name);
-
         attributes.remove(name);
     }
 
@@ -336,7 +335,7 @@ public class LogFactoryImpl extends LogFactory {
      */
     public void setAttribute(String name, Object value) {
         if( proxyFactory != null )
-            proxyFactory.setAttribute(name,value);
+            proxyFactory.setAttribute(name, value);
 
         if (value == null) {
             attributes.remove(name);
@@ -350,6 +349,52 @@ public class LogFactoryImpl extends LogFactory {
     // ------------------------------------------------------ Protected Methods
 
 
+
+    protected String getLogClassName() {
+        // Identify the Log implementation class we will be using
+        if (logClassName != null) {
+            return logClassName;
+        }
+
+        logClassName = (String) getAttribute(LOG_PROPERTY);
+
+        if (logClassName == null) { // @deprecated
+            logClassName = (String) getAttribute(LOG_PROPERTY_OLD);
+        }
+
+        if (logClassName == null) {
+            try {
+                logClassName = System.getProperty(LOG_PROPERTY);
+            } catch (SecurityException e) {
+                ;
+            }
+        }
+
+        if (logClassName == null) { // @deprecated
+            try {
+                logClassName = System.getProperty(LOG_PROPERTY_OLD);
+            } catch (SecurityException e) {
+                ;
+            }
+        }
+
+        if ((logClassName == null) && isLog4JAvailable()) {
+            logClassName = LOG4JLOGIMPL;
+        }
+
+        if ((logClassName == null) && isJdk14Available()) {
+            logClassName =
+                "org.apache.commons.logging.impl.Jdk14Logger";
+        }
+
+        if (logClassName == null) {
+            logClassName = LOG_DEFAULT;
+        }
+
+        return logClassName;
+    }
+
+
     /**
      * <p>Return the <code>Constructor</code> that can be called to instantiate
      * new {@link org.apache.commons.logging.Log} instances.</p>
@@ -367,42 +412,10 @@ public class LogFactoryImpl extends LogFactory {
 
         // Return the previously identified Constructor (if any)
         if (logConstructor != null) {
-            return (logConstructor);
+            return logConstructor;
         }
 
-        // Identify the Log implementation class we will be using
-        String logClassName = null;
-        if (logClassName == null) {
-            logClassName = (String) getAttribute(LOG_PROPERTY);
-        }
-        if (logClassName == null) { // @deprecated
-            logClassName = (String) getAttribute(LOG_PROPERTY_OLD);
-        }
-        if (logClassName == null) {
-            try {
-                logClassName = System.getProperty(LOG_PROPERTY);
-            } catch (SecurityException e) {
-                ;
-            }
-        }
-        if (logClassName == null) { // @deprecated
-            try {
-                logClassName = System.getProperty(LOG_PROPERTY_OLD);
-            } catch (SecurityException e) {
-                ;
-            }
-        }
-        if ((logClassName == null) && isLog4JAvailable()) {
-            logClassName =
-                "org.apache.commons.logging.impl.Log4JCategoryLog";
-        }
-        if ((logClassName == null) && isJdk14Available()) {
-            logClassName =
-                "org.apache.commons.logging.impl.Jdk14Logger";
-        }
-        if (logClassName == null) {
-            logClassName = LOG_DEFAULT;
-        }
+        String logClassName = getLogClassName();
 
         // Attempt to load the Log implementation class
         Class logClass = null;
@@ -437,13 +450,15 @@ public class LogFactoryImpl extends LogFactory {
                 ("No suitable Log constructor " +
                  logConstructorSignature+ " for " + logClassName, t);
         }
-
     }
 
+
     /**
      * MUST KEEP THIS METHOD PRIVATE
      * 
-     * <p>Exposing this method establishes a security violation.
+     * <p>Exposing this method outside of
+     * <code>org.apache.commons.logging.LogFactoryImpl</code>
+     * will create a security violation:
      * This method uses <code>AccessController.doPrivileged()</code>.
      * </p>
      * 
@@ -478,12 +493,13 @@ public class LogFactoryImpl extends LogFactory {
         throw (ClassNotFoundException)result;
     }
 
+
     protected void guessConfig() {
-        if( isLog4JAvailable() ) {
+        if (getLogClassName() == LOG4JLOGIMPL) {
             proxyFactory = null;
             try {
                 Class proxyClass=
-                    loadClass( "org.apache.commons.logging.impl.Log4jFactory" );
+                    loadClass("org.apache.commons.logging.impl.Log4jFactory");
                 if (proxyClass != null) {
                     proxyFactory = (LogFactory)proxyClass.newInstance();
                 }
@@ -494,7 +510,7 @@ public class LogFactoryImpl extends LogFactory {
         // other logger specific initialization
         // ...
     }
-    
+
 
     /**
      * Is <em>JDK 1.4 or later</em> logging available?
@@ -508,7 +524,6 @@ public class LogFactoryImpl extends LogFactory {
         } catch (Throwable t) {
             return (false);
         }
-
     }
 
 
@@ -524,7 +539,6 @@ public class LogFactoryImpl extends LogFactory {
         } catch (Throwable t) {
             return (false);
         }
-
     }
 
 
@@ -537,9 +551,7 @@ public class LogFactoryImpl extends LogFactory {
      * @exception LogConfigurationException if a new instance cannot
      *  be created
      */
-    protected Log newInstance(String name)
-        throws LogConfigurationException {
-
+    protected Log newInstance(String name) throws LogConfigurationException {
         Log instance = null;
 
         try {
@@ -554,6 +566,5 @@ public class LogFactoryImpl extends LogFactory {
         } catch (Throwable t) {
             throw new LogConfigurationException(t);
         }
-
     }
 }