Skip to content
Snippets Groups Projects
Verified Commit 0f56ff60 authored by Jordi Gómez's avatar Jordi Gómez
Browse files

feat: checking for existing comments before commenting one more time

parent 2809d9f7
No related branches found
No related tags found
No related merge requests found
......@@ -11,7 +11,9 @@
*/
package org.eclipsefoundation.git.eca.api;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
import org.eclipsefoundation.core.service.APIMiddleware.BaseAPIParameters;
......@@ -19,17 +21,20 @@ import org.eclipsefoundation.git.eca.api.models.GithubAccessToken;
import org.eclipsefoundation.git.eca.api.models.GithubApplicationInstallationData;
import org.eclipsefoundation.git.eca.api.models.GithubCommit;
import org.eclipsefoundation.git.eca.api.models.GithubCommitStatusRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestComment;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestCommentRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
import org.jboss.resteasy.reactive.RestResponse;
import jakarta.ws.rs.BeanParam;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.PATCH;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.HttpHeaders;
import jakarta.ws.rs.core.Response;
......@@ -107,6 +112,24 @@ public interface GithubAPI {
@HeaderParam("X-GitHub-Api-Version") String apiVersion, @PathParam("org") String organization, @PathParam("repo") String repo,
@PathParam("pullNumber") int pullNumber, GithubPullRequestCommentRequest commentRequest);
/**
* Lists comments on a pull request.
*
* @param bearer authorization header value, access token for application with access to repo
* @param apiVersion the version of the GH API to target when making the request
* @param organization the organization that owns the targeted repo
* @param repo the repo name that is being targeted
* @param pullNumber the number of the pull request
* @param perPage number of items to return per page, max 100
* @param page page number to return
* @return list of comments on the pull request
*/
@GET
@Path("repos/{org}/{repo}/pulls/{pullNumber}/comments")
public RestResponse<List<GithubPullRequestComment>> getComments(@HeaderParam(HttpHeaders.AUTHORIZATION) String bearer,
@HeaderParam("X-GitHub-Api-Version") String apiVersion, @PathParam("org") String organization, @PathParam("repo") String repo,
@PathParam("pullNumber") int pullNumber, @QueryParam("per_page") int perPage, @QueryParam("page") int page);
/**
* Requires a JWT bearer token for the application to retrieve installations for. Returns a list of installations for the given
* application.
......@@ -144,4 +167,36 @@ public interface GithubAPI {
@Path("installation/repositories")
public Response getInstallationRepositories(@BeanParam BaseAPIParameters params, @HeaderParam(HttpHeaders.AUTHORIZATION) String bearer);
/**
* Recursively fetches all comments from a pull request by handling pagination manually.
*
* @param bearer GitHub bearer token
* @param apiVersion GitHub API version
* @param org organization name
* @param repo repository name
* @param prNumber pull request number
* @return List of all comments from the pull request
*/
default List<GithubPullRequestComment> getAllPullRequestComments(String bearer, String apiVersion, String org, String repo,
int prNumber) {
List<GithubPullRequestComment> allComments = new ArrayList<>();
int page = 1;
int perPage = 100; // GitHub's maximum items per page
// Given that there's no pagination in the response, we need to query until we get an empty response, that would mean that we've
// reached the end
while (true) {
RestResponse<List<GithubPullRequestComment>> response = getComments(bearer, apiVersion, org, repo, prNumber, perPage, page);
List<GithubPullRequestComment> comments = response.getEntity();
if (Objects.isNull(comments) || comments.isEmpty()) {
break;
}
allComments.addAll(comments);
page++;
}
return allComments;
}
}
/*
* Copyright (c) 2025 Eclipse Foundation
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.eclipsefoundation.git.eca.api.models;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
import com.google.auto.value.AutoValue;
@AutoValue
@JsonDeserialize(builder = AutoValue_GithubPullRequestComment.Builder.class)
public abstract class GithubPullRequestComment {
public abstract String getBody();
public abstract int getId();
public abstract GithubUser getUser();
@AutoValue
@JsonDeserialize(builder = AutoValue_GithubPullRequestComment_GithubUser.Builder.class)
public abstract static class GithubUser {
public abstract int getId();
public static Builder builder() {
return new AutoValue_GithubPullRequestComment_GithubUser.Builder();
}
@AutoValue.Builder
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
public abstract Builder setId(int id);
public abstract GithubUser build();
}
}
public static Builder builder() {
return new AutoValue_GithubPullRequestComment.Builder();
}
@AutoValue.Builder
@JsonPOJOBuilder(withPrefix = "set")
public abstract static class Builder {
public abstract Builder setBody(String body);
public abstract Builder setId(int id);
public abstract Builder setUser(GithubUser user);
public abstract GithubPullRequestComment build();
}
}
......@@ -32,6 +32,7 @@ import org.eclipsefoundation.git.eca.api.models.GithubApplicationInstallationDat
import org.eclipsefoundation.git.eca.api.models.GithubCommit;
import org.eclipsefoundation.git.eca.api.models.GithubCommit.ParentCommit;
import org.eclipsefoundation.git.eca.api.models.GithubCommitStatusRequest;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestComment;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestCommentRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
......@@ -145,7 +146,7 @@ public class GithubHelper {
}
LOGGER.debug("Found {} commits for '{}#{}'", commitShas.size(), fullRepoName, prNo);
// retrieve the webhook tracking info, or generate an entry to track this PR if it's missing.
// retrieve the webhook tracking info, or generate an entry to track this PR
GithubWebhookTracking updatedTracking = retrieveAndUpdateTrackingInformation(wrapper, installationId, org, repoName,
prResponse.get());
if (updatedTracking == null) {
......@@ -491,8 +492,7 @@ public class GithubHelper {
* @throws IllegalArgumentException if the request parameter is null
*/
private void commentOnFailure(GithubWebhookRequest request, Set<String> usernames, Set<String> errors) {
// This should never happen given the logic behind the getPassed, but adding this just in
// case that logic changes
// This should never happen given the logic behind the getPassed, but adding this just in case that logic changes
if (errors.isEmpty()) {
return;
}
......@@ -502,10 +502,27 @@ public class GithubHelper {
String repositoryName = request.getRepository().getName();
Integer pullRequestNumber = request.getPullRequest().getNumber();
// Get existing comments using pagination
List<GithubPullRequestComment> comments = ghApi
.getAllPullRequestComments(ghBearerString, apiVersion, login, repositoryName, pullRequestNumber);
Set<String> nonMentionedUsers = usernames
.stream()
.filter(username -> comments
.stream()
.noneMatch(comment -> Objects.requireNonNullElse(comment.getBody(), "").contains(String.format("@%s", username))))
.collect(Collectors.toSet());
// If all the users have already been mentioned, skip commenting
if (nonMentionedUsers.isEmpty()) {
LOGGER.debug("All users have already been mentioned in the comments, skipping comment creation.");
return;
}
GithubPullRequestCommentRequest comment = GithubPullRequestCommentRequest
.builder()
.setBody(failureMessage.data("reasons", errors).data("usernames", usernames).render())
.setCommitId(Objects.requireNonNull(request, "Request cannot be null").getPullRequest().getHead().getSha())
.setCommitId(request.getPullRequest().getHead().getSha())
.setPath(".")
.build();
......@@ -514,7 +531,7 @@ public class GithubHelper {
jwtHelper.getGithubAccessToken(request.getInstallation().getId()).getToken());
LOGGER
.trace("Adding comment to PR {} in repo {}/{}: {}", pullRequestNumber, TransformationHelper.formatLog(login),
.trace("Adding new comment to PR {} in repo {}/{}: {}", pullRequestNumber, TransformationHelper.formatLog(login),
TransformationHelper.formatLog(repositoryName), comment.getBody());
ghApi.addComment(ghBearerString, apiVersion, login, repositoryName, pullRequestNumber, comment);
......
......@@ -54,6 +54,7 @@ class GithubHelperTest {
@Test
void testHandleGithubWebhookValidation_WithErrors() {
// Set up test data for the PR
String orgName = "test-org";
String repoName = "test-repo";
int prNumber = 42;
......@@ -61,9 +62,11 @@ class GithubHelperTest {
Commit testCommit1 = createTestCommit("abc123", "testUser", "test@example.com");
Commit testCommit2 = createTestCommit("def456", "testUser2", "test2@example.com");
// Create webhook request simulating a GitHub PR event
GithubWebhookRequest request = createTestWebhookRequest("12345", repoName, orgName, prNumber, "abc123");
List<Commit> commits = List.of(testCommit1, testCommit2);
// Build a validation request as the service would receive it
ValidationRequest vr = ValidationRequest
.builder()
.setRepoUrl(URI.create("https://github.com/" + orgName + "/" + repoName))
......@@ -71,13 +74,17 @@ class GithubHelperTest {
.setCommits(commits)
.build();
// Create a mock validation response with multiple errors
ValidationResponse mockResponse = createErrorValidationResponse(
Map.of(testCommit1, List.of("Missing ECA for user"), testCommit2, List.of("Missing ECA for user", "Another error")));
// Mock validation service to return our error response
when(validationService.validateIncomingRequest(Mockito.any(), Mockito.any())).thenReturn(mockResponse);
// Execute the validation handler
helper.handleGithubWebhookValidation(request, vr, null);
// Prepare the expected error comment that should be posted to GitHub
var expectedRequestBody = GithubPullRequestCommentRequest
.builder()
.setBody(
......@@ -104,11 +111,53 @@ class GithubHelperTest {
.setPath(".")
.build();
// Verify that the correct error comment was added to the PR
verify(ghApi)
.addComment(Mockito.anyString(), Mockito.anyString(), Mockito.eq(orgName), Mockito.eq(repoName), Mockito.eq(prNumber),
Mockito.eq(expectedRequestBody));
}
@Test
void testHandleGithubWebhookValidation_NoErrors() {
// Set up test data for the PR
String orgName = "test-org";
String repoName = "test-repo";
int prNumber = 42;
Commit testCommit = createTestCommit("abc123", "testUser", "test@example.com");
// Create webhook request simulating a GitHub PR event
GithubWebhookRequest request = createTestWebhookRequest("12345", repoName, orgName, prNumber, "abc123");
List<Commit> commits = List.of(testCommit);
// Build a validation request as the service would receive it
ValidationRequest vr = ValidationRequest
.builder()
.setRepoUrl(URI.create("https://github.com/" + orgName + "/" + repoName))
.setProvider(ProviderType.GITHUB)
.setCommits(commits)
.build();
// Create a mock successful validation response with no errors
ValidationResponse mockResponse = ValidationResponse
.builder()
.setStrictMode(false)
.setTrackedProject(true)
.setCommits(Map.of("abc123", CommitStatus.builder().build()))
.build();
// Configure mock to return our success response
when(validationService.validateIncomingRequest(Mockito.any(), Mockito.any())).thenReturn(mockResponse);
// Execute the validation handler
helper.handleGithubWebhookValidation(request, vr, null);
// Verify that no comment was added since there were no errors
verify(ghApi, Mockito.never())
.addComment(Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt(),
Mockito.any());
}
/**
* Creates a GitHub webhook request for testing purposes.
*
......
......@@ -31,6 +31,7 @@ import org.eclipsefoundation.git.eca.api.models.GithubCommit.CommitData;
import org.eclipsefoundation.git.eca.api.models.GithubCommit.GitCommitUser;
import org.eclipsefoundation.git.eca.api.models.GithubCommit.GithubCommitUser;
import org.eclipsefoundation.git.eca.api.models.GithubCommitStatusRequest;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestComment;
import org.eclipsefoundation.git.eca.api.models.GithubPullRequestCommentRequest;
import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
import org.jboss.resteasy.reactive.RestResponse;
......@@ -97,12 +98,6 @@ public class MockGithubAPI implements GithubAPI {
return Response.ok().build();
}
@Override
public Response addComment(String bearer, String apiVersion, String organization, String repo, int pullNumber,
GithubPullRequestCommentRequest comment) {
return Response.ok().build();
}
@Override
public RestResponse<List<GithubApplicationInstallationData>> getInstallations(BaseAPIParameters params, String bearer) {
throw new UnsupportedOperationException("Unimplemented method 'getInstallations'");
......@@ -118,4 +113,16 @@ public class MockGithubAPI implements GithubAPI {
throw new UnsupportedOperationException("Unimplemented method 'getInstallationRepositories'");
}
@Override
public RestResponse<List<GithubPullRequestComment>> getComments(String bearer, String apiVersion, String organization, String repo,
int pullNumber, int perPage, int page) {
return RestResponse.ok();
}
@Override
public Response addComment(String bearer, String apiVersion, String organization, String repo, int pullNumber,
GithubPullRequestCommentRequest commentRequest) {
return Response.ok().build();
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment