SLING-5756 - Async appender is not started upon reattach in case of log config change
stop and start appender in detach and attach calls respectively. Copied RetryLoop from Sling Testing Tools
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1764709 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/AppenderTracker.java b/src/main/java/org/apache/sling/commons/log/logback/internal/AppenderTracker.java
index cda97ce..b45f457 100644
--- a/src/main/java/org/apache/sling/commons/log/logback/internal/AppenderTracker.java
+++ b/src/main/java/org/apache/sling/commons/log/logback/internal/AppenderTracker.java
@@ -62,7 +62,6 @@
public Object addingService(final ServiceReference reference) {
final Appender<ILoggingEvent> appender = (Appender<ILoggingEvent>) super.addingService(reference);
appender.setContext(loggerContext);
- appender.start();
final AppenderInfo ai = new AppenderInfo(reference, appender);
appenders.put(reference, ai);
@@ -84,11 +83,6 @@
public void removedService(final ServiceReference reference, final Object service) {
final AppenderInfo ai = appenders.remove(reference);
this.detachAppender(ai);
- if ( ai != null ) {
- ai.appender.stop();
- // context can't be unset
- }
-
super.removedService(reference, service);
}
@@ -103,22 +97,27 @@
logger.detachAppender(ai.appender);
}
+ ai.appender.stop();
}
}
private void attachAppender(final AppenderInfo ai) {
if (ai != null) {
+
+ ai.appender.start();
+
for (final String name : ai.getLoggers()) {
final Logger logger = loggerContext.getLogger(name);
-
- logger.addAppender(ai.appender);
+ if (!logger.isAttached(ai.appender)) {
+ logger.addAppender(ai.appender);
+ }
}
}
}
@Override
public void onResetStart(final LoggerContext context) {
- attachAppenders();
+
}
@Override
diff --git a/src/test/java/org/apache/sling/commons/log/logback/integration/ITAppenderServices.java b/src/test/java/org/apache/sling/commons/log/logback/integration/ITAppenderServices.java
index 2b4f278..7762664 100644
--- a/src/test/java/org/apache/sling/commons/log/logback/integration/ITAppenderServices.java
+++ b/src/test/java/org/apache/sling/commons/log/logback/integration/ITAppenderServices.java
@@ -42,8 +42,11 @@
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.cm.Configuration;
import org.osgi.service.cm.ConfigurationAdmin;
+import org.osgi.service.event.Event;
+import org.osgi.service.event.EventAdmin;
import org.slf4j.LoggerFactory;
+import static org.apache.sling.commons.log.logback.integration.ITConfigFragments.RESET_EVENT_TOPIC;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -62,6 +65,9 @@
private ServiceRegistration sr;
+ @Inject
+ private EventAdmin eventAdmin;
+
static {
// uncomment to enable debugging of this test class
// paxRunnerVmOption = DEBUG_VM_OPTION;
@@ -70,7 +76,10 @@
@Override
protected Option addExtraOptions() {
- return composite(configAdmin(), mavenBundle("commons-io", "commons-io").versionAsInProject());
+ return composite(
+ configAdmin(), mavenBundle("commons-io", "commons-io").versionAsInProject(),
+ mavenBundle("org.apache.felix", "org.apache.felix.eventadmin").versionAsInProject()
+ );
}
@Test
@@ -168,6 +177,32 @@
assertEquals(2, ta.events.size());
}
+ @Test
+ public void appenderRestartPostReset() throws Exception{
+ final TestAppender ta = registerAppender("ROOT");
+ delay();
+
+ assertTrue(ta.isStarted());
+ final int stopCount = ta.stopCount;
+ final int startCount = ta.startCount;
+
+ eventAdmin.sendEvent(new Event(RESET_EVENT_TOPIC, (Dictionary)null));
+
+ new RetryLoop(new RetryLoop.Condition() {
+ @Override
+ public String getDescription() {
+ return "Stopcount not increased";
+ }
+
+ @Override
+ public boolean isTrue() throws Exception {
+ return ta.stopCount > stopCount && ta.startCount > startCount;
+ }
+ }, 10, 100);
+
+ assertTrue(ta.isStarted());
+ }
+
@After
public void unregisterAppender(){
sr.unregister();
@@ -187,6 +222,8 @@
private static class TestAppender extends AppenderBase<ILoggingEvent> {
final List<ILoggingEvent> events = new ArrayList<ILoggingEvent>();
final List<String> msgs = new ArrayList<String>();
+ int stopCount;
+ int startCount;
@Override
protected void append(ILoggingEvent eventObject) {
@@ -203,6 +240,18 @@
events.clear();
msgs.clear();
}
+
+ @Override
+ public void stop() {
+ super.stop();
+ stopCount++;
+ }
+
+ @Override
+ public void start() {
+ super.start();
+ startCount++;
+ }
}
}
diff --git a/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigFragments.java b/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigFragments.java
index e600d3e..58ba484 100644
--- a/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigFragments.java
+++ b/src/test/java/org/apache/sling/commons/log/logback/integration/ITConfigFragments.java
@@ -45,7 +45,7 @@
@RunWith(PaxExam.class)
@ExamReactorStrategy(PerClass.class)
public class ITConfigFragments extends LogTestBase {
- private static final String RESET_EVENT_TOPIC = "org/apache/sling/commons/log/RESET";
+ static final String RESET_EVENT_TOPIC = "org/apache/sling/commons/log/RESET";
@Inject
private BundleContext bundleContext;
diff --git a/src/test/java/org/apache/sling/commons/log/logback/integration/RetryLoop.java b/src/test/java/org/apache/sling/commons/log/logback/integration/RetryLoop.java
new file mode 100644
index 0000000..de5105c
--- /dev/null
+++ b/src/test/java/org/apache/sling/commons/log/logback/integration/RetryLoop.java
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.sling.commons.log.logback.integration;
+
+import static org.junit.Assert.fail;
+
+/** Convenience class for retrying tests
+ * until timeout or success.
+ */
+public class RetryLoop {
+
+ private final long timeout;
+
+ /** Interface for conditions to check, isTrue will be called
+ * repeatedly until success or timeout */
+ public interface Condition {
+ /** Used in failure messages to describe what was expected */
+ String getDescription();
+
+ /** If true we stop retrying. The RetryLoop retries on AssertionError,
+ * so if tests fail in this method they are not reported as
+ * failures but retried.
+ */
+ boolean isTrue() throws Exception;
+ }
+
+ /** Retry Condition c until it returns true or timeout. See {@link Condition}
+ * for isTrue semantics.
+ */
+ public RetryLoop(Condition c, int timeoutSeconds, int intervalBetweenTriesMsec) {
+ timeout = System.currentTimeMillis() + timeoutSeconds * 1000L;
+ while(System.currentTimeMillis() < timeout) {
+ try {
+ if(c.isTrue()) {
+ return;
+ }
+ } catch(AssertionError ae) {
+ // Retry JUnit tests failing in the condition as well
+ reportException(ae);
+ } catch(Exception e) {
+ reportException(e);
+ }
+
+ try {
+ Thread.sleep(intervalBetweenTriesMsec);
+ } catch(InterruptedException ignore) {
+ }
+ }
+
+ onTimeout();
+ fail("RetryLoop failed, condition is false after " + timeoutSeconds + " seconds: "
+ + c.getDescription());
+ }
+
+ /** Can be overridden to report Exceptions that happen in the retry loop */
+ protected void reportException(Throwable t) {
+ }
+
+ /** Called if the loop times out without success, just before failing */
+ protected void onTimeout() {
+ }
+
+ protected long getRemainingTimeSeconds() {
+ return Math.max(0L, (timeout - System.currentTimeMillis()) / 1000L);
+ }
+}
\ No newline at end of file
