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-50636] Use @DataBoundSetter for optional pipeline params #48

Merged
merged 3 commits into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 54 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<properties>
<!-- Baseline Jenkins version you use to build the plugin. Users must have this version or newer to run. -->
<jenkins.version>1.625.3</jenkins.version>
<jenkins.version>2.0</jenkins.version>
<!-- Java Level to use. Java 7 required when using core >= 1.612 -->
<java.level>7</java.level>
</properties>
Expand Down Expand Up @@ -51,13 +51,65 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<version>2.1</version>
<version>2.12</version>
</dependency>
<dependency>
<groupId>net.sf.opencsv</groupId>
<artifactId>opencsv</artifactId>
<version>1.7</version>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-job</artifactId>
<version>2.10</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
<version>2.40</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-basic-steps</artifactId>
<version>2.6</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-durable-task-step</artifactId>
<version>2.8</version>
<scope>test</scope>
</dependency>

<!-- Satisfy upper bounds dependency warnings -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.10</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-support</artifactId>
<version>2.14</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
<version>2.20</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<version>2.12</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
144 changes: 101 additions & 43 deletions src/main/java/hudson/plugins/plot/PlotBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
import hudson.Util;
import hudson.model.AbstractProject;
import hudson.model.Run;
import hudson.model.TaskListener;
Expand All @@ -13,11 +14,13 @@
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import javax.annotation.CheckForNull;
import javax.servlet.ServletException;
import jenkins.tasks.SimpleBuildStep;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;

