From 883eb0cb3e1c9151d65d94db9fa5683bbc5a699a Mon Sep 17 00:00:00 2001 From: Piotr Jagielski Date: Fri, 29 Apr 2016 10:33:14 +0200 Subject: [PATCH 1/2] provider support --- .../touk/sputnik/configuration/CliOption.java | 5 +-- .../sputnik/configuration/CliWrapper.java | 1 + .../touk/sputnik/configuration/Provider.java | 26 +++++++++++++++ .../GithubPatchset.java => Patchset.java} | 6 ++-- .../sputnik/connector/PatchsetBuilder.java | 32 +++++++++++++++++++ .../connector/github/GithubFacade.java | 5 +-- .../connector/github/GithubFacadeBuilder.java | 8 +++-- .../github/GithubPatchsetBuilder.java | 28 ---------------- .../sputnik/connector/saas/SaasConnector.java | 16 +++++----- .../connector/saas/SaasFacadeBuilder.java | 4 +-- .../connector/github/GithubFacadeTest.java | 4 ++- .../connector/saas/SaasFacadeTest.java | 5 +++ 12 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 src/main/java/pl/touk/sputnik/configuration/Provider.java rename src/main/java/pl/touk/sputnik/connector/{github/GithubPatchset.java => Patchset.java} (53%) create mode 100644 src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java delete mode 100644 src/main/java/pl/touk/sputnik/connector/github/GithubPatchsetBuilder.java diff --git a/src/main/java/pl/touk/sputnik/configuration/CliOption.java b/src/main/java/pl/touk/sputnik/configuration/CliOption.java index 57834c89..50eab105 100644 --- a/src/main/java/pl/touk/sputnik/configuration/CliOption.java +++ b/src/main/java/pl/touk/sputnik/configuration/CliOption.java @@ -10,8 +10,9 @@ 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), - BUILD_ID("cli.buildId", "Optional build id for using Sputnik for Github", null); + API_KEY("cli.apiKey", "Optional API key for using Sputnik as a service", null), + BUILD_ID("cli.buildId", "Optional build id for using Sputnik as a service", null), + PROVIDER("cli.provider", "Optional SCM provider (GitHub, GitLab) for using Sputnik as a service", null); private String key; private String description; diff --git a/src/main/java/pl/touk/sputnik/configuration/CliWrapper.java b/src/main/java/pl/touk/sputnik/configuration/CliWrapper.java index cf7076aa..864aa541 100644 --- a/src/main/java/pl/touk/sputnik/configuration/CliWrapper.java +++ b/src/main/java/pl/touk/sputnik/configuration/CliWrapper.java @@ -25,6 +25,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)); + localOptions.addOption(buildOption(CliOption.PROVIDER, true, false)); return localOptions; } diff --git a/src/main/java/pl/touk/sputnik/configuration/Provider.java b/src/main/java/pl/touk/sputnik/configuration/Provider.java new file mode 100644 index 00000000..0a5dd046 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/configuration/Provider.java @@ -0,0 +1,26 @@ +package pl.touk.sputnik.configuration; + +import org.apache.commons.lang3.StringUtils; + +public enum Provider { + + GITHUB("github"), GITLAB("gitlab"); + + private final String name; + + Provider(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public static Provider from(String name) { + if (StringUtils.isNoneBlank(name)) { + return Provider.valueOf(name.toUpperCase()); + } else { + return null; + } + } +} diff --git a/src/main/java/pl/touk/sputnik/connector/github/GithubPatchset.java b/src/main/java/pl/touk/sputnik/connector/Patchset.java similarity index 53% rename from src/main/java/pl/touk/sputnik/connector/github/GithubPatchset.java rename to src/main/java/pl/touk/sputnik/connector/Patchset.java index cfda0375..a66f765e 100644 --- a/src/main/java/pl/touk/sputnik/connector/github/GithubPatchset.java +++ b/src/main/java/pl/touk/sputnik/connector/Patchset.java @@ -1,11 +1,13 @@ -package pl.touk.sputnik.connector.github; +package pl.touk.sputnik.connector; import lombok.AllArgsConstructor; import lombok.Data; +import pl.touk.sputnik.configuration.Provider; @Data @AllArgsConstructor -public class GithubPatchset { +public class Patchset { private final Integer pullRequestId; private final String projectPath; + private final Provider provider; } diff --git a/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java b/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java new file mode 100644 index 00000000..04b36d9a --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java @@ -0,0 +1,32 @@ +package pl.touk.sputnik.connector; + +import org.apache.commons.lang3.math.NumberUtils; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.configuration.CliOption; +import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.configuration.GeneralOption; +import pl.touk.sputnik.configuration.Provider; + +import static org.apache.commons.lang3.Validate.isTrue; +import static org.apache.commons.lang3.Validate.notBlank; +import static org.apache.commons.lang3.Validate.notNull; + +public class PatchsetBuilder { + + @NotNull + public static Patchset build(Configuration configuration) { + String pullRequestId = configuration.getProperty(CliOption.PULL_REQUEST_ID); + String project = configuration.getProperty(GeneralOption.PROJECT); + String repository = configuration.getProperty(GeneralOption.REPOSITORY); + Provider provider = Provider.from(configuration.getProperty(CliOption.PROVIDER)); + + notBlank(pullRequestId, "You must provide non blank pull request id"); + isTrue(NumberUtils.isNumber(pullRequestId), "Integer value as pull request id required"); + notBlank(project, "You must provide non blank project key"); + notBlank(repository, "You must provide non blank repository slug"); + notNull(provider, "You must provide non blank SCM provider"); + + return new Patchset(Integer.parseInt(pullRequestId), String.format("%s/%s", project, repository), provider); + } + +} diff --git a/src/main/java/pl/touk/sputnik/connector/github/GithubFacade.java b/src/main/java/pl/touk/sputnik/connector/github/GithubFacade.java index 886936e3..3b8448af 100644 --- a/src/main/java/pl/touk/sputnik/connector/github/GithubFacade.java +++ b/src/main/java/pl/touk/sputnik/connector/github/GithubFacade.java @@ -11,6 +11,7 @@ import pl.touk.sputnik.configuration.GeneralOptionNotSupportedException; import pl.touk.sputnik.connector.ConnectorFacade; import pl.touk.sputnik.connector.Connectors; +import pl.touk.sputnik.connector.Patchset; import pl.touk.sputnik.review.Review; import pl.touk.sputnik.review.ReviewFile; @@ -24,7 +25,7 @@ public class GithubFacade implements ConnectorFacade { private final Repo repo; - private final GithubPatchset githubPatchset; + private final Patchset patchset; @Override public Connectors name() { @@ -66,6 +67,6 @@ public void publish(Review review) { } private Pull getPull() { - return repo.pulls().get(githubPatchset.getPullRequestId()); + return repo.pulls().get(patchset.getPullRequestId()); } } \ No newline at end of file diff --git a/src/main/java/pl/touk/sputnik/connector/github/GithubFacadeBuilder.java b/src/main/java/pl/touk/sputnik/connector/github/GithubFacadeBuilder.java index 5add429c..1400948f 100644 --- a/src/main/java/pl/touk/sputnik/connector/github/GithubFacadeBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/github/GithubFacadeBuilder.java @@ -9,6 +9,8 @@ import org.jetbrains.annotations.NotNull; import pl.touk.sputnik.configuration.Configuration; import pl.touk.sputnik.configuration.GeneralOption; +import pl.touk.sputnik.connector.Patchset; +import pl.touk.sputnik.connector.PatchsetBuilder; @Slf4j public class GithubFacadeBuilder { @@ -16,7 +18,7 @@ public class GithubFacadeBuilder { @NotNull public GithubFacade build(Configuration configuration) { - GithubPatchset githubPatchset = GithubPatchsetBuilder.build(configuration); + Patchset patchset = PatchsetBuilder.build(configuration); String oAuthKey = configuration.getProperty(GeneralOption.GITHUB_API_KEY); Github github = new RtGithub( @@ -25,7 +27,7 @@ public GithubFacade build(Configuration configuration) { .through(RetryWire.class) ); - Repo repo = github.repos().get(new Coordinates.Simple(githubPatchset.getProjectPath())); - return new GithubFacade(repo, githubPatchset); + Repo repo = github.repos().get(new Coordinates.Simple(patchset.getProjectPath())); + return new GithubFacade(repo, patchset); } } diff --git a/src/main/java/pl/touk/sputnik/connector/github/GithubPatchsetBuilder.java b/src/main/java/pl/touk/sputnik/connector/github/GithubPatchsetBuilder.java deleted file mode 100644 index 3e62878a..00000000 --- a/src/main/java/pl/touk/sputnik/connector/github/GithubPatchsetBuilder.java +++ /dev/null @@ -1,28 +0,0 @@ -package pl.touk.sputnik.connector.github; - -import org.apache.commons.lang3.math.NumberUtils; -import org.jetbrains.annotations.NotNull; -import pl.touk.sputnik.configuration.CliOption; -import pl.touk.sputnik.configuration.Configuration; -import pl.touk.sputnik.configuration.GeneralOption; - -import static org.apache.commons.lang3.Validate.isTrue; -import static org.apache.commons.lang3.Validate.notBlank; - -public class GithubPatchsetBuilder { - - @NotNull - public static GithubPatchset build(Configuration configuration) { - String pullRequestId = configuration.getProperty(CliOption.PULL_REQUEST_ID); - String project = configuration.getProperty(GeneralOption.PROJECT); - String repository = configuration.getProperty(GeneralOption.REPOSITORY); - - notBlank(pullRequestId, "You must provide non blank Github pull request id"); - isTrue(NumberUtils.isNumber(pullRequestId), "Integer value as pull request id required"); - notBlank(project, "You must provide non blank Github project key"); - notBlank(repository, "You must provide non blank Github repository slug"); - - return new GithubPatchset(Integer.parseInt(pullRequestId), String.format("%s/%s", project, repository)); - } - -} diff --git a/src/main/java/pl/touk/sputnik/connector/saas/SaasConnector.java b/src/main/java/pl/touk/sputnik/connector/saas/SaasConnector.java index 7608d36c..cac9f3f3 100644 --- a/src/main/java/pl/touk/sputnik/connector/saas/SaasConnector.java +++ b/src/main/java/pl/touk/sputnik/connector/saas/SaasConnector.java @@ -12,7 +12,7 @@ import org.apache.http.message.BasicNameValuePair; import org.jetbrains.annotations.NotNull; import pl.touk.sputnik.connector.Connector; -import pl.touk.sputnik.connector.github.GithubPatchset; +import pl.touk.sputnik.connector.Patchset; import pl.touk.sputnik.connector.http.HttpConnector; import java.io.IOException; @@ -26,19 +26,19 @@ public class SaasConnector implements Connector { private HttpConnector httpConnector; - private GithubPatchset githubPatchset; + private Patchset patchset; 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"; + private static final String FILES_URL_FORMAT = "/api/%s/%s/pulls/%d/files"; + private static final String VIOLATIONS_URL_FORMAT = "/api/%s/%s/pulls/%d/violations"; @NotNull @Override public String listFiles() throws URISyntaxException, IOException { - URI uri = httpConnector.buildUri(createUrl(githubPatchset, FILES_URL_FORMAT), params()); + URI uri = httpConnector.buildUri(createUrl(patchset, FILES_URL_FORMAT), params()); HttpGet request = new HttpGet(uri); CloseableHttpResponse httpResponse = httpConnector.logAndExecute(request); return httpConnector.consumeAndLogEntity(httpResponse); @@ -48,15 +48,15 @@ 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), params()); + URI uri = httpConnector.buildUri(createUrl(patchset, VIOLATIONS_URL_FORMAT), params()); HttpPost httpPost = new HttpPost(uri); httpPost.setEntity(new StringEntity(violationsAsJson, ContentType.APPLICATION_JSON)); CloseableHttpResponse httpResponse = httpConnector.logAndExecute(httpPost); return httpConnector.consumeAndLogEntity(httpResponse); } - private String createUrl(GithubPatchset patchset, String formatUrl) { - return String.format(formatUrl, patchset.getProjectPath(), patchset.getPullRequestId()); + private String createUrl(Patchset patchset, String formatUrl) { + return String.format(formatUrl, patchset.getProvider().getName(), patchset.getProjectPath(), patchset.getPullRequestId()); } @NotNull diff --git a/src/main/java/pl/touk/sputnik/connector/saas/SaasFacadeBuilder.java b/src/main/java/pl/touk/sputnik/connector/saas/SaasFacadeBuilder.java index 0afbc327..5915b035 100644 --- a/src/main/java/pl/touk/sputnik/connector/saas/SaasFacadeBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/saas/SaasFacadeBuilder.java @@ -9,7 +9,7 @@ import pl.touk.sputnik.configuration.CliOption; import pl.touk.sputnik.configuration.Configuration; import pl.touk.sputnik.connector.ConnectorDetails; -import pl.touk.sputnik.connector.github.GithubPatchsetBuilder; +import pl.touk.sputnik.connector.PatchsetBuilder; import pl.touk.sputnik.connector.http.HttpConnector; import pl.touk.sputnik.connector.http.HttpHelper; @@ -28,7 +28,7 @@ public SaasFacade build(Configuration configuration) { return new SaasFacade(new SaasConnector( new HttpConnector(closeableHttpClient, httpClientContext, connectorDetails.getPath()), - GithubPatchsetBuilder.build(configuration), apiKey, buildId), new Gson()); + PatchsetBuilder.build(configuration), apiKey, buildId), new Gson()); } } diff --git a/src/test/java/pl/touk/sputnik/connector/github/GithubFacadeTest.java b/src/test/java/pl/touk/sputnik/connector/github/GithubFacadeTest.java index 7576d3a6..1b8baa4a 100644 --- a/src/test/java/pl/touk/sputnik/connector/github/GithubFacadeTest.java +++ b/src/test/java/pl/touk/sputnik/connector/github/GithubFacadeTest.java @@ -12,7 +12,9 @@ import org.mockito.runners.MockitoJUnitRunner; import pl.touk.sputnik.configuration.Configuration; import pl.touk.sputnik.configuration.ConfigurationSetup; +import pl.touk.sputnik.configuration.Provider; import pl.touk.sputnik.connector.FacadeConfigUtil; +import pl.touk.sputnik.connector.Patchset; import pl.touk.sputnik.review.*; import javax.json.Json; @@ -64,7 +66,7 @@ public void setUp() throws IOException { when(pull.repo().git().commits()).thenReturn(commits); config = new ConfigurationSetup().setUp(FacadeConfigUtil.getHttpConfig("github"), GITHUB_PATCHSET_MAP); - githubFacade = new GithubFacade(repo, new GithubPatchset(SOME_PULL_REQUEST_ID, projectPath())); + githubFacade = new GithubFacade(repo, new Patchset(SOME_PULL_REQUEST_ID, projectPath(), Provider.GITHUB)); } @Test diff --git a/src/test/java/pl/touk/sputnik/connector/saas/SaasFacadeTest.java b/src/test/java/pl/touk/sputnik/connector/saas/SaasFacadeTest.java index fdd6025e..0c57008b 100644 --- a/src/test/java/pl/touk/sputnik/connector/saas/SaasFacadeTest.java +++ b/src/test/java/pl/touk/sputnik/connector/saas/SaasFacadeTest.java @@ -17,6 +17,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.assertj.core.api.Assertions.assertThat; +import static pl.touk.sputnik.configuration.Provider.GITHUB; public class SaasFacadeTest extends HttpConnectorEnv { @@ -28,6 +29,7 @@ public class SaasFacadeTest extends HttpConnectorEnv { private static final Map GITHUB_PATCHSET_MAP = ImmutableMap.of( "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(), "cli.apiKey", SOME_API_KEY, + "cli.provider", GITHUB.getName(), "connector.repository", SOME_REPOSITORY, "connector.project", SOME_PROJECT ); @@ -77,6 +79,7 @@ public void shouldThrowOnWrongApiKey() throws Exception { SaasFacade saasFacade = buildFacade(ImmutableMap.of( "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(), "cli.apiKey", "WRONG_API_KEY", + "cli.provider", GITHUB.getName(), "connector.repository", SOME_REPOSITORY, "connector.project", SOME_PROJECT )); @@ -92,6 +95,7 @@ public void shouldThrowOnWrongApiKey() throws Exception { public void shouldHandleEmptyApiKey() throws Exception { SaasFacade saasFacade = buildFacade(ImmutableMap.of( "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(), + "cli.provider", GITHUB.getName(), "connector.repository", SOME_REPOSITORY, "connector.project", SOME_PROJECT )); @@ -110,6 +114,7 @@ public void shouldSendBuildIdIfProvided() throws Exception { SaasFacade saasFacade = buildFacade(ImmutableMap.of( "cli.pullRequestId", SOME_PULL_REQUEST_ID.toString(), "cli.buildId", "11223344", + "cli.provider", GITHUB.getName(), "connector.repository", SOME_REPOSITORY, "connector.project", SOME_PROJECT )); From 5932729d6c3a6d77537c7015195a6c71e2167086 Mon Sep 17 00:00:00 2001 From: Piotr Jagielski Date: Wed, 11 May 2016 12:16:09 +0200 Subject: [PATCH 2/2] utility class --- src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java b/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java index 04b36d9a..5ce5e693 100644 --- a/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java +++ b/src/main/java/pl/touk/sputnik/connector/PatchsetBuilder.java @@ -11,7 +11,9 @@ import static org.apache.commons.lang3.Validate.notBlank; import static org.apache.commons.lang3.Validate.notNull; -public class PatchsetBuilder { +public final class PatchsetBuilder { + + private PatchsetBuilder() { } @NotNull public static Patchset build(Configuration configuration) {