On Sun, 2004-05-09 at 06:16, matthew.hawthorne wrote:
> robert burrell donkin wrote:
> > funnily enough, if commons logging was to be created again, i'd (with 
> > hindsight) consider something along those lines. the bridging 
> > implementations would be in a separate, optional jar and the two classes 
> > required in the public logging API would be copied over into each 
> > component (as part of the build). if the implementation jar is not 
> > present, error and fatal levels (only) would log to system.err. those 
> > users wanting logging would have to grab and drop in the implementation 
> > jar.

Doh! Can I vote +2 on this :-)

In fact I liked the idea so much, that attached is a proposed
implementation.

I suggest that the attached LogSource class, together with o.a.c.l.Log, 
be copied into every project. Of course all the LogFactory.getLog calls
would need to be changed to LogSource.getLog calls too.

The effect would be that users do not need to download commons-logging
in order to use the libraries. If they want to enable logging, they just
place commons-logging.jar in the classpath and then configure it as
normal. Price: disk space for 1 interface and 1 trivial class, plus a
trivial performance hit in the getLog method if they do enable logging.

As BeanUtils and Digester are winding up for releases, I would love to
get this in if people are happy with the idea. And the nice thing is, we
wouldn't need to wait for a logging release to do this!

The code deliberately discards all logging rather than writing to
stderr; if output is being generated then there must be a configuration
system to enable/disable it, which leads back towards commons-logging
functionality. The point here is to provide something *so* simple that
it *never* needs to change, and hence it is safe to copy the code into
multiple places. By discarding log output, we behave exactly as if
logging had never been used in the first place which I think is
appropriate for an *optional logging* library.

Note that this code doesn't try to do anything clever with classloaders.
Any mucking about with context class loader, etc. is handled by
LogFactory, *not* this class whose responsibility is only to hand off to
commons-logging or do nothing.

The mechanism Robert suggests for the build (auto copy from the logging
jar or dir as part of the build) is nice, but the point is that Log and
LogSource should never need updating, so just committing copies of them
into the CVS repo for each project should also work, and be simpler.

