From 8a8d7e3faeee23c7a75f3d3347ec2d46789297b9 Mon Sep 17 00:00:00 2001
From: Martin Lowe <martin.lowe@eclipse-foundation.org>
Date: Thu, 5 Dec 2024 16:04:03 -0500
Subject: [PATCH] update: add GH webhook event check for "installation" type
 events

Enabled by default, installation type events get triggered when the
associated GH application is installed into an organization or
repository. By extracting the current scheduled task to the GitHub
helper, we can call this arbitrarily when there are new installations
as well as handle catch-up on a regular schedule.

Resolves #140
---
 ...alidationHelper.java => GithubHelper.java} |  93 ++++++++++++++-
 .../eca/resource/GithubWebhooksResource.java  |  11 +-
 .../git/eca/resource/StatusResource.java      |   6 +-
 .../impl/DefaultGithubApplicationService.java |   4 +-
 .../tasks/GithubInstallationUpdateTask.java   | 111 +-----------------
 .../eca/tasks/GithubRevalidationQueue.java    |   4 +-
 6 files changed, 109 insertions(+), 120 deletions(-)
 rename src/main/java/org/eclipsefoundation/git/eca/helper/{GithubValidationHelper.java => GithubHelper.java} (81%)

diff --git a/src/main/java/org/eclipsefoundation/git/eca/helper/GithubValidationHelper.java b/src/main/java/org/eclipsefoundation/git/eca/helper/GithubHelper.java
similarity index 81%
rename from src/main/java/org/eclipsefoundation/git/eca/helper/GithubValidationHelper.java
rename to src/main/java/org/eclipsefoundation/git/eca/helper/GithubHelper.java
index 054ebad6..803567ed 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/helper/GithubValidationHelper.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/helper/GithubHelper.java
@@ -12,8 +12,11 @@
 package org.eclipsefoundation.git.eca.helper;
 
 import java.net.URI;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Date;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -23,6 +26,7 @@ import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.eclipse.microprofile.rest.client.inject.RestClient;
 import org.eclipsefoundation.core.service.APIMiddleware;
 import org.eclipsefoundation.git.eca.api.GithubAPI;
+import org.eclipsefoundation.git.eca.api.models.GithubApplicationInstallationData;
 import org.eclipsefoundation.git.eca.api.models.GithubCommit;
 import org.eclipsefoundation.git.eca.api.models.GithubCommit.ParentCommit;
 import org.eclipsefoundation.git.eca.api.models.GithubCommitStatusRequest;
@@ -30,6 +34,7 @@ import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest;
 import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
 import org.eclipsefoundation.git.eca.config.WebhooksConfig;
 import org.eclipsefoundation.git.eca.dto.CommitValidationStatus;
+import org.eclipsefoundation.git.eca.dto.GithubApplicationInstallation;
 import org.eclipsefoundation.git.eca.dto.GithubWebhookTracking;
 import org.eclipsefoundation.git.eca.model.Commit;
 import org.eclipsefoundation.git.eca.model.GitUser;
@@ -41,6 +46,7 @@ import org.eclipsefoundation.git.eca.namespace.ProviderType;
 import org.eclipsefoundation.git.eca.service.GithubApplicationService;
 import org.eclipsefoundation.git.eca.service.ValidationService;
 import org.eclipsefoundation.git.eca.service.ValidationStatusService;
+import org.eclipsefoundation.http.model.FlatRequestWrapper;
 import org.eclipsefoundation.http.model.RequestWrapper;
 import org.eclipsefoundation.persistence.dao.PersistenceDao;
 import org.eclipsefoundation.persistence.model.RDBMSQuery;
@@ -64,8 +70,8 @@ import jakarta.ws.rs.core.Response;
  * scheduled tasks for revalidation.
  */
 @ApplicationScoped
