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

core/src/main/java/hudson/model/TopLevelItemDescriptor.java

@@ -292,7 +292,7 @@
     }
 
     @Restricted(NoExternalUse.class)
     public FormValidation doCheckDisplayNameOrNull(@AncestorInPath TopLevelItem item, @QueryParameter String value) {
-        return Jenkins.get().doCheckDisplayName(value, item.getName());
+        return Jenkins.get().checkDisplayName(value, item);
     }
 }

core/src/main/java/jenkins/model/Jenkins.java

@@ -343,7 +343,7 @@
  * @author Kohsuke Kawaguchi
  */
 @ExportedBean
 public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLevelItemGroup, StaplerProxy, StaplerFallback,
         ModifiableViewGroup, AccessControlled, DescriptorByNameOwner,
         ModelObjectWithContextMenu, ModelObjectWithChildren, OnMaster, Loadable {
     private final transient Queue queue;
@@ -2388,7 +2388,7 @@
             return false;
         }
 
         public FormValidation doCheckNumExecutors(@QueryParameter String value) {
             return FormValidation.validateNonNegativeInteger(value);
         }
 
@@ -4195,7 +4195,7 @@
      * @since 2.414
      */
     @RequirePOST
     public HttpRedirect doQuietDown(@QueryParameter boolean block,
                                     @QueryParameter int timeout,
                                     @QueryParameter @CheckForNull String message,
                                     @QueryParameter boolean safeRestart) throws InterruptedException, IOException {
@@ -4633,7 +4633,7 @@
      *
      * @since 2.414
      */
     public HttpResponse doSafeRestart(StaplerRequest req, @QueryParameter("message") String message) throws IOException, ServletException, RestartNotSupportedException {
         checkPermission(MANAGE);
         if (req != null && req.getMethod().equals("GET")) {
             return HttpResponses.forwardToView(this, "_safeRestart.jelly");
@@ -4959,7 +4959,7 @@
     /**
      * If the user chose the default JDK, make sure we got 'java' in PATH.
      */
     public FormValidation doDefaultJDKCheck(StaplerRequest request, @QueryParameter String value) {
         if (!JDK.isDefaultName(value))
             // assume the user configured named ones properly in system config ---
             // or else system config should have reported form field validation errors.
@@ -4976,7 +4976,7 @@
      * Checks if a top-level view with the given name exists and
      * make sure that the name is good as a view name.
      */
     public FormValidation doCheckViewName(@QueryParameter String value) {
         checkPermission(View.CREATE);
 
         String name = fixEmpty(value);
@@ -5002,7 +5002,7 @@
      * @deprecated 1.512
      */
     @Deprecated
     public FormValidation doViewExistsCheck(@QueryParameter String value) {
         checkPermission(View.CREATE);
 
         String view = fixEmpty(value);
@@ -5290,8 +5290,9 @@
      * 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
@@ -5315,8 +5316,8 @@
      * @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
@@ -5338,17 +5339,45 @@
      * 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 {

test/src/test/java/jenkins/model/JenkinsTest.java

@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
     }
 
@@ -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);
     }
 
@@ -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);
     }