Skip to content

Commit

Permalink
Fix GitHubService when adding a new history issue and one already exi…
Browse files Browse the repository at this point in the history
…sts whose title is the prefix of the new one
  • Loading branch information
yrodiere committed May 2, 2024
1 parent 3b00dfe commit 1f42c35
Show file tree
Hide file tree
Showing 11 changed files with 327 additions and 179 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private List<Participant> registerParticipants(DrawRef drawRef, Lottery lottery,
continue;
}

if (notifier.hasClosedDedicatedIssue(username)) {
if (notifier.isIgnoring(username)) {
Log.debugf("Skipping user %s whose dedicated issue is closed", username);
continue;
}
Expand Down

This file was deleted.

211 changes: 107 additions & 104 deletions src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.BinaryOperator;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Stream;

import io.quarkus.github.lottery.message.MessageFormatter;
import io.quarkus.github.lottery.util.Streams;
import org.kohsuke.github.GHDirection;
import org.kohsuke.github.GHIssue;
import org.kohsuke.github.GHIssueComment;
Expand All @@ -33,6 +30,8 @@
import io.quarkiverse.githubapp.GitHubClientProvider;
import io.quarkiverse.githubapp.GitHubConfigFileProvider;
import io.quarkus.github.lottery.config.LotteryConfig;
import io.quarkus.github.lottery.message.MessageFormatter;
import io.quarkus.github.lottery.util.Streams;
import io.quarkus.logging.Log;
import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient;

Expand Down Expand Up @@ -200,7 +199,7 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set<String> initialActio
queryCommentsBuilder.since(Date.from(lastEventActionSideInstant));
}

