Skip to content

Commit

Permalink
Allow ignoring issues by labels for stale/stewardship
Browse files Browse the repository at this point in the history
  • Loading branch information
@yrodiere
yrodiere committed Dec 11, 2024
1 parent 9aa9762 commit 9d0e198
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 41 deletions.
14 changes: 13 additions & 1 deletion README.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,12 @@ participants:
maxIssues: 2
stale:
maxIssues: 5
ignoreLabels: ["triage/on-ice"]
----

`labels`::
The labels identifying issues you are interested in, as a maintainer.
Issues mentioned in notifications will have at least one of those labels.
Issues mentioned in notifications will have at least one of these labels.
+
Array of Strings, mandatory, no default.
`days`::
Expand All @@ -213,6 +214,11 @@ How many issues, at most, you wish to be included in the "Stale" category
for each notification.
+
Integer, mandatory, no default.
`stale.ignoreLabels`::
The labels identifying issues that should be ignored for the "Stale" category.
Issues mentioned in notifications will never have any one of these labels.
+
Array of Strings, optional, defaults to an empty array.

[[participants-stewardship]]
=== Stewardship
Expand Down Expand Up @@ -244,6 +250,7 @@ participants:
stewardship:
days: ["MONDAY"]
maxIssues: 5
ignoreLabels: ["triage/on-ice"]
----

`days`::
Expand All @@ -255,6 +262,11 @@ How many issues, at most, you wish to be included in the "stewardship" category
for each notification.
+
Integer, mandatory, no default.
`ignoreLabels`::
The labels identifying issues that should be ignored for the "stewardship" category.
Issues mentioned in notifications will never have any one of these labels.
+
Array of Strings, optional, defaults to an empty array.

[[participants-suspending]]
=== Suspending notifications
Expand Down
16 changes: 10 additions & 6 deletions src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,27 @@ public Provided(@JsonProperty(required = true) Duration delay,
}

public record Stale(
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) {
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification,
@JsonProperty(required = true) List<String> ignoreLabels) {
// https://stackoverflow.com/a/71539100/6692043
// Also gives us a less verbose constructor for tests
@JsonCreator
public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) {
this(new Notification(delay, timeout));
public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout,
@JsonProperty(required = false) List<String> ignoreLabels) {
this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels);
}
}
}

