Merge pull request #135 from TouK/optional_api_key

Optional api key and build id param

src/main/java/pl/touk/sputnik/configuration/CliOption.java

@@ -10,7 +10,8 @@ public enum CliOption implements ConfigurationOption {
     CHANGE_ID("cli.changeId", "Gerrit change id", null),
     REVISION_ID("cli.revisionId", "Gerrit revision id", null),
     PULL_REQUEST_ID("cli.pullRequestId", "Stash pull request id", null),
-    API_KEY("cli.apiKey", "Optional API key for using Sputnik for Github", null);
+    API_KEY("cli.apiKey", "Optional API key for using Sputnik for Github", null),
+    BUILD_ID("cli.buildId", "Optional build id for using Sputnik for Github", null);
 
     private String key;
     private String description;

src/main/java/pl/touk/sputnik/configuration/CliWrapper.java

@@ -24,6 +24,7 @@ private Options createOptions() {
 
         localOptions.addOption(buildOption(CliOption.PULL_REQUEST_ID, true, false));
         localOptions.addOption(buildOption(CliOption.API_KEY, true, false));
+        localOptions.addOption(buildOption(CliOption.BUILD_ID, true, false));
 
         return localOptions;
     }

src/main/java/pl/touk/sputnik/connector/saas/SaasConnector.java

@@ -2,6 +2,7 @@
 
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
@@ -17,6 +18,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.ArrayList;
 import java.util.List;
 
 @AllArgsConstructor
@@ -26,19 +28,17 @@ public class SaasConnector implements Connector {
     private HttpConnector httpConnector;
     private GithubPatchset githubPatchset;
     private String apiKey;
+    private String buildId;
 
     private static final String API_KEY_PARAM = "key";
+    private static final String BUILD_ID_PARAM = "build_id";
     private static final String FILES_URL_FORMAT = "/api/github/%s/pulls/%d/files";
     private static final String VIOLATIONS_URL_FORMAT = "/api/github/%s/pulls/%d/violations";
 
-    public List<String> getReviewFiles() {
-        return null;
-    }
-
     @NotNull
     @Override
     public String listFiles() throws URISyntaxException, IOException {
-        URI uri = httpConnector.buildUri(createUrl(githubPatchset, FILES_URL_FORMAT), apiKeyParam());
+        URI uri = httpConnector.buildUri(createUrl(githubPatchset, FILES_URL_FORMAT), params());
         HttpGet request = new HttpGet(uri);
         CloseableHttpResponse httpResponse = httpConnector.logAndExecute(request);
         return httpConnector.consumeAndLogEntity(httpResponse);
@@ -48,7 +48,7 @@ public String listFiles() throws URISyntaxException, IOException {
     @Override
     public String sendReview(String violationsAsJson) throws URISyntaxException, IOException {
         log.info("Sending violations: {}", violationsAsJson);
-        URI uri = httpConnector.buildUri(createUrl(githubPatchset, VIOLATIONS_URL_FORMAT), apiKeyParam());
+        URI uri = httpConnector.buildUri(createUrl(githubPatchset, VIOLATIONS_URL_FORMAT), params());
         HttpPost httpPost = new HttpPost(uri);
         httpPost.setEntity(new StringEntity(violationsAsJson, ContentType.APPLICATION_JSON));
         CloseableHttpResponse httpResponse = httpConnector.logAndExecute(httpPost);
@@ -60,7 +60,14 @@ private String createUrl(GithubPatchset patchset, String formatUrl) {
     }
 
     @NotNull
-    private NameValuePair apiKeyParam() {
-        return new BasicNameValuePair(API_KEY_PARAM, apiKey);
+    private NameValuePair[] params() {
+        List<NameValuePair> params = new ArrayList<>();
+        if (StringUtils.isNotBlank(apiKey)) {
+            params.add(new BasicNameValuePair(API_KEY_PARAM, apiKey));
+        }
+        if (StringUtils.isNotBlank(buildId)) {
+            params.add(new BasicNameValuePair(BUILD_ID_PARAM, buildId));
+        }
+        return params.toArray(new NameValuePair[params.size()]);
     }
 }

src/main/java/pl/touk/sputnik/connector/saas/SaasFacadeBuilder.java

@@ -13,26 +13,22 @@
 import pl.touk.sputnik.connector.http.HttpConnector;
 import pl.touk.sputnik.connector.http.HttpHelper;
 
-import static org.apache.commons.lang3.Validate.notBlank;
-
 public class SaasFacadeBuilder {
 
     private HttpHelper httpHelper = new HttpHelper();
 
     @NotNull
     public SaasFacade build(Configuration configuration) {
         ConnectorDetails connectorDetails = new ConnectorDetails(configuration);
-
         String apiKey = configuration.getProperty(CliOption.API_KEY);
-        notBlank(apiKey, "You must provide non blank Sputnik API key");
-
+        String buildId = configuration.getProperty(CliOption.BUILD_ID);
         HttpHost httpHost = httpHelper.buildHttpHost(connectorDetails);
         HttpClientContext httpClientContext = httpHelper.buildClientContext(httpHost, new BasicScheme());
         CloseableHttpClient closeableHttpClient = httpHelper.buildClient(httpHost, connectorDetails);
 
         return new SaasFacade(new SaasConnector(
                 new HttpConnector(closeableHttpClient, httpClientContext, connectorDetails.getPath()),
-                GithubPatchsetBuilder.build(configuration), apiKey), new Gson());
+                GithubPatchsetBuilder.build(configuration), apiKey, buildId), new Gson());
     }
 
 }

src/test/java/pl/touk/sputnik/HttpConnectorEnv.java

@@ -1,5 +1,6 @@
 package pl.touk.sputnik;
 
+import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
 import com.github.tomakehurst.wiremock.client.UrlMatchingStrategy;
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpStatus;
@@ -9,13 +10,17 @@
 
 public class HttpConnectorEnv {
 
-    protected void stubGet(UrlMatchingStrategy url, String responseFile) throws Exception {
+    protected void stubGet(UrlMatchingStrategy url, ResponseDefinitionBuilder responseDefinitionBuilder) {
         stubFor(get(url)
                 .withHeader("Authorization", equalTo("Basic dXNlcjpwYXNz"))
-                .willReturn(aResponse()
+                .willReturn(responseDefinitionBuilder));
+    }
+
+    protected void stubGet(UrlMatchingStrategy url, String responseFile) throws Exception {
+        stubGet(url, aResponse()
                         .withStatus(HttpStatus.SC_OK)
                         .withHeader("Content-Type", "application/json")
-                        .withBody(IOUtils.toString(getClass().getResourceAsStream(responseFile)))));
+                        .withBody(IOUtils.toString(getClass().getResourceAsStream(responseFile))));
     }
 
     protected void stubPost(UrlMatchingStrategy url, String responseFile) throws Exception {

src/test/java/pl/touk/sputnik/connector/saas/SaasFacadeTest.java

@@ -73,7 +73,59 @@ public void shouldPublishReview() throws Exception {
     }
 
     @Test(expected = SaasException.class)
-    public void shouldHandleWrongApiKey() throws Exception {
+    public void shouldThrowOnWrongApiKey() throws Exception {
+        SaasFacade saasFacade = buildFacade(ImmutableMap.of(
+                "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(),
+                "cli.apiKey", "WRONG_API_KEY",
+                "connector.repository", SOME_REPOSITORY,
+                "connector.project", SOME_PROJECT
+        ));
+        stubGet(urlEqualTo(String.format(
+                "%s/api/github/%s/%s/pulls/%s/files?key=%s",
+                FacadeConfigUtil.PATH, SOME_PROJECT, SOME_REPOSITORY, SOME_PULL_REQUEST_ID, "WRONG_API_KEY")),
+                aResponse().withStatus(403));
+
         saasFacade.listFiles();
     }
+
+    @Test
+    public void shouldHandleEmptyApiKey() throws Exception {
+        SaasFacade saasFacade = buildFacade(ImmutableMap.of(
+                "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(),
+                "connector.repository", SOME_REPOSITORY,
+                "connector.project", SOME_PROJECT
+        ));
+
+        stubGet(urlEqualTo(String.format(
+                "%s/api/github/%s/%s/pulls/%s/files",
+                FacadeConfigUtil.PATH, SOME_PROJECT, SOME_REPOSITORY, SOME_PULL_REQUEST_ID)), "/json/saas-files.json");
+
+        List<ReviewFile> files = saasFacade.listFiles();
+
+        assertThat(files).extracting("reviewFilename").containsOnly("src/main/java/TestFile.java", "src/main/java/TestFile2.java");
+    }
+
+    @Test
+    public void shouldSendBuildIdIfProvided() throws Exception {
+        SaasFacade saasFacade = buildFacade(ImmutableMap.of(
+                "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(),
+                "cli.buildId", "11223344",
+                "connector.repository", SOME_REPOSITORY,
+                "connector.project", SOME_PROJECT
+        ));
+
+        stubGet(urlEqualTo(String.format(
+                "%s/api/github/%s/%s/pulls/%s/files?build_id=%s",
+                FacadeConfigUtil.PATH, SOME_PROJECT, SOME_REPOSITORY, SOME_PULL_REQUEST_ID, "11223344")), "/json/saas-files.json");
+
+        List<ReviewFile> files = saasFacade.listFiles();
+
+        assertThat(files).extracting("reviewFilename").containsOnly("src/main/java/TestFile.java", "src/main/java/TestFile2.java");
+    }
+
+    protected SaasFacade buildFacade(Map<String, String> configMap) {
+        Configuration config = new ConfigurationSetup().setUp(FacadeConfigUtil.getHttpConfig("saas"), configMap);
+        return new SaasFacadeBuilder().build(config);
+    }
+
 }