SONAR-23932 Drop support of deprecated org.sonar.api.web.ServletFilter

5 files changed, 44 insertions, 124 deletions

diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
index 4e91cd37f7c..1e6fe58235d 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
@@ -41,7 +41,6 @@ import org.sonar.api.server.http.HttpRequest;
 import org.sonar.api.server.http.HttpResponse;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.web.HttpFilter;
-import org.sonar.api.web.ServletFilter;
 import org.sonar.server.http.JavaxHttpRequest;
 import org.sonar.server.http.JavaxHttpResponse;
 import org.sonar.server.platform.PlatformImpl;
@@ -55,10 +54,7 @@ public class MasterServletFilter implements Filter {
   private static final Logger LOG = LoggerFactory.getLogger(MasterServletFilter.class);
   private static volatile MasterServletFilter instance;
 
-  @Deprecated(forRemoval = true)
-  private ServletFilter[] filters;
   private HttpFilter[] httpFilters;
-  private FilterConfig config;
 
   public MasterServletFilter() {
     if (instance != null) {
@@ -71,9 +67,8 @@ public class MasterServletFilter implements Filter {
   public void init(FilterConfig config) {
     // Filters are already available in the container unless a database migration is required. See
     // org.sonar.server.startup.RegisterServletFilters.
-    List<ServletFilter> servletFilterList = PlatformImpl.getInstance().getContainer().getComponentsByType(ServletFilter.class);
     List<HttpFilter> httpFilterList = PlatformImpl.getInstance().getContainer().getComponentsByType(HttpFilter.class);
-    init(config, servletFilterList, httpFilterList);
+    init(httpFilterList);
   }
 
   @CheckForNull
@@ -86,10 +81,8 @@ public class MasterServletFilter implements Filter {
     MasterServletFilter.instance = instance;
   }
 
-  void init(FilterConfig config, List<ServletFilter> filters, List<HttpFilter> httpFilters) {
-    this.config = config;
+  void init(List<HttpFilter> httpFilters) {
     initHttpFilters(httpFilters);
-    initServletFilters(filters);
   }
 
   public void initHttpFilters(List<HttpFilter> filterExtensions) {
@@ -118,26 +111,10 @@ public class MasterServletFilter implements Filter {
       .anyMatch(s -> s.startsWith(SCIM_FILTER_PATH));
   }
 
-  @Deprecated(forRemoval = true)
-  public void initServletFilters(List<ServletFilter> filterExtensions) {
-    LinkedList<ServletFilter> filterList = new LinkedList<>();
-    for (ServletFilter extension : filterExtensions) {
-      try {
-        LOG.info(String.format("Initializing servlet filter %s [pattern=%s]", extension, extension.doGetPattern().label()));
-        extension.init(config);
-        // adding deprecated extensions as last
-        filterList.addLast(extension);
-      } catch (Exception e) {
-        throw new IllegalStateException("Fail to initialize servlet filter: " + extension + ". Message: " + e.getMessage(), e);
-      }
-    }
-    filters = filterList.toArray(new ServletFilter[0]);
-  }
-
   @Override
   public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws ServletException, IOException {
     HttpServletRequest hsr = (HttpServletRequest) request;
-    if (filters.length == 0 && httpFilters.length == 0) {
+    if (httpFilters.length == 0) {
       chain.doFilter(hsr, response);
     } else {
       String path = hsr.getRequestURI().replaceFirst(hsr.getContextPath(), "");
@@ -151,29 +128,16 @@ public class MasterServletFilter implements Filter {
     Arrays.stream(httpFilters)
       .filter(filter -> filter.doGetPattern().matches(path))
       .forEachOrdered(godChain::addFilter);
-
-    Arrays.stream(filters)
-      .filter(filter -> filter.doGetPattern().matches(path))
-      .forEachOrdered(godChain::addFilter);
   }
 
   @Override
   public void destroy() {
-    for (ServletFilter filter : filters) {
-      filter.destroy();
-    }
-
     for (HttpFilter filter : httpFilters) {
       filter.destroy();
     }
   }
 
   @VisibleForTesting
-  ServletFilter[] getFilters() {
-    return filters;
-  }
-
-  @VisibleForTesting
   HttpFilter[] getHttpFilters() {
     return httpFilters;
   }
diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/RegisterServletFilters.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/RegisterServletFilters.java
index fc53679faca..6f5e5a32ed9 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/RegisterServletFilters.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/RegisterServletFilters.java
@@ -22,31 +22,22 @@ package org.sonar.server.platform.web;
 import java.util.Arrays;
 import org.sonar.api.Startable;
 import org.sonar.api.web.HttpFilter;
-import org.sonar.api.web.ServletFilter;
 import org.springframework.beans.factory.annotation.Autowired;
 
 /**
  * @since 3.5
  */
 public class RegisterServletFilters implements Startable {
-  private final ServletFilter[] servletFilters;
   private final HttpFilter[] httpFilters;
 
   @Autowired(required = false)
-  @Deprecated(since = "10.1", forRemoval = true)
-  public RegisterServletFilters(ServletFilter[] servletFilters, HttpFilter[] httpFilters) {
-    this.servletFilters = servletFilters;
-    this.httpFilters = httpFilters;
-  }
-
-  @Autowired(required = false)
   public RegisterServletFilters(HttpFilter[] httpFilters) {
-    this(new ServletFilter[0], httpFilters);
+    this.httpFilters = httpFilters;
   }
 
   @Autowired(required = false)
   public RegisterServletFilters() {
-    this(new ServletFilter[0], new HttpFilter[0]);
+    this(new HttpFilter[0]);
   }
 
   @Override
@@ -57,7 +48,6 @@ public class RegisterServletFilters implements Startable {
       // while spring was not completely up.
       // See https://jira.sonarsource.com/browse/SONAR-3612
       masterServletFilter.initHttpFilters(Arrays.asList(httpFilters));
-      masterServletFilter.initServletFilters(Arrays.asList(servletFilters));
     }
   }
 
diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/WebPagesFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/WebPagesFilter.java
index acff9e079a3..7738429dd79 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/WebPagesFilter.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/WebPagesFilter.java
@@ -29,7 +29,7 @@ import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import org.sonar.api.web.ServletFilter;
+import org.sonar.api.web.UrlPattern;
 import org.sonar.server.platform.PlatformImpl;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -48,7 +48,7 @@ public class WebPagesFilter implements Filter {
   private static final String CACHE_CONTROL_HEADER = "Cache-Control";
   private static final String CACHE_CONTROL_VALUE = "no-cache, no-store, must-revalidate";
 
-  private static final ServletFilter.UrlPattern URL_PATTERN = ServletFilter.UrlPattern
+  private static final UrlPattern URL_PATTERN = UrlPattern
     .builder()
     .excludes(patterns())
     .excludes("/api/v2/*")
@@ -60,7 +60,8 @@ public class WebPagesFilter implements Filter {
     this(PlatformImpl.getInstance().getContainer().getComponentByType(WebPagesCache.class));
   }
 
-  @VisibleForTesting WebPagesFilter(WebPagesCache webPagesCache) {
+  @VisibleForTesting
+  WebPagesFilter(WebPagesCache webPagesCache) {
     this.webPagesCache = webPagesCache;
   }
 
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
index f8d63216c39..def2f8d1438 100644
--- a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
@@ -24,28 +24,24 @@ import java.util.Collections;
 import java.util.List;
 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 org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.InOrder;
 import org.mockito.Mockito;
 import org.slf4j.event.Level;
 import org.sonar.api.server.http.HttpRequest;
 import org.sonar.api.server.http.HttpResponse;
-import org.sonar.api.testfixtures.log.LogTester;
+import org.sonar.api.testfixtures.log.LogTesterJUnit5;
 import org.sonar.api.web.HttpFilter;
-import org.sonar.api.web.ServletFilter;
-import org.sonar.api.web.ServletFilter.UrlPattern;
 import org.sonar.server.http.JavaxHttpRequest;
 import org.sonar.server.http.JavaxHttpResponse;
 
 import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -58,34 +54,29 @@ import static org.mockito.Mockito.when;
 
 public class MasterServletFilterTest {
 
-  @Rule
-  public LogTester logTester = new LogTester();
+  @RegisterExtension
+  private final LogTesterJUnit5 logTester = new LogTesterJUnit5();
 
-  @Before
+  @BeforeEach
   public void resetSingleton() {
     MasterServletFilter.setInstance(null);
   }
 
   @Test
-  public void should_init_and_destroy_filters() throws ServletException {
-    ServletFilter servletFilter = createMockServletFilter();
+  void should_init_and_destroy_filters() {
     HttpFilter httpFilter = createMockHttpFilter();
-    FilterConfig config = mock(FilterConfig.class);
     MasterServletFilter master = new MasterServletFilter();
-    master.init(config, singletonList(servletFilter),  singletonList(httpFilter));
+    master.init(singletonList(httpFilter));
 
-    assertThat(master.getFilters()).containsOnly(servletFilter);
     assertThat(master.getHttpFilters()).containsOnly(httpFilter);
-    verify(servletFilter).init(config);
     verify(httpFilter).init();
 
     master.destroy();
-    verify(servletFilter).destroy();
     verify(httpFilter).destroy();
   }
 
   @Test
-  public void servlet_container_should_instantiate_only_a_single_master_instance() {
+  void servlet_container_should_instantiate_only_a_single_master_instance() {
     new MasterServletFilter();
 
     assertThatThrownBy(MasterServletFilter::new)
@@ -94,25 +85,22 @@ public class MasterServletFilterTest {
   }
 
   @Test
-  public void should_propagate_initialization_failure() throws Exception {
-    ServletFilter filter = createMockServletFilter();
-    doThrow(new IllegalStateException("foo")).when(filter).init(any(FilterConfig.class));
+  void should_propagate_initialization_failure() {
+    HttpFilter filter = createMockHttpFilter();
+    doThrow(new IllegalStateException("foo")).when(filter).init();
 
-    FilterConfig config = mock(FilterConfig.class);
     MasterServletFilter filters = new MasterServletFilter();
 
-    List<ServletFilter> servletFilters = singletonList(filter);
-    List<HttpFilter> httpFilters = emptyList();
-    assertThatThrownBy(() -> filters.init(config, servletFilters, httpFilters))
+    List<HttpFilter> httpFilters = singletonList(filter);
+    assertThatThrownBy(() -> filters.init(httpFilters))
       .isInstanceOf(IllegalStateException.class)
       .hasMessageContaining("foo");
   }
 
   @Test
-  public void filters_should_be_optional() throws Exception {
-    FilterConfig config = mock(FilterConfig.class);
+  void filters_should_be_optional() throws Exception {
     MasterServletFilter filters = new MasterServletFilter();
-    filters.init(config, Collections.emptyList(), Collections.emptyList());
+    filters.init(Collections.emptyList());
 
     ServletRequest request = mock(HttpServletRequest.class);
     ServletResponse response = mock(HttpServletResponse.class);
@@ -123,7 +111,7 @@ public class MasterServletFilterTest {
   }
 
   @Test
-  public void should_add_scim_filter_first_for_scim_request() throws Exception {
+  void should_add_scim_filter_first_for_scim_request() throws Exception {
     String scimPath = "/api/scim/v2/Groups";
     HttpServletRequest request = mock(HttpServletRequest.class);
     HttpServletResponse response = mock(HttpServletResponse.class);
@@ -137,33 +125,18 @@ public class MasterServletFilterTest {
 
     FilterChain chain = mock(FilterChain.class);
 
-    ServletFilter filter1 = mockFilter(ServletFilter.class, request, response);
-    ServletFilter filter2 = mockFilter(ServletFilter.class, request, response);
     HttpFilter filter3 = mockHttpFilter(WebServiceFilter.class, httpRequest, httpResponse);
     HttpFilter filter4 = mockHttpFilter(HttpFilter.class, httpRequest, httpResponse);
     when(filter3.doGetPattern()).thenReturn(org.sonar.api.web.UrlPattern.builder().includes(scimPath).build());
 
     MasterServletFilter filters = new MasterServletFilter();
-    filters.init(mock(FilterConfig.class), asList(filter1, filter2), asList(filter4, filter3));
+    filters.init(asList(filter4, filter3));
 
     filters.doFilter(request, response, chain);
 
-    InOrder inOrder = Mockito.inOrder(filter1, filter2, filter3, filter4);
+    InOrder inOrder = Mockito.inOrder(filter3, filter4);
     inOrder.verify(filter3).doFilter(any(), any(), any());
     inOrder.verify(filter4).doFilter(any(), any(), any());
-    inOrder.verify(filter1).doFilter(any(), any(), any());
-    inOrder.verify(filter2).doFilter(any(), any(), any());
-  }
-
-  private ServletFilter mockFilter(Class<? extends ServletFilter> filterClazz, HttpServletRequest request, ServletResponse response) throws IOException, ServletException {
-    ServletFilter filter = mock(filterClazz);
-    when(filter.doGetPattern()).thenReturn(UrlPattern.builder().build());
-    doAnswer(invocation -> {
-      FilterChain argument = invocation.getArgument(2, FilterChain.class);
-      argument.doFilter(request, response);
-      return null;
-    }).when(filter).doFilter(any(), any(), any());
-    return filter;
   }
 
   private HttpFilter mockHttpFilter(Class<? extends HttpFilter> filterClazz, HttpRequest request, HttpResponse response) throws IOException {
@@ -178,22 +151,16 @@ public class MasterServletFilterTest {
   }
 
   @Test
-  public void display_servlet_filter_patterns_in_INFO_log() {
+  void display_servlet_filter_patterns_in_INFO_log() {
     HttpFilter filter = new PatternFilter(org.sonar.api.web.UrlPattern.builder().includes("/api/issues").excludes("/batch/projects").build());
     FilterConfig config = mock(FilterConfig.class);
     MasterServletFilter master = new MasterServletFilter();
 
-    master.init(config, emptyList(), singletonList(filter));
+    master.init(singletonList(filter));
 
     assertThat(logTester.logs(Level.INFO)).containsOnly("Initializing servlet filter PatternFilter [pattern=UrlPattern{inclusions=[/api/issues], exclusions=[/batch/projects]}]");
   }
 
-  private static ServletFilter createMockServletFilter() {
-    ServletFilter filter = mock(ServletFilter.class);
-    when(filter.doGetPattern()).thenReturn(UrlPattern.builder().build());
-    return filter;
-  }
-
   private static HttpFilter createMockHttpFilter() {
     HttpFilter filter = mock(HttpFilter.class);
     when(filter.doGetPattern()).thenReturn(org.sonar.api.web.UrlPattern.builder().build());
@@ -215,7 +182,7 @@ public class MasterServletFilterTest {
 
     @Override
     public void doFilter(HttpRequest request, HttpResponse response, org.sonar.api.web.FilterChain chain) {
-      //Not needed for this test
+      // Not needed for this test
     }
 
     @Override
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/RegisterServletFiltersTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/RegisterServletFiltersTest.java
index e3fd66a22e5..d288806b9a2 100644
--- a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/RegisterServletFiltersTest.java
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/RegisterServletFiltersTest.java
@@ -19,35 +19,33 @@
  */
 package org.sonar.server.platform.web;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 import org.sonar.api.web.HttpFilter;
-import org.sonar.api.web.ServletFilter;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.mockito.ArgumentMatchers.anyList;
 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() {
+  void should_not_fail_if_master_filter_is_not_up() {
     MasterServletFilter.setInstance(null);
-    new RegisterServletFilters(new ServletFilter[2], new HttpFilter[2]).start();
-  }
 
-  @Test
-  public void should_register_filters_if_master_filter_is_up() {
-    MasterServletFilter.setInstance(mock(MasterServletFilter.class));
-    new RegisterServletFilters(new ServletFilter[2], new HttpFilter[2]).start();
+    RegisterServletFilters underTest = new RegisterServletFilters(new HttpFilter[2]);
 
-    verify(MasterServletFilter.getInstance()).initServletFilters(anyList());
+    assertThatCode(underTest::start)
+      .doesNotThrowAnyException();
   }
 
   @Test
-  public void filters_should_be_optional() {
+  void filters_should_be_optional() {
     MasterServletFilter.setInstance(mock(MasterServletFilter.class));
-    new RegisterServletFilters().start();
-    // do not fail
+
+    RegisterServletFilters underTest = new RegisterServletFilters();
+    assertThatCode(underTest::start)
+      .doesNotThrowAnyException();
+
     verify(MasterServletFilter.getInstance()).initHttpFilters(anyList());
-    verify(MasterServletFilter.getInstance()).initServletFilters(anyList());
   }
 }