Optional<GHIssueComment> lastComment = toStream(queryCommentsBuilder.list()).reduce(last());
Optional<GHIssueComment> lastComment = toStream(queryCommentsBuilder.list()).reduce(Streams.last());
if (lastComment.isEmpty()) {
// No action since the label was assigned.
return IssueActionSide.TEAM;
Expand All @@ -216,117 +215,124 @@ private Function<GHIssue, Issue> toIssueRecord() {
}

/**
* Checks whether an issue identified by its assignee and topic (title prefix) exists, but has been closed.
* Retrieves a topic backed by GitHub issues.
*
* @param assignee The GitHub username of issue assignee.
* @param topic The issue's topic, a string that should be prefixed to the issue title.
*
* @throws IOException If a GitHub API call fails.
* @throws java.io.UncheckedIOException If a GitHub API call fails.
* @see #commentOnDedicatedIssue(String, String, String, String)
* @param ref The topic reference.
*/
public boolean hasClosedDedicatedIssue(String assignee, String topic)
throws IOException {
var existingIssue = getDedicatedIssue(assignee, topic);
return existingIssue.isPresent() && GHIssueState.CLOSED.equals(existingIssue.get().getState());
public Topic topic(TopicRef ref) {
return new Topic(ref);
}

/**
* Adds a comment to an issue identified by its assignee and topic (title prefix).
*
* @param assignee The GitHub username of issue assignee.
* Two calls with the same username + topic will result in a comment on the same issue.
* @param topic The issue's topic, a string that will be prefixed to the issue title.
* Two calls with the same username + topic will result in a comment on the same issue.
* @param topicSuffix A string that should be appended to the topic in the issue title.
* Each time that suffix changes for a new comment,
* the issue title will be updated,
* and so will the subject of any email notification sent as a result of that comment.
* In conversation-based email clients such as GMail,
* this will result in the comment appearing in a new conversation,
* which can be useful to avoid huge conversations
* @param markdownBody The body of the comment to add.
*
* @throws IOException If a GitHub API call fails.
* @throws java.io.UncheckedIOException If a GitHub API call fails.
*/
public void commentOnDedicatedIssue(String assignee, String topic, String topicSuffix, String markdownBody)
throws IOException {
String targetTitle = topic + topicSuffix;
var existingIssue = getDedicatedIssue(assignee, topic);
GHIssue issue;
if (existingIssue.isPresent()) {
issue = existingIssue.get();
if (!issue.getTitle().equals(targetTitle)) {
issue.setTitle(targetTitle);
}
if (GHIssueState.CLOSED.equals(issue.getState())) {
issue.reopen();
}
try {
// We try to minimize the last comment on a best-effort basis,
// taking into account only recent comments,
// to avoid performance hogs on issues with many comments.
// (There's no way to retrieve comments of an issue in anti-chronological order...)
Optional<GHIssueComment> lastRecentComment = getAppCommentsSince(issue,
clock.instant().minus(21, ChronoUnit.DAYS))
.reduce(last());
lastRecentComment.ifPresent(this::minimizeOutdatedComment);
} catch (Exception e) {
Log.errorf(e, "Failed to minimize last notification for issue %s#%s", ref.repositoryName(), issue.getNumber());
}
public class Topic {
private final TopicRef ref;

// Update the issue description with the content of the latest comment,
// for convenience.
// This must be done before the comment, so that notifications triggered by the comment are only sent
// when the issue is fully updated.
issue.setBody(messageFormatter.formatDedicatedIssueBodyMarkdown(topic, markdownBody));
} else {
issue = createDedicatedIssue(assignee, targetTitle, topic, markdownBody);
private Topic(TopicRef ref) {
this.ref = ref;
}

issue.comment(markdownBody);
}
/**
* Checks whether all issues of this topic exist, but have been closed.
*
* @throws IOException If a GitHub API call fails.
* @throws java.io.UncheckedIOException If a GitHub API call fails.
* @see #comment(String, String)
*/
public boolean isClosed() throws IOException {
var existingIssue = getDedicatedIssues().findFirst();
return existingIssue.isPresent() && GHIssueState.CLOSED.equals(existingIssue.get().getState());
}

public Stream<String> extractCommentsFromDedicatedIssue(String assignee, String topic, Instant since)
throws IOException {
return getDedicatedIssue(assignee, topic)
.map(uncheckedIO(issue -> getAppCommentsSince(issue, since)))
.orElse(Stream.of())
.map(GHIssueComment::getBody);
}
/**
* Adds a comment to an issue identified by its assignee and topic (title prefix).
*
* @param topicSuffix A string that should be appended to the topic in the issue title.
* Each time that suffix changes for a new comment,
* the issue title will be updated,
* and so will the subject of any email notification sent as a result of that comment.
* In conversation-based email clients such as GMail,
* this will result in the comment appearing in a new conversation,
* which can be useful to avoid huge conversations.
* @param markdownBody The body of the comment to add.
*
* @throws IOException If a GitHub API call fails.
* @throws java.io.UncheckedIOException If a GitHub API call fails.
*/
public void comment(String topicSuffix, String markdownBody)
throws IOException {
var dedicatedIssue = getDedicatedIssues().findFirst();
if (ref.expectedSuffixStart() != null && !topicSuffix.startsWith(ref.expectedSuffixStart())
|| ref.expectedSuffixStart() == null && !topicSuffix.isEmpty()) {
throw new IllegalArgumentException(
"expectedSuffixStart = '%s' but topicSuffix = '%s'".formatted(ref.expectedSuffixStart(), topicSuffix));
}
String targetTitle = ref.topic() + topicSuffix;
GHIssue issue;
if (dedicatedIssue.isPresent()) {
issue = dedicatedIssue.get();
if (!issue.getTitle().equals(targetTitle)) {
issue.setTitle(targetTitle);
}
if (GHIssueState.CLOSED.equals(issue.getState())) {
issue.reopen();
}
try {
// We try to minimize the last comment on a best-effort basis,
// taking into account only recent comments,
// to avoid performance hogs on issues with many comments.
// (There's no way to retrieve comments of an issue in anti-chronological order...)
Optional<GHIssueComment> lastRecentComment = getAppCommentsSince(issue,
clock.instant().minus(21, ChronoUnit.DAYS))
.reduce(Streams.last());
lastRecentComment.ifPresent(GitHubRepository.this::minimizeOutdatedComment);
} catch (Exception e) {
Log.errorf(e, "Failed to minimize last notification for issue %s#%s",
GitHubRepository.this.ref.repositoryName(), issue.getNumber());
}

// Update the issue description with the content of the latest comment,
// for convenience.
// This must be done before the comment, so that notifications triggered by the comment are only sent
// when the issue is fully updated.
issue.setBody(messageFormatter.formatDedicatedIssueBodyMarkdown(ref.topic(), markdownBody));
} else {
issue = createDedicatedIssue(targetTitle, markdownBody);
}

private Optional<GHIssue> getDedicatedIssue(String assignee, String topic) throws IOException {
var builder = repository().queryIssues().creator(appLogin());
if (assignee != null) {
builder.assignee(assignee);
issue.comment(markdownBody);
}
builder.state(GHIssueState.ALL);
// Try exact match first to avoid confusion if there are two issues and one is
// the exact topic while the other just starts with the topic.
// Example:
// topic = Lottery history for quarkusio/quarkus
// issue1.title = Lottery history for quarkusio/quarkusio.github.io
// issue2.title = Lottery history for quarkusio/quarkus
for (var issue : builder.list()) {
if (issue.getTitle().equals(topic)) {
return Optional.of(issue);

private Stream<GHIssue> getDedicatedIssues() throws IOException {
var builder = repository().queryIssues().creator(appLogin());
if (ref.assignee() != null) {
builder.assignee(ref.assignee());
}
builder.state(GHIssueState.ALL);
return Streams.toStream(builder.list())
.filter(ref.expectedSuffixStart() != null
? issue -> issue.getTitle().startsWith(ref.topic() + ref.expectedSuffixStart())
// Try exact match in this case to avoid confusion if there are two issues and one is
// the exact topic while the other just starts with the topic.
// Example:
// topic = Lottery history for quarkusio/quarkus
// issue1.title = Lottery history for quarkusio/quarkusio.github.io
// issue2.title = Lottery history for quarkusio/quarkus
: issue -> issue.getTitle().equals(ref.topic()));
}
for (var issue : builder.list()) {
if (issue.getTitle().startsWith(topic)) {
return Optional.of(issue);
}

public Stream<String> extractComments(Instant since)
throws IOException {
return getDedicatedIssues()
.flatMap(uncheckedIO(issue -> getAppCommentsSince(issue, since)))
.map(GHIssueComment::getBody);
}
return Optional.empty();
}

private GHIssue createDedicatedIssue(String username, String title, String topic, String lastCommentMarkdownBody)
throws IOException {
return repository().createIssue(title)
.assignee(username)
.body(messageFormatter.formatDedicatedIssueBodyMarkdown(topic, lastCommentMarkdownBody))
.create();
private GHIssue createDedicatedIssue(String title, String lastCommentMarkdownBody)
throws IOException {
return repository().createIssue(title)
.assignee(ref.assignee())
.body(messageFormatter.formatDedicatedIssueBodyMarkdown(ref.topic(), lastCommentMarkdownBody))
.create();
}
}

private Stream<GHIssueComment> getAppCommentsSince(GHIssue issue, Instant since) {
Expand Down Expand Up @@ -355,7 +361,4 @@ mutation MinimizeOutdatedContent($subjectId: ID!) {
}
}

private <T> BinaryOperator<T> last() {
return (first, second) -> second;
}
}
30 changes: 30 additions & 0 deletions src/main/java/io/quarkus/github/lottery/github/TopicRef.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.github.lottery.github;

/**
* A reference to a {@link GitHubRepository#topic(TopicRef) topic} in a GitHub repository.
*
* @param assignee The GitHub username of issue assignee.
* @param topic The issue's topic, a string that should be prefixed to the title of dedicated issues.
* @param expectedSuffixStart If issue titles are suffixed (e.g. with the last update date),
* the (constant) string these suffixes are expected to start with.
* If issue titles are identical to the topic, a {@code null} string.
*/
public record TopicRef(String assignee,
String topic,
String expectedSuffixStart) {

public TopicRef {
if (expectedSuffixStart != null && expectedSuffixStart.isEmpty()) {
throw new IllegalArgumentException("expectedSuffixStart must not be empty; pass either null or a non-empty string");
}
}

public static TopicRef history(String topic) {
return new TopicRef(null, topic, null);
}

public static TopicRef notification(String assignee, String topic) {
return new TopicRef(assignee, topic, " (updated");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.IOException;
import java.util.List;

import io.quarkus.github.lottery.github.TopicRef;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

Expand All @@ -27,18 +28,20 @@ public class HistoryService {
public LotteryHistory fetch(DrawRef drawRef, LotteryConfig config) throws IOException {
var persistenceRepo = persistenceRepo(drawRef, config);
var history = new LotteryHistory(drawRef.instant(), config.buckets());
String historyTopic = messageFormatter.formatHistoryTopicText(drawRef);
persistenceRepo.extractCommentsFromDedicatedIssue(null, historyTopic, history.since())
persistenceRepo.topic(historyTopic(drawRef)).extractComments(history.since())
.flatMap(uncheckedIO(message -> messageFormatter.extractPayloadFromHistoryBodyMarkdown(message).stream()))
.forEach(history::add);
return history;
}

public void append(DrawRef drawRef, LotteryConfig config, List<LotteryReport.Serialized> reports) throws IOException {
var persistenceRepo = persistenceRepo(drawRef, config);
String historyTopic = messageFormatter.formatHistoryTopicText(drawRef);
String commentBody = messageFormatter.formatHistoryBodyMarkdown(drawRef, reports);
persistenceRepo.commentOnDedicatedIssue(null, historyTopic, "", commentBody);
persistenceRepo.topic(historyTopic(drawRef)).comment("", commentBody);
}

private TopicRef historyTopic(DrawRef drawRef) {
return TopicRef.history(messageFormatter.formatHistoryTopicText(drawRef));
}

GitHubRepository persistenceRepo(DrawRef drawRef, LotteryConfig config) {
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/io/quarkus/github/lottery/notification/Notifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io.quarkus.github.lottery.draw.DrawRef;
import io.quarkus.github.lottery.draw.LotteryReport;
import io.quarkus.github.lottery.github.GitHubRepository;
import io.quarkus.github.lottery.github.TopicRef;
import io.quarkus.github.lottery.message.MessageFormatter;

public class Notifier implements AutoCloseable {
Expand All @@ -24,19 +25,23 @@ public void close() {
notificationRepository.close();
}

public boolean hasClosedDedicatedIssue(String username) throws IOException {
String topic = formatter.formatNotificationTopicText(drawRef, username);
return notificationRepository.hasClosedDedicatedIssue(username, topic);
public boolean isIgnoring(String username) throws IOException {
return notificationRepository.topic(notificationTopic(username))
.isClosed();
}

public void send(LotteryReport report) throws IOException {
if (!drawRef.equals(report.drawRef())) {
throw new IllegalStateException("Cannot send reports for different draws; expected '" + drawRef
+ "', got '" + report.drawRef() + "'.");
}
String topic = formatter.formatNotificationTopicText(drawRef, report.username());
String topicSuffix = formatter.formatNotificationTopicSuffixText(report);
String body = formatter.formatNotificationBodyMarkdown(report, notificationRepository.ref());
notificationRepository.commentOnDedicatedIssue(report.username(), topic, topicSuffix, body);
notificationRepository.topic(notificationTopic(report.username()))
.comment(topicSuffix, body);
}

private TopicRef notificationTopic(String username) {
return TopicRef.notification(username, formatter.formatNotificationTopicText(drawRef, username));
}
}
5 changes: 5 additions & 0 deletions src/main/java/io/quarkus/github/lottery/util/Streams.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Spliterators;
import java.util.function.BinaryOperator;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

Expand Down Expand Up @@ -64,4 +65,8 @@ public T next() {
public static <T> Stream<T> toStream(PagedIterable<T> iterable) {
return StreamSupport.stream(iterable.spliterator(), false);
}

public static <T> BinaryOperator<T> last() {
return (first, second) -> second;
}
}
Loading

0 comments on commit 1f42c35

Please sign in to comment.