making existing duplicate cloud name editable as part of fixes for cl…

…oud tracking (jenkinsci#401) * making exisitng duplicate cloud name editable * addressing pr comments and making corresponding changes for fleet label cloud * Update src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java Co-authored-by: Prathibha Datta Kumar <[email protected]> * Update src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java Co-authored-by: Prathibha Datta Kumar <[email protected]> * Update src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly Co-authored-by: Prathibha Datta Kumar <[email protected]> * Update src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud/config.jelly Co-authored-by: Prathibha Datta Kumar <[email protected]> --------- Co-authored-by: Prathibha Datta Kumar <[email protected]>
henrykie · Aug 24, 2023 · 05b24e2 · 05b24e2
1 parent efbed25
commit 05b24e2
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 2 deletions.
diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/CloudNames.java b/src/main/java/com/amazon/jenkins/ec2fleet/CloudNames.java
@@ -1,5 +1,6 @@
 package com.amazon.jenkins.ec2fleet;
 
+import hudson.slaves.Cloud;
 import jenkins.model.Jenkins;
 
 import java.util.Collections;
@@ -12,6 +13,8 @@ public static Boolean isUnique(final String name) {
     return !Jenkins.get().clouds.stream().anyMatch(c -> c.name.equals(name));
   }
 
+  public static Boolean isDuplicated(final String name) { return Jenkins.get().clouds.stream().filter(c -> c.name.equals(name)).count() > 1; }
+
   public static String generateUnique(final String defaultName) {
     final Set<String> usedNames = Jenkins.get().clouds != null
             ? Jenkins.get().clouds.stream().map(c -> c.name).collect(Collectors.toSet())

diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetCloud.java
@@ -973,6 +973,11 @@ public FormValidation doCheckName(@QueryParameter final String name, @QueryParam
             if (Boolean.valueOf(isNewCloud) && !CloudNames.isUnique(name)) {
                 return FormValidation.error("Please choose a unique name. Existing clouds: " + Jenkins.get().clouds.stream().map(c -> c.name).collect(Collectors.joining(",")));
             }
+            else if (!Boolean.valueOf(isNewCloud) && CloudNames.isDuplicated(name)) {
+                Set<String> uniqueNames = new HashSet<>();
+                Jenkins.get().clouds.forEach(cloud -> {uniqueNames.add(cloud.name);});
+                return FormValidation.error("This cloud name is not unique. Please choose a unique name and click save. Existing clouds: " + uniqueNames);
+            }
 
             return FormValidation.ok();
         }
@@ -1001,6 +1006,8 @@ public String getDefaultCloudName() {
             return CloudNames.generateUnique(DEFAULT_FLEET_CLOUD_ID);
         }
 
+        public Boolean isExistingCloudNameDuplicated(@QueryParameter final String name) { return CloudNames.isDuplicated(name); }
+
         public FormValidation doCheckFleet(@QueryParameter final String fleet) {
             if (StringUtils.isEmpty(fleet)) {
                 return FormValidation.error("Please select a valid EC2 Fleet");

diff --git a/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java b/src/main/java/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud.java
@@ -863,6 +863,11 @@ public FormValidation doCheckName(@QueryParameter final String name, @QueryParam
             if (Boolean.valueOf(isNewCloud) && !CloudNames.isUnique(name)) {
                 return FormValidation.error("Please choose a unique name. Existing clouds: " + Jenkins.get().clouds.stream().map(c -> c.name).collect(Collectors.joining(",")));
             }
+            else if (!Boolean.valueOf(isNewCloud) && CloudNames.isDuplicated(name)) {
+                Set<String> uniqueNames = new HashSet<>();
+                Jenkins.get().clouds.forEach(cloud -> {uniqueNames.add(cloud.name);});
+                return FormValidation.error("This cloud name is not unique. Please choose a unique name and click save. Existing clouds: " + uniqueNames);
+            }
 
             return FormValidation.ok();
         }
@@ -871,6 +876,8 @@ public String getDefaultCloudName() {
             return CloudNames.generateUnique(DEFAULT_FLEET_CLOUD_ID);
         }
 
+        public Boolean isExistingCloudNameDuplicated(@QueryParameter final String name) { return CloudNames.isDuplicated(name); }
+
         @Override
         public boolean configure(final StaplerRequest req, final JSONObject formData) throws FormException {
             req.bindJSON(this, formData);

diff --git a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetCloud/config.jelly
@@ -13,7 +13,7 @@
 
     <f:entry title="${%Name}" field="name">
         <j:choose>
-            <j:when test="${instance != null}">
+            <j:when test="${instance != null and not descriptor.isExistingCloudNameDuplicated(instance.name)}">
                 <!-- Warning: Using <j:set var="readOnlyMode" value="true"/> instead of readOnlyTextbox shows some weird, hard-to-debug behavior where it makes the name null for existing clouds when a new cloud is created.-->
                 <f:readOnlyTextbox/>
             </j:when>

diff --git a/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud/config.jelly b/src/main/resources/com/amazon/jenkins/ec2fleet/EC2FleetLabelCloud/config.jelly
@@ -13,7 +13,7 @@
 
     <f:entry title="${%Name}" field="name">
         <j:choose>
-            <j:when test="${instance != null}">
+            <j:when test="${instance != null and not descriptor.isExistingCloudNameDuplicated(instance.name)}">
                 <!-- Warning: Using <j:set var="readOnlyMode" value="true"/> instead of readOnlyTextbox shows some weird, hard-to-debug behavior where it makes the name null for existing clouds when a new cloud is created.-->
                 <f:readOnlyTextbox/>
             </j:when>

diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/CloudNamesTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/CloudNamesTest.java
@@ -31,6 +31,40 @@ public void isUnique_false() {
     Assert.assertFalse(CloudNames.isUnique("SomeDefaultName"));
   }
 
+  @Test
+  public void isDuplicated_false() {
+    j.jenkins.clouds.add(new EC2FleetCloud("TestCloud", null, null, null, null, null,
+        "test-label", null, null, false, false,
+        0, 0, 0, 0, 0, true, false,
+        "-1", false, 0, 0, false,
+        10, false));
+
+    j.jenkins.clouds.add(new EC2FleetCloud("TestCloud2", null, null, null, null, null,
+        "test-label", null, null, false, false,
+        0, 0, 0, 0, 0, true, false,
+        "-1", false, 0, 0, false,
+        10, false));
+
+    Assert.assertFalse(CloudNames.isDuplicated("TestCloud"));
+  }
+
+  @Test
+  public void isDuplicated_true() {
+    j.jenkins.clouds.add(new EC2FleetCloud("TestCloud", null, null, null, null, null,
+        "test-label", null, null, false, false,
+        0, 0, 0, 0, 0, true, false,
+        "-1", false, 0, 0, false,
+        10, false));
+
+    j.jenkins.clouds.add(new EC2FleetCloud("TestCloud", null, null, null, null, null,
+        "test-label", null, null, false, false,
+        0, 0, 0, 0, 0, true, false,
+        "-1", false, 0, 0, false,
+        10, false));
+
+    Assert.assertTrue(CloudNames.isDuplicated("TestCloud"));
+  }
+
   @Test
   public void generateUnique_noSuffix() {
     Assert.assertEquals("FleetCloud", CloudNames.generateUnique("FleetCloud"));

diff --git a/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java b/src/test/java/com/amazon/jenkins/ec2fleet/UiIntegrationTest.java
@@ -32,6 +32,7 @@
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
@@ -308,4 +309,24 @@ public void verifyCloudNameReadOnlyAfterCloudCreated() throws Exception {
         List<DomElement> elementsByName = IntegrationTest.getElementsByNameWithoutJdk(page, "_.name");
         assertTrue(((HtmlTextInput) elementsByName.get(0)).isReadOnly());
     }
+
+    @Test
+    public void verifyExistingDuplicateCloudNamesEditable() throws Exception {
+        j.jenkins.clouds.add(new EC2FleetCloud("test-cloud", null, null, null, null, "",
+            "label", null, null, false, false,
+            0, 0, 0, 0, 0, true, false,
+            "-1", false, 0, 0, false,
+            10, false));
+        j.jenkins.clouds.add(new EC2FleetCloud("test-cloud", null, null, null, null, "",
+            "label", null, null, false, false,
+            0, 0, 0, 0, 0, true, false,
+            "-1", false, 0, 0, false,
+            10, false));
+
+        HtmlPage page = j.createWebClient().goTo("configureClouds");
+
+        List<DomElement> elementsByName = IntegrationTest.getElementsByNameWithoutJdk(page, "_.name");
+        assertFalse(((HtmlTextInput) elementsByName.get(0)).isReadOnly());
+        assertFalse(((HtmlTextInput) elementsByName.get(1)).isReadOnly());
+    }
 }