From 51c7659db811d47f10c80a9bee6d86ee3ba46304 Mon Sep 17 00:00:00 2001 From: Jochen Delabie Date: Thu, 16 Jul 2026 14:53:33 +0200 Subject: [PATCH 1/3] Migrate TestingBotTestEmbed to Jakarta (StaplerRequest2) + fix stale-render bug TestingBotTestEmbed.doIndex was the last javax.servlet / StaplerRequest usage. Migrate to StaplerRequest2 / StaplerResponse2 / jakarta.servlet.ServletException (the stack the plugin already builds against under parent 6.x / jenkins 2.541.3). Also fix the shared-mutable-field render bug (review finding): doIndex wrote the matched session into the shared 'tbo' field and never reset it, so a later request with an unmatched sessionId re-rendered the previously selected session's media. Resolve into a local and reset the selection each request. Drop the unused Stapler/Logger imports. mvn clean verify green (21 tests, SpotBugs 0); no javax.servlet references remain in src. --- .../java/testingbot/TestingBotTestEmbed.java | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/main/java/testingbot/TestingBotTestEmbed.java b/src/main/java/testingbot/TestingBotTestEmbed.java index 10dc935..805de89 100644 --- a/src/main/java/testingbot/TestingBotTestEmbed.java +++ b/src/main/java/testingbot/TestingBotTestEmbed.java @@ -2,18 +2,14 @@ import hudson.model.Run; import jenkins.model.RunAction2; -import org.kohsuke.stapler.Stapler; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.StaplerResponse2; -import javax.servlet.ServletException; +import jakarta.servlet.ServletException; import java.io.IOException; import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; public class TestingBotTestEmbed implements RunAction2 { - private static final Logger logger = Logger.getLogger(TestingBotTestEmbed.class.getName()); private transient Run run; private List ids; private TestingBotBuildObject tbo; @@ -48,32 +44,28 @@ public void onLoad(Run run) { } /** - * * @param req Standard Request Object * @param rsp Standard Response Object - * @throws IOException Unable to load show.jelly template + * @throws IOException Unable to load the view template */ @SuppressWarnings("unused") // used by stapler - public void doIndex(StaplerRequest req, StaplerResponse rsp) + public void doIndex(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException { String sessionId = req.getParameter("sessionId"); - for (TestingBotBuildObject tbo : this.ids) { - if (tbo.getSessionId().equals(sessionId)) { - this.tbo = tbo; + // Reset the selection each request so a subsequent unmatched request does not render a + // previously-selected session's media (this action is shared across requests for the run). + TestingBotBuildObject selected = null; + for (TestingBotBuildObject candidate : this.ids) { + if (candidate.getSessionId().equals(sessionId)) { + selected = candidate; + break; } } - if (this.tbo != null) { - try { - req.getView(this, "show.jelly").forward(req, rsp); - } catch (ServletException e) { - throw new IOException(e); - } - } else { - try { - req.getView(this, "index.jelly").forward(req, rsp); - } catch (ServletException e) { - throw new IOException(e); - } + this.tbo = selected; + try { + req.getView(this, selected != null ? "show.jelly" : "index.jelly").forward(req, rsp); + } catch (ServletException e) { + throw new IOException(e); } } From 1fd83d69af507ca4e79f539d3afa97b4e3c8ea37 Mon Sep 17 00:00:00 2001 From: Jochen Delabie Date: Thu, 16 Jul 2026 15:13:53 +0200 Subject: [PATCH 2/3] Make embed session selection request-scoped; stop persisting TestingbotTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/main/java/testingbot/TestingBotBuildObject.java | 4 +++- src/main/java/testingbot/TestingBotTestEmbed.java | 11 +++-------- .../testingbot/TestingBotTestEmbed/show.jelly | 7 ++++--- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/main/java/testingbot/TestingBotBuildObject.java b/src/main/java/testingbot/TestingBotBuildObject.java index 0bc7441..eedb954 100644 --- a/src/main/java/testingbot/TestingBotBuildObject.java +++ b/src/main/java/testingbot/TestingBotBuildObject.java @@ -11,7 +11,9 @@ public class TestingBotBuildObject implements Serializable { private String testName; private final boolean isPassed; private String authHash; - private TestingbotTest test; + // Transient: only used in the constructor to derive environmentName, and never read afterwards. + // Persisting it would fail the JEP-200 class filter (TestingbotTest is not marshalable) and bloat build.xml. + private transient TestingbotTest test; private String environmentName; public TestingBotBuildObject(String sessionId, String className, String testName, boolean isPassed, String authHash, TestingbotTest test) { diff --git a/src/main/java/testingbot/TestingBotTestEmbed.java b/src/main/java/testingbot/TestingBotTestEmbed.java index 805de89..05a6a5a 100644 --- a/src/main/java/testingbot/TestingBotTestEmbed.java +++ b/src/main/java/testingbot/TestingBotTestEmbed.java @@ -12,7 +12,6 @@ public class TestingBotTestEmbed implements RunAction2 { private transient Run run; private List ids; - private TestingBotBuildObject tbo; public TestingBotTestEmbed(List ids) { this.ids = ids; @@ -52,8 +51,6 @@ public void onLoad(Run run) { public void doIndex(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException { String sessionId = req.getParameter("sessionId"); - // Reset the selection each request so a subsequent unmatched request does not render a - // previously-selected session's media (this action is shared across requests for the run). TestingBotBuildObject selected = null; for (TestingBotBuildObject candidate : this.ids) { if (candidate.getSessionId().equals(sessionId)) { @@ -61,7 +58,9 @@ public void doIndex(StaplerRequest2 req, StaplerResponse2 rsp) break; } } - this.tbo = selected; + // Pass the selection through the request (this action is shared across concurrent requests, + // so a per-instance field would let requests overwrite each other's selected session). + req.setAttribute("tbo", selected); try { req.getView(this, selected != null ? "show.jelly" : "index.jelly").forward(req, rsp); } catch (ServletException e) { @@ -73,10 +72,6 @@ public Run getRun() { return run; } - public TestingBotBuildObject getTbo() { - return tbo; - } - public List getSessionIds() { return ids; } diff --git a/src/main/resources/testingbot/TestingBotTestEmbed/show.jelly b/src/main/resources/testingbot/TestingBotTestEmbed/show.jelly index 9daf2a1..1f23d1e 100644 --- a/src/main/resources/testingbot/TestingBotTestEmbed/show.jelly +++ b/src/main/resources/testingbot/TestingBotTestEmbed/show.jelly @@ -5,11 +5,12 @@ -

${it.tbo.testName}

- View on TestingBot + +

${tbo.testName}

+ View on TestingBot
- +
From 0a78b1768d4db27931475f82f599d864513de3f7 Mon Sep 17 00:00:00 2001 From: Jochen Delabie Date: Thu, 16 Jul 2026 15:16:20 +0200 Subject: [PATCH 3/3] Drop the unused TestingbotTest field entirely (fix SE_TRANSIENT_FIELD_NOT_RESTORED) Making the field transient tripped SpotBugs SE_TRANSIENT_FIELD_NOT_RESTORED. Since the TestingbotTest is only consumed by the constructor to derive environmentName and is never read afterwards, don't store it at all (drop the field and the unused getTest/setTest). Resolves the JEP-200 marshal failure without a suppression. mvn clean verify green (SpotBugs 0, 21 tests). --- .../testingbot/TestingBotBuildObject.java | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/src/main/java/testingbot/TestingBotBuildObject.java b/src/main/java/testingbot/TestingBotBuildObject.java index eedb954..100c02f 100644 --- a/src/main/java/testingbot/TestingBotBuildObject.java +++ b/src/main/java/testingbot/TestingBotBuildObject.java @@ -11,9 +11,6 @@ public class TestingBotBuildObject implements Serializable { private String testName; private final boolean isPassed; private String authHash; - // Transient: only used in the constructor to derive environmentName, and never read afterwards. - // Persisting it would fail the JEP-200 class filter (TestingbotTest is not marshalable) and bloat build.xml. - private transient TestingbotTest test; private String environmentName; public TestingBotBuildObject(String sessionId, String className, String testName, boolean isPassed, String authHash, TestingbotTest test) { @@ -22,8 +19,9 @@ public TestingBotBuildObject(String sessionId, String className, String testName this.testName = testName; this.isPassed = isPassed; this.authHash = authHash; - this.test = test; - this.environmentName = test.getBrowser() + " | " + test.getOs(); + // TestingbotTest is only used here to derive the environment label; it is not stored (it is not + // marshalable under the JEP-200 class filter, and nothing reads it afterwards). + this.environmentName = test == null ? "" : test.getBrowser() + " | " + test.getOs(); } /** @@ -83,20 +81,6 @@ public boolean getIsPassed() { return isPassed; } - /** - * @return the test - */ - public TestingbotTest getTest() { - return test; - } - - /** - * @param test the test to set - */ - public void setTest(TestingbotTest test) { - this.test = test; - } - /** * @return the authHash */