From 982f944ad7e1c3ba178e83f98535fd7f7f2d5c6a Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Jul 2023 12:36:38 +0200 Subject: [PATCH 1/5] [SECURITY-3033] Submit POST requests as needed --- .../hudson/plugins/rebuild/Root.java | 12 -- .../rebuild/AbstractRebuildAction.java | 69 ++++++ .../sonyericsson/rebuild/RebuildAction.java | 199 ++++++------------ .../rebuild/RebuildActionFactory.java | 16 +- .../RebuildLastCompletedBuildAction.java | 62 +++++- .../rebuild/RebuildProjectActionFactory.java | 2 +- .../com/sonyericsson/rebuild/Rebuilder.java | 12 +- .../AbstractRebuildAction/action.jelly | 6 + 8 files changed, 204 insertions(+), 174 deletions(-) delete mode 100644 src/main/java/com/sonyericsson/hudson/plugins/rebuild/Root.java create mode 100644 src/main/java/com/sonyericsson/rebuild/AbstractRebuildAction.java create mode 100644 src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly diff --git a/src/main/java/com/sonyericsson/hudson/plugins/rebuild/Root.java b/src/main/java/com/sonyericsson/hudson/plugins/rebuild/Root.java deleted file mode 100644 index 51aa9be5..00000000 --- a/src/main/java/com/sonyericsson/hudson/plugins/rebuild/Root.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.sonyericsson.hudson.plugins.rebuild; - -import com.sonyericsson.rebuild.RebuildAction; -/** - * Root class. - * This class is declared only for - * backward compatibility. - * - * @author Shemeer S; - */ -public class Root extends RebuildAction { -} diff --git a/src/main/java/com/sonyericsson/rebuild/AbstractRebuildAction.java b/src/main/java/com/sonyericsson/rebuild/AbstractRebuildAction.java new file mode 100644 index 00000000..fcad789e --- /dev/null +++ b/src/main/java/com/sonyericsson/rebuild/AbstractRebuildAction.java @@ -0,0 +1,69 @@ +package com.sonyericsson.rebuild; + +import edu.umd.cs.findbugs.annotations.CheckForNull; +import hudson.matrix.MatrixRun; +import hudson.model.Action; +import hudson.model.Item; +import hudson.model.Job; +import hudson.model.Queue; +import hudson.model.Run; + +public abstract class AbstractRebuildAction implements Action { + + @Override + public String getIconFileName() { + if (isRebuildAvailable()) { + return "clock.png"; + } else { + return null; + } + } + + /** + * Method for checking whether the rebuild functionality would be available + * for build. + * + * @return boolean + */ + public boolean isRebuildAvailable() { + Job project = getProject(); + return project != null + && project.hasPermission(Item.BUILD) + && project.isBuildable() + && project instanceof Queue.Task + && !isMatrixRun() + && !isRebuildDisabled(); + + } + + // Jelly + public abstract String getTaskUrl(); + + // Jelly + public abstract boolean isRequiresPOST(); + + private boolean isRebuildDisabled() { + RebuildSettings settings = getProject().getProperty(RebuildSettings.class); + return settings != null && settings.getRebuildDisabled(); + } + + /** + * Method will return current project. + * + * @return currentProject. + */ + public abstract Job getProject(); + + @CheckForNull + protected abstract Run getRun(); + + /** + * Method for checking whether current build is sub job(MatrixRun) of Matrix + * build. + * + * @return boolean + */ + public boolean isMatrixRun() { + return getRun() instanceof MatrixRun; + } +} diff --git a/src/main/java/com/sonyericsson/rebuild/RebuildAction.java b/src/main/java/com/sonyericsson/rebuild/RebuildAction.java index 75966cd3..336f3a2e 100644 --- a/src/main/java/com/sonyericsson/rebuild/RebuildAction.java +++ b/src/main/java/com/sonyericsson/rebuild/RebuildAction.java @@ -38,7 +38,6 @@ import java.util.List; import java.util.Set; -import hudson.matrix.MatrixRun; import hudson.model.BooleanParameterValue; import hudson.model.Cause; import hudson.model.CauseAction; @@ -55,13 +54,14 @@ import hudson.model.RunParameterValue; import hudson.model.StringParameterValue; import jenkins.model.Jenkins; +import jenkins.model.RunAction2; import net.sf.json.JSONArray; import net.sf.json.JSONNull; import net.sf.json.JSONObject; -import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.interceptor.RequirePOST; /** * Rebuild RootAction implementation class. This class will basically reschedule @@ -69,18 +69,14 @@ * * @author Shemeer S; */ -public class RebuildAction implements Action { +public class RebuildAction extends AbstractRebuildAction implements RunAction2 { private static final String SVN_TAG_PARAM_CLASS = "hudson.scm.listtagsparameter.ListSubversionTagsParameterValue"; + private /* quasi-final */ Run run; /* * All the below transient variables are declared only for backward * compatibility of the rebuild plugin. */ - private transient String rebuildurl = "rebuild"; - private transient String parameters = "rebuildParam"; - private transient String p = "parameter"; - private transient Run build; - private transient ParametersDefinitionProperty pdp; private static final String PARAMETERIZED_URL = "parameterized"; /** * Rebuild Descriptor. @@ -90,52 +86,8 @@ public class RebuildAction implements Action { /** * RebuildAction constructor. */ - public RebuildAction() { - } - - /** - * Getter method for pdp. - * - * @return pdp. - */ - public ParametersDefinitionProperty getPdp() { - return pdp; - } - - /** - * Getter method for build. - * - * @return build. - */ - public Run getBuild() { - return build; - } - - /** - * Getter method for p. - * - * @return p. - */ - public String getP() { - return p; - } - - /** - * Getter method for parameters. - * - * @return parameters. - */ - public String getParameters() { - return parameters; - } - - /** - * Getter method for rebuildurl. - * - * @return rebuildurl. - */ - public String getRebuildurl() { - return rebuildurl; + public RebuildAction(Run run) { + this.run = run; } /** @@ -147,50 +99,55 @@ public boolean isRememberPasswordEnabled() { return DESCRIPTOR.getRebuildConfiguration().isRememberPasswordEnabled(); } - /** - * Method will return current project. - * - * @return currentProject. - */ - public Job getProject() { - if (build != null) { - return build.getParent(); + @Override + public String getDisplayName() { + if (isRebuildAvailable()) { + return "Rebuild"; + } else { + return null; } + } - Job currentProject = null; - StaplerRequest request = Stapler.getCurrentRequest(); - if (request != null) { - currentProject = request.findAncestorObject(Job.class); - } + @Override + public String getUrlName() { + return "rebuild"; + } - return currentProject; + @Override + public Run getRun() { + return run; } + /** + * Decouple {@link #getUrlName()} from the actual URL, otherwise we cannot customize the target. + * @return + */ @Override - public String getIconFileName() { + public String getTaskUrl() { if (isRebuildAvailable()) { - return "clock.png"; + if (!isRequiresPOST()) { + return "rebuild/parameterized"; + } + return "rebuild/"; // trailing / needed to prevent redirect to 405 } else { return null; } } @Override - public String getDisplayName() { - if (isRebuildAvailable()) { - return "Rebuild"; - } else { - return null; - } + public Job getProject() { + return run.getParent(); } - @Override - public String getUrlName() { - if (isRebuildAvailable()) { - return "rebuild"; - } else { - return null; + public boolean isRequiresPOST() { + if (run != null) { + ParametersAction paramAction = run.getAction(ParametersAction.class); + if (paramAction != null) { + RebuildSettings settings = getProject().getProperty(RebuildSettings.class); + return settings != null && settings.getAutoRebuild(); + } } + return true; } /** @@ -199,22 +156,22 @@ public String getUrlName() { * * @param request StaplerRequest the request. * @param response StaplerResponse the response handler. - * @throws IOException in case of Stapler issues + * @throws java.io.IOException in case of Stapler issues */ + @RequirePOST public void doIndex(StaplerRequest request, StaplerResponse response) throws IOException { - Run currentBuild = request.findAncestorObject(Run.class); - if (currentBuild != null) { - ParametersAction paramAction = currentBuild.getAction(ParametersAction.class); + if (run != null) { + ParametersAction paramAction = run.getAction(ParametersAction.class); if (paramAction != null) { - RebuildSettings settings = (RebuildSettings)getProject().getProperty(RebuildSettings.class); + RebuildSettings settings = getProject().getProperty(RebuildSettings.class); if (settings != null && settings.getAutoRebuild() || request.getParameter("autorebuild") != null) { - parameterizedRebuild(currentBuild, response); + parameterizedRebuild(run, response); } else { response.sendRedirect(PARAMETERIZED_URL); } } else { - nonParameterizedRebuild(currentBuild, response); + nonParameterizedRebuild(run, response); } } } @@ -233,9 +190,9 @@ public void parameterizedRebuild(Run currentBuild, StaplerResponse response) thr project.checkPermission(Item.BUILD); if (isRebuildAvailable()) { - List actions = constructRebuildActions(build, currentBuild.getAction(ParametersAction.class)); + List actions = constructRebuildActions(run, currentBuild.getAction(ParametersAction.class)); - Jenkins.get().getQueue().schedule2((Queue.Task) build.getParent(), 0, actions); + Jenkins.get().getQueue().schedule2((Queue.Task) run.getParent(), 0, actions); response.sendRedirect("../../"); } } @@ -251,7 +208,7 @@ public void nonParameterizedRebuild(Run currentBuild, StaplerResponse response) throws IOException { getProject().checkPermission(Item.BUILD); - List actions = constructRebuildActions(build, null); + List actions = constructRebuildActions(run, null); Jenkins.get().getQueue().schedule2((Queue.Task) currentBuild.getParent(), 0, actions); response.sendRedirect("../../"); } @@ -276,11 +233,10 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl req.getView(this, "index.jelly").forward(req, rsp); return; } - build = req.findAncestorObject(Run.class); - ParametersDefinitionProperty paramDefProp = build.getParent().getProperty( + ParametersDefinitionProperty paramDefProp = run.getParent().getProperty( ParametersDefinitionProperty.class); List values = new ArrayList<>(); - ParametersAction paramAction = build.getAction(ParametersAction.class); + ParametersAction paramAction = run.getAction(ParametersAction.class); JSONObject formData = req.getSubmittedForm(); if (!formData.isEmpty()) { JSONArray a = JSONArray.fromObject(formData.get("parameter")); @@ -309,8 +265,8 @@ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws Servl } } - List actions = constructRebuildActions(build, new ParametersAction(values)); - Jenkins.get().getQueue().schedule2((Queue.Task) build.getParent(), 0, actions); + List actions = constructRebuildActions(run, new ParametersAction(values)); + Jenkins.get().getQueue().schedule2((Queue.Task) run.getParent(), 0, actions); rsp.sendRedirect("../../"); } @@ -362,45 +318,6 @@ private void copyRebuildDispatcherActions(Run fromBuild, List acti actions.addAll(propagatingActions); } - /** - * Method for checking whether current build is sub job(MatrixRun) of Matrix - * build. - * - * @return boolean - */ - public boolean isMatrixRun() { - StaplerRequest request = Stapler.getCurrentRequest(); - if (request != null) { - build = request.findAncestorObject(Run.class); - if (build != null && build instanceof MatrixRun) { - return true; - } - } - return false; - } - - /** - * Method for checking whether the rebuild functionality would be available - * for build. - * - * @return boolean - */ - public boolean isRebuildAvailable() { - Job project = getProject(); - return project != null - && project.hasPermission(Item.BUILD) - && project.isBuildable() - && project instanceof Queue.Task - && !isMatrixRun() - && !isRebuildDisabled(); - - } - - private boolean isRebuildDisabled() { - RebuildSettings settings = (RebuildSettings)getProject().getProperty(RebuildSettings.class); - return settings != null && settings.getRebuildDisabled(); - } - /** * Method for getting the ParameterValue instance from ParameterDefinition * or ParamterAction. @@ -516,4 +433,14 @@ public RebuildParameterPage getRebuildParameterPage(ParameterValue value) { // So Jelly fallback could occur. return null; } + + @Override + public void onAttached(Run r) { + this.run = r; + } + + @Override + public void onLoad(Run r) { + this.run = r; + } } diff --git a/src/main/java/com/sonyericsson/rebuild/RebuildActionFactory.java b/src/main/java/com/sonyericsson/rebuild/RebuildActionFactory.java index e0fc9a37..b72d86d7 100644 --- a/src/main/java/com/sonyericsson/rebuild/RebuildActionFactory.java +++ b/src/main/java/com/sonyericsson/rebuild/RebuildActionFactory.java @@ -28,10 +28,10 @@ import hudson.model.Action; import hudson.model.Queue; import hudson.model.Run; -import hudson.model.TransientBuildActionFactory; import jenkins.model.Jenkins; import java.util.Collection; +import jenkins.model.TransientActionFactory; import static java.util.Collections.emptyList; import static java.util.Collections.singleton; @@ -40,7 +40,12 @@ * Enables rebuild for builds that ran before installing the rebuild plugin. */ @Extension -public class RebuildActionFactory extends TransientBuildActionFactory { +public class RebuildActionFactory extends TransientActionFactory { + + @Override + public Class type() { + return Run.class; + } @Override public Collection createFor(Run build) { @@ -52,7 +57,10 @@ public Collection createFor(Run build) { return emptyList(); } - boolean hasRebuildAction = build.getAction(RebuildAction.class) != null; + + // These actions used to be permanently attached to the Run + // Build#getActions is deprecated but the only way to prevent a stack overflow here + boolean hasRebuildAction = build.getActions().stream().anyMatch(it -> it instanceof RebuildAction); if (hasRebuildAction) { return emptyList(); } @@ -62,6 +70,6 @@ public Collection createFor(Run build) { return emptyList(); } } - return singleton(new RebuildAction()); + return singleton(new RebuildAction(build)); } } diff --git a/src/main/java/com/sonyericsson/rebuild/RebuildLastCompletedBuildAction.java b/src/main/java/com/sonyericsson/rebuild/RebuildLastCompletedBuildAction.java index 0d30517b..482bd5e0 100644 --- a/src/main/java/com/sonyericsson/rebuild/RebuildLastCompletedBuildAction.java +++ b/src/main/java/com/sonyericsson/rebuild/RebuildLastCompletedBuildAction.java @@ -23,34 +23,74 @@ */ package com.sonyericsson.rebuild; -import hudson.model.Job; +import hudson.model.AbstractProject; +import hudson.model.Run; /** * Reschedules last completed build for the project if available. * Otherwise it behaves as if the user clicked on the build now button. */ -public class RebuildLastCompletedBuildAction extends RebuildAction { +public class RebuildLastCompletedBuildAction extends AbstractRebuildAction { + + private final AbstractProject project; + + public RebuildLastCompletedBuildAction(AbstractProject project) { + this.project = project; + } @Override - public String getUrlName() { - Job project = getProject(); - if (project == null) { - return null; - } + public AbstractProject getProject() { + return project; + } + @Override + protected Run getRun() { + return null; + } + + @Override + public String getUrlName() { boolean isBuildable = project.isBuildable(); - boolean hasCompletedBuild = project.getLastCompletedBuild() != null; if (isBuildable) { - if (hasCompletedBuild) { - return "lastCompletedBuild/rebuild"; + final Run lastCompletedBuild = project.getLastCompletedBuild(); + if (lastCompletedBuild != null) { + final RebuildAction action = lastCompletedBuild.getAction(RebuildAction.class); + // TODO This will have unexpected results if the job configuration changed between link rendering + // and when the user clicks. Seems preferable to rebuilding a "wrong" build (finished since link was + // rendered though). + return "lastCompletedBuild/" + action.getTaskUrl(); } else { - return "build?delay=0sec"; + // This used to link to "Build Now" but that doesn't work for parameterized builds + return null; } } else { return null; } } + @Override + public String getIconFileName() { + return super.getIconFileName(); + } + + @Override + public String getTaskUrl() { + return getUrlName(); + } + + @Override + public boolean isRequiresPOST() { + final Run lastCompletedBuild = project.getLastCompletedBuild(); + if (lastCompletedBuild == null) { + return false; + } + final RebuildAction action = lastCompletedBuild.getAction(RebuildAction.class); + // TODO This will have unexpected results if the job configuration changed between link rendering + // and when the user clicks. Seems preferable to rebuilding a "wrong" build (finished since link was + // rendered though). + return action.isRequiresPOST(); + } + @Override public String getDisplayName() { return "Rebuild Last"; diff --git a/src/main/java/com/sonyericsson/rebuild/RebuildProjectActionFactory.java b/src/main/java/com/sonyericsson/rebuild/RebuildProjectActionFactory.java index c00b37e4..a6fb72cd 100644 --- a/src/main/java/com/sonyericsson/rebuild/RebuildProjectActionFactory.java +++ b/src/main/java/com/sonyericsson/rebuild/RebuildProjectActionFactory.java @@ -47,6 +47,6 @@ public Collection createFor(AbstractProject abstractProject) { if (abstractProject instanceof MatrixConfiguration) { return emptyList(); } - return singleton(new RebuildLastCompletedBuildAction()); + return singleton(new RebuildLastCompletedBuildAction(abstractProject)); } } diff --git a/src/main/java/com/sonyericsson/rebuild/Rebuilder.java b/src/main/java/com/sonyericsson/rebuild/Rebuilder.java index 04bc860a..8c435423 100644 --- a/src/main/java/com/sonyericsson/rebuild/Rebuilder.java +++ b/src/main/java/com/sonyericsson/rebuild/Rebuilder.java @@ -29,7 +29,6 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.model.listeners.RunListener; -import jenkins.model.Jenkins; /** * Runtime Listner class which allows the user to rebuild the parameterized build. @@ -48,15 +47,8 @@ public Rebuilder() { @Override public void onCompleted(Run build, @NonNull TaskListener listener) { - for (RebuildValidator rebuildValidator : Jenkins.get(). - getExtensionList(RebuildValidator.class)) { - if (rebuildValidator.isApplicable(build)) { - return; - } - } - RebuildAction rebuildAction = new RebuildAction(); - // TODO what is the purpose of this? If eligible, RebuildActionFactory would already be adding it anyway (without saving anything to XML). - build.addAction(rebuildAction); + // This used to attach a RebuildAction to the build, but RebuildActionFactory takes care of this. + // Left for compatibility. } } diff --git a/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly b/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly new file mode 100644 index 00000000..47247bdc --- /dev/null +++ b/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly @@ -0,0 +1,6 @@ + + + + + + From 2f9e298a25299bb8f67e018b8c51b4eca7a67752 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Jul 2023 14:33:30 +0200 Subject: [PATCH 2/5] Remove redundant null checks --- src/main/java/com/sonyericsson/rebuild/RebuildAction.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/main/java/com/sonyericsson/rebuild/RebuildAction.java b/src/main/java/com/sonyericsson/rebuild/RebuildAction.java index 336f3a2e..42fcc6cb 100644 --- a/src/main/java/com/sonyericsson/rebuild/RebuildAction.java +++ b/src/main/java/com/sonyericsson/rebuild/RebuildAction.java @@ -184,9 +184,6 @@ public void doIndex(StaplerRequest request, StaplerResponse response) throws IOE */ public void parameterizedRebuild(Run currentBuild, StaplerResponse response) throws IOException { Job project = getProject(); - if (project == null) { - return; - } project.checkPermission(Item.BUILD); if (isRebuildAvailable()) { @@ -223,9 +220,6 @@ public void nonParameterizedRebuild(Run currentBuild, StaplerResponse */ public void doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws ServletException, IOException { Job project = getProject(); - if (project == null) { - return; - } project.checkPermission(Item.BUILD); if (isRebuildAvailable()) { if (!req.getMethod().equals("POST")) { From 5ee8da5293dc67ea936e3258f4aa7c83a1bff78e Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Fri, 14 Jul 2023 14:53:33 +0200 Subject: [PATCH 3/5] Adapt tests to no longer directly navigate to 'rebuild' URL --- .../rebuild/RebuildValidatorTest.java | 79 ++++++++++--------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java b/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java index 3442f452..30daff77 100644 --- a/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java +++ b/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java @@ -23,7 +23,11 @@ */ package com.sonyericsson.rebuild; +import java.net.URL; +import org.htmlunit.HttpMethod; +import org.htmlunit.Page; import org.htmlunit.WebAssert; +import org.htmlunit.WebRequest; import org.htmlunit.html.HtmlAnchor; import org.htmlunit.html.HtmlPage; @@ -204,11 +208,12 @@ public void testWhenProjectWithNoParamsDefinedThenRebuildofBuildWithParamsShould FreeStyleProject project = j.createFreeStyleProject(); // Build (#1) - project.scheduleBuild2(0, new Cause.UserIdCause(), - new ParametersAction(new StringParameterValue("name", "ABC"))) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.UserIdCause(), + new ParametersAction(new StringParameterValue("name", "ABC"))) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild"); + final WebClient webClient = j.createWebClient(); + final HtmlPage buildPage = webClient.getPage(freeStyleBuild); + final HtmlPage rebuildConfigPage = buildPage.getAnchorByText("Rebuild").click(); WebAssert.assertElementPresentByXPath(rebuildConfigPage, "//div[@name='parameter']/input[@value='ABC']"); } @@ -233,8 +238,12 @@ public void testAutoRebuildAsRequestParameter() project.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(new StringParameterValue("name", "ABC"))) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild?autorebuild"); + try (WebClient webClient = j.createWebClient()) { + final WebRequest webRequest = new WebRequest( + webClient.createCrumbedUrl(project.getUrl() + "1/rebuild/?autorebuild"), + HttpMethod.POST); + webClient.getPage(webRequest); + } Run r = project.getBuild("2"); assertNotNull(r); ParametersAction paramAction = r.getAction(ParametersAction.class); @@ -258,11 +267,11 @@ public void testWhenProjectWithParamsThenRebuildProjectExecutesRebuildOfLastBuil new StringParameterDefinition("name", "defaultValue"))); // Build (#1) - project.scheduleBuild2(0, new Cause.UserIdCause(), - new ParametersAction(new StringParameterValue("name", "test"))) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.UserIdCause(), + new ParametersAction(new StringParameterValue("name", "test"))) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild"); + final HtmlPage buildPage = j.createWebClient().getPage(freeStyleBuild); + HtmlPage rebuildConfigPage = buildPage.getAnchorByText("Rebuild").click(); // Rebuild (#2) j.submit(rebuildConfigPage.getFormByName("config")); @@ -270,8 +279,8 @@ public void testWhenProjectWithParamsThenRebuildProjectExecutesRebuildOfLastBuil WebAssert.assertLinkPresentWithText(projectPage, "Rebuild Last"); HtmlAnchor rebuildHref = projectPage.getAnchorByText("Rebuild Last"); - assertEquals("Rebuild Last should point to the second build", "/jenkins/" - + project.getUrl() + "lastCompletedBuild/rebuild", + assertEquals("Rebuild Last should point to the second build", + "lastCompletedBuild/rebuild/parameterized", rebuildHref.getHrefAttribute()); } @@ -290,11 +299,11 @@ public void testWhenProjectWithCauseThenCauseIsCopiedAndUserIdCauseAdded() new StringParameterDefinition("name", "defaultValue"))); // Build (#1) - project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), - new ParametersAction(new StringParameterValue("name", "test"))) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), + new ParametersAction(new StringParameterValue("name", "test"))) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild"); + final HtmlPage buildPage = j.createWebClient().getPage(freeStyleBuild); + HtmlPage rebuildConfigPage = buildPage.getAnchorByText("Rebuild").click(); // Rebuild (#2) j.submit(rebuildConfigPage.getFormByName("config")); @@ -342,13 +351,13 @@ public void testWhenProjectWithChainedRebuildsThenSingleUserIdCauseAndReplayCaus new StringParameterDefinition("name", "defaultValue"))); // Build (#1) - project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), - new ParametersAction(new StringParameterValue("name", "test"))) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), + new ParametersAction(new StringParameterValue("name", "test"))) .get(); // Rebuild (#2) WebClient webClient = j.createWebClient(); - HtmlPage rebuildConfigPage1 = webClient.getPage(project, "1/rebuild"); + HtmlPage rebuildConfigPage1 = webClient.getPage(freeStyleBuild).getAnchorByText("Rebuild").click(); j.submit(rebuildConfigPage1.getFormByName("config")); j.createWebClient().getPage(project).getAnchorByText("Rebuild Last") @@ -359,7 +368,7 @@ public void testWhenProjectWithChainedRebuildsThenSingleUserIdCauseAndReplayCaus } // Rebuild (#3) - HtmlPage rebuildConfigPage2 = webClient.getPage(project, "2/rebuild"); + HtmlPage rebuildConfigPage2 = webClient.getPage(project.getBuildByNumber(2)).getAnchorByText("Rebuild").click(); j.submit(rebuildConfigPage2.getFormByName("config")); j.createWebClient().getPage(project).getAnchorByText("Rebuild Last") @@ -456,8 +465,6 @@ public boolean isApplicable(AbstractBuild build) { * Creates a new freestyle project and build with a parameter value whose * type is unknown to rebuild plugin. Rebuild and verify that an no * exception occurs and page is displayed correctly. - * - * {@link RebuildableParameterValue}. * * @throws Exception * Exception @@ -470,13 +477,12 @@ public void testRebuildUnsupportedUnknownParameterValue() throws Exception { new UnsupportedUnknownParameterDefinition("param1", "defaultValue"))); - j.assertBuildStatusSuccess(project.scheduleBuild2(0, + FreeStyleBuild build = j.assertBuildStatusSuccess(project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), new ParametersAction(new UnsupportedUnknownParameterValue( "param1", "value1")))); - FreeStyleBuild build = project.getLastBuild(); // it is trying to fallback and use the - HtmlPage page = wc.getPage(build, "rebuild"); + HtmlPage page = wc.getPage(build).getAnchorByText("Rebuild").click(); // Check the hardcoded description is showing properly. assertTrue(page.asNormalizedText().contains( "Configuration page for UnsupportedUnknownParameterValue")); @@ -484,8 +490,7 @@ public void testRebuildUnsupportedUnknownParameterValue() throws Exception { /** * Creates a new freestyle project and build with a parameter value whose - * type is unknown to rebuild plugin. Verify that rebuild succeeds if that - * parameter value supports {@link RebuildableParameterValue}. + * type is unknown to rebuild plugin. Verify that rebuild succeeds. * * @throws Exception * Exception @@ -504,7 +509,7 @@ public void testRebuildSupportedUnknownParameterValue() throws Exception { new SupportedUnknownParameterValue("param1", "value1")))); FreeStyleBuild build = project.getLastBuild(); - HtmlPage page = wc.getPage(build, "rebuild"); + HtmlPage page = wc.getPage(build).getAnchorByText("Rebuild").click(); assertTrue(page.asNormalizedText(), page.asNormalizedText().contains("This is a mark for test")); } @@ -516,12 +521,11 @@ public void testRebuildDispatcherExtensionActionIsCopied() throws Exception { new StringParameterDefinition("name", "defaultValue"))); // Build (#1) - project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), - new ParametersAction(new StringParameterValue("name", "test")), - new RebuildDispatcherTestAction(true)) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), + new ParametersAction(new StringParameterValue("name", "test")), + new RebuildDispatcherTestAction(true)) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild"); + HtmlPage rebuildConfigPage = j.createWebClient().getPage(freeStyleBuild).getAnchorByText("Rebuild").click(); // Rebuild (#2) j.submit(rebuildConfigPage.getFormByName("config")); @@ -547,12 +551,11 @@ public void testRebuildDispatcherExtensionActionIsNotCopied() throws Exception { new StringParameterDefinition("name", "defaultValue"))); // Build (#1) - project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), - new ParametersAction(new StringParameterValue("name", "test")), - new RebuildDispatcherTestAction(false)) + final FreeStyleBuild freeStyleBuild = project.scheduleBuild2(0, new Cause.RemoteCause("host", "note"), + new ParametersAction(new StringParameterValue("name", "test")), + new RebuildDispatcherTestAction(false)) .get(); - HtmlPage rebuildConfigPage = j.createWebClient().getPage(project, - "1/rebuild"); + HtmlPage rebuildConfigPage = j.createWebClient().getPage(freeStyleBuild).getAnchorByText("Rebuild").click(); // Rebuild (#2) j.submit(rebuildConfigPage.getFormByName("config")); From feedeb3316a99404e0f1a13d161ad98f89725c19 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Wed, 30 Aug 2023 22:45:54 +0200 Subject: [PATCH 4/5] Fix relative links --- .../com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly b/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly index 47247bdc..4680418b 100644 --- a/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly +++ b/src/main/resources/com/sonyericsson/rebuild/AbstractRebuildAction/action.jelly @@ -1,6 +1,6 @@ - + From 67cabb6cd42e0f8060f6e809da6e492580490a19 Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Thu, 31 Aug 2023 08:40:38 +0200 Subject: [PATCH 5/5] Fix test --- .../rebuild/RebuildValidatorTest.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java b/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java index 30daff77..94a56d89 100644 --- a/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java +++ b/src/test/java/com/sonyericsson/rebuild/RebuildValidatorTest.java @@ -23,9 +23,7 @@ */ package com.sonyericsson.rebuild; -import java.net.URL; import org.htmlunit.HttpMethod; -import org.htmlunit.Page; import org.htmlunit.WebAssert; import org.htmlunit.WebRequest; import org.htmlunit.html.HtmlAnchor; @@ -51,7 +49,13 @@ import org.junit.Rule; import org.junit.Test; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.endsWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; @@ -279,9 +283,8 @@ public void testWhenProjectWithParamsThenRebuildProjectExecutesRebuildOfLastBuil WebAssert.assertLinkPresentWithText(projectPage, "Rebuild Last"); HtmlAnchor rebuildHref = projectPage.getAnchorByText("Rebuild Last"); - assertEquals("Rebuild Last should point to the second build", - "lastCompletedBuild/rebuild/parameterized", - rebuildHref.getHrefAttribute()); + assertThat(rebuildHref.getHrefAttribute(), + endsWith("lastCompletedBuild/rebuild/parameterized")); } /**