Expand All @@ -34,133 +37,188 @@
*/
public class PlotBuilder extends Builder implements SimpleBuildStep {

// Required fields
private final String group;
private final String title;
private final String numBuilds;
private final String yaxis;
private final String style;
private final boolean useDescr;
private final boolean exclZero;
private final boolean logarithmic;
private final boolean keepRecords;
private final String yaxisMinimum;
private final String yaxisMaximum;

// Optional fields
@CheckForNull
private String title;
@CheckForNull
private String numBuilds;
@CheckForNull
private String yaxis;
@CheckForNull
private String yaxisMinimum;
@CheckForNull
private String yaxisMaximum;
private boolean useDescr;
private boolean exclZero;
private boolean logarithmic;
private boolean keepRecords;

// Generated?
@SuppressWarnings("visibilitymodifier")
public String csvFileName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably this field should be moved to the top where other Required fields are located.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is a little odd, since it's not directly specified by the user with the snippet generator. I'm assuming it's generated here through reflection when the step is created.

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to move, your assumption is correct. Maybe, in this case, explanation comment from where it comes will be better than Generate?.

/**
* List of data series.
*/
@SuppressWarnings("visibilitymodifier")
public List<Series> series;
@SuppressWarnings("visibilitymodifier")
public List<CSVSeries> csvSeries;
@SuppressWarnings("visibilitymodifier")
public List<PropertiesSeries> propertiesSeries;
@SuppressWarnings("visibilitymodifier")
public List<XMLSeries> xmlSeries;

// Fields in config.jelly must match the parameter names in the "DataBoundConstructor"
@SuppressWarnings("parameternumber")
// Similarly, any optional @DataBoundSetter properties must match
@DataBoundConstructor
public PlotBuilder(String group, String title, String numBuilds, String yaxis, String style,
boolean useDescr, boolean exclZero, boolean logarithmic, boolean keepRecords,
String yaxisMinimum, String yaxisMaximum, String csvFileName,
List<CSVSeries> csvSeries, List<PropertiesSeries> propertiesSeries,
List<XMLSeries> xmlSeries) {
public PlotBuilder(String group, String style, String csvFileName) {
this.group = group;
this.title = title;
this.numBuilds = numBuilds;
this.yaxis = yaxis;
this.style = style;
this.useDescr = useDescr;
this.exclZero = exclZero;
this.logarithmic = logarithmic;
this.keepRecords = keepRecords;
this.yaxisMinimum = yaxisMinimum;
this.yaxisMaximum = yaxisMaximum;
this.csvFileName = csvFileName;
this.csvSeries = csvSeries;
this.propertiesSeries = propertiesSeries;
this.xmlSeries = xmlSeries;
this.series = new ArrayList<>();
if (csvSeries != null) {
this.series.addAll(csvSeries);
}
if (xmlSeries != null) {
this.series.addAll(xmlSeries);
}
if (propertiesSeries != null) {
this.series.addAll(propertiesSeries);
}
}

public String getGroup() {
return group;
}

public String getStyle() {
return style;
}

@CheckForNull
public String getTitle() {
return title;
}

@DataBoundSetter
public final void setTitle(@CheckForNull String title) {
this.title = Util.fixEmptyAndTrim(title);
}

@CheckForNull
public String getNumBuilds() {
return numBuilds;
}

@DataBoundSetter
public final void setNumBuilds(@CheckForNull String numBuilds) {
this.numBuilds = Util.fixEmptyAndTrim(numBuilds);
}

@CheckForNull
public String getYaxis() {
return yaxis;
}

public String getStyle() {
return style;
@DataBoundSetter
public final void setYaxis(@CheckForNull String yaxis) {
this.yaxis = Util.fixEmptyAndTrim(yaxis);
}

public boolean getUseDescr() {
return useDescr;
}

@DataBoundSetter
public void setUseDescr(boolean useDescr) {
this.useDescr = useDescr;
}

public boolean getExclZero() {
return exclZero;
}

@DataBoundSetter
public void setExclZero(boolean exclZero) {
this.exclZero = exclZero;
}

public boolean getLogarithmic() {
return logarithmic;
}

@DataBoundSetter
public void setLogarithmic(boolean logarithmic) {
this.logarithmic = logarithmic;
}

public boolean getKeepRecords() {
return keepRecords;
}

@DataBoundSetter
public void setKeepRecords(boolean keepRecords) {
this.keepRecords = keepRecords;
}

@CheckForNull
public String getYaxisMinimum() {
return yaxisMinimum;
}

@DataBoundSetter
public final void setYaxisMinimum(@CheckForNull String yaxisMinimum) {
this.yaxisMinimum = Util.fixEmptyAndTrim(yaxisMinimum);
}

@CheckForNull
public String getYaxisMaximum() {
return yaxisMaximum;
}

public List<Series> getSeries() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified that this one was actually never called from outside. Should be safe to remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super happy with the removal of this, as it splits the pipeline and freestyle jobs further from each other. I tried taking parts of the freestyle config.jelly, unfortunately it's not just simple replacement. I'm not familiar enough with freestyle jobs to say why that is.

return series;
@DataBoundSetter
public final void setYaxisMaximum(@CheckForNull String yaxisMaximum) {
this.yaxisMaximum = Util.fixEmptyAndTrim(yaxisMaximum);
}

public List<CSVSeries> getCsvSeries() {
return csvSeries;
}

@DataBoundSetter
public void setCsvSeries(List<CSVSeries> csvSeries) {
this.csvSeries = csvSeries;
}

public List<PropertiesSeries> getPropertiesSeries() {
return propertiesSeries;
}

@DataBoundSetter
public void setPropertiesSeries(List<PropertiesSeries> propertiesSeries) {
this.propertiesSeries = propertiesSeries;
}

public List<XMLSeries> getXmlSeries() {
return xmlSeries;
}

@DataBoundSetter
public void setXmlSeries(List<XMLSeries> xmlSeries) {
this.xmlSeries = xmlSeries;
}

@Override
public void perform(Run<?, ?> build, FilePath workspace, Launcher launcher,
TaskListener listener) {
List<Plot> plots = new ArrayList<>();
Plot plot = new Plot(title, yaxis, group, numBuilds, csvFileName, style,
useDescr, keepRecords, exclZero, logarithmic,
yaxisMinimum, yaxisMaximum);

List<Series> series = new ArrayList<>();
if (csvSeries != null) {
series.addAll(csvSeries);
}
if (xmlSeries != null) {
series.addAll(xmlSeries);
}
if (propertiesSeries != null) {
series.addAll(propertiesSeries);
}

plot.series = series;
plot.addBuild(build, listener.getLogger(), workspace);
plots.add(plot);
Expand Down
Loading