]> source.dussan.org Git - sonarqube.git/commitdiff
Fix Quality flaws
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Wed, 18 Apr 2018 19:33:48 +0000 (21:33 +0200)
committerSonarTech <sonartech@sonarsource.com>
Thu, 10 May 2018 18:20:53 +0000 (20:20 +0200)
21 files changed:
server/sonar-db-dao/src/test/java/org/sonar/db/ce/LogsIteratorInputStreamTest.java
server/sonar-main/src/main/java/org/sonar/application/command/EsJvmOptions.java
server/sonar-main/src/main/java/org/sonar/application/es/EsYmlSettings.java
server/sonar-server/build.gradle
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadQualityProfilesStep.java
server/sonar-server/src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java
server/sonar-server/src/main/java/org/sonar/server/ui/PageRepository.java
server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalAction.java
server/sonar-server/src/main/java/org/sonar/server/ui/ws/OrganizationAction.java
server/sonar-server/src/main/resources/org/sonar/server/ui/ws/settings-example.json
server/sonar-server/src/test/java/org/sonar/server/plugins/StaticResourcesServletTest.java
server/sonar-server/src/test/java/org/sonar/server/ui/PageRepositoryTest.java
server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java
server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalActionTest.java
server/sonar-server/src/test/java/org/sonar/server/ui/ws/OrganizationActionTest.java
server/sonar-server/src/test/java/org/sonar/server/ui/ws/SettingsActionTest.java
sonar-core/src/main/java/org/sonar/core/platform/PluginRepository.java
sonar-plugin-api/src/main/java/org/sonar/api/web/page/Page.java
sonar-plugin-api/src/test/java/org/sonar/api/web/page/ContextTest.java
sonar-plugin-api/src/test/java/org/sonar/api/web/page/PageTest.java
sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ActiveRulesPublisherTest.java

index d21f9a1f4926c6a3770478f2cec8b8e93539d646..89463e56b63f2b504bc204b0569447c668bc382c 100644 (file)
@@ -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 {
index 966249fb63055f1d672c6881d021189e33432329..bc2cc930cbc3a9ffd1b17e13f097e1fad96ccf76 100644 (file)
@@ -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<EsJvmOptions> {
     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);
     }
index 8911abeb4a26eaec7891d103a01e9e1f744658dd..68412fc10fb98b63b22d75d2c77f18ccec45ce2a 100644 (file)
@@ -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);
     }
index ac661c36f38fc7a640b82f8beb7f5e7aa84cbc6a..78bf82df66261b4826eab4a676b8a97bbfeab174 100644 (file)
@@ -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'
index 4037cbb763cba175397be6637f3a50f898fbd037..88eecf3ce99466c30a8f174cf6201aae2681173d 100644 (file)
@@ -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<String, String> params = new HashMap<>(input.getParamsByKey());
+    Map<String, String> params = new HashMap<>(input.getParamsByKeyMap());
     return new ActiveRule(key, input.getSeverity().name(), params, input.getCreatedAt(), rule.getPluginKey());
   }
 }
index 3f2da92f6ae2df9e8f86d2d48584ae5c8038e46c..3e5ad22e85bca1ff878b9d8b98f2bcef810ec279 100644 (file)
@@ -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);
+    }
+  }
 }
index 9635d587456163ce5fec69be914cb0c5fe1c5286..0929a36d4380564851262382a72b5c36f85c01f0 100644 (file)
@@ -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<PageDefinition> definitions;
   private List<Page> 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<Page> 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<Page> 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);
     };
   }
 
index ababbeb65e537492ec005a8c6742d068b7815d6f..19baa4411087a9261439f307973e7fe681d425a7 100644 (file)
@@ -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())
index 8542426fb8e61ab6858c0fb527d5103df0e7b1f4..ca03a893042859f6592b40b92997fa189a54ac34 100644 (file)
@@ -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<Page> 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<Page> writePage(JsonWriter json) {
-    return page -> json.beginObject()
-      .prop("key", page.getKey())
-      .prop("name", page.getName())
-      .endObject();
-  }
 }
index 383b5e0cc5a8a159041eeaacb7fcc2b8eb95f79f..bd5bb3de19db3ca4751dd1f2bfd868e947b28355 100644 (file)
@@ -2,8 +2,8 @@
   "showUpdateCenter": true,
   "extensions": [
     {
-      "name": "Views",
-      "url": "/plugins/configuration/views"
+      "name": "Sample",
+      "url": "/plugins/configuration/sample"
     }
   ]
 }
index fab7ebee9cf20498be9ba8b49ff437da05435c0b..fad36be82144d4e8c4e4e5a6ed21c76826a720db 100644 (file)
 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);
+      }
+    }
   }
 }
index df25db5fa4aee2995d9537b51453d927cb50e43f..89165d859903d56013182adb030148b1e5cefd16 100644 (file)
@@ -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<Page> 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<Page> 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<Page> 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<Page> 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<Page> 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");
index ec18a37b36d59f11b7013c2d7c59c0d97d8a3691..7ddcd2b32c061191158562b299bc1cec5165604b 100644 (file)
@@ -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);
index c09bdcea4587ba12498ed77b9b4a3b01fdb89171..adc91a98de50638b3ea8ad3707a4bf809b22f59b 100644 (file)
@@ -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);
index 44d67fb5eddc7b6b3d29eb5c6742b2832e928369..986475ca37bad01e14beaf4306aa8805b9bd35a4 100644 (file)
@@ -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);
index 4cc6e07eb3429c32739f8d4f8408c5932d989197..2b4034c6b246d62cd19a33c3d56db4c1148ce595 100644 (file)
@@ -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);
index 6c26e37c640370f95981bd7cf680e0836cbb8053..7d38a6730a341c3ea1e162b67a65b38be36cbe44 100644 (file)
@@ -35,6 +35,9 @@ public interface PluginRepository {
 
   Collection<PluginInfo> getPluginInfos();
 
+  /**
+   * @throws IllegalArgumentException if the plugin does not exist
+   */
   PluginInfo getPluginInfo(String key);
 
   /**
index 6b6841058562c9437c64dc92e3c418b7b043d2a8..b6be3dd80472f0d720ec3acc5b2a1f086adb2f78 100644 (file)
@@ -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: <code>my_plugin/my_page</code>
      */
     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");
     }
 
index 651f0a6323cf85350058b7394ae79558ddc80a40..5ee17718279217450140a1aacc8076c06b6baa98 100644 (file)
@@ -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<Page> 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
index 54b2f54ef2e6c4625362ee341bda791b5ea3fa81..efc9308d717bbb9ed5c0bd8686ceed791e48e9aa 100644 (file)
@@ -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();
+  }
 }
index 8b40d9afa0bb386aae0880f720e5ead9da2eb9c8..0849c44f0519c66dbb40aab915a7df935b362c0d 100644 (file)
@@ -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();
     }