Fix logging of public url

6 files changed, 46 insertions, 57 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClient.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClient.java
index 16bac82b84f..cea28187c9a 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClient.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClient.java
@@ -28,8 +28,6 @@ import com.google.gson.JsonParser;
 import java.net.HttpURLConnection;
 import java.util.ArrayList;
 import java.util.List;
-import javax.annotation.Nullable;
-import org.apache.commons.lang.StringUtils;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.utils.MessageException;
 import org.sonar.api.utils.log.Logger;
@@ -49,16 +47,10 @@ public class BatchWsClient {
 
   private final WsClient target;
   private final boolean hasCredentials;
-  private final String publicBaseUrl;
 
-  public BatchWsClient(WsClient target, boolean hasCredentials, @Nullable String publicBaseUrl) {
+  public BatchWsClient(WsClient target, boolean hasCredentials) {
     this.target = target;
     this.hasCredentials = hasCredentials;
-    if (StringUtils.isBlank(publicBaseUrl)) {
-      this.publicBaseUrl = target.wsConnector().baseUrl();
-    } else {
-      this.publicBaseUrl = publicBaseUrl.replaceAll("(/)+$", "") + "/";
-    }
   }
 
   /**
@@ -80,15 +72,6 @@ public class BatchWsClient {
     return target.wsConnector().baseUrl();
   }
 
-  /**
-   * The public URL is optionally configured on server. If not, then the regular {@link #baseUrl()} is returned.
-   * URL has a trailing slash.
-   * See https://jira.sonarsource.com/browse/SONAR-4239
-   */
-  public String publicBaseUrl() {
-    return publicBaseUrl;
-  }
-
   @VisibleForTesting
   WsConnector wsConnector() {
     return target.wsConnector();
diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClientProvider.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClientProvider.java
index a8ac402da35..dd922755930 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClientProvider.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/BatchWsClientProvider.java
@@ -55,7 +55,7 @@ public class BatchWsClientProvider extends ProviderAdapter {
         .url(url)
         .credentials(login, settings.property(CoreProperties.PASSWORD));
 
-      wsClient = new BatchWsClient(new HttpWsClient(builder.build()), login != null, settings.property(CoreProperties.SERVER_BASE_URL));
+      wsClient = new BatchWsClient(new HttpWsClient(builder.build()), login != null);
     }
     return wsClient;
   }
diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java b/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java
index 524a9a034f5..6f96639d2bd 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java
@@ -33,6 +33,7 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang.StringUtils;
 import org.picocontainer.Startable;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.BatchSide;
@@ -176,13 +177,13 @@ public class ReportPublisher implements Startable {
       String effectiveKey = projectReactor.getRoot().getKeyWithBranch();
       metadata.put("projectKey", effectiveKey);
 
-      URL dashboardUrl = HttpUrl.parse(wsClient.publicBaseUrl()).newBuilder()
+      URL dashboardUrl = HttpUrl.parse(publicUrl()).newBuilder()
         .addPathSegment("dashboard").addPathSegment("index").addPathSegment(effectiveKey)
         .build()
         .url();
       metadata.put("dashboardUrl", dashboardUrl.toExternalForm());
 
-      URL taskUrl = HttpUrl.parse(wsClient.publicBaseUrl()).newBuilder()
+      URL taskUrl = HttpUrl.parse(publicUrl()).newBuilder()
         .addPathSegment("api").addPathSegment("ce").addPathSegment("task")
         .addQueryParameter("id", taskId)
         .build()
@@ -213,4 +214,16 @@ 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 publicUrl = settings.getString(CoreProperties.SERVER_BASE_URL);
+    if (StringUtils.isBlank(publicUrl)) {
+      return wsClient.baseUrl();
+    }
+    return publicUrl.replaceAll("(/)+$", "") + "/";
+  }
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientProviderTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientProviderTest.java
index 3b19135fde0..9a741392f42 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientProviderTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientProviderTest.java
@@ -40,7 +40,6 @@ public class BatchWsClientProviderTest {
 
     assertThat(client).isNotNull();
     assertThat(client.baseUrl()).isEqualTo("http://localhost:9000/");
-    assertThat(client.publicBaseUrl()).isEqualTo("http://localhost:9000/");
     HttpConnector httpConnector = (HttpConnector) client.wsConnector();
     assertThat(httpConnector.baseUrl()).isEqualTo("http://localhost:9000/");
     assertThat(httpConnector.okHttpClient().getProxy()).isNull();
diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientTest.java
index f7b9a216b7b..ecee9fb06e1 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/BatchWsClientTest.java
@@ -48,39 +48,13 @@ public class BatchWsClientTest {
   WsClient wsClient = mock(WsClient.class, Mockito.RETURNS_DEEP_STUBS);
 
   @Test
-  public void define_public_url() {
-    when(wsClient.wsConnector().baseUrl()).thenReturn("https://local/");
-    BatchWsClient underTest = new BatchWsClient(wsClient, true, "https://public/");
-    assertThat(underTest.baseUrl()).isEqualTo("https://local/");
-    assertThat(underTest.publicBaseUrl()).isEqualTo("https://public/");
-  }
-
-  /**
-   * Returned URL has trailing slash, even if configured URL doesn't have.
-   * That's useful for {@link com.squareup.okhttp.HttpUrl}
-   */
-  @Test
-  public void public_url_has_trailing_slash() {
-    BatchWsClient underTest = new BatchWsClient(wsClient, true, "https://public");
-    assertThat(underTest.publicBaseUrl()).isEqualTo("https://public/");
-  }
-
-
-  @Test
-  public void public_url_is_the_base_url_by_default() {
-    when(wsClient.wsConnector().baseUrl()).thenReturn("https://local/");
-    BatchWsClient underTest = new BatchWsClient(wsClient, true, null);
-    assertThat(underTest.publicBaseUrl()).isEqualTo("https://local/");
-  }
-
-  @Test
   public void log_and_profile_request_if_debug_level() throws Exception {
     WsRequest request = newRequest();
     WsResponse response = newResponse().setRequestUrl("https://local/api/issues/search");
     when(wsClient.wsConnector().call(request)).thenReturn(response);
 
     logTester.setLevel(LoggerLevel.DEBUG);
-    BatchWsClient underTest = new BatchWsClient(wsClient, false, null);
+    BatchWsClient underTest = new BatchWsClient(wsClient, false);
 
     WsResponse result = underTest.call(request);
 
@@ -103,7 +77,7 @@ public class BatchWsClientTest {
     WsResponse response = newResponse().setCode(401);
     when(wsClient.wsConnector().call(request)).thenReturn(response);
 
-    new BatchWsClient(wsClient, false, null).call(request);
+    new BatchWsClient(wsClient, false).call(request);
   }
 
   @Test
@@ -115,7 +89,7 @@ public class BatchWsClientTest {
     WsResponse response = newResponse().setCode(401);
     when(wsClient.wsConnector().call(request)).thenReturn(response);
 
-    new BatchWsClient(wsClient, /* credentials are configured */true, null).call(request);
+    new BatchWsClient(wsClient, /* credentials are configured */true).call(request);
   }
 
   @Test
@@ -129,7 +103,7 @@ public class BatchWsClientTest {
       .setContent("{\"errors\":[{\"msg\":\"missing scan permission\"}, {\"msg\":\"missing another permission\"}]}");
     when(wsClient.wsConnector().call(request)).thenReturn(response);
 
-    new BatchWsClient(wsClient, true, null).call(request);
+    new BatchWsClient(wsClient, true).call(request);
   }
 
   private MockWsResponse newResponse() {
diff --git a/sonar-batch/src/test/java/org/sonar/batch/report/ReportPublisherTest.java b/sonar-batch/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
index dce14a13b8e..ebc2e97735f 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/report/ReportPublisherTest.java
@@ -64,7 +64,6 @@ public class ReportPublisherTest {
     root = ProjectDefinition.create().setKey("struts").setWorkDir(temp.getRoot());
     when(reactor.getRoot()).thenReturn(root);
     when(wsClient.baseUrl()).thenReturn("https://localhost/");
-    when(wsClient.publicBaseUrl()).thenReturn("https://public/");
   }
 
   @Test
@@ -74,21 +73,42 @@ public class ReportPublisherTest {
     underTest.logSuccess("TASK-123");
 
     assertThat(logTester.logs(LoggerLevel.INFO))
-      .contains("ANALYSIS SUCCESSFUL, you can browse https://public/dashboard/index/struts")
+      .contains("ANALYSIS SUCCESSFUL, you can browse https://localhost/dashboard/index/struts")
       .contains("Note that you will be able to access the updated dashboard once the server has processed the submitted analysis report")
-      .contains("More about the report processing at https://public/api/ce/task?id=TASK-123");
+      .contains("More about the report processing at https://localhost/api/ce/task?id=TASK-123");
 
     File detailsFile = new File(temp.getRoot(), "analysis-details.json");
     JsonAssert.assertJson(readFileToString(detailsFile)).isSimilarTo("{" +
       "\"projectKey\": \"struts\"," +
-      "\"dashboardUrl\": \"https://public/dashboard/index/struts\"," +
+      "\"dashboardUrl\": \"https://localhost/dashboard/index/struts\"," +
       "\"ceTaskId\": \"TASK-123\"," +
-      "\"ceTaskUrl\": \"https://public/api/ce/task?id=TASK-123\"" +
+      "\"ceTaskUrl\": \"https://localhost/api/ce/task?id=TASK-123\"" +
       "}"
       );
   }
 
   @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]);
+
+    underTest.logSuccess("TASK-123");
+
+    assertThat(logTester.logs(LoggerLevel.INFO))
+      .contains("ANALYSIS SUCCESSFUL, you can browse https://publicserver/sonarqube/dashboard/index/struts")
+      .contains("More about the report processing at https://publicserver/sonarqube/api/ce/task?id=TASK-123");
+
+    File detailsFile = new File(temp.getRoot(), "analysis-details.json");
+    JsonAssert.assertJson(readFileToString(detailsFile)).isSimilarTo("{" +
+      "\"projectKey\": \"struts\"," +
+      "\"dashboardUrl\": \"https://publicserver/sonarqube/dashboard/index/struts\"," +
+      "\"ceTaskId\": \"TASK-123\"," +
+      "\"ceTaskUrl\": \"https://publicserver/sonarqube/api/ce/task?id=TASK-123\"" +
+      "}"
+    );
+  }
+
+  @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]);