Merge pull request #194 from Vlatombe/JENKINS-65333

[JENKINS-65333] Sort credentials entries only for UI
jenkinsci · Apr 9, 2021 · 92abc7d · 92abc7d
2 parents 589d639 + 6bd2f3d
commit 92abc7d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsProvider.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsProvider.java
@@ -438,7 +438,6 @@ public static <C extends Credentials> List<C> lookupCredentials(@NonNull Class<C
                 }
             }
         }
-        result.sort(new CredentialsNameComparator());
         return result;
     }
 
@@ -575,8 +574,6 @@ public static <C extends Credentials> List<C> lookupCredentials(@NonNull Class<C
                 }
             }
         }
-
-        result.sort(new CredentialsNameComparator());
         return result;
     }
 
@@ -1180,6 +1177,7 @@ public <C extends IdCredentials> ListBoxModel getCredentialIds(@NonNull Class<C>
         return getCredentials(type, itemGroup, authentication, domainRequirements)
                 .stream()
                 .filter(matcher::matches)
+                .sorted(new CredentialsNameComparator())
                 .map(c -> new ListBoxModel.Option(CredentialsNameProvider.name(c), c.getId()))
                 .collect(Collectors.toCollection(ListBoxModel::new));
     }
@@ -1255,6 +1253,7 @@ public <C extends IdCredentials> ListBoxModel getCredentialIds(@NonNull Class<C>
         return getCredentials(type, item, authentication, domainRequirements)
                 .stream()
                 .filter(matcher::matches)
+                .sorted(new CredentialsNameComparator())
                 .map(c -> new ListBoxModel.Option(CredentialsNameProvider.name(c), c.getId()))
                 .collect(Collectors.toCollection(ListBoxModel::new));
     }

diff --git a/src/test/java/com/cloudbees/plugins/credentials/CredentialsProviderTest.java b/src/test/java/com/cloudbees/plugins/credentials/CredentialsProviderTest.java
@@ -27,6 +27,7 @@
 import com.cloudbees.plugins.credentials.domains.Domain;
 import com.cloudbees.plugins.credentials.domains.DomainRequirement;
 import com.cloudbees.plugins.credentials.impl.DummyCredentials;
+import com.cloudbees.plugins.credentials.impl.DummyIdCredentials;
 import com.cloudbees.plugins.credentials.impl.DummyLegacyCredentials;
 import hudson.model.Descriptor;
 import hudson.model.FreeStyleProject;
@@ -39,12 +40,14 @@
 import hudson.slaves.JNLPLauncher;
 import hudson.slaves.RetentionStrategy;
 import hudson.security.ACL;
+import hudson.util.ListBoxModel;
 import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.context.SecurityContext;
 import org.acegisecurity.context.SecurityContextHolder;
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import java.io.ByteArrayInputStream;
@@ -55,6 +58,9 @@
 import java.util.Collections;
 import java.util.List;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -335,4 +341,30 @@ public void trackingOfFingerprintDependsOnConfiguration() throws Exception {
             CredentialsProvider.FINGERPRINT_ENABLED = true;
         }
     }
+
+    @Test
+    @Issue("JENKINS-65333")
+    public void insertionOrderLookupCredentials() {
+        assertThat(CredentialsProvider.lookupCredentials(Credentials.class, (Item) null, ACL.SYSTEM, Collections.emptyList()), hasSize(0));
+        SystemCredentialsProvider.getInstance().getCredentials().add(new DummyIdCredentials("1", CredentialsScope.SYSTEM, "beta", "bar", "description 1"));
+        SystemCredentialsProvider.getInstance().getCredentials().add(new DummyIdCredentials("2", CredentialsScope.SYSTEM, "alpha", "bar", "description 2"));
+        List<DummyIdCredentials> credentials = CredentialsProvider.lookupCredentials(DummyIdCredentials.class, (Item) null, ACL.SYSTEM, Collections.emptyList());
+        assertThat(credentials, hasSize(2));
+        // Insertion order
+        assertThat(credentials.get(0).getUsername(), is("beta"));
+        assertThat(credentials.get(1).getUsername(), is("alpha"));
+    }
+
+    @Test
+    @Issue("JENKINS-65333")
+    public void credentialsSortedByNameInUI() {
+        assertThat(CredentialsProvider.lookupCredentials(Credentials.class, (Item) null, ACL.SYSTEM, Collections.emptyList()), hasSize(0));
+        SystemCredentialsProvider.getInstance().getCredentials().add(new DummyIdCredentials("1", CredentialsScope.SYSTEM, "beta", "bar", "description 1"));
+        SystemCredentialsProvider.getInstance().getCredentials().add(new DummyIdCredentials("2", CredentialsScope.SYSTEM, "alpha", "bar", "description 2"));
+        ListBoxModel options = CredentialsProvider.listCredentials(DummyIdCredentials.class, (Item) null, ACL.SYSTEM, Collections.emptyList(), CredentialsMatchers.always());
+        // Options are sorted by name
+        assertThat(options, hasSize(2));
+        assertThat(options.get(0).value, is("2"));
+        assertThat(options.get(1).value, is("1"));
+    }
 }