Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

1084: Automatically update repository labels from Skara config #1194

Closed
wants to merge 2 commits into from
+426 −7
Closed

1084: Automatically update repository labels from Skara config #1194

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
083f830
SKARA-1084
erikj79 Jun 23, 2021
cab38fb
Added test and log messages
erikj79 Jul 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 .gitignore
View file
Open in desktop
@@ -31,3 +31,4 @@
build/
out/
bin/
manual-test-settings.properties
15 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveWorkItem.java
View file
Open in desktop
@@ -61,14 +61,19 @@ public String toString() {

@Override
public boolean concurrentWith(WorkItem other) {
if (!(other instanceof ArchiveWorkItem)) {
return true;
if (!(other instanceof ArchiveWorkItem otherArchiveItem)) {
if (!(other instanceof LabelsUpdaterWorkItem otherLabelsUpdaterItem)) {
kevinrushforth marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
if (!bot.equals(otherLabelsUpdaterItem.bot())) {
return true;
}
return false;
}
ArchiveWorkItem otherItem = (ArchiveWorkItem)other;
if (!pr.id().equals(otherItem.pr.id())) {
if (!pr.id().equals(otherArchiveItem.pr.id())) {
return true;
}
if (!bot.codeRepo().name().equals(otherItem.bot.codeRepo().name())) {
if (!bot.codeRepo().name().equals(otherArchiveItem.bot.codeRepo().name())) {
return true;
}
return false;
78 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/LabelsUpdaterWorkItem.java
View file
Open in desktop
@@ -0,0 +1,78 @@
package org.openjdk.skara.bots.mlbridge;

import org.openjdk.skara.bot.WorkItem;
import org.openjdk.skara.issuetracker.Label;

import java.nio.file.Path;
import java.util.*;
import java.util.logging.Logger;

/**
* This WorkItem runs once when the bots starts up to update the repository
* with all mailing list labels configured for it.
*/
public class LabelsUpdaterWorkItem implements WorkItem {
private static final Logger log = Logger.getLogger(LabelsUpdaterWorkItem.class.getName());

private final MailingListBridgeBot bot;

public LabelsUpdaterWorkItem(MailingListBridgeBot bot) {
this.bot = bot;
}

public MailingListBridgeBot bot() {
return bot;
}

@Override
public boolean concurrentWith(WorkItem other) {
if (!(other instanceof LabelsUpdaterWorkItem otherItem)) {
return true;
}
if (!bot.equals(otherItem.bot)) {
return true;
}
return false;
}

@Override
public Collection<WorkItem> run(Path scratchPath) {
if (bot.labelsUpdated()) {
return List.of();
}

var existingLabelsMap = new HashMap<String, Label>();
bot.codeRepo().labels().forEach(l -> existingLabelsMap.put(l.name(), l));

var configuredLabels = bot.lists().stream()
.flatMap(configuration -> configuration.labels().stream()
.map(labelName -> new Label(labelName, configuration.list().toString())))
.toList();

for (Label configuredLabel : configuredLabels) {
var existingLabel = existingLabelsMap.get(configuredLabel.name());
if (existingLabel == null) {
log.info("Adding label: " + configuredLabel.name() + " to repo: " + bot.codeRepo().name());
bot.codeRepo().addLabel(configuredLabel);
} else if (!existingLabel.description().equals(configuredLabel.description())) {
log.info("Updating label: " + configuredLabel.name() + " with description: "
+ configuredLabel.description() + " for repo: " + bot.codeRepo().name());
bot.codeRepo().updateLabel(configuredLabel);
}
}

log.fine("Done updating labels for: " + bot.codeRepo());
bot.setLabelsUpdated(true);
return List.of();
}

@Override
public String botName() {
return MailingListBridgeBotFactory.NAME;
}

@Override
public String workItemName() {
return "labels-updater";
}
}
14 bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBot.java
View file
Open in desktop
@@ -62,6 +62,7 @@ public class MailingListBridgeBot implements Bot {

private ZonedDateTime lastPartialUpdate;
private ZonedDateTime lastFullUpdate;
private volatile boolean labelsUpdated = false;

MailingListBridgeBot(EmailAddress from, HostedRepository repo, HostedRepository archive, String archiveRef,
HostedRepository censusRepo, String censusRef, List<MailingListConfiguration> lists,
@@ -188,9 +189,22 @@ Optional<Path> seedStorage() {
return Optional.ofNullable(seedStorage);
}

public boolean labelsUpdated() {
return labelsUpdated;
}

public void setLabelsUpdated(boolean labelsUpdated) {
this.labelsUpdated = labelsUpdated;
}

@Override
public List<WorkItem> getPeriodicItems() {
List<WorkItem> ret = new LinkedList<>();

if (!labelsUpdated) {
ret.add(new LabelsUpdaterWorkItem(this));
}

List<PullRequest> prs;

if (lastFullUpdate == null || lastFullUpdate.isBefore(ZonedDateTime.now().minus(Duration.ofMinutes(10)))) {
85 bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/LabelsUpdaterTests.java
View file
Open in desktop
@@ -0,0 +1,85 @@
package org.openjdk.skara.bots.mlbridge;

import org.junit.jupiter.api.*;
import org.openjdk.skara.email.EmailAddress;
import org.openjdk.skara.test.*;

import java.io.IOException;
import java.util.List;
import java.util.Set;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class LabelsUpdaterTests {

@Test
void simple(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var listServer = new TestMailmanServer();) {
var targetRepo = credentials.getHostedRepository();
var listAddress = EmailAddress.parse(listServer.createList("test"));
var mlBot = MailingListBridgeBot.newBuilder()
.repo(targetRepo)
.lists(List.of(new MailingListConfiguration(listAddress, Set.of("foo", "bar"))))
.build();

// Check that the repo contains no labels
assertTrue(targetRepo.labels().isEmpty(), "Repo has labels from the start: " + targetRepo.labels());

// Run an archive pass
TestBotRunner.runPeriodicItems(mlBot);

assertEquals(2, targetRepo.labels().size(), "Wrong number of labels");
assertTrue(targetRepo.labels().stream()
.anyMatch(l -> l.name().equals("foo") && l.description().orElseThrow().equals(listAddress.toString())),
"No label 'foo' found");
assertTrue(targetRepo.labels().stream()
.anyMatch(l -> l.name().equals("bar") && l.description().orElseThrow().equals(listAddress.toString())),
"No label 'bar' found");

// Run again and expect no change
TestBotRunner.runPeriodicItems(mlBot);

assertEquals(2, targetRepo.labels().size(), "Wrong number of labels");
}
}

@Test
void update(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var listServer = new TestMailmanServer();) {
var targetRepo = credentials.getHostedRepository();
var listAddress = EmailAddress.parse(listServer.createList("test"));
var listAddress2 = EmailAddress.parse(listServer.createList("test2"));
var mlBot = MailingListBridgeBot.newBuilder()
.repo(targetRepo)
.lists(List.of(new MailingListConfiguration(listAddress, Set.of("foo"))))
.build();

// Check that the repo contains no labels
assertTrue(targetRepo.labels().isEmpty(), "Repo has labels from the start: " + targetRepo.labels());

// Run an archive pass
TestBotRunner.runPeriodicItems(mlBot);

assertEquals(1, targetRepo.labels().size(), "Wrong number of labels");
assertTrue(targetRepo.labels().stream()
.anyMatch(l -> l.name().equals("foo") && l.description().orElseThrow().equals(listAddress.toString())),
"No label 'foo' found");

var mlBot2 = MailingListBridgeBot.newBuilder()
.repo(targetRepo)
.lists(List.of(new MailingListConfiguration(listAddress2, Set.of("foo"))))
.build();

// Run second bot and expect label to have updated
TestBotRunner.runPeriodicItems(mlBot2);

assertEquals(1, targetRepo.labels().size(), "Wrong number of labels");
assertTrue(targetRepo.labels().stream()
.anyMatch(l -> l.name().equals("foo") && l.description().orElseThrow().equals(listAddress2.toString())),
"No label 'foo' found");
}
}
}
12 bots/tester/src/test/java/org/openjdk/skara/bots/tester/InMemoryHostedRepository.java
View file
Open in desktop
@@ -223,4 +223,16 @@ public void restrictPushAccess(Branch branch, HostUser user) {
public List<Label> labels() {
return List.of();
}

@Override
public void addLabel(Label label) {
}

@Override
public void updateLabel(Label label) {
}

@Override
public void deleteLabel(Label label) {
}
}
2 forge/build.gradle
View file
Open in desktop
@@ -25,6 +25,7 @@ module {
name = 'org.openjdk.skara.forge'
test {
requires 'org.openjdk.skara.test'
requires 'org.openjdk.skara.proxy'
requires 'org.junit.jupiter.api'
requires 'jdk.httpserver'
opens 'org.openjdk.skara.forge' to 'org.junit.platform.commons'
@@ -45,6 +46,7 @@ dependencies {
implementation project(':issuetracker')

testImplementation project(':test')
testImplementation project(':proxy')
}

publishing {
3 forge/src/main/java/org/openjdk/skara/forge/HostedRepository.java
View file
Open in desktop
@@ -90,6 +90,9 @@ URI createPullRequestUrl(HostedRepository target,
boolean canPush(HostUser user);
void restrictPushAccess(Branch branch, HostUser users);
List<Label> labels();
void addLabel(Label label);
void updateLabel(Label label);
void deleteLabel(Label label);

default PullRequest createPullRequest(HostedRepository target,
String targetRef,
33 forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java
View file
Open in desktop
@@ -582,4 +582,37 @@ public List<Label> labels() {
.map(o -> new Label(o.get("name").asString(), o.get("description").asString()))
.collect(Collectors.toList());
}

@Override
public void addLabel(Label label) {
var params = JSON.object()
.put("name", label.name())
// Color is Gray and matches all current labels
.put("color", "ededed");
Copy link
Member

@kevinrushforth kevinrushforth Jul 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this overwrite the existing color scheme? For a new repo, will it always use gray for all labels?

Copy link
Member Author

@erikj79 erikj79 Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If addLabel() is called on an existing label, then I think the color scheme will be overwritten. The intention not to change any label, just to add the ones that are missing. The updateLabel method will not change the color.

Note that the labels we are adding with this patch are just the ones for mailing lists. Do you think we need to add automation for colors for the other set of labels we use (e.g. rfr, ready, csr etc)?

Copy link
Member

@kevinrushforth kevinrushforth Jul 7, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we do add automation for colors for the other set of labels, that could be done as a follow-on fix (not sure it's needed though). I mainly wanted to make sure we wouldn't overwrite the ones for rfr, ready, etc. with gray.

if (label.description().isPresent()) {
params.put("description", label.description().get());
}
request.post("labels")
.body(params)
.execute();
}

@Override
public void updateLabel(Label label) {
var params = JSON.object();
if (label.description().isPresent()) {
params.put("description", label.description().get());
} else {
params.put("description", JSONValue.fromNull());
}
request.post("labels/" + label.name())
.body(params)
.execute();
}

@Override
public void deleteLabel(Label label) {
request.delete("labels/" + label.name())
.execute();
}
}
2 forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabHost.java
View file
Open in desktop
@@ -46,7 +46,7 @@ public class GitLabHost implements Forge {

private HostUser cachedCurrentUser = null;

GitLabHost(String name, URI uri, boolean useSsh, Credential pat, Set<String> groups) {
public GitLabHost(String name, URI uri, boolean useSsh, Credential pat, Set<String> groups) {
this.name = name;
this.uri = uri;
this.useSsh = useSsh;
34 forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabRepository.java
View file
Open in desktop
@@ -602,4 +602,38 @@ public List<Label> labels() {
.map(o -> new Label(o.get("name").asString(), o.get("description").asString()))
.collect(Collectors.toList());
}

@Override
public void addLabel(Label label) {
var params = JSON.object()
.put("name", label.name())
// Color is Blue-Gray and matches all current labels
.put("color", "#428BCA");
if (label.description().isPresent()) {
params.put("description", label.description().get());
}
request.post("labels")
.body(params)
.execute();
}

@Override
public void updateLabel(Label label) {
var params = JSON.object()
.put("new_name", label.name());
if (label.description().isPresent()) {
params.put("description", label.description().get());
} else {
throw new UnsupportedOperationException("Gitlab does not support clearing the description");
}
request.put("labels/" + label.name())
.body(params)
.execute();
}

@Override
public void deleteLabel(Label label) {
request.delete("labels/" + label.name())
.execute();
}
}