SONAR-7748 Implement API Server.getPublicRootUrl() on scanner side

4 files changed, 46 insertions, 52 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/platform/DefaultServer.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/platform/DefaultServer.java
index b747438a9f5..60ad445afa7 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/batch/platform/DefaultServer.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/platform/DefaultServer.java
@@ -30,17 +30,19 @@ import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.BatchSide;
 import org.sonar.api.config.Settings;
 import org.sonar.api.platform.Server;
-import org.sonar.batch.bootstrap.GlobalProperties;
+import org.sonar.batch.bootstrap.BatchWsClient;
+
+import static org.apache.commons.lang.StringUtils.trimToEmpty;
 
 @BatchSide
 public class DefaultServer extends Server {
 
   private Settings settings;
-  private GlobalProperties props;
+  private BatchWsClient client;
 
-  public DefaultServer(Settings settings, GlobalProperties props) {
+  public DefaultServer(Settings settings, BatchWsClient client) {
     this.settings = settings;
-    this.props = props;
+    this.client = client;
   }
 
   @Override
@@ -85,7 +87,12 @@ public class DefaultServer extends Server {
 
   @Override
   public String getPublicRootUrl() {
-    return null;
+    String baseUrl = trimToEmpty(settings.getString(CoreProperties.SERVER_BASE_URL));
+    if (baseUrl.isEmpty()) {
+      // If server base URL was not configured in Sonar server then is is better to take URL configured on batch side
+      baseUrl = client.baseUrl();
+    }
+    return StringUtils.removeEnd(baseUrl, "/");
   }
 
   @Override
@@ -100,7 +107,7 @@ public class DefaultServer extends Server {
 
   @Override
   public String getURL() {
-    return StringUtils.removeEnd(StringUtils.defaultIfBlank(props.property("sonar.host.url"), "http://localhost:9000"), "/");
+    return StringUtils.removeEnd(client.baseUrl(), "/");
   }
 
   @Override
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/report/ReportPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/report/ReportPublisher.java
index 2539aa065ee..b47217b8859 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/batch/report/ReportPublisher.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/report/ReportPublisher.java
@@ -34,10 +34,10 @@ import java.util.Map;
 import javax.annotation.Nullable;
 import org.apache.commons.io.FileUtils;
 import org.picocontainer.Startable;
-import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.BatchSide;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.config.Settings;
+import org.sonar.api.platform.Server;
 import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.TempFolder;
 import org.sonar.api.utils.ZipUtils;
@@ -52,7 +52,6 @@ import org.sonarqube.ws.WsCe;
 import org.sonarqube.ws.client.PostRequest;
 import org.sonarqube.ws.client.WsResponse;
 
-import static org.apache.commons.lang.StringUtils.trimToEmpty;
 import static org.sonar.core.util.FileUtils.deleteQuietly;
 
 @BatchSide
@@ -71,14 +70,16 @@ public class ReportPublisher implements Startable {
   private final DefaultAnalysisMode analysisMode;
   private final TempFolder temp;
   private final ReportPublisherStep[] publishers;
+  private final Server server;
 
   private File reportDir;
   private ScannerReportWriter writer;
 
-  public ReportPublisher(Settings settings, BatchWsClient wsClient, AnalysisContextReportPublisher contextPublisher,
+  public ReportPublisher(Settings settings, BatchWsClient wsClient, Server server, AnalysisContextReportPublisher contextPublisher,
     ImmutableProjectReactor projectReactor, DefaultAnalysisMode analysisMode, TempFolder temp, ReportPublisherStep[] publishers) {
     this.settings = settings;
     this.wsClient = wsClient;
+    this.server = server;
     this.contextPublisher = contextPublisher;
     this.projectReactor = projectReactor;
     this.analysisMode = analysisMode;
@@ -93,7 +94,7 @@ public class ReportPublisher implements Startable {
     contextPublisher.init(writer);
 
     if (!analysisMode.isIssues() && !analysisMode.isMediumTest()) {
-      String publicUrl = publicUrl();
+      String publicUrl = server.getPublicRootUrl();
       if (HttpUrl.parse(publicUrl) == null) {
         throw MessageException.of("Failed to parse public URL set in SonarQube server: " + publicUrl);
       }
@@ -185,7 +186,7 @@ public class ReportPublisher implements Startable {
     if (taskId == null) {
       LOG.info("ANALYSIS SUCCESSFUL");
     } else {
-      String publicUrl = publicUrl();
+      String publicUrl = server.getPublicRootUrl();
       HttpUrl httpUrl = HttpUrl.parse(publicUrl);
 
       Map<String, String> metadata = new LinkedHashMap<>();
@@ -230,17 +231,4 @@ public class ReportPublisher implements Startable {
       throw new IllegalStateException("Unable to dump " + file, e);
     }
   }
-
-  /**
-   * The public URL is optionally configured on server. If not, then the regular URL is returned.
-   * See https://jira.sonarsource.com/browse/SONAR-4239
-   */
-  private String publicUrl() {
-    String baseUrl = trimToEmpty(settings.getString(CoreProperties.SERVER_BASE_URL));
-    if (baseUrl.isEmpty()) {
-      // If server base URL was not configured in Sonar server then is is better to take URL configured on batch side
-      baseUrl = wsClient.baseUrl();
-    }
-    return baseUrl.replaceAll("(/)+$", "");
-  }
 }
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/batch/platform/DefaultServerTest.java b/sonar-scanner-engine/src/test/java/org/sonar/batch/platform/DefaultServerTest.java
index 801848354fc..c3df69af094 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/batch/platform/DefaultServerTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/batch/platform/DefaultServerTest.java
@@ -19,11 +19,11 @@
  */
 package org.sonar.batch.platform;
 
-import org.sonar.batch.bootstrap.GlobalProperties;
-
 import org.junit.Test;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
+import org.sonar.batch.bootstrap.BatchWsClient;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -37,10 +37,10 @@ public class DefaultServerTest {
     settings.setProperty(CoreProperties.SERVER_VERSION, "2.2");
     settings.setProperty(CoreProperties.SERVER_STARTTIME, "2010-05-18T17:59:00+0000");
     settings.setProperty(CoreProperties.PERMANENT_SERVER_ID, "abcde");
-    GlobalProperties props = mock(GlobalProperties.class);
-    when(props.property("sonar.host.url")).thenReturn("http://foo.com");
+    BatchWsClient client = mock(BatchWsClient.class);
+    when(client.baseUrl()).thenReturn("http://foo.com");
 
-    DefaultServer metadata = new DefaultServer(settings, props);
+    DefaultServer metadata = new DefaultServer(settings, client);
 
     assertThat(metadata.getId()).isEqualTo("123");
     assertThat(metadata.getVersion()).isEqualTo("2.2");
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/batch/report/ReportPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
index bb54bfb9113..f48e7f05717 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
@@ -29,10 +29,10 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.Mockito;
-import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.config.Settings;
+import org.sonar.api.platform.Server;
 import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.TempFolder;
 import org.sonar.api.utils.log.LogTester;
@@ -54,13 +54,14 @@ public class ReportPublisherTest {
 
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
-  
+
   @Rule
   public ExpectedException exception = ExpectedException.none();
 
   DefaultAnalysisMode mode = mock(DefaultAnalysisMode.class);
   Settings settings = new Settings(new PropertyDefinitions(CorePropertyDefinitions.all()));
   BatchWsClient wsClient = mock(BatchWsClient.class, Mockito.RETURNS_DEEP_STUBS);
+  Server server = mock(Server.class);
   ImmutableProjectReactor reactor = mock(ImmutableProjectReactor.class);
   ProjectDefinition root;
   AnalysisContextReportPublisher contextPublisher = mock(AnalysisContextReportPublisher.class);
@@ -69,12 +70,12 @@ public class ReportPublisherTest {
   public void setUp() {
     root = ProjectDefinition.create().setKey("struts").setWorkDir(temp.getRoot());
     when(reactor.getRoot()).thenReturn(root);
-    when(wsClient.baseUrl()).thenReturn("https://localhost/");
+    when(server.getPublicRootUrl()).thenReturn("https://localhost");
   }
 
   @Test
   public void log_and_dump_information_about_report_uploading() throws IOException {
-    ReportPublisher underTest = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+    ReportPublisher underTest = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
 
     underTest.logSuccess("TASK-123");
 
@@ -86,17 +87,16 @@ public class ReportPublisherTest {
     File detailsFile = new File(temp.getRoot(), "report-task.txt");
     assertThat(readFileToString(detailsFile)).isEqualTo(
       "projectKey=struts\n" +
-      "serverUrl=https://localhost\n" +
-      "dashboardUrl=https://localhost/dashboard/index/struts\n" +
-      "ceTaskId=TASK-123\n" +
-      "ceTaskUrl=https://localhost/api/ce/task?id=TASK-123\n"
-      );
+        "serverUrl=https://localhost\n" +
+        "dashboardUrl=https://localhost/dashboard/index/struts\n" +
+        "ceTaskId=TASK-123\n" +
+        "ceTaskUrl=https://localhost/api/ce/task?id=TASK-123\n");
   }
 
   @Test
   public void log_public_url_if_defined() throws IOException {
-    settings.setProperty(CoreProperties.SERVER_BASE_URL, "https://publicserver/sonarqube");
-    ReportPublisher underTest = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+    when(server.getPublicRootUrl()).thenReturn("https://publicserver/sonarqube");
+    ReportPublisher underTest = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
 
     underTest.logSuccess("TASK-123");
 
@@ -107,18 +107,17 @@ public class ReportPublisherTest {
     File detailsFile = new File(temp.getRoot(), "report-task.txt");
     assertThat(readFileToString(detailsFile)).isEqualTo(
       "projectKey=struts\n" +
-      "serverUrl=https://publicserver/sonarqube\n" +
-      "dashboardUrl=https://publicserver/sonarqube/dashboard/index/struts\n" +
-      "ceTaskId=TASK-123\n" +
-      "ceTaskUrl=https://publicserver/sonarqube/api/ce/task?id=TASK-123\n"
-    );
+        "serverUrl=https://publicserver/sonarqube\n" +
+        "dashboardUrl=https://publicserver/sonarqube/dashboard/index/struts\n" +
+        "ceTaskId=TASK-123\n" +
+        "ceTaskUrl=https://publicserver/sonarqube/api/ce/task?id=TASK-123\n");
   }
-  
+
   @Test
   public void fail_if_public_url_malformed() throws IOException {
-    settings.setProperty(CoreProperties.SERVER_BASE_URL, "invalid");
-    ReportPublisher underTest = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
-    
+    when(server.getPublicRootUrl()).thenReturn("invalid");
+    ReportPublisher underTest = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+
     exception.expect(MessageException.class);
     exception.expectMessage("Failed to parse public URL set in SonarQube server: invalid");
     underTest.start();
@@ -126,7 +125,7 @@ public class ReportPublisherTest {
 
   @Test
   public void log_but_not_dump_information_when_report_is_not_uploaded() {
-    ReportPublisher underTest = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+    ReportPublisher underTest = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
 
     underTest.logSuccess(/* report not uploaded, no server task */null);
 
@@ -143,7 +142,7 @@ public class ReportPublisherTest {
     settings.setProperty("sonar.batch.keepReport", true);
     Path reportDir = temp.getRoot().toPath().resolve("batch-report");
     Files.createDirectory(reportDir);
-    ReportPublisher underTest = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+    ReportPublisher underTest = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
 
     underTest.start();
     underTest.stop();
@@ -154,7 +153,7 @@ public class ReportPublisherTest {
   public void should_delete_report_by_default() throws IOException {
     Path reportDir = temp.getRoot().toPath().resolve("batch-report");
     Files.createDirectory(reportDir);
-    ReportPublisher job = new ReportPublisher(settings, wsClient, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
+    ReportPublisher job = new ReportPublisher(settings, wsClient, server, contextPublisher, reactor, mode, mock(TempFolder.class), new ReportPublisherStep[0]);
 
     job.start();
     job.stop();