Author: ceki
Date: Mon Oct 20 20:51:02 2008
New Revision: 1852
Added:
logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/
logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/AppenderAttachableImplTest.java
logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/PackageTest.java
Modified:
logback/trunk/logback-access/src/main/java/ch/qos/logback/access/jetty/RequestLogImpl.java
logback/trunk/logback-access/src/main/java/ch/qos/logback/access/spi/AccessContext.java
logback/trunk/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java
logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/Logger.java
logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/HLogger.java
logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/control/ControlLogger.java
logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachable.java
logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java
logback/trunk/logback-core/src/test/java/ch/qos/logback/core/AllCoreTest.java
Log:
- detachAppender in AppenderAttachable now returns a boolean instead of an Appender.
- iteratorForAppenders() in AppenderAttachable now returns Iterator<Appender<E>> instead of just Iterator
- modified Logger to invoke callAppender without synchronization on this
- AppenderAttachableImpl modified to take advantage of the fact that reads occur much more
often than writes
- associated test cases
Modified: logback/trunk/logback-access/src/main/java/ch/qos/logback/access/jetty/RequestLogImpl.java
==============================================================================
--- logback/trunk/logback-access/src/main/java/ch/qos/logback/access/jetty/RequestLogImpl.java (original)
+++ logback/trunk/logback-access/src/main/java/ch/qos/logback/access/jetty/RequestLogImpl.java Mon Oct 20 20:51:02 2008
@@ -166,7 +166,7 @@
getStatusManager().add(
new InfoStatus("RequestLog added to RequestLogRegistry with name: "
+ getName(), this));
-
+
started = true;
} catch (JoranException e) {
@@ -198,7 +198,7 @@
public boolean isStopping() {
return false;
}
-
+
public boolean isStopped() {
return !started;
}
@@ -211,7 +211,7 @@
aai.addAppender(newAppender);
}
- public Iterator iteratorForAppenders() {
+ public Iterator<Appender<AccessEvent>> iteratorForAppenders() {
return aai.iteratorForAppenders();
}
@@ -232,7 +232,7 @@
return aai.detachAppender(appender);
}
- public Appender<AccessEvent> detachAppender(String name) {
+ public boolean detachAppender(String name) {
return aai.detachAppender(name);
}
Modified: logback/trunk/logback-access/src/main/java/ch/qos/logback/access/spi/AccessContext.java
==============================================================================
--- logback/trunk/logback-access/src/main/java/ch/qos/logback/access/spi/AccessContext.java (original)
+++ logback/trunk/logback-access/src/main/java/ch/qos/logback/access/spi/AccessContext.java Mon Oct 20 20:51:02 2008
@@ -39,7 +39,7 @@
return aai.detachAppender(appender);
}
- public Appender<AccessEvent> detachAppender(String name) {
+ public boolean detachAppender(String name) {
return aai.detachAppender(name);
}
@@ -51,7 +51,7 @@
return aai.isAttached(appender);
}
- public Iterator iteratorForAppenders() {
+ public Iterator<Appender<AccessEvent>> iteratorForAppenders() {
return aai.iteratorForAppenders();
}
Modified: logback/trunk/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java
==============================================================================
--- logback/trunk/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java (original)
+++ logback/trunk/logback-access/src/main/java/ch/qos/logback/access/tomcat/LogbackValve.java Mon Oct 20 20:51:02 2008
@@ -148,7 +148,7 @@
aai.addAppender(newAppender);
}
- public Iterator iteratorForAppenders() {
+ public Iterator<Appender<AccessEvent>> iteratorForAppenders() {
return aai.iteratorForAppenders();
}
@@ -169,7 +169,7 @@
return aai.detachAppender(appender);
}
- public Appender<AccessEvent> detachAppender(String name) {
+ public boolean detachAppender(String name) {
return aai.detachAppender(name);
}
Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/Logger.java
==============================================================================
--- logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/Logger.java (original)
+++ logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/Logger.java Mon Oct 20 20:51:02 2008
@@ -66,6 +66,26 @@
*/
private List<Logger> childrenList;
+ /**
+ * It is assumed that once the 'aai' variable is set to a non-null value, it
+ * will never be reset to null. it is further assumed that only place where
+ * the 'aai'ariable is set is within the addAppender method. This method is
+ * synchronized on 'this' (Logger) protecting against simultaneous
+ * re-configuration of this logger (a very unlikely scenario).
+ *
+ * <p>
+ * It is further assumed that the AppenderAttachableImpl is responsible for
+ * its internal synchronization and thread safety. Thus, we can get away with
+ * *not* synchronizing on the 'aai' (check null/ read) because
+ * <p>
+ * 1) the 'aai' variable is immutable once set to non-null
+ * <p>
+ * 2) 'aai' is getAndSet only within addAppender which is synchronized
+ * <p>
+ * 3) all the other methods check whether 'aai' is null
+ * <p>
+ * 4) AppenderAttachableImpl is thread safe
+ */
private transient AppenderAttachableImpl<LoggingEvent> aai;
/**
* Additivity is set to true by default, that is children inherit the
@@ -193,7 +213,6 @@
if (level == null) {
effectiveLevelInt = newParentLevel.levelInt;
-
// propagate the parent levelInt change to this logger's children
if (childrenList != null) {
int len = childrenList.size();
@@ -209,19 +228,21 @@
* Remove all previously added appenders from this logger instance. <p/> This
* is useful when re-reading configuration information.
*/
- public synchronized void detachAndStopAllAppenders() {
+ public void detachAndStopAllAppenders() {
if (aai != null) {
aai.detachAndStopAllAppenders();
}
}
- public synchronized Appender<LoggingEvent> detachAppender(String name) {
+ public boolean detachAppender(String name) {
if (aai == null) {
- return null;
+ return false;
}
return aai.detachAppender(name);
}
+ // this method MUST be synchronized. See comments on 'aai' field for further
+ // details.
public synchronized void addAppender(Appender<LoggingEvent> newAppender) {
if (aai == null) {
aai = new AppenderAttachableImpl<LoggingEvent>();
@@ -236,7 +257,8 @@
return aai.isAttached(appender);
}
- public synchronized Iterator iteratorForAppenders() {
+ @SuppressWarnings("unchecked")
+ public Iterator<Appender<LoggingEvent>> iteratorForAppenders() {
if (aai == null) {
return Collections.EMPTY_LIST.iterator();
}
@@ -254,22 +276,16 @@
* Invoke all the appenders of this logger.
*
* @param event
- * The event to log
+ * The event to log
*/
public void callAppenders(LoggingEvent event) {
int writes = 0;
- // System.out.println("callAppenders");
for (Logger l = this; l != null; l = l.parent) {
- // System.out.println("l="+l.getName());
- // Protected against simultaneous call to addAppender, removeAppender,...
- synchronized (l) {
- writes += l.appendLoopOnAppenders(event);
- if (!l.additive) {
- break;
- }
+ writes += l.appendLoopOnAppenders(event);
+ if (!l.additive) {
+ break;
}
}
-
// No appenders in hierarchy
if (writes == 0) {
loggerContext.noAppenderDefinedWarning(this);
@@ -277,17 +293,17 @@
}
private int appendLoopOnAppenders(LoggingEvent event) {
- int size = 0;
if (aai != null) {
- size = aai.appendLoopOnAppenders(event);
+ return aai.appendLoopOnAppenders(event);
+ } else {
+ return 0;
}
- return size;
}
/**
* Remove the appender passed as parameter form the list of appenders.
*/
- public synchronized boolean detachAppender(Appender appender) {
+ public boolean detachAppender(Appender appender) {
if (aai == null) {
return false;
}
@@ -304,9 +320,9 @@
* logger.
*
* @param lastPart
- * the suffix (i.e. last part) of the child logger
- * name. This parameter may not include dots, i.e. the
- * logger separator character.
+ * the suffix (i.e. last part) of the child logger name. This
+ * parameter may not include dots, i.e. the logger separator
+ * character.
* @return
*/
Logger createChildByLastNamePart(final String lastPart) {
Modified: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/HLogger.java
==============================================================================
--- logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/HLogger.java (original)
+++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/HLogger.java Mon Oct 20 20:51:02 2008
@@ -25,6 +25,8 @@
public class HLogger extends MarkerIgnoringBase {
+ private static final long serialVersionUID = 1L;
+
static int instanceCount = 0;
/**
Modified: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/control/ControlLogger.java
==============================================================================
--- logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/control/ControlLogger.java (original)
+++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/control/ControlLogger.java Mon Oct 20 20:51:02 2008
@@ -17,6 +17,8 @@
* See javadoc for ControlLoggerContext.
*/
public class ControlLogger extends MarkerIgnoringBase {
+
+ private static final long serialVersionUID = 1L;
final ControlLogger parent;
final String name;
Level level;
Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachable.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachable.java (original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachable.java Mon Oct 20 20:51:02 2008
@@ -27,7 +27,7 @@
/**
* Get an iterator for appenders contained in the parent object.
*/
- public Iterator iteratorForAppenders();
+ public Iterator<Appender<E>> iteratorForAppenders();
/**
* Get an appender by name.
@@ -41,9 +41,9 @@
public boolean isAttached(Appender<E> appender);
/**
- * Detach all previously added appenders.
+ * Detach and stop all previously added appenders.
*/
- void detachAndStopAllAppenders();
+ void detachAndStopAllAppenders();
/**
* Detach the appender passed as parameter from the list of appenders.
@@ -54,5 +54,5 @@
* Detach the appender with the name passed as parameter from the list of
* appenders.
*/
- Appender<E> detachAppender(String name);
+ boolean detachAppender(String name);
}
Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java (original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/spi/AppenderAttachableImpl.java Mon Oct 20 20:51:02 2008
@@ -9,9 +9,8 @@
*/
package ch.qos.logback.core.spi;
-import java.util.ArrayList;
import java.util.Iterator;
-import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
import ch.qos.logback.core.Appender;
@@ -22,20 +21,17 @@
*/
public class AppenderAttachableImpl<E> implements AppenderAttachable<E> {
- final private List<Appender<E>> appenderList = new ArrayList<Appender<E>>();
+ final private CopyOnWriteArrayList<Appender<E>> appenderList = new CopyOnWriteArrayList<Appender<E>>();
/**
* Attach an appender. If the appender is already in the list in won't be
* added again.
*/
public void addAppender(Appender<E> newAppender) {
- // Null values for newAppender parameter are strictly forbidden.
if (newAppender == null) {
- throw new IllegalArgumentException("Cannot null as an appener");
- }
- if (!appenderList.contains(newAppender)) {
- appenderList.add(newAppender);
+ throw new IllegalArgumentException("Null argument disallowed");
}
+ appenderList.addIfAbsent(newAppender);
}
/**
@@ -43,12 +39,9 @@
*/
public int appendLoopOnAppenders(E e) {
int size = 0;
- Appender<E> appender;
-
- size = appenderList.size();
- for (int i = 0; i < size; i++) {
- appender = (Appender<E>) appenderList.get(i);
+ for (Appender<E> appender : appenderList) {
appender.doAppend(e);
+ size++;
}
return size;
}
@@ -57,9 +50,9 @@
* Get all attached appenders as an Enumeration. If there are no attached
* appenders <code>null</code> is returned.
*
- * @return Enumeration An enumeration of attached appenders.
+ * @return Iterator An iterator of attached appenders.
*/
- public Iterator iteratorForAppenders() {
+ public Iterator<Appender<E>> iteratorForAppenders() {
return appenderList.iterator();
}
@@ -74,18 +67,11 @@
if (name == null) {
return null;
}
-
- int size = appenderList.size();
- Appender<E> appender;
-
- for (int i = 0; i < size; i++) {
- appender = appenderList.get(i);
-
+ for (Appender<E> appender : appenderList) {
if (name.equals(appender.getName())) {
return appender;
}
}
-
return null;
}
@@ -99,18 +85,11 @@
if (appender == null) {
return false;
}
-
- int size = appenderList.size();
- Appender a;
-
- for (int i = 0; i < size; i++) {
- a = (Appender) appenderList.get(i);
-
+ for (Appender<E> a : appenderList) {
if (a == appender) {
return true;
}
}
-
return false;
}
@@ -118,13 +97,9 @@
* Remove and stop all previously attached appenders.
*/
public void detachAndStopAllAppenders() {
- int len = appenderList.size();
-
- for (int i = 0; i < len; i++) {
- Appender a = (Appender) appenderList.get(i);
+ for (Appender<E> a : appenderList) {
a.stop();
}
-
appenderList.clear();
}
@@ -143,18 +118,15 @@
* Remove the appender with the name passed as parameter form the list of
* appenders.
*/
- public Appender<E> detachAppender(String name) {
+ public boolean detachAppender(String name) {
if (name == null) {
- return null;
+ return false;
}
-
- int size = appenderList.size();
-
- for (int i = 0; i < size; i++) {
- if (name.equals((appenderList.get(i)).getName())) {
- return appenderList.remove(i);
+ for (Appender<E> a : appenderList) {
+ if (name.equals((a).getName())) {
+ return appenderList.remove(a);
}
}
- return null;
+ return false;
}
}
Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/AllCoreTest.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/AllCoreTest.java (original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/AllCoreTest.java Mon Oct 20 20:51:02 2008
@@ -23,6 +23,7 @@
suite.addTest(ch.qos.logback.core.PackageTest.suite());
suite.addTest(ch.qos.logback.core.joran.PackageTest.suite());
suite.addTest(ch.qos.logback.core.appender.PackageTest.suite());
+ suite.addTest(ch.qos.logback.core.spi.PackageTest.suite());
suite.addTest(ch.qos.logback.core.rolling.PackageTest.suite());
return suite;
}
Added: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/AppenderAttachableImplTest.java
==============================================================================
--- (empty file)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/AppenderAttachableImplTest.java Mon Oct 20 20:51:02 2008
@@ -0,0 +1,177 @@
+/**
+ * Logback: the generic, reliable, fast and flexible logging framework.
+ *
+ * Copyright (C) 2000-2008, QOS.ch
+ *
+ * This library is free software, you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License as published by the Free
+ * Software Foundation.
+ */
+package ch.qos.logback.core.spi;
+
+
+import static org.junit.Assert.*;
+import ch.qos.logback.core.appender.NOPAppender;
+import ch.qos.logback.core.Appender;
+import ch.qos.logback.core.layout.NopLayout;
+
+import java.util.Iterator;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * This test case verifies all the methods of AppenderAttableImpl work properly.
+ *
+ * @author Raplh Goers
+ */
+public class AppenderAttachableImplTest {
+
+
+ private AppenderAttachableImpl<TestEvent> aai;
+
+ @Before
+ public void setUp() throws Exception {
+ aai = new AppenderAttachableImpl<TestEvent>();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ aai = null;
+ }
+
+ @Test
+ public void testAddAppender() throws Exception {
+ TestEvent event = new TestEvent();
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ ta = new NOPAppender<TestEvent>();
+ ta.setName("test");
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ int size = aai.appendLoopOnAppenders(event);
+ assertTrue("Incorrect number of appenders", size == 2);
+ }
+
+ @Test
+ public void testIteratorForAppenders() throws Exception {
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ NOPAppender<TestEvent> tab = new NOPAppender<TestEvent>();
+ tab.setName("test");
+ tab.setLayout(new NopLayout<TestEvent>());
+ tab.start();
+ aai.addAppender(tab);
+ Iterator<Appender<TestEvent>> iter = aai.iteratorForAppenders();
+ int size = 0;
+ while (iter.hasNext()) {
+ ++size;
+ Appender<TestEvent> app = iter.next();
+ assertTrue("Bad Appender", app == ta || app == tab);
+ }
+ assertTrue("Incorrect number of appenders", size == 2);
+ }
+
+ @Test
+ public void getGetAppender() throws Exception {
+ NOPAppender<TestEvent> test = new NOPAppender<TestEvent>();
+ test.setLayout(new NopLayout<TestEvent>());
+ test.setName("test");
+ test.start();
+ aai.addAppender(test);
+
+ NOPAppender<TestEvent> testOther = new NOPAppender<TestEvent>();
+ testOther.setName("testOther");
+ testOther.setLayout(new NopLayout<TestEvent>());
+ testOther.start();
+ aai.addAppender(testOther);
+
+ Appender a = aai.getAppender("testOther");
+ assertNotNull("Could not find appender", a);
+ assertTrue("Wrong appender", a == testOther);
+
+ a = aai.getAppender("test");
+ assertNotNull("Could not find appender", a);
+ assertTrue("Wrong appender", a == test);
+ a = aai.getAppender("NotThere");
+ assertNull("Appender was returned", a);
+ }
+
+ @Test
+ public void testIsAttached() throws Exception {
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ NOPAppender<TestEvent> tab = new NOPAppender<TestEvent>();
+ tab.setName("test");
+ tab.setLayout(new NopLayout<TestEvent>());
+ tab.start();
+ aai.addAppender(tab);
+ assertTrue("Appender is not attached", aai.isAttached(ta));
+ assertTrue("Appender is not attached", aai.isAttached(tab));
+ }
+
+ @Test
+ public void testDetachAndStopAllAppenders() throws Exception {
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ NOPAppender<TestEvent> tab = new NOPAppender<TestEvent>();
+ tab.setName("test");
+ tab.setLayout(new NopLayout<TestEvent>());
+ tab.start();
+ aai.addAppender(tab);
+ assertTrue("Appender was not started", tab.isStarted());
+ aai.detachAndStopAllAppenders();
+ assertNull("Appender was not removed", aai.getAppender("test"));
+ assertFalse("Appender was not stopped", tab.isStarted());
+ }
+
+ @Test
+ public void testDetachAppender() throws Exception {
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ NOPAppender<TestEvent> tab = new NOPAppender<TestEvent>();
+ tab.setName("test");
+ tab.setLayout(new NopLayout<TestEvent>());
+ tab.start();
+ aai.addAppender(tab);
+ assertTrue("Appender not detached", aai.detachAppender(tab));
+ assertNull("Appender was not removed", aai.getAppender("test"));
+ assertFalse("Appender detach error", aai.detachAppender(tab));
+ }
+
+ @Test
+ public void testDetachAppenderByName() throws Exception {
+ NOPAppender<TestEvent> ta = new NOPAppender<TestEvent>();
+ ta.setName("test1");
+ ta.setLayout(new NopLayout<TestEvent>());
+ ta.start();
+ aai.addAppender(ta);
+ NOPAppender<TestEvent> tab = new NOPAppender<TestEvent>();
+ tab.setName("test");
+ tab.setLayout(new NopLayout<TestEvent>());
+ tab.start();
+ aai.addAppender(tab);
+
+ assertTrue(aai.detachAppender("test"));
+ assertTrue(aai.detachAppender("test1"));
+ assertFalse( aai.detachAppender("test1"));
+ }
+
+ private static class TestEvent {
+
+ }
+
+}
+
Added: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/PackageTest.java
==============================================================================
--- (empty file)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/spi/PackageTest.java Mon Oct 20 20:51:02 2008
@@ -0,0 +1,24 @@
+/**
+ * Logback: the generic, reliable, fast and flexible logging framework.
+ *
+ * Copyright (C) 2000-2008, QOS.ch
+ *
+ * This library is free software, you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser General Public License as published by the Free
+ * Software Foundation.
+ */
+package ch.qos.logback.core.spi;
+
+import junit.framework.JUnit4TestAdapter;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+
+public class PackageTest {
+ public static Test suite() {
+ TestSuite suite = new TestSuite();
+ suite.addTest(new JUnit4TestAdapter(AppenderAttachableImplTest.class));
+ return suite;
+ }
+
+}
