SONAR-3612 Custom servlet filters are not loaded after database upgrade

10 files changed, 254 insertions, 45 deletions

diff --git a/pom.xml b/pom.xml
index 746f1cd7ae1..0204dc7eb03 100644
--- a/pom.xml
+++ b/pom.xml
@@ -71,7 +71,7 @@
   </prerequisites>
 
   <properties>
-    <sonarJava.version>1.1</sonarJava.version>
+    <sonarJava.version>1.2-M2</sonarJava.version>
     <sonarGwt.version>3.3.1</sonarGwt.version>
     <h2.version>1.3.167</h2.version>
     <jetty.version>6.1.25</jetty.version>
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
index a7a486dc9ef..a7aa641167e 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.api.web;
 
-import com.google.common.annotations.Beta;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import org.sonar.api.ServerExtension;
@@ -29,7 +28,6 @@ import javax.servlet.Filter;
 /**
  * @since 3.1
  */
-@Beta
 public abstract class ServletFilter implements ServerExtension, Filter {
 
   /**
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
index 398cbb6d247..f5e0f999c3e 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
@@ -19,16 +19,30 @@
  */
 package org.sonar.api.web;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+
+import java.io.IOException;
 
 import static org.fest.assertions.Assertions.assertThat;
 
 public class ServletFilterTest {
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   @Test
   public void matchAll() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.create("/*");
     assertThat(pattern.matches("/")).isTrue();
     assertThat(pattern.matches("/foo/ooo")).isTrue();
+    assertThat(pattern.toString()).isEqualTo("/*");
   }
 
   @Test
@@ -58,4 +72,50 @@ public class ServletFilterTest {
     assertThat(pattern.matches("/foo/")).isFalse();
     assertThat(pattern.matches("/bar")).isFalse();
   }
+
+  @Test
+  public void url_pattern_cant_be_empty() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Empty url");
+    ServletFilter.UrlPattern.create("");
+  }
+
+  @Test
+  public void filter_should_return_url_pattern() {
+    ServletFilter filter = new FakeFilter();
+    assertThat(filter.doGetPattern().getUrl()).isEqualTo("/fake");
+  }
+
+  @Test
+  public void filter_should_apply_to_all_urls_by_default() {
+    ServletFilter filter = new DefaultFilter();
+    assertThat(filter.doGetPattern().getUrl()).isEqualTo("/*");
+  }
+
+  static class FakeFilter extends ServletFilter {
+    @Override
+    public UrlPattern doGetPattern() {
+      return UrlPattern.create("/fake");
+    }
+
+    public void init(FilterConfig filterConfig) throws ServletException {
+    }
+
+    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
+    }
+
+    public void destroy() {
+    }
+  }
+
+  static class DefaultFilter extends ServletFilter {
+    public void init(FilterConfig filterConfig) throws ServletException {
+    }
+
+    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
+    }
+
+    public void destroy() {
+    }
+  }
 }
diff --git a/sonar-server/src/dev/web.xml b/sonar-server/src/dev/web.xml
index 9fc07c9f20a..fbf1d357710 100644
--- a/sonar-server/src/dev/web.xml
+++ b/sonar-server/src/dev/web.xml
@@ -34,7 +34,7 @@
 
   <filter>
     <filter-name>ServletFilters</filter-name>
-    <filter-class>org.sonar.server.platform.ServletFilters</filter-class>
+    <filter-class>org.sonar.server.platform.MasterServletFilter</filter-class>
   </filter>
   <filter>
     <filter-name>DatabaseSessionFilter</filter-name>
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/ServletFilters.java b/sonar-server/src/main/java/org/sonar/server/platform/MasterServletFilter.java
index 303a1e6c850..56bb22aac1d 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/ServletFilters.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/MasterServletFilter.java
@@ -25,8 +25,14 @@ import com.google.common.collect.Lists;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.web.ServletFilter;
 
-import javax.servlet.*;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
+
 import java.io.IOException;
 import java.util.Iterator;
 import java.util.List;
@@ -34,23 +40,37 @@ import java.util.List;
 /**
  * Inspired by http://stackoverflow.com/a/7592883/229031
  */