-public class GithubValidationHelper {
-    private static final Logger LOGGER = LoggerFactory.getLogger(GithubValidationHelper.class);
+public class GithubHelper {
+    private static final Logger LOGGER = LoggerFactory.getLogger(GithubHelper.class);
 
     private static final String VALIDATION_LOGGING_MESSAGE = "Setting validation state for {}/#{} to {}";
 
@@ -83,6 +89,8 @@ public class GithubValidationHelper {
     PersistenceDao dao;
     @Inject
     FilterService filters;
+    @Inject
+    JwtHelper jwt;
 
     @Inject
     ValidationService validation;
@@ -352,6 +360,49 @@ public class GithubValidationHelper {
         return dao.get(new RDBMSQuery<>(wrapper, filters.get(GithubWebhookTracking.class), params)).stream().findFirst();
     }
 
+    /**
+     * Using the configured GitHub application JWT and application ID, fetches all active installations and updates the DB cache containing
+     * the installation data. After all records are updated, the starting timestamp is used to delete stale records that were updated before
+     * the starting time.
+     * 
+     * This does not use locking, but with the way that updates are done to look for long stale entries, multiple concurrent runs would not
+     * lead to a loss of data/integrity.
+     */
+    public void updateAppInstallationRecords() {
+        // get the installations for the currently configured app
+        List<GithubApplicationInstallationData> installations = middleware
+                .getAll(i -> ghApi.getInstallations(i, "Bearer " + jwt.generateJwt()));
+        // check that there are installations, none may indicate an issue, and better to fail pessimistically
+        if (installations.isEmpty()) {
+            LOGGER.warn("Did not find any installations for the currently configured Github application");
+            return;
+        }
+        // trace log the installations for more context
+        LOGGER.debug("Found {} installations to cache", installations.size());
+
+        // create a common timestamp that looks for entries stale for more than a day
+        Date startingTimestamp = new Date(ZonedDateTime.now().minus(1, ChronoUnit.DAYS).toInstant().toEpochMilli());
+        // from installations, build records and start the processing for each entry
+        List<GithubApplicationInstallation> installationRecords = installations.stream().map(this::processInstallation).toList();
+
+        // once records are prepared, persist them back to the database with updates where necessary as a batch
+        RequestWrapper wrap = new FlatRequestWrapper(URI.create("https://api.eclipse.org/git/webhooks/github/installations"));
+        List<GithubApplicationInstallation> repoRecords = dao
+                .add(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class)), installationRecords);
+        if (repoRecords.size() != installationRecords.size()) {
+            LOGGER.warn("Background update to installation records had a size mismatch, cleaning will be skipped for this run");
+            return;
+        }
+
+        // build query to do cleanup of stale records
+        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
+        params.add(GitEcaParameterNames.APPLICATION_ID_RAW, Integer.toString(webhooksConfig.github().appId()));
+        params.add(GitEcaParameterNames.LAST_UPDATED_BEFORE_RAW, DateTimeHelper.toRFC3339(startingTimestamp));
+
+        // run the delete call, removing stale entries
+        dao.delete(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class), params));
+    }
+
     /**
      * Simple helper method so we don't have to repeat static strings in multiple places.
      * 
@@ -435,7 +486,43 @@ public class GithubValidationHelper {
             throw new BadRequestException("Missing fields in order to prepare request: " + StringUtils.join(missingFields, ' '));
         }
     }
-    
+
+    /**
+     * Converts the raw installation data from Github into a short record to be persisted to database as a form of persistent caching.
+     * Checks database for existing record, and returns record with a touched date for existing entries.
+     * 
+     * @param ghInstallation raw Github installation record for current application
+     * @return the new or updated installation record to be persisted to the database.
+     */
+    private GithubApplicationInstallation processInstallation(GithubApplicationInstallationData ghInstallation) {
+        RequestWrapper wrap = new FlatRequestWrapper(URI.create("https://api.eclipse.org/git/webhooks/github/installations"));
+        // build the lookup query for the current installation record
+        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
+        params.add(GitEcaParameterNames.APPLICATION_ID_RAW, Integer.toString(webhooksConfig.github().appId()));
+        params.add(GitEcaParameterNames.INSTALLATION_ID_RAW, Integer.toString(ghInstallation.getId()));
+
+        // lookup existing records in the database
+        List<GithubApplicationInstallation> existingRecords = dao
+                .get(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class), params));
+
+        // check for existing entry, creating if missing
+        GithubApplicationInstallation installation;
+        if (existingRecords == null || existingRecords.isEmpty()) {
+            installation = new GithubApplicationInstallation();
+            installation.setAppId(webhooksConfig.github().appId());
+            installation.setInstallationId(ghInstallation.getId());
+        } else {
+            installation = existingRecords.get(0);
+        }
+        // update the basic stats to handle renames, and update last updated time
+        // login is technically nullable, so this might be missing. This is best we can do, as we can't look up by id
+        installation
+                .setName(StringUtils.isNotBlank(ghInstallation.getAccount().getLogin()) ? ghInstallation.getAccount().getLogin()
+                        : "UNKNOWN");
+        installation.setLastUpdated(new Date());
+        return installation;
+    }
+
     /**
      * Retrieves the full repo name for a given org and repo name, used for storage and legacy support.
      *
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubWebhooksResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubWebhooksResource.java
index 453188aa..f4559d56 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/GithubWebhooksResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/GithubWebhooksResource.java
@@ -19,7 +19,7 @@ import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest;
 import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
 import org.eclipsefoundation.git.eca.dto.GithubWebhookTracking;
 import org.eclipsefoundation.git.eca.helper.CaptchaHelper;
-import org.eclipsefoundation.git.eca.helper.GithubValidationHelper;
+import org.eclipsefoundation.git.eca.helper.GithubHelper;
 import org.eclipsefoundation.git.eca.model.RevalidationResponse;
 import org.eclipsefoundation.git.eca.model.ValidationRequest;
 import org.eclipsefoundation.git.eca.namespace.GitEcaParameterNames;
@@ -58,9 +58,11 @@ public class GithubWebhooksResource extends CommonResource {
     GithubApplicationService ghAppService;
 
     @Inject
-    GithubValidationHelper validationHelper;
+    GithubHelper validationHelper;
     @Inject
     CaptchaHelper captchaHelper;
+    @Inject
+    GithubHelper ghHelper;
 
     /**
      * Entry point for processing Github webhook requests. Accepts standard fields as described in the <a href=
@@ -104,6 +106,9 @@ public class GithubWebhooksResource extends CommonResource {
                 // set the revalidation flag to reprocess if the initial attempt fails before exiting
                 setRevalidationFlagForTracking(tracking);
             }
+        } else if ("installation".equalsIgnoreCase(eventType)) {
+            // manually trigger the application installation update 
+            ghHelper.updateAppInstallationRecords();
         }
 
         return Response.ok().build();
@@ -133,7 +138,7 @@ public class GithubWebhooksResource extends CommonResource {
             @QueryParam(GitEcaParameterNames.INSTALLATION_ID_RAW) String installationId,
             @QueryParam(GitEcaParameterNames.PULL_REQUEST_NUMBER_RAW) Integer prNo,
             @FormParam("h-captcha-response") String captchaResponse) {
-        String sanitizedRepoName = GithubValidationHelper.getFullRepoName(orgName, repoName);
+        String sanitizedRepoName = GithubHelper.getFullRepoName(orgName, repoName);
         // retrieve and check that the PR exists
         Optional<PullRequest> prResponse = ghAppService.getPullRequest(installationId, orgName, repoName, prNo);
         if (prResponse.isEmpty()) {
diff --git a/src/main/java/org/eclipsefoundation/git/eca/resource/StatusResource.java b/src/main/java/org/eclipsefoundation/git/eca/resource/StatusResource.java
index 2625fa16..da295822 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/resource/StatusResource.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/resource/StatusResource.java
@@ -17,7 +17,7 @@ import org.apache.commons.lang3.StringUtils;
 import org.eclipsefoundation.efservices.api.models.Project;
 import org.eclipsefoundation.git.eca.config.EclipseQuteTemplateExtensions;
 import org.eclipsefoundation.git.eca.dto.CommitValidationStatus;
-import org.eclipsefoundation.git.eca.helper.GithubValidationHelper;
+import org.eclipsefoundation.git.eca.helper.GithubHelper;
 import org.eclipsefoundation.git.eca.helper.ProjectHelper;
 import org.eclipsefoundation.git.eca.model.Commit;
 import org.eclipsefoundation.git.eca.model.ValidationRequest;
@@ -65,7 +65,7 @@ public class StatusResource extends CommonResource {
     ValidationStatusService validationStatus;
 
     @Inject
-    GithubValidationHelper validationHelper;
+    GithubHelper validationHelper;
     @Inject
     ProjectHelper projects;
 
@@ -146,7 +146,7 @@ public class StatusResource extends CommonResource {
     public Response getCommitValidationForGithub(@PathParam("org") String org, @PathParam("repoName") String repoName,
             @PathParam("prNo") Integer prNo) {
         // generate the URL used to retrieve valid projects
-        String repoUrl = GithubValidationHelper.getRepoUrl(org, repoName);
+        String repoUrl = GithubHelper.getRepoUrl(org, repoName);
 
         try {
             // check that the passed repo has a valid installation
diff --git a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
index 02bbc0f3..6720a561 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/service/impl/DefaultGithubApplicationService.java
@@ -28,7 +28,7 @@ import org.eclipsefoundation.git.eca.api.GithubAPI;
 import org.eclipsefoundation.git.eca.api.models.GithubWebhookRequest.PullRequest;
 import org.eclipsefoundation.git.eca.config.WebhooksConfig;
 import org.eclipsefoundation.git.eca.dto.GithubApplicationInstallation;
-import org.eclipsefoundation.git.eca.helper.GithubValidationHelper;
+import org.eclipsefoundation.git.eca.helper.GithubHelper;
 import org.eclipsefoundation.git.eca.helper.JwtHelper;
 import org.eclipsefoundation.git.eca.namespace.GitEcaParameterNames;
 import org.eclipsefoundation.git.eca.service.GithubApplicationService;
@@ -118,7 +118,7 @@ public class DefaultGithubApplicationService implements GithubApplicationService
 
     @Override
     public Optional<PullRequest> getPullRequest(String installationId, String org, String repoName, Integer pullRequest) {
-        String fullRepoName = GithubValidationHelper.getFullRepoName(org, repoName);
+        String fullRepoName = GithubHelper.getFullRepoName(org, repoName);
         // create param map to better handle multiple PRs from a single repo in short time span
         MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
         params.add(GitEcaParameterNames.PULL_REQUEST_NUMBER_RAW, Integer.toString(pullRequest));
diff --git a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubInstallationUpdateTask.java b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubInstallationUpdateTask.java
index 5b24ae85..5215090c 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubInstallationUpdateTask.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubInstallationUpdateTask.java
@@ -11,26 +11,8 @@
  */
 package org.eclipsefoundation.git.eca.tasks;
 
