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.

Sign up for GitHub

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

Jump to bottom

[JENKINS-72988] validate displayname against items in the same ItemGroup #9152

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[JENKINS-72988] validate displayname against items in the same ItemGroup #9152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb9afd1
[JENKINS-72988] validate displayname only against items in same ItemG…
mawinter69 Apr 9, 2024
0e06d65
adjust method name to avoid conflict with old implementations
mawinter69 Apr 9, 2024
cffdb9f
Merge branch 'master' into JENKINS-72988
krisstern May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions core/src/main/java/hudson/model/AbstractProject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1934,11 +1934,6 @@ public boolean isApplicable(Descriptor descriptor) {
return true;
}

@Restricted(DoNotUse.class)
public FormValidation doCheckDisplayNameOrNull(@AncestorInPath AbstractProject project, @QueryParameter String value) {
return Jenkins.get().doCheckDisplayName(value, project.getName());
}

@Restricted(DoNotUse.class)
public FormValidation doCheckAssignedLabelString(@AncestorInPath AbstractProject<?, ?> project,
@QueryParameter String value) {
Expand Down
41 changes: 35 additions & 6 deletions core/src/main/java/jenkins/model/Jenkins.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5286,8 +5286,9 @@ public View getStaplerFallback() {
* job that the user is configuring though to prevent a validation warning
* if the user sets the displayName to what it currently is.
*/
boolean isDisplayNameUnique(String displayName, String currentJobName) {
Collection<TopLevelItem> itemCollection = items.values();
boolean isDisplayNameUnique(ItemGroup<?> itemGroup, String displayName, String currentJobName) {

Collection<TopLevelItem> itemCollection = (Collection<TopLevelItem>) itemGroup.getItems(t -> t instanceof TopLevelItem);

// if there are a lot of projects, we'll have to store their
// display names in a HashSet or something for a quick check
Expand All @@ -5311,8 +5312,8 @@ else if (displayName.equals(item.getDisplayName())) {
* @param name The name to test
* @param currentJobName The name of the job that the user is configuring
*/
boolean isNameUnique(String name, String currentJobName) {
Item item = getItem(name);
boolean isNameUnique(ItemGroup<?> itemGroup, String name, String currentJobName) {
Item item = itemGroup.getItem(name);

if (null == item) {
// the candidate name didn't return any items so the name is unique
Expand All @@ -5334,17 +5335,45 @@ else if (item.getName().equals(currentJobName)) {
* existing display names or project names
* @param displayName The display name to test
* @param jobName The name of the job the user is configuring
*
* @deprecated use {@link TopLevelItemDescriptor#doCheckDisplayNameOrNull(TopLevelItem, String)}
*/
@Deprecated
public FormValidation doCheckDisplayName(@QueryParameter String displayName,
@QueryParameter String jobName) {
displayName = displayName.trim();

LOGGER.fine(() -> "Current job name is " + jobName);

if (!isNameUnique(displayName, jobName)) {
if (!isNameUnique(this, displayName, jobName)) {
return FormValidation.warning(Messages.Jenkins_CheckDisplayName_NameNotUniqueWarning(displayName));
}
else if (!isDisplayNameUnique(this, displayName, jobName)) {
return FormValidation.warning(Messages.Jenkins_CheckDisplayName_DisplayNameNotUniqueWarning(displayName));
}
else {
return FormValidation.ok();
}
}

/**
* Checks to see if the candidate displayName collides with any
* existing display names or project names in the items parent group
* @param displayName The display name to test
* @param item The item to check for duplicates
*/
@Restricted(NoExternalUse.class)
public FormValidation checkDisplayName(String displayName,
TopLevelItem item) {
displayName = displayName.trim();
String jobName = item.getName();

LOGGER.fine(() -> "Current job name is " + jobName);

if (!isNameUnique(item.getParent(), displayName, jobName)) {
return FormValidation.warning(Messages.Jenkins_CheckDisplayName_NameNotUniqueWarning(displayName));
}
else if (!isDisplayNameUnique(displayName, jobName)) {
else if (!isDisplayNameUnique(item.getParent(), displayName, jobName)) {
return FormValidation.warning(Messages.Jenkins_CheckDisplayName_DisplayNameNotUniqueWarning(displayName));
}
else {
Expand Down
20 changes: 10 additions & 10 deletions test/src/test/java/jenkins/model/JenkinsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ public void testIsDisplayNameUniqueTrue() throws Exception {
p.setDisplayName("displayName");

Jenkins jenkins = Jenkins.get();
assertTrue(jenkins.isDisplayNameUnique("displayName1", curJobName));
assertTrue(jenkins.isDisplayNameUnique(jobName, curJobName));
assertTrue(jenkins.isDisplayNameUnique(jenkins, "displayName1", curJobName));
assertTrue(jenkins.isDisplayNameUnique(jenkins, jobName, curJobName));
}

@Test
Expand All @@ -220,7 +220,7 @@ public void testIsDisplayNameUniqueFalse() throws Exception {
p.setDisplayName(displayName);

Jenkins jenkins = Jenkins.get();
assertFalse(jenkins.isDisplayNameUnique(displayName, curJobName));
assertFalse(jenkins.isDisplayNameUnique(jenkins, displayName, curJobName));
}

@Test
Expand All @@ -233,7 +233,7 @@ public void testIsDisplayNameUniqueSameAsCurrentJob() throws Exception {

Jenkins jenkins = Jenkins.get();
// should be true as we don't test against the current job
assertTrue(jenkins.isDisplayNameUnique(displayName, curJobName));
assertTrue(jenkins.isDisplayNameUnique(jenkins, displayName, curJobName));
}

@Test
Expand All @@ -244,7 +244,7 @@ public void testIsNameUniqueTrue() throws Exception {
j.createFreeStyleProject(jobName);

Jenkins jenkins = Jenkins.get();
assertTrue(jenkins.isNameUnique("jobName1", curJobName));
assertTrue(jenkins.isNameUnique(jenkins, "jobName1", curJobName));
}

@Test
Expand All @@ -255,7 +255,7 @@ public void testIsNameUniqueFalse() throws Exception {
j.createFreeStyleProject(jobName);

Jenkins jenkins = Jenkins.get();
assertFalse(jenkins.isNameUnique(jobName, curJobName));
assertFalse(jenkins.isNameUnique(jenkins, jobName, curJobName));
}

@Test
Expand All @@ -267,7 +267,7 @@ public void testIsNameUniqueSameAsCurrentJob() throws Exception {

Jenkins jenkins = Jenkins.get();
// true because we don't test against the current job
assertTrue(jenkins.isNameUnique(curJobName, curJobName));
assertTrue(jenkins.isNameUnique(jenkins, curJobName, curJobName));
}

@Test
Expand All @@ -281,7 +281,7 @@ public void testDoCheckDisplayNameUnique() throws Exception {
p.setDisplayName("displayName");

Jenkins jenkins = Jenkins.get();
FormValidation v = jenkins.doCheckDisplayName("1displayName", curJobName);
FormValidation v = jenkins.checkDisplayName("1displayName", curProject);
assertEquals(FormValidation.ok(), v);
}

Expand All @@ -297,7 +297,7 @@ public void testDoCheckDisplayNameSameAsDisplayName() throws Exception {
p.setDisplayName(displayName);

Jenkins jenkins = Jenkins.get();
FormValidation v = jenkins.doCheckDisplayName(displayName, curJobName);
FormValidation v = jenkins.checkDisplayName(displayName, curProject);
assertEquals(FormValidation.Kind.WARNING, v.kind);
}

Expand All @@ -313,7 +313,7 @@ public void testDoCheckDisplayNameSameAsJobName() throws Exception {
p.setDisplayName(displayName);

Jenkins jenkins = Jenkins.get();
FormValidation v = jenkins.doCheckDisplayName(jobName, curJobName);
FormValidation v = jenkins.checkDisplayName(jobName, curProject);
assertEquals(FormValidation.Kind.WARNING, v.kind);
}

Expand Down