Do not populate repo list in case of installation:deleted event (#1690)

* Do not populate repo list in case of installation:deleted event * Implement discussed way to handle the situation * Fix codestyle * Fix build (I hope?) * Update GHEventPayload.java * Update GHEventPayload.java * Fix missing import * Update src/test/java/org/kohsuke/github/GHEventPayloadTest.java * Update src/test/java/org/kohsuke/github/GHEventPayloadTest.java * Apply suggestions from code review * Update test data --------- Co-authored-by: Liam Newman <bitwiseman@gmail.com>
hub4j · Mar 14, 2024 · eb269bd · eb269bd
1 parent 0fac2a9
commit eb269bd
Show file tree

Hide file tree

Showing 12 changed files with 766 additions and 22 deletions.
diff --git a/src/main/java/org/kohsuke/github/GHEventPayload.java b/src/main/java/org/kohsuke/github/GHEventPayload.java
@@ -1,10 +1,12 @@
 package org.kohsuke.github;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonSetter;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 import java.io.IOException;
 import java.io.Reader;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
@@ -265,16 +267,47 @@ void lateBind() {
      * @see <a href="https://docs.github.com/en/rest/reference/apps#installations">GitHub App Installation</a>
      */
     public static class Installation extends GHEventPayload {
-        private List<GHRepository> repositories;
+
+        private List<Repository> repositories;
+        private List<GHRepository> ghRepositories = null;
 
         /**
-         * Gets repositories.
+         * Gets repositories. For the "deleted" action please rather call {@link #getRawRepositories()}
          *
          * @return the repositories
          */
         public List<GHRepository> getRepositories() {
+            if ("deleted".equalsIgnoreCase(getAction())) {
+                throw new IllegalStateException("Can't call #getRepositories() on Installation event "
+                        + "with 'deleted' action. Call #getRawRepositories() instead.");
+            }
+
+            if (ghRepositories == null) {
+                ghRepositories = new ArrayList<>(repositories.size());
+                try {
+                    for (Repository singleRepo : repositories) {
+                        // populate each repository
+                        // the repository information provided here is so limited
+                        // as to be unusable without populating, so we do it eagerly
+                        ghRepositories.add(this.root().getRepositoryById(singleRepo.getId()));
+                    }
+                } catch (IOException e) {
+                    throw new GHException("Failed to refresh repositories", e);
+                }
+            }
+
+            return Collections.unmodifiableList(ghRepositories);
+        }
+
+        /**
+         * Returns a list of raw, unpopulated repositories. Useful when calling from within Installation event with
+         * action "deleted". You can't fetch the info for repositories of an already deleted installation.
+         *
+         * @return the list of raw Repository records
+         */
+        public List<Repository> getRawRepositories() {
             return Collections.unmodifiableList(repositories);
-        };
+        }
 
         /**
          * Late bind.
@@ -286,17 +319,64 @@ void lateBind() {
                         "Expected installation payload, but got something else. Maybe we've got another type of event?");
             }
             super.lateBind();
-            if (repositories != null && !repositories.isEmpty()) {
-                try {
-                    for (GHRepository singleRepo : repositories) {
-                        // populate each repository
-                        // the repository information provided here is so limited
-                        // as to be unusable without populating, so we do it eagerly
-                        singleRepo.populate();
-                    }
-                } catch (IOException e) {
-                    throw new GHException("Failed to refresh repositories", e);
-                }
+        }
+
+        /**
+         * A special minimal implementation of a {@link GHRepository} which contains only fields from "Properties of
+         * repositories" from <a href=
+         * "https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads#installation">here</a>
+         */
+        public static class Repository {
+            private long id;
+            private String fullName;
+            private String name;
+            private String nodeId;
+            @JsonProperty(value = "private")
+            private boolean isPrivate;
+
+            /**
+             * Get the id.
+             *
+             * @return the id
+             */
+            public long getId() {
+                return id;
+            }
+
+            /**
+             * Gets the full name.
+             *
+             * @return the full name
+             */
+            public String getFullName() {
+                return fullName;
+            }
+
+            /**
+             * Gets the name.
+             *
+             * @return the name
+             */
+            public String getName() {
+                return name;
+            }
+
+            /**
+             * Gets the node id.
+             *
+             * @return the node id
+             */
+            public String getNodeId() {
+                return nodeId;
+            }
+
+            /**
+             * Gets the repository private flag.
+             *
+             * @return whether the repository is private
+             */
+            public boolean isPrivate() {
+                return isPrivate;
             }
         }
     }

diff --git a/src/test/java/org/kohsuke/github/GHEventPayloadTest.java b/src/test/java/org/kohsuke/github/GHEventPayloadTest.java
@@ -990,8 +990,33 @@ public void InstallationRepositoriesEvent() throws Exception {
      *             the exception
      */
     @Test
-    @Payload("installation")
-    public void InstallationEvent() throws Exception {
+    @Payload("installation_created")
+    public void InstallationCreatedEvent() throws Exception {
+        final GHEventPayload.Installation event = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
+                .build()
+                .parseEventPayload(payload.asReader(), GHEventPayload.Installation.class);
+
+        assertThat(event.getAction(), is("created"));
+        assertThat(event.getInstallation().getId(), is(43898337L));
+        assertThat(event.getInstallation().getAccount().getLogin(), is("CronFire"));
+
+        assertThat(event.getRepositories().isEmpty(), is(false));
+        assertThat(event.getRepositories().get(0).getId(), is(1296269L));
+        assertThat(event.getRawRepositories().isEmpty(), is(false));
+        assertThat(event.getRawRepositories().get(0).getId(), is(1296269L));
+
+        assertThat(event.getSender().getLogin(), is("Haarolean"));
+    }
+
+    /**
+     * Installation event.
+     *
+     * @throws Exception
+     *             the exception
+     */
+    @Test
+    @Payload("installation_deleted")
+    public void InstallationDeletedEvent() throws Exception {
         final GHEventPayload.Installation event = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
                 .build()
                 .parseEventPayload(payload.asReader(), GHEventPayload.Installation.class);
@@ -1000,12 +1025,13 @@ public void InstallationEvent() throws Exception {
         assertThat(event.getInstallation().getId(), is(2L));
         assertThat(event.getInstallation().getAccount().getLogin(), is("octocat"));
 
-        assertThat(event.getRepositories().get(0).getId(), is(1296269L));
-        assertThat(event.getRepositories().get(0).getNodeId(), is("MDEwOlJlcG9zaXRvcnkxMjk2MjY5"));
-        assertThat(event.getRepositories().get(0).getName(), is("Hello-World"));
-        assertThat(event.getRepositories().get(0).getFullName(), is("octocat/Hello-World"));
-        assertThat(event.getRepositories().get(0).isPrivate(), is(false));
-        assertThat(event.getRepositories().get(0).getOwner().getLogin(), is("octocat"));
+        assertThrows(IllegalStateException.class, () -> event.getRepositories().isEmpty());
+        assertThat(event.getRawRepositories().isEmpty(), is(false));
+        assertThat(event.getRawRepositories().get(0).getId(), is(1296269L));
+        assertThat(event.getRawRepositories().get(0).getNodeId(), is("MDEwOlJlcG9zaXRvcnkxMjk2MjY5"));
+        assertThat(event.getRawRepositories().get(0).getName(), is("Hello-World"));
+        assertThat(event.getRawRepositories().get(0).getFullName(), is("octocat/Hello-World"));
+        assertThat(event.getRawRepositories().get(0).isPrivate(), is(false));
 
         assertThat(event.getSender().getLogin(), is("octocat"));
     }

diff --git a/src/test/resources/org/kohsuke/github/GHEventPayloadTest/installation_created.json b/src/test/resources/org/kohsuke/github/GHEventPayloadTest/installation_created.json
@@ -0,0 +1,98 @@
+{
+  "action": "created",
+  "installation": {
+    "id": 43898337,
+    "account": {
+      "login": "CronFire",
+      "id": 68755481,
+      "node_id": "MDEyOk9yZ2FuaXphdGlvbjY4NzU1NDgx",
+      "avatar_url": "https://avatars.githubusercontent.com/u/68755481?v=4",
+      "gravatar_id": "",
+      "url": "https://api.github.com/users/CronFire",
+      "html_url": "https://github.com/CronFire",
+      "followers_url": "https://api.github.com/users/CronFire/followers",
+      "following_url": "https://api.github.com/users/CronFire/following{/other_user}",
+      "gists_url": "https://api.github.com/users/CronFire/gists{/gist_id}",
+      "starred_url": "https://api.github.com/users/CronFire/starred{/owner}{/repo}",
+      "subscriptions_url": "https://api.github.com/users/CronFire/subscriptions",
+      "organizations_url": "https://api.github.com/users/CronFire/orgs",
+      "repos_url": "https://api.github.com/users/CronFire/repos",
+      "events_url": "https://api.github.com/users/CronFire/events{/privacy}",
+      "received_events_url": "https://api.github.com/users/CronFire/received_events",
+      "type": "Organization",
+      "site_admin": false
+    },
+    "repository_selection": "selected",
+    "access_tokens_url": "https://api.github.com/app/installations/43898337/access_tokens",
+    "repositories_url": "https://api.github.com/installation/repositories",
+    "html_url": "https://github.com/organizations/CronFire/settings/installations/43898337",
+    "app_id": 421464,
+    "app_slug": "kapybro-dev",
+    "target_id": 68755481,
+    "target_type": "Organization",
+    "permissions": {
+      "checks": "write",
+      "issues": "write",
+      "actions": "read",
+      "members": "read",
+      "contents": "write",
+      "metadata": "read",
+      "statuses": "write",
+      "single_file": "read",
+      "pull_requests": "write",
+      "administration": "read"
+    },
+    "events": [
+      "issues",
+      "issue_comment",
+      "organization",
+      "public",
+      "pull_request",
+      "pull_request_review",
+      "pull_request_review_comment",
+      "push",
+      "repository",
+      "status"
+    ],
+    "created_at": "2023-11-11T10:55:06.000+08:00",
+    "updated_at": "2023-11-11T10:55:06.000+08:00",
+    "single_file_name": ".github/kapybro/config.yml",
+    "has_multiple_single_files": true,
+    "single_file_paths": [
+      ".github/kapybro/config.yml",
+      ".github/kapybro/rules.yml"
+    ],
+    "suspended_by": null,
+    "suspended_at": null
+  },
+  "repositories": [
+    {
+      "id": 1296269,
+      "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5",
+      "name": "Hello-World",
+      "full_name": "octocat/Hello-World",
+      "private": false
+    }
+  ],
+  "requester": null,
+  "sender": {
+    "login": "Haarolean",
+    "id": 1494347,
+    "node_id": "MDQ6VXNlcjE0OTQzNDc=",
+    "avatar_url": "https://avatars.githubusercontent.com/u/1494347?v=4",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/Haarolean",
+    "html_url": "https://github.com/Haarolean",
+    "followers_url": "https://api.github.com/users/Haarolean/followers",
+    "following_url": "https://api.github.com/users/Haarolean/following{/other_user}",
+    "gists_url": "https://api.github.com/users/Haarolean/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/Haarolean/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/Haarolean/subscriptions",
+    "organizations_url": "https://api.github.com/users/Haarolean/orgs",
+    "repos_url": "https://api.github.com/users/Haarolean/repos",
+    "events_url": "https://api.github.com/users/Haarolean/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/Haarolean/received_events",
+    "type": "User",
+    "site_admin": false
+  }
+}
diff --git a/...thub/GHEventPayloadTest/installation.json → ...ventPayloadTest/installation_deleted.json b/...thub/GHEventPayloadTest/installation.json → ...ventPayloadTest/installation_deleted.json