public record Stewardship(
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) {
@JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification,
@JsonProperty(required = false) List<String> ignoreLabels) {
// https://stackoverflow.com/a/71539100/6692043
// Also gives us a less verbose constructor for tests
@JsonCreator
public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) {
this(new Notification(delay, timeout));
public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout,
@JsonProperty(required = false) List<String> ignoreLabels) {
this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels);
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/main/java/io/quarkus/github/lottery/draw/Lottery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
String label = config.triage().label();
var cutoff = now.minus(config.triage().notification().delay());
var history = lotteryHistory.triage();
draws.add(triage.bucket.createDraw(repo.issuesWithLabelLastUpdatedBefore(label, cutoff)
draws.add(triage.bucket.createDraw(repo.issuesWithLabelLastUpdatedBefore(label, Set.of(), cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand Down Expand Up @@ -155,9 +155,11 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
}
if (stale.hasParticipation()) {
var cutoff = now.minus(config.maintenance().stale().notification().delay());
// Remove duplicates, but preserve order
var ignoreLabels = new LinkedHashSet<>(config.maintenance().stale().ignoreLabels());
var history = lotteryHistory.stale();
draws.add(stale.createDraw(
repo.issuesWithLabelLastUpdatedBefore(areaLabel, cutoff)
repo.issuesWithLabelLastUpdatedBefore(areaLabel, ignoreLabels, cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand All @@ -176,9 +178,10 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List<Draw
Set<Integer> allWinnings) throws IOException {
if (stewardship.bucket.hasParticipation()) {
var cutoff = now.minus(config.stewardship().notification().delay());
var ignoreLabels = new LinkedHashSet<>(config.stewardship().ignoreLabels());
var history = lotteryHistory.stewardship();
draws.add(bucket.createDraw(
repo.issuesLastUpdatedBefore(cutoff)
repo.issuesLastUpdatedBefore(ignoreLabels, cutoff)
.filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number()))
.iterator(),
allWinnings));
Expand Down
27 changes: 17 additions & 10 deletions src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static io.quarkus.github.lottery.github.GitHubSearchClauses.author;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.isIssue;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.label;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.not;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.repo;
import static io.quarkus.github.lottery.github.GitHubSearchClauses.updatedBefore;
import static io.quarkus.github.lottery.util.Streams.toStream;
Expand Down Expand Up @@ -121,37 +122,43 @@ public Optional<LotteryConfig> fetchLotteryConfig() throws IOException {
/**
* Lists issues that were last updated before the given instant.
*
* @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned).
* @param updatedBefore An instant; all returned issues must have been last updated before that instant.
* @return A lazily populated stream of matching issues.
* @throws java.io.UncheckedIOException In case of I/O failure.
*/
public Stream<Issue> issuesLastUpdatedBefore(Instant updatedBefore) {
return toStream(searchIssues()
public Stream<Issue> issuesLastUpdatedBefore(Set<String> ignoreLabels, Instant updatedBefore) {
var builder = searchIssues()
.isOpen()
.q(updatedBefore(updatedBefore))
.sort(GHIssueSearchBuilder.Sort.UPDATED)
.order(GHDirection.DESC)
.list())
.map(toIssueRecord());
.order(GHDirection.DESC);
if (!ignoreLabels.isEmpty()) {
builder.q(not(anyLabel(ignoreLabels)));
}
return toStream(builder.list()).map(toIssueRecord());
}

/**
* Lists issues with the given label that were last updated before the given instant.
*
* @param label A GitHub label; if non-null, all returned issues must have been assigned that label.
* @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned).
* @param updatedBefore An instant; all returned issues must have been last updated before that instant.
* @return A lazily populated stream of matching issues.
* @throws java.io.UncheckedIOException In case of I/O failure.
*/
public Stream<Issue> issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) {
return toStream(searchIssues()
public Stream<Issue> issuesWithLabelLastUpdatedBefore(String label, Set<String> ignoreLabels, Instant updatedBefore) {
var builder = searchIssues()
.isOpen()
.q(label(label))
.q(updatedBefore(updatedBefore))
.sort(GHIssueSearchBuilder.Sort.UPDATED)
.order(GHDirection.DESC)
.list())
.map(toIssueRecord());
.order(GHDirection.DESC);
if (!ignoreLabels.isEmpty()) {
builder.q(not(anyLabel(ignoreLabels)));
}
return toStream(builder.list()).map(toIssueRecord());
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ public final class GitHubSearchClauses {
private GitHubSearchClauses() {
}

public static String not(String clause) {
return "-" + clause;
}

public static String repo(GitHubRepositoryRef ref) {
return "repo:" + ref.repositoryName();
}
Expand Down
97 changes: 91 additions & 6 deletions src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,11 @@ void fetchLotteryConfig() throws IOException {
stale:
delay: P60D
timeout: P14D
ignoreLabels: ["triage/on-ice"]
stewardship:
delay: P60D
timeout: P14D
ignoreLabels: ["triage/on-ice"]
participants:
- username: "yrodiere"
triage:
Expand Down Expand Up @@ -233,9 +235,10 @@ void fetchLotteryConfig() throws IOException {
new LotteryConfig.Buckets.Maintenance.Feedback.Provided(
Duration.ofDays(7), Duration.ofDays(3))),
new LotteryConfig.Buckets.Maintenance.Stale(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14),
List.of("triage/on-ice"))),
new LotteryConfig.Buckets.Stewardship(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))),
List.of(
new LotteryConfig.Participant("yrodiere",
Optional.empty(),
Expand Down Expand Up @@ -324,6 +327,7 @@ void fetchLotteryConfig_minimal() throws IOException {
stewardship:
delay: P60D
timeout: P14D
participants:
""");
})
.when(() -> {
Expand All @@ -347,9 +351,9 @@ void fetchLotteryConfig_minimal() throws IOException {
new LotteryConfig.Buckets.Maintenance.Feedback.Provided(
Duration.ofDays(7), Duration.ofDays(3))),
new LotteryConfig.Buckets.Maintenance.Stale(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14), List.of())),
new LotteryConfig.Buckets.Stewardship(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14), List.of())),
List.of()));
})
.then().github(mocks -> {
Expand Down Expand Up @@ -381,7 +385,46 @@ void issuesLastUpdatedBefore() throws IOException {
.when(() -> {
var repo = gitHubService.repository(repoRef);

assertThat(repo.issuesLastUpdatedBefore(cutoff))
assertThat(repo.issuesLastUpdatedBefore(Set.of(), cutoff))
.containsExactlyElementsOf(stubIssueList(1, 3, 2, 4));
})
.then().github(mocks -> {
verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName());
verify(searchIssuesBuilderMock).q("is:issue");
verify(searchIssuesBuilderMock).isOpen();
verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00");
verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED);
verify(searchIssuesBuilderMock).order(GHDirection.DESC);
verifyNoMoreInteractions(searchIssuesBuilderMock);
verifyNoMoreInteractions(mocks.ghObjects());
});
}

@Test
void issuesLastUpdatedBefore_ignoreLabels() throws IOException {
var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus");

Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC);
Instant cutoff = now.minus(1, ChronoUnit.DAYS);

var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class,
withSettings().defaultAnswer(Answers.RETURNS_SELF));
given()
.github(mocks -> {
var clientMock = mocks.installationClient(installationRef.installationId());

when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock);
var issue1Mock = mockIssueForLottery(mocks, 1);
var issue2Mock = mockIssueForLottery(mocks, 3);
var issue3Mock = mockIssueForLottery(mocks, 2);
var issue4Mock = mockIssueForLottery(mocks, 4);
var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock);
when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks);
})
.when(() -> {
var repo = gitHubService.repository(repoRef);

assertThat(repo.issuesLastUpdatedBefore(Set.of("triage/on-ice"), cutoff))
.containsExactlyElementsOf(stubIssueList(1, 3, 2, 4));
})
.then().github(mocks -> {
Expand All @@ -391,6 +434,7 @@ void issuesLastUpdatedBefore() throws IOException {
verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00");
verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED);
verify(searchIssuesBuilderMock).order(GHDirection.DESC);
verify(searchIssuesBuilderMock).q("-label:triage/on-ice");
verifyNoMoreInteractions(searchIssuesBuilderMock);
verifyNoMoreInteractions(mocks.ghObjects());
});
Expand Down Expand Up @@ -420,7 +464,47 @@ void issuesWithLabelLastUpdatedBefore() throws IOException {
.when(() -> {
var repo = gitHubService.repository(repoRef);

assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", cutoff))
assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of(), cutoff))
.containsExactlyElementsOf(stubIssueList(1, 3, 2, 4));
})
.then().github(mocks -> {
verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName());
verify(searchIssuesBuilderMock).q("is:issue");
verify(searchIssuesBuilderMock).isOpen();
verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED);
verify(searchIssuesBuilderMock).order(GHDirection.DESC);
verify(searchIssuesBuilderMock).q("label:triage/needs-triage");
verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00");
verifyNoMoreInteractions(searchIssuesBuilderMock);
verifyNoMoreInteractions(mocks.ghObjects());
});
}

@Test
void issuesWithLabelLastUpdatedBefore_ignoreLabels() throws IOException {
var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus");

Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC);
Instant cutoff = now.minus(1, ChronoUnit.DAYS);

var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class,
withSettings().defaultAnswer(Answers.RETURNS_SELF));
given()
.github(mocks -> {
var clientMock = mocks.installationClient(installationRef.installationId());

when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock);
var issue1Mock = mockIssueForLottery(mocks, 1);
var issue2Mock = mockIssueForLottery(mocks, 3);
var issue3Mock = mockIssueForLottery(mocks, 2);
var issue4Mock = mockIssueForLottery(mocks, 4);
var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock);
when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks);
})
.when(() -> {
var repo = gitHubService.repository(repoRef);

assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of("triage/on-ice"), cutoff))
.containsExactlyElementsOf(stubIssueList(1, 3, 2, 4));
})
.then().github(mocks -> {
Expand All @@ -431,6 +515,7 @@ void issuesWithLabelLastUpdatedBefore() throws IOException {
verify(searchIssuesBuilderMock).order(GHDirection.DESC);
verify(searchIssuesBuilderMock).q("label:triage/needs-triage");
verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00");
verify(searchIssuesBuilderMock).q("-label:triage/on-ice");
verifyNoMoreInteractions(searchIssuesBuilderMock);
verifyNoMoreInteractions(mocks.ghObjects());
});
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ private static LotteryConfig defaultConfig() {
new LotteryConfig.Buckets.Maintenance.Feedback.Provided(
Duration.ofDays(7), Duration.ofDays(3))),
new LotteryConfig.Buckets.Maintenance.Stale(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))),
new LotteryConfig.Buckets.Stewardship(
Duration.ofDays(60), Duration.ofDays(14))),
Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))),
List.of());
}

Expand Down
Loading

0 comments on commit 9d0e198

Please sign in to comment.