- 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

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 $
+ * $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.13 $
+ * $Revision: 1.14 $
- * $Date: 2002/10/17 23:00:04 $
+ * $Date: 2002/10/19 17:25:18 $
  *
  * ====================================================================
  *
@@ -86,7 +86,8 @@ import java.util.Properties;
  *
  * @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 {
+        if (factory == null  &&  props != null) {
-            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) {
-                    if (factoryClass == null) {
-                        factoryClass = FACTORY_DEFAULT;
-                    }
                 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,7 +366,6 @@ public abstract class LogFactory {
              * Always cache using context class loader..
              */
             cacheFactory(contextClassLoader, factory);
-        }
 
             if( props!=null ) {
                 Enumeration names = props.propertyNames();
@@ -370,6 +375,7 @@ public abstract class LogFactory {
                     factory.setAttribute(name, value);
                 }
             }
+        }
         
         return factory;
     }
@@ -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
                 }
             }
 

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 $
+ * $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.16 $
+ * $Revision: 1.17 $
- * $Date: 2002/09/27 02:16:44 $
+ * $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)
+    public Log getInstance(Class clazz) throws LogConfigurationException {
-        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)
+    public Log getInstance(String name) throws LogConfigurationException {
-        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);
     }
 
@@ -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 = getLogClassName();
-        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;
-        }
 
         // 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,8 +493,9 @@ public class LogFactoryImpl extends LogFactory {
         throw (ClassNotFoundException)result;
     }
 
+
     protected void guessConfig() {
-        if( isLog4JAvailable() ) {
+        if (getLogClassName() == LOG4JLOGIMPL) {
             proxyFactory = null;
             try {
                 Class proxyClass=
@@ -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)
+    protected Log newInstance(String name) throws LogConfigurationException {
-        throws LogConfigurationException {
-
         Log instance = null;
 
         try {
@@ -554,6 +566,5 @@ public class LogFactoryImpl extends LogFactory {
         } catch (Throwable t) {
             throw new LogConfigurationException(t);
         }
-
     }
 }