Repository: hub4j/github-api
Pull request #1105 (2021-04-16 16:17:23)

GHIssue.addLabels and removeLabels() should return List<GHLabel> not void [#1050]
# Description 
Return `List<GHLabel>` instead of void from GHIssue.addLabels() and removeLabel(s) functions. Included `void` bridge to maintain binary compatibility.
Includes changes to Java docs and test case changes.


Patch:
diff --git a/src/main/java/org/kohsuke/github/GHIssue.java b/src/main/java/org/kohsuke/github/GHIssue.java
index 2e9d2b597..a379d2f6e 100644
--- a/src/main/java/org/kohsuke/github/GHIssue.java
+++ b/src/main/java/org/kohsuke/github/GHIssue.java
@@ -328,13 +328,15 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Labels that are already present on the target are ignored.
      *
+     * @return the complete list of labels including the new additions
      * @param names
      *            Names of the label
      * @throws IOException
      *             the io exception
      */
-    public void addLabels(String... names) throws IOException {
-        _addLabels(Arrays.asList(names));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> addLabels(String... names) throws IOException {
+        return _addLabels(Arrays.asList(names));
     }
 
     /**
@@ -342,13 +344,15 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Labels that are already present on the target are ignored.
      *
+     * @return the complete list of labels including the new additions
      * @param labels
      *            the labels
      * @throws IOException
      *             the io exception
      */
-    public void addLabels(GHLabel... labels) throws IOException {
-        addLabels(Arrays.asList(labels));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> addLabels(GHLabel... labels) throws IOException {
+        return addLabels(Arrays.asList(labels));
     }
 
     /**
@@ -356,17 +360,23 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Labels that are already present on the target are ignored.
      *
+     * @return the complete list of labels including the new additions
      * @param labels
      *            the labels
      * @throws IOException
      *             the io exception
      */
-    public void addLabels(Collection<GHLabel> labels) throws IOException {
-        _addLabels(GHLabel.toNames(labels));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> addLabels(Collection<GHLabel> labels) throws IOException {
+        return _addLabels(GHLabel.toNames(labels));
     }
 
-    private void _addLabels(Collection<String> names) throws IOException {
-        root.createRequest().with("labels", names).method("POST").withUrlPath(getIssuesApiRoute() + "/labels").send();
+    private List<GHLabel> _addLabels(Collection<String> names) throws IOException {
+        return Arrays.asList(root.createRequest()
+                .with("labels", names)
+                .method("POST")
+                .withUrlPath(getIssuesApiRoute() + "/labels")
+                .fetch(GHLabel[].class));
     }
 
     /**
@@ -374,13 +384,18 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Attempting to remove a label that is not present throws {@link GHFileNotFoundException}.
      *
+     * @return the remaining list of labels
      * @param name
      *            the name
      * @throws IOException
      *             the io exception, throws {@link GHFileNotFoundException} if label was not present.
      */
-    public void removeLabel(String name) throws IOException {
-        root.createRequest().method("DELETE").withUrlPath(getIssuesApiRoute() + "/labels", name).send();
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> removeLabel(String name) throws IOException {
+        return Arrays.asList(root.createRequest()
+                .method("DELETE")
+                .withUrlPath(getIssuesApiRoute() + "/labels", name)
+                .fetch(GHLabel[].class));
     }
 
     /**
@@ -388,13 +403,15 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Attempting to remove labels that are not present on the target are ignored.
      *
+     * @return the remaining list of labels
      * @param names
      *            the names
      * @throws IOException
      *             the io exception
      */
-    public void removeLabels(String... names) throws IOException {
-        _removeLabels(Arrays.asList(names));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> removeLabels(String... names) throws IOException {
+        return _removeLabels(Arrays.asList(names));
     }
 
     /**
@@ -402,14 +419,16 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Attempting to remove labels that are not present on the target are ignored.
      *
+     * @return the remaining list of labels
      * @param labels
      *            the labels
      * @throws IOException
      *             the io exception
      * @see #removeLabels(String...) #removeLabels(String...)
      */
-    public void removeLabels(GHLabel... labels) throws IOException {
-        removeLabels(Arrays.asList(labels));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> removeLabels(GHLabel... labels) throws IOException {
+        return removeLabels(Arrays.asList(labels));
     }
 
     /**
@@ -417,23 +436,27 @@ public class GHIssue extends GHObject implements Reactable {
      *
      * Attempting to remove labels that are not present on the target are ignored.
      *
+     * @return the remaining list of labels
      * @param labels
      *            the labels
      * @throws IOException
      *             the io exception
      */
-    public void removeLabels(Collection<GHLabel> labels) throws IOException {
-        _removeLabels(GHLabel.toNames(labels));
+    @WithBridgeMethods(void.class)
+    public List<GHLabel> removeLabels(Collection<GHLabel> labels) throws IOException {
+        return _removeLabels(GHLabel.toNames(labels));
     }
 
-    private void _removeLabels(Collection<String> names) throws IOException {
+    private List<GHLabel> _removeLabels(Collection<String> names) throws IOException {
+        List<GHLabel> remainingLabels = Collections.emptyList();
         for (String name : names) {
             try {
-                removeLabel(name);
+                remainingLabels = removeLabel(name);
             } catch (GHFileNotFoundException e) {
                 // when trying to remove multiple labels, we ignore already removed
             }
         }
+        return remainingLabels;
     }
 
     /**


Test patch:
diff --git a/src/test/java/org/kohsuke/github/GHPullRequestTest.java b/src/test/java/org/kohsuke/github/GHPullRequestTest.java
index 87a2c5f10..49730bdf5 100644
--- a/src/test/java/org/kohsuke/github/GHPullRequestTest.java
+++ b/src/test/java/org/kohsuke/github/GHPullRequestTest.java
@@ -440,22 +440,25 @@ public class GHPullRequestTest extends AbstractGitHubWireMockTest {
         String addedLabel2 = "addLabels_label_name_2";
         String addedLabel3 = "addLabels_label_name_3";
 
-        p.addLabels(addedLabel1);
+        List<GHLabel> resultingLabels = p.addLabels(addedLabel1);
+        assertEquals(1, resultingLabels.size());
+        GHLabel ghLabel = resultingLabels.get(0);
+        assertThat(ghLabel.getName(), equalTo(addedLabel1));
 
         int requestCount = mockGitHub.getRequestCount();
-        p.addLabels(addedLabel2, addedLabel3);
+        resultingLabels = p.addLabels(addedLabel2, addedLabel3);
         // multiple labels can be added with one api call
         assertThat(mockGitHub.getRequestCount(), equalTo(requestCount + 1));
 
-        Collection<GHLabel> labels = getRepository().getPullRequest(p.getNumber()).getLabels();
-        assertEquals(3, labels.size());
-        assertThat(labels,
+        assertEquals(3, resultingLabels.size());
+        assertThat(resultingLabels,
                 containsInAnyOrder(hasProperty("name", equalTo(addedLabel1)),
                         hasProperty("name", equalTo(addedLabel2)),
                         hasProperty("name", equalTo(addedLabel3))));
 
         // Adding a label which is already present does not throw an error
-        p.addLabels(addedLabel1);
+        resultingLabels = p.addLabels(ghLabel);
+        assertThat(resultingLabels.size(), equalTo(3));
     }
 
     @Test
@@ -471,9 +474,8 @@ public class GHPullRequestTest extends AbstractGitHubWireMockTest {
         GHPullRequest p2 = getRepository().getPullRequest(p1.getNumber());
         p2.addLabels(addedLabel2);
 
-        p1.addLabels(addedLabel1);
+        Collection<GHLabel> labels = p1.addLabels(addedLabel1);
 
-        Collection<GHLabel> labels = getRepository().getPullRequest(p1.getNumber()).getLabels();
         assertEquals(2, labels.size());
         assertThat(labels,
                 containsInAnyOrder(hasProperty("name", equalTo(addedLabel1)),
@@ -491,19 +493,19 @@ public class GHPullRequestTest extends AbstractGitHubWireMockTest {
 
         Collection<GHLabel> labels = getRepository().getPullRequest(p.getNumber()).getLabels();
         assertEquals(3, labels.size());
+        GHLabel ghLabel3 = labels.stream().filter(label -> label3.equals(label.getName())).findFirst().get();
 
         int requestCount = mockGitHub.getRequestCount();
-        p.removeLabels(label2, label3);
+        List<GHLabel> resultingLabels = p.removeLabels(label2, label3);
         // each label deleted is a separate api call
         assertThat(mockGitHub.getRequestCount(), equalTo(requestCount + 2));
 
-        labels = getRepository().getPullRequest(p.getNumber()).getLabels();
-        assertEquals(1, labels.size());
-        assertEquals(label1, labels.iterator().next().getName());
+        assertEquals(1, resultingLabels.size());
+        assertEquals(label1, resultingLabels.get(0).getName());
 
         // Removing some labels that are not present does not throw
         // This is consistent with earlier behavior and with addLabels()
-        p.removeLabels(label3);
+        p.removeLabels(ghLabel3);
 
         // Calling removeLabel() on label that is not present will throw
         try {