-import java.net.URI;
-import java.util.Date;
-import java.util.List;
-
-import org.apache.commons.lang3.StringUtils;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
-import org.eclipse.microprofile.rest.client.inject.RestClient;
-import org.eclipsefoundation.core.service.APIMiddleware;
-import org.eclipsefoundation.git.eca.api.GithubAPI;
-import org.eclipsefoundation.git.eca.api.models.GithubApplicationInstallationData;
-import org.eclipsefoundation.git.eca.config.WebhooksConfig;
-import org.eclipsefoundation.git.eca.dto.GithubApplicationInstallation;
-import org.eclipsefoundation.git.eca.helper.JwtHelper;
-import org.eclipsefoundation.git.eca.namespace.GitEcaParameterNames;
-import org.eclipsefoundation.http.model.FlatRequestWrapper;
-import org.eclipsefoundation.http.model.RequestWrapper;
-import org.eclipsefoundation.persistence.dao.PersistenceDao;
-import org.eclipsefoundation.persistence.model.RDBMSQuery;
-import org.eclipsefoundation.persistence.service.FilterService;
-import org.eclipsefoundation.utils.helper.DateTimeHelper;
+import org.eclipsefoundation.git.eca.helper.GithubHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -40,8 +22,6 @@ import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.context.control.ActivateRequestContext;
 import jakarta.enterprise.inject.Instance;
 import jakarta.inject.Inject;
