diff options
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()); } } |