From: Simon Brandhof Date: Wed, 18 Apr 2018 19:33:48 +0000 (+0200) Subject: Fix Quality flaws X-Git-Tag: 7.5~1230 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5097d1877b4d9d49a5a35a57ba4002ceb30b839e;p=sonarqube.git Fix Quality flaws --- diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/ce/LogsIteratorInputStreamTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/ce/LogsIteratorInputStreamTest.java index d21f9a1f492..89463e56b63 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/ce/LogsIteratorInputStreamTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/ce/LogsIteratorInputStreamTest.java @@ -20,7 +20,7 @@ package org.sonar.db.ce; import java.io.IOException; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import org.apache.commons.io.IOUtils; import org.junit.Rule; @@ -73,11 +73,11 @@ public class LogsIteratorInputStreamTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("LogsIterator can't be empty or already read"); - new LogsIteratorInputStream(iterator, Charset.forName("UTF-8")); + new LogsIteratorInputStream(iterator, StandardCharsets.UTF_8); } private static LogsIteratorInputStream create(String... lines) { - return new LogsIteratorInputStream(CloseableIterator.from(Arrays.asList(lines).iterator()), Charset.forName("UTF-8")); + return new LogsIteratorInputStream(CloseableIterator.from(Arrays.asList(lines).iterator()), StandardCharsets.UTF_8); } private static String read(LogsIteratorInputStream logsIteratorInputStream) throws IOException { diff --git a/server/sonar-main/src/main/java/org/sonar/application/command/EsJvmOptions.java b/server/sonar-main/src/main/java/org/sonar/application/command/EsJvmOptions.java index 966249fb630..bc2cc930cbc 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/command/EsJvmOptions.java +++ b/server/sonar-main/src/main/java/org/sonar/application/command/EsJvmOptions.java @@ -21,7 +21,7 @@ package org.sonar.application.command; import java.io.File; import java.io.IOException; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.LinkedHashMap; import java.util.Map; @@ -63,7 +63,7 @@ public class EsJvmOptions extends JvmOptions { String jvmOptions = getAll().stream().collect(Collectors.joining("\n")); String jvmOptionsContent = ELASTICSEARCH_JVM_OPTIONS_HEADER + jvmOptions; try { - Files.write(file.toPath(), jvmOptionsContent.getBytes(Charset.forName("UTF-8"))); + Files.write(file.toPath(), jvmOptionsContent.getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { throw new IllegalStateException("Cannot write Elasticsearch jvm options file", e); } diff --git a/server/sonar-main/src/main/java/org/sonar/application/es/EsYmlSettings.java b/server/sonar-main/src/main/java/org/sonar/application/es/EsYmlSettings.java index 8911abeb4a2..68412fc10fb 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/es/EsYmlSettings.java +++ b/server/sonar-main/src/main/java/org/sonar/application/es/EsYmlSettings.java @@ -21,7 +21,7 @@ package org.sonar.application.es; import java.io.File; import java.io.IOException; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.Map; import org.yaml.snakeyaml.DumperOptions; @@ -48,7 +48,7 @@ public class EsYmlSettings { Yaml yaml = new Yaml(dumperOptions); String output = ELASTICSEARCH_YML_OPTIONS_HEADER + yaml.dump(elasticsearchSettings); try { - Files.write(file.toPath(), output.getBytes(Charset.forName("UTF-8"))); + Files.write(file.toPath(), output.getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { throw new IllegalStateException("Cannot write Elasticsearch yml settings file", e); } diff --git a/server/sonar-server/build.gradle b/server/sonar-server/build.gradle index ac661c36f38..78bf82df662 100644 --- a/server/sonar-server/build.gradle +++ b/server/sonar-server/build.gradle @@ -75,6 +75,8 @@ dependencies { testCompile 'org.apache.logging.log4j:log4j-core' testCompile 'org.assertj:assertj-core' testCompile 'org.assertj:assertj-guava' + testCompile 'org.eclipse.jetty:jetty-server' + testCompile 'org.eclipse.jetty:jetty-servlet' testCompile 'org.hamcrest:hamcrest-all' testCompile 'org.mockito:mockito-core' testCompile 'org.reflections:reflections' diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadQualityProfilesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadQualityProfilesStep.java index 4037cbb763c..88eecf3ce99 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadQualityProfilesStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadQualityProfilesStep.java @@ -71,7 +71,7 @@ public class LoadQualityProfilesStep implements ComputationStep { private static ActiveRule convert(ScannerReport.ActiveRule input, Rule rule) { RuleKey key = RuleKey.of(input.getRuleRepository(), input.getRuleKey()); - Map params = new HashMap<>(input.getParamsByKey()); + Map params = new HashMap<>(input.getParamsByKeyMap()); return new ActiveRule(key, input.getSeverity().name(), params, input.getCreatedAt(), rule.getPluginKey()); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java b/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java index 3f2da92f6ae..3e5ad22e85b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java +++ b/server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java @@ -23,6 +23,7 @@ import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import javax.annotation.CheckForNull; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -31,7 +32,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; -import org.sonar.core.platform.ComponentContainer; import org.sonar.core.platform.PluginRepository; import org.sonar.server.platform.Platform; import org.sonarqube.ws.MediaTypes; @@ -45,6 +45,17 @@ public class StaticResourcesServlet extends HttpServlet { private static final Logger LOG = Loggers.get(StaticResourcesServlet.class); private static final long serialVersionUID = -2577454614650178426L; + private final System system; + + @VisibleForTesting + StaticResourcesServlet(System system) { + this.system = system; + } + + public StaticResourcesServlet() { + this.system = new System(); + } + @Override public void doGet(HttpServletRequest request, HttpServletResponse response) { String pluginKey = getPluginKey(request); @@ -52,13 +63,13 @@ public class StaticResourcesServlet extends HttpServlet { InputStream in = null; OutputStream out = null; try { - PluginRepository pluginRepository = getContainer().getComponentByType(PluginRepository.class); + PluginRepository pluginRepository = system.getPluginRepository(); if (!pluginRepository.hasPlugin(pluginKey)) { silentlySendError(response, SC_NOT_FOUND); return; } - in = pluginRepository.getPluginInstance(pluginKey).getClass().getClassLoader().getResourceAsStream(resource); + in = system.openResourceStream(pluginKey, resource, pluginRepository); if (in != null) { // mime type must be set before writing response body completeContentType(response, resource); @@ -78,18 +89,13 @@ public class StaticResourcesServlet extends HttpServlet { } } - @VisibleForTesting - protected ComponentContainer getContainer() { - return Platform.getInstance().getContainer(); - } - - private static void silentlySendError(HttpServletResponse response, int error) { - if (response.isCommitted()) { + private void silentlySendError(HttpServletResponse response, int error) { + if (system.isCommitted(response)) { LOG.trace("Response is committed. Cannot send error response code {}", error); return; } try { - response.sendError(error); + system.sendError(response, error); } catch (IOException e) { LOG.trace("Failed to send error code {}: {}", error, e); } @@ -102,20 +108,37 @@ public class StaticResourcesServlet extends HttpServlet { return StringUtils.substringAfter(request.getRequestURI(), request.getContextPath() + request.getServletPath() + "/"); } - @VisibleForTesting - protected String getPluginKey(HttpServletRequest request) { + private static String getPluginKey(HttpServletRequest request) { return StringUtils.substringBefore(getPluginKeyAndResourcePath(request), "/"); } /** * Note that returned value should not have a leading "/" - see {@link Class#resolveName(String)}. */ - protected String getResourcePath(HttpServletRequest request) { + private static String getResourcePath(HttpServletRequest request) { return "static/" + StringUtils.substringAfter(getPluginKeyAndResourcePath(request), "/"); } - @VisibleForTesting - void completeContentType(HttpServletResponse response, String filename) { + private static void completeContentType(HttpServletResponse response, String filename) { response.setContentType(MediaTypes.getByFilename(filename)); } + + static class System { + PluginRepository getPluginRepository() { + return Platform.getInstance().getContainer().getComponentByType(PluginRepository.class); + } + + @CheckForNull + InputStream openResourceStream(String pluginKey, String resource, PluginRepository pluginRepository) throws Exception { + return pluginRepository.getPluginInstance(pluginKey).getClass().getClassLoader().getResourceAsStream(resource); + } + + boolean isCommitted(HttpServletResponse response) { + return response.isCommitted(); + } + + void sendError(HttpServletResponse response, int error) throws IOException { + response.sendError(error); + } + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/PageRepository.java b/server/sonar-server/src/main/java/org/sonar/server/ui/PageRepository.java index 9635d587456..0929a36d438 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/PageRepository.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/PageRepository.java @@ -20,7 +20,6 @@ package org.sonar.server.ui; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import java.util.Collections; import java.util.List; @@ -46,8 +45,6 @@ import static org.sonar.core.util.stream.MoreCollectors.toList; @ServerSide public class PageRepository implements Startable { - private static final Splitter PAGE_KEY_SPLITTER = Splitter.on("/").omitEmptyStrings(); - private final PluginRepository pluginRepository; private final List definitions; private List pages; @@ -60,15 +57,7 @@ public class PageRepository implements Startable { public PageRepository(PluginRepository pluginRepository, PageDefinition[] pageDefinitions) { this.pluginRepository = pluginRepository; - definitions = ImmutableList.copyOf(pageDefinitions); - } - - private static Consumer checkWellFormed() { - return page -> { - boolean isWellFormed = PAGE_KEY_SPLITTER.splitToList(page.getKey()).size() == 2; - checkState(isWellFormed, "Page '%s' with key '%s' does not respect the format plugin_key/extension_point_key (ex: governance/project_dump)", - page.getName(), page.getKey()); - }; + this.definitions = ImmutableList.copyOf(pageDefinitions); } @Override @@ -76,7 +65,6 @@ public class PageRepository implements Startable { Context context = new Context(); definitions.forEach(definition -> definition.define(context)); pages = context.getPages().stream() - .peek(checkWellFormed()) .peek(checkPluginExists()) .sorted(comparing(Page::getKey)) .collect(toList()); @@ -115,9 +103,9 @@ public class PageRepository implements Startable { private Consumer checkPluginExists() { return page -> { - String plugin = PAGE_KEY_SPLITTER.splitToList(page.getKey()).get(0); - boolean pluginExists = pluginRepository.hasPlugin(plugin); - checkState(pluginExists, "Page '%s' references plugin '%s' that does not exist", page.getName(), plugin); + String pluginKey = page.getPluginKey(); + boolean pluginExists = pluginRepository.hasPlugin(pluginKey); + checkState(pluginExists, "Page '%s' references plugin '%s' that does not exist", page.getName(), pluginKey); }; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalAction.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalAction.java index ababbeb65e5..19baa441108 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalAction.java @@ -32,6 +32,7 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService.NewController; import org.sonar.api.utils.text.JsonWriter; +import org.sonar.api.web.page.Page; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.dialect.H2; @@ -136,7 +137,7 @@ public class GlobalAction implements NavigationWsAction, Startable { private void writePages(JsonWriter json) { json.name("globalPages").beginArray(); - for (org.sonar.api.web.page.Page page : pageRepository.getGlobalPages(false)) { + for (Page page : pageRepository.getGlobalPages(false)) { json.beginObject() .prop("key", page.getKey()) .prop("name", page.getName()) diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/OrganizationAction.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/OrganizationAction.java index 8542426fb8e..ca03a893042 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/OrganizationAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/OrganizationAction.java @@ -20,7 +20,6 @@ package org.sonar.server.ui.ws; import java.util.List; -import java.util.function.Consumer; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; @@ -117,14 +116,10 @@ public class OrganizationAction implements NavigationWsAction { private static void writePages(JsonWriter json, List pages) { json.beginArray(); - pages.forEach(writePage(json)); + pages.forEach(p -> json.beginObject() + .prop("key", p.getKey()) + .prop("name", p.getName()) + .endObject()); json.endArray(); } - - private static Consumer writePage(JsonWriter json) { - return page -> json.beginObject() - .prop("key", page.getKey()) - .prop("name", page.getName()) - .endObject(); - } } diff --git a/server/sonar-server/src/main/resources/org/sonar/server/ui/ws/settings-example.json b/server/sonar-server/src/main/resources/org/sonar/server/ui/ws/settings-example.json index 383b5e0cc5a..bd5bb3de19d 100644 --- a/server/sonar-server/src/main/resources/org/sonar/server/ui/ws/settings-example.json +++ b/server/sonar-server/src/main/resources/org/sonar/server/ui/ws/settings-example.json @@ -2,8 +2,8 @@ "showUpdateCenter": true, "extensions": [ { - "name": "Views", - "url": "/plugins/configuration/views" + "name": "Sample", + "url": "/plugins/configuration/sample" } ] } diff --git a/server/sonar-server/src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java b/server/sonar-server/src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java index fab7ebee9cf..fad36be8214 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java @@ -20,182 +20,243 @@ package org.sonar.server.plugins; import java.io.IOException; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; +import java.io.InputStream; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import javax.servlet.http.HttpServletResponse; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; import org.apache.catalina.connector.ClientAbortException; +import org.apache.commons.io.IOUtils; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.Plugin; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.core.platform.ComponentContainer; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class StaticResourcesServletTest { @Rule public LogTester logTester = new LogTester(); + private Server jetty; - private HttpServletRequest request = mock(HttpServletRequest.class); - private HttpServletResponse response = mock(HttpServletResponse.class); - private ComponentContainer componentContainer = mock(ComponentContainer.class); private PluginRepository pluginRepository = mock(PluginRepository.class); - private StaticResourcesServlet underTest = new StaticResourcesServlet() { - @Override - protected ComponentContainer getContainer() { - return componentContainer; + private TestSystem system = new TestSystem(pluginRepository); + + @Before + public void setUp() throws Exception { + jetty = new Server(0); + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/"); + ServletHolder servletHolder = new ServletHolder(new StaticResourcesServlet(system)); + context.addServlet(servletHolder, "/static/*"); + jetty.setHandler(context); + jetty.start(); + } + + @After + public void tearDown() throws Exception { + if (jetty != null) { + jetty.stop(); } - }; + } + + private Response call(String path) throws Exception { + OkHttpClient client = new OkHttpClient(); + Request request = new Request.Builder() + .url(jetty.getURI().resolve(path).toString()) + .build(); + return client.newCall(request).execute(); + } @Test - public void shouldDeterminePluginKey() { - mockRequest("myplugin", "image.png"); - assertThat(underTest.getPluginKey(request)).isEqualTo("myplugin"); + public void return_content_if_exists_in_installed_plugin() throws Exception { + system.answer = IOUtils.toInputStream("bar"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - mockRequest("myplugin", "images/image.png"); - assertThat(underTest.getPluginKey(request)).isEqualTo("myplugin"); + Response response = call("/static/myplugin/foo.txt"); - mockRequest("myplugin", ""); - assertThat(underTest.getPluginKey(request)).isEqualTo("myplugin"); + assertThat(response.isSuccessful()).isTrue(); + assertThat(response.body().string()).isEqualTo("bar"); + assertThat(system.calledResource).isEqualTo("static/foo.txt"); } @Test - public void shouldDetermineResourcePath() { - mockRequest("myplugin", "image.png"); - assertThat(underTest.getResourcePath(request)).isEqualTo("static/image.png"); + public void return_content_of_folder_of_installed_plugin() throws Exception { + system.answer = IOUtils.toInputStream("bar"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - mockRequest("myplugin", "images/image.png"); - assertThat(underTest.getResourcePath(request)).isEqualTo("static/images/image.png"); + Response response = call("/static/myplugin/foo/bar.txt"); - mockRequest("myplugin", ""); - assertThat(underTest.getResourcePath(request)).isEqualTo("static/"); + assertThat(response.isSuccessful()).isTrue(); + assertThat(response.body().string()).isEqualTo("bar"); + assertThat(system.calledResource).isEqualTo("static/foo/bar.txt"); } @Test - public void completeMimeType() { - HttpServletResponse response = mock(HttpServletResponse.class); - underTest.completeContentType(response, "static/sqale/sqale.css"); - verify(response).setContentType("text/css"); + public void mime_type_is_set_on_response() throws Exception { + system.answer = IOUtils.toInputStream("bar"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); + + Response response = call("/static/myplugin/foo.css"); + + assertThat(response.header("Content-Type")).isEqualTo("text/css"); + assertThat(response.body().string()).isEqualTo("bar"); } @Test - public void does_not_fail_nor_log_ERROR_when_response_is_already_committed_and_plugin_does_not_exist() throws ServletException, IOException { - mockPluginForResourceDoesNotExist(); - mockSendErrorOnCommittedResponse(); + public void return_404_if_resource_not_found_in_installed_plugin() throws Exception { + system.answer = null; + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); + assertThat(response.code()).isEqualTo(404); assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Response is committed. Cannot send error response code 404"); + assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); } @Test - public void does_not_fail_nor_log_ERROR_when_sendError_throws_IOException_and_plugin_does_not_exist() throws ServletException, IOException { - mockPluginForResourceDoesNotExist(); - mockSendErrorThrowsIOException(); + public void return_404_if_plugin_does_not_exist() throws Exception { + system.answer = null; + when(pluginRepository.hasPlugin("myplugin")).thenReturn(false); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); + assertThat(response.code()).isEqualTo(404); assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Failed to send error code 404: java.io.IOException: Simulating sendError throwing IOException"); - } - - private void mockPluginForResourceDoesNotExist() { - String pluginKey = "myplugin"; - mockRequest(pluginKey, "image.png"); - when(pluginRepository.hasPlugin(pluginKey)).thenReturn(false); - when(componentContainer.getComponentByType(PluginRepository.class)).thenReturn(pluginRepository); + assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); } @Test - public void does_not_fail_nor_log_ERROR_when_response_is_already_committed_and_plugin_exists_but_not_resource() throws ServletException, IOException { - mockPluginExistsButNoResource(); - mockSendErrorOnCommittedResponse(); + public void return_resource_if_exists_in_requested_plugin() throws Exception { + system.answer = IOUtils.toInputStream("bar"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); + when(pluginRepository.getPluginInfo("myplugin")).thenReturn(new PluginInfo("myplugin")); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); + assertThat(response.isSuccessful()).isTrue(); + assertThat(response.body().string()).isEqualTo("bar"); assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Response is committed. Cannot send error response code 404"); + assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty(); } @Test - public void does_not_fail_nor_log_ERROR_when_sendError_throws_IOException_and_plugin_exists_but_not_resource() throws ServletException, IOException { - mockPluginExistsButNoResource(); - mockSendErrorThrowsIOException(); + public void do_not_fail_nor_log_ERROR_when_response_is_already_committed_and_plugin_does_not_exist() throws Exception { + system.answer = null; + system.isCommitted = true; + when(pluginRepository.hasPlugin("myplugin")).thenReturn(false); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); + assertThat(response.code()).isEqualTo(200); assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Failed to send error code 404: java.io.IOException: Simulating sendError throwing IOException"); + assertThat(logTester.logs(LoggerLevel.TRACE)).contains("Response is committed. Cannot send error response code 404"); } - private void mockPluginExistsButNoResource() { - String pluginKey = "myplugin"; - mockRequest(pluginKey, "image.png"); - when(pluginRepository.hasPlugin(pluginKey)).thenReturn(true); - when(pluginRepository.getPluginInstance(pluginKey)).thenReturn(mock(Plugin.class)); - when(componentContainer.getComponentByType(PluginRepository.class)).thenReturn(pluginRepository); + @Test + public void do_not_fail_nor_log_ERROR_when_sendError_throws_IOException_and_plugin_does_not_exist() throws Exception { + system.sendErrorException = new IOException("Simulating sendError throwing IOException"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(false); + + Response response = call("/static/myplugin/foo.css"); + + assertThat(response.code()).isEqualTo(200); + assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); + assertThat(logTester.logs(LoggerLevel.TRACE)).contains("Failed to send error code 404: java.io.IOException: Simulating sendError throwing IOException"); } @Test - public void does_not_fail_nor_log_not_attempt_to_send_error_if_ClientAbortException_is_raised() throws ServletException, IOException { - String pluginKey = "myplugin"; - mockRequest(pluginKey, "foo.txt"); - when(pluginRepository.hasPlugin(pluginKey)).thenReturn(true); - when(pluginRepository.getPluginInstance(pluginKey)).thenReturn(new TestPluginA()); - when(componentContainer.getComponentByType(PluginRepository.class)).thenReturn(pluginRepository); - when(response.getOutputStream()).thenThrow(new ClientAbortException("Simulating ClientAbortException")); + public void do_not_fail_nor_log_ERROR_when_response_is_already_committed_and_resource_does_not_exist_in_installed_plugin() throws Exception { + system.isCommitted = true; + system.answer = null; + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); + assertThat(response.code()).isEqualTo(200); assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); - assertThat(logTester.logs(LoggerLevel.TRACE)).containsOnly("Client canceled loading resource [static/foo.txt] from plugin [myplugin]: org.apache.catalina.connector.ClientAbortException: Simulating ClientAbortException"); - verify(response, times(0)).sendError(anyInt()); + assertThat(logTester.logs(LoggerLevel.TRACE)).contains("Response is committed. Cannot send error response code 404"); } @Test - public void does_not_fail_when_response_is_committed_after_other_error() throws ServletException, IOException { - mockRequest("myplugin", "image.png"); - when(componentContainer.getComponentByType(PluginRepository.class)).thenThrow(new RuntimeException("Simulating a error")); - mockSendErrorOnCommittedResponse(); + public void do_not_fail_nor_log_not_attempt_to_send_error_if_ClientAbortException_is_raised() throws Exception { + system.answerException = new ClientAbortException("Simulating ClientAbortException"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); - assertThat(logTester.logs(LoggerLevel.ERROR)).containsOnly("Unable to load resource [static/image.png] from plugin [myplugin]"); + assertThat(response.code()).isEqualTo(200); + assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); + assertThat(logTester.logs(LoggerLevel.TRACE)).contains( + "Client canceled loading resource [static/foo.css] from plugin [myplugin]: org.apache.catalina.connector.ClientAbortException: Simulating ClientAbortException"); } @Test - public void does_not_fail_when_sendError_throws_IOException_after_other_error() throws ServletException, IOException { - mockRequest("myplugin", "image.png"); - when(componentContainer.getComponentByType(PluginRepository.class)).thenThrow(new RuntimeException("Simulating a error")); - mockSendErrorThrowsIOException(); + public void do_not_fail_when_response_is_committed_after_other_error() throws Exception { + system.isCommitted = true; + system.answerException = new RuntimeException("Simulating a error"); + when(pluginRepository.hasPlugin("myplugin")).thenReturn(true); - underTest.doGet(request, response); + Response response = call("/static/myplugin/foo.css"); - assertThat(logTester.logs(LoggerLevel.ERROR)).containsOnly("Unable to load resource [static/image.png] from plugin [myplugin]"); + assertThat(response.code()).isEqualTo(200); + assertThat(logTester.logs(LoggerLevel.ERROR)).contains("Unable to load resource [static/foo.css] from plugin [myplugin]"); } - private void mockSendErrorThrowsIOException() throws IOException { - doThrow(new IOException("Simulating sendError throwing IOException")).when(response).sendError(anyInt()); - } + private static class TestSystem extends StaticResourcesServlet.System { + private final PluginRepository pluginRepository; + @Nullable + private InputStream answer; + private Exception answerException = null; + @Nullable + private String calledResource; + private boolean isCommitted = false; + private IOException sendErrorException = null; + + TestSystem(PluginRepository pluginRepository) { + this.pluginRepository = pluginRepository; + } - private void mockSendErrorOnCommittedResponse() throws IOException { - when(response.isCommitted()).thenReturn(true); - doThrow(new IllegalStateException("Simulating sendError call when already committed")).when(response).sendError(anyInt()); - } + @Override + PluginRepository getPluginRepository() { + return pluginRepository; + } + + @CheckForNull + @Override + InputStream openResourceStream(String pluginKey, String resource, PluginRepository pluginRepository) throws Exception { + calledResource = resource; + if (answerException != null) { + throw answerException; + } + return answer; + } - private void mockRequest(String plugin, String resource) { - when(request.getContextPath()).thenReturn("/"); - when(request.getServletPath()).thenReturn("static"); - when(request.getRequestURI()).thenReturn("/static/" + plugin + "/" + resource); + @Override + boolean isCommitted(HttpServletResponse response) { + return isCommitted; + } + + @Override + void sendError(HttpServletResponse response, int error) throws IOException { + if (sendErrorException != null) { + throw sendErrorException; + } else { + super.sendError(response, error); + } + } } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/PageRepositoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/PageRepositoryTest.java index df25db5fa4a..89165d85990 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/PageRepositoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/PageRepositoryTest.java @@ -29,11 +29,12 @@ import org.sonar.api.utils.log.LogTester; import org.sonar.api.web.page.Page; import org.sonar.api.web.page.Page.Qualifier; import org.sonar.api.web.page.PageDefinition; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; -import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.web.page.Page.Scope.COMPONENT; @@ -45,7 +46,7 @@ public class PageRepositoryTest { @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule - public LogTester LOGGER = new LogTester(); + public LogTester logTester = new LogTester(); private PluginRepository pluginRepository = mock(PluginRepository.class); @@ -53,7 +54,8 @@ public class PageRepositoryTest { @Before public void setUp() { - when(pluginRepository.hasPlugin(anyString())).thenReturn(true); + when(pluginRepository.hasPlugin(any())).thenReturn(true); + when(pluginRepository.getPluginInfo(any())).thenReturn(new PluginInfo("unused")); } @Test @@ -62,12 +64,13 @@ public class PageRepositoryTest { .addPage(Page.builder("my_plugin/K1").setName("N1").build()) .addPage(Page.builder("my_plugin/K3").setName("N3").build()); PageDefinition secondPlugin = context -> context.addPage(Page.builder("my_plugin/K2").setName("N2").build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {firstPlugin, secondPlugin}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{firstPlugin, secondPlugin}); underTest.start(); List result = underTest.getAllPages(); - assertThat(result).extracting(Page::getKey, Page::getName) + assertThat(result) + .extracting(Page::getKey, Page::getName) .containsExactly( tuple("my_plugin/K1", "N1"), tuple("my_plugin/K2", "N2"), @@ -84,12 +87,14 @@ public class PageRepositoryTest { .addPage(Page.builder("my_plugin/K4").setName("K4").setScope(GLOBAL).build()) .addPage(Page.builder("my_plugin/K5").setName("K5").setScope(COMPONENT).setComponentQualifiers(Qualifier.VIEW).build()) .addPage(Page.builder("my_plugin/K6").setName("K6").setScope(COMPONENT).setComponentQualifiers(Qualifier.APP).build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {plugin}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{plugin}); underTest.start(); List result = underTest.getComponentPages(false, Qualifiers.PROJECT); - assertThat(result).extracting(Page::getKey).containsExactly("my_plugin/K2"); + assertThat(result) + .extracting(Page::getKey) + .containsExactly("my_plugin/K2"); } @Test @@ -107,12 +112,14 @@ public class PageRepositoryTest { .addPage(Page.builder("my_plugin/K1").setName("N1").build()) .addPage(Page.builder("my_plugin/K2").setName("N2").build()) .addPage(Page.builder("my_plugin/K3").setName("N3").build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {plugin}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{plugin}); underTest.start(); List result = underTest.getGlobalPages(false); - assertThat(result).extracting(Page::getKey).containsExactly("my_plugin/K1", "my_plugin/K2", "my_plugin/K3"); + assertThat(result) + .extracting(Page::getKey) + .containsExactly("my_plugin/K1", "my_plugin/K2", "my_plugin/K3"); } @Test @@ -124,12 +131,14 @@ public class PageRepositoryTest { .addPage(Page.builder("my_plugin/O2").setName("O2").setScope(ORGANIZATION).build()) .addPage(Page.builder("my_plugin/O3").setName("O3").setScope(ORGANIZATION).build()) .addPage(Page.builder("my_plugin/OA1").setName("OA1").setScope(ORGANIZATION).setAdmin(true).build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {plugin}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{plugin}); underTest.start(); List result = underTest.getOrganizationPages(false); - assertThat(result).extracting(Page::getKey).containsExactly("my_plugin/O1", "my_plugin/O2", "my_plugin/O3"); + assertThat(result) + .extracting(Page::getKey) + .containsExactly("my_plugin/O1", "my_plugin/O2", "my_plugin/O3"); } @Test @@ -137,12 +146,14 @@ public class PageRepositoryTest { PageDefinition plugin = context -> context .addPage(Page.builder("my_plugin/O1").setName("O1").setScope(ORGANIZATION).build()) .addPage(Page.builder("my_plugin/O2").setName("O2").setScope(ORGANIZATION).setAdmin(true).build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {plugin}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{plugin}); underTest.start(); List result = underTest.getOrganizationPages(true); - assertThat(result).extracting(Page::getKey).containsExactly("my_plugin/O2"); + assertThat(result) + .extracting(Page::getKey) + .containsExactly("my_plugin/O2"); } @Test @@ -153,26 +164,13 @@ public class PageRepositoryTest { underTest.getAllPages(); } - @Test - public void fail_if_page_with_wrong_format() { - PageDefinition plugin = context -> context - .addPage(Page.builder("my_key").setName("N1").build()) - .addPage(Page.builder("my_plugin/my_key").setName("N2").build()); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {plugin}); - - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("Page 'N1' with key 'my_key' does not respect the format plugin_key/extension_point_key (ex: governance/project_dump)"); - - underTest.start(); - } - @Test public void fail_if_page_with_unknown_plugin() { PageDefinition governance = context -> context.addPage(Page.builder("governance/my_key").setName("N1").build()); PageDefinition plugin42 = context -> context.addPage(Page.builder("plugin_42/my_key").setName("N2").build()); pluginRepository = mock(PluginRepository.class); when(pluginRepository.hasPlugin("governance")).thenReturn(true); - underTest = new PageRepository(pluginRepository, new PageDefinition[] {governance, plugin42}); + underTest = new PageRepository(pluginRepository, new PageDefinition[]{governance, plugin42}); expectedException.expect(IllegalStateException.class); expectedException.expectMessage("Page 'N2' references plugin 'plugin_42' that does not exist"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java index ec18a37b36d..7ddcd2b32c0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java @@ -35,6 +35,7 @@ import org.sonar.api.web.page.Page; import org.sonar.api.web.page.Page.Qualifier; import org.sonar.api.web.page.PageDefinition; import org.sonar.core.component.DefaultResourceTypes; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import org.sonar.db.DbClient; import org.sonar.db.DbTester; @@ -61,11 +62,11 @@ import org.sonar.server.qualityprofile.QualityProfile; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ui.PageRepository; import org.sonar.server.ws.WsActionTester; +import org.sonar.updatecenter.common.Version; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.measures.CoreMetrics.QUALITY_PROFILES_KEY; @@ -592,7 +593,8 @@ public class ComponentActionTest { private void init(Page... pages) { PluginRepository pluginRepository = mock(PluginRepository.class); - when(pluginRepository.hasPlugin(anyString())).thenReturn(true); + when(pluginRepository.hasPlugin(any())).thenReturn(true); + when(pluginRepository.getPluginInfo(any())).thenReturn(new PluginInfo("unused").setVersion(Version.create("1.0"))); PageRepository pageRepository = new PageRepository(pluginRepository, new PageDefinition[] {context -> { for (Page page : pages) { context.addPage(page); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalActionTest.java index c09bdcea458..adc91a98de5 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalActionTest.java @@ -28,6 +28,7 @@ import org.sonar.api.resources.ResourceTypeTree; import org.sonar.api.resources.ResourceTypes; import org.sonar.api.web.page.Page; import org.sonar.api.web.page.PageDefinition; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import org.sonar.db.DbClient; import org.sonar.db.dialect.H2; @@ -40,9 +41,10 @@ import org.sonar.server.platform.WebServer; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ui.PageRepository; import org.sonar.server.ws.WsActionTester; +import org.sonar.updatecenter.common.Version; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -283,7 +285,8 @@ public class GlobalActionTest { when(dbClient.getDatabase().getDialect()).thenReturn(new H2()); when(server.getVersion()).thenReturn("6.42"); PluginRepository pluginRepository = mock(PluginRepository.class); - when(pluginRepository.hasPlugin(anyString())).thenReturn(true); + when(pluginRepository.hasPlugin(any())).thenReturn(true); + when(pluginRepository.getPluginInfo(any())).thenReturn(new PluginInfo("unused").setVersion(Version.create("1.0"))); PageRepository pageRepository = new PageRepository(pluginRepository, new PageDefinition[] {context -> { for (Page page : pages) { context.addPage(page); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/OrganizationActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/OrganizationActionTest.java index 44d67fb5edd..986475ca37b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/OrganizationActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/OrganizationActionTest.java @@ -28,6 +28,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; import org.sonar.api.web.page.Page; import org.sonar.api.web.page.PageDefinition; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import org.sonar.db.DbClient; import org.sonar.db.DbTester; @@ -41,11 +42,11 @@ import org.sonar.server.ui.PageRepository; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsActionTester; +import org.sonar.updatecenter.common.Version; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.web.page.Page.Scope.ORGANIZATION; @@ -245,7 +246,8 @@ public class OrganizationActionTest { private void initWithPages(Page... pages) { PluginRepository pluginRepository = mock(PluginRepository.class); - when(pluginRepository.hasPlugin(anyString())).thenReturn(true); + when(pluginRepository.hasPlugin(any())).thenReturn(true); + when(pluginRepository.getPluginInfo(any())).thenReturn(new PluginInfo("unused").setVersion(Version.create("1.0"))); PageRepository pageRepository = new PageRepository(pluginRepository, new PageDefinition[] {context -> { for (Page page : pages) { context.addPage(page); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/SettingsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/SettingsActionTest.java index 4cc6e07eb34..2b4034c6b24 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/SettingsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/SettingsActionTest.java @@ -24,13 +24,14 @@ import org.junit.Test; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.web.page.Page; import org.sonar.api.web.page.PageDefinition; +import org.sonar.core.platform.PluginInfo; import org.sonar.core.platform.PluginRepository; import org.sonar.process.ProcessProperties; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ui.PageRepository; import org.sonar.server.ws.WsActionTester; -import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.test.JsonAssert.assertJson; @@ -80,7 +81,8 @@ public class SettingsActionTest { private void init(Page... pages) { PluginRepository pluginRepository = mock(PluginRepository.class); - when(pluginRepository.hasPlugin(anyString())).thenReturn(true); + when(pluginRepository.hasPlugin(any())).thenReturn(true); + when(pluginRepository.getPluginInfo(any())).thenReturn(new PluginInfo("unused")); PageRepository pageRepository = new PageRepository(pluginRepository, new PageDefinition[] {context -> { for (Page page : pages) { context.addPage(page); diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginRepository.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginRepository.java index 6c26e37c640..7d38a6730a3 100644 --- a/sonar-core/src/main/java/org/sonar/core/platform/PluginRepository.java +++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginRepository.java @@ -35,6 +35,9 @@ public interface PluginRepository { Collection getPluginInfos(); + /** + * @throws IllegalArgumentException if the plugin does not exist + */ PluginInfo getPluginInfo(String key); /** diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/web/page/Page.java b/sonar-plugin-api/src/main/java/org/sonar/api/web/page/Page.java index 6b684105856..b6be3dd8047 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/web/page/Page.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/web/page/Page.java @@ -58,6 +58,10 @@ public final class Page { return key; } + public String getPluginKey() { + return key.substring(0, key.indexOf('/')); + } + public String getName() { return name; } @@ -115,6 +119,10 @@ public final class Page { * @param key It must respect the format plugin_key/page_identifier. Example: my_plugin/my_page */ private Builder(String key) { + requireNonNull(key, "Key must not be null"); + if (key.split("/").length != 2) { + throw new IllegalArgumentException("Page key [" + key + "] is not valid. It must contain a single slash, for example my_plugin/my_page."); + } this.key = requireNonNull(key, "Key must not be null"); } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/web/page/ContextTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/web/page/ContextTest.java index 651f0a6323c..5ee17718279 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/web/page/ContextTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/web/page/ContextTest.java @@ -48,17 +48,17 @@ public class ContextTest { @Test public void ordered_by_name() { underTest - .addPage(Page.builder("K1").setName("N2").build()) - .addPage(Page.builder("K2").setName("N3").build()) - .addPage(Page.builder("K3").setName("N1").build()); + .addPage(Page.builder("fake/K1").setName("N2").build()) + .addPage(Page.builder("fake/K2").setName("N3").build()) + .addPage(Page.builder("fake/K3").setName("N1").build()); Collection result = underTest.getPages(); assertThat(result).extracting(Page::getKey, Page::getName) .containsOnly( - tuple("K3", "N1"), - tuple("K1", "N2"), - tuple("K2", "N3")); + tuple("fake/K3", "N1"), + tuple("fake/K1", "N2"), + tuple("fake/K2", "N3")); } @Test diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/web/page/PageTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/web/page/PageTest.java index 54b2f54ef2e..efc9308d717 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/web/page/PageTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/web/page/PageTest.java @@ -49,6 +49,7 @@ public class PageTest { .build(); assertThat(result.getKey()).isEqualTo("governance/project_dump"); + assertThat(result.getPluginKey()).isEqualTo("governance"); assertThat(result.getName()).isEqualTo("Project Dump"); assertThat(result.getComponentQualifiers()).containsOnly(PROJECT, MODULE); assertThat(result.getScope()).isEqualTo(COMPONENT); @@ -134,4 +135,20 @@ public class PageTest { underTest.setComponentQualifiers(PROJECT).build(); } + + @Test + public void fail_if_key_does_not_contain_a_slash() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Page key [project_dump] is not valid. It must contain a single slash, for example my_plugin/my_page."); + + Page.builder("project_dump").setName("Project Dump").build(); + } + + @Test + public void fail_if_key_contains_more_than_one_slash() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Page key [governance/project/dump] is not valid. It must contain a single slash, for example my_plugin/my_page."); + + Page.builder("governance/project/dump").setName("Project Dump").build(); + } } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ActiveRulesPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ActiveRulesPublisherTest.java index 8b40d9afa0b..0849c44f051 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ActiveRulesPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ActiveRulesPublisherTest.java @@ -20,7 +20,6 @@ package org.sonar.scanner.report; import java.io.File; -import java.util.Arrays; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -35,6 +34,7 @@ import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReportReader; import org.sonar.scanner.protocol.output.ScannerReportWriter; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; public class ActiveRulesPublisherTest { @@ -48,7 +48,7 @@ public class ActiveRulesPublisherTest { ScannerReportWriter writer = new ScannerReportWriter(outputDir); NewActiveRule ar = new ActiveRulesBuilder().create(RuleKey.of("java", "S001")).setSeverity("BLOCKER").setParam("p1", "v1"); - ActiveRules activeRules = new DefaultActiveRules(Arrays.asList(ar)); + ActiveRules activeRules = new DefaultActiveRules(singletonList(ar)); ActiveRulesPublisher underTest = new ActiveRulesPublisher(activeRules); underTest.publish(writer); @@ -59,9 +59,9 @@ public class ActiveRulesPublisherTest { assertThat(reportAr.getRuleRepository()).isEqualTo("java"); assertThat(reportAr.getRuleKey()).isEqualTo("S001"); assertThat(reportAr.getSeverity()).isEqualTo(Constants.Severity.BLOCKER); - assertThat(reportAr.getParamsByKey()).hasSize(1); - assertThat(reportAr.getParamsByKey().entrySet().iterator().next().getKey()).isEqualTo("p1"); - assertThat(reportAr.getParamsByKey().entrySet().iterator().next().getValue()).isEqualTo("v1"); + assertThat(reportAr.getParamsByKeyMap()).hasSize(1); + assertThat(reportAr.getParamsByKeyMap().entrySet().iterator().next().getKey()).isEqualTo("p1"); + assertThat(reportAr.getParamsByKeyMap().entrySet().iterator().next().getValue()).isEqualTo("v1"); assertThat(readIt.hasNext()).isFalse(); }