From b342388f7538c889dd250163983bf6c24f7c98b7 Mon Sep 17 00:00:00 2001 From: Eva Anker Date: Fri, 7 Jul 2017 14:44:51 +0200 Subject: [PATCH] Approvals v2 works --- .../java/previewcode/backend/APIModule.java | 2 + .../backend/DTO/ApprovedGroup.java | 5 +- .../backend/DTO/GHApproveStatus.java | 46 ++++++++++++ .../backend/DTO/GitHubPullRequest.java | 20 ++++- .../backend/DTO/HunkApprovals.java | 5 +- .../backend/DTO/OrderingStatus.java | 4 +- .../java/previewcode/backend/MainModule.java | 3 +- .../api/filter/GitHubAccessTokenFilter.java | 4 +- .../backend/api/v1/AssigneesAPI.java | 57 -------------- .../backend/api/v1/WebhookAPI.java | 8 +- .../backend/api/v2/ApprovalsAPI.java | 64 ++++++++-------- .../backend/services/DatabaseService.java | 44 +++++------ .../backend/services/GithubService.java | 52 ++++++------- .../backend/services/IDatabaseService.java | 2 - .../backend/services/IGithubService.java | 50 +++++++++++++ .../backend/services/actiondsl/ActionDSL.java | 5 ++ .../backend/api/v2/EndPointTest.java | 62 +++++++++------ .../backend/services/DatabaseServiceTest.java | 75 ------------------- .../backend/services/GitHubServiceTest.java | 2 +- 19 files changed, 256 insertions(+), 254 deletions(-) create mode 100644 src/main/java/previewcode/backend/DTO/GHApproveStatus.java delete mode 100644 src/main/java/previewcode/backend/api/v1/AssigneesAPI.java create mode 100644 src/main/java/previewcode/backend/services/IGithubService.java diff --git a/src/main/java/previewcode/backend/APIModule.java b/src/main/java/previewcode/backend/APIModule.java index 71c4526..85c2f12 100644 --- a/src/main/java/previewcode/backend/APIModule.java +++ b/src/main/java/previewcode/backend/APIModule.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.databind.module.SimpleModule; import com.google.inject.servlet.ServletModule; import io.atlassian.fugue.Unit; +import io.vavr.jackson.datatype.VavrModule; import org.jboss.resteasy.plugins.guice.ext.JaxrsModule; import previewcode.backend.api.exceptionmapper.*; import previewcode.backend.api.v2.ApprovalsAPI; @@ -61,6 +62,7 @@ public ObjectMapper getContext(Class type) { private static ObjectMapper createDefaultMapper() { ObjectMapper mapper = new ObjectMapper(); mapper.registerModule(new UnitSerializerModule()); + mapper.registerModule(new VavrModule()); return mapper; } diff --git a/src/main/java/previewcode/backend/DTO/ApprovedGroup.java b/src/main/java/previewcode/backend/DTO/ApprovedGroup.java index 922cac4..6aced3a 100644 --- a/src/main/java/previewcode/backend/DTO/ApprovedGroup.java +++ b/src/main/java/previewcode/backend/DTO/ApprovedGroup.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import previewcode.backend.database.GroupID; +import java.util.List; import java.util.Map; @@ -18,11 +19,11 @@ public class ApprovedGroup { * All the hunks in this group */ @JsonProperty("hunks") - public final Map hunks; + public final List hunks; public final GroupID groupID; - public ApprovedGroup(ApproveStatus approved, Map hunks, GroupID groupID) { + public ApprovedGroup(ApproveStatus approved, List hunks, GroupID groupID) { this.approved = approved; this.hunks = hunks; this.groupID = groupID; diff --git a/src/main/java/previewcode/backend/DTO/GHApproveStatus.java b/src/main/java/previewcode/backend/DTO/GHApproveStatus.java new file mode 100644 index 0000000..2ed6e11 --- /dev/null +++ b/src/main/java/previewcode/backend/DTO/GHApproveStatus.java @@ -0,0 +1,46 @@ +package previewcode.backend.DTO; + +public class GHApproveStatus extends GitHubStatus { + + private static final String CONTEXT = "preview-code/approve-status"; + private static final String PENDING_DESCRIPTION = "Not all hunks have been reviewed"; + private static final String SUCCESS_DESCRIPTION = "All hunks have been accepted"; + private static final String FAILURE_DESCRIPTION = "There are rejected hunks"; + + public GHApproveStatus(GitHubPullRequest pullRequest) { + super("pending", PENDING_DESCRIPTION, CONTEXT, pullRequest.previewCodeUrl()); + } + + private GHApproveStatus(String state, String description, String url) { + super(state, description, CONTEXT, url); + } + + public Boolean isComplete() { + return this.state.equals("success"); + } + + public Boolean isPending() { + return this.state.equals("pending"); + } + + public Boolean isFailure() { + return this.state.equals("failure"); + } + + public GHApproveStatus complete() { + return new GHApproveStatus("success", SUCCESS_DESCRIPTION, this.url); + } + + public GHApproveStatus pending() { + return new GHApproveStatus("pending", PENDING_DESCRIPTION, this.url); + } + + public GHApproveStatus failure() { + return new GHApproveStatus("failure", FAILURE_DESCRIPTION, this.url); + } + + @Override + public String toString() { + return "GHApproveStatus{" + state + "}"; + } +} diff --git a/src/main/java/previewcode/backend/DTO/GitHubPullRequest.java b/src/main/java/previewcode/backend/DTO/GitHubPullRequest.java index f2762d4..31974d4 100644 --- a/src/main/java/previewcode/backend/DTO/GitHubPullRequest.java +++ b/src/main/java/previewcode/backend/DTO/GitHubPullRequest.java @@ -11,6 +11,7 @@ public class GitHubPullRequest { public final String title; public final Integer number; public final PullRequestLinks links; + public final Base base; private static final String PREVIEW_URL = "https://preview-code.com/"; @@ -20,15 +21,28 @@ public GitHubPullRequest( @JsonProperty("body") String body, @JsonProperty("url") String url, @JsonProperty("number") Integer number, - @JsonProperty("_links") PullRequestLinks links) { + @JsonProperty("base") Base base, + @JsonProperty("_links") PullRequestLinks links + ) { this.title = title; this.body = body; this.url = url; this.number = number; this.links = links; + this.base = base; } - public String previewCodeUrl(GitHubRepository repository) { - return PREVIEW_URL + repository.fullName + "/pulls/" + this.number; + public String previewCodeUrl() { + return PREVIEW_URL + base.repo.fullName + "/pulls/" + this.number; + } + + @JsonIgnoreProperties(ignoreUnknown=true) + public static class Base { + public final GitHubRepository repo; + + @JsonCreator + public Base(@JsonProperty("repo") GitHubRepository repo) { + this.repo = repo; + } } } diff --git a/src/main/java/previewcode/backend/DTO/HunkApprovals.java b/src/main/java/previewcode/backend/DTO/HunkApprovals.java index 9e0817d..e9c35b2 100644 --- a/src/main/java/previewcode/backend/DTO/HunkApprovals.java +++ b/src/main/java/previewcode/backend/DTO/HunkApprovals.java @@ -13,13 +13,16 @@ public class HunkApprovals { @JsonProperty("hunkID") public String hunkChecksum; + @JsonProperty("approved") + public final ApproveStatus approved; /** * Per user the approvals status */ @JsonProperty("approvals") public Map approvals; - public HunkApprovals(HunkChecksum hunkChecksum, Map approvals) { + public HunkApprovals(HunkChecksum hunkChecksum, ApproveStatus approved, Map approvals) { + this.approved = approved; this.approvals = approvals; this.hunkChecksum = hunkChecksum.checksum; } diff --git a/src/main/java/previewcode/backend/DTO/OrderingStatus.java b/src/main/java/previewcode/backend/DTO/OrderingStatus.java index 59870ad..3186015 100644 --- a/src/main/java/previewcode/backend/DTO/OrderingStatus.java +++ b/src/main/java/previewcode/backend/DTO/OrderingStatus.java @@ -12,8 +12,8 @@ public class OrderingStatus extends GitHubStatus { private static final String PENDING_DESCRIPTION = "Waiting for author to order changes"; private static final String SUCCESS_DESCRIPTION = "Changes have been ordered by the author"; - public OrderingStatus(GitHubPullRequest pullRequest, GitHubRepository repository) { - super("pending", PENDING_DESCRIPTION, CONTEXT, pullRequest.previewCodeUrl(repository)); + public OrderingStatus(GitHubPullRequest pullRequest) { + super("pending", PENDING_DESCRIPTION, CONTEXT, pullRequest.previewCodeUrl()); } private OrderingStatus(String state, String description, String url) { diff --git a/src/main/java/previewcode/backend/MainModule.java b/src/main/java/previewcode/backend/MainModule.java index 9742739..6ed966d 100644 --- a/src/main/java/previewcode/backend/MainModule.java +++ b/src/main/java/previewcode/backend/MainModule.java @@ -23,6 +23,7 @@ import previewcode.backend.api.filter.IJWTTokenCreator; import previewcode.backend.api.filter.JWTTokenCreator; import previewcode.backend.api.v1.*; +import previewcode.backend.services.IGithubService; import previewcode.backend.services.interpreters.DatabaseInterpreter; import previewcode.backend.services.interpreters.GitHubAuthInterpreter; import previewcode.backend.services.http.HttpRequestExecutor; @@ -66,7 +67,6 @@ public void configureServlets() { this.bind(StatusAPI.class); this.bind(PullRequestAPI.class); this.bind(CommentsAPI.class); - this.bind(AssigneesAPI.class); this.bind(TrackerAPI.class); this.bind(WebhookAPI.class); @@ -74,6 +74,7 @@ public void configureServlets() { this.bind(ResteasyJackson2Provider.class); this.bind(IDatabaseService.class).to(DatabaseService.class); + this.bind(IGithubService.class).to(GithubService.class); try { HttpRequestExecutor http = new HttpRequestExecutor(); diff --git a/src/main/java/previewcode/backend/api/filter/GitHubAccessTokenFilter.java b/src/main/java/previewcode/backend/api/filter/GitHubAccessTokenFilter.java index 7b41fe4..dc9d4e5 100644 --- a/src/main/java/previewcode/backend/api/filter/GitHubAccessTokenFilter.java +++ b/src/main/java/previewcode/backend/api/filter/GitHubAccessTokenFilter.java @@ -1,9 +1,9 @@ package previewcode.backend.api.filter; import org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext; +import previewcode.backend.services.IGithubService; import previewcode.backend.services.interpreters.RequestContextActionInterpreter; import previewcode.backend.services.interpreters.GitHubAuthInterpreter; -import previewcode.backend.services.GithubService; import previewcode.backend.services.actiondsl.Interpreter; import javax.inject.Inject; @@ -23,7 +23,7 @@ public class GitHubAccessTokenFilter implements ContainerRequestFilter { private GitHubAuthInterpreter gitHubAuthInterpreter; @Inject - private GithubService.V2 authService; + private IGithubService.V2 authService; @Override public void filter(ContainerRequestContext ctx) throws IOException { diff --git a/src/main/java/previewcode/backend/api/v1/AssigneesAPI.java b/src/main/java/previewcode/backend/api/v1/AssigneesAPI.java deleted file mode 100644 index 54013e6..0000000 --- a/src/main/java/previewcode/backend/api/v1/AssigneesAPI.java +++ /dev/null @@ -1,57 +0,0 @@ -package previewcode.backend.api.v1; - -import javax.ws.rs.Consumes; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.core.MediaType; - -import previewcode.backend.DTO.ApproveRequest; -import previewcode.backend.services.FirebaseService; - -import com.google.inject.Inject; -import previewcode.backend.services.GithubService; -import org.kohsuke.github.GHMyself; - -import java.io.IOException; - -/** - * API endpoint for approving hunks - * - */ -@Path("v1/{owner}/{name}/pulls/{number}/approve") -public class AssigneesAPI { - - @Inject - private FirebaseService firebaseService; - - @Inject - private GithubService githubService; - - /** - * Creates a pull request - * - * @param owner - * The owner of the repository on which the pull request is created - * @param name - * The owner of the repository on which the pull request is created - * @param body - * The body of the pull request - * @param number - * The number of the pull request - * @return The number of the newly made pull request - */ - @POST - @Consumes(MediaType.APPLICATION_JSON) - public ApproveRequest setApprove(@PathParam("owner") String owner, - @PathParam("name") String name, - @PathParam("number") String number, - ApproveRequest body) throws IOException { - GHMyself user = githubService.getLoggedInUser(); - if (!body.githubLogin.equals(user.getLogin())) { - throw new IllegalArgumentException("Can not set approve status of other user"); - } - firebaseService.setApproved(owner, name, number, body); - return body; - } -} diff --git a/src/main/java/previewcode/backend/api/v1/WebhookAPI.java b/src/main/java/previewcode/backend/api/v1/WebhookAPI.java index 67dcb71..ed0358e 100644 --- a/src/main/java/previewcode/backend/api/v1/WebhookAPI.java +++ b/src/main/java/previewcode/backend/api/v1/WebhookAPI.java @@ -65,7 +65,7 @@ public Response onWebhookPost( if (action.equals("opened")) { Pair repoAndPull = readRepoAndPullFromWebhook(body); PRComment comment = new PRComment(constructMarkdownComment(repoAndPull.first, repoAndPull.second)); - OrderingStatus pendingStatus = new OrderingStatus(repoAndPull.second, repoAndPull.first); + OrderingStatus pendingStatus = new OrderingStatus(repoAndPull.second); firebaseService.addDefaultData(new PullRequestIdentifier(repoAndPull.first, repoAndPull.second)); @@ -80,7 +80,7 @@ public Response onWebhookPost( } else if (action.equals("synchronize")) { Pair repoAndPull = readRepoAndPullFromWebhook(body); - OrderingStatus pendingStatus = new OrderingStatus(repoAndPull.second, repoAndPull.first); + OrderingStatus pendingStatus = new OrderingStatus(repoAndPull.second); githubService.setOrderingStatus(repoAndPull.second, pendingStatus); } } else if (eventType.equals("pull_request_review")) { @@ -98,9 +98,9 @@ public Response onWebhookPost( } private String constructMarkdownComment(GitHubRepository repo, GitHubPullRequest pullRequest) { - return "This pull request can be reviewed with [Preview Code](" + pullRequest.previewCodeUrl(repo) + ").\n" + + return "This pull request can be reviewed with [Preview Code](" + pullRequest.previewCodeUrl() + ").\n" + "To speed up the review process and get better feedback on your changes, " + - "please **[order your changes](" + pullRequest.previewCodeUrl(repo) + ").**\n"; + "please **[order your changes](" + pullRequest.previewCodeUrl() + ").**\n"; } private Pair readRepoAndPullFromWebhook(JsonNode body) throws JsonProcessingException { diff --git a/src/main/java/previewcode/backend/api/v2/ApprovalsAPI.java b/src/main/java/previewcode/backend/api/v2/ApprovalsAPI.java index ba4f267..9c262cb 100644 --- a/src/main/java/previewcode/backend/api/v2/ApprovalsAPI.java +++ b/src/main/java/previewcode/backend/api/v2/ApprovalsAPI.java @@ -2,15 +2,17 @@ import com.google.inject.Inject; import io.atlassian.fugue.Unit; -import io.vavr.collection.List; import previewcode.backend.DTO.*; import previewcode.backend.services.IDatabaseService; +import previewcode.backend.services.IGithubService; import previewcode.backend.services.actiondsl.Interpreter; import javax.inject.Named; import javax.ws.rs.*; import javax.ws.rs.core.Response; +import java.io.IOException; + import static previewcode.backend.services.actiondsl.ActionDSL.*; /** @@ -22,6 +24,9 @@ public class ApprovalsAPI { private Interpreter interpreter; private IDatabaseService databaseService; + @Inject + private IGithubService githubService; + @Inject public ApprovalsAPI(@Named("database-interp") Interpreter interpreter, IDatabaseService databaseService) { this.interpreter = interpreter; @@ -30,9 +35,10 @@ public ApprovalsAPI(@Named("database-interp") Interpreter interpreter, IDatabase /** * Fetches all approvals and shows if pr/groups/hunks are (dis)approved - * @param owner The owner of the repository - * @param name The name of the repository - * @param number The pullrequest number + * + * @param owner The owner of the repository + * @param name The name of the repository + * @param number The pullrequest number * @return if the pullrequest and the groups and hunks are disapproved or approved */ @Path("getApprovals") @@ -45,44 +51,38 @@ public Response getApprovals(@PathParam("owner") String owner, return interpreter.evaluateToResponse(action); } - /** - * Fetches all approvals and shows per hunk whom (dis)approved - * @param owner The owner of the repository - * @param name The name of the repository - * @param number The pullrequest number - * @return per hunk the approval status of the reviewers - */ - @Path("getHunkApprovals") - @GET - public Response getHunkApprovals(@PathParam("owner") String owner, - @PathParam("name") String name, - @PathParam("number") Integer number) { - PullRequestIdentifier pull = new PullRequestIdentifier(owner, name, number); - Action> action = databaseService.getHunkApprovals(pull); - - return interpreter.evaluateToResponse(action); - } - /** * Sets the approval from a user on a hunk. + * Calls github when the status of the pr changes. * - * @param owner The owner of the repository - * @param name The name of the repository - * @param number The pullrequest number - * @param body Hunk approval information + * @param owner The owner of the repository + * @param name The name of the repository + * @param number The pullrequest number + * @param body Hunk approval information * @return A unit response */ @Path("setApprove") @POST public Response setApprove(@PathParam("owner") String owner, - @PathParam("name") String name, - @PathParam("number") Integer number, - ApproveRequest body) { - - // TODO: check if user is a reviewer on this PR + @PathParam("name") String name, + @PathParam("number") Integer number, + ApproveRequest body) throws IOException { PullRequestIdentifier pull = new PullRequestIdentifier(owner, name, number); - Action action = databaseService.setApproval(pull, body); + Action action = databaseService.getApproval(pull).map( + approvedPullRequest -> approvedPullRequest.approved + ).then(oldStatus -> databaseService.setApproval(pull, body).pure(oldStatus)).then( + oldStatus -> databaseService.getApproval(pull).map(newStatus -> { + if (!oldStatus.equals(newStatus.approved)) { + try { + githubService.setPRStatus(githubService.fetchPullRequest(pull), newStatus.approved); + } catch (IOException e) { + sneakyThrow(e); + } + } + return unit; + }) + ); return interpreter.evaluateToResponse(action); } diff --git a/src/main/java/previewcode/backend/services/DatabaseService.java b/src/main/java/previewcode/backend/services/DatabaseService.java index 7372c96..6474a11 100644 --- a/src/main/java/previewcode/backend/services/DatabaseService.java +++ b/src/main/java/previewcode/backend/services/DatabaseService.java @@ -7,6 +7,7 @@ import previewcode.backend.database.*; import previewcode.backend.services.actions.DatabaseActions; +import java.util.ArrayList; import java.util.HashMap; import java.util.Map; import java.util.function.Function; @@ -66,35 +67,36 @@ public Action clearExistingGroups(PullRequestID dbPullId) { .map(unit -> dbPullId); } - @Override - public Action> getHunkApprovals(PullRequestIdentifier pull) { - - return fetchPullRequestGroups(pull) - .then(traverse(group -> fetchHunks(group.id))) - .map(flatten()) - .then(traverse(hunk -> hunk.fetchApprovals.map(approvals -> { - Map m = new HashMap<>(); - approvals.forEach(approval -> m.put(approval.approver, approval.approveStatus)); - return new HunkApprovals(hunk.checksum, m); - }))); - } - private static Action getGroupApproval(GroupID groupID) { return fetchHunks(groupID).then( hunks -> traverse(hunks, (Hunk h) -> h.fetchApprovals.map(approvals -> { - Map map = new HashMap<>(); - map.put(h.checksum.checksum, isHunkApproved(approvals.map(a -> a.approveStatus))); + Map approvalMap = new HashMap<>(); + approvals.forEach(approval -> approvalMap.put(approval.approver, approval.approveStatus)); + + Map map = new HashMap<>(); + HunkApprovals hApprovals = new HunkApprovals(h.checksum, isHunkApproved(approvals.map(a -> a.approveStatus)), approvalMap); + map.put(h.checksum.checksum, hApprovals); return map; })) - .map(DatabaseService::combineMaps) - .map(approvals -> new ApprovedGroup( - isGroupApproved(hunks.length(), approvals), - approvals, - groupID) - ) + .map(DatabaseService::combineMaps) + .map(approvals -> { + Map approvalMap = new HashMap<>(); + java.util.List hunkList = new ArrayList(); + approvals.forEach((hunk, approval) -> { + approvalMap.put(hunk, approval.approved); + hunkList.add(approval); + }); + + return new ApprovedGroup( + isGroupApproved(hunks.length(), approvalMap), + hunkList, + groupID); + }) ); } + + private static Map combineMaps(List> maps) { return maps.fold(new HashMap<>(), (a, b) -> { a.putAll(b); diff --git a/src/main/java/previewcode/backend/services/GithubService.java b/src/main/java/previewcode/backend/services/GithubService.java index aebde7c..6ab7afd 100644 --- a/src/main/java/previewcode/backend/services/GithubService.java +++ b/src/main/java/previewcode/backend/services/GithubService.java @@ -39,34 +39,7 @@ * An abstract class that connects with GitHub */ @RequestScoped -public class GithubService { - - public static class V2 { - - private static final String GITHUB_WEBHOOK_SECRET_HEADER = "X-Hub-Signature"; - private static final String TOKEN_PARAMETER = "access_token"; - - public Action authenticate() { - return getUserAgent.then(isWebHookUserAgent).then(isWebHook -> { - if (isWebHook) { - return with(getRequestBody) - .and(getHeader(GITHUB_WEBHOOK_SECRET_HEADER)) - .then(verifyWebHookSecret) - .then(getJsonBody) - .map(InstallationID::fromJson) - .then(authenticateInstallation); - } else { - return getQueryParam(TOKEN_PARAMETER) - .map(o -> o.getOrElseThrow(NoTokenException::new)) - .map(GitHubUserToken::fromString) - .then(getUser) - .toUnit(); - } - }); - } - - - } +public class GithubService implements IGithubService { private static final Logger logger = LoggerFactory.getLogger(GithubService.class); private static final OkHttpClient OK_HTTP_CLIENT = new OkHttpClient(); @@ -92,6 +65,7 @@ protected GithubService(@Named("github.user") final Provider gitHubProvi githubProvider = gitHubProvider; } + @Override public GHMyself getLoggedInUser() throws IOException { return this.githubProvider.get().getMyself(); } @@ -109,6 +83,7 @@ public GHMyself getLoggedInUser() throws IOException { * The body of the pull request * @return The number of the newly made pull request */ + @Override public PrNumber createPullRequest(String owner, String name, PRbody body) { try { @@ -221,6 +196,7 @@ public Boolean isOwner(String owner, String name, int number){ * @param identifier The identifier object containing owner, name and number of the pull to fetch. * @throws IOException when the request fails */ + @Override public GitHubPullRequest fetchPullRequest(PullRequestIdentifier identifier) throws IOException { logger.info("Fetching pull request from GitHub API..."); @@ -269,7 +245,7 @@ public void placePullRequestComment(GitHubPullRequest pullRequest, PRComment com * @throws IOException when the request fails */ public void setOrderingStatus(GitHubPullRequest pullRequest, OrderingStatus status) throws IOException { - logger.info("Setting pull request status to: " + status); + logger.info("Setting pull request ordering status to: " + status); Request createStatus = tokenBuilder.addToken(new Request.Builder()) .url(pullRequest.links.statuses) .post(toJson(status)) @@ -278,6 +254,24 @@ public void setOrderingStatus(GitHubPullRequest pullRequest, OrderingStatus stat this.execute(createStatus); } + @Override + public void setPRStatus(GitHubPullRequest pullRequest, ApproveStatus status) throws IOException { + logger.info("Setting pull request approval status to: " + status); + GHApproveStatus ghStatus = new GHApproveStatus(pullRequest); + if(status == ApproveStatus.APPROVED){ + ghStatus = ghStatus.complete(); + } + else if(status == ApproveStatus.DISAPPROVED){ + ghStatus = ghStatus.failure(); + } + Request createStatus = tokenBuilder.addToken(new Request.Builder()) + .url(pullRequest.links.statuses) + .post(toJson(ghStatus)) + .build(); + + this.execute(createStatus); + } + public Optional getOrderingStatus(GitHubPullRequest pullRequest) throws IOException { logger.info("Fetching pull request status from GitHub API"); Request getStatus = tokenBuilder.addToken(new Request.Builder()) diff --git a/src/main/java/previewcode/backend/services/IDatabaseService.java b/src/main/java/previewcode/backend/services/IDatabaseService.java index e4a86d1..4be80b0 100644 --- a/src/main/java/previewcode/backend/services/IDatabaseService.java +++ b/src/main/java/previewcode/backend/services/IDatabaseService.java @@ -18,6 +18,4 @@ public interface IDatabaseService { Action> fetchPullRequestGroups(PullRequestIdentifier pull); Action getApproval(PullRequestIdentifier pull); - - Action> getHunkApprovals(PullRequestIdentifier pull); } diff --git a/src/main/java/previewcode/backend/services/IGithubService.java b/src/main/java/previewcode/backend/services/IGithubService.java new file mode 100644 index 0000000..d1a4c07 --- /dev/null +++ b/src/main/java/previewcode/backend/services/IGithubService.java @@ -0,0 +1,50 @@ +package previewcode.backend.services; + +import io.atlassian.fugue.Unit; +import org.kohsuke.github.GHMyself; +import previewcode.backend.DTO.*; +import previewcode.backend.api.exceptionmapper.NoTokenException; +import previewcode.backend.services.actiondsl.ActionDSL; + +import java.io.IOException; + +import static previewcode.backend.services.actiondsl.ActionDSL.with; +import static previewcode.backend.services.actions.GitHubActions.*; +import static previewcode.backend.services.actions.RequestContextActions.*; + +public interface IGithubService { + GHMyself getLoggedInUser() throws IOException; + + PrNumber createPullRequest(String owner, String name, PRbody body); + + GitHubPullRequest fetchPullRequest(PullRequestIdentifier identifier) throws IOException; + + void setPRStatus(GitHubPullRequest pullRequest, ApproveStatus status) throws IOException; + + public static class V2 { + + private static final String GITHUB_WEBHOOK_SECRET_HEADER = "X-Hub-Signature"; + private static final String TOKEN_PARAMETER = "access_token"; + + public ActionDSL.Action authenticate() { + return getUserAgent.then(isWebHookUserAgent).then(isWebHook -> { + if (isWebHook) { + return with(getRequestBody) + .and(getHeader(GITHUB_WEBHOOK_SECRET_HEADER)) + .then(verifyWebHookSecret) + .then(getJsonBody) + .map(InstallationID::fromJson) + .then(authenticateInstallation); + } else { + return getQueryParam(TOKEN_PARAMETER) + .map(o -> o.getOrElseThrow(NoTokenException::new)) + .map(GitHubUserToken::fromString) + .then(getUser) + .toUnit(); + } + }); + } + + + } +} diff --git a/src/main/java/previewcode/backend/services/actiondsl/ActionDSL.java b/src/main/java/previewcode/backend/services/actiondsl/ActionDSL.java index a2f42ad..c9294cc 100644 --- a/src/main/java/previewcode/backend/services/actiondsl/ActionDSL.java +++ b/src/main/java/previewcode/backend/services/actiondsl/ActionDSL.java @@ -277,6 +277,11 @@ public static CheckedFunction1 toUnit(Consumer f) { }; } + @SuppressWarnings("unchecked") + public static R sneakyThrow(Throwable t) throws T { + throw (T) t; + } + public static Interpreter interpret() { return new Interpreter(); } diff --git a/src/test/java/previewcode/backend/api/v2/EndPointTest.java b/src/test/java/previewcode/backend/api/v2/EndPointTest.java index 503fd2c..fc35773 100644 --- a/src/test/java/previewcode/backend/api/v2/EndPointTest.java +++ b/src/test/java/previewcode/backend/api/v2/EndPointTest.java @@ -1,11 +1,14 @@ package previewcode.backend.api.v2; import com.google.inject.name.Names; +import io.atlassian.fugue.Try; import io.atlassian.fugue.Unit; import io.vavr.collection.List; import org.junit.jupiter.api.Test; +import org.kohsuke.github.GHMyself; import previewcode.backend.APIModule; import previewcode.backend.DTO.*; +import previewcode.backend.services.IGithubService; import previewcode.backend.services.actiondsl.Interpreter; import previewcode.backend.test.helpers.ApiEndPointTest; import previewcode.backend.database.PullRequestGroup; @@ -15,6 +18,7 @@ import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.Response; +import java.io.IOException; import java.util.ArrayList; import static org.assertj.core.api.Assertions.*; @@ -68,23 +72,34 @@ public void getApprovalsApiIsReachable(WebTarget target) { assertThat(response.getLength()).isZero(); assertThat(response.getStatus()).isEqualTo(200); } +} +class TestModule extends APIModule implements IDatabaseService, IGithubService { - @Test - public void getHunkApprovalsApiIsReachable(WebTarget target) { - Response response = target - .path("/v2/preview-code/backend/pulls/42/getHunkApprovals") - .request("application/json") - .get(); + public TestModule() {} + + @SuppressWarnings("unchecked") + @Override + public void configureServlets() { + super.configureServlets(); + // The DatabaseService always returns a no-op action + this.bind(IDatabaseService.class).toInstance(this); + this.bind(IGithubService.class).toInstance(this); +// The interpreter always evaluates any action to Unit + this.bind(Interpreter.class).to(TestInterpreter.class); + this.bind(Interpreter.class).annotatedWith(Names.named("database-interp")).to(TestInterpreter.class); - assertThat(response.getLength()).isZero(); - assertThat(response.getStatus()).isEqualTo(200); } -} -class TestModule extends APIModule implements IDatabaseService { + public static class TestInterpreter extends Interpreter { + + @Override + protected Try run(Action action) { + return Try.successful((A) unit); + } + } + - public TestModule() {} @Override public Action updateOrdering(PullRequestIdentifier pullRequestIdentifier, List body) { @@ -111,22 +126,25 @@ public Action getApproval(PullRequestIdentifier pull) { return new NoOp<>(); } + + @Override - public Action> getHunkApprovals(PullRequestIdentifier pull) { - return new NoOp<>(); + public GHMyself getLoggedInUser() throws IOException { + return null; } + @Override + public PrNumber createPullRequest(String owner, String name, PRbody body) { + return null; + } - @SuppressWarnings("unchecked") @Override - public void configureServlets() { - super.configureServlets(); - // The DatabaseService always returns a no-op action - this.bind(IDatabaseService.class).toInstance(this); + public GitHubPullRequest fetchPullRequest(PullRequestIdentifier identifier) throws IOException { + return null; + } + + @Override + public void setPRStatus(GitHubPullRequest pullRequest, ApproveStatus status) throws IOException { -// The interpreter always evaluates any action to Unit - this.bind(Interpreter.class).annotatedWith(Names.named("database-interp")).toInstance( - interpret().on(NoOp.class).apply(x -> unit) - ); } } diff --git a/src/test/java/previewcode/backend/services/DatabaseServiceTest.java b/src/test/java/previewcode/backend/services/DatabaseServiceTest.java index 486fb77..23e5ef5 100644 --- a/src/test/java/previewcode/backend/services/DatabaseServiceTest.java +++ b/src/test/java/previewcode/backend/services/DatabaseServiceTest.java @@ -262,79 +262,4 @@ void getApproval_fetches_hunk_approvals(){ assertThat(stepper.next()).containsOnly(new FetchHunkApprovals(id)); assertThat(stepper.next()).isEmpty(); } - - @Test - void getHunkApproval_fetches_pull_pullRequest() { - Action dbAction = service.getHunkApprovals(pullIdentifier); - - List> peek = interpret().stepwiseEval(dbAction).peek(); - assertThat(peek).containsOnly(fetchPull(pullIdentifier)); - } - - @Test - void getHunkApproval_fetches_pull_groups(){ - Action dbAction = service.getHunkApprovals(pullIdentifier); - - Interpreter.Stepper stepper = interpret() - .on(FetchPull.class).returnA(pullRequestID) - .stepwiseEval(dbAction); - List> next = stepper.next(); - assertThat(next).containsOnly(fetchGroups(pullRequestID)); - } - - @Test - void getHunkApproval_fetches_hunks(){ - Action dbAction = service.getHunkApprovals(pullIdentifier); - - List oneGroup = List.of( - new PullRequestGroup(new GroupID(42L), "Group A", "Description A", defaultGroup) - ); - - Interpreter.Stepper stepper = interpret() - .on(FetchPull.class).returnA(pullRequestID) - .on(FetchGroupsForPull.class).returnA(oneGroup) - .stepwiseEval(dbAction); - stepper.next(); - List> next = stepper.next(); - assertThat(next).containsOnly(fetchHunks(oneGroup.head().id)); - } - - @Test - void getHunkApproval_fetches_hunk_approvals(){ - Action dbAction = service.getHunkApprovals(pullIdentifier); - - HunkChecksum checksum = new HunkChecksum("abcd"); - HunkID id = new HunkID(1L); - List oneHunk = List.of(new Hunk(id, new GroupID(2L), checksum)); - - Interpreter.Stepper stepper = interpret() - .on(FetchPull.class).returnA(pullRequestID) - .on(FetchGroupsForPull.class).returnA(groups) - .on(FetchHunksForGroup.class).returnA(oneHunk) - .stepwiseEval(dbAction); - stepper.next(); - stepper.next(); - - List> next = stepper.next(); - assertThat(next.length()).isEqualTo(2); - assertThat(next).containsOnly(fetchApprovals(id)); - } - - @Test - void getHunkApproval_no_action_after_fetching_hunkapprovals(){ - Action dbAction = service.getHunkApprovals(pullIdentifier); - - Interpreter.Stepper stepper = interpret() - .on(FetchPull.class).returnA(pullRequestID) - .on(FetchGroupsForPull.class).returnA(groups) - .on(FetchHunksForGroup.class).returnA(hunkIDs.map(id -> new Hunk(new HunkID(-1L), new GroupID(2L), id))) - .on(FetchHunkApprovals.class).returnA(List.empty()) - .stepwiseEval(dbAction); - stepper.next(); - stepper.next(); - stepper.next(); - - List> next = stepper.next(); - assertThat(next).isEmpty(); - } } diff --git a/src/test/java/previewcode/backend/services/GitHubServiceTest.java b/src/test/java/previewcode/backend/services/GitHubServiceTest.java index 36b43e2..865d3ee 100644 --- a/src/test/java/previewcode/backend/services/GitHubServiceTest.java +++ b/src/test/java/previewcode/backend/services/GitHubServiceTest.java @@ -19,7 +19,7 @@ public class GitHubServiceTest { - GithubService.V2 ghService = new GithubService.V2(); + IGithubService.V2 ghService = new IGithubService.V2(); Action authAction = ghService.authenticate();