Skip to content

Commit

Permalink
Improve UI validation and add more tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@cfoote
cfoote committed Jan 19, 2021
1 parent 4a00991 commit 599db27
Show file tree
Hide file tree
Showing 5 changed files with 266 additions and 102 deletions.
174 changes: 105 additions & 69 deletions src/main/java/org/jenkins/plugins/lockableresources/RequiredResourcesProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.jenkins.plugins.lockableresources.queue.Utils;
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript;
import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -136,7 +137,8 @@ public RequiredResourcesProperty newInstance(StaplerRequest req, JSONObject form

public FormValidation doCheckResourceNames(@QueryParameter String value,
@QueryParameter String labelName,
@QueryParameter boolean script) {
@QueryParameter boolean script,
@AncestorInPath AbstractProject<?,?> project) {
String labelVal = Util.fixEmptyAndTrim(labelName);
String names = Util.fixEmptyAndTrim(value);

Expand All @@ -148,6 +150,7 @@ public FormValidation doCheckResourceNames(@QueryParameter String value,
} else {
List<String> wrongNames = new ArrayList<>();
List<String> varNames = new ArrayList<>();
List<String> unknownParams = new ArrayList<>();
for (String name : names.split("\\s+")) {
boolean found = false;
for (LockableResource r : LockableResourcesManager.get()
Expand All @@ -158,86 +161,119 @@ public FormValidation doCheckResourceNames(@QueryParameter String value,
}
}
if (!found) {
if (Utils.isVariable(name)) {
varNames.add(name);
if (Utils.containsParameter(name)) {
List<String> badParams = Utils.checkParameters(name, project);
if (!badParams.isEmpty()) {
unknownParams.addAll(badParams);
}
varNames.add(name);
} else {
wrongNames.add(name);
}
}
}
if (wrongNames.isEmpty() && varNames.isEmpty()) {
return FormValidation.ok();
} else if (wrongNames.isEmpty()) {
return FormValidation
.warning("The following resources cannot be validated as they are the environment variables: "
+ varNames);
} else {
return FormValidation
.error("The following resources do not exist: "
+ wrongNames);
}
}
}

public FormValidation doCheckLabelName(
@QueryParameter String value,
@QueryParameter String resourceNames,
@QueryParameter boolean script) {
String label = Util.fixEmptyAndTrim(value);
String names = Util.fixEmptyAndTrim(resourceNames);

if (label == null) {
return FormValidation.ok();
} else if (names != null || script) {
return FormValidation.error(
"Only label, groovy expression, or resources can be defined, not more than one.");
} else {
if (LockableResourcesManager.get().isValidLabel(label)) {
return FormValidation.ok();
} else {
return FormValidation.error(
"The label does not exist: " + label);
}
}
}
if (wrongNames.isEmpty() && varNames.isEmpty() && unknownParams.isEmpty()) {
return FormValidation.ok();
} else if (!wrongNames.isEmpty()) {
return FormValidation
.error("The following resources do not exist: "
+ wrongNames);
} else if (!unknownParams.isEmpty()) {
return FormValidation
.error("The following parameters do not exist: "
+ unknownParams);
} else {
return FormValidation
.warning("The following resources cannot be validated as they contain parameter values: "
+ varNames);
}
}
}

public FormValidation doCheckLabelName(@QueryParameter String value,
@QueryParameter String resourceNames,
@QueryParameter boolean script,
@AncestorInPath AbstractProject<?,?> project) {

String label = Util.fixEmptyAndTrim(value);
String names = Util.fixEmptyAndTrim(resourceNames);

if (label == null) {
return FormValidation.ok();
} else if (names != null || script) {
return FormValidation.error(
"Only label, groovy expression, or resources can be defined, not more than one.");
} else {
if (LockableResourcesManager.get().isValidLabel(label)) {
return FormValidation.ok();
} else if (Utils.containsParameter(label)) {
List<String> badParams = Utils.checkParameters(label, project);
if (!badParams.isEmpty()) {
return FormValidation
.error("The following parameters do not exist: "
+ badParams);
}
return FormValidation
.warning("The label cannot be validated as it contains a parameter value: "
+ label);
} else {
return FormValidation.error(
"The label does not exist: " + label);
}
}
}

