Skip to content
Permalink
Browse files

[FIXED JENKINS-39553] Make GitHub plugin BuildableItem aware (#153)

* [FIXED JENKINS-39533] Make GitHub plugin BuildableItem aware

* Address code review comments from Oleg

* not actually deprecated

* Address review comments
  • Loading branch information
stephenc authored and KostyaSha committed Dec 11, 2016
1 parent ebfcc1b commit bd2e945b329ed86af1b5ab52358c87e50e3fb4aa
Showing with 269 additions and 93 deletions.
  1. +6 −6 src/main/java/com/cloudbees/jenkins/Cleaner.java
  2. +8 −5 src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java
  3. +48 −23 src/main/java/com/cloudbees/jenkins/GitHubRepositoryNameContributor.java
  4. +4 −4 src/main/java/com/cloudbees/jenkins/GitHubTrigger.java
  5. +23 −7 src/main/java/com/cloudbees/jenkins/GitHubWebHook.java
  6. +4 −4 src/main/java/org/jenkinsci/plugins/github/config/GitHubPluginConfig.java
  7. +78 −3 src/main/java/org/jenkinsci/plugins/github/extension/GHEventsSubscriber.java
  8. +10 −0 src/main/java/org/jenkinsci/plugins/github/util/FluentIterableWrapper.java
  9. +39 −21 src/main/java/org/jenkinsci/plugins/github/util/JobInfoHelpers.java
  10. +28 −5 src/main/java/org/jenkinsci/plugins/github/webhook/WebhookManager.java
  11. +3 −3 src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java
  12. +3 −3 src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/PingGHEventSubscriber.java
  13. +2 −1 src/test/java/com/cloudbees/jenkins/GitHubWebHookTest.java
  14. +2 −1 src/test/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitorTest.java
  15. +2 −1 src/test/java/org/jenkinsci/plugins/github/extension/GHEventsSubscriberTest.java
  16. +2 −1 src/test/java/org/jenkinsci/plugins/github/test/GHMockRule.java
  17. +4 −3 src/test/java/org/jenkinsci/plugins/github/util/JobInfoHelpersTest.java
  18. +3 −2 src/test/java/org/jenkinsci/plugins/github/webhook/WebhookManagerTest.java
@@ -1,7 +1,7 @@
package com.cloudbees.jenkins;

import hudson.Extension;
import hudson.model.Job;
import hudson.model.Item;
import hudson.model.PeriodicWork;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.github.GitHubPlugin;
@@ -28,7 +28,7 @@
public class Cleaner extends PeriodicWork {
/**
* Queue contains repo names prepared to cleanup.
* After configure method on job, trigger calls {@link #onStop(Job)}
* After configure method on job, trigger calls {@link #onStop(Item)}
* which converts to repo names with help of contributors.
*
* This queue is thread-safe, so any thread can write or
@@ -39,8 +39,8 @@
/**
* Called when a {@link GitHubPushTrigger} is about to be removed.
*/
/* package */ void onStop(Job<?, ?> job) {
cleanQueue.addAll(GitHubRepositoryNameContributor.parseAssociatedNames(job));
/* package */ void onStop(Item item) {
cleanQueue.addAll(GitHubRepositoryNameContributor.parseAssociatedNames(item));
}

@Override
@@ -61,8 +61,8 @@ protected void doRun() throws Exception {

URL url = GitHubPlugin.configuration().getHookUrl();

List<Job> jobs = Jenkins.getInstance().getAllItems(Job.class);
List<GitHubRepositoryName> aliveRepos = from(jobs)
List<Item> items = Jenkins.getInstance().getAllItems(Item.class);
List<GitHubRepositoryName> aliveRepos = from(items)
.filter(isAlive()) // live repos
.transformAndConcat(associatedNames()).toList();

@@ -27,6 +27,8 @@
import org.jenkinsci.plugins.github.config.GitHubPluginConfig;
import org.jenkinsci.plugins.github.internal.GHPluginConfigException;
import org.jenkinsci.plugins.github.migration.Migrator;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.slf4j.Logger;
@@ -368,19 +370,20 @@ private static ThreadFactory threadFactory() {
}

/**
* Checks that repo defined in this job is not in administrative monitor as failed to be registered.
* Checks that repo defined in this item is not in administrative monitor as failed to be registered.
* If that so, shows warning with some instructions
*
* @param job - to check against. Should be not null and have at least one repo defined
* @param item - to check against. Should be not null and have at least one repo defined
*
* @return warning or empty string
* @since 1.17.0
*/
@SuppressWarnings("unused")
public FormValidation doCheckHookRegistered(@AncestorInPath Job<?, ?> job) {
Preconditions.checkNotNull(job, "Job can't be null if wants to check hook in monitor");
@Restricted(NoExternalUse.class) // invoked from Stapler
public FormValidation doCheckHookRegistered(@AncestorInPath Item item) {
Preconditions.checkNotNull(item, "Item can't be null if wants to check hook in monitor");

Collection<GitHubRepositoryName> repos = GitHubRepositoryNameContributor.parseAssociatedNames(job);
Collection<GitHubRepositoryName> repos = GitHubRepositoryNameContributor.parseAssociatedNames(item);

for (GitHubRepositoryName repo : repos) {
if (monitor.isProblemWith(repo)) {
@@ -7,6 +7,7 @@
import hudson.Util;
import hudson.model.AbstractProject;
import hudson.model.EnvironmentContributor;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.TaskListener;
import hudson.plugins.git.GitSCM;
@@ -36,41 +37,57 @@
* Looks at the definition of {@link AbstractProject} and list up the related github repositories,
* then puts them into the collection.
*
* @deprecated Use {@link #parseAssociatedNames(Job, Collection)}
* @deprecated Use {@link #parseAssociatedNames(Item, Collection)}
*/
@Deprecated
public void parseAssociatedNames(AbstractProject<?, ?> job, Collection<GitHubRepositoryName> result) {
parseAssociatedNames((Job) job, result);
parseAssociatedNames((Item) job, result);
}

/**
* Looks at the definition of {@link Job} and list up the related github repositories,
* then puts them into the collection.
* @deprecated Use {@link #parseAssociatedNames(Item, Collection)}
*/
@Deprecated
public /*abstract*/ void parseAssociatedNames(Job<?, ?> job, Collection<GitHubRepositoryName> result) {
if (overriddenMethodHasDeprecatedSignature(job)) {
parseAssociatedNames((AbstractProject) job, result);
} else {
throw new AbstractMethodError("you must override the new overload of parseAssociatedNames");
}
parseAssociatedNames((Item) job, result);
}

/**
* To select backward compatible method with old extensions
* with overridden {@link #parseAssociatedNames(AbstractProject, Collection)}
*
* @param job - parameter to check for old class
*
* @return true if overridden deprecated method
* Looks at the definition of {@link Item} and list up the related github repositories,
* then puts them into the collection.
* @param item the item.
* @param result the collection to add repository names to
* @since FIXME
*/
private boolean overriddenMethodHasDeprecatedSignature(Job<?, ?> job) {
return Util.isOverridden(
@SuppressWarnings("deprecation")
public /*abstract*/ void parseAssociatedNames(Item item, Collection<GitHubRepositoryName> result) {
if (Util.isOverridden(
GitHubRepositoryNameContributor.class,
getClass(),
"parseAssociatedNames",
Job.class,
Collection.class
)) {
// if this impl is legacy, it cannot contribute to non-jobs, so not an error
if (item instanceof Job) {
parseAssociatedNames((Job<?, ?>) item, result);
}
} else if (Util.isOverridden(
GitHubRepositoryNameContributor.class,
getClass(),
"parseAssociatedNames",
AbstractProject.class,
Collection.class
) && job instanceof AbstractProject;
)) {
// if this impl is legacy, it cannot contribute to non-projects, so not an error
if (item instanceof AbstractProject) {
parseAssociatedNames((AbstractProject<?, ?>) item, result);
}
} else {
throw new AbstractMethodError("you must override the new overload of parseAssociatedNames");
}
}

public static ExtensionList<GitHubRepositoryNameContributor> all() {
@@ -82,13 +99,21 @@ private boolean overriddenMethodHasDeprecatedSignature(Job<?, ?> job) {
*/
@Deprecated
public static Collection<GitHubRepositoryName> parseAssociatedNames(AbstractProject<?, ?> job) {
return parseAssociatedNames((Job) job);
return parseAssociatedNames((Item) job);
}

/**
* @deprecated Use {@link #parseAssociatedNames(Item)}
*/
@Deprecated
public static Collection<GitHubRepositoryName> parseAssociatedNames(Job<?, ?> job) {
return parseAssociatedNames((Item) job);
}

public static Collection<GitHubRepositoryName> parseAssociatedNames(Item item) {
Set<GitHubRepositoryName> names = new HashSet<GitHubRepositoryName>();
for (GitHubRepositoryNameContributor c : all()) {
c.parseAssociatedNames(job, names);
c.parseAssociatedNames(item, names);
}
return names;
}
@@ -99,11 +124,11 @@ private boolean overriddenMethodHasDeprecatedSignature(Job<?, ?> job) {
@Extension
public static class FromSCM extends GitHubRepositoryNameContributor {
@Override
public void parseAssociatedNames(Job<?, ?> job, Collection<GitHubRepositoryName> result) {
SCMTriggerItem item = SCMTriggerItems.asSCMTriggerItem(job);
EnvVars envVars = buildEnv(job);
if (item != null) {
for (SCM scm : item.getSCMs()) {
public void parseAssociatedNames(Item item, Collection<GitHubRepositoryName> result) {
SCMTriggerItem triggerItem = SCMTriggerItems.asSCMTriggerItem(item);
EnvVars envVars = item instanceof Job ? buildEnv((Job) item) : new EnvVars();
if (triggerItem != null) {
for (SCM scm : triggerItem.getSCMs()) {
addRepositories(scm, envVars, result);
}
}
@@ -3,7 +3,7 @@
import hudson.Extension;
import hudson.Util;
import hudson.model.AbstractProject;
import hudson.model.Job;
import hudson.model.Item;
import hudson.triggers.Trigger;
import jenkins.model.ParameterizedJobMixIn;

@@ -46,9 +46,9 @@
@Extension
class GitHubRepositoryNameContributorImpl extends GitHubRepositoryNameContributor {
@Override
public void parseAssociatedNames(Job<?, ?> job, Collection<GitHubRepositoryName> result) {
if (job instanceof ParameterizedJobMixIn.ParameterizedJob) {
ParameterizedJobMixIn.ParameterizedJob p = (ParameterizedJobMixIn.ParameterizedJob) job;
public void parseAssociatedNames(Item item, Collection<GitHubRepositoryName> result) {
if (item instanceof ParameterizedJobMixIn.ParameterizedJob) {
ParameterizedJobMixIn.ParameterizedJob p = (ParameterizedJobMixIn.ParameterizedJob) item;
// TODO use standard method in 1.621+
for (GitHubTrigger ght : Util.filter(p.getTriggers().values(), GitHubTrigger.class)) {
result.addAll(ght.getGitHubRepositories());
@@ -3,6 +3,7 @@
import com.google.common.base.Function;
import hudson.Extension;
import hudson.ExtensionPoint;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.RootAction;
import hudson.model.UnprotectedRootAction;
@@ -70,21 +71,36 @@ public String getUrlName() {
* {@code GitHubWebHook.get().registerHookFor(job);}
*
* @param job not null project to register hook for
* @deprecated use {@link #registerHookFor(Item)}
*/
@Deprecated
public void registerHookFor(Job job) {
reRegisterHookForJob().apply(job);
}

/**
* If any wants to auto-register hook, then should call this method
* Example code:
* {@code GitHubWebHook.get().registerHookFor(item);}
*
* @param item not null item to register hook for
* @since FIXME
*/
public void registerHookFor(Item item) {
reRegisterHookForJob().apply(item);
}

/**
* Calls {@link #registerHookFor(Job)} for every project which have subscriber
*
* @return list of jobs which jenkins tried to register hook
*/
public List<Job> reRegisterAllHooks() {
return from(getJenkinsInstance().getAllItems(Job.class))
public List<Item> reRegisterAllHooks() {
return from(getJenkinsInstance().getAllItems(Item.class))
.filter(isBuildable())
.filter(isAlive())
.transform(reRegisterHookForJob()).toList();
.transform(reRegisterHookForJob())
.toList();
}

/**
@@ -101,11 +117,11 @@ public void doIndex(@Nonnull @GHEventHeader GHEvent event, @Nonnull @GHEventPayl
.transform(processEvent(event, payload)).toList();
}

private Function<Job, Job> reRegisterHookForJob() {
return new Function<Job, Job>() {
private <T extends Item> Function<T, T> reRegisterHookForJob() {
return new Function<T, T>() {
@Override
public Job apply(Job job) {
LOGGER.debug("Calling registerHooks() for {}", notNull(job, "Job can't be null").getFullName());
public T apply(T job) {
LOGGER.debug("Calling registerHooks() for {}", notNull(job, "Item can't be null").getFullName());

// We should handle wrong url of self defined hook url here in any case with try-catch :(
URL hookUrl;
@@ -6,7 +6,7 @@
import hudson.Extension;
import hudson.XmlFile;
import hudson.model.Descriptor;
import hudson.model.Job;
import hudson.model.Item;
import hudson.util.FormValidation;
import jenkins.model.GlobalConfiguration;
import jenkins.model.Jenkins;
@@ -181,10 +181,10 @@ public FormValidation doReRegister() {
return FormValidation.warning("Works only when Jenkins manages hooks (one ore more creds specified)");
}

List<Job> registered = GitHubWebHook.get().reRegisterAllHooks();
List<Item> registered = GitHubWebHook.get().reRegisterAllHooks();

LOGGER.info("Called registerHooks() for {} jobs", registered.size());
return FormValidation.ok("Called re-register hooks for %s jobs", registered.size());
LOGGER.info("Called registerHooks() for {} items", registered.size());
return FormValidation.ok("Called re-register hooks for %s items", registered.size());
}

@SuppressWarnings("unused")

2 comments on commit bd2e945

@uhafner

This comment has been minimized.

Copy link
Member

@uhafner uhafner replied Dec 11, 2016

Still the wrong issue number in the title. This causes scm links in the wrong issue..

@KostyaSha

This comment has been minimized.

Copy link
Member

@KostyaSha KostyaSha replied Dec 11, 2016

You can still use review github functionality in PR.

Please sign in to comment.
You can’t perform that action at this time.