refactored cleaner and push trigger to work with hook manager

MerkushevKirill · MerkushevKirill · commit f02238e4598d · 2015-06-17T03:32:46.000+03:00
diff --git a/src/main/java/com/cloudbees/jenkins/Cleaner.java b/src/main/java/com/cloudbees/jenkins/Cleaner.java
@@ -3,22 +3,20 @@
 import com.cloudbees.jenkins.GitHubPushTrigger.DescriptorImpl;
 import hudson.Extension;
 import hudson.model.AbstractProject;
-import hudson.model.Hudson;
 import hudson.model.PeriodicWork;
 import hudson.triggers.Trigger;
-import hudson.util.TimeUnit2;
-import org.kohsuke.github.GHException;
-import org.kohsuke.github.GHHook;
-import org.kohsuke.github.GHRepository;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.github.webhook.WebhookManager;
 
-import java.io.IOException;
 import java.net.URL;
-import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.TimeUnit;
+
+import static org.jenkinsci.plugins.github.util.FluentIterableWrapper.from;
+import static org.jenkinsci.plugins.github.util.JobInfoHelpers.associatedNames;
+import static org.jenkinsci.plugins.github.util.JobInfoHelpers.withTrigger;
 
 /**
  * Removes post-commit hooks from repositories that we no longer care.
@@ -29,70 +27,52 @@
  */
 @Extension
 public class Cleaner extends PeriodicWork {
-    private final Set<GitHubRepositoryName> couldHaveBeenRemoved = new HashSet<GitHubRepositoryName>();
+    /**
+     * Queue contains repo names prepared to cleanup.
+     * After configure method on job, trigger calls {@link #onStop(AbstractProject)}
+     * which converts to repo names with help of contributors.
+     *
+     * This queue is thread-safe, so any thread can write or
+     * fetch names to this queue without additional sync
+     */
+    private final Queue<GitHubRepositoryName> namesq = new ConcurrentLinkedQueue<GitHubRepositoryName>();
 
     /**
      * Called when a {@link GitHubPushTrigger} is about to be removed.
      */
-    synchronized void onStop(AbstractProject<?,?> job) {
-        couldHaveBeenRemoved.addAll(GitHubRepositoryNameContributor.parseAssociatedNames(job));
+    /* package */ void onStop(AbstractProject<?, ?> job) {
+        namesq.addAll(GitHubRepositoryNameContributor.parseAssociatedNames(job));
     }
 
     @Override
     public long getRecurrencePeriod() {
-        return TimeUnit2.MINUTES.toMillis(3);
+        return TimeUnit.MINUTES.toMillis(3);
     }
 
+    /**
+     * Each run this work fetches alive repo names (which has trigger for it)
+     * then if names queue is not empty (any job was reconfigured with GH trigger change),
+     * next name passed to {@link WebhookManager} with list of active names to check and unregister old hooks
+     *
+     * @throws Exception
+     */
     @Override
     protected void doRun() throws Exception {
-        List<GitHubRepositoryName> names;
-        synchronized (this) {// atomically obtain what we need to check
-            names = new ArrayList<GitHubRepositoryName>(couldHaveBeenRemoved);
-            couldHaveBeenRemoved.clear();
-        }
+        URL url = Trigger.all().get(DescriptorImpl.class).getHookUrl();
 
-        // subtract all the live repositories
-        for (AbstractProject<?,?> job : Hudson.getInstance().getAllItems(AbstractProject.class)) {
-            GitHubPushTrigger trigger = job.getTrigger(GitHubPushTrigger.class);
-            if (trigger!=null) {
-                names.removeAll(GitHubRepositoryNameContributor.parseAssociatedNames(job));
-            }
-        }
+        List<AbstractProject> jobs = Jenkins.getInstance().getAllItems(AbstractProject.class);
+        List<GitHubRepositoryName> alive = from(jobs)
+                .filter(withTrigger(GitHubPushTrigger.class))  // live repos
+                .transformAndConcat(associatedNames()).toList();
 
-        // these are the repos that we are no longer interested.
-        // erase our hooks
-        OUTER:
-        for (GitHubRepositoryName r : names) {
-            for (GHRepository repo : r.resolve()) {
-                try {
-                    removeHook(repo, Trigger.all().get(DescriptorImpl.class).getHookUrl());
-                    LOGGER.fine("Removed a hook from "+r+"");
-                    continue OUTER;
-                } catch (Throwable e) {
-                    LOGGER.log(Level.WARNING,"Failed to remove hook from "+r,e);
-                }
-            }
-        }
-    }
+        while (!namesq.isEmpty()) {
+            GitHubRepositoryName name = namesq.poll();
 
-    //Maybe we should create a remove hook method in the Github API
-    //something like public void removeHook(String name, Map<String,String> config)
-    private void removeHook(GHRepository repo, URL url) {
-        try {
-            String urlExternalForm = url.toExternalForm();
-            for (GHHook h : repo.getHooks()) {
-                if (h.getName().equals("jenkins") && h.getConfig().get("jenkins_hook_url").equals(urlExternalForm)) {
-                    h.delete();
-                }
-            }
-        } catch (IOException e) {
-            throw new GHException("Failed to update post-commit hooks", e);
+            WebhookManager.forHookUrl(url).unregisterFor(name, alive);
         }
     }
 
     public static Cleaner get() {
         return PeriodicWork.all().get(Cleaner.class);
     }
-
-    private static final Logger LOGGER = Logger.getLogger(Cleaner.class.getName());
 }
diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java
@@ -1,27 +1,37 @@
 package com.cloudbees.jenkins;
 
+import com.google.common.base.Charsets;
+import com.google.common.base.Function;
 import hudson.Extension;
 import hudson.Util;
 import hudson.console.AnnotatedLargeText;
+import hudson.model.AbstractProject;
 import hudson.model.Action;
-import hudson.model.Hudson;
-import hudson.model.Hudson.MasterComputer;
 import hudson.model.Item;
-import hudson.model.AbstractProject;
 import hudson.model.Project;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 import hudson.util.FormValidation;
 import hudson.util.SequentialExecutionQueue;
 import hudson.util.StreamTaskListener;
+import jenkins.model.Jenkins;
+import jenkins.model.Jenkins.MasterComputer;
+import net.sf.json.JSONObject;
+import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.jelly.XMLOutput;
+import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
+import org.jenkinsci.plugins.github.webhook.WebhookManager;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+import org.kohsuke.stapler.StaplerRequest;
 
+import javax.inject.Inject;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.nio.charset.Charset;
 import java.security.interfaces.RSAPublicKey;
 import java.text.DateFormat;
 import java.util.ArrayList;
@@ -33,19 +43,10 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import jenkins.model.Jenkins;
-import net.sf.json.JSONObject;
-
-import org.apache.commons.codec.binary.Base64;
-import org.apache.commons.jelly.XMLOutput;
-import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
-import org.kohsuke.github.GHException;
-import org.kohsuke.github.GHRepository;
-import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.QueryParameter;
-import org.kohsuke.stapler.StaplerRequest;
-
-import javax.inject.Inject;
+import static org.jenkinsci.plugins.github.util.FluentIterableWrapper.from;
+import static org.jenkinsci.plugins.github.util.JobInfoHelpers.isBuildable;
+import static org.jenkinsci.plugins.github.util.JobInfoHelpers.withTrigger;
+import static org.jenkinsci.plugins.github.webhook.WebhookManager.forHookUrl;
 
 /**
  * Triggers a build when we receive a GitHub post-commit webhook.
@@ -149,40 +150,16 @@ public void start(AbstractProject<?, ?> project, boolean newInstance) {
 
     /**
      * Tries to register hook for current associated job.
+     * Do this lazily to avoid blocking the UI thread.
      * Useful for using from groovy scripts.
      * @since 1.11.2
      */
     public void registerHooks() {
-        // make sure we have hooks installed. do this lazily to avoid blocking the UI thread.
-        final Collection<GitHubRepositoryName> names = GitHubRepositoryNameContributor.parseAssociatedNames(job);
-
-        getDescriptor().queue.execute(new Runnable() {
-            public void run() {
-                LOGGER.log(Level.INFO, "Adding GitHub webhooks for {0}", names);
-
-                for (GitHubRepositoryName name : names) {
-                    for (GHRepository repo : name.resolve()) {
-                        try {
-                            if(createJenkinsHook(repo, getDescriptor().getHookUrl())) {
-                                break;
-                            }
-                        } catch (Throwable e) {
-                            LOGGER.log(Level.WARNING, "Failed to add GitHub webhook for "+name, e);
-                        }
-                    }
-                }
-            }
-        });
+        URL hookUrl = getDescriptor().getHookUrl();
+        Runnable hookRegistrator = forHookUrl(hookUrl).registerFor(job);
+        getDescriptor().queue.execute(hookRegistrator);
     }
 
-    private boolean createJenkinsHook(GHRepository repo, URL url) {
-        try {
-            repo.createHook("jenkins", Collections.singletonMap("jenkins_hook_url", url.toExternalForm()), null, true);
-            return true;
-        } catch (IOException e) {
-            throw new GHException("Failed to update jenkins hooks", e);
-        }
-    }
 
     @Override
     public void stop() {
@@ -233,7 +210,7 @@ public String getLog() throws IOException {
          * @since 1.350
          */
         public void writeLogTo(XMLOutput out) throws IOException {
-            new AnnotatedLargeText<GitHubWebHookPollingAction>(getLogFile(), Charset.defaultCharset(),true,this).writeHtmlTo(0,out.asWriter());
+            new AnnotatedLargeText<GitHubWebHookPollingAction>(getLogFile(), Charsets.UTF_8, true, this).writeHtmlTo(0, out.asWriter());
         }
     }
 
@@ -278,8 +255,14 @@ public void setManageHook(boolean v) {
         /**
          * Returns the URL that GitHub should post.
          */
-        public URL getHookUrl() throws MalformedURLException {
-            return hookUrl!=null ? new URL(hookUrl) : new URL(Hudson.getInstance().getRootUrl()+GitHubWebHook.get().getUrlName()+'/');
+        public URL getHookUrl() {
+            try {
+                return hookUrl != null
+                        ? new URL(hookUrl)
+                        : new URL(Jenkins.getInstance().getRootUrl() + GitHubWebHook.get().getUrlName() + '/');
+            } catch (MalformedURLException e) {
+                throw new RuntimeException("Hook url is malformed", e);
+            }
         }
 
         public boolean hasOverrideURL() {
@@ -337,22 +320,26 @@ public FormValidation doReRegister() {
                 return FormValidation.error("Works only when Jenkins manages hooks");
             }
 
-            int triggered = 0;
-            for (AbstractProject<?,?> job : getJenkinsInstance().getAllItems(AbstractProject.class)) {
-                if (!job.isBuildable()) {
-                    continue;
-                }
+            List<GitHubPushTrigger> registered = from(getJenkinsInstance().getAllItems(AbstractProject.class))
+                    .filter(isBuildable())
+                    .filter(withTrigger(GitHubPushTrigger.class))
+                    .transform(reRegisterHooks()).toList();
 
-                GitHubPushTrigger trigger = job.getTrigger(GitHubPushTrigger.class);
-                if (trigger!=null) {
+
+            LOGGER.log(Level.INFO, "Called registerHooks() for {0} jobs", registered.size());
+            return FormValidation.ok("Called re-register hooks for %s jobs", registered.size());
+        }
+
+        private Function<AbstractProject, GitHubPushTrigger> reRegisterHooks() {
+            return new Function<AbstractProject, GitHubPushTrigger>() {
+                @Override
+                public GitHubPushTrigger apply(AbstractProject job) {
+                    GitHubPushTrigger trigger = (GitHubPushTrigger) job.getTrigger(GitHubPushTrigger.class);
                     LOGGER.log(Level.FINE, "Calling registerHooks() for {0}", job.getFullName());
                     trigger.registerHooks();
-                    triggered++;
+                    return trigger;
                 }
-            }
-
-            LOGGER.log(Level.INFO, "Called registerHooks() for {0} jobs", triggered);
-            return FormValidation.ok("Called re-register hooks for " + triggered + " jobs");
+            };
         }
 
         public static final Jenkins getJenkinsInstance() throws IllegalStateException {