public FormValidation doCheckResourceNumber(@QueryParameter String value,
@QueryParameter String resourceNames,
@QueryParameter String labelName,
@QueryParameter String resourceMatchScript)
{
public FormValidation doCheckResourceNumber(@QueryParameter String value,
@QueryParameter String resourceNames,
@QueryParameter String labelName,
@QueryParameter String resourceMatchScript,
@AncestorInPath AbstractProject<?,?> project) {

String number = Util.fixEmptyAndTrim(value);
String names = Util.fixEmptyAndTrim(resourceNames);
String label = Util.fixEmptyAndTrim(labelName);
String number = Util.fixEmptyAndTrim(value);
String names = Util.fixEmptyAndTrim(resourceNames);
String label = Util.fixEmptyAndTrim(labelName);
String script = Util.fixEmptyAndTrim(resourceMatchScript);

if (number == null || number.equals("") || number.trim().equals("0")) {
return FormValidation.ok();
}
if (number == null || number.equals("0")) {
return FormValidation.ok();
}

int numAsInt;
try {
numAsInt = Integer.parseInt(number);
} catch(NumberFormatException e) {
return FormValidation.error(
"Could not parse the given value as integer.");
}
int numResources = 0;
if (names != null) {
numResources = names.split("\\s+").length;
} else if (label != null || script != null) {
numResources = Integer.MAX_VALUE;
int numAsInt;
try {
numAsInt = Integer.parseInt(number);
} catch(NumberFormatException e) {
if (Utils.isParameter(number)) {
List<String> badParams = Utils.checkParameters(number, project);
if (!badParams.isEmpty()) {
return FormValidation
.error("The following parameter does not exist: "
+ badParams);
}
return FormValidation
.warning("The value cannot be validated as it is a parameter value: "
+ number);
}

return FormValidation.error(
"Could not parse the given value as integer.");
}
int numResources = 0;
if (names != null) {
numResources = names.split("\\s+").length;
} else if (label != null) {
numResources = LockableResourcesManager.get().getResourcesWithLabel(label, null).size();
} else if (script != null) {
numResources = Integer.MAX_VALUE;
}

if (numResources < numAsInt) {
return FormValidation.error(String.format(
"Given amount %d is greater than amount of resources: %d.",
numAsInt,
numResources));
}
return FormValidation.ok();
}
if (numResources < numAsInt) {
return FormValidation.error(String.format(
"Given amount %d is greater than amount of resources: %d.",
numAsInt,
numResources));
}
return FormValidation.ok();
}

public AutoCompletionCandidates doAutoCompleteLabelName(
@QueryParameter String value) {
Expand Down
45 changes: 43 additions & 2 deletions src/main/java/org/jenkins/plugins/lockableresources/queue/Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,23 @@

import hudson.EnvVars;
import hudson.matrix.MatrixConfiguration;
import hudson.model.AbstractProject;
import hudson.model.Job;
import hudson.model.ParametersDefinitionProperty;
import hudson.model.Queue;

import hudson.model.Run;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.jenkins.plugins.lockableresources.RequiredResourcesProperty;

import javax.annotation.Nonnull;

public final class Utils {
private Utils() {
}
Expand All @@ -26,6 +36,11 @@ private Utils() {
*/
private static final Pattern VARIABLE = Pattern.compile("\\$([A-Za-z0-9_]+|\\{[A-Za-z0-9_.]+\\})");

/**
* Pattern for capturing parameters. ${xyz} but not $${xyz}
*/
private static final Pattern PARAMETER = Pattern.compile("(?<!\\$)\\$\\{([A-Za-z0-9_.]+)\\}");

public static Job<?, ?> getProject(Queue.Item item) {
if (item.task instanceof Job)
return (Job<?, ?>) item.task;
Expand Down Expand Up @@ -53,7 +68,33 @@ public static LockableResourcesStruct requiredResources(
return null;
}

public static boolean isVariable(String name) {
return VARIABLE.matcher(name).matches();
@Nonnull
public static List<String> getProjectParameterNames(AbstractProject<?,?> project) {
ParametersDefinitionProperty params = project.getProperty(ParametersDefinitionProperty.class);
if (params != null)
return params.getParameterDefinitionNames();
return Collections.emptyList();
}

public static boolean isParameter(String s) {
return PARAMETER.matcher(s).matches();
}

public static boolean containsParameter(String s) {
return PARAMETER.matcher(s).find();
}

@Nonnull
public static List<String> checkParameters(String s, AbstractProject<?, ?> project) {
List<String> unknownParameters = new ArrayList<>();
List<String> paramNames = getProjectParameterNames(project);
Matcher m = PARAMETER.matcher(s);
while (m.find()) {
String param = m.group(1);
if (!paramNames.contains(param)) {
unknownParameters.add(param);
}
}
return unknownParameters;
}
}
32 changes: 32 additions & 0 deletions src/test/java/org/jenkins/plugins/lockableresources/FreeStyleProjectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,38 @@ public void configRoundTripWithParam() throws Exception {
assertNull(resourcesProp.getResourceMatchScript());
}

@Test
public void configRoundTripWithLabelParam() throws Exception {
FreeStyleProject withLabel = j.createFreeStyleProject("withLabelParam");
withLabel.addProperty(new RequiredResourcesProperty(null, null, null, "${labelParam}", null));
FreeStyleProject withLabelRoundTrip = j.configRoundtrip(withLabel);

RequiredResourcesProperty withLabelProp =
withLabelRoundTrip.getProperty(RequiredResourcesProperty.class);
assertNotNull(withLabelProp);
assertNull(withLabelProp.getResourceNames());
assertNull(withLabelProp.getResourceNamesVar());
assertNull(withLabelProp.getResourceNumber());
assertEquals("${labelParam}", withLabelProp.getLabelName());
assertNull(withLabelProp.getResourceMatchScript());
}

@Test
public void configRoundTripWithNumParam() throws Exception {
FreeStyleProject withNum = j.createFreeStyleProject("withNumParam");
withNum.addProperty(new RequiredResourcesProperty(null, null, "${resNum}", "some-resources", null));
FreeStyleProject withNumRoundTrip = j.configRoundtrip(withNum);

RequiredResourcesProperty withNumProp =
withNumRoundTrip.getProperty(RequiredResourcesProperty.class);
assertNotNull(withNumProp);
assertNull(withNumProp.getResourceNames());
assertNull(withNumProp.getResourceNamesVar());
assertEquals("${resNum}", withNumProp.getResourceNumber());
assertEquals("some-resources", withNumProp.getLabelName());
assertNull(withNumProp.getResourceMatchScript());
}

@Test
public void configRoundTripWithScript() throws Exception {
FreeStyleProject withScript = j.createFreeStyleProject("withScript");
Expand Down
1 change: 1 addition & 0 deletions src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,7 @@ public void lockWithInvalidLabel() throws Exception {
WorkflowRun b1 = p.scheduleBuild2(0).waitForStart();
j.waitForCompletion(b1);
j.assertBuildStatus(Result.FAILURE, b1);
j.waitUntilNoActivity();
j.assertLogContains("The label does not exist: invalidLabel", b1);
isPaused(b1, 0, 0);
}
Expand Down
Loading

0 comments on commit 599db27

Please sign in to comment.