From a7cf5200f71acbce8f538e3167174f6d9e01bccc Mon Sep 17 00:00:00 2001 From: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com> Date: Wed, 29 Mar 2023 10:59:26 +0200 Subject: [PATCH] Make sorting of available icons more stable --- .../plugins/foldericon/CustomFolderIcon.java | 23 +++---- .../foldericon/CustomFolderIconTest.java | 65 ++++++++++++------- 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/main/java/jenkins/plugins/foldericon/CustomFolderIcon.java b/src/main/java/jenkins/plugins/foldericon/CustomFolderIcon.java index d5cd0feb..31c0175c 100644 --- a/src/main/java/jenkins/plugins/foldericon/CustomFolderIcon.java +++ b/src/main/java/jenkins/plugins/foldericon/CustomFolderIcon.java @@ -41,7 +41,8 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; import java.io.IOException; -import java.util.List; +import java.util.Comparator; +import java.util.Set; import java.util.UUID; import java.util.logging.Level; import java.util.logging.Logger; @@ -79,24 +80,24 @@ public CustomFolderIcon(String foldericon) { * * @return all the icons that have been uploaded, sorted descending by {@link FilePath#lastModified()}. */ - public static List getAvailableIcons() { + public static Set getAvailableIcons() { try { FilePath iconDir = Jenkins.get().getRootPath().child(USER_CONTENT_PATH).child(PLUGIN_PATH); if (iconDir.exists()) { - return iconDir.list().stream().sorted((file1, file2) -> { + return iconDir.list().stream().sorted(Comparator.comparingLong((FilePath file) -> { try { - return Long.compare(file2.lastModified(), file1.lastModified()); - } catch (Exception ex) { + return file.lastModified(); + } catch (IOException | InterruptedException ex) { return 0; } - }).map(FilePath::getName).collect(Collectors.toList()); + }).reversed()).map(FilePath::getName).collect(Collectors.toSet()); } else { - return List.of(); + return Set.of(); } } catch (IOException | InterruptedException ex) { LOGGER.log(Level.WARNING, ex, () -> "Unable to list available icons!"); - return List.of(); + return Set.of(); } } @@ -207,12 +208,12 @@ public HttpResponse doCleanup(StaplerRequest req) { FilePath iconDir = Jenkins.get().getRootPath().child(USER_CONTENT_PATH).child(PLUGIN_PATH); - List existingIcons = getAvailableIcons(); + Set existingIcons = getAvailableIcons(); - List usedIcons = Jenkins.get().getAllItems(AbstractFolder.class).stream() + Set usedIcons = Jenkins.get().getAllItems(AbstractFolder.class).stream() .filter(folder -> folder.getIcon() instanceof CustomFolderIcon) .map(folder -> ((CustomFolderIcon) folder.getIcon()).getFoldericon()) - .collect(Collectors.toList()); + .collect(Collectors.toSet()); if (usedIcons.isEmpty() || existingIcons.removeAll(usedIcons)) { for (String icon : existingIcons) { diff --git a/src/test/java/jenkins/plugins/foldericon/CustomFolderIconTest.java b/src/test/java/jenkins/plugins/foldericon/CustomFolderIconTest.java index 4d17acd1..924766f6 100644 --- a/src/test/java/jenkins/plugins/foldericon/CustomFolderIconTest.java +++ b/src/test/java/jenkins/plugins/foldericon/CustomFolderIconTest.java @@ -53,7 +53,9 @@ import java.io.RandomAccessFile; import java.lang.reflect.Field; import java.util.Arrays; +import java.util.Comparator; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.*; @@ -612,7 +614,7 @@ void testDoCleanupFileNotDeletedWithMockedException(JenkinsRule r) throws Except */ @Test void testGetAvailableIcons(JenkinsRule r) throws Exception { - List icons = CustomFolderIcon.getAvailableIcons(); + Set icons = CustomFolderIcon.getAvailableIcons(); assertNotNull(icons); assertTrue(icons.isEmpty()); @@ -621,30 +623,33 @@ void testGetAvailableIcons(JenkinsRule r) throws Exception { FilePath iconDir = userContent.child("customFolderIcons"); iconDir.mkdirs(); - String filename1 = System.nanoTime() + ".png"; + long timestamp1 = System.nanoTime(); + String filename1 = timestamp1 + ".png"; FilePath file1 = iconDir.child(filename1); - file1.touch(System.nanoTime()); + file1.touch(timestamp1); - String filename2 = System.nanoTime() + ".png"; + long timestamp2 = System.nanoTime(); + String filename2 = timestamp2 + ".png"; FilePath file2 = iconDir.child(filename2); - file2.touch(System.nanoTime()); + file2.touch(timestamp2); - String filename3 = System.nanoTime() + ".png"; + long timestamp3 = System.nanoTime(); + String filename3 = timestamp3 + ".png"; FilePath file3 = iconDir.child(filename3); - file3.touch(System.nanoTime()); + file3.touch(timestamp3); icons = CustomFolderIcon.getAvailableIcons(); assertNotNull(icons); assertEquals(3, icons.size()); - List expected = Arrays.asList(file1, file2, file3).stream().sorted((filePath1, filePath2) -> { + Set expected = Arrays.asList(file1, file2, file3).stream().sorted(Comparator.comparingLong((FilePath file) -> { try { - return Long.compare(filePath2.lastModified(), filePath1.lastModified()); - } catch (Exception ex) { + return file.lastModified(); + } catch (IOException | InterruptedException ex) { return 0; } - }).map(FilePath::getName).collect(Collectors.toList()); + }).reversed()).map(FilePath::getName).collect(Collectors.toSet()); assertEquals(expected, icons); } @@ -656,7 +661,7 @@ void testGetAvailableIcons(JenkinsRule r) throws Exception { */ @Test void testGetAvailableIconsThrowingExceptions(JenkinsRule r) throws Exception { - List icons = CustomFolderIcon.getAvailableIcons(); + Set icons = CustomFolderIcon.getAvailableIcons(); assertNotNull(icons); assertTrue(icons.isEmpty()); @@ -665,13 +670,20 @@ void testGetAvailableIconsThrowingExceptions(JenkinsRule r) throws Exception { FilePath iconDir = userContent.child("customFolderIcons"); iconDir.mkdirs(); - String filename1 = System.nanoTime() + ".png"; + long timestamp1 = System.nanoTime(); + String filename1 = timestamp1 + ".png"; FilePath file1 = iconDir.child(filename1); - file1.touch(System.nanoTime()); + file1.touch(timestamp1); - String filename2 = System.nanoTime() + ".png"; + long timestamp2 = System.nanoTime(); + String filename2 = timestamp2 + ".png"; FilePath file2 = iconDir.child(filename2); - file2.touch(System.nanoTime()); + file2.touch(timestamp2); + + long timestamp3 = System.nanoTime(); + String filename3 = timestamp3 + ".png"; + FilePath file3 = iconDir.child(filename3); + file3.touch(timestamp3); try (MockedConstruction mocked = Mockito.mockConstructionWithAnswer(FilePath.class, invocation -> { String call = invocation.toString(); @@ -694,7 +706,7 @@ void testGetAvailableIconsThrowingExceptions(JenkinsRule r) throws Exception { */ @Test void testGetAvailableIconsComparatorThrowingExceptions(JenkinsRule r) throws Exception { - List icons = CustomFolderIcon.getAvailableIcons(); + Set icons = CustomFolderIcon.getAvailableIcons(); assertNotNull(icons); assertTrue(icons.isEmpty()); @@ -703,17 +715,20 @@ void testGetAvailableIconsComparatorThrowingExceptions(JenkinsRule r) throws Exc FilePath iconDir = userContent.child("customFolderIcons"); iconDir.mkdirs(); - String filename1 = System.nanoTime() + ".png"; + long timestamp1 = System.nanoTime(); + String filename1 = timestamp1 + ".png"; FilePath file1 = iconDir.child(filename1); - file1.touch(System.nanoTime()); + file1.touch(timestamp1); - String filename2 = System.nanoTime() + ".png"; + long timestamp2 = System.nanoTime(); + String filename2 = timestamp2 + ".png"; FilePath file2 = iconDir.child(filename2); - file2.touch(System.nanoTime()); + file2.touch(timestamp2); - String filename3 = System.nanoTime() + ".png"; + long timestamp3 = System.nanoTime(); + String filename3 = timestamp3 + ".png"; FilePath file3 = iconDir.child(filename3); - file3.touch(System.nanoTime()); + file3.touch(timestamp3);; final int[] counter = {0}; @@ -760,13 +775,13 @@ void testGetAvailableIconsComparatorThrowingExceptions(JenkinsRule r) throws Exc assertEquals(3, icons.size()); } - List expected = Arrays.asList(file1, file2, file3).stream().sorted((filePath1, filePath2) -> { + Set expected = Arrays.asList(file1, file2, file3).stream().sorted((filePath1, filePath2) -> { try { return Long.compare(filePath2.lastModified(), filePath1.lastModified()); } catch (Exception ex) { return 0; } - }).map(FilePath::getName).collect(Collectors.toList()); + }).map(FilePath::getName).collect(Collectors.toSet()); assertEquals(expected, icons); }