-import jakarta.ws.rs.core.MultivaluedHashMap;
-import jakarta.ws.rs.core.MultivaluedMap;
 
 /**
  * Using the configured database, Github installation records are tracked for all installations available for the
@@ -54,20 +34,9 @@ public class GithubInstallationUpdateTask {
 
     @ConfigProperty(name = "eclipse.git-eca.tasks.gh-installation.enabled", defaultValue = "true")
     Instance<Boolean> isEnabled;
+    
     @Inject
-    WebhooksConfig config;
-
-    @RestClient
-    GithubAPI gh;
-
-    @Inject
-    JwtHelper jwt;
-    @Inject
-    APIMiddleware middle;
-    @Inject
-    PersistenceDao dao;
-    @Inject
-    FilterService filters;
+    GithubHelper helper;
 
     @PostConstruct
     void init() {
@@ -86,78 +55,6 @@ public class GithubInstallationUpdateTask {
         if (!Boolean.TRUE.equals(isEnabled.get())) {
             return;
         }
-
-        // get the installations for the currently configured app
-        List<GithubApplicationInstallationData> installations = middle
-                .getAll(i -> gh.getInstallations(i, "Bearer " + jwt.generateJwt()));
-        // check that there are installations
-        if (installations.isEmpty()) {
-            LOGGER.warn("Did not find any installations for the currently configured Github application");
-            return;
-        }
-        // trace log the installations for more context
-        LOGGER.debug("Found {} installations to cache", installations.size());
-
-        // create a common timestamp for easier lookups of stale entries
-        Date startingTimestamp = new Date();
-        // from installations, build records and start the processing for each entry
-        List<GithubApplicationInstallation> installationRecords = installations
-                .stream()
-                .map(this::processInstallation)
-                .toList();
-
-        // once records are prepared, persist them back to the database with updates where necessary as a batch
-        RequestWrapper wrap = new FlatRequestWrapper(URI.create("https://api.eclipse.org/git/webhooks/github/installations"));
-        List<GithubApplicationInstallation> repoRecords = dao
-                .add(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class)), installationRecords);
-        if (repoRecords.size() != installationRecords.size()) {
-            LOGGER.warn("Background update to installation records had a size mismatch, cleaning will be skipped for this run");
-            return;
-        }
-
-        // build query to do cleanup of stale records
-        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
-        params.add(GitEcaParameterNames.APPLICATION_ID_RAW, Integer.toString(config.github().appId()));
-        params.add(GitEcaParameterNames.LAST_UPDATED_BEFORE_RAW, DateTimeHelper.toRFC3339(startingTimestamp));
-
-        // run the delete call, removing stale entries
-        dao.delete(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class), params));
-    }
-
-    /**
-     * Converts the raw installation data from Github into a short record to be persisted to database as a form of
-     * persistent caching. Checks database for existing record, and returns record with a touched date for existing entries.
-     * 
-     * @param ghInstallation raw Github installation record for current application
-     * @return the new or updated installation record to be persisted to the database.
-     */
-    private GithubApplicationInstallation processInstallation(GithubApplicationInstallationData ghInstallation) {
-        RequestWrapper wrap = new FlatRequestWrapper(URI.create("https://api.eclipse.org/git/webhooks/github/installations"));
-        // build the lookup query for the current installation record
-        MultivaluedMap<String, String> params = new MultivaluedHashMap<>();
-        params.add(GitEcaParameterNames.APPLICATION_ID_RAW, Integer.toString(config.github().appId()));
-        params.add(GitEcaParameterNames.INSTALLATION_ID_RAW, Integer.toString(ghInstallation.getId()));
-
-        // lookup existing records in the database
-        List<GithubApplicationInstallation> existingRecords = dao
-                .get(new RDBMSQuery<>(wrap, filters.get(GithubApplicationInstallation.class), params));
-
-        // check for existing entry, creating if missing
-        GithubApplicationInstallation installation;
-        if (existingRecords == null || existingRecords.isEmpty()) {
-            installation = new GithubApplicationInstallation();
-            installation.setAppId(config.github().appId());
-            installation.setInstallationId(ghInstallation.getId());
-        } else {
-            installation = existingRecords.get(0);
-        }
-        // update the basic stats to handle renames, and update last updated time
-        // login is technically nullable, so this might be missing. This is best we can do, as we can't look up by id
-        installation
-                .setName(StringUtils.isNotBlank(ghInstallation.getAccount().getLogin()) ? ghInstallation.getAccount().getLogin()
-                        : "UNKNOWN");
-        installation.setLastUpdated(new Date());
-        return installation;
-
+        helper.updateAppInstallationRecords();
     }
 }
diff --git a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
index 35bddfdf..bdb9f0c6 100644
--- a/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
+++ b/src/main/java/org/eclipsefoundation/git/eca/tasks/GithubRevalidationQueue.java
@@ -17,7 +17,7 @@ import java.util.List;
 
 import org.eclipse.microprofile.config.inject.ConfigProperty;
 import org.eclipsefoundation.git.eca.dto.GithubWebhookTracking;
-import org.eclipsefoundation.git.eca.helper.GithubValidationHelper;
+import org.eclipsefoundation.git.eca.helper.GithubHelper;
 import org.eclipsefoundation.git.eca.namespace.GitEcaParameterNames;
 import org.eclipsefoundation.http.model.FlatRequestWrapper;
 import org.eclipsefoundation.http.model.RequestWrapper;
@@ -62,7 +62,7 @@ public class GithubRevalidationQueue {
     FilterService filters;
 
     @Inject
-    GithubValidationHelper validationHelper;
+    GithubHelper validationHelper;
 
     @PostConstruct
     void init() {
-- 
GitLab