Skip to content

Commit

Permalink
1919: Make it possible to disable the "/clean" command per repository
Browse files Browse the repository at this point in the history
Reviewed-by: erikj
  • Loading branch information
zhaosongzs committed Apr 4, 2024
1 parent 42cae16 commit 7b71ff0
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -53,6 +53,11 @@ public boolean allowedInBody() {
@Override
public void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, ScratchArea scratchArea, CommandInvocation command, List<Comment> allComments, PrintWriter reply)
{
if (!bot.cleanCommandEnabled()) {
reply.println("The `/clean` pull request command is not enabled for this repository");
return;
}

if (!censusInstance.isCommitter(command.user())) {
reply.println("Only OpenJDK [Committers](https://openjdk.org/bylaws#committer) can use the `/clean` command");
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -80,6 +80,7 @@ class PullRequestBot implements Bot {
private final Approval approval;
private boolean initialRun = true;
private final boolean versionMismatchWarning;
private final boolean cleanCommandEnabled;

private Instant lastFullUpdate;

Expand All @@ -94,7 +95,7 @@ class PullRequestBot implements Bot {
Set<String> integrators, Set<Integer> excludeCommitCommentsFrom, boolean enableCsr, boolean enableJep,
boolean reviewCleanBackport, String mlbridgeBotName, MergePullRequestReviewConfiguration reviewMerge, boolean processPR, boolean processCommit,
boolean enableMerge, Set<String> mergeSources, boolean jcheckMerge, boolean enableBackport,
Map<String, List<PRRecord>> issuePRMap, Approval approval, boolean versionMismatchWarning) {
Map<String, List<PRRecord>> issuePRMap, Approval approval, boolean versionMismatchWarning, boolean cleanCommandEnabled) {
remoteRepo = repo;
this.censusRepo = censusRepo;
this.censusRef = censusRef;
Expand Down Expand Up @@ -131,6 +132,7 @@ class PullRequestBot implements Bot {
this.issuePRMap = issuePRMap;
this.approval = approval;
this.versionMismatchWarning = versionMismatchWarning;
this.cleanCommandEnabled = cleanCommandEnabled;

autoLabelled = new HashSet<>();
poller = new PullRequestPoller(repo, true);
Expand Down Expand Up @@ -403,6 +405,10 @@ public boolean versionMismatchWarning() {
return versionMismatchWarning;
}

public boolean cleanCommandEnabled() {
return cleanCommandEnabled;
}

public void addIssuePRMapping(String issueId, PRRecord prRecord) {
issuePRMap.putIfAbsent(issueId, new LinkedList<>());
List<PRRecord> prRecords = issuePRMap.get(issueId);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -67,6 +67,7 @@ public class PullRequestBotBuilder {
private Map<String, List<PRRecord>> issuePRMap;
private Approval approval = null;
private boolean versionMismatchWarning = false;
private boolean cleanCommandEnabled = true;

PullRequestBotBuilder() {
}
Expand Down Expand Up @@ -251,12 +252,17 @@ public PullRequestBotBuilder versionMismatchWarning(boolean versionMismatchWarni
return this;
}

public PullRequestBotBuilder cleanCommandEnabled(boolean cleanCommandEnabled) {
this.cleanCommandEnabled = cleanCommandEnabled;
return this;
}

public PullRequestBot build() {
return new PullRequestBot(repo, censusRepo, censusRef, labelConfiguration, externalPullRequestCommands,
externalCommitCommands, blockingCheckLabels, readyLabels, twoReviewersLabels, twentyFourHoursLabels,
readyComments, issueProject, ignoreStaleReviews, allowedTargetBranches, seedStorage, confOverrideRepo,
confOverrideName, confOverrideRef, censusLink, forks, integrators, excludeCommitCommentsFrom, enableCsr,
enableJep, reviewCleanBackport, mlbridgeBotName, reviewMerge, processPR, processCommit, enableMerge,
mergeSources, jcheckMerge, enableBackport, issuePRMap, approval, versionMismatchWarning);
mergeSources, jcheckMerge, enableBackport, issuePRMap, approval, versionMismatchWarning, cleanCommandEnabled);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -263,6 +263,10 @@ public List<Bot> create(BotConfiguration configuration) {
botBuilder.versionMismatchWarning(repo.value().get("versionMismatchWarning").asBoolean());
}

if (repo.value().contains("cleanCommandEnabled")) {
botBuilder.cleanCommandEnabled(repo.value().get("cleanCommandEnabled").asBoolean());
}

var prBot = botBuilder.build();
pullRequestBotMap.put(repository.name(), prBot);
ret.add(prBot);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,4 +347,74 @@ void missingBackportHash(TestInfo testInfo) throws IOException {
assertLastCommentContains(pr, ", with an original hash, as clean");
}
}

@Test
void cleanCommandDisabled(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory(false)) {

var author = credentials.getHostedRepository();
var integrator = credentials.getHostedRepository();
var issues = credentials.getIssueProject();
var censusBuilder = credentials.getCensusBuilder()
.addCommitter(author.forge().currentUser().id())
.addReviewer(integrator.forge().currentUser().id());
var bot = PullRequestBot.newBuilder()
.repo(integrator)
.censusRepo(censusBuilder.build())
.issueProject(issues)
.issuePRMap(new HashMap<>())
.cleanCommandEnabled(false)
.build();

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());

var newFile = localRepo.root().resolve("a_new_file.txt");
Files.writeString(newFile, "a\nb\nc\nd");
localRepo.add(newFile);
var issue1 = credentials.createIssue(issues, "An issue");
var issue1Number = issue1.id().split("-")[1];
var originalMessage = issue1Number + ": An issue\n" +
"\n" +
"Reviewed-by: integrationreviewer2";
var masterHash = localRepo.commit(originalMessage, "integrationcommitter1", "integrationcommitter1@openjdk.org");

localRepo.push(masterHash, author.authenticatedUrl(), "master", true);

var releaseBranch = localRepo.branch(masterHash, "release");
localRepo.checkout(releaseBranch);
Files.writeString(newFile, "a\nb\nc\nd\ne");
localRepo.add(newFile);
var issue2 = credentials.createIssue(issues, "Another issue");
var issue2Number = issue2.id().split("-")[1];
var upstreamMessage = issue2Number + ": Another issue\n" +
"\n" +
"Reviewed-by: integrationreviewer2";
var upstreamHash = localRepo.commit(upstreamMessage, "integrationcommitter1", "integrationcommitter1@openjdk.org");
localRepo.push(upstreamHash, author.authenticatedUrl(), "refs/heads/release", true);

// "backport" the new file to the master branch
localRepo.checkout(localRepo.defaultBranch());
var editBranch = localRepo.branch(masterHash, "edit");
localRepo.checkout(editBranch);
Files.writeString(newFile, "a\nb\nc\nd\nd");
localRepo.add(newFile);
var editHash = localRepo.commit("Backport", "duke", "duke@openjdk.org");
localRepo.push(editHash, author.authenticatedUrl(), "refs/heads/edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "Backport " + upstreamHash.hex());

TestBotRunner.runPeriodicItems(bot);

// The bot should not have added the "clean" label
assertFalse(pr.store().labelNames().contains("clean"));

// Use the "/clean" pull request command to mark the backport PR as clean
pr.addComment("/clean");
TestBotRunner.runPeriodicItems(bot);
// The pr shouldn't have clean label since clean command is disabled
assertFalse(pr.store().labelNames().contains("clean"));
assertLastCommentContains(pr, "The `/clean` pull request command is not enabled for this repository");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2023 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -156,6 +156,7 @@ public void testCreate() {
}
},
"versionMismatchWarning": true,
"cleanCommandEnabled": false,
}
},
"forks": {
Expand Down Expand Up @@ -208,6 +209,7 @@ public void testCreate() {
assertFalse(pullRequestBot5.jcheckMerge());
assertTrue(pullRequestBot5.enableBackport());
assertFalse(pullRequestBot5.versionMismatchWarning());
assertTrue(pullRequestBot5.cleanCommandEnabled());

var pullRequestBot6 = (PullRequestBot) bots.stream()
.filter(bot -> bot.toString().equals("PullRequestBot@repo6"))
Expand Down Expand Up @@ -242,6 +244,7 @@ public void testCreate() {
assertFalse(pullRequestBot7.jcheckMerge());
assertEquals("https://example.com", pullRequestBot7.approval().documentLink());
assertTrue(pullRequestBot7.versionMismatchWarning());
assertFalse(pullRequestBot7.cleanCommandEnabled());

var csrIssueBot1 = (CSRIssueBot) bots.stream()
.filter(bot -> bot.toString().equals("CSRIssueBot@TEST"))
Expand Down

0 comments on commit 7b71ff0

Please sign in to comment.