Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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 bd2e945
Show file tree
Hide file tree
Showing 18 changed files with 269 additions and 93 deletions.
12 changes: 6 additions & 6 deletions src/main/java/com/cloudbees/jenkins/Cleaner.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;
Expand All @@ -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
Expand All @@ -39,8 +39,8 @@ public class Cleaner extends PeriodicWork {
/**
* 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
Expand All @@ -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();

Expand Down
13 changes: 8 additions & 5 deletions src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -36,41 +37,57 @@ public abstract class GitHubRepositoryNameContributor implements ExtensionPoint
* 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() {
Expand All @@ -82,13 +99,21 @@ public static ExtensionList<GitHubRepositoryNameContributor> all() {
*/
@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;
}
Expand All @@ -99,11 +124,11 @@ public static Collection<GitHubRepositoryName> parseAssociatedNames(Job<?, ?> jo
@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);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/cloudbees/jenkins/GitHubTrigger.java
Expand Up @@ -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;

Expand Down Expand Up @@ -46,9 +46,9 @@ public interface GitHubTrigger {
@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());
Expand Down
30 changes: 23 additions & 7 deletions src/main/java/com/cloudbees/jenkins/GitHubWebHook.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

/**
Expand All @@ -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;
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down

2 comments on commit bd2e945

@uhafner
Copy link
Member

@uhafner uhafner commented on bd2e945 Dec 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@KostyaSha
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still use review github functionality in PR.

Please sign in to comment.