SONAR-4675 remove ability to customize logback-access.xml

6 files changed, 37 insertions, 34 deletions

diff --git a/sonar-application/src/main/assembly/conf/sonar.properties b/sonar-application/src/main/assembly/conf/sonar.properties
index d32af10ba48..66afe8c0e93 100644
--- a/sonar-application/src/main/assembly/conf/sonar.properties
+++ b/sonar-application/src/main/assembly/conf/sonar.properties
@@ -150,6 +150,10 @@ sonar.jdbc.timeBetweenEvictionRunsMillis=30000
 #sonar.web.ajp.acceptCount=25
 #sonar.web.https.acceptCount=25
 
+# Access logs are generated in the file logs/access.log. This file is rolled over when it's 5Mb.
+# An archive of 3 files is kept in the same directory.
+# Access logs are enabled by default.
+#sonar.web.accessLogs.enable=true
 
 
 #--------------------------------------------------------------------------------------------------
diff --git a/sonar-application/src/main/java/org/sonar/application/EmbeddedTomcat.java b/sonar-application/src/main/java/org/sonar/application/EmbeddedTomcat.java
index b92783982d3..f50397a0a16 100644
--- a/sonar-application/src/main/java/org/sonar/application/EmbeddedTomcat.java
+++ b/sonar-application/src/main/java/org/sonar/application/EmbeddedTomcat.java
@@ -19,9 +19,7 @@
  */
 package org.sonar.application;
 
-import org.apache.catalina.LifecycleEvent;
 import org.apache.catalina.LifecycleException;
