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 2d3253f1a036..e607a696125d 100644 --- a/core/src/main/java/org/jenkins/ui/icon/IconSet.java +++ b/core/src/main/java/org/jenkins/ui/icon/IconSet.java @@ -26,22 +26,15 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; -import hudson.Functions; -import hudson.PluginWrapper; -import hudson.Util; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; -import jenkins.model.Jenkins; -import org.apache.commons.io.IOUtils; import org.apache.commons.jelly.JellyContext; -import org.apache.commons.lang.StringUtils; +import org.jenkins.ui.symbol.Symbol; +import org.jenkins.ui.symbol.SymbolRequest; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -54,15 +47,12 @@ public class IconSet { public static final IconSet icons = new IconSet(); - // 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<>(); private Map iconsByClassSpec = new ConcurrentHashMap<>(); private Map coreIcons = new ConcurrentHashMap<>(); - private static final String PLACEHOLDER_SVG = "Close"; private static final Icon NO_ICON = new Icon("_", "_", "_"); public IconSet() { @@ -76,90 +66,20 @@ public static void initPageVariables(JellyContext context) { context.setVariable("icons", icons); } - private static String prependTitleIfRequired(String icon, String title) { - if (StringUtils.isNotBlank(title)) { - return "" + Util.xmlEscape(title) + "" + icon; - } - return icon; - } - // for Jelly + @SuppressWarnings("unused") @Restricted(NoExternalUse.class) public static String getSymbol(String name, String title, String tooltip, String htmlTooltip, String classes, String pluginName, String id) { - String translatedName = cleanName(name); - - 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=\").*?(\")", ""); - symbol = symbol.replaceAll("(data-html-tooltip=\").*?(\")", ""); - symbol = symbol.replaceAll("(id=\").*?(\")", ""); - if (!tooltip.isEmpty() && htmlTooltip.isEmpty()) { - symbol = symbol.replaceAll(").*()", "$1$2"); - symbol = symbol.replaceAll("(class=\").*?(\")", "$1$2"); - symbol = symbol.replaceAll("(tooltip=\").*?(\")", "$1$2"); - symbol = symbol.replaceAll("(data-html-tooltip=\").*?(\")", "$1$2"); - symbol = symbol.replaceAll("(id=\").*?(\")", ""); - if (!tooltip.isEmpty() && htmlTooltip.isEmpty()) { - symbol = symbol.replaceAll("> SYMBOLS = new ConcurrentHashMap<>(); + static final String PLACEHOLDER_SVG = + "Close"; + + /** + * A substring of the placeholder so that tests can match it. + */ + static final String PLACEHOLDER_MATCHER = "M368 368L144 144M368 144L144 368"; + + private Symbol() {} + + /** + * Generates the svg markup for the given symbol name and attributes. + * @param request the symbol request object. + * @return The svg markup for the symbol. + * @since TODO + */ + public static String get(@NonNull SymbolRequest request) { + String name = request.getName(); + String title = request.getTitle(); + String tooltip = request.getTooltip(); + String htmlTooltip = request.getHtmlTooltip(); + String classes = request.getClasses(); + String pluginName = request.getPluginName(); + String id = request.getId(); + + String identifier = StringUtils.defaultIfBlank(pluginName, "core"); + + String symbol = SYMBOLS + .computeIfAbsent(identifier, key -> new ConcurrentHashMap<>()) + .computeIfAbsent(name, key -> loadSymbol(identifier, key)); + if (StringUtils.isNotBlank(tooltip) && StringUtils.isBlank(htmlTooltip)) { + symbol = symbol.replaceAll("" + Util.xmlEscape(title) + "" + symbol; + } + return symbol; + } + + + @SuppressFBWarnings(value = {"NP_LOAD_OF_KNOWN_NULL_VALUE", "RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE"}, justification = "Spotbugs doesn't grok try-with-resources") + private static String loadSymbol(String namespace, String name) { + String markup = PLACEHOLDER_SVG; + ClassLoader classLoader = getClassLoader(namespace); + if (classLoader != null) { + try (InputStream inputStream = classLoader.getResourceAsStream("images/symbols/" + name + ".svg")) { + if (inputStream != null) { + markup = IOUtils.toString(inputStream, StandardCharsets.UTF_8); + } else { + LOGGER.log(Level.FINE, "Missing symbol " + name + " in " + namespace); + } + } catch (IOException e) { + LOGGER.log(Level.FINE, "Failed to load symbol " + name, e); + } + } + return markup.replaceAll("().*?()", "$1$2") + .replaceAll("A Symbol specification, to be passed to {@link Symbol#get(SymbolRequest)}. + * + *

Create an instance using {@link Builder}. + * + * @since TODO + */ +public final class SymbolRequest { + private static final Logger LOGGER = Logger.getLogger(SymbolRequest.class.getName()); + + /** + * The name of the symbol. + */ + @NonNull + private final String name; + + /** + * The symbol title. + */ + @CheckForNull + private final String title; + /** + * The tooltip to display when hovering over the symbol. Only displayed if {@link #htmlTooltip} is not set. + */ + @CheckForNull + private final String tooltip; + /** + * An HTML tooltip to display when hovering over the symbol. Overrides any value of the {@link #tooltip} field. + */ + @CheckForNull + private final String htmlTooltip; + /** + * Additional CSS classes to apply to the symbol. + */ + @CheckForNull + private final String classes; + /** + * The name of the plugin to load the symbol from. If null, the symbol will be resolved from core. + */ + @CheckForNull + private final String pluginName; + + /** + * The html id of the symbol. + */ + @CheckForNull + private final String id; + + @NonNull + public String getName() { + return name; + } + + @CheckForNull + public String getTitle() { + return title; + } + + @CheckForNull + public String getTooltip() { + return tooltip; + } + + @CheckForNull + public String getHtmlTooltip() { + return htmlTooltip; + } + + @CheckForNull + public String getClasses() { + return classes; + } + + @CheckForNull + public String getPluginName() { + return pluginName; + } + + @CheckForNull + public String getId() { + return id; + } + + private SymbolRequest(@NonNull String name, @CheckForNull String title, @CheckForNull String tooltip, @CheckForNull String htmlTooltip, @CheckForNull String classes, @CheckForNull String pluginName, + @CheckForNull String id) { + this.name = name; + this.title = title; + this.tooltip = tooltip; + this.htmlTooltip = htmlTooltip; + this.classes = classes; + this.pluginName = pluginName; + this.id = id; + } + + public static class Builder { + @CheckForNull + private String name; + @CheckForNull + private String title; + @CheckForNull + private String tooltip; + @CheckForNull + private String htmlTooltip; + @CheckForNull + private String classes; + @CheckForNull + private String pluginName; + @CheckForNull + private String id; + @CheckForNull + private String raw; + + @CheckForNull + public String getName() { + return name; + } + + public Builder withName(@NonNull String name) { + this.name = name; + return this; + } + + @CheckForNull + public String getTitle() { + return title; + } + + public Builder withTitle(@CheckForNull String title) { + this.title = title; + return this; + } + + @CheckForNull + public String getTooltip() { + return tooltip; + } + + public Builder withTooltip(@CheckForNull String tooltip) { + this.tooltip = tooltip; + return this; + } + + @CheckForNull + public String getHtmlTooltip() { + return htmlTooltip; + } + + public Builder withHtmlTooltip(@CheckForNull String htmlTooltip) { + this.htmlTooltip = htmlTooltip; + return this; + } + + @CheckForNull + public String getClasses() { + return classes; + } + + public Builder withClasses(@CheckForNull String classes) { + this.classes = classes; + return this; + } + + @CheckForNull + public String getPluginName() { + return pluginName; + } + + public Builder withPluginName(@CheckForNull String pluginName) { + this.pluginName = pluginName; + return this; + } + + @CheckForNull + public String getId() { + return id; + } + + public Builder withId(@CheckForNull String id) { + this.id = id; + return this; + } + + @CheckForNull + public String getRaw() { + return raw; + } + + public Builder withRaw(@CheckForNull String raw) { + this.raw = raw; + return this; + } + + @NonNull + public SymbolRequest build() { + if (name == null && pluginName == null && raw != null) { + parseRaw(raw); + LOGGER.fine(() -> "\"" + raw + "\" parsed to name: " + name + " and pluginName: " + pluginName); + } + if (name == null) { + throw new IllegalArgumentException("name cannot be null"); + } + return new SymbolRequest(name, title, tooltip, htmlTooltip, classes, pluginName, id); + } + + private void parseRaw(@NonNull String raw) { + String[] s = raw.split(" "); + if (s.length <= 2) { + for (String element : s) { + if (element.startsWith("symbol-")) { + name = element.substring("symbol-".length()); + } + if (element.startsWith("plugin-")) { + pluginName = element.substring("plugin-".length()); + } + if (name != null && pluginName != null) { + break; + } + } + } else { + throw new IllegalArgumentException("raw must be in the format \"symbol- plugin-\""); + } + } + } +} 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 cce22685cdac..cc34ec8b3b82 100644 --- a/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java +++ b/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java @@ -1,7 +1,7 @@ package org.jenkins.ui.icon; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.not; diff --git a/core/src/test/java/org/jenkins/ui/symbol/SymbolTest.java b/core/src/test/java/org/jenkins/ui/symbol/SymbolTest.java new file mode 100644 index 000000000000..095663ac2810 --- /dev/null +++ b/core/src/test/java/org/jenkins/ui/symbol/SymbolTest.java @@ -0,0 +1,193 @@ +package org.jenkins.ui.symbol; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import org.apache.commons.io.IOUtils; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +public class SymbolTest { + public static final String SCIENCE_PATH; + + public static final String IMAGES_SYMBOLS_SCIENCE_PATH_XML = "/images/symbols/science.path.xml"; + + static { + try { + try (InputStream resourceAsStream = SymbolTest.class.getResourceAsStream(IMAGES_SYMBOLS_SCIENCE_PATH_XML)) { + if (resourceAsStream == null) { + throw new IllegalStateException("Could not find resource" + IMAGES_SYMBOLS_SCIENCE_PATH_XML); + } + SCIENCE_PATH = IOUtils.toString(resourceAsStream, StandardCharsets.UTF_8).trim(); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Test + @DisplayName("Get symbol should build the symbol with given attributes") + void getSymbol() { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTitle("Title") + .withTooltip("Tooltip") + .withClasses("class1 class2") + .withId("id") + .build() + ); + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, containsString("Title")); + assertThat(symbol, containsString("tooltip=\"Tooltip\"")); + assertThat(symbol, containsString("class=\"class1 class2\"")); + assertThat(symbol, containsString("id=\"id\"")); + } + + @Test + @DisplayName("HTML tooltip overrides tooltip") + void htmlTooltipOverridesTooltip() { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTooltip("Tooltip") + .withHtmlTooltip("

Some HTML Tooltip

") + .build() + ); + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, not(containsString("tooltip=\"Tooltip\""))); + assertThat(symbol, containsString("data-html-tooltip=\"<p>Some HTML Tooltip</p>\"")); + } + + @Test + @DisplayName("Invalid strings should throw IllegalArgumentException") + void invalidRawString() { + assertThrows(IllegalArgumentException.class, () -> new SymbolRequest.Builder().build()); + assertThrows(IllegalArgumentException.class, () -> new SymbolRequest.Builder().withRaw("").build()); + assertThrows(IllegalArgumentException.class, () -> new SymbolRequest.Builder().withRaw("foobar").build()); + assertThrows(IllegalArgumentException.class, () -> new SymbolRequest.Builder().withRaw("plugin-foo").build()); + assertThrows(IllegalArgumentException.class, () -> new SymbolRequest.Builder().withRaw("symbol-foo plugin-bar someclass").build()); + } + + @Test + @DisplayName("Given a raw string it can be parsed to a name") + void rawStringCore() { + SymbolRequest symbol = new SymbolRequest.Builder().withRaw("symbol-gear").build(); + assertNull(symbol.getPluginName()); + assertEquals("gear", symbol.getName()); + } + + @Test + @DisplayName("Given a raw string it can be parsed to a name and plugin") + void rawStringPlugin() { + SymbolRequest symbol = new SymbolRequest.Builder().withRaw("symbol-science plugin-foo").build(); + assertEquals("foo", symbol.getPluginName()); + assertEquals("science", symbol.getName()); + } + + @Test + @DisplayName("Given a cached symbol, a new request should not return attributes from the cache") + void getSymbol_cachedSymbolDoesntReturnAttributes() { + Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTitle("Title") + .withTooltip("Tooltip") + .withClasses("class1 class2") + .withId("id") + .build() + ); + String symbol = Symbol.get(new SymbolRequest.Builder().withName("science").build()); + + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, not(containsString("Title"))); + assertThat(symbol, not(containsString("tooltip=\"Tooltip\""))); + assertThat(symbol, not(containsString("class=\"class1 class2\""))); + assertThat(symbol, not(containsString("id=\"id\""))); + + } + + @Test + @DisplayName("Given a cached symbol, a new request can specify new attributes to use") + void getSymbol_cachedSymbolAllowsSettingAllAttributes() { + Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTitle("Title") + .withTooltip("Tooltip") + .withClasses("class1 class2") + .withId("id") + .build() + ); + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTitle("Title2") + .withTooltip("Tooltip2") + .withClasses("class3 class4") + .withId("id2") + .build() + ); + + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, not(containsString("Title"))); + assertThat(symbol, not(containsString("tooltip=\"Tooltip\""))); + assertThat(symbol, not(containsString("class=\"class1 class2\""))); + assertThat(symbol, not(containsString("id=\"id\""))); + assertThat(symbol, containsString("Title2")); + assertThat(symbol, containsString("tooltip=\"Tooltip2\"")); + assertThat(symbol, containsString("class=\"class3 class4\"")); + assertThat(symbol, containsString("id=\"id2\"")); + } + + /** + * YUI tooltips require that the attribute not be set, otherwise a white rectangle will show on hover + * TODO: This might be able to be removed when we move away from YUI tooltips to a better solution + */ + @Test + @DisplayName("When omitting tooltip from attributes, the symbol should not have a tooltip") + void getSymbol_notSettingTooltipDoesntAddTooltipAttribute() { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withTitle("Title") + .withClasses("class1 class2") + .withId("id") + .build() + ); + + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, not(containsString("tooltip"))); + } + + @Test + @DisplayName("When resolving a missing symbol, a placeholder is generated instead") + void missingSymbolDefaultsToPlaceholder() { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("missing-icon") + .build() + ); + assertThat(symbol, not(containsString(SCIENCE_PATH))); + assertThat(symbol, containsString(Symbol.PLACEHOLDER_MATCHER)); + } + + @Test + @DisplayName("If tooltip is not provided symbol should never have a tooltip") + void getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand() { + SymbolRequest.Builder builder = new SymbolRequest.Builder() + .withName("science") + .withTitle("Title") + .withTooltip("With&Ampersand") + .withClasses("class1 class2") + .withId("id"); + String symbol = Symbol.get(builder.build()); + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, containsString("tooltip")); + // Remove tooltip + builder.withTooltip(null); + symbol = Symbol.get(builder.build()); + assertThat(symbol, containsString(SCIENCE_PATH)); + assertThat(symbol, not(containsString("tooltip"))); + } +} diff --git a/core/src/test/resources/images/symbols/science.path.xml b/core/src/test/resources/images/symbols/science.path.xml new file mode 100644 index 000000000000..5fc00c0db8c2 --- /dev/null +++ b/core/src/test/resources/images/symbols/science.path.xml @@ -0,0 +1 @@ + diff --git a/core/src/test/resources/images/symbols/science.svg b/core/src/test/resources/images/symbols/science.svg new file mode 100644 index 000000000000..f06499dd99cb --- /dev/null +++ b/core/src/test/resources/images/symbols/science.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/pom.xml b/test/pom.xml index ca10028af4c0..1a5fb217370a 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -267,6 +267,14 @@ THE SOFTWARE. ${project.build.outputDirectory}/old-remoting remoting-unsupported.jar + + io.jenkins.plugins + design-library + 91.v257f311ea_1dc + hpi + ${project.build.outputDirectory}/plugins + design-library.jpi + diff --git a/test/src/test/java/org/jenkins/ui/symbol/SymbolJenkinsTest.java b/test/src/test/java/org/jenkins/ui/symbol/SymbolJenkinsTest.java new file mode 100644 index 000000000000..65d225e292d7 --- /dev/null +++ b/test/src/test/java/org/jenkins/ui/symbol/SymbolJenkinsTest.java @@ -0,0 +1,46 @@ +package org.jenkins.ui.symbol; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.jupiter.api.DisplayName; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.RealJenkinsRule; + +public class SymbolJenkinsTest { + @Rule + public RealJenkinsRule rjr = new RealJenkinsRule().addPlugins("plugins/design-library.jpi"); + + @Test + @DisplayName("When resolving a symbol from a missing plugin, the placeholder is generated instead") + public void missingSymbolFromPluginDefaultsToPlaceholder() throws Throwable { + rjr.then(SymbolJenkinsTest::_missingSymbolFromPluginDefaultsToPlaceholder); + } + + private static void _missingSymbolFromPluginDefaultsToPlaceholder(JenkinsRule j) { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("science") + .withPluginName("missing-plugin") + .build() + ); + assertThat(symbol, containsString(Symbol.PLACEHOLDER_MATCHER)); + } + + @Test + @DisplayName("Resolving a valid symbol from an installed plugin does not return the placeholder") + public void resolvingSymbolFromPlugin() throws Throwable { + rjr.then(SymbolJenkinsTest::_resolvingSymbolFromPlugin); + } + + private static void _resolvingSymbolFromPlugin(JenkinsRule j) { + String symbol = Symbol.get(new SymbolRequest.Builder() + .withName("app-bar") + .withPluginName("design-library") + .build() + ); + assertThat(symbol, not(containsString(Symbol.PLACEHOLDER_MATCHER))); + } +}