Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-57903]- Refactor PromotionConditionDescriptor extension Point to make it Pipeline Compatible. #137

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
72aca25
Update Jenkins Core requirement to 2.60.3, use the modern Parent POM
oleg-nenashev May 31, 2019
daa7a82
Fix Jenkinsfile
oleg-nenashev May 31, 2019
0a9e676
Update Jenkins Core to 2.138.x and cleanup Jenkins instance access AP…
oleg-nenashev Jun 1, 2019
084e389
Merge branch 'facelift' of https://github.com/jenkinsci/promoted-buil…
oleg-nenashev Jun 1, 2019
5666eef
More FindBugs cleanup
oleg-nenashev Jun 2, 2019
c3965f0
[JENKINS-57839] - Initial PromotionBadge compatibility code for Jenki…
oleg-nenashev Jun 4, 2019
9c51d9e
[JENKINS-57903]- Refactor PromotionConditionDescriptor class and othe…
dernDren161 Jun 20, 2019
69f69be
Delete README.txt
oleg-nenashev Jun 20, 2019
7317087
Add badges to README
oleg-nenashev Jun 20, 2019
162697b
Remove Contributing from README
oleg-nenashev Jun 20, 2019
68dd6c8
Create CONTRIBUTING.md
oleg-nenashev Jun 20, 2019
fb198ac
Merge pull request #138 from jenkinsci/doc-cleanup
oleg-nenashev Jun 21, 2019
d1ce53d
Maven Fix- For the PromotionConditionDescriptor
dernDren161 Jun 21, 2019
8aa5cec
Stub code for logic review
dernDren161 Jun 22, 2019
bca15f9
Fix tests after ugrading the Jenkins and JTH baseline
oleg-nenashev Jun 22, 2019
e6b87eb
FindBugs: Cleanup unrequired checks in Groovy conditions
oleg-nenashev Jun 22, 2019
bd7af12
Merge pull request #124 from jenkinsci/facelift
oleg-nenashev Jun 22, 2019
4ebbf5e
[maven-release-plugin] prepare release promoted-builds-3.3
oleg-nenashev Jun 22, 2019
64a5bd4
[maven-release-plugin] prepare for next development iteration
oleg-nenashev Jun 22, 2019
307199d
Merge branches
oleg-nenashev Jun 22, 2019
0eeda41
Fix compilation errors in AddPromotionBadge
oleg-nenashev Jun 22, 2019
2f6b639
Restore code from d1ce53de6aa5622899b21768cca4e813c0fab024 by @dernDr…
oleg-nenashev Jun 22, 2019
dd6de8a
Resolve Maven upper bounds issues
oleg-nenashev Jun 22, 2019
8e4fca7
Fix compilation issues in GroovyConditionDescriptor
oleg-nenashev Jun 22, 2019
4fa0b38
PR chnages w.r.t the reviews
dernDren161 Jun 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/main/java/hudson/plugins/promoted_builds/Promotion.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import hudson.model.Run;
import hudson.model.User;
import hudson.plugins.promoted_builds.conditions.ManualCondition;
import hudson.plugins.promoted_builds.pipeline.PromotionRun;
import hudson.plugins.promoted_builds.util.JenkinsHelper;
import hudson.security.Permission;
import hudson.security.PermissionGroup;
Expand Down Expand Up @@ -60,11 +61,11 @@
import org.kohsuke.stapler.StaplerResponse;