-import org.apache.catalina.LifecycleListener;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.commons.io.FileUtils;
@@ -67,7 +65,7 @@ class EmbeddedTomcat {
     tomcat.getHost().setDeployOnStartup(true);
 
     Props props = Props.create(env);
-    Logging.configure(tomcat, env);
+    Logging.configure(tomcat, env, props);
     Connectors.configure(tomcat, props);
     Webapp.configure(tomcat, env, props);
     tomcat.start();
diff --git a/sonar-application/src/main/java/org/sonar/application/Logging.java b/sonar-application/src/main/java/org/sonar/application/Logging.java
index ae5a5a4d6b3..cde5d970574 100644
--- a/sonar-application/src/main/java/org/sonar/application/Logging.java
+++ b/sonar-application/src/main/java/org/sonar/application/Logging.java
@@ -27,12 +27,12 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.bridge.SLF4JBridgeHandler;
 
-import java.io.File;
 import java.util.logging.LogManager;
 
 class Logging {
 
-  static final String CONF_PATH = "conf/logback-access.xml";
+  static final String ACCESS_RELATIVE_PATH = "web/WEB-INF/config/logback-access.xml";
+  static final String PROPERTY_ENABLE_ACCESS_LOGS = "sonar.web.accessLogs.enable";
 
   static void init() {
     // Configure java.util.logging, used by Tomcat, in order to forward to slf4j
@@ -40,18 +40,20 @@ class Logging {
     SLF4JBridgeHandler.install();
   }
 
-  static void configure(Tomcat tomcat, Env env) {
+  static void configure(Tomcat tomcat, Env env, Props props) {
     tomcat.setSilent(false);
     tomcat.getService().addLifecycleListener(new StartupLogger(LoggerFactory.getLogger(Logging.class)));
 
-    LogbackValve valve = new LogbackValve();
-    valve.setQuiet(true);
-    File confFile = env.file(CONF_PATH);
-    if (!confFile.exists()) {
-      throw new IllegalStateException("File is missing: " + confFile.getAbsolutePath());
+    configureLogbackAccess(tomcat, env, props);
+  }
+
+  private static void configureLogbackAccess(Tomcat tomcat, Env env, Props props) {
+    if (props.booleanOf(PROPERTY_ENABLE_ACCESS_LOGS, true)) {
+      LogbackValve valve = new LogbackValve();
+      valve.setQuiet(true);
+      valve.setFilename(env.file(ACCESS_RELATIVE_PATH).getAbsolutePath());
+      tomcat.getHost().getPipeline().addValve(valve);
     }
-    valve.setFilename(confFile.getAbsolutePath());
-    tomcat.getHost().getPipeline().addValve(valve);
   }
 
   static class StartupLogger implements LifecycleListener {
diff --git a/sonar-application/src/main/assembly/conf/logback-access.xml b/sonar-application/src/test/fake-app/web/WEB-INF/config/logback-access.xml
index e403ff47162..3d79231a7e3 100644
--- a/sonar-application/src/main/assembly/conf/logback-access.xml
+++ b/sonar-application/src/test/fake-app/web/WEB-INF/config/logback-access.xml
@@ -4,7 +4,7 @@
 
   Log HTTP requests in the file logs/access.log
 
-  See http://logback.qos.ch/access.html#configuration
+  See http://logback.qos.ch/access.html#configuration and Tomcat configuration in sonar-application.
 
 -->
 
diff --git a/sonar-application/src/test/java/org/sonar/application/EnvTest.java b/sonar-application/src/test/java/org/sonar/application/EnvTest.java
index 0aab59fa26e..27749840297 100644
--- a/sonar-application/src/test/java/org/sonar/application/EnvTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/EnvTest.java
@@ -34,6 +34,12 @@ public class EnvTest {
   public TemporaryFolder temp = new TemporaryFolder();
 
   @Test
+  public void testName() throws Exception {
+    File file = new File(getClass().getResource("/org/sonar/application/LoggingTest/logback-access.xml").toURI());
+    assertThat(file.exists()).isTrue();
+  }
+
+  @Test
   public void files() throws Exception {
     File home = temp.newFolder();
     File confFile = new File(home, "conf/sonar.properties");
diff --git a/sonar-application/src/test/java/org/sonar/application/LoggingTest.java b/sonar-application/src/test/java/org/sonar/application/LoggingTest.java
index 39808394b88..35e10890e55 100644
--- a/sonar-application/src/test/java/org/sonar/application/LoggingTest.java
+++ b/sonar-application/src/test/java/org/sonar/application/LoggingTest.java
@@ -24,12 +24,15 @@ import org.apache.catalina.Lifecycle;
 import org.apache.catalina.LifecycleEvent;
 import org.apache.catalina.Valve;
 import org.apache.catalina.startup.Tomcat;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.mockito.ArgumentMatcher;
 import org.mockito.Mockito;
 import org.slf4j.Logger;
 
 import java.io.File;
+import java.util.Properties;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
@@ -37,39 +40,29 @@ import static org.mockito.Matchers.argThat;
 import static org.mockito.Mockito.*;
 
 public class LoggingTest {
+
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+
   @Test
-  public void configure_access_logs() throws Exception {
+  public void enable_access_logs_by_Default() throws Exception {
     Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS);
+    Props props = new Props(new Properties());
     Env env = mock(Env.class);
-    final File propsFile = new File(getClass().getResource("/org/sonar/application/LoggingTest/logback-access.xml").toURI());
-    when(env.file(Logging.CONF_PATH)).thenReturn(propsFile);
-    Logging.configure(tomcat, env);
+    when(env.file(Logging.ACCESS_RELATIVE_PATH)).thenReturn(temp.newFile("logback-access.xml"));
+    Logging.configure(tomcat, env, props);
 
     verify(tomcat.getHost().getPipeline()).addValve(argThat(new ArgumentMatcher<Valve>() {
       @Override
       public boolean matches(Object o) {
         LogbackValve v = (LogbackValve) o;
-        return v.getFilename().equals(propsFile.getAbsolutePath());
+        String confFile = v.getFilename();
+        return confFile.endsWith("logback-access.xml");
       }
     }));
   }
 
   @Test
-  public void fail_if_missing_conf_file() throws Exception {
-    Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS);
-    Env env = mock(Env.class);
-    final File confFile = new File("target/does_not_exist/logback-access.xml");
-    when(env.file(Logging.CONF_PATH)).thenReturn(confFile);
-
-    try {
-      Logging.configure(tomcat, env);
-      fail();
-    } catch (IllegalStateException e) {
-      assertThat(e).hasMessage("File is missing: " + confFile.getAbsolutePath());
-    }
-  }
-
-  @Test
   public void log_when_started() {
     Logger logger = mock(Logger.class);
     Logging.StartupLogger listener = new Logging.StartupLogger(logger);