Skip to content

Commit 1fefc77

Browse files
gsmetgsmet
committed
Fix concurrency issues with GHIssue addLabels and removeLabels
Fixes hub4j#1049
1 parent 199eee4 commit 1fefc77

67 files changed

Lines changed: 7773 additions & 21 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/main/java/org/kohsuke/github/GHIssue.java‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -360,17 +360,7 @@ public void addLabels(Collection<GHLabel> labels) throws IOException {
360360
}
361361

362362
private void _addLabels(Collection<String> names) throws IOException {
363-
List<String> newLabels = new ArrayList<String>();
364-
365-
for (GHLabel label : getLabels()) {
366-
newLabels.add(label.getName());
367-
}
368-
for (String name : names) {
369-
if (!newLabels.contains(name)) {
370-
newLabels.add(name);
371-
}
372-
}
373-
setLabels(newLabels.toArray(new String[0]));
363+
root.createRequest().with("labels", names).method("POST").withUrlPath(getIssuesApiRoute() + "/labels").send();
374364
}
375365

376366
/**
@@ -411,15 +401,9 @@ public void removeLabels(Collection<GHLabel> labels) throws IOException {
411401
}
412402

413403
private void _removeLabels(Collection<String> names) throws IOException {
414-
List<String> newLabels = new ArrayList<String>();
415-
416-
for (GHLabel l : getLabels()) {
417-
if (!names.contains(l.getName())) {
418-
newLabels.add(l.getName());
419-
}
404+
for (String name : names) {
405+
root.createRequest().method("DELETE").withUrlPath(getIssuesApiRoute() + "/labels", name).send();
420406
}
421-
422-
setLabels(newLabels.toArray(new String[0]));
423407
}
424408

425409
/**

‎src/test/java/org/kohsuke/github/GHIssueEventTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void testEventsForSingleIssue() throws Exception {
1818
GHIssue issue = builder.create();
1919

2020
// Generate some events.
21-
issue.addLabels("test-label");
21+
issue.setLabels("test-label");
2222

2323
// Test that the events are present.
2424
List<GHIssueEvent> list = issue.listEvents().toList();

‎src/test/java/org/kohsuke/github/GHPullRequestTest.java‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import java.util.Collections;
1010
import java.util.List;
1111

12-
import static org.hamcrest.CoreMatchers.*;
12+
import static org.hamcrest.Matchers.*;
1313

1414
/**
1515
* @author Kohsuke Kawaguchi
@@ -421,6 +421,66 @@ public void setLabels() throws Exception {
421421
assertEquals(label, labels.iterator().next().getName());
422422
}
423423

424+
@Test
425+
// Requires push access to the test repo to pass
426+
public void addLabels() throws Exception {
427+
GHPullRequest p = getRepository().createPullRequest("addLabels", "test/stable", "master", "## test");
428+
String addedLabel1 = "addLabels_label_name_1";
429+
String addedLabel2 = "addLabels_label_name_2";
430+
String addedLabel3 = "addLabels_label_name_3";
431+
432+
p.addLabels(addedLabel1);
433+
p.addLabels(addedLabel2, addedLabel3);
434+
435+
Collection<GHLabel> labels = getRepository().getPullRequest(p.getNumber()).getLabels();
436+
assertEquals(3, labels.size());
437+
assertThat(labels,
438+
containsInAnyOrder(hasProperty("name", equalTo(addedLabel1)),
439+
hasProperty("name", equalTo(addedLabel2)),
440+
hasProperty("name", equalTo(addedLabel3))));
441+
}
442+
443+
@Test
444+
// Requires push access to the test repo to pass
445+
public void addLabelsConcurrencyIssue() throws Exception {
446+
String addedLabel1 = "addLabelsConcurrencyIssue_label_name_1";
447+
String addedLabel2 = "addLabelsConcurrencyIssue_label_name_2";
448+
449+
GHPullRequest p1 = getRepository()
450+
.createPullRequest("addLabelsConcurrencyIssue", "test/stable", "master", "## test");
451+
p1.getLabels();
452+
453+
GHPullRequest p2 = getRepository().getPullRequest(p1.getNumber());
454+
p2.addLabels(addedLabel2);
455+
456+
p1.addLabels(addedLabel1);
457+
458+
Collection<GHLabel> labels = getRepository().getPullRequest(p1.getNumber()).getLabels();
459+
assertEquals(2, labels.size());
460+
assertThat(labels,
461+
containsInAnyOrder(hasProperty("name", equalTo(addedLabel1)),
462+
hasProperty("name", equalTo(addedLabel2))));
463+
}
464+
465+
@Test
466+
// Requires push access to the test repo to pass
467+
public void removeLabels() throws Exception {
468+
GHPullRequest p = getRepository().createPullRequest("removeLabels", "test/stable", "master", "## test");
469+
String label1 = "removeLabels_label_name_1";
470+
String label2 = "removeLabels_label_name_2";
471+
String label3 = "removeLabels_label_name_3";
472+
p.setLabels(label1, label2, label3);
473+
474+
Collection<GHLabel> labels = getRepository().getPullRequest(p.getNumber()).getLabels();
475+
assertEquals(3, labels.size());
476+
477+
p.removeLabels(label2, label3);
478+
479+
labels = getRepository().getPullRequest(p.getNumber()).getLabels();
480+
assertEquals(1, labels.size());
481+
assertEquals(label1, labels.iterator().next().getName());
482+
}
483+
424484
@Test
425485
// Requires push access to the test repo to pass
426486
public void setAssignee() throws Exception {

‎src/test/resources/org/kohsuke/github/GHPullRequestTest/wiremock/addLabels/__files/orgs_hub4j-test-org-2.json‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
{
2+
"login": "hub4j-test-org",
3+
"id": 7544739,
4+
"node_id": "MDEyOk9yZ2FuaXphdGlvbjc1NDQ3Mzk=",
5+
"url": "https://api.github.com/orgs/hub4j-test-org",
6+
"repos_url": "https://api.github.com/orgs/hub4j-test-org/repos",
7+
"events_url": "https://api.github.com/orgs/hub4j-test-org/events",
8+
"hooks_url": "https://api.github.com/orgs/hub4j-test-org/hooks",
9+
"issues_url": "https://api.github.com/orgs/hub4j-test-org/issues",
10+
"members_url": "https://api.github.com/orgs/hub4j-test-org/members{/member}",
11+
"public_members_url": "https://api.github.com/orgs/hub4j-test-org/public_members{/member}",
12+
"avatar_url": "https://avatars.githubusercontent.com/u/7544739?v=4",
13+
"description": "Hub4j Test Org Description (this could be null or blank too)",
14+
"name": "Hub4j Test Org Name (this could be null or blank too)",
15+
"company": null,
16+
"blog": "https://hub4j.url.io/could/be/null",
17+
"location": "Hub4j Test Org Location (this could be null or blank too)",
18+
"email": "[email protected]",
19+
"twitter_username": null,
20+
"is_verified": false,
21+
"has_organization_projects": true,
22+
"has_repository_projects": true,
23+
"public_repos": 12,
24+
"public_gists": 0,
25+
"followers": 0,
26+
"following": 0,
27+
"html_url": "https://github.com/hub4j-test-org",
28+
"created_at": "2014-05-10T19:39:11Z",
29+
"updated_at": "2020-06-04T05:56:10Z",
30+
"type": "Organization",
31+
"total_private_repos": 2,
32+
"owned_private_repos": 2,
33+
"private_gists": 0,
34+
"disk_usage": 154,
35+
"collaborators": 0,
36+
"billing_email": "[email protected]",
37+
"default_repository_permission": "none",
38+
"members_can_create_repositories": false,
39+
"two_factor_requirement_enabled": false,
40+
"members_can_create_pages": true,
41+
"members_can_create_public_pages": true,
42+
"members_can_create_private_pages": true,
43+
"plan": {
44+
"name": "free",
45+
"space": 976562499,
46+
"private_repos": 10000,
47+
"filled_seats": 22,
48+
"seats": 3
49+
}
50+
}

0 commit comments

Comments
 (0)