From 8313e9ffeea22a90b0dc79c66fc214753bb5c534 Mon Sep 17 00:00:00 2001 From: xjusko Date: Thu, 20 Mar 2025 12:26:13 +0100 Subject: [PATCH] Add check for incorrect rebase by checking for duplicate commits and skip PR rules check(mentions, reviewers) if any appears. --- .../wildfly/bot/PullRequestRuleProcessor.java | 4 ++ .../org/wildfly/bot/util/GithubProcessor.java | 32 ++++++++++ .../wildfly/bot/PRSkipPullRequestTest.java | 61 ++++++++++++++++++- .../java/org/wildfly/bot/PRTriggerTest.java | 1 + 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java b/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java index 8872a45..8668edb 100644 --- a/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java +++ b/src/main/java/org/wildfly/bot/PullRequestRuleProcessor.java @@ -46,6 +46,10 @@ void pullRequestRuleCheck( return; } + if (githubProcessor.hasDuplicateCommitInBase(pullRequest, pullRequestPayload.getRepository())) { + return; + } + GHRepository repository = pullRequest.getRepository(); SequencedMap> ccMentionsWithRules = new LinkedHashMap<>(); Set reviewers = new HashSet<>(); diff --git a/src/main/java/org/wildfly/bot/util/GithubProcessor.java b/src/main/java/org/wildfly/bot/util/GithubProcessor.java index 7216fe9..40d52c3 100644 --- a/src/main/java/org/wildfly/bot/util/GithubProcessor.java +++ b/src/main/java/org/wildfly/bot/util/GithubProcessor.java @@ -7,11 +7,14 @@ import jakarta.inject.Inject; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jboss.logging.Logger; +import org.kohsuke.github.GHCommit; +import org.kohsuke.github.GHCommitQueryBuilder; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHPerson; import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHPullRequestCommitDetail; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; @@ -389,4 +392,33 @@ private String createCCMentionComment(SequencedMap> ccMenti ) .toList()); } + + public boolean hasDuplicateCommitInBase(GHPullRequest pullRequest, GHRepository repository) + throws IOException { + String baseBranch = pullRequest.getBase().getRef(); + GHCommitQueryBuilder commitQuery = repository.queryCommits(); + if (commitQuery == null) { + return false; + } + + Set baseCommitSHAs = commitQuery + .from(baseBranch) + .pageSize(100) + .list() + .toSet() + .stream() + .map(GHCommit::getSHA1) + .collect(Collectors.toSet()); + + for (GHPullRequestCommitDetail prCommit : pullRequest.listCommits()) { + String prSha = prCommit.getSha(); + if (baseCommitSHAs.contains(prSha)) { + LOG.infof("Skipping rules due to incorrect rebase detected: commit %s is already in the base branch %s", + prSha, + baseBranch); + return true; + } + } + return false; + } } diff --git a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java index a009577..077bda1 100644 --- a/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java +++ b/src/test/java/org/wildfly/bot/PRSkipPullRequestTest.java @@ -4,6 +4,11 @@ import io.quarkus.test.junit.QuarkusTest; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.kohsuke.github.GHCommit; +import org.kohsuke.github.GHCommitQueryBuilder; +import org.kohsuke.github.GHPullRequestCommitDetail; +import org.kohsuke.github.PagedIterable; +import org.kohsuke.github.PagedIterator; import org.wildfly.bot.utils.TestConstants; import org.wildfly.bot.utils.WildflyGitHubBotTesting; import org.wildfly.bot.utils.mocking.Mockable; @@ -11,9 +16,18 @@ import org.wildfly.bot.utils.testing.PullRequestJson; import org.wildfly.bot.utils.testing.internal.TestModel; +import java.util.Set; + +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; @QuarkusTest @GitHubAppTest @@ -81,4 +95,49 @@ void testSkippingFormatCheckOnDraft() throws Throwable { verifyNoMoreInteractions(mocks.pullRequest(pullRequestJson.id())); }); } -} + + @Test + void testSkippingRulesOnIncorrectRebase() throws Throwable { + final String duplicateSHA = "sha1"; + final String baseBranch = "main"; + pullRequestJson = TestModel.setPullRequestJsonBuilder(pullRequestJsonBuilder -> pullRequestJsonBuilder); + mockedContext = MockedGHPullRequest.builder(pullRequestJson.id()); + TestModel.given( + mocks -> { + WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext); + GHCommit commit1 = mock(GHCommit.class); + GHCommit commit2 = mock(GHCommit.class); + + when(commit1.getSHA1()).thenReturn(duplicateSHA); + when(commit2.getSHA1()).thenReturn("sha2"); + + // Create a fake PagedIterable for repository commits. + Set fakeRepoCommitSet = Set.of(commit1, commit2); + PagedIterable fakeRepoCommits = mock(PagedIterable.class); + GHCommitQueryBuilder mockedQueryBuilder = mock(GHCommitQueryBuilder.class); + + when(mocks.repository(TestConstants.TEST_REPO).queryCommits()).thenReturn(mockedQueryBuilder); + when(mockedQueryBuilder.from(eq(baseBranch))).thenReturn(mockedQueryBuilder); + when(mockedQueryBuilder.pageSize(anyInt())).thenReturn(mockedQueryBuilder); + when(mockedQueryBuilder.list()).thenReturn(fakeRepoCommits); + when(fakeRepoCommits.toSet()).thenReturn(fakeRepoCommitSet); + + GHPullRequestCommitDetail fakePRCommit = mock(GHPullRequestCommitDetail.class); + PagedIterator mockIterator = mock(PagedIterator.class); + PagedIterable fakePRCommits = mock(PagedIterable.class); + + when(fakePRCommit.getSha()).thenReturn(duplicateSHA); + when(mockIterator.hasNext()).thenReturn(true, false); + when(mockIterator.next()).thenReturn(fakePRCommit); + when(mocks.pullRequest(pullRequestJson.id()).listCommits()).thenReturn(fakePRCommits); + when(fakePRCommits._iterator(anyInt())).thenReturn(mockIterator); + }) + .pullRequestEvent(pullRequestJson) + .then(mocks -> { + verify(mocks.pullRequest(pullRequestJson.id())).getBase(); + verify(mocks.repository(TestConstants.TEST_REPO)).queryCommits(); + verify(mocks.pullRequest(pullRequestJson.id()), never()).addLabels(nullable(String[].class)); + verify(mocks.pullRequest(pullRequestJson.id()), never()).requestReviewers(any()); + }); + } +} \ No newline at end of file diff --git a/src/test/java/org/wildfly/bot/PRTriggerTest.java b/src/test/java/org/wildfly/bot/PRTriggerTest.java index 8b15fb0..1504eea 100644 --- a/src/test/java/org/wildfly/bot/PRTriggerTest.java +++ b/src/test/java/org/wildfly/bot/PRTriggerTest.java @@ -185,6 +185,7 @@ private void checkValidRun(SsePullRequestPayload ssePullRequestPayload, Action a Mockito.verify(repo, Mockito.atMostOnce()).listLabels(); Mockito.verify(repo, Mockito.atMostOnce()).createLabel(ArgumentMatchers.anyString(), ArgumentMatchers.anyString()); + Mockito.verify(repo, Mockito.atMostOnce()).queryCommits(); Mockito.verifyNoMoreInteractions(repo); }); }