Skip to content

Commit df5af62

Browse files
akash-manna-skyakash-manna-sky
authored
Tools trend chart should use names and not IDs in legend (#3236)
1 parent 713b914 commit df5af62

File tree

6 files changed

+280
-4
lines changed

6 files changed

+280
-4
lines changed

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/charts/ToolsTrendChart.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
import edu.hm.hafner.echarts.LineSeries.StackedMode;
88
import edu.hm.hafner.echarts.LinesChartModel;
99

10+
import java.util.Collections;
11+
import java.util.Map;
12+
1013
import io.jenkins.plugins.analysis.core.util.AnalysisBuildResult;
1114
import io.jenkins.plugins.echarts.JenkinsPalette;
1215

@@ -16,6 +19,25 @@
1619
* @author Ullrich Hafner
1720
*/
1821
public class ToolsTrendChart implements TrendChart {
22+
private final Map<String, String> toolNames;
23+
24+
/**
25+
* Creates a chart without tool name mappings. Tool IDs will be displayed directly.
26+
*/
27+
public ToolsTrendChart() {
28+
this(Collections.emptyMap());
29+
}
30+
31+
/**
32+
* Creates a chart with tool name mappings.
33+
*
34+
* @param toolNames
35+
* a map from tool IDs to human-readable names
36+
*/
37+
public ToolsTrendChart(final Map<String, String> toolNames) {
38+
this.toolNames = toolNames;
39+
}
40+
1941
@Override
2042
public LinesChartModel create(final Iterable<? extends BuildResult<AnalysisBuildResult>> results,
2143
final ChartModelConfiguration configuration) {
@@ -25,10 +47,11 @@ public LinesChartModel create(final Iterable<? extends BuildResult<AnalysisBuild
2547
var model = new LinesChartModel(lineModel);
2648

2749
int index = 0;
28-
for (String name : lineModel.getDataSetIds()) {
29-
var lineSeries = new LineSeries(name, JenkinsPalette.chartColor(index).normal(),
50+
for (String toolId : lineModel.getDataSetIds()) {
51+
String displayName = toolNames.getOrDefault(toolId, toolId);
52+
var lineSeries = new LineSeries(displayName, JenkinsPalette.chartColor(index).normal(),
3053
StackedMode.SEPARATE_LINES, FilledMode.LINES);
31-
lineSeries.addAll(lineModel.getSeries(name));
54+
lineSeries.addAll(lineModel.getSeries(toolId));
3255
model.addSeries(lineSeries);
3356
index++;
3457
}

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/model/AggregatedTrendAction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ private LinesChartModel createChartModel(final ChartModelConfiguration configura
9595
if (lastBuild == null) {
9696
return new LinesChartModel();
9797
}
98-
return new ToolsTrendChart().create(new CompositeBuildResultsIterable(lastBuild), configuration);
98+
var nameRegistry = ToolNameRegistry.fromBuild(lastBuild);
99+
return new ToolsTrendChart(nameRegistry.asMap()).create(new CompositeBuildResultsIterable(lastBuild), configuration);
99100
}
100101

101102
@Override

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/model/JobAction.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,10 @@ private TrendChart selectChart(final String chartType) {
222222
}
223223
}
224224
if (numberOfTools > 1) {
225+
Run<?, ?> lastBuild = owner.getLastBuild();
226+
if (lastBuild != null) {
227+
return new ToolsTrendChart(ToolNameRegistry.fromBuild(lastBuild).asMap());
228+
}
225229
return new ToolsTrendChart();
226230
}
227231
else {

‎plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package io.jenkins.plugins.analysis.core.model;
2+
3+
import java.util.Collections;
4+
import java.util.HashMap;
5+
import java.util.Map;
6+
7+
import org.apache.commons.lang3.StringUtils;
8+
import org.apache.commons.text.StringEscapeUtils;
9+
10+
import hudson.model.Run;
11+
12+
/**
13+
* Registry that maps tool IDs to their human-readable names. This is used to display tool names instead of IDs in
14+
* trend charts and other visualizations.
15+
*
16+
* @author Akash Manna
17+
*/
18+
public class ToolNameRegistry {
19+
private final Map<String, String> idToNameMap;
20+
21+
/**
22+
* Creates an empty registry.
23+
*/
24+
public ToolNameRegistry() {
25+
this(new HashMap<>());
26+
}
27+
28+
/**
29+
* Creates a registry with the given ID-to-name mapping.
30+
*
31+
* @param idToNameMap
32+
* the mapping of tool IDs to names
33+
*/
34+
public ToolNameRegistry(final Map<String, String> idToNameMap) {
35+
this.idToNameMap = new HashMap<>(idToNameMap);
36+
}
37+
38+
/**
39+
* Creates a registry from the {@link ResultAction}s of a build. Each action provides a tool ID and name, which
40+
* are stored in the registry for later lookup. Names are HTML-escaped at creation time.
41+
*
42+
* @param build
43+
* the build that contains the result actions
44+
*
45+
* @return a registry containing all tool IDs and HTML-escaped names from the build
46+
*/
47+
public static ToolNameRegistry fromBuild(final Run<?, ?> build) {
48+
Map<String, String> mapping = new HashMap<>();
49+
LabelProviderFactory factory = new LabelProviderFactory();
50+
for (ResultAction action : build.getActions(ResultAction.class)) {
51+
String id = action.getId();
52+
String name = action.getName();
53+
if (StringUtils.isBlank(name)) {
54+
name = factory.create(id).getName();
55+
}
56+
mapping.put(id, StringEscapeUtils.escapeHtml4(name));
57+
}
58+
return new ToolNameRegistry(mapping);
59+
}
60+
61+
/**
62+
* Returns the human-readable name for a tool ID. If the ID is not registered, attempts to look up the name from
63+
* the {@link LabelProviderFactory}. If that also fails, returns the ID itself. Names returned are already
64+
* HTML-escaped.
65+
*
66+
* @param id
67+
* the tool ID
68+
*
69+
* @return the HTML-escaped human-readable name, or the escaped ID if no name is found
70+
*/
71+
public String getName(final String id) {
72+
if (idToNameMap.containsKey(id)) {
73+
return idToNameMap.get(id);
74+
}
75+
var labelProvider = new LabelProviderFactory().create(id);
76+
return StringEscapeUtils.escapeHtml4(labelProvider.getName());
77+
}
78+
79+
/**
80+
* Registers a tool ID with its corresponding name. The name will be HTML-escaped before storing.
81+
*
82+
* @param id
83+
* the tool ID
84+
* @param name
85+
* the human-readable name
86+
*/
87+
public void register(final String id, final String name) {
88+
idToNameMap.put(id, StringEscapeUtils.escapeHtml4(name));
89+
}
90+
91+
/**
92+
* Returns whether the registry contains a mapping for the given tool ID.
93+
*
94+
* @param id
95+
* the tool ID
96+
*
97+
* @return {@code true} if the registry contains a mapping for the ID, {@code false} otherwise
98+
*/
99+
public boolean contains(final String id) {
100+
return idToNameMap.containsKey(id);
101+
}
102+
103+
/**
104+
* Returns the number of registered tool IDs.
105+
*
106+
* @return the number of registered IDs
107+
*/
108+
public int size() {
109+
return idToNameMap.size();
110+
}
111+
112+
/**
113+
* Returns the ID-to-name mapping as an immutable map. The names in the returned map are already HTML-escaped.
114+
*
115+
* @return an immutable map from tool IDs to HTML-escaped names
116+
*/
117+
public Map<String, String> asMap() {
118+
return Collections.unmodifiableMap(idToNameMap);
119+
}
120+
}

‎plugin/src/test/java/io/jenkins/plugins/analysis/core/charts/ToolsTrendChartTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.HashSet;
1414
import java.util.List;
1515

16+
import io.jenkins.plugins.analysis.core.model.ToolNameRegistry;
1617
import io.jenkins.plugins.analysis.core.util.AnalysisBuildResult;
1718

1819
import static io.jenkins.plugins.analysis.core.charts.BuildResultStubs.*;
@@ -94,4 +95,41 @@ private boolean hasDuplicates(final List<String> list) {
9495
int sizeWithoutDuplicate = new HashSet<>(list).size();
9596
return list.size() > sizeWithoutDuplicate;
9697
}
98+
99+
@Test
100+
void shouldUseToolNamesInsteadOfIds() {
101+
ToolNameRegistry registry = new ToolNameRegistry();
102+
registry.register(CHECK_STYLE, "CheckStyle Warnings");
103+
registry.register(SPOT_BUGS, "SpotBugs Issues");
104+
105+
var chart = new ToolsTrendChart(registry.asMap());
106+
107+
List<BuildResult<AnalysisBuildResult>> compositeResults = new ArrayList<>();
108+
compositeResults.add(new BuildResult<>(new Build(1), new CompositeBuildResult(List.of(
109+
createAnalysisBuildResult(CHECK_STYLE, 1), createAnalysisBuildResult(SPOT_BUGS, 3)))));
110+
compositeResults.add(new BuildResult<>(new Build(2), new CompositeBuildResult(List.of(
111+
createAnalysisBuildResult(CHECK_STYLE, 2), createAnalysisBuildResult(SPOT_BUGS, 4)))));
112+
113+
var model = chart.create(compositeResults, new ChartModelConfiguration());
114+
115+
assertThatJson(model.getSeries().get(0)).node("name").isEqualTo("CheckStyle Warnings");
116+
assertThatJson(model.getSeries().get(1)).node("name").isEqualTo("SpotBugs Issues");
117+
}
118+
119+
@Test
120+
void shouldEscapeHtmlInToolNames() {
121+
ToolNameRegistry registry = new ToolNameRegistry();
122+
registry.register("custom", "<script>alert('xss')</script>");
123+
124+
var chart = new ToolsTrendChart(registry.asMap());
125+
126+
List<BuildResult<AnalysisBuildResult>> results = new ArrayList<>();
127+
results.add(new BuildResult<>(new Build(1), new CompositeBuildResult(List.of(
128+
createAnalysisBuildResult("custom", 5)))));
129+
130+
var model = chart.create(results, new ChartModelConfiguration());
131+
132+
assertThatJson(model.getSeries().get(0)).node("name")
133+
.isEqualTo("&lt;script&gt;alert('xss')&lt;/script&gt;");
134+
}
97135
}

‎plugin/src/test/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistryTest.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package io.jenkins.plugins.analysis.core.model;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import hudson.model.Run;
6+
7+
import static org.assertj.core.api.Assertions.*;
8+
import static org.mockito.Mockito.*;
9+
10+
/**
11+
* Tests the class {@link ToolNameRegistry}.
12+
*
13+
* @author Akash Manna
14+
*/
15+
class ToolNameRegistryTest {
16+
@Test
17+
void shouldCreateEmptyRegistry() {
18+
ToolNameRegistry registry = new ToolNameRegistry();
19+
20+
assertThat(registry.size()).isEqualTo(0);
21+
assertThat(registry.contains("checkstyle")).isFalse();
22+
}
23+
24+
@Test
25+
void shouldRegisterAndRetrieveNames() {
26+
ToolNameRegistry registry = new ToolNameRegistry();
27+
28+
registry.register("checkstyle", "CheckStyle Warnings");
29+
registry.register("spotbugs", "SpotBugs");
30+
31+
assertThat(registry.size()).isEqualTo(2);
32+
assertThat(registry.contains("checkstyle")).isTrue();
33+
assertThat(registry.contains("spotbugs")).isTrue();
34+
assertThat(registry.getName("checkstyle")).isEqualTo("CheckStyle Warnings");
35+
assertThat(registry.getName("spotbugs")).isEqualTo("SpotBugs");
36+
}
37+
38+
@Test
39+
void shouldEscapeHtmlInNames() {
40+
ToolNameRegistry registry = new ToolNameRegistry();
41+
42+
registry.register("custom", "<script>alert('xss')</script>");
43+
44+
assertThat(registry.getName("custom")).isEqualTo("&lt;script&gt;alert('xss')&lt;/script&gt;");
45+
assertThat(registry.asMap().get("custom")).isEqualTo("&lt;script&gt;alert('xss')&lt;/script&gt;");
46+
}
47+
48+
@Test
49+
void shouldReturnEscapedIdForUnknownId() {
50+
ToolNameRegistry registry = new ToolNameRegistry();
51+
52+
registry.register("unknown", "unknown");
53+
assertThat(registry.getName("unknown")).isEqualTo("unknown");
54+
55+
registry.register("<script>", "<script>");
56+
assertThat(registry.getName("<script>")).isEqualTo("&lt;script&gt;");
57+
}
58+
59+
@Test
60+
void shouldCreateRegistryFromBuild() {
61+
Run<?, ?> build = mock(Run.class);
62+
63+
ResultAction checkstyleAction = mock(ResultAction.class);
64+
when(checkstyleAction.getId()).thenReturn("checkstyle");
65+
when(checkstyleAction.getName()).thenReturn("CheckStyle");
66+
67+
ResultAction spotbugsAction = mock(ResultAction.class);
68+
when(spotbugsAction.getId()).thenReturn("spotbugs");
69+
when(spotbugsAction.getName()).thenReturn("SpotBugs");
70+
71+
when(build.getActions(ResultAction.class)).thenReturn(java.util.List.of(checkstyleAction, spotbugsAction));
72+
73+
ToolNameRegistry registry = ToolNameRegistry.fromBuild(build);
74+
75+
assertThat(registry.size()).isEqualTo(2);
76+
assertThat(registry.getName("checkstyle")).isEqualTo("CheckStyle");
77+
assertThat(registry.getName("spotbugs")).isEqualTo("SpotBugs");
78+
}
79+
80+
@Test
81+
void shouldFallbackToIdForRegisteredIds() {
82+
ToolNameRegistry registry = new ToolNameRegistry();
83+
84+
registry.register("checkstyle", "CheckStyle");
85+
registry.register("unknownToolId", "Unknown Tool");
86+
87+
assertThat(registry.getName("checkstyle")).isEqualTo("CheckStyle");
88+
assertThat(registry.getName("unknownToolId")).isEqualTo("Unknown Tool");
89+
}
90+
}

0 commit comments

Comments
 (0)