-public class ServletFilters implements Filter {
+public class MasterServletFilter implements Filter {
 
+  public static MasterServletFilter INSTANCE;
   private ServletFilter[] filters;
+  private FilterConfig config;
+
+  public MasterServletFilter() {
+    if (INSTANCE != null) {
+      throw new IllegalStateException("Servlet filter " + getClass().getName() + " is already instantiated");
+    }
+    INSTANCE = this;
+  }
 
   public void init(FilterConfig config) throws ServletException {
+    // Filters are already available in picocontainer unless a database migration is required. See org.sonar.server.startup.RegisterServletFilters.
     init(config, Platform.getInstance().getContainer().getComponentsByType(ServletFilter.class));
   }
 
-  @VisibleForTesting
-  void init(FilterConfig config, List<ServletFilter> extensions) throws ServletException {
+  void init(FilterConfig config, List<ServletFilter> filters) throws ServletException {
+    this.config = config;
+    initFilters(filters);
+  }
+
+  public void initFilters(List<ServletFilter> filterExtensions) throws ServletException {
     List<Filter> filterList = Lists.newArrayList();
-    for (ServletFilter extension : extensions) {
+    for (ServletFilter extension : filterExtensions) {
       try {
-        LoggerFactory.getLogger(ServletFilters.class).info(String.format("Initializing servlet filter %s [pattern=%s]", extension, extension.doGetPattern()));
+        LoggerFactory.getLogger(MasterServletFilter.class).info(String.format("Initializing servlet filter %s [pattern=%s]", extension, extension.doGetPattern()));
         extension.init(config);
         filterList.add(extension);
-      } catch (RuntimeException e) {
+      } catch (Exception e) {
         throw new IllegalStateException("Fail to initialize servlet filter: " + extension + ". Message: " + e.getMessage(), e);
       }
     }
diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
index 6bbd8bcca31..d7cf11cc662 100644
--- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
+++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java
@@ -265,6 +265,7 @@ public final class Platform {
     startupContainer.addSingleton(RegisterNewDashboards.class);
     startupContainer.addSingleton(RenameDeprecatedPropertyKeys.class);
     startupContainer.addSingleton(LogServerId.class);
+    startupContainer.addSingleton(RegisterServletFilters.class);
     startupContainer.startComponents();
 
     startupContainer.getComponentByType(ServerLifecycleNotifier.class).notifyStart();
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterServletFilters.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterServletFilters.java
new file mode 100644
index 00000000000..5dcd965a641
--- /dev/null
+++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterServletFilters.java
@@ -0,0 +1,52 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.server.startup;
+
+import org.sonar.api.web.ServletFilter;
+import org.sonar.server.platform.MasterServletFilter;
+
+import javax.servlet.ServletException;
+
+import java.util.Arrays;
+
+/**
+ * @since 3.5
+ */
+public class RegisterServletFilters {
+  private final ServletFilter[] filters;
+
+  public RegisterServletFilters(ServletFilter[] filters) {
+    this.filters = filters;
+  }
+
+  public RegisterServletFilters() {
+    this(new ServletFilter[0]);
+  }
+
+  public void start() throws ServletException {
+    if (MasterServletFilter.INSTANCE != null) {
+      // Probably a database upgrade. MasterSlaveFilter was instantiated by the servlet container
+      // while picocontainer was not completely up.
+      // See https://jira.codehaus.org/browse/SONAR-3612
+      MasterServletFilter.INSTANCE.initFilters(Arrays.asList(filters));
+    }
+  }
+}
+
diff --git a/sonar-server/src/main/webapp/WEB-INF/web.xml b/sonar-server/src/main/webapp/WEB-INF/web.xml
index 4d5b040522e..47c60f5ff48 100644
--- a/sonar-server/src/main/webapp/WEB-INF/web.xml
+++ b/sonar-server/src/main/webapp/WEB-INF/web.xml
@@ -33,7 +33,7 @@
 
   <filter>
     <filter-name>ServletFilters</filter-name>
-    <filter-class>org.sonar.server.platform.ServletFilters</filter-class>
+    <filter-class>org.sonar.server.platform.MasterServletFilter</filter-class>
   </filter>
   <filter>
     <filter-name>DatabaseSessionFilter</filter-name>
diff --git a/sonar-server/src/test/java/org/sonar/server/platform/ServletFiltersTest.java b/sonar-server/src/test/java/org/sonar/server/platform/MasterServletFilterTest.java
index 6070b4891bf..0a329e3bf7e 100644
--- a/sonar-server/src/test/java/org/sonar/server/platform/ServletFiltersTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/platform/MasterServletFilterTest.java
@@ -1,61 +1,84 @@
 /*
- * Sonar, open source software quality management tool.
- * Copyright (C) 2008-2012 SonarSource
- * mailto:contact AT sonarsource DOT com
- *
- * Sonar 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; either
- * version 3 of the License, or (at your option) any later version.
- *
- * Sonar is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with Sonar; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
- */
+* Sonar, open source software quality management tool.
+* Copyright (C) 2008-2012 SonarSource
+* mailto:contact AT sonarsource DOT com
+*
+* Sonar 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; either
+* version 3 of the License, or (at your option) any later version.
+*
+* Sonar is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with Sonar; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+*/
 package org.sonar.server.platform;
 
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.web.ServletFilter;
 
-import javax.servlet.*;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
-public class ServletFiltersTest {
+public class MasterServletFilterTest {
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
+  @Before
+  public void resetSingleton() {
+    MasterServletFilter.INSTANCE = null;
+  }
+
   @Test
-  public void initAndDestroyFilters() throws Exception {
+  public void should_init_and_destroy_filters() throws Exception {
     ServletFilter filter = mock(ServletFilter.class);
     FilterConfig config = mock(FilterConfig.class);
-    ServletFilters filters = new ServletFilters();
-    filters.init(config, Arrays.asList(filter));
+    MasterServletFilter master = new MasterServletFilter();
+    master.init(config, Arrays.asList(filter));
 
-    assertThat(filters.getFilters()).containsOnly(filter);
+    assertThat(master.getFilters()).containsOnly(filter);
     verify(filter).init(config);
 
-    filters.destroy();
+    master.destroy();
     verify(filter).destroy();
   }
 
   @Test
-  public void initFilters_propagate_failure() throws Exception {
+  public void servlet_container_should_instantiate_only_a_single_master_instance() throws Exception {
+    new MasterServletFilter();
+
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Servlet filter org.sonar.server.platform.MasterServletFilter is already instantiated");
+    new MasterServletFilter();
+  }
+
+  @Test
+  public void should_propagate_initialization_failure() throws Exception {
     thrown.expect(IllegalStateException.class);
     thrown.expectMessage("foo");
 
@@ -63,14 +86,14 @@ public class ServletFiltersTest {
     doThrow(new IllegalStateException("foo")).when(filter).init(any(FilterConfig.class));
 
     FilterConfig config = mock(FilterConfig.class);
-    ServletFilters filters = new ServletFilters();
+    MasterServletFilter filters = new MasterServletFilter();
     filters.init(config, Arrays.asList(filter));
   }
 
   @Test
-  public void doFilter_no_filters() throws Exception {
+  public void filters_should_be_optional() throws Exception {
     FilterConfig config = mock(FilterConfig.class);
-    ServletFilters filters = new ServletFilters();
+    MasterServletFilter filters = new MasterServletFilter();
     filters.init(config, Collections.<ServletFilter>emptyList());
 
     ServletRequest request = mock(HttpServletRequest.class);
@@ -82,11 +105,11 @@ public class ServletFiltersTest {
   }
 
   @Test
-  public void doFilter_keep_same_order() throws Exception {
+  public void should_keep_filter_ordering() throws Exception {
     TrueFilter filter1 = new TrueFilter();
     TrueFilter filter2 = new TrueFilter();
 
-    ServletFilters filters = new ServletFilters();
+    MasterServletFilter filters = new MasterServletFilter();
     filters.init(mock(FilterConfig.class), Arrays.<ServletFilter>asList(filter1, filter2));
 
     HttpServletRequest request = mock(HttpServletRequest.class);
@@ -102,7 +125,8 @@ public class ServletFiltersTest {
 
   private static final class TrueFilter extends ServletFilter {
     private static int globalCount = 0;
-    int count = 0;
+    private int count = 0;
+
     public void init(FilterConfig filterConfig) throws ServletException {
     }
 
diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterServletFiltersTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterServletFiltersTest.java
new file mode 100644
index 00000000000..49619df752c
--- /dev/null
+++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterServletFiltersTest.java
@@ -0,0 +1,54 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2012 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar 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; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.server.startup;
+
+import org.junit.Test;
+import org.sonar.api.web.ServletFilter;
+import org.sonar.server.platform.MasterServletFilter;
+
+import javax.servlet.ServletException;
+
+import static org.mockito.Matchers.anyListOf;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+public class RegisterServletFiltersTest {
+  @Test
+  public void should_not_fail_if_master_filter_is_not_up() throws ServletException {
+    MasterServletFilter.INSTANCE = null;
+    new RegisterServletFilters(new ServletFilter[2]).start();
+  }
+
+  @Test
+  public void should_register_filters_if_master_filter_is_up() throws ServletException {
+    MasterServletFilter.INSTANCE = mock(MasterServletFilter.class);
+    new RegisterServletFilters(new ServletFilter[2]).start();
+
+    verify(MasterServletFilter.INSTANCE).initFilters(anyListOf(ServletFilter.class));
+  }
+
+  @Test
+  public void filters_should_be_optional() throws ServletException {
+    MasterServletFilter.INSTANCE = mock(MasterServletFilter.class);
+    new RegisterServletFilters().start();
+    // do not fail
+    verify(MasterServletFilter.INSTANCE).initFilters(anyListOf(ServletFilter.class));
+  }
+}