/**
* Records a promotion process.
* Records a promotion process for {@link AbstractProject}s.
*
* @author Kohsuke Kawaguchi
*/
public class Promotion extends AbstractBuild<PromotionProcess,Promotion> {
public class Promotion extends AbstractBuild<PromotionProcess,Promotion> implements PromotionRun {

public Promotion(PromotionProcess job) throws IOException {
super(job);
Expand Down Expand Up @@ -113,6 +114,7 @@ public EnvVars getEnvironment(TaskListener listener) throws IOException, Interru

// Augment environment with target build's information
String rootUrl = JenkinsHelper.getInstance().getRootUrl();
//TODO: Refactor to Run
AbstractBuild<?, ?> target = getTarget();
if(rootUrl!=null)
e.put("PROMOTED_URL",rootUrl+target.getUrl());
Expand Down Expand Up @@ -162,7 +164,7 @@ public EnvVars getEnvironment(TaskListener listener) throws IOException, Interru
}

// Allow the promotion status to contribute to build environment
getStatus().buildEnvVars(this, e);
getStatus().buildEnvVars(this, e, listener);

return e;
}
Expand Down Expand Up @@ -239,6 +241,21 @@ public String getUserId() {
return User.getUnknown().getId();
}

@Nonnull
@Override
public Run<?, ?> getPromotedRun() {
return getTarget();
}


@Nonnull
@Override
public Run<?, ?> getPromotionRun() {
return this;
}

//TODO: move to a default method
@Override
public List<ParameterValue> getParameterValues(){
List<ParameterValue> values=new ArrayList<ParameterValue>();
ParametersAction parametersAction=getParametersActions(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

import hudson.EnvVars;
import hudson.model.AbstractBuild;
import hudson.model.Run;
import hudson.model.TaskListener;
import org.kohsuke.stapler.export.ExportedBean;

import javax.annotation.Nonnull;

/**
* Captures the information about how/when the promotion criteria is satisfied.
*
Expand All @@ -18,11 +22,22 @@ public abstract class PromotionBadge {
/**
* Called by {@link Status} to allow promotion badges to contribute environment variables.
*
* @param build
* The calling build. Never null.
* @param run
* The calling run.
* @param env
* Environment variables should be added to this map.
*/
public void buildEnvVars(@Nonnull Run<?,?> run, EnvVars env, TaskListener listener) {
// Default implementation when the method is not overridden
if (run instanceof AbstractBuild) {
buildEnvVars((AbstractBuild<?,?>)run, env);
}
}

/**
* @deprecated Use {@link #buildEnvVars(Run, EnvVars, TaskListener)}
*/
@Deprecated
public void buildEnvVars(AbstractBuild<?,?> build, EnvVars env) {
// by default don't contribute any variables
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import hudson.model.AbstractProject;
import hudson.model.Descriptor;
import hudson.model.Job;
import hudson.model.TaskListener;

import javax.annotation.Nonnull;

/**
* {@link Descriptor} for {@link PromotionCondition}.
Expand All @@ -25,5 +29,11 @@ protected PromotionConditionDescriptor() {
* @return
* true to allow user to configure this promotion condition for the given project.
*/
public boolean isApplicable(@Nonnull Job<?,?> item,@Nonnull TaskListener listener){
if(item instanceof AbstractProject){
return isApplicable((AbstractProject)item, TaskListener.NULL);
}
return isApplicable((Job) item, listener);
}
public abstract boolean isApplicable(AbstractProject<?,?> item);
oleg-nenashev marked this conversation as resolved.
Show resolved Hide resolved
}
20 changes: 17 additions & 3 deletions src/main/java/hudson/plugins/promoted_builds/Status.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import hudson.model.ParameterDefinition;
import hudson.model.ParameterValue;
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.plugins.promoted_builds.conditions.ManualCondition;
import hudson.util.Iterators;
import net.sf.json.JSONArray;
Expand Down Expand Up @@ -147,17 +149,29 @@ public AbstractBuild<?,?> getTarget() {
return _parent != null ? _parent.owner : null;
}



/**
* Called by {@link Promotion} to allow status to contribute environment variables.
*
* @param build
* The calling build. Never null.
* @param run
* The calling run
* @param env
* Environment variables should be added to this map.
*/
public void buildEnvVars(Run<?,?> run, EnvVars env, TaskListener listener) {
for (PromotionBadge badge : badges) {
badge.buildEnvVars(run, env, listener);
}
}

/**
* @deprecated Use {@link #buildEnvVars(Run, EnvVars, TaskListener)}
*/
@Deprecated
public void buildEnvVars(AbstractBuild<?,?> build, EnvVars env) {
for (PromotionBadge badge : badges) {
badge.buildEnvVars(build, env);
badge.buildEnvVars(build, env, TaskListener.NULL);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,9 @@
import hudson.Extension;
import hudson.Util;
import hudson.console.HyperlinkNote;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.AutoCompletionCandidates;
import hudson.model.Cause;
import hudson.model.*;
import hudson.model.Cause.UpstreamCause;
import hudson.model.Fingerprint;
import hudson.model.Fingerprint.BuildPtr;
import hudson.model.InvisibleAction;
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.listeners.RunListener;
import hudson.plugins.promoted_builds.JobPropertyImpl;
import hudson.plugins.promoted_builds.PromotionBadge;
Expand All @@ -46,6 +36,7 @@
import java.util.Stack;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

import org.kohsuke.stapler.export.Exported;

Expand Down Expand Up @@ -205,6 +196,13 @@ void add(AbstractBuild<?,?> b) {

@Extension
public static final class DescriptorImpl extends PromotionConditionDescriptor {
public boolean isApplicable(@Nonnull Job<?,?> item, @Nonnull TaskListener listener) {
if(item instanceof AbstractProject){
return isApplicable((AbstractProject)item, TaskListener.NULL);
}
return true;
}

public boolean isApplicable(AbstractProject<?,?> item) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import hudson.PluginManager;
import hudson.Util;
import hudson.model.AbstractBuild;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.plugins.promoted_builds.PromotionBadge;
import hudson.plugins.promoted_builds.PromotionCondition;
import hudson.plugins.promoted_builds.PromotionProcess;
Expand Down Expand Up @@ -130,8 +132,7 @@ public String getDisplayLabel() {
}

@Override
public void buildEnvVars(final AbstractBuild<?, ?> build, final EnvVars env) {
super.buildEnvVars(build, env);
public void buildEnvVars(final Run<?, ?> build, final EnvVars env, TaskListener listener) {
for (final Map.Entry<String, String> entry :
variables.entrySet()) {
env.put(entry.getKey(), entry.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,53 @@
import hudson.PluginManager;
import hudson.PluginWrapper;
import hudson.model.AbstractProject;
import hudson.model.Job;
import hudson.model.TaskListener;
import hudson.plugins.promoted_builds.PromotionConditionDescriptor;
import javafx.concurrent.Task;
import jenkins.model.Jenkins;

import javax.annotation.Nonnull;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Declared outside GroovyCondition as it depends on the optional 'script-security' dependency
*/
@Extension(optional = true)
public final class GroovyConditionDescriptor extends PromotionConditionDescriptor {
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No specific reason known for this just maybe because of the Job integrity it required multiple descriptors in pipelines(maybe) , just that the code compiled correctly this way would it break sth if final is removed?

public class GroovyConditionDescriptor extends PromotionConditionDescriptor {

private static final Logger LOGGER = Logger.getLogger(GroovyConditionDescriptor.class.getName());

public GroovyConditionDescriptor() {
super(GroovyCondition.class);
}

@Override
public boolean isApplicable(final AbstractProject<?, ?> item) {
// TODO switch to Jenkins.getActiveInstance() once bumped to 1.590
final Jenkins jenkins = Jenkins.getInstance();
if (jenkins == null) {
// Jenkins not started or shut down
return false;
}
final PluginManager pluginManager = jenkins.getPluginManager();
if (pluginManager == null) {
LOGGER.log(Level.WARNING, "No PluginManager");
return false;
public boolean isApplicable(@Nonnull Job<?,?> item, TaskListener listener){
if(item instanceof AbstractProject){
return isApplicable((AbstractProject)item, TaskListener.NULL);
}else{
Copy link
Member

Choose a reason for hiding this comment

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

minor nitpick: formatting. Leave blank spaces for if and else statements like everywhere else.

Choose a reason for hiding this comment

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

Your IDE could be configured to do it for you

final Jenkins jenkins = Jenkins.getInstance();
if (jenkins == null) {
// Jenkins not started or shut down
return false;
}
final PluginManager pluginManager = jenkins.getPluginManager();
if (pluginManager == null) {
LOGGER.log(Level.WARNING, "No PluginManager");
return false;
}
final PluginWrapper plugin = pluginManager.getPlugin("script-security");
return plugin != null && plugin.isActive();
}
final PluginWrapper plugin = pluginManager.getPlugin("script-security");
return plugin != null && plugin.isActive();

}

public boolean isApplicable(AbstractProject<?, ?> item) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Why? it will make it unusable

}


@Override
public String getDisplayName() {
return Messages.GroovyCondition_DisplayName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,14 @@

import hudson.EnvVars;
import hudson.Extension;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.Descriptor;
import hudson.model.Hudson;
import hudson.model.InvisibleAction;
import hudson.model.SimpleParameterDefinition;
import hudson.model.ParameterDefinition;
import hudson.model.ParameterValue;
import hudson.model.User;
import hudson.plugins.promoted_builds.PromotionPermissionHelper;
import hudson.model.*;
Copy link
Member

Choose a reason for hiding this comment

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

Please do not squash imports in the production code.
Your IDE can be reconfigured to disable it

import hudson.plugins.promoted_builds.Promotion;
import hudson.plugins.promoted_builds.PromotionBadge;
import hudson.plugins.promoted_builds.PromotionCondition;
import hudson.plugins.promoted_builds.PromotionConditionDescriptor;
import hudson.plugins.promoted_builds.Promotion;
import hudson.plugins.promoted_builds.PromotionPermissionHelper;
import hudson.plugins.promoted_builds.PromotionProcess;
import hudson.plugins.promoted_builds.pipeline.PromotionRun;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -36,6 +29,7 @@

import javax.servlet.ServletException;

import hudson.plugins.promoted_builds.pipeline.PromotionRun;
import net.sf.json.JSONArray;
import net.sf.json.JSONObject;

Expand Down Expand Up @@ -283,23 +277,33 @@ public List<ParameterValue> getParameterValues() {
return values != null ? values : Collections.<ParameterValue>emptyList();
}

//TODO, TBD: Refactor API to PromotionRun ?
@Override
public void buildEnvVars(AbstractBuild<?, ?> build, EnvVars env) {
if (!(build instanceof Promotion)) {
throw new IllegalStateException ("Wrong build type. Expected a Promotion, but got "+build.getClass());
public void buildEnvVars(Run<?, ?> run, EnvVars env, TaskListener listener) {
// TODO: Refactor to support Pipeline Promotion types
if (!(run instanceof PromotionRun)) {
throw new IllegalStateException ("Wrong build type. Expected a PromotionRun, but got "+run.getClass());
}

List<ParameterValue> params = ((Promotion) build).getParameterValues();

PromotionRun promotion = (PromotionRun)run;
List<ParameterValue> params = ((PromotionRun) run).getParameterValues();
if (params != null) {
for (ParameterValue value : params) {
value.buildEnvVars(build, env);
value.buildEnvironment(promotion.getPromotionRun(), env);
}
}
}
}

@Extension
public static final class DescriptorImpl extends PromotionConditionDescriptor {
public boolean isApplicable(@Nonnull Job<?,?> item, @Nonnull TaskListener listener) {
if(item instanceof AbstractProject){
return isApplicable((AbstractProject)item, TaskListener.NULL);
}
return true;
}

public boolean isApplicable(AbstractProject<?,?> item) {
return true;
}
Expand Down
Loading