Skip to content

Commit 1fd83d6

Browse files
Make embed session selection request-scoped; stop persisting TestingbotTest
Review finding: TestingBotTestEmbed.doIndex stored the selected session in a shared per-action field that concurrent requests could overwrite (wrong session's media/authHash). Pass the selection through a request attribute (request-scoped) that show.jelly reads; drop the shared 'tbo' field/getter. While verifying the render end-to-end, found that persisting the action failed the JEP-200 class filter ('Refusing to marshal com.testingbot.models.TestingbotTest') — so the embedded-reports feature could not be saved at all. TestingBotBuildObject's 'test' field is only used in the constructor to derive environmentName and is never read afterwards, so make it transient. Verified on a running Jenkins: the action now saves (build.xml contains it) and both the show and index views render.
1 parent 51c7659 commit 1fd83d6

3 files changed

Lines changed: 10 additions & 12 deletions

File tree

‎src/main/java/testingbot/TestingBotBuildObject.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ public class TestingBotBuildObject implements Serializable {
1111
private String testName;
1212
private final boolean isPassed;
1313
private String authHash;
14-
private TestingbotTest test;
14+
// Transient: only used in the constructor to derive environmentName, and never read afterwards.
15+
// Persisting it would fail the JEP-200 class filter (TestingbotTest is not marshalable) and bloat build.xml.
16+
private transient TestingbotTest test;
1517
private String environmentName;
1618

1719
public TestingBotBuildObject(String sessionId, String className, String testName, boolean isPassed, String authHash, TestingbotTest test) {

‎src/main/java/testingbot/TestingBotTestEmbed.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
public class TestingBotTestEmbed implements RunAction2 {
1313
private transient Run run;
1414
private List<TestingBotBuildObject> ids;
15-
private TestingBotBuildObject tbo;
1615

1716
public TestingBotTestEmbed(List<TestingBotBuildObject> ids) {
1817
this.ids = ids;
@@ -52,16 +51,16 @@ public void onLoad(Run<?, ?> run) {
5251
public void doIndex(StaplerRequest2 req, StaplerResponse2 rsp)
5352
throws IOException {
5453
String sessionId = req.getParameter("sessionId");
55-
// Reset the selection each request so a subsequent unmatched request does not render a
56-
// previously-selected session's media (this action is shared across requests for the run).
5754
TestingBotBuildObject selected = null;
5855
for (TestingBotBuildObject candidate : this.ids) {
5956
if (candidate.getSessionId().equals(sessionId)) {
6057
selected = candidate;
6158
break;
6259
}
6360
}
64-
this.tbo = selected;
61+
// Pass the selection through the request (this action is shared across concurrent requests,
62+
// so a per-instance field would let requests overwrite each other's selected session).
63+
req.setAttribute("tbo", selected);
6564
try {
6665
req.getView(this, selected != null ? "show.jelly" : "index.jelly").forward(req, rsp);
6766
} catch (ServletException e) {
@@ -73,10 +72,6 @@ public Run getRun() {
7372
return run;
7473
}
7574

76-
public TestingBotBuildObject getTbo() {
77-
return tbo;
78-
}
79-
8075
public List<TestingBotBuildObject> getSessionIds() {
8176
return ids;
8277
}

‎src/main/resources/testingbot/TestingBotTestEmbed/show.jelly‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
<st:include page="sidepanel.jelly" it="${it.run}" optional="true" />
66
</l:side-panel>
77
<l:main-panel>
8-
<h1>${it.tbo.testName}</h1>
9-
<a href="https://testingbot.com/tests/${it.tbo.sessionId}?auth=${it.tbo.authHash}&amp;ref=jenkins" target="_blank">View on TestingBot</a>
8+
<j:set var="tbo" value="${request.getAttribute('tbo')}"/>
9+
<h1>${tbo.testName}</h1>
10+
<a href="https://testingbot.com/tests/${tbo.sessionId}?auth=${tbo.authHash}&amp;ref=jenkins" target="_blank">View on TestingBot</a>
1011
<l:ajax>
1112
<div>
12-
<iframe src="https://testingbot.com/mini/?id=${it.tbo.sessionId}&amp;auth=${it.tbo.authHash}&amp;ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe>
13+
<iframe src="https://testingbot.com/mini/?id=${tbo.sessionId}&amp;auth=${tbo.authHash}&amp;ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe>
1314
</div>
1415
</l:ajax>
1516
</l:main-panel>

0 commit comments

Comments
 (0)