From 04887f08ce06eaae75ef21b6dfd008e38e865d9c Mon Sep 17 00:00:00 2001 From: Jan Faracik <43062514+janfaracik@users.noreply.github.com> Date: Tue, 29 Mar 2022 05:52:43 +0100 Subject: [PATCH] [JENKINS-68137] Enable custom icon symbols for plugins (#6395) Co-authored-by: Ullrich Hafner Co-authored-by: Tim Jacomb --- core/src/main/java/hudson/Functions.java | 20 +++++++++++ .../java/org/jenkins/ui/icon/IconSet.java | 33 +++++++++++++++---- .../main/resources/lib/hudson/actions.jelly | 2 +- .../resources/lib/hudson/ballColorTd.jelly | 2 +- core/src/main/resources/lib/layout/icon.jelly | 1 + core/src/test/java/hudson/FunctionsTest.java | 30 +++++++++++++++++ .../java/org/jenkins/ui/icon/IconSetTest.java | 12 +++---- test/src/test/java/lib/layout/IconTest.java | 2 +- 8 files changed, 87 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index d0f0b5c84f..879791296a 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -2320,6 +2320,26 @@ public class Functions { return iconMetadata; } + @Restricted(NoExternalUse.class) + public static String extractPluginNameFromIconSrc(String iconSrc) { + if (iconSrc == null) { + return ""; + } + + if (!iconSrc.contains("plugin-")) { + return ""; + } + + String[] arr = iconSrc.split(" "); + for (String element : arr) { + if (element.startsWith("plugin-")) { + return element.replace("plugin-", ""); + } + } + + return ""; + } + @Restricted(NoExternalUse.class) public static String tryGetIconPath(String iconGuess, JellyContext context) { if (iconGuess == null) { diff --git a/core/src/main/java/org/jenkins/ui/icon/IconSet.java b/core/src/main/java/org/jenkins/ui/icon/IconSet.java index 1b0559c55c..3f7dd29792 100644 --- a/core/src/main/java/org/jenkins/ui/icon/IconSet.java +++ b/core/src/main/java/org/jenkins/ui/icon/IconSet.java @@ -24,6 +24,8 @@ package org.jenkins.ui.icon; +import hudson.PluginWrapper; +import hudson.Util; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -32,6 +34,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import jenkins.model.Jenkins; import org.apache.commons.io.IOUtils; import org.apache.commons.jelly.JellyContext; import org.apache.commons.lang.StringUtils; @@ -47,7 +50,8 @@ public class IconSet { public static final IconSet icons = new IconSet(); - private static final Map SYMBOLS = new ConcurrentHashMap<>(); + // keyed by plugin name / core, and then symbol name returning the SVG as a string + private static final Map> SYMBOLS = new ConcurrentHashMap<>(); private Map iconsByCSSSelector = new ConcurrentHashMap<>(); private Map iconsByUrl = new ConcurrentHashMap<>(); @@ -77,11 +81,14 @@ public class IconSet { // for Jelly @Restricted(NoExternalUse.class) - public static String getSymbol(String name, String title, String tooltip, String classes) { + public static String getSymbol(String name, String title, String tooltip, String classes, String pluginName) { String translatedName = cleanName(name); - if (SYMBOLS.containsKey(translatedName)) { - String symbol = SYMBOLS.get(translatedName); + String identifier = Util.fixEmpty(pluginName) == null ? "core" : pluginName; + Map symbolsForLookup = SYMBOLS.computeIfAbsent(identifier, (key) -> new ConcurrentHashMap<>()); + + if (symbolsForLookup.containsKey(translatedName)) { + String symbol = symbolsForLookup.get(translatedName); symbol = symbol.replaceAll("(class=\")[^&]*?(\")", "$1$2"); symbol = symbol.replaceAll("(tooltip=\")[^&]*?(\")", ""); if (!tooltip.isEmpty()) { @@ -92,7 +99,7 @@ public class IconSet { } // Load symbol if it exists - InputStream inputStream = IconSet.class.getResourceAsStream("/images/symbols/" + translatedName + ".svg"); + InputStream inputStream = getClassLoader(identifier).getResourceAsStream("images/symbols/" + translatedName + ".svg"); String symbol = null; try { @@ -116,11 +123,25 @@ public class IconSet { symbol = symbol.replaceAll(" - + diff --git a/core/src/main/resources/lib/hudson/ballColorTd.jelly b/core/src/main/resources/lib/hudson/ballColorTd.jelly index 74beb1e506..5c53f790fb 100644 --- a/core/src/main/resources/lib/hudson/ballColorTd.jelly +++ b/core/src/main/resources/lib/hudson/ballColorTd.jelly @@ -58,7 +58,7 @@ THE SOFTWARE. - + diff --git a/core/src/main/resources/lib/layout/icon.jelly b/core/src/main/resources/lib/layout/icon.jelly index 68705fa085..5e0479554a 100644 --- a/core/src/main/resources/lib/layout/icon.jelly +++ b/core/src/main/resources/lib/layout/icon.jelly @@ -69,6 +69,7 @@ THE SOFTWARE. + diff --git a/core/src/test/java/hudson/FunctionsTest.java b/core/src/test/java/hudson/FunctionsTest.java index cf34b0912e..85ae2ca704 100644 --- a/core/src/test/java/hudson/FunctionsTest.java +++ b/core/src/test/java/hudson/FunctionsTest.java @@ -25,6 +25,8 @@ package hudson; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.emptyString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -369,6 +371,34 @@ public class FunctionsTest { assertEquals("<discarded/>\n", Functions.printLogRecordHtml(lr, null)[3]); } + @Test + public void extractPluginNameFromIconSrcHandlesNull() { + String result = Functions.extractPluginNameFromIconSrc(null); + + assertThat(result, is(emptyString())); + } + + @Test + public void extractPluginNameFromIconSrcHandlesEmptyString() { + String result = Functions.extractPluginNameFromIconSrc(""); + + assertThat(result, is(emptyString())); + } + + @Test + public void extractPluginNameFromIconSrcOnlyReturnsPluginFromStart() { + String result = Functions.extractPluginNameFromIconSrc("symbol-plugin-mailer plugin-design-library"); + + assertThat(result, is(equalTo("design-library"))); + } + + @Test + public void extractPluginNameFromIconSrcExtractsPlugin() { + String result = Functions.extractPluginNameFromIconSrc("symbol-padlock plugin-design-library"); + + assertThat(result, is(equalTo("design-library"))); + } + @Issue("JDK-6507809") @Test public void printThrowable() { // Basics: a single exception. No change. diff --git a/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java b/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java index 9d1027b871..c52a4a45dd 100644 --- a/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java +++ b/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java @@ -21,7 +21,7 @@ public class IconSetTest { @Test void getSymbol() { - String symbol = IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2"); + String symbol = IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", ""); assertThat(symbol, containsString("Title")); assertThat(symbol, containsString("tooltip=\"Tooltip\"")); @@ -30,8 +30,8 @@ public class IconSetTest { @Test void getSymbol_cachedSymbolDoesntReturnAttributes() { - IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2"); - String symbol = IconSet.getSymbol("download", "", "", ""); + IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", ""); + String symbol = IconSet.getSymbol("download", "", "", "", ""); assertThat(symbol, not(containsString("Title"))); assertThat(symbol, not(containsString("tooltip=\"Tooltip\""))); @@ -40,8 +40,8 @@ public class IconSetTest { @Test void getSymbol_cachedSymbolAllowsSettingAllAttributes() { - IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2"); - String symbol = IconSet.getSymbol("download", "Title2", "Tooltip2", "class3 class4"); + IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", ""); + String symbol = IconSet.getSymbol("download", "Title2", "Tooltip2", "class3 class4", ""); assertThat(symbol, not(containsString("Title"))); assertThat(symbol, not(containsString("tooltip=\"Tooltip\""))); @@ -57,7 +57,7 @@ public class IconSetTest { */ @Test void getSymbol_notSettingTooltipDoesntAddTooltipAttribute() { - String symbol = IconSet.getSymbol("download", "Title", "", "class1 class2"); + String symbol = IconSet.getSymbol("download", "Title", "", "class1 class2", ""); assertThat(symbol, not(containsString("tooltip"))); } diff --git a/test/src/test/java/lib/layout/IconTest.java b/test/src/test/java/lib/layout/IconTest.java index c82f341067..efe9952688 100644 --- a/test/src/test/java/lib/layout/IconTest.java +++ b/test/src/test/java/lib/layout/IconTest.java @@ -94,7 +94,7 @@ public class IconTest { DomElement ballColorAborted = p.getElementById("ballColorAborted"); List ballIcons = StreamSupport.stream(ballColorAborted.getChildElements().spliterator(), false).collect(Collectors.toList()); - assertIconToSvgIconOkay(ballIcons.get(0).getFirstElementChild(), "icon-aborted icon-lg"); + assertIconToSvgIconOkay(ballIcons.get(0).getFirstElementChild(), "icon-aborted"); DomElement statusIcons = p.getElementById("statusIcons"); List statusIconsList = StreamSupport.stream(statusIcons.getChildElements().spliterator(), false).collect(Collectors.toList());