Hi,
Robert: thanks for committing the plugins Exception patch.
Here is a patch to convert the Plugins module logging to always use the
Log object returned by digester.getLogger().
I still think that the way Digester centralizes logging is flawed.
However I have more interest in getting plugins finished than debating
logging, so this patch should resolve any arguments for the moment.
Maybe sometime later I will think about the logging issue and put
forward a more concrete proposal. At least now I think I understand the
requirements that led to the current implementation.
As the governor of California would say, "I'll be back".. :-)
Regards,
Simon
PS: I hope I've got the right licence header on new file LogUtils.java
this time!
/*
* $Header: $
* $Revision: $
* $Date: $
*
* ====================================================================
*
* The Apache Software License, Version 1.1
*
* Copyright (c) 2001-2003 The Apache Software Foundation. All rights
* reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in
* the documentation and/or other materials provided with the
* distribution.
*
* 3. The end-user documentation included with the redistribution,
* if any, must include the following acknowledgement:
* "This product includes software developed by the
* Apache Software Foundation (http://www.apache.org/)."
* Alternately, this acknowledgement may appear in the software itself,
* if and wherever such third-party acknowledgements normally appear.
*
* 4. The names "Apache", "The Jakarta Project", "Commons", and "Apache Software
* Foundation" must not be used to endorse or promote products derived
* from this software without prior written permission. For written
* permission, please contact [EMAIL PROTECTED]
*
* 5. Products derived from this software may not be called "Apache",
* "Apache" nor may "Apache" appear in their names without prior
* written permission of the Apache Software Foundation.
*
* THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
* ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
* ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
* OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
* ====================================================================
*
* This software consists of voluntary contributions made by many
* individuals on behalf of the Apache Software Foundation. For more
* information on the Apache Software Foundation, please see
* <http://www.apache.org/>.
*
*/
package org.apache.commons.digester.plugins;
import org.apache.commons.digester.Digester;
import org.apache.commons.logging.Log;
/**
* Simple utility class to assist in logging.
* <p>
* The Digester module has an interesting approach to logging:
* all logging should be done via the Log object stored on the
* digester instance that the object *doing* the logging is associated
* with.
* <p>
* This is done because apparently some "container"-type applications
* such as Avalon and Tomcat need to be able to configure different logging
* for different <i>instances</i> of the Digester class which have been
* loaded from the same ClassLoader [info from Craig McClanahan].
* Not only the logging of the Digester instance should be affected; all
* objects associated with that Digester instance should obey the
* reconfiguration of their owning Digester instance's logging. The current
* solution is to force all objects to output logging info via a single
* Log object stored on the Digester instance they are associated with.
* <p>
* Of course this causes problems if logging is attempted before an
* object <i>has</i> a valid reference to its owning Digester. The
* getLogging method provided here resolves this issue by returning a
* Log object which silently discards all logging output in this
* situation.
* <p>
* And it also implies that logging filtering can no longer be applied
* to subcomponents of the Digester, because all logging is done via
* a single Log object (a single Category). C'est la vie...
*
* @author Simon Kitching
*/
public class LogUtils {
/**
* Get the Log object associated with the specified Digester instance,
* or a "no-op" logging object if the digester reference is null.
* <p>
* You should use this method instead of digester.getLogger() in
* any situation where the digester might be null.
*/
public static Log getLogger(Digester digester) {
if (digester == null) {
return new org.apache.commons.logging.impl.NoOpLog();
}
return digester.getLogger();
}
}
? LogUtils.java
Index: Declaration.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/Declaration.java,v
retrieving revision 1.4
diff -u -r1.4 Declaration.java
--- Declaration.java 27 Oct 2003 13:37:35 -0000 1.4
+++ Declaration.java 27 Oct 2003 22:14:01 -0000
@@ -69,7 +69,6 @@
import org.apache.commons.beanutils.MethodUtils;
import org.apache.commons.digester.Digester;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
/**
* Simple structure to store the set of attributes that can be present on
@@ -78,7 +77,6 @@
* @author Simon Kitching
*/
public class Declaration {
- private static Log log = LogFactory.getLog(Declaration.class);
/**
* The name of the method looked for on the plugin class and any
@@ -225,7 +223,11 @@
*/
public void init(Digester digester)
throws PluginWrappedException {
- log.debug("init being called!");
+ Log log = digester.getLogger();
+ boolean debug = log.isDebugEnabled();
+ if (debug) {
+ log.debug("init being called!");
+ }
if (initialised_) {
throw new PluginAssertionError("Init called multiple times.");
@@ -292,7 +294,11 @@
public void configure(Digester digester, String pattern)
throws PluginWrappedException {
- log.debug("configure being called!");
+ Log log = digester.getLogger();
+ boolean debug = log.isDebugEnabled();
+ if (debug) {
+ log.debug("configure being called!");
+ }
if (!initialised_) {
throw new PluginAssertionError("Not initialised.");
@@ -350,7 +356,7 @@
// look for rule class
{
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("plugin class type:" + pluginClass_.getName());
}
String ruleClassName = pluginClass_.getName() + "RuleInfo";
@@ -385,7 +391,7 @@
// try autoSetProperties
if (autoSetProperties_) {
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("adding autoset for pattern [" + pattern + "]");
}
digester.addSetProperties(pattern);
@@ -410,6 +416,7 @@
is.close();
}
catch(IOException ioe) {
+ Log log = digester.getLogger();
log.warn("Unable to close stream after reading rules", ioe);
}
}
Index: PluginCreateRule.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginCreateRule.java,v
retrieving revision 1.4
diff -u -r1.4 PluginCreateRule.java
--- PluginCreateRule.java 27 Oct 2003 13:37:35 -0000 1.4
+++ PluginCreateRule.java 27 Oct 2003 22:14:02 -0000
@@ -70,7 +70,6 @@
import org.apache.commons.digester.Rule;
import org.apache.commons.digester.Rules;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
/**
* Allows the original rules for parsing the configuration file to define
@@ -81,8 +80,6 @@
*/
public class PluginCreateRule extends Rule implements InitializableRule {
- private static Log log = LogFactory.getLog(PluginCreateRule.class);
-
private static final String PLUGIN_CLASS_ATTR = "plugin-class";
private static final String PLUGIN_ID_ATTR = "plugin-id";
@@ -195,8 +192,12 @@
*/
public void postRegisterInit(String pattern)
throws PluginConfigurationException {
- log.debug("PluginCreateRule.postRegisterInit" +
- ": rule registered for pattern [" + pattern + "]");
+ Log log = LogUtils.getLogger(digester);
+ boolean debug = log.isDebugEnabled();
+ if (debug) {
+ log.debug("PluginCreateRule.postRegisterInit" +
+ ": rule registered for pattern [" + pattern + "]");
+ }
if (digester == null) {
// We require setDigester to be called before this method.
@@ -300,7 +301,9 @@
String namespace, String name,
org.xml.sax.Attributes attributes)
throws java.lang.Exception {
- if (log.isDebugEnabled()) {
+ Log log = digester.getLogger();
+ boolean debug = log.isDebugEnabled();
+ if (debug) {
log.debug("PluginCreateRule.begin" + ": pattern=[" + pattern_ + "]" +
" match=[" + digester.getMatch() + "]");
}
@@ -325,7 +328,7 @@
PluginManager pluginManager = localRules_.getPluginManager();
Declaration currDeclaration = null;
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("PluginCreateRule.begin: installing new plugin: "
+ "oldrules=" + oldRules.toString()
+ ", localrules=" + localRules_.toString());
@@ -376,7 +379,7 @@
Object instance = pluginClass.newInstance();
getDigester().push(instance);
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug(
"PluginCreateRule.begin" + ": pattern=[" + pattern_ + "]" +
" match=[" + digester.getMatch() + "]" +
@@ -391,7 +394,7 @@
// fire the begin method of all custom rules
Rules oldRules = digester.getRules();
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("PluginCreateRule.begin: firing nested rules: "
+ "oldrules=" + oldRules.toString()
+ ", localrules=" + localRules_.toString());
@@ -402,7 +405,7 @@
delegateBegin(namespace, name, attributes);
digester.setRules(oldRules);
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("PluginCreateRule.begin: restored old rules to "
+ "oldrules=" + oldRules.toString());
}
@@ -491,6 +494,7 @@
throws java.lang.Exception {
// Fire "begin" events for all relevant rules
+ Log log = digester.getLogger();
boolean debug = log.isDebugEnabled();
String match = digester.getMatch();
List rules = digester.getRules().match(namespace, match);
@@ -510,6 +514,7 @@
public void delegateBody(String namespace, String name, String text)
throws Exception {
// Fire "body" events for all relevant rules
+ Log log = digester.getLogger();
boolean debug = log.isDebugEnabled();
String match = digester.getMatch();
List rules = digester.getRules().match(namespace, match);
@@ -529,6 +534,7 @@
public void delegateEnd(String namespace, String name)
throws Exception {
// Fire "end" events for all relevant rules in reverse order
+ Log log = digester.getLogger();
boolean debug = log.isDebugEnabled();
String match = digester.getMatch();
List rules = digester.getRules().match(namespace, match);
Index: PluginDeclarationRule.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginDeclarationRule.java,v
retrieving revision 1.4
diff -u -r1.4 PluginDeclarationRule.java
--- PluginDeclarationRule.java 27 Oct 2003 13:37:35 -0000 1.4
+++ PluginDeclarationRule.java 27 Oct 2003 22:14:02 -0000
@@ -68,7 +68,6 @@
import org.apache.commons.beanutils.MethodUtils;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
/**
* A Digester rule which allows the user to pre-declare a class which is to
@@ -82,7 +81,6 @@
*/
public class PluginDeclarationRule extends Rule {
- private static Log log = LogFactory.getLog(PluginDeclarationRule.class);
//------------------- constructors ---------------------------------------
@@ -114,6 +112,10 @@
String name,
org.xml.sax.Attributes attributes)
throws java.lang.Exception {
+
+ Log log = digester.getLogger();
+ boolean debug = log.isDebugEnabled();
+
String id = attributes.getValue("id");
String pluginClassName = attributes.getValue("class");
String ruleMethodName = attributes.getValue("method");
@@ -122,7 +124,7 @@
String ruleFile = attributes.getValue("file");
String autoSetPropertiesStr = attributes.getValue("setprops");
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug(
"mapping id [" + id + "] -> [" + pluginClassName + "]");
}
@@ -184,7 +186,9 @@
// the same external file at multiple locations within a
// parent document. if the declaration is identical,
// then we just ignore it.
- log.debug("plugin redeclaration is identical: ignoring");
+ if (debug) {
+ log.debug("plugin redeclaration is identical: ignoring");
+ }
return;
} else {
throw new PluginInvalidInputException(
Index: PluginManager.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginManager.java,v
retrieving revision 1.3
diff -u -r1.3 PluginManager.java
--- PluginManager.java 9 Oct 2003 21:09:48 -0000 1.3
+++ PluginManager.java 27 Oct 2003 22:14:02 -0000
@@ -66,7 +66,6 @@
import org.apache.commons.digester.Digester;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
/**
* Coordinates between PluginDeclarationRule and PluginCreateRule objects,
@@ -79,7 +78,6 @@
*/
public class PluginManager {
- private static Log log = LogFactory.getLog(PluginManager.class);
/** Map of classname->Declaration */
private HashMap declarationsByClass_ = new HashMap();
@@ -89,7 +87,7 @@
/** the parent manager to which this one may delegate lookups. */
private PluginManager parent_;
-
+
//------------------- constructors ---------------------------------------
/** Constructor. */
@@ -105,10 +103,17 @@
/**
* Add the declaration to the set of known declarations.
+ * <p>
+ * TODO: somehow get a reference to a Digester object
+ * so that we can really log here. Currently, all
+ * logging is disabled from this method.
*
[EMAIL PROTECTED] decl an object representing a plugin class.
*/
public void addDeclaration(Declaration decl) {
+ Log log = LogUtils.getLogger(null);
+ boolean debug = log.isDebugEnabled();
+
Class pluginClass = decl.getPluginClass();
String id = decl.getId();
@@ -116,7 +121,7 @@
if (id != null) {
declarationsById_.put(id, decl);
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug(
"Indexing plugin-id [" + id + "]"
+ " -> class [" + pluginClass.getName() + "]");
Index: PluginRules.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/digester/src/java/org/apache/commons/digester/plugins/PluginRules.java,v
retrieving revision 1.3
diff -u -r1.3 PluginRules.java
--- PluginRules.java 9 Oct 2003 21:09:48 -0000 1.3
+++ PluginRules.java 27 Oct 2003 22:14:02 -0000
@@ -75,7 +75,6 @@
import org.apache.commons.digester.Rules;
import org.apache.commons.digester.RulesBase;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
/**
* A custom digester Rules manager which must be used as the Rules object
@@ -85,7 +84,6 @@
*/
public class PluginRules implements Rules {
- private static Log log = LogFactory.getLog(PluginRules.class);
/**
* The rules implementation that we are "enhancing" with plugins
@@ -230,10 +228,14 @@
* @param rule Rule instance to be registered
*/
public void add(String pattern, Rule rule) {
- if (log.isDebugEnabled()) {
+ Log log = LogUtils.getLogger(digester_);
+ boolean debug = log.isDebugEnabled();
+
+ if (debug) {
log.debug("add entry" + ": mapping pattern [" + pattern + "]" +
" to rule of type [" + rule.getClass().getName() + "]");
}
+
decoratedRules_.add(pattern, rule);
if (rule instanceof InitializableRule) {
@@ -245,13 +247,15 @@
// initialisable rule to remember that its initialisation
// failed, and to throw the exception when begin is
// called for the first time.
- log.debug("Rule initialisation failed", e);
+ if (debug) {
+ log.debug("Rule initialisation failed", e);
+ }
// throw e; -- alas, can't do this
return;
}
}
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug("add exit" + ": mapped pattern [" + pattern + "]" +
" to rule of type [" + rule.getClass().getName() + "]");
}
@@ -298,16 +302,20 @@
* @param pattern Nesting pattern to be matched
*/
public List match(String namespaceURI, String pattern) {
- if (log.isDebugEnabled()) {
+ Log log = LogUtils.getLogger(digester_);
+ boolean debug = log.isDebugEnabled();
+
+ if (debug) {
log.debug(
"Matching pattern [" + pattern
+ "] on rules object " + this.toString());
}
+
List matches;
if ((currPluginCreateRule_ != null) &&
(pattern.length() > currPluginCreateRule_.getPattern().length())) {
// assert pattern.startsWith(currPluginCreateRule.getPattern())
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug(
"Pattern [" + pattern + "] matching PluginCreateRule "
+ currPluginCreateRule_.toString());
@@ -332,15 +340,20 @@
* endPlugin method is called.
*/
public void beginPlugin(PluginCreateRule pcr) {
+ Log log = LogUtils.getLogger(digester_);
+ boolean debug = log.isDebugEnabled();
+
if (currPluginCreateRule_ != null) {
throw new PluginAssertionError(
"endPlugin called when currPluginCreateRule_ is not null.");
}
- if (log.isDebugEnabled()) {
+
+ if (debug) {
log.debug(
"Entering PluginCreateRule " + pcr.toString()
+ " on rules object " + this.toString());
}
+
currPluginCreateRule_ = pcr;
}
@@ -349,6 +362,9 @@
* See [EMAIL PROTECTED] #beginPlugin}.
*/
public void endPlugin(PluginCreateRule pcr) {
+ Log log = LogUtils.getLogger(digester_);
+ boolean debug = log.isDebugEnabled();
+
if (currPluginCreateRule_ == null) {
throw new PluginAssertionError(
"endPlugin called when currPluginCreateRule_ is null.");
@@ -360,7 +376,7 @@
}
currPluginCreateRule_ = null;
- if (log.isDebugEnabled()) {
+ if (debug) {
log.debug(
"Leaving PluginCreateRule " + pcr.toString()
+ " on rules object " + this.toString());
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