Does copying of classes sound scary? The comments in the LogSource.java
file describe why I think this is not a problem. But here's another
angle on it. If you compile an app with version 1.0.0 of a lib, then try
to run with a version that has a different binary API, what happens?
Boom - and you expect it. And if you run with multiple copies of a class
and they are identical, what happens? Nothing - because they are all the
same. So a problem only occurs when the ABI is the same, but the
implementation is different (eg one lib has a bugfixed version, the
other doesn't). And because LogSource is trivial this shouldn't occur. I
certainly wouldn't recommend copying the complete commons-logging into
each project because that code is not trivial.

Question: what about situations where multiple libraries are running
with different security policies? Might this cause problems when each
lib contains its own copy of o.a.c.l.Log ? Would this be worse than if
they didn't have their own copy?

> 
> Here's my opinion on the best approach to logging for each component:
> 
> Each component creates a simple interface for logging (which could just 
> be a
> copy of o.a.c.l.Log copied into the components namespace, becoming, for 
> example,
> o.a.c.beanutils.Log)
> 
> The component also creates a LogFactory, which checks if commons logging 
> is in
> the classpath.  If it is, it returns an implementation which delegates 
> to commons-logging.
> If it isn't, it just sends messages of the appropriate level to 
> system.err (just like robert suggests).
> 
> This may sound a bit complicated, but it's really not.  To me, this is a 
> good example of how to
> deal with optional dependencies.  In this example, [beanutils] functions 
> fine without
> commons-logging, but adding commons-logging to the classpath enhances 
> the logging capabilities.
> 
> The only problem would be that beanutils would have to invoke the 
> commons-logging methods
> through reflection, since making the calls directly would result in a 
> compile-time dependency
> on commons-logging.  This would slow things down a bit.
> 
> Is this reasonable, or too much work?

Unfortunately, I think the delegation step is just too much overhead.
The log4j docs describe how much effort they went to to make calls fast
in the case where the message is not logged. But commons-logging adds
overhead to every call, and as far as I can see this approach would add
another level of calls, including **reflection** which has significant
overheads. 

The approach I suggest only uses Method.invoke within the getLog method.

This solution is definitely "cleaner", as it avoids having multiple
copies of the Log and LogSource class in multiple libraries. However by
hiding the o.a.c.l.Log interface behing a {component}.Log interface I
believe this forces indirect invocation (Method.invoke) on every
debug/info/warn/error call (filtered or not) which seems unacceptable to
me. If I've misunderstood you, please set me straight..

Maybe a bytecode generation implementation of this idea would work, ie
have a factory that generates custom classes to bridge between the local
component's Log interface and the commons-logging Log interface. But
that's a serious project!

I seem to remember sun introducing a class recently into the std
libraries which uses bytecode generation to create classes on the fly; I
think it was intended for use in swing apps to avoid too many anonymous
inner class declarations. Anyone remember what it was? [I'm not thinking
of java.lang.reflect.Proxy].





I look forward to comments....

Cheers,

Simon
/* $Id$
 *
 * Copyright 2001-2004 The Apache Software Foundation.
 * 
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 * 
 *      http://www.apache.org/licenses/LICENSE-2.0
 * 
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */ 
package org.apache.commons.logging;

import org.apache.commons.logging.Log;
import java.lang.reflect.Method;

/**
 * <p>Factory for Log objects, which uses commons-logging to generate them
 * if commons-logging is available. If commons-logging is not present in the
 * classpath, then a Log object is returned which just discards all input.
 * <p>
 * Libraries and applications which want to have an <i>optional</i> dependency
 * on commons-logging should distribute this class and the class
 * org.apache.commons.logging.Log with the library or application, and should
 * use the getLog method on this class rather than the equivalent methods on
 * the org.apache.commons.logging.LogFactory class. When used in environments
 * where commons-logging is not present, all logging is simply ignored. When
 * the commons-logging library is present, logging occurs as normal.
 * <p>
 * Applications which are willing to have a dependency on the commons-logging
 * library should use LogFactory.getLog instead, as there is a <i>small</i>
 * performance penalty for using this class.
 * <p>
 * Note that it is not normally good practice to copy classes, as having
 * multiple implementations of a class which have binary-compatible APIs but
 * different behaviours (including bugs) can lead to confusing behaviour.
 * However this does not apply to interfaces, which have no behaviours, and
 * this class has been designed to be extremely simple and stable. Changes
 * are not expected to occur except at major library revisions.
 */

public class LogSource {
    private static Class factory = null;
    private static Method method = null;
    private static NullLog nullLog = new NullLog();
    
    static {
        do {
            try {
                factory = Class.forName("org.apache.commons.logging.LogFactory");
            }
            catch(ClassNotFoundException e) {
                break;
            }
            
            try {
                method = factory.getMethod("getLog", new Class[] {String.class});
            } catch(NoSuchMethodException e) {
                break;
            }
            
            try {
                method.invoke(null, new String[] {""});
            } catch(Exception e) {
                System.err.println(
                    "Unable to initialize commons logging:" + e.getMessage());
            }
        } while(false);
    }
    
    public static Log getLog(String category) {
        if (method == null) {
            return nullLog;
        } else {
            try {
                return (Log) method.invoke(null, new String[] {category});
            } catch(Exception e) {
                // if there is something badly wrong with commons-logging,
                // then it should have been reported once at class load, so
                // don't complain here
                return nullLog;
            }
        }
    }
    
    public static Log getLog(Class clazz) {
        return getLog(clazz.getName());
    }

    private static class NullLog implements Log {
        public NullLog() { }
        public NullLog(String name) { }
        public void trace(Object message) { }
        public void trace(Object message, Throwable t) { }
        public void debug(Object message) { }
        public void debug(Object message, Throwable t) { }
        public void info(Object message) { }
        public void info(Object message, Throwable t) { }
        public void warn(Object message) { }
        public void warn(Object message, Throwable t) { }
        public void error(Object message) { }
        public void error(Object message, Throwable t) { }
        public void fatal(Object message) { }
        public void fatal(Object message, Throwable t) { }
    
        public final boolean isDebugEnabled() { return false; }
        public final boolean isErrorEnabled() { return false; }
        public final boolean isFatalEnabled() { return false; }
        public final boolean isInfoEnabled() { return false; }
        public final boolean isTraceEnabled() { return false; }
        public final boolean isWarnEnabled() { return false; }
    }
}

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to