From 18a077aac51e118a93ccc796dd96264ee19e862e Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 2 Dec 2015 13:42:47 +0100 Subject: [PATCH] Fix logging of public url --- .../sonar/batch/bootstrap/BatchWsClient.java | 19 +---------- .../bootstrap/BatchWsClientProvider.java | 2 +- .../sonar/batch/report/ReportPublisher.java | 17 ++++++++-- .../bootstrap/BatchWsClientProviderTest.java | 1 - .../batch/bootstrap/BatchWsClientTest.java | 34 +++---------------- .../batch/report/ReportPublisherTest.java | 30 +++++++++++++--- 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 @@ -47,32 +47,6 @@ 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(); @@ -80,7 +54,7 @@ public class BatchWsClientTest { 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,20 +73,41 @@ 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]); -